Re: [PATCH v3 2/9] ARM: dts: nspire: Use syscon-reboot to handle restart

[Andrew Davis]

On 10/27/22 2:33 PM, Krzysztof Kozlowski wrote:
> On 27/10/2022 14:13, Andrew Davis wrote:
> > Writing this bit can be handled by the syscon-reboot driver.
> > Add this node to DT.
> >
> > Signed-off-by: Andrew Davis <afd@xxxxxx>
> > Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> > Tested-by: Fabian Vogt <fabian@xxxxxxxxxxxxxx>
> > Reviewed-by: Fabian Vogt <fabian@xxxxxxxxxxxxxx>
> > ---
> >   arch/arm/boot/dts/nspire.dtsi | 7 +++++++
> >   1 file changed, 7 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/nspire.dtsi b/arch/arm/boot/dts/nspire.dtsi
> > index bb240e6a3a6f..48fbc9d533c3 100644
> > --- a/arch/arm/boot/dts/nspire.dtsi
> > +++ b/arch/arm/boot/dts/nspire.dtsi
> > @@ -172,7 +172,14 @@ rtc: rtc@90090000 {
> >   			};
> >   
> >   			misc: misc@900a0000 {
> > +				compatible = "ti,nspire-misc", "syscon", "simple-mfd";
>
> You have syscon and simple-mfd, but bindings in patch #1 say only syscon.

I'm not following, are you just saying my wording in the patch message just
wasn't complete?

Or are you saying something more about nodes that are both syscon and simple-mfd?
In that case, having both syscon and simple-mfd seems rather common, looks like
you added the rule for it[0].

Thinking on this, they almost represent the same thing. simple-mfd says "my child
nodes should be considered devices", why do we need that? Couldn't we simply state
that "syscon" node's children are always devices, I mean what else could they be,
syscon is an MFD after all (and lives in drivers/mfd/).

"syscon" often just says, others can use the registers within this node, so as a
different option, make "syscon" a property of "simple-mfd" nodes. I'm seeing all
these examples of devices that should have been children of the "syscon" device,
but instead use

regmap = <&x>;
syscon = <&x>;

or similar and put the device node out somewhere random. And in those cases,
wouldn't it have been more correct to use the normal "reg" and "regions" to
define the registers belonging to the child node/device?..

Thanks,
Andrew

[0] https://lore.kernel.org/all/20220817142246.828762-5-krzysztof.kozlowski@xxxxxxxxxx/

> Best regards,
> Krzysztof