Re: [PATCH v6 04/17] dt-bindings: spi: dw: Add AMD Pensando Elba SoC SPI Controller bindings

["Larson, Bradley" <Bradley.Larson@amd.com>]

On 9/11/22 11:34 AM, Serge Semin wrote:
> On Wed, Aug 31, 2022 at 06:28:46PM +0000, Larson, Bradley wrote:
>> On 8/21/22 10:49 AM, Serge Semin wrote:
>>> On Sat, Aug 20, 2022 at 12:57:37PM -0700, Brad Larson wrote:
>>>> From: Brad Larson <blarson@amd.com>
>>>>
>>>> The AMD Pensando Elba SoC has integrated the DW APB SPI Controller
>>>>
>>>> Signed-off-by: Brad Larson <blarson@amd.com>
>>>> ---
>>>>    .../devicetree/bindings/spi/snps,dw-apb-ssi.yaml      | 11 +++++++++++
>>>>    1 file changed, 11 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>>> index 37c3c272407d..403d6416f7ac 100644
>>>> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>>> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>>> @@ -37,6 +37,15 @@ allOf:
>>>>        else:
>>>>          required:
>>>>            - interrupts
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            enum:
>>>> +              - amd,pensando-elba-spi
>>>> +    then:
>>>> +      required:
>>>> +        - amd,pensando-elba-syscon
>>> Please add the "amd,pensando-elba-syscon" property definition as I
>>> asked here:
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20220704131810.kabkuy6e4qmhfm3n%40mobilestation%2F&amp;data=05%7C01%7CBradley.Larson%40amd.com%7C5f61c8f8130c47fa814208da94243e17%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637985180686357705%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=uzcSBjuxs1JZFXiRGFVLGojAsipJACXEGskYdGmr7qA%3D&amp;reserved=0
>> Proposing this addition:
>>
>> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>> @@ -148,6 +148,15 @@ 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"
>> +    maxItems: 1
>> +    description:
>> +      A phandle to syscon used to access the spi chip-select override
>> register.
>> +    items:
>> +      - items:
>> +        - description: phandle to the syscon node
>> +
> No. What Krzysztof and I asked was to add the property definition
> into the allOf: [ if ...,  ] statement. Please read more carefully my
> last comment:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20220704131810.kabkuy6e4qmhfm3n%40mobilestation%2F&amp;data=05%7C01%7CBradley.Larson%40amd.com%7C5f61c8f8130c47fa814208da94243e17%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637985180686357705%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=uzcSBjuxs1JZFXiRGFVLGojAsipJACXEGskYdGmr7qA%3D&amp;reserved=0
> The definition is supposed to look like this:
>
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: amd,pensando-elba-spi
>> +    then:
>    +      properties:
>    +        amd,pensando-elba-syscon
>    +          $ref: /schemas/types.yaml#/definitions/phandle
>    +          description: AMD Pensando Elba SoC system controller
>> +      required:
>> +        - amd,pensando-elba-syscon
> * Please also note that I've replaced "enum:" with "const:" in the if
> statement above.
>
> The difference with what you suggested is that my version is
> applicable for the Pensando ELBA SPI controller only, while your
> update will cause applying the "amd,pensando-elba-syscon" property
> constraints for all DW SSI controllers which isn't what we would want.


Yes, I see by moving this property into the allOf its only applicable 
for compatible "amd,pensando-elba-spi". Also changing "enum:" to 
"const:" as shown.  Yes on the DT-bindings check. Rob Herring's bot 
indicated an error but I had none in checking V6 patchset.  I did a 
dtschema update and it went from dtschema 2022.3.2 to dtschema-2022.8.3 
and will see if that is the reason.

Regards,
Brad