On Fri, Aug 30, 2024 at 12:55:32PM +0200, Lorenzo Bianconi wrote:
> [...]
>
> > >>>
> > >>> 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";
> };
> };
Didn't Rob basically tell you how to do it earlier in the thread?
What you've got now makes no sense, the compatibles only exist in that
to probe drivers, which you can do from the mfd driver with
mfd_add_devices() or w/e that function is called.
Cheers,
Conor.
Attachment:
signature.asc
Description: PGP signature
