[PATCH] ARM: imx: drop devlinks to reset-controller node

[Philipp Zabel]

Hi Saravana,

(Cc: Michael)

On Tue, 2021-09-21 at 09:47 -0700, Saravana Kannan wrote:
> Hi Philipp,
> 
> On Tue, Sep 21, 2021 at 8:05 AM Philipp Zabel <p.zabel at pengutronix.de> wrote:
> > Hi Saravana,
> > 
> > On Tue, 2021-09-21 at 07:45 -0700, Saravana Kannan wrote:
> > > On Tue, Sep 21, 2021 at 5:50 AM Philipp Zabel <p.zabel at pengutronix.de> wrote:
> > > > Starting with commit 6b2117ad65f1 ("of: property: fw_devlink: Add
> > > > support for "resets" and "pwms""), the imx-drm driver fails to load
> > > > due to forever dormant devlinks to the reset-controller node. This
> > > > node is never associated with a struct device.
> > > > 
> > > > Drop those links, the reset controller is always available.
> > > > 
> > > > Signed-off-by: Philipp Zabel <p.zabel at pengutronix.de>
> > > > ---
> > > >  arch/arm/mach-imx/src.c | 10 +++++++++-
> > > >  1 file changed, 9 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/arch/arm/mach-imx/src.c b/arch/arm/mach-imx/src.c
> > > > index 95fd1fbb0826..6924ac19a762 100644
> > > > --- a/arch/arm/mach-imx/src.c
> > > > +++ b/arch/arm/mach-imx/src.c
> > > > @@ -178,9 +178,17 @@ void __init imx_src_init(void)
> > > >         WARN_ON(!src_base);
> > > > 
> > > >         imx_reset_controller.of_node = np;
> > > > -       if (IS_ENABLED(CONFIG_RESET_CONTROLLER))
> > > > +       if (IS_ENABLED(CONFIG_RESET_CONTROLLER)) {
> > > >                 reset_controller_register(&imx_reset_controller);
> > > > 
> > > > +               /*
> > > > +                * The reset-controller node is never associated with a struct
> > > > +                * device, which trips up fw_devlink=on. As a workaround, drop
> > > > +                * all devlinks to/from this node.
> > > > +                */
> > > > +               fw_devlink_purge_absent_suppliers(&np->fwnode);
> > > > +       }
> > > > +
> > > 
> > > Hey Philipp,
> > > 
> > > Can you point me to the .dts file that corresponds to this platform? I
> > > want to take a closer look.
> > 
> > Thank you, the relevant file is arch/arm/boot/dts/imx6qdl.dtsi, device
> > nodes labeled as &src and &ipu1. This is for example included by
> > arch/arm/boot/dts/imx6q-nitrogen6x.dts. There's a second &ipu2 added in
> > arch/arm/boot/dts/imx6qd.dtsi as well.
> 
> I took a closer look at the DT. Is there really no struct device for
> this node?

Yes, this node is handled by platform code in arch/arm/mach-imx/src.c as
the reset controller register also contains bits to bring up the CPUs.

> Based on the DT, it looks like a device would automatically
> get created for this node. So it looks like it's more of an issue with
> the device not being probed by a real driver? Why can't you convert
> the reset controller driver to a proper platform driver instead of
> this patch? I think that's the right fix.

In this case, I could indeed add a platform driver for the reset
controller part. I'm not yet sure that this is always the right thing.

I just learned from Michael that there are other cases where fw_devlink
follows of_graph links from capture devices to connector nodes that have
a compatible, such as "composite-video-connector" or "svideo-connector", 
but no associated driver. These are just parsed to determine the
connector type in drivers/media/v4l2-core/v4l2-fwnode.c. Would you
suggest to create platform devices for these as well?

regards
Philipp