Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 11 Sep 2022 21:34:15 +0300
From:   Serge Semin <fancer.lancer@gmail.com>
To:     "Larson, Bradley" <Bradley.Larson@amd.com>
Cc:     Brad Larson <brad@pensando.io>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
        "adrian.hunter@intel.com" <adrian.hunter@intel.com>,
        "alcooperx@gmail.com" <alcooperx@gmail.com>,
        "andy.shevchenko@gmail.com" <andy.shevchenko@gmail.com>,
        "arnd@arndb.de" <arnd@arndb.de>,
        "brijeshkumar.singh@amd.com" <brijeshkumar.singh@amd.com>,
        "catalin.marinas@arm.com" <catalin.marinas@arm.com>,
        "gsomlo@gmail.com" <gsomlo@gmail.com>,
        "gerg@linux-m68k.org" <gerg@linux-m68k.org>,
        "krzk@kernel.org" <krzk@kernel.org>,
        "krzysztof.kozlowski+dt@linaro.org" 
        <krzysztof.kozlowski+dt@linaro.org>,
        "lee.jones@linaro.org" <lee.jones@linaro.org>,
        "broonie@kernel.org" <broonie@kernel.org>,
        "yamada.masahiro@socionext.com" <yamada.masahiro@socionext.com>,
        "p.zabel@pengutronix.de" <p.zabel@pengutronix.de>,
        "piotrs@cadence.com" <piotrs@cadence.com>,
        "p.yadav@ti.com" <p.yadav@ti.com>,
        "rdunlap@infradead.org" <rdunlap@infradead.org>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "samuel@sholland.org" <samuel@sholland.org>,
        "Suthikulpanit, Suravee" <Suravee.Suthikulpanit@amd.com>,
        "Lendacky, Thomas" <Thomas.Lendacky@amd.com>,
        "ulf.hansson@linaro.org" <ulf.hansson@linaro.org>,
        "will@kernel.org" <will@kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>
Subject: Re: [PATCH v6 04/17] dt-bindings: spi: dw: Add AMD Pensando Elba SoC
 SPI Controller bindings
Message-ID: <20220911183415.ufeasmm3uev7wqx4@mobilestation>
References: <20220820195750.70861-1-brad@pensando.io>
 <20220820195750.70861-5-brad@pensando.io>
 <20220821174952.5i5pfftxt53dmqv3@mobilestation>
 <0c2ac76e-45ff-b5a1-f33a-177eea04c17d@amd.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <0c2ac76e-45ff-b5a1-f33a-177eea04c17d@amd.com>
Precedence: bulk

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%7C1c4f822c81424048873508da839d90fc%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637967010019245894%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=xl9OU9P9QK3wLHc25hQZK393ylULd41qc4HB2Zt%2F0BQ%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://lore.kernel.org/lkml/20220704131810.kabkuy6e4qmhfm3n@mobilestation/
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.

-Sergey

>  ?patternProperties:
>  ?? "^.*@[0-9a-f]+$":
>  ???? type: object
> 
> Regards,
> Brad