[...]
> >>>
> >>> Hi Rob, thanks a lot for the hint, I hope we can finally find a solution
> >>> on how to implement this.
> >>>
> >>> In Documentation the block is called GPIO Controller. As explained it does
> >>> expose pinctrl function AND pwm (with regs in the middle)
> >>>
> >>> Is this semplification really needed? It does pose some problem driver
> >>> wise (on where to put the driver, in what subsystem) and also on the
> >>
> >> Sorry, but no, dt-bindings do not affect the driver at all in such way.
> >> Nothing changes in your driver in such aspect, no dilemma where to put
> >> it (the same place as before).
> >>
> >
> > Ok, from the proposed node structure, is it problematic to move the
> > gpio-controller and -cells in the pinctrl node? And also the pwm-cells
> > to the pwm node?
>
> The move is just unnecessary and not neat. You design DTS based on your
> drivers architecture and this is exactly what we want to avoid.
>
> > This is similar to how it's done by broadcom GPIO MFD [1] that also
>
> There are 'reg' fields, which is the main problem here. I don't like
> that arguments because it entirely misses the discussions - about that
> binding or other bindings - happening prior to merge.
>
> > expose pinctrl and other device in the same register block as MFD
> > childs.
> >
> > This would be the final node block.
> >
> > mfd@1fbf0200 {
> > compatible = "airoha,en7581-gpio-mfd";
> > reg = <0x0 0x1fbf0200 0x0 0xc0>;
> >
> > interrupt-parent = <&gic>;
> > interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
> >
> > pio: pinctrl {
> > compatible = "airoha,en7581-pinctrl";
> >
> > gpio-controller;
> > #gpio-cells = <2>;
> >
> > interrupt-controller;
> > #interrupt-cells = <2>;
>
> No resources here...
ack. iiuc, all the properties will be in the parent node (mfd) and we will
have just the compatible strings in the child ones, right? Something like:
mfd@1fbf0200 {
compatible = "airoha,en7581-gpio-mfd";
reg = <0x0 0x1fbf0200 0x0 0xc0>;
gpio-controller;
#gpio-cells = <2>;
...
#pwm-cells = <3>;
pio: pinctrl {
compatible = "airoha,en7581-pinctrl";
};
pwm: pwm {
compatible = "airoha,en7581-pwm";
};
};
>
> > };
> >
> > pwm: pwm {
> > compatible = "airoha,en7581-pwm";
> >
> > #pwm-cells = <3>;
> > status = "disabled";
>
> And why is it disabled? No external resources. There is no benefit of
> this node.
This is just a copy-paster error.
Regards,
Lorenzo
>
> > };
> > };
> >
> > I also link the implementation of the MFD driver [2]
> >
> > [1] https://elixir.bootlin.com/linux/v6.10.7/source/Documentation/devicetree/bindings/mfd/brcm,bcm6318-gpio-sysctl.yaml
> > [2] https://github.com/Ansuel/linux/blob/airoha-mfd/drivers/mfd/airoha-en7581-gpio-mfd.c
>
>
> Best regards,
> Krzysztof
>
Attachment:
signature.asc
Description: PGP signature
