| Message ID | 20220820195750.70861-5-brad@pensando.io |
|---|---|
| State | New |
| Headers | show |
| Series | Support AMD Pensando Elba SoC | expand |
On 8/22/22 11:19 AM, Krzysztof Kozlowski wrote: > On 20/08/2022 22:57, 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 > There is no such property. You cannot make it required without first > defining it. Added the definition of 'amd,pensando-elba-syscon' to snps,dw-apb-ssi.yaml >> properties: >> compatible: >> @@ -75,6 +84,8 @@ properties: >> - renesas,r9a06g032-spi # RZ/N1D >> - renesas,r9a06g033-spi # RZ/N1S >> - const: renesas,rzn1-spi # RZ/N1 >> + - description: AMD Pensando Elba SoC SPI Controller >> + const: amd,pensando-elba-spi > Don't add stuff at the end, but in some logical (usually alphabetical) > place. The order is already broken as everyone likes to add stuff in > conflict-style, so just add it before baikal, for example. Yes, tried to follow existing style. Will add it before baikal. Regards, Brad
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&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
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 properties: compatible: @@ -75,6 +84,8 @@ properties: - renesas,r9a06g032-spi # RZ/N1D - renesas,r9a06g033-spi # RZ/N1S - const: renesas,rzn1-spi # RZ/N1 + - description: AMD Pensando Elba SoC SPI Controller + const: amd,pensando-elba-spi reg: minItems: 1