Re: [PATCH 1/6] dt-bindings: mfd: add binding for Apple Mac System Management Controller

From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Arnd Bergmann <arnd@arndb.de>, Lee Jones <lee@kernel.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Alyssa Rosenzweig <alyssa@rosenzweig.io>,
	asahi@lists.linux.dev, Bartosz Golaszewski <brgl@bgdev.pl>,
	Hector Martin <marcan@marcan.st>,
	linux-arm-kernel@lists.infradead.org, linux-gpio@vger.kernel.org,
	Sven Peter <sven@svenpeter.dev>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	devicetree@vger.kernel.org
Subject: Re: [PATCH 1/6] dt-bindings: mfd: add binding for Apple Mac System Management Controller
Date: Thu, 1 Sep 2022 16:24:20 +0100	[thread overview]
Message-ID: <YxDOpCq0vIlt4VNa@shell.armlinux.org.uk> (raw)
In-Reply-To: <928ddeff-efac-920c-7bbf-dda35a942b93@linaro.org>

On Thu, Sep 01, 2022 at 06:15:46PM +0300, Krzysztof Kozlowski wrote:
> On 01/09/2022 18:12, Russell King (Oracle) wrote:
> >>> +  compatible:
> >>> +    items:
> >>> +      - enum:
> >>> +        - apple,t8103-smc
> >>
> >> You miss two spaces of indentation on this level.
> > 
> > Should that be picked up by the dt checker?
> 
> I think yamllint complains about it. It is not a hard-dependency, so
> maybe you don't have it installed.
> 
> > 
> >>> +        - apple,t8112-smc
> >>> +        - apple,t6000-smc
> >>
> >> Bring some order here - either alphabetical or by date of release (as in
> >> other Apple schemas). I think t6000 was before t8112, so it's none of
> >> that orders.
> > 
> > Ok.
> > 
> >>> +      - const: apple,smc
> >>> +
> >>> +  reg:
> >>> +    description: Two regions, one for the SMC area and one for the SRAM area.
> >>
> >> You need constraints for size/order, so in this context list with
> >> described items.
> > 
> > How do I do that? I tried maxItems/minItems set to 2, but the dt checker
> > objected to it.
> 
> One way:
> reg:
>   items:
>     - description: SMC area
>     - description: SRAM area
> 
> but actually this is very similar what you wrote for reg-names - kind of
> obvious, so easier way:
> 
> reg:
>   maxItems: 2

Doesn't work. With maxItems: 2, the example fails, yet it correctly lists
two regs which are 64-bit address and 64-bit size - so in total 8 32-bit
ints.

Documentation/devicetree/bindings/mfd/apple,smc.example.dtb: smc@23e400000: reg: [[2, 1044381696], [0, 16384], [2, 1071644672], [0, 1048576]] is too long
        From schema: /home/rmk/git/linux-rmk/Documentation/devicetree/bindings/mfd/apple,smc.yaml

Hence, I originally had maxItems: 2, and ended up deleting it because of
the dt checker.

With the two descriptions, it's the same failure.

I think the problem is that the checker has no knowledge in the example
of how big each address and size element of the reg property is. So,
it's interpreting it as four entries of 32-bit address,size pairs
instead of two entries of 64-bit address,size pairs. Yep, that's it,
if I increase the number of "- description" entries to four then it's
happy.

So, what's the solution?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel