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

[Rob Herring <robh+dt@xxxxxxxxxx>]

On Thu, Sep 1, 2022 at 11:47 AM Russell King (Oracle)
<linux@xxxxxxxxxxxxxxx> wrote:
>
> On Thu, Sep 01, 2022 at 07:25:03PM +0300, Krzysztof Kozlowski wrote:
> > On 01/09/2022 18:56, Russell King (Oracle) wrote:
> > >
> > > 8<===
> > > From: "Russell King (Oracle)" <rmk+kernel@xxxxxxxxxxxxxxx>
> > > Subject: [PATCH] dt-bindings: mfd: add binding for Apple Mac System Management
> > >  Controller
> > >
> > > Add a DT binding for the Apple Mac System Management Controller.
> > >
> > > Signed-off-by: Russell King (Oracle) <rmk+kernel@xxxxxxxxxxxxxxx>
> >
> > Yes, looks good.
> >
> > I won't add Reviewed-by tag, because I think it would confuse Patchwork,
> > so please send a v2 at some point.
>
> Thanks. Do you have any suggestions for patch 2? Should I merge the
> description in patch 2 into this file?
>
> The full dts for this series looks like this:
>
>                 smc: smc@23e400000 {
>                         compatible = "apple,t8103-smc", "apple,smc";
>                         reg = <0x2 0x3e400000 0x0 0x4000>,
>                                 <0x2 0x3fe00000 0x0 0x100000>;
>                         reg-names = "smc", "sram";
>                         mboxes = <&smc_mbox>;
>
>                         smc_gpio: gpio {
>                                 gpio-controller;
>                                 #gpio-cells = <2>;
>                         };
>                 };
>
> but the fuller version in the asahi linux tree looks like:
>
>                 smc: smc@23e400000 {
>                         compatible = "apple,t8103-smc", "apple,smc";
>                         reg = <0x2 0x3e400000 0x0 0x4000>,
>                                 <0x2 0x3fe00000 0x0 0x100000>;
>                         reg-names = "smc", "sram";
>                         mboxes = <&smc_mbox>;
>
>                         smc_gpio: gpio {
>                                 gpio-controller;
>                                 #gpio-cells = <2>;

Only 2 properties doesn't really need its own schema doc. However, I
would just move these to the parent node.

>                         };
>
>                         smc_rtc: rtc {
>                                 nvmem-cells = <&rtc_offset>;
>                                 nvmem-cell-names = "rtc_offset";
>                         };
>
>                         smc_reboot: reboot {
>                                 nvmem-cells = <&shutdown_flag>, <&boot_stage>,
>                                         <&boot_error_count>, <&panic_count>, <&pm_setting>;
>                                 nvmem-cell-names = "shutdown_flag", "boot_stage",
>                                         "boot_error_count", "panic_count", "pm_setting";

Not really much reason to split these up either because you can just
fetch the entry you want by name.

How confident are the asahi folks that this is a complete binding?

Rob