Re: (EXT) Re: (EXT) Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
On Mon, Jul 4, 2022 at 12:07 AM Alexander Stein
wrote:
>
> Am Freitag, 1. Juli 2022, 09:02:22 CEST schrieb Saravana Kannan:
> > On Thu, Jun 30, 2022 at 11:02 PM Alexander Stein
> >
> > wrote:
> > > Hi Saravana,
> > >
> > > Am Freitag, 1. Juli 2022, 02:37:14 CEST schrieb Saravana Kannan:
> > > > On Thu, Jun 23, 2022 at 5:08 AM Alexander Stein
> > > >
> > > > wrote:
> > > > > Hi,
> > > > >
> > > > > Am Dienstag, 21. Juni 2022, 09:28:43 CEST schrieb Tony Lindgren:
> > > > > > Hi,
> > > > > >
> > > > > > * Saravana Kannan [700101 02:00]:
> > > > > > > Now that fw_devlink=on by default and fw_devlink supports
> > > > > > > "power-domains" property, the execution will never get to the
> > > > > > > point
> > > > > > > where driver_deferred_probe_check_state() is called before the
> > > > > > > supplier
> > > > > > > has probed successfully or before deferred probe timeout has
> > > > > > > expired.
> > > > > > >
> > > > > > > So, delete the call and replace it with -ENODEV.
> > > > > >
> > > > > > Looks like this causes omaps to not boot in Linux next. With this
> > > > > > simple-pm-bus fails to probe initially as the power-domain is not
> > > > > > yet available. On platform_probe() genpd_get_from_provider() returns
> > > > > > -ENOENT.
> > > > > >
> > > > > > Seems like other stuff is potentially broken too, any ideas on
> > > > > > how to fix this?
> > > > >
> > > > > I think I'm hit by this as well, although I do not get a lockup.
> > > > > In my case I'm using
> > > > > arch/arm64/boot/dts/freescale/imx8mq-tqma8mq-mba8mx.dts and probing of
> > > > > 3832.blk-ctrl fails as the power-domain is not (yet) registed.
> > > >
> > > > Ok, took a look.
> > > >
> > > > The problem is that there are two drivers for the same device and they
> > > > both initialize this device.
> > > >
> > > > gpc: gpc@303a {
> > > >
> > > > compatible = "fsl,imx8mq-gpc";
> > > >
> > > > }
> > > >
> > > > $ git grep -l "fsl,imx7d-gpc" -- drivers/
> > > > drivers/irqchip/irq-imx-gpcv2.c
> > > > drivers/soc/imx/gpcv2.c
> > > >
> > > > IMHO, this is a bad/broken design.
> > > >
> > > > So what's happening is that fw_devlink will block the probe of
> > > > 3832.blk-ctrl until 303a.gpc is initialized. And it stops
> > > > blocking the probe of 3832.blk-ctrl as soon as the first driver
> > > > initializes the device. In this case, it's the irqchip driver.
> > > >
> > > > I'd recommend combining these drivers into one. Something like the
> > > > patch I'm attaching (sorry for the attachment, copy-paste is mangling
> > > > the tabs). Can you give it a shot please?
> > >
> > > I tried this patch and it delayed the driver initialization (those of UART
> > > as
> > > well BTW). Unfortunately the driver fails the same way:
> > Thanks for testing the patch!
> >
> > > > [1.125253] imx8m-blk-ctrl 3832.blk-ctrl: error -ENODEV: failed
> > > > to
> > >
> > > attach power domain "bus"
> > >
> > > More than that it even introduced some more errors:
> > > > [0.008160] irq: no irq domain found for gpc@303a !
> >
> > So the idea behind my change was that as long as the irqchip isn't the
> > root of the irqdomain (might be using the terms incorrectly) like the
> > gic, you can make it a platform driver. And I was trying to hack up a
> > patch that's the equivalent of platform_irqchip_probe() (which just
> > ends up eventually calling the callback you use in IRQCHIP_DECLARE().
> > I probably made some mistake in the quick hack that I'm sure if
> > fixable.
> >
> > > > [0.013251] Failed to map interrupt for
> > > > /soc@0/bus@3040/timer@306a
> >
> > However, this timer driver also uses TIMER_OF_DECLARE() which can't
> > handle failure to get the IRQ (because it's can't -EPROBE_DEFER). So,
> > this means, the timer driver inturn needs to be converted to a
> > platform driver if it's supposed to work with the IRQCHIP_DECLARE()
> > being converted to a platform driver.
> >
> > But that's a can of worms not worth opening. But then I remembered
> > this simpler workaround will work and it is pretty much a variant of
> > the workaround that's already in the gpc's irqchip driver to allow two
> > drivers to probe the same device (people really should stop doing
> > that).
> >
> > Can you drop my previous hack patch and try this instead please? I'm
> > 99% sure this will work.
> >
> > diff --git a/drivers/irqchip/irq-imx-gpcv2.c
> > b/drivers/irqchip/irq-imx-gpcv2.c index b9c22f764b4d..8a0e82067924 100644
> > --- a/drivers/irqchip/irq-imx-gpcv2.c
> > +++ b/drivers/irqchip/irq-imx-gpcv2.c
> > @@ -283,6 +283,7 @@ static int __init imx_gpcv2_irqchip_init(struct
> > device_node *node,
> > * later the GPC power domain driver will not be skipped.
> > */
> > of_node_clear_flag(node, OF_POPULATED);
> > + fwnode_dev_initialized(domain->fwnode, false);
> > return 0;
> > }
>
> Just to be sure here, I tried this patch on top of next-20220701 but
> unfortunately this doesn't
Re: (EXT) Re: (EXT) Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
Am Freitag, 1. Juli 2022, 09:02:22 CEST schrieb Saravana Kannan:
> On Thu, Jun 30, 2022 at 11:02 PM Alexander Stein
>
> wrote:
> > Hi Saravana,
> >
> > Am Freitag, 1. Juli 2022, 02:37:14 CEST schrieb Saravana Kannan:
> > > On Thu, Jun 23, 2022 at 5:08 AM Alexander Stein
> > >
> > > wrote:
> > > > Hi,
> > > >
> > > > Am Dienstag, 21. Juni 2022, 09:28:43 CEST schrieb Tony Lindgren:
> > > > > Hi,
> > > > >
> > > > > * Saravana Kannan [700101 02:00]:
> > > > > > Now that fw_devlink=on by default and fw_devlink supports
> > > > > > "power-domains" property, the execution will never get to the
> > > > > > point
> > > > > > where driver_deferred_probe_check_state() is called before the
> > > > > > supplier
> > > > > > has probed successfully or before deferred probe timeout has
> > > > > > expired.
> > > > > >
> > > > > > So, delete the call and replace it with -ENODEV.
> > > > >
> > > > > Looks like this causes omaps to not boot in Linux next. With this
> > > > > simple-pm-bus fails to probe initially as the power-domain is not
> > > > > yet available. On platform_probe() genpd_get_from_provider() returns
> > > > > -ENOENT.
> > > > >
> > > > > Seems like other stuff is potentially broken too, any ideas on
> > > > > how to fix this?
> > > >
> > > > I think I'm hit by this as well, although I do not get a lockup.
> > > > In my case I'm using
> > > > arch/arm64/boot/dts/freescale/imx8mq-tqma8mq-mba8mx.dts and probing of
> > > > 3832.blk-ctrl fails as the power-domain is not (yet) registed.
> > >
> > > Ok, took a look.
> > >
> > > The problem is that there are two drivers for the same device and they
> > > both initialize this device.
> > >
> > > gpc: gpc@303a {
> > >
> > > compatible = "fsl,imx8mq-gpc";
> > >
> > > }
> > >
> > > $ git grep -l "fsl,imx7d-gpc" -- drivers/
> > > drivers/irqchip/irq-imx-gpcv2.c
> > > drivers/soc/imx/gpcv2.c
> > >
> > > IMHO, this is a bad/broken design.
> > >
> > > So what's happening is that fw_devlink will block the probe of
> > > 3832.blk-ctrl until 303a.gpc is initialized. And it stops
> > > blocking the probe of 3832.blk-ctrl as soon as the first driver
> > > initializes the device. In this case, it's the irqchip driver.
> > >
> > > I'd recommend combining these drivers into one. Something like the
> > > patch I'm attaching (sorry for the attachment, copy-paste is mangling
> > > the tabs). Can you give it a shot please?
> >
> > I tried this patch and it delayed the driver initialization (those of UART
> > as
> > well BTW). Unfortunately the driver fails the same way:
> Thanks for testing the patch!
>
> > > [1.125253] imx8m-blk-ctrl 3832.blk-ctrl: error -ENODEV: failed
> > > to
> >
> > attach power domain "bus"
> >
> > More than that it even introduced some more errors:
> > > [0.008160] irq: no irq domain found for gpc@303a !
>
> So the idea behind my change was that as long as the irqchip isn't the
> root of the irqdomain (might be using the terms incorrectly) like the
> gic, you can make it a platform driver. And I was trying to hack up a
> patch that's the equivalent of platform_irqchip_probe() (which just
> ends up eventually calling the callback you use in IRQCHIP_DECLARE().
> I probably made some mistake in the quick hack that I'm sure if
> fixable.
>
> > > [0.013251] Failed to map interrupt for
> > > /soc@0/bus@3040/timer@306a
>
> However, this timer driver also uses TIMER_OF_DECLARE() which can't
> handle failure to get the IRQ (because it's can't -EPROBE_DEFER). So,
> this means, the timer driver inturn needs to be converted to a
> platform driver if it's supposed to work with the IRQCHIP_DECLARE()
> being converted to a platform driver.
>
> But that's a can of worms not worth opening. But then I remembered
> this simpler workaround will work and it is pretty much a variant of
> the workaround that's already in the gpc's irqchip driver to allow two
> drivers to probe the same device (people really should stop doing
> that).
>
> Can you drop my previous hack patch and try this instead please? I'm
> 99% sure this will work.
>
> diff --git a/drivers/irqchip/irq-imx-gpcv2.c
> b/drivers/irqchip/irq-imx-gpcv2.c index b9c22f764b4d..8a0e82067924 100644
> --- a/drivers/irqchip/irq-imx-gpcv2.c
> +++ b/drivers/irqchip/irq-imx-gpcv2.c
> @@ -283,6 +283,7 @@ static int __init imx_gpcv2_irqchip_init(struct
> device_node *node,
> * later the GPC power domain driver will not be skipped.
> */
> of_node_clear_flag(node, OF_POPULATED);
> + fwnode_dev_initialized(domain->fwnode, false);
> return 0;
> }
Just to be sure here, I tried this patch on top of next-20220701 but
unfortunately this doesn't fix the original problem either. The timer errors
are gone though.
The probe of imx8m-blk-ctrl got slightly delayed (from 0.74 to 0.90s printk
time) but results in the identical error message.
Best regards,
Alexander
