Re: [PATCH 18/18] arm64: apple: Add initial Mac Mini 2020 (M1) devicetree

From: Krzysztof Kozlowski <krzk@kernel.org>
To: Tony Lindgren <tony@atomide.com>
Cc: Arnd Bergmann <arnd@kernel.org>,
	devicetree@vger.kernel.org, Marc Zyngier <maz@kernel.org>,
	Hector Martin <marcan@marcan.st>,
	linux-kernel@vger.kernel.org, soc@kernel.org, robh+dt@kernel.org,
	Olof Johansson <olof@lixom.net>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 18/18] arm64: apple: Add initial Mac Mini 2020 (M1) devicetree
Date: Wed, 10 Feb 2021 13:55:48 +0100	[thread overview]
Message-ID: <20210210125548.sdeadc4ncoxu3ikj@kozik-lap> (raw)
In-Reply-To: <YCPE2lPpBlhCi7TH@atomide.com>

On Wed, Feb 10, 2021 at 01:34:50PM +0200, Tony Lindgren wrote:
> * Hector Martin <marcan@marcan.st> [210210 11:14]:
> > On 10/02/2021 19.19, Tony Lindgren wrote:
> > > * Hector Martin 'marcan' <marcan@marcan.st> [210208 12:05]:
> > > > On 08/02/2021 20.04, Krzysztof Kozlowski wrote:
> > > ...
> > > 
> > > > > > +	clk24: clk24 {
> > > > > 
> > > > > Just "clock". Node names should be generic.
> > > > 
> > > > Really? Almost every other device device tree uses unique clock node names.
> > > 
> > > Yeah please just use generic node name "clock". FYI, we're still hurting
> > > because of this for the TI clock node names years after because the drivers
> > > got a chance to rely on the clock node name..
> > > 
> > > Using "clock" means your clock driver code won't get a chance to wrongly
> > > use the node name and you avoid similar issues.
> > 
> > That means it'll end up like this (so that we can have more than one
> > fixed-clock):
> > 
> > clocks {
> >     #address-cells = <1>;
> >     #size-cells = <0>;
> > 
> >     clk123: clock@0 {
> >         ...
> >         reg = <0>
> >     }
> > 
> >     clk456: clock@1 {
> >         ...
> >         reg = <1>
> >     }
> > }
> > 
> > Correct?
> 
> Yeah, just don't use an imaginary dummy index for the reg. Use a real
> register offset from a clock controller instance base, and a register
> bit offset too if needed.

No, there is no need for fake "clocks" node with fake addresses. If you
have multiple clocks, the rules are the same as for other similar cases,
e.g. leds:

{
    clock-0 {
       ...
    };

    clock-1 {
        ..
    };

    soc@0 {
    };
}

This should not generate any dtc W=1 warnings and work with dtschema
(you need to check for both).

Best regards,
Krzysztof

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