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

Larson, Bradley
2022-09-14T18:47:50Z

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