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

From: Saravana Kannan <saravanak@google.com>
To: Philipp Zabel <p.zabel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org,
	Shawn Guo <shawnguo@kernel.org>,
	 kernel@pengutronix.de
Subject: Re: [PATCH] ARM: imx: drop devlinks to reset-controller node
Date: Tue, 21 Sep 2021 09:47:43 -0700	[thread overview]
Message-ID: <CAGETcx_wr0=FKRNmbGfN7Emg=2XooWFZx5jwX4ZfbTPZCMaPEQ@mail.gmail.com> (raw)
In-Reply-To: <30a8de2b70fba9046dfcc8d19faf5b44870493e2.camel@pengutronix.de>

Hi Philipp,

On Tue, Sep 21, 2021 at 8:05 AM Philipp Zabel <p.zabel@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@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@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? 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.

-Saravana

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel