1. Patchew
  2. linux
  3. [PATCH v9 00/15] Support AMD Pensando Elba SoC
  4. View patch

[PATCH v9 04/15] dt-bindings: spi: dw: Add AMD Pensando Elba SoC SPI Controller bindings

Brad Larson posted 15 patches 2 years, 7 months ago
There is a newer version of this series
[PATCH v9 04/15] dt-bindings: spi: dw: Add AMD Pensando Elba SoC SPI Controller bindings
Posted by Brad Larson 2 years, 7 months ago 
The AMD Pensando Elba SoC has integrated the DW APB SPI Controller

Signed-off-by: Brad Larson <blarson@amd.com>
---

Changes since v6:
- Define property amd,pensando-elba-syscon
- Move compatible amd,pensando-elba-spi ahead of baikal,bt1-ssi

---
 .../devicetree/bindings/spi/snps,dw-apb-ssi.yaml   | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
index d33b72fabc5d..96b072835de0 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
@@ -37,6 +37,18 @@ allOf:
     else:
       required:
         - interrupts
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: amd,pensando-elba-spi
+    then:
+      properties:
+        amd,pensando-elba-syscon:
+          $ref: /schemas/types.yaml#/definitions/phandle-array
+          description: AMD Pensando Elba SoC system controller
+      required:
+        - amd,pensando-elba-syscon
 
 properties:
   compatible:
@@ -63,6 +75,8 @@ properties:
         const: intel,keembay-ssi
       - description: Intel Thunder Bay SPI Controller
         const: intel,thunderbay-ssi
+      - description: AMD Pensando Elba SoC SPI Controller
+        const: amd,pensando-elba-spi
       - description: Baikal-T1 SPI Controller
         const: baikal,bt1-ssi
       - description: Baikal-T1 System Boot SPI Controller
-- 
2.17.1
Re: [PATCH v9 04/15] dt-bindings: spi: dw: Add AMD Pensando Elba SoC SPI Controller bindings
Posted by Krzysztof Kozlowski 2 years, 7 months ago 
On 19/01/2023 04:51, Brad Larson wrote:
> The AMD Pensando Elba SoC has integrated the DW APB SPI Controller
> 
> Signed-off-by: Brad Larson <blarson@amd.com>
> ---
> 
> Changes since v6:
> - Define property amd,pensando-elba-syscon
> - Move compatible amd,pensando-elba-spi ahead of baikal,bt1-ssi
> 
> ---
>  .../devicetree/bindings/spi/snps,dw-apb-ssi.yaml   | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> index d33b72fabc5d..96b072835de0 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> @@ -37,6 +37,18 @@ allOf:
>      else:
>        required:
>          - interrupts
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: amd,pensando-elba-spi
> +    then:
> +      properties:
> +        amd,pensando-elba-syscon:
> +          $ref: /schemas/types.yaml#/definitions/phandle-array
> +          description: AMD Pensando Elba SoC system controller

And nothing here - neither in commit msg nor here - explains why do you
need it and what is it for.

Define properties in top level "properties:" and here only allow (:
true) or disallow (: false) them.

> +      required:
> +        - amd,pensando-elba-syscon
>  
>  properties:
>    compatible:
> @@ -63,6 +75,8 @@ properties:
>          const: intel,keembay-ssi
>        - description: Intel Thunder Bay SPI Controller
>          const: intel,thunderbay-ssi
> +      - description: AMD Pensando Elba SoC SPI Controller
> +        const: amd,pensando-elba-spi
>        - description: Baikal-T1 SPI Controller
>          const: baikal,bt1-ssi
>        - description: Baikal-T1 System Boot SPI Controller

Best regards,
Krzysztof
Re: [PATCH v9 04/15] dt-bindings: spi: dw: Add AMD Pensando Elba SoC SPI Controller bindings
Posted by Brad Larson 2 years, 7 months ago 
On 19/01/2023 7:55 UTC, Krzysztof Kozlowski wrote:
>On 19/01/2023 04:51, Brad Larson wrote:
>> The AMD Pensando Elba SoC has integrated the DW APB SPI Controller
>> 
...
>>  .../devicetree/bindings/spi/snps,dw-apb-ssi.yaml   | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>> index d33b72fabc5d..96b072835de0 100644
>> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>> @@ -37,6 +37,18 @@ allOf:
>>      else:
>>        required:
>>          - interrupts
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: amd,pensando-elba-spi
>> +    then:
>> +      properties:
>> +        amd,pensando-elba-syscon:
>> +          $ref: /schemas/types.yaml#/definitions/phandle-array
>> +          description: AMD Pensando Elba SoC system controller
>
>And nothing here - neither in commit msg nor here - explains why do you
>need it and what is it for.

Adding property amd,pensando-elba-syscon was a result of this thread:
https://lore.kernel.org/lkml/20220621101159.stvan53rvr6qugna@mobilestation/

>Define properties in top level "properties:" and here only allow (:
>true) or disallow (: false) them.

Ok, will change this if the syscon property remains.

>> +      required:
>> +        - amd,pensando-elba-syscon
>>  
>>  properties:
>>    compatible:
>> @@ -63,6 +75,8 @@ properties:
>>          const: intel,keembay-ssi
>>        - description: Intel Thunder Bay SPI Controller
>>          const: intel,thunderbay-ssi
>> +      - description: AMD Pensando Elba SoC SPI Controller
>> +        const: amd,pensando-elba-spi
>>        - description: Baikal-T1 SPI Controller
>>          const: baikal,bt1-ssi
>>        - description: Baikal-T1 System Boot SPI Controller

Regards,
Brad
Re: [PATCH v9 04/15] dt-bindings: spi: dw: Add AMD Pensando Elba SoC SPI Controller bindings
Posted by Krzysztof Kozlowski 2 years, 7 months ago 
On 24/01/2023 02:57, Brad Larson wrote:
> On 19/01/2023 7:55 UTC, Krzysztof Kozlowski wrote:
>> On 19/01/2023 04:51, Brad Larson wrote:
>>> The AMD Pensando Elba SoC has integrated the DW APB SPI Controller
>>>
> ...
>>>  .../devicetree/bindings/spi/snps,dw-apb-ssi.yaml   | 14 ++++++++++++++
>>>  1 file changed, 14 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>> index d33b72fabc5d..96b072835de0 100644
>>> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>> @@ -37,6 +37,18 @@ allOf:
>>>      else:
>>>        required:
>>>          - interrupts
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: amd,pensando-elba-spi
>>> +    then:
>>> +      properties:
>>> +        amd,pensando-elba-syscon:
>>> +          $ref: /schemas/types.yaml#/definitions/phandle-array
>>> +          description: AMD Pensando Elba SoC system controller
>>
>> And nothing here - neither in commit msg nor here - explains why do you
>> need it and what is it for.
> 
> Adding property amd,pensando-elba-syscon was a result of this thread:
> https://lore.kernel.org/lkml/20220621101159.stvan53rvr6qugna@mobilestation/
> 

But it is not in the code. The code should tell what the property does,
what is its purpose, how it is used etc. Your property description
basically copies the name without giving any new information.

Best regards,
Krzysztof
Re: [PATCH v9 04/15] dt-bindings: spi: dw: Add AMD Pensando Elba SoC SPI Controller bindings
Posted by Brad Larson 2 years, 7 months ago 
On 24/01/2023 7:55 UTC, Krzysztof Kozlowski wrote:
>On 24/01/2023 02:57, Brad Larson wrote:
>> On 19/01/2023 7:55 UTC, Krzysztof Kozlowski wrote:
>>> On 19/01/2023 04:51, Brad Larson wrote:
>>>> The AMD Pensando Elba SoC has integrated the DW APB SPI Controller
>>>>
>> ...
>>>>  .../devicetree/bindings/spi/snps,dw-apb-ssi.yaml   | 14 ++++++++++++++
>>>>  1 file changed, 14 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>>> index d33b72fabc5d..96b072835de0 100644
>>>> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>>> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>>> @@ -37,6 +37,18 @@ allOf:
>>>>      else:
>>>>        required:
>>>>          - interrupts
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            const: amd,pensando-elba-spi
>>>> +    then:
>>>> +      properties:
>>>> +        amd,pensando-elba-syscon:
>>>> +          $ref: /schemas/types.yaml#/definitions/phandle-array
>>>> +          description: AMD Pensando Elba SoC system controller
>>>
>>> And nothing here - neither in commit msg nor here - explains why do you
>>> need it and what is it for.
>> 
>> Adding property amd,pensando-elba-syscon was a result of this thread:
>> https://lore.kernel.org/lkml/20220621101159.stvan53rvr6qugna@mobilestation/
>> 
>
> But it is not in the code. The code should tell what the property does,
> what is its purpose, how it is used etc. Your property description
> basically copies the name without giving any new information.

Yes, I looked past the description, thanks.  See below the updated description
and added amd,pensando-elba-syscon definition to top level properties.  The
property is added to the end as I see partial alphabetical ordering.

--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
@@ -37,6 +37,17 @@ allOf:
     else:
       required:
         - interrupts
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: amd,pensando-elba-spi
+    then:
+      required:
+        - amd,pensando-elba-syscon
+    else:
+      properties:
+        amd,pensando-elba-syscon: false
 
 properties:
   compatible:
@@ -63,6 +74,8 @@ properties:
         const: intel,keembay-ssi
       - description: Intel Thunder Bay SPI Controller
         const: intel,thunderbay-ssi
+      - description: AMD Pensando Elba SoC SPI Controller
+        const: amd,pensando-elba-spi
       - description: Baikal-T1 SPI Controller
         const: baikal,bt1-ssi
       - description: Baikal-T1 System Boot SPI Controller
@@ -136,6 +149,12 @@ properties:
       of the designware controller, and the upper limit is also subject to
       controller configuration.
 
+  amd,pensando-elba-syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: |
+      Block address to control four spi chip-selects.  The Elba SoC
+      does not use ssi.
+
 patternProperties:
   "^.*@[0-9a-f]+$":
     type: object

Regards,
Brad

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.