[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e0edd195-8642-3b59-beaf-a2027877d33a@amd.com>
Date: Wed, 14 Sep 2022 18:47:37 +0000
From: "Larson, Bradley" <Bradley.Larson@....com>
To: Serge Semin <fancer.lancer@...il.com>,
"Larson, Bradley" <Bradley.Larson@....com>
CC: Brad Larson <brad@...sando.io>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
"adrian.hunter@...el.com" <adrian.hunter@...el.com>,
"alcooperx@...il.com" <alcooperx@...il.com>,
"andy.shevchenko@...il.com" <andy.shevchenko@...il.com>,
"arnd@...db.de" <arnd@...db.de>,
"brijeshkumar.singh@....com" <brijeshkumar.singh@....com>,
"catalin.marinas@....com" <catalin.marinas@....com>,
"gsomlo@...il.com" <gsomlo@...il.com>,
"gerg@...ux-m68k.org" <gerg@...ux-m68k.org>,
"krzk@...nel.org" <krzk@...nel.org>,
"krzysztof.kozlowski+dt@...aro.org"
<krzysztof.kozlowski+dt@...aro.org>,
"lee.jones@...aro.org" <lee.jones@...aro.org>,
"broonie@...nel.org" <broonie@...nel.org>,
"yamada.masahiro@...ionext.com" <yamada.masahiro@...ionext.com>,
"p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
"piotrs@...ence.com" <piotrs@...ence.com>,
"p.yadav@...com" <p.yadav@...com>,
"rdunlap@...radead.org" <rdunlap@...radead.org>,
"robh+dt@...nel.org" <robh+dt@...nel.org>,
"samuel@...lland.org" <samuel@...lland.org>,
"Suthikulpanit, Suravee" <Suravee.Suthikulpanit@....com>,
"Lendacky, Thomas" <Thomas.Lendacky@....com>,
"ulf.hansson@...aro.org" <ulf.hansson@...aro.org>,
"will@...nel.org" <will@...nel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH v6 04/17] dt-bindings: spi: dw: Add AMD Pensando Elba SoC
SPI Controller bindings
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@....com>
>>>>
>>>> The AMD Pensando Elba SoC has integrated the DW APB SPI Controller
>>>>
>>>> Signed-off-by: Brad Larson <blarson@....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&data=05%7C01%7CBradley.Larson%40amd.com%7C5f61c8f8130c47fa814208da94243e17%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637985180686357705%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=uzcSBjuxs1JZFXiRGFVLGojAsipJACXEGskYdGmr7qA%3D&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&data=05%7C01%7CBradley.Larson%40amd.com%7C5f61c8f8130c47fa814208da94243e17%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637985180686357705%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=uzcSBjuxs1JZFXiRGFVLGojAsipJACXEGskYdGmr7qA%3D&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
Powered by blists - more mailing lists