Re: Re: Re: Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
Am Dienstag, 5. Juli 2022, 03:24:33 CEST schrieb Saravana Kannan:
> 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
> > > +++
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
On Fri, Jul 1, 2022 at 12:13 PM Saravana Kannan wrote: > > On Fri, Jul 1, 2022 at 8:08 AM Sudeep Holla wrote: > > > > Hi, Saravana, > > > > On Fri, Jul 01, 2022 at 01:26:12AM -0700, Saravana Kannan wrote: > > > > [...] > > > > > Can you check if this hack helps? If so, then I can think about > > > whether we can pick it up without breaking everything else. Copy-paste > > > tab mess up warning. > > > > Sorry for jumping in late and not even sure if this is right thread. > > I have not bisected anything yet, but I am seeing issues on my Juno R2 > > with SCMI enabled power domains and Coresight AMBA devices. > > > > OF: amba_device_add() failed (-19) for /etf@2001 > > OF: amba_device_add() failed (-19) for /tpiu@2003 > > OF: amba_device_add() failed (-19) for /funnel@2004 > > OF: amba_device_add() failed (-19) for /etr@2007 > > OF: amba_device_add() failed (-19) for /stm@2010 > > OF: amba_device_add() failed (-19) for /replicator@2012 > > OF: amba_device_add() failed (-19) for /cpu-debug@2201 > > OF: amba_device_add() failed (-19) for /etm@2204 > > OF: amba_device_add() failed (-19) for /cti@2202 > > OF: amba_device_add() failed (-19) for /funnel@220c > > OF: amba_device_add() failed (-19) for /cpu-debug@2211 > > OF: amba_device_add() failed (-19) for /etm@2214 > > OF: amba_device_add() failed (-19) for /cti@2212 > > OF: amba_device_add() failed (-19) for /cpu-debug@2301 > > OF: amba_device_add() failed (-19) for /etm@2304 > > OF: amba_device_add() failed (-19) for /cti@2302 > > OF: amba_device_add() failed (-19) for /funnel@230c > > OF: amba_device_add() failed (-19) for /cpu-debug@2311 > > OF: amba_device_add() failed (-19) for /etm@2314 > > OF: amba_device_add() failed (-19) for /cti@2312 > > OF: amba_device_add() failed (-19) for /cpu-debug@2321 > > OF: amba_device_add() failed (-19) for /etm@2324 > > OF: amba_device_add() failed (-19) for /cti@2322 > > OF: amba_device_add() failed (-19) for /cpu-debug@2331 > > OF: amba_device_add() failed (-19) for /etm@2334 > > OF: amba_device_add() failed (-19) for /cti@2332 > > OF: amba_device_add() failed (-19) for /cti@2002 > > OF: amba_device_add() failed (-19) for /cti@2011 > > OF: amba_device_add() failed (-19) for /funnel@2013 > > OF: amba_device_add() failed (-19) for /etf@2014 > > OF: amba_device_add() failed (-19) for /funnel@2015 > > OF: amba_device_add() failed (-19) for /cti@2016 > > > > These are working fine with deferred probe in the mainline. > > I tried the hack you have suggested here(rather Tony's version), > > Thanks for trying that. > > > also > > tried with fw_devlink=0 and fw_devlink=1 > > 0 and 1 aren't valid input to fw_devlink. But yeah, I don't expect > disabling it to make anything better. > > > && fw_devlink.strict=0 > > No change in the behaviour. > > > > The DTS are in arch/arm64/boot/dts/arm/juno-*-scmi.dts and there > > coresight devices are mostly in juno-cs-r1r2.dtsi > > Thanks > > > Let me know if there is anything obvious or you want me to bisect which > > means I need more time. I can do that next week. > > I'll let you know once I poke at the DTS. We need to figure out why > fw_devlink wasn't blocking these from getting to the error (same as in > Tony's case). But since these are amba devices, I think I have some > guesses. > > This is an old series that had some issues in some cases and I haven't > gotten around to looking at it. You can give that a shot if you can > apply it to a recent tree. > https://lore.kernel.org/lkml/20210304195101.3843496-1-sarava...@google.com/ I rebased it to driver-core-next and tested the patch (for correctness, not with your issue though). I'm fairly sure it should help with your issue. Can you give it a shot please? https://lore.kernel.org/lkml/20220705083934.3974140-1-sarava...@google.com/T/#u -Saravana > > After looking at that old patch again, I think I know what's going on. > For normal devices, the pm domain attach happens AFTER the device is > added and fw_devlink has had a chance to set up device links. And if > the suppliers aren't ready, really_probe() won't get as far as > dev_pm_domain_attach(). But for amba, the clock and pm domain > suppliers are "grabbed" before adding the device. > > So with that old patch + always returning -EPROBE_DEFER in > amba_device_add() if amba_read_periphid() fails should fix your issue. > > -Saravana ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
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
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
On Fri, Jul 1, 2022 at 8:08 AM Sudeep Holla wrote: > > Hi, Saravana, > > On Fri, Jul 01, 2022 at 01:26:12AM -0700, Saravana Kannan wrote: > > [...] > > > Can you check if this hack helps? If so, then I can think about > > whether we can pick it up without breaking everything else. Copy-paste > > tab mess up warning. > > Sorry for jumping in late and not even sure if this is right thread. > I have not bisected anything yet, but I am seeing issues on my Juno R2 > with SCMI enabled power domains and Coresight AMBA devices. > > OF: amba_device_add() failed (-19) for /etf@2001 > OF: amba_device_add() failed (-19) for /tpiu@2003 > OF: amba_device_add() failed (-19) for /funnel@2004 > OF: amba_device_add() failed (-19) for /etr@2007 > OF: amba_device_add() failed (-19) for /stm@2010 > OF: amba_device_add() failed (-19) for /replicator@2012 > OF: amba_device_add() failed (-19) for /cpu-debug@2201 > OF: amba_device_add() failed (-19) for /etm@2204 > OF: amba_device_add() failed (-19) for /cti@2202 > OF: amba_device_add() failed (-19) for /funnel@220c > OF: amba_device_add() failed (-19) for /cpu-debug@2211 > OF: amba_device_add() failed (-19) for /etm@2214 > OF: amba_device_add() failed (-19) for /cti@2212 > OF: amba_device_add() failed (-19) for /cpu-debug@2301 > OF: amba_device_add() failed (-19) for /etm@2304 > OF: amba_device_add() failed (-19) for /cti@2302 > OF: amba_device_add() failed (-19) for /funnel@230c > OF: amba_device_add() failed (-19) for /cpu-debug@2311 > OF: amba_device_add() failed (-19) for /etm@2314 > OF: amba_device_add() failed (-19) for /cti@2312 > OF: amba_device_add() failed (-19) for /cpu-debug@2321 > OF: amba_device_add() failed (-19) for /etm@2324 > OF: amba_device_add() failed (-19) for /cti@2322 > OF: amba_device_add() failed (-19) for /cpu-debug@2331 > OF: amba_device_add() failed (-19) for /etm@2334 > OF: amba_device_add() failed (-19) for /cti@2332 > OF: amba_device_add() failed (-19) for /cti@2002 > OF: amba_device_add() failed (-19) for /cti@2011 > OF: amba_device_add() failed (-19) for /funnel@2013 > OF: amba_device_add() failed (-19) for /etf@2014 > OF: amba_device_add() failed (-19) for /funnel@2015 > OF: amba_device_add() failed (-19) for /cti@2016 > > These are working fine with deferred probe in the mainline. > I tried the hack you have suggested here(rather Tony's version), Thanks for trying that. > also > tried with fw_devlink=0 and fw_devlink=1 0 and 1 aren't valid input to fw_devlink. But yeah, I don't expect disabling it to make anything better. > && fw_devlink.strict=0 > No change in the behaviour. > > The DTS are in arch/arm64/boot/dts/arm/juno-*-scmi.dts and there > coresight devices are mostly in juno-cs-r1r2.dtsi Thanks > Let me know if there is anything obvious or you want me to bisect which > means I need more time. I can do that next week. I'll let you know once I poke at the DTS. We need to figure out why fw_devlink wasn't blocking these from getting to the error (same as in Tony's case). But since these are amba devices, I think I have some guesses. This is an old series that had some issues in some cases and I haven't gotten around to looking at it. You can give that a shot if you can apply it to a recent tree. https://lore.kernel.org/lkml/20210304195101.3843496-1-sarava...@google.com/ After looking at that old patch again, I think I know what's going on. For normal devices, the pm domain attach happens AFTER the device is added and fw_devlink has had a chance to set up device links. And if the suppliers aren't ready, really_probe() won't get as far as dev_pm_domain_attach(). But for amba, the clock and pm domain suppliers are "grabbed" before adding the device. So with that old patch + always returning -EPROBE_DEFER in amba_device_add() if amba_read_periphid() fails should fix your issue. -Saravana ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
Hi, Saravana, On Fri, Jul 01, 2022 at 01:26:12AM -0700, Saravana Kannan wrote: [...] > Can you check if this hack helps? If so, then I can think about > whether we can pick it up without breaking everything else. Copy-paste > tab mess up warning. Sorry for jumping in late and not even sure if this is right thread. I have not bisected anything yet, but I am seeing issues on my Juno R2 with SCMI enabled power domains and Coresight AMBA devices. OF: amba_device_add() failed (-19) for /etf@2001 OF: amba_device_add() failed (-19) for /tpiu@2003 OF: amba_device_add() failed (-19) for /funnel@2004 OF: amba_device_add() failed (-19) for /etr@2007 OF: amba_device_add() failed (-19) for /stm@2010 OF: amba_device_add() failed (-19) for /replicator@2012 OF: amba_device_add() failed (-19) for /cpu-debug@2201 OF: amba_device_add() failed (-19) for /etm@2204 OF: amba_device_add() failed (-19) for /cti@2202 OF: amba_device_add() failed (-19) for /funnel@220c OF: amba_device_add() failed (-19) for /cpu-debug@2211 OF: amba_device_add() failed (-19) for /etm@2214 OF: amba_device_add() failed (-19) for /cti@2212 OF: amba_device_add() failed (-19) for /cpu-debug@2301 OF: amba_device_add() failed (-19) for /etm@2304 OF: amba_device_add() failed (-19) for /cti@2302 OF: amba_device_add() failed (-19) for /funnel@230c OF: amba_device_add() failed (-19) for /cpu-debug@2311 OF: amba_device_add() failed (-19) for /etm@2314 OF: amba_device_add() failed (-19) for /cti@2312 OF: amba_device_add() failed (-19) for /cpu-debug@2321 OF: amba_device_add() failed (-19) for /etm@2324 OF: amba_device_add() failed (-19) for /cti@2322 OF: amba_device_add() failed (-19) for /cpu-debug@2331 OF: amba_device_add() failed (-19) for /etm@2334 OF: amba_device_add() failed (-19) for /cti@2332 OF: amba_device_add() failed (-19) for /cti@2002 OF: amba_device_add() failed (-19) for /cti@2011 OF: amba_device_add() failed (-19) for /funnel@2013 OF: amba_device_add() failed (-19) for /etf@2014 OF: amba_device_add() failed (-19) for /funnel@2015 OF: amba_device_add() failed (-19) for /cti@2016 These are working fine with deferred probe in the mainline. I tried the hack you have suggested here(rather Tony's version), also tried with fw_devlink=0 and fw_devlink=1 && fw_devlink.strict=0 No change in the behaviour. The DTS are in arch/arm64/boot/dts/arm/juno-*-scmi.dts and there coresight devices are mostly in juno-cs-r1r2.dtsi Let me know if there is anything obvious or you want me to bisect which means I need more time. I can do that next week. -- Regards, Sudeep ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
* Saravana Kannan [220701 08:21]:
> On Fri, Jul 1, 2022 at 1:10 AM Saravana Kannan wrote:
> >
> > On Thu, Jun 30, 2022 at 11:12 PM Tony Lindgren wrote:
> > >
> > > * Tony Lindgren [220701 08:33]:
> > > > * Saravana Kannan [220630 23:25]:
> > > > > On Thu, Jun 30, 2022 at 4:26 PM Rob Herring wrote:
> > > > > >
> > > > > > On Thu, Jun 30, 2022 at 5:11 PM Saravana Kannan
> > > > > > wrote:
> > > > > > >
> > > > > > > On Mon, Jun 27, 2022 at 2:10 AM Tony Lindgren
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > * Saravana Kannan [220623 08:17]:
> > > > > > > > > On Thu, Jun 23, 2022 at 12:01 AM Tony Lindgren
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > * Saravana Kannan [220622 19:05]:
> > > > > > > > > > > On Tue, Jun 21, 2022 at 9:59 PM Tony Lindgren
> > > > > > > > > > > wrote:
> > > > > > > > > > > > This issue is no directly related fw_devlink. It is a
> > > > > > > > > > > > side effect of
> > > > > > > > > > > > removing driver_deferred_probe_check_state(). We no
> > > > > > > > > > > > longer return
> > > > > > > > > > > > -EPROBE_DEFER at the end of
> > > > > > > > > > > > driver_deferred_probe_check_state().
> > > > > > > > > > >
> > > > > > > > > > > Yes, I understand the issue. But
> > > > > > > > > > > driver_deferred_probe_check_state()
> > > > > > > > > > > was deleted because fw_devlink=on should have short
> > > > > > > > > > > circuited the
> > > > > > > > > > > probe attempt with an -EPROBE_DEFER before reaching the
> > > > > > > > > > > bus/driver
> > > > > > > > > > > probe function and hitting this -ENOENT failure. That's
> > > > > > > > > > > why I was
> > > > > > > > > > > asking the other questions.
> > > > > > > > > >
> > > > > > > > > > OK. So where is the -EPROBE_DEFER supposed to happen without
> > > > > > > > > > driver_deferred_probe_check_state() then?
> > > > > > > > >
> > > > > > > > > device_links_check_suppliers() call inside really_probe()
> > > > > > > > > would short
> > > > > > > > > circuit and return an -EPROBE_DEFER if the device links are
> > > > > > > > > created as
> > > > > > > > > expected.
> > > > > > > >
> > > > > > > > OK
> > > > > > > >
> > > > > > > > > > Hmm so I'm not seeing any supplier for the top level ocp
> > > > > > > > > > device in
> > > > > > > > > > the booting case without your patches. I see the suppliers
> > > > > > > > > > for the
> > > > > > > > > > ocp child device instances only.
> > > > > > > > >
> > > > > > > > > Hmmm... this is strange (that the device link isn't there),
> > > > > > > > > but this
> > > > > > > > > is what I suspected.
> > > > > > > >
> > > > > > > > Yup, maybe it's because of the supplier being a device in the
> > > > > > > > child
> > > > > > > > interconnect for the ocp.
> > > > > > >
> > > > > > > Ugh... yeah, this is why the normal (not SYNC_STATE_ONLY) device
> > > > > > > link
> > > > > > > isn't being created.
> > > > > > >
> > > > > > > So the aggregated view is something like (I had to set tabs = 4
> > > > > > > space
> > > > > > > to fit it within 80 cols):
> > > > > > >
> > > > > > > ocp: ocp { <= Consumer
> > > > > > > compatible = "simple-pm-bus";
> > > > > > > power-domains = <_per>; <=== Supplier ref
> > > > > > >
> > > > > > > l4_wkup: interconnect@44c0 {
> > > > > > > compatible = "ti,am33xx-l4-wkup", "simple-pm-bus";
> > > > > > >
> > > > > > > segment@20 { /* 0x44e0 */
> > > > > > > compatible = "simple-pm-bus";
> > > > > > >
> > > > > > > target-module@0 { /* 0x44e0, ap 8 58.0 */
> > > > > > > compatible = "ti,sysc-omap4", "ti,sysc";
> > > > > > >
> > > > > > > prcm: prcm@0 {
> > > > > > > compatible = "ti,am3-prcm", "simple-bus";
> > > > > > >
> > > > > > > prm_per: prm@c00 { <= Actual
> > > > > > > Supplier
> > > > > > > compatible = "ti,am3-prm-inst",
> > > > > > > "ti,omap-prm-inst";
> > > > > > > };
> > > > > > > };
> > > > > > > };
> > > > > > > };
> > > > > > > };
> > > > > > > };
> > > > > > >
> > > > > > > The power-domain supplier is the great-great-great-grand-child of
> > > > > > > the
> > > > > > > consumer. It's not clear to me how this is valid. What does it
> > > > > > > even
> > > > > > > mean?
> > > > > > >
> > > > > > > Rob, is this considered a valid DT?
> > > > > >
> > > > > > Valid DT for broken h/w.
> > > > >
> > > > > I'm not sure even in that case it's valid. When the parent device is
> > > > > in reset (when the SoC is coming out of reset), there's no way the
> > > > > descendant is functional. And if the descendant is not functional, how
> > > > > is the parent device powered up? This just feels like an incorrect
> > > > > representation of the real h/w.
> > > >
>
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
On Fri, Jul 1, 2022 at 1:10 AM Saravana Kannan wrote:
>
> On Thu, Jun 30, 2022 at 11:12 PM Tony Lindgren wrote:
> >
> > * Tony Lindgren [220701 08:33]:
> > > * Saravana Kannan [220630 23:25]:
> > > > On Thu, Jun 30, 2022 at 4:26 PM Rob Herring wrote:
> > > > >
> > > > > On Thu, Jun 30, 2022 at 5:11 PM Saravana Kannan
> > > > > wrote:
> > > > > >
> > > > > > On Mon, Jun 27, 2022 at 2:10 AM Tony Lindgren
> > > > > > wrote:
> > > > > > >
> > > > > > > * Saravana Kannan [220623 08:17]:
> > > > > > > > On Thu, Jun 23, 2022 at 12:01 AM Tony Lindgren
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > * Saravana Kannan [220622 19:05]:
> > > > > > > > > > On Tue, Jun 21, 2022 at 9:59 PM Tony Lindgren
> > > > > > > > > > wrote:
> > > > > > > > > > > This issue is no directly related fw_devlink. It is a
> > > > > > > > > > > side effect of
> > > > > > > > > > > removing driver_deferred_probe_check_state(). We no
> > > > > > > > > > > longer return
> > > > > > > > > > > -EPROBE_DEFER at the end of
> > > > > > > > > > > driver_deferred_probe_check_state().
> > > > > > > > > >
> > > > > > > > > > Yes, I understand the issue. But
> > > > > > > > > > driver_deferred_probe_check_state()
> > > > > > > > > > was deleted because fw_devlink=on should have short
> > > > > > > > > > circuited the
> > > > > > > > > > probe attempt with an -EPROBE_DEFER before reaching the
> > > > > > > > > > bus/driver
> > > > > > > > > > probe function and hitting this -ENOENT failure. That's why
> > > > > > > > > > I was
> > > > > > > > > > asking the other questions.
> > > > > > > > >
> > > > > > > > > OK. So where is the -EPROBE_DEFER supposed to happen without
> > > > > > > > > driver_deferred_probe_check_state() then?
> > > > > > > >
> > > > > > > > device_links_check_suppliers() call inside really_probe() would
> > > > > > > > short
> > > > > > > > circuit and return an -EPROBE_DEFER if the device links are
> > > > > > > > created as
> > > > > > > > expected.
> > > > > > >
> > > > > > > OK
> > > > > > >
> > > > > > > > > Hmm so I'm not seeing any supplier for the top level ocp
> > > > > > > > > device in
> > > > > > > > > the booting case without your patches. I see the suppliers
> > > > > > > > > for the
> > > > > > > > > ocp child device instances only.
> > > > > > > >
> > > > > > > > Hmmm... this is strange (that the device link isn't there), but
> > > > > > > > this
> > > > > > > > is what I suspected.
> > > > > > >
> > > > > > > Yup, maybe it's because of the supplier being a device in the
> > > > > > > child
> > > > > > > interconnect for the ocp.
> > > > > >
> > > > > > Ugh... yeah, this is why the normal (not SYNC_STATE_ONLY) device
> > > > > > link
> > > > > > isn't being created.
> > > > > >
> > > > > > So the aggregated view is something like (I had to set tabs = 4
> > > > > > space
> > > > > > to fit it within 80 cols):
> > > > > >
> > > > > > ocp: ocp { <= Consumer
> > > > > > compatible = "simple-pm-bus";
> > > > > > power-domains = <_per>; <=== Supplier ref
> > > > > >
> > > > > > l4_wkup: interconnect@44c0 {
> > > > > > compatible = "ti,am33xx-l4-wkup", "simple-pm-bus";
> > > > > >
> > > > > > segment@20 { /* 0x44e0 */
> > > > > > compatible = "simple-pm-bus";
> > > > > >
> > > > > > target-module@0 { /* 0x44e0, ap 8 58.0 */
> > > > > > compatible = "ti,sysc-omap4", "ti,sysc";
> > > > > >
> > > > > > prcm: prcm@0 {
> > > > > > compatible = "ti,am3-prcm", "simple-bus";
> > > > > >
> > > > > > prm_per: prm@c00 { <= Actual
> > > > > > Supplier
> > > > > > compatible = "ti,am3-prm-inst",
> > > > > > "ti,omap-prm-inst";
> > > > > > };
> > > > > > };
> > > > > > };
> > > > > > };
> > > > > > };
> > > > > > };
> > > > > >
> > > > > > The power-domain supplier is the great-great-great-grand-child of
> > > > > > the
> > > > > > consumer. It's not clear to me how this is valid. What does it even
> > > > > > mean?
> > > > > >
> > > > > > Rob, is this considered a valid DT?
> > > > >
> > > > > Valid DT for broken h/w.
> > > >
> > > > I'm not sure even in that case it's valid. When the parent device is
> > > > in reset (when the SoC is coming out of reset), there's no way the
> > > > descendant is functional. And if the descendant is not functional, how
> > > > is the parent device powered up? This just feels like an incorrect
> > > > representation of the real h/w.
> > >
> > > It should be correct representation based on scanning the interconnects
> > > and looking at the documentation. Some interconnect parts are wired
> > > always-on and some interconnect instances may be dual-mapped.
>
> Thanks for helping to debug this. Appreciate it.
>
>
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
On Thu, Jun 30, 2022 at 11:12 PM Tony Lindgren wrote:
>
> * Tony Lindgren [220701 08:33]:
> > * Saravana Kannan [220630 23:25]:
> > > On Thu, Jun 30, 2022 at 4:26 PM Rob Herring wrote:
> > > >
> > > > On Thu, Jun 30, 2022 at 5:11 PM Saravana Kannan
> > > > wrote:
> > > > >
> > > > > On Mon, Jun 27, 2022 at 2:10 AM Tony Lindgren
> > > > > wrote:
> > > > > >
> > > > > > * Saravana Kannan [220623 08:17]:
> > > > > > > On Thu, Jun 23, 2022 at 12:01 AM Tony Lindgren
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > * Saravana Kannan [220622 19:05]:
> > > > > > > > > On Tue, Jun 21, 2022 at 9:59 PM Tony Lindgren
> > > > > > > > > wrote:
> > > > > > > > > > This issue is no directly related fw_devlink. It is a side
> > > > > > > > > > effect of
> > > > > > > > > > removing driver_deferred_probe_check_state(). We no longer
> > > > > > > > > > return
> > > > > > > > > > -EPROBE_DEFER at the end of
> > > > > > > > > > driver_deferred_probe_check_state().
> > > > > > > > >
> > > > > > > > > Yes, I understand the issue. But
> > > > > > > > > driver_deferred_probe_check_state()
> > > > > > > > > was deleted because fw_devlink=on should have short circuited
> > > > > > > > > the
> > > > > > > > > probe attempt with an -EPROBE_DEFER before reaching the
> > > > > > > > > bus/driver
> > > > > > > > > probe function and hitting this -ENOENT failure. That's why I
> > > > > > > > > was
> > > > > > > > > asking the other questions.
> > > > > > > >
> > > > > > > > OK. So where is the -EPROBE_DEFER supposed to happen without
> > > > > > > > driver_deferred_probe_check_state() then?
> > > > > > >
> > > > > > > device_links_check_suppliers() call inside really_probe() would
> > > > > > > short
> > > > > > > circuit and return an -EPROBE_DEFER if the device links are
> > > > > > > created as
> > > > > > > expected.
> > > > > >
> > > > > > OK
> > > > > >
> > > > > > > > Hmm so I'm not seeing any supplier for the top level ocp device
> > > > > > > > in
> > > > > > > > the booting case without your patches. I see the suppliers for
> > > > > > > > the
> > > > > > > > ocp child device instances only.
> > > > > > >
> > > > > > > Hmmm... this is strange (that the device link isn't there), but
> > > > > > > this
> > > > > > > is what I suspected.
> > > > > >
> > > > > > Yup, maybe it's because of the supplier being a device in the child
> > > > > > interconnect for the ocp.
> > > > >
> > > > > Ugh... yeah, this is why the normal (not SYNC_STATE_ONLY) device link
> > > > > isn't being created.
> > > > >
> > > > > So the aggregated view is something like (I had to set tabs = 4 space
> > > > > to fit it within 80 cols):
> > > > >
> > > > > ocp: ocp { <= Consumer
> > > > > compatible = "simple-pm-bus";
> > > > > power-domains = <_per>; <=== Supplier ref
> > > > >
> > > > > l4_wkup: interconnect@44c0 {
> > > > > compatible = "ti,am33xx-l4-wkup", "simple-pm-bus";
> > > > >
> > > > > segment@20 { /* 0x44e0 */
> > > > > compatible = "simple-pm-bus";
> > > > >
> > > > > target-module@0 { /* 0x44e0, ap 8 58.0 */
> > > > > compatible = "ti,sysc-omap4", "ti,sysc";
> > > > >
> > > > > prcm: prcm@0 {
> > > > > compatible = "ti,am3-prcm", "simple-bus";
> > > > >
> > > > > prm_per: prm@c00 { <= Actual Supplier
> > > > > compatible = "ti,am3-prm-inst",
> > > > > "ti,omap-prm-inst";
> > > > > };
> > > > > };
> > > > > };
> > > > > };
> > > > > };
> > > > > };
> > > > >
> > > > > The power-domain supplier is the great-great-great-grand-child of the
> > > > > consumer. It's not clear to me how this is valid. What does it even
> > > > > mean?
> > > > >
> > > > > Rob, is this considered a valid DT?
> > > >
> > > > Valid DT for broken h/w.
> > >
> > > I'm not sure even in that case it's valid. When the parent device is
> > > in reset (when the SoC is coming out of reset), there's no way the
> > > descendant is functional. And if the descendant is not functional, how
> > > is the parent device powered up? This just feels like an incorrect
> > > representation of the real h/w.
> >
> > It should be correct representation based on scanning the interconnects
> > and looking at the documentation. Some interconnect parts are wired
> > always-on and some interconnect instances may be dual-mapped.
Thanks for helping to debug this. Appreciate it.
> >
> > We have a quirk to probe prm/prcm first with pdata_quirks_init_clocks().
:'(
I checked out the code. These prm devices just get populated with NULL
as the parent. So they are effectively top level devices from the
perspective of driver core.
> > Maybe that also now fails in addition to the top level interconnect
> > probing no longer
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
Hi Saravana,
On Fri, Jul 1, 2022 at 1:11 AM Saravana Kannan wrote:
> On Mon, Jun 27, 2022 at 2:10 AM Tony Lindgren wrote:
> > * Saravana Kannan [220623 08:17]:
> > > On Thu, Jun 23, 2022 at 12:01 AM Tony Lindgren wrote:
> > > > * Saravana Kannan [220622 19:05]:
> > > > > On Tue, Jun 21, 2022 at 9:59 PM Tony Lindgren
> > > > > wrote:
> > > > > > This issue is no directly related fw_devlink. It is a side effect of
> > > > > > removing driver_deferred_probe_check_state(). We no longer return
> > > > > > -EPROBE_DEFER at the end of driver_deferred_probe_check_state().
> > > > >
> > > > > Yes, I understand the issue. But driver_deferred_probe_check_state()
> > > > > was deleted because fw_devlink=on should have short circuited the
> > > > > probe attempt with an -EPROBE_DEFER before reaching the bus/driver
> > > > > probe function and hitting this -ENOENT failure. That's why I was
> > > > > asking the other questions.
> > > >
> > > > OK. So where is the -EPROBE_DEFER supposed to happen without
> > > > driver_deferred_probe_check_state() then?
> > >
> > > device_links_check_suppliers() call inside really_probe() would short
> > > circuit and return an -EPROBE_DEFER if the device links are created as
> > > expected.
> >
> > OK
> >
> > > > Hmm so I'm not seeing any supplier for the top level ocp device in
> > > > the booting case without your patches. I see the suppliers for the
> > > > ocp child device instances only.
> > >
> > > Hmmm... this is strange (that the device link isn't there), but this
> > > is what I suspected.
> >
> > Yup, maybe it's because of the supplier being a device in the child
> > interconnect for the ocp.
>
> Ugh... yeah, this is why the normal (not SYNC_STATE_ONLY) device link
> isn't being created.
>
> So the aggregated view is something like (I had to set tabs = 4 space
> to fit it within 80 cols):
>
> ocp: ocp { <= Consumer
> compatible = "simple-pm-bus";
> power-domains = <_per>; <=== Supplier ref
>
> l4_wkup: interconnect@44c0 {
> compatible = "ti,am33xx-l4-wkup", "simple-pm-bus";
>
> segment@20 { /* 0x44e0 */
> compatible = "simple-pm-bus";
>
> target-module@0 { /* 0x44e0, ap 8 58.0 */
> compatible = "ti,sysc-omap4", "ti,sysc";
>
> prcm: prcm@0 {
> compatible = "ti,am3-prcm", "simple-bus";
>
> prm_per: prm@c00 { <= Actual Supplier
> compatible = "ti,am3-prm-inst",
> "ti,omap-prm-inst";
> };
> };
> };
> };
> };
> };
>
> The power-domain supplier is the great-great-great-grand-child of the
> consumer. It's not clear to me how this is valid. What does it even
> mean?
>
> Rob, is this considered a valid DT?
>
> Geert, thoughts on whether this is a correct use of simple-pm-bus device?
Well, if the hardware is wired that way...
It's not that dissimilar from CPU cores, and interrupt and GPIO
controllers in power domains and clocked by controllable clocks:
you can cut the branch you're sitting on, and you have to be careful
when going to sleep, and make sure your wake-up sources are still
functional.
> Also, how is the power domain attach/get working in this case? As far
> as I can tell, at least for "simple-pm-bus" devices, the pm domain
> attachment is happening under:
> really_probe() -> call_driver_probe -> platform_probe() ->
> dev_pm_domain_attach()
>
> So, how is the pm domain attach succeeding in the first place without
> my changes?
That's a software thing ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
Hi Saravana,
On Fri, Jul 1, 2022 at 2:37 AM Saravana Kannan wrote:
> On Thu, Jun 23, 2022 at 5:08 AM Alexander Stein
> wrote:
> > Am Dienstag, 21. Juni 2022, 09:28:43 CEST schrieb Tony Lindgren:
> > > * 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
You missed the "driver" in arch/arm/mach-imx/src.c ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: (EXT) Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
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;
}
-Saravana
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
* Tony Lindgren [220701 08:33]:
> * Saravana Kannan [220630 23:25]:
> > On Thu, Jun 30, 2022 at 4:26 PM Rob Herring wrote:
> > >
> > > On Thu, Jun 30, 2022 at 5:11 PM Saravana Kannan
> > > wrote:
> > > >
> > > > On Mon, Jun 27, 2022 at 2:10 AM Tony Lindgren wrote:
> > > > >
> > > > > * Saravana Kannan [220623 08:17]:
> > > > > > On Thu, Jun 23, 2022 at 12:01 AM Tony Lindgren
> > > > > > wrote:
> > > > > > >
> > > > > > > * Saravana Kannan [220622 19:05]:
> > > > > > > > On Tue, Jun 21, 2022 at 9:59 PM Tony Lindgren
> > > > > > > > wrote:
> > > > > > > > > This issue is no directly related fw_devlink. It is a side
> > > > > > > > > effect of
> > > > > > > > > removing driver_deferred_probe_check_state(). We no longer
> > > > > > > > > return
> > > > > > > > > -EPROBE_DEFER at the end of
> > > > > > > > > driver_deferred_probe_check_state().
> > > > > > > >
> > > > > > > > Yes, I understand the issue. But
> > > > > > > > driver_deferred_probe_check_state()
> > > > > > > > was deleted because fw_devlink=on should have short circuited
> > > > > > > > the
> > > > > > > > probe attempt with an -EPROBE_DEFER before reaching the
> > > > > > > > bus/driver
> > > > > > > > probe function and hitting this -ENOENT failure. That's why I
> > > > > > > > was
> > > > > > > > asking the other questions.
> > > > > > >
> > > > > > > OK. So where is the -EPROBE_DEFER supposed to happen without
> > > > > > > driver_deferred_probe_check_state() then?
> > > > > >
> > > > > > device_links_check_suppliers() call inside really_probe() would
> > > > > > short
> > > > > > circuit and return an -EPROBE_DEFER if the device links are created
> > > > > > as
> > > > > > expected.
> > > > >
> > > > > OK
> > > > >
> > > > > > > Hmm so I'm not seeing any supplier for the top level ocp device in
> > > > > > > the booting case without your patches. I see the suppliers for the
> > > > > > > ocp child device instances only.
> > > > > >
> > > > > > Hmmm... this is strange (that the device link isn't there), but this
> > > > > > is what I suspected.
> > > > >
> > > > > Yup, maybe it's because of the supplier being a device in the child
> > > > > interconnect for the ocp.
> > > >
> > > > Ugh... yeah, this is why the normal (not SYNC_STATE_ONLY) device link
> > > > isn't being created.
> > > >
> > > > So the aggregated view is something like (I had to set tabs = 4 space
> > > > to fit it within 80 cols):
> > > >
> > > > ocp: ocp { <= Consumer
> > > > compatible = "simple-pm-bus";
> > > > power-domains = <_per>; <=== Supplier ref
> > > >
> > > > l4_wkup: interconnect@44c0 {
> > > > compatible = "ti,am33xx-l4-wkup", "simple-pm-bus";
> > > >
> > > > segment@20 { /* 0x44e0 */
> > > > compatible = "simple-pm-bus";
> > > >
> > > > target-module@0 { /* 0x44e0, ap 8 58.0 */
> > > > compatible = "ti,sysc-omap4", "ti,sysc";
> > > >
> > > > prcm: prcm@0 {
> > > > compatible = "ti,am3-prcm", "simple-bus";
> > > >
> > > > prm_per: prm@c00 { <= Actual Supplier
> > > > compatible = "ti,am3-prm-inst",
> > > > "ti,omap-prm-inst";
> > > > };
> > > > };
> > > > };
> > > > };
> > > > };
> > > > };
> > > >
> > > > The power-domain supplier is the great-great-great-grand-child of the
> > > > consumer. It's not clear to me how this is valid. What does it even
> > > > mean?
> > > >
> > > > Rob, is this considered a valid DT?
> > >
> > > Valid DT for broken h/w.
> >
> > I'm not sure even in that case it's valid. When the parent device is
> > in reset (when the SoC is coming out of reset), there's no way the
> > descendant is functional. And if the descendant is not functional, how
> > is the parent device powered up? This just feels like an incorrect
> > representation of the real h/w.
>
> It should be correct representation based on scanning the interconnects
> and looking at the documentation. Some interconnect parts are wired
> always-on and some interconnect instances may be dual-mapped.
>
> We have a quirk to probe prm/prcm first with pdata_quirks_init_clocks().
> Maybe that also now fails in addition to the top level interconnect
> probing no longer producing -EPROBE_DEFER.
>
> > > So the domain must be default on and then simple-pm-bus is going to
> > > hold a reference to the domain preventing it from ever getting powered
> > > off and things seem to work. Except what happens during suspend?
> >
> > But how can simple-pm-bus even get a reference? The PM domain can't
> > get added until we are well into the probe of the simple-pm-bus and
> > AFAICT the genpd attach is done before the driver probe is even
> > called.
>
> The prm/prcm gets of_platform_populate() called on it early.
The
Re: (EXT) Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
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:
> [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 !
> [0.013251] Failed to map interrupt for
> /soc@0/bus@3040/timer@306a
> [0.020152] Failed to initialize '/soc@0/bus@3040/timer@306a':
> -22
I kept the timestamps to show that these errors happen very early. So now the
usage of the "global" interrupt parent, set at line 18,
> interrupt-parent = <>;
is not possible at this point of boot time.
Best regards,
Alexander
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
* Saravana Kannan [220630 23:25]:
> On Thu, Jun 30, 2022 at 4:26 PM Rob Herring wrote:
> >
> > On Thu, Jun 30, 2022 at 5:11 PM Saravana Kannan
> > wrote:
> > >
> > > On Mon, Jun 27, 2022 at 2:10 AM Tony Lindgren wrote:
> > > >
> > > > * Saravana Kannan [220623 08:17]:
> > > > > On Thu, Jun 23, 2022 at 12:01 AM Tony Lindgren
> > > > > wrote:
> > > > > >
> > > > > > * Saravana Kannan [220622 19:05]:
> > > > > > > On Tue, Jun 21, 2022 at 9:59 PM Tony Lindgren
> > > > > > > wrote:
> > > > > > > > This issue is no directly related fw_devlink. It is a side
> > > > > > > > effect of
> > > > > > > > removing driver_deferred_probe_check_state(). We no longer
> > > > > > > > return
> > > > > > > > -EPROBE_DEFER at the end of driver_deferred_probe_check_state().
> > > > > > >
> > > > > > > Yes, I understand the issue. But
> > > > > > > driver_deferred_probe_check_state()
> > > > > > > was deleted because fw_devlink=on should have short circuited the
> > > > > > > probe attempt with an -EPROBE_DEFER before reaching the
> > > > > > > bus/driver
> > > > > > > probe function and hitting this -ENOENT failure. That's why I was
> > > > > > > asking the other questions.
> > > > > >
> > > > > > OK. So where is the -EPROBE_DEFER supposed to happen without
> > > > > > driver_deferred_probe_check_state() then?
> > > > >
> > > > > device_links_check_suppliers() call inside really_probe() would short
> > > > > circuit and return an -EPROBE_DEFER if the device links are created as
> > > > > expected.
> > > >
> > > > OK
> > > >
> > > > > > Hmm so I'm not seeing any supplier for the top level ocp device in
> > > > > > the booting case without your patches. I see the suppliers for the
> > > > > > ocp child device instances only.
> > > > >
> > > > > Hmmm... this is strange (that the device link isn't there), but this
> > > > > is what I suspected.
> > > >
> > > > Yup, maybe it's because of the supplier being a device in the child
> > > > interconnect for the ocp.
> > >
> > > Ugh... yeah, this is why the normal (not SYNC_STATE_ONLY) device link
> > > isn't being created.
> > >
> > > So the aggregated view is something like (I had to set tabs = 4 space
> > > to fit it within 80 cols):
> > >
> > > ocp: ocp { <= Consumer
> > > compatible = "simple-pm-bus";
> > > power-domains = <_per>; <=== Supplier ref
> > >
> > > l4_wkup: interconnect@44c0 {
> > > compatible = "ti,am33xx-l4-wkup", "simple-pm-bus";
> > >
> > > segment@20 { /* 0x44e0 */
> > > compatible = "simple-pm-bus";
> > >
> > > target-module@0 { /* 0x44e0, ap 8 58.0 */
> > > compatible = "ti,sysc-omap4", "ti,sysc";
> > >
> > > prcm: prcm@0 {
> > > compatible = "ti,am3-prcm", "simple-bus";
> > >
> > > prm_per: prm@c00 { <= Actual Supplier
> > > compatible = "ti,am3-prm-inst",
> > > "ti,omap-prm-inst";
> > > };
> > > };
> > > };
> > > };
> > > };
> > > };
> > >
> > > The power-domain supplier is the great-great-great-grand-child of the
> > > consumer. It's not clear to me how this is valid. What does it even
> > > mean?
> > >
> > > Rob, is this considered a valid DT?
> >
> > Valid DT for broken h/w.
>
> I'm not sure even in that case it's valid. When the parent device is
> in reset (when the SoC is coming out of reset), there's no way the
> descendant is functional. And if the descendant is not functional, how
> is the parent device powered up? This just feels like an incorrect
> representation of the real h/w.
It should be correct representation based on scanning the interconnects
and looking at the documentation. Some interconnect parts are wired
always-on and some interconnect instances may be dual-mapped.
We have a quirk to probe prm/prcm first with pdata_quirks_init_clocks().
Maybe that also now fails in addition to the top level interconnect
probing no longer producing -EPROBE_DEFER.
> > So the domain must be default on and then simple-pm-bus is going to
> > hold a reference to the domain preventing it from ever getting powered
> > off and things seem to work. Except what happens during suspend?
>
> But how can simple-pm-bus even get a reference? The PM domain can't
> get added until we are well into the probe of the simple-pm-bus and
> AFAICT the genpd attach is done before the driver probe is even
> called.
The prm/prcm gets of_platform_populate() called on it early.
Regards,
Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
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?
-Saravana
From 08b8795b6300de89502a26ba3347b2e54d43381d Mon Sep 17 00:00:00 2001
From: Saravana Kannan
Date: Thu, 30 Jun 2022 17:04:26 -0700
Subject: [PATCH] combine drivers
---
drivers/irqchip/irq-imx-gpcv2.c | 19 ---
drivers/soc/imx/gpcv2.c | 10 ++
2 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/irqchip/irq-imx-gpcv2.c b/drivers/irqchip/irq-imx-gpcv2.c
index b9c22f764b4d..621211e5800a 100644
--- a/drivers/irqchip/irq-imx-gpcv2.c
+++ b/drivers/irqchip/irq-imx-gpcv2.c
@@ -199,13 +199,13 @@ static const struct of_device_id gpcv2_of_match[] = {
{ /* END */ }
};
-static int __init imx_gpcv2_irqchip_init(struct device_node *node,
- struct device_node *parent)
+int imx_gpcv2_irqchip_init(struct platform_device *pdev,
+ unsigned long core_num)
{
+ struct device_node *node = pdev->dev.of_node;
+ struct device_node *parent = of_irq_find_parent(node);
struct irq_domain *parent_domain, *domain;
struct gpcv2_irqchip_data *cd;
- const struct of_device_id *id;
- unsigned long core_num;
int i;
if (!parent) {
@@ -213,14 +213,6 @@ static int __init imx_gpcv2_irqchip_init(struct device_node *node,
return -ENODEV;
}
- id = of_match_node(gpcv2_of_match, node);
- if (!id) {
- pr_err("%pOF: unknown compatibility string\n", node);
- return -ENODEV;
- }
-
- core_num = (unsigned long)id->data;
-
parent_domain = irq_find_host(parent);
if (!parent_domain) {
pr_err("%pOF: unable to get parent domain\n", node);
@@ -285,6 +277,3 @@ static int __init imx_gpcv2_irqchip_init(struct device_node *node,
of_node_clear_flag(node, OF_POPULATED);
return 0;
}
-
-IRQCHIP_DECLARE(imx_gpcv2_imx7d, "fsl,imx7d-gpc", imx_gpcv2_irqchip_init);
-IRQCHIP_DECLARE(imx_gpcv2_imx8mq, "fsl,imx8mq-gpc", imx_gpcv2_irqchip_init);
diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 85aa86e1338a..3ca5382ff5d3 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -305,6 +305,7 @@ struct imx_pgc_domain_data {
size_t domains_num;
const struct regmap_access_table *reg_access_table;
const struct imx_pgc_regs *pgc_regs;
+ unsigned long irq_core_num;
};
static inline struct imx_pgc_domain *
@@ -549,6 +550,7 @@ static const struct imx_pgc_domain_data imx7_pgc_domain_data = {
.domains_num = ARRAY_SIZE(imx7_pgc_domains),
.reg_access_table = _access_table,
.pgc_regs = _pgc_regs,
+ .irq_core_num = 2,
};
static const struct imx_pgc_domain imx8m_pgc_domains[] = {
@@ -718,6 +720,7 @@ static const struct imx_pgc_domain_data imx8m_pgc_domain_data = {
.domains_num = ARRAY_SIZE(imx8m_pgc_domains),
.reg_access_table = _access_table,
.pgc_regs = _pgc_regs,
+ .irq_core_num = 4,
};
static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
@@ -1425,6 +1428,9 @@ static struct platform_driver imx_pgc_domain_driver = {
};
builtin_platform_driver(imx_pgc_domain_driver)
+extern int imx_gpcv2_irqchip_init(struct platform_device *pdev,
+ unsigned long core_num);
+
static int imx_gpcv2_probe(struct platform_device *pdev)
{
const struct imx_pgc_domain_data *domain_data =
@@ -1444,6
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
On Thu, Jun 30, 2022 at 4:26 PM Rob Herring wrote:
>
> On Thu, Jun 30, 2022 at 5:11 PM Saravana Kannan wrote:
> >
> > On Mon, Jun 27, 2022 at 2:10 AM Tony Lindgren wrote:
> > >
> > > * Saravana Kannan [220623 08:17]:
> > > > On Thu, Jun 23, 2022 at 12:01 AM Tony Lindgren wrote:
> > > > >
> > > > > * Saravana Kannan [220622 19:05]:
> > > > > > On Tue, Jun 21, 2022 at 9:59 PM Tony Lindgren
> > > > > > wrote:
> > > > > > > This issue is no directly related fw_devlink. It is a side effect
> > > > > > > of
> > > > > > > removing driver_deferred_probe_check_state(). We no longer return
> > > > > > > -EPROBE_DEFER at the end of driver_deferred_probe_check_state().
> > > > > >
> > > > > > Yes, I understand the issue. But driver_deferred_probe_check_state()
> > > > > > was deleted because fw_devlink=on should have short circuited the
> > > > > > probe attempt with an -EPROBE_DEFER before reaching the bus/driver
> > > > > > probe function and hitting this -ENOENT failure. That's why I was
> > > > > > asking the other questions.
> > > > >
> > > > > OK. So where is the -EPROBE_DEFER supposed to happen without
> > > > > driver_deferred_probe_check_state() then?
> > > >
> > > > device_links_check_suppliers() call inside really_probe() would short
> > > > circuit and return an -EPROBE_DEFER if the device links are created as
> > > > expected.
> > >
> > > OK
> > >
> > > > > Hmm so I'm not seeing any supplier for the top level ocp device in
> > > > > the booting case without your patches. I see the suppliers for the
> > > > > ocp child device instances only.
> > > >
> > > > Hmmm... this is strange (that the device link isn't there), but this
> > > > is what I suspected.
> > >
> > > Yup, maybe it's because of the supplier being a device in the child
> > > interconnect for the ocp.
> >
> > Ugh... yeah, this is why the normal (not SYNC_STATE_ONLY) device link
> > isn't being created.
> >
> > So the aggregated view is something like (I had to set tabs = 4 space
> > to fit it within 80 cols):
> >
> > ocp: ocp { <= Consumer
> > compatible = "simple-pm-bus";
> > power-domains = <_per>; <=== Supplier ref
> >
> > l4_wkup: interconnect@44c0 {
> > compatible = "ti,am33xx-l4-wkup", "simple-pm-bus";
> >
> > segment@20 { /* 0x44e0 */
> > compatible = "simple-pm-bus";
> >
> > target-module@0 { /* 0x44e0, ap 8 58.0 */
> > compatible = "ti,sysc-omap4", "ti,sysc";
> >
> > prcm: prcm@0 {
> > compatible = "ti,am3-prcm", "simple-bus";
> >
> > prm_per: prm@c00 { <= Actual Supplier
> > compatible = "ti,am3-prm-inst",
> > "ti,omap-prm-inst";
> > };
> > };
> > };
> > };
> > };
> > };
> >
> > The power-domain supplier is the great-great-great-grand-child of the
> > consumer. It's not clear to me how this is valid. What does it even
> > mean?
> >
> > Rob, is this considered a valid DT?
>
> Valid DT for broken h/w.
I'm not sure even in that case it's valid. When the parent device is
in reset (when the SoC is coming out of reset), there's no way the
descendant is functional. And if the descendant is not functional, how
is the parent device powered up? This just feels like an incorrect
representation of the real h/w.
> So the domain must be default on and then simple-pm-bus is going to
> hold a reference to the domain preventing it from ever getting powered
> off and things seem to work. Except what happens during suspend?
But how can simple-pm-bus even get a reference? The PM domain can't
get added until we are well into the probe of the simple-pm-bus and
AFAICT the genpd attach is done before the driver probe is even
called.
-Saravana
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
On Thu, Jun 30, 2022 at 5:11 PM Saravana Kannan wrote:
>
> On Mon, Jun 27, 2022 at 2:10 AM Tony Lindgren wrote:
> >
> > * Saravana Kannan [220623 08:17]:
> > > On Thu, Jun 23, 2022 at 12:01 AM Tony Lindgren wrote:
> > > >
> > > > * Saravana Kannan [220622 19:05]:
> > > > > On Tue, Jun 21, 2022 at 9:59 PM Tony Lindgren
> > > > > wrote:
> > > > > > This issue is no directly related fw_devlink. It is a side effect of
> > > > > > removing driver_deferred_probe_check_state(). We no longer return
> > > > > > -EPROBE_DEFER at the end of driver_deferred_probe_check_state().
> > > > >
> > > > > Yes, I understand the issue. But driver_deferred_probe_check_state()
> > > > > was deleted because fw_devlink=on should have short circuited the
> > > > > probe attempt with an -EPROBE_DEFER before reaching the bus/driver
> > > > > probe function and hitting this -ENOENT failure. That's why I was
> > > > > asking the other questions.
> > > >
> > > > OK. So where is the -EPROBE_DEFER supposed to happen without
> > > > driver_deferred_probe_check_state() then?
> > >
> > > device_links_check_suppliers() call inside really_probe() would short
> > > circuit and return an -EPROBE_DEFER if the device links are created as
> > > expected.
> >
> > OK
> >
> > > > Hmm so I'm not seeing any supplier for the top level ocp device in
> > > > the booting case without your patches. I see the suppliers for the
> > > > ocp child device instances only.
> > >
> > > Hmmm... this is strange (that the device link isn't there), but this
> > > is what I suspected.
> >
> > Yup, maybe it's because of the supplier being a device in the child
> > interconnect for the ocp.
>
> Ugh... yeah, this is why the normal (not SYNC_STATE_ONLY) device link
> isn't being created.
>
> So the aggregated view is something like (I had to set tabs = 4 space
> to fit it within 80 cols):
>
> ocp: ocp { <= Consumer
> compatible = "simple-pm-bus";
> power-domains = <_per>; <=== Supplier ref
>
> l4_wkup: interconnect@44c0 {
> compatible = "ti,am33xx-l4-wkup", "simple-pm-bus";
>
> segment@20 { /* 0x44e0 */
> compatible = "simple-pm-bus";
>
> target-module@0 { /* 0x44e0, ap 8 58.0 */
> compatible = "ti,sysc-omap4", "ti,sysc";
>
> prcm: prcm@0 {
> compatible = "ti,am3-prcm", "simple-bus";
>
> prm_per: prm@c00 { <= Actual Supplier
> compatible = "ti,am3-prm-inst",
> "ti,omap-prm-inst";
> };
> };
> };
> };
> };
> };
>
> The power-domain supplier is the great-great-great-grand-child of the
> consumer. It's not clear to me how this is valid. What does it even
> mean?
>
> Rob, is this considered a valid DT?
Valid DT for broken h/w.
So the domain must be default on and then simple-pm-bus is going to
hold a reference to the domain preventing it from ever getting powered
off and things seem to work. Except what happens during suspend?
Rob
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
On Mon, Jun 27, 2022 at 2:10 AM Tony Lindgren wrote:
>
> * Saravana Kannan [220623 08:17]:
> > On Thu, Jun 23, 2022 at 12:01 AM Tony Lindgren wrote:
> > >
> > > * Saravana Kannan [220622 19:05]:
> > > > On Tue, Jun 21, 2022 at 9:59 PM Tony Lindgren wrote:
> > > > > This issue is no directly related fw_devlink. It is a side effect of
> > > > > removing driver_deferred_probe_check_state(). We no longer return
> > > > > -EPROBE_DEFER at the end of driver_deferred_probe_check_state().
> > > >
> > > > Yes, I understand the issue. But driver_deferred_probe_check_state()
> > > > was deleted because fw_devlink=on should have short circuited the
> > > > probe attempt with an -EPROBE_DEFER before reaching the bus/driver
> > > > probe function and hitting this -ENOENT failure. That's why I was
> > > > asking the other questions.
> > >
> > > OK. So where is the -EPROBE_DEFER supposed to happen without
> > > driver_deferred_probe_check_state() then?
> >
> > device_links_check_suppliers() call inside really_probe() would short
> > circuit and return an -EPROBE_DEFER if the device links are created as
> > expected.
>
> OK
>
> > > Hmm so I'm not seeing any supplier for the top level ocp device in
> > > the booting case without your patches. I see the suppliers for the
> > > ocp child device instances only.
> >
> > Hmmm... this is strange (that the device link isn't there), but this
> > is what I suspected.
>
> Yup, maybe it's because of the supplier being a device in the child
> interconnect for the ocp.
Ugh... yeah, this is why the normal (not SYNC_STATE_ONLY) device link
isn't being created.
So the aggregated view is something like (I had to set tabs = 4 space
to fit it within 80 cols):
ocp: ocp { <= Consumer
compatible = "simple-pm-bus";
power-domains = <_per>; <=== Supplier ref
l4_wkup: interconnect@44c0 {
compatible = "ti,am33xx-l4-wkup", "simple-pm-bus";
segment@20 { /* 0x44e0 */
compatible = "simple-pm-bus";
target-module@0 { /* 0x44e0, ap 8 58.0 */
compatible = "ti,sysc-omap4", "ti,sysc";
prcm: prcm@0 {
compatible = "ti,am3-prcm", "simple-bus";
prm_per: prm@c00 { <= Actual Supplier
compatible = "ti,am3-prm-inst", "ti,omap-prm-inst";
};
};
};
};
};
};
The power-domain supplier is the great-great-great-grand-child of the
consumer. It's not clear to me how this is valid. What does it even
mean?
Rob, is this considered a valid DT?
Geert, thoughts on whether this is a correct use of simple-pm-bus device?
Also, how is the power domain attach/get working in this case? As far
as I can tell, at least for "simple-pm-bus" devices, the pm domain
attachment is happening under:
really_probe() -> call_driver_probe -> platform_probe() ->
dev_pm_domain_attach()
So, how is the pm domain attach succeeding in the first place without
my changes?
> > Now we need to figure out why it's missing. There are only a few
> > things that could cause this and I don't see any of those. I already
> > checked to make sure the power domain in this instance had a proper
> > driver with a probe() function -- if it didn't, then that's one thing
> > that'd could have caused the missing device link. The device does seem
> > to have a proper driver, so looks like I can rule that out.
> >
> > Can you point me to the dts file that corresponds to the specific
> > board you are testing this one? I probably won't find anything, but I
> > want to rule out some of the possibilities.
>
> You can use the beaglebone black dts for example, that's
> arch/arm/boot/dts/am335x-boneblack.dts and uses am33xx.dtsi for
> ocp interconnect with simple-pm-bus.
>
> > All the device link creation logic is inside drivers/base/core.c. So
> > if you can look at the existing messages or add other stuff to figure
> > out why the device link isn't getting created, that'd be handy. In
> > either case, I'll continue staring at the DT and code to see what
> > might be happening here.
>
> In device_links_check_suppliers() I see these ocp suppliers:
>
> platform ocp: device_links_check_suppliers: 1024: supplier 44e00d00.prm:
> link->status: 0 link->flags: 01c0
> platform ocp: device_links_check_suppliers: 1024: supplier 44e01000.prm:
> link->status: 0 link->flags: 01c0
> platform ocp: device_links_check_suppliers: 1024: supplier 44e00c00.prm:
> link->status: 0 link->flags: 01c0
> platform ocp: device_links_check_suppliers: 1024: supplier 44e00e00.prm:
> link->status: 0 link->flags: 01c0
> platform ocp: device_links_check_suppliers: 1024: supplier 44e01100.prm:
> link->status: 0 link->flags: 01c0
> platform ocp: device_links_check_suppliers: 1024: supplier fixedregulator0:
> link->status: 1
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
* Saravana Kannan [220623 08:17]: > On Thu, Jun 23, 2022 at 12:01 AM Tony Lindgren wrote: > > > > * Saravana Kannan [220622 19:05]: > > > On Tue, Jun 21, 2022 at 9:59 PM Tony Lindgren wrote: > > > > This issue is no directly related fw_devlink. It is a side effect of > > > > removing driver_deferred_probe_check_state(). We no longer return > > > > -EPROBE_DEFER at the end of driver_deferred_probe_check_state(). > > > > > > Yes, I understand the issue. But driver_deferred_probe_check_state() > > > was deleted because fw_devlink=on should have short circuited the > > > probe attempt with an -EPROBE_DEFER before reaching the bus/driver > > > probe function and hitting this -ENOENT failure. That's why I was > > > asking the other questions. > > > > OK. So where is the -EPROBE_DEFER supposed to happen without > > driver_deferred_probe_check_state() then? > > device_links_check_suppliers() call inside really_probe() would short > circuit and return an -EPROBE_DEFER if the device links are created as > expected. OK > > Hmm so I'm not seeing any supplier for the top level ocp device in > > the booting case without your patches. I see the suppliers for the > > ocp child device instances only. > > Hmmm... this is strange (that the device link isn't there), but this > is what I suspected. Yup, maybe it's because of the supplier being a device in the child interconnect for the ocp. > Now we need to figure out why it's missing. There are only a few > things that could cause this and I don't see any of those. I already > checked to make sure the power domain in this instance had a proper > driver with a probe() function -- if it didn't, then that's one thing > that'd could have caused the missing device link. The device does seem > to have a proper driver, so looks like I can rule that out. > > Can you point me to the dts file that corresponds to the specific > board you are testing this one? I probably won't find anything, but I > want to rule out some of the possibilities. You can use the beaglebone black dts for example, that's arch/arm/boot/dts/am335x-boneblack.dts and uses am33xx.dtsi for ocp interconnect with simple-pm-bus. > All the device link creation logic is inside drivers/base/core.c. So > if you can look at the existing messages or add other stuff to figure > out why the device link isn't getting created, that'd be handy. In > either case, I'll continue staring at the DT and code to see what > might be happening here. In device_links_check_suppliers() I see these ocp suppliers: platform ocp: device_links_check_suppliers: 1024: supplier 44e00d00.prm: link->status: 0 link->flags: 01c0 platform ocp: device_links_check_suppliers: 1024: supplier 44e01000.prm: link->status: 0 link->flags: 01c0 platform ocp: device_links_check_suppliers: 1024: supplier 44e00c00.prm: link->status: 0 link->flags: 01c0 platform ocp: device_links_check_suppliers: 1024: supplier 44e00e00.prm: link->status: 0 link->flags: 01c0 platform ocp: device_links_check_suppliers: 1024: supplier 44e01100.prm: link->status: 0 link->flags: 01c0 platform ocp: device_links_check_suppliers: 1024: supplier fixedregulator0: link->status: 1 link->flags: 01c0 No -EPROBE_DEFER is returned in device_links_check_suppliers() for 44e00c00.prm supplier for beaglebone black for example, 0 gets returned. Regards, Tony ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
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. See the (filtered) dmesg output: > [0.744245] PM: Added domain provider from > /soc@0/bus@3000/gpc@303a/pgc/power-domain@0 [0.744756] PM: > Added domain provider from > /soc@0/bus@3000/gpc@303a/pgc/power-domain@2 [0.745012] PM: > Added domain provider from > /soc@0/bus@3000/gpc@303a/pgc/power-domain@3 [0.745268] PM: > Added domain provider from > /soc@0/bus@3000/gpc@303a/pgc/power-domain@4 [0.746121] PM: > Added domain provider from > /soc@0/bus@3000/gpc@303a/pgc/power-domain@7 [0.746400] PM: > Added domain provider from > /soc@0/bus@3000/gpc@303a/pgc/power-domain@8 [0.746665] PM: > Added domain provider from > /soc@0/bus@3000/gpc@303a/pgc/power-domain@9 [0.746927] PM: > Added domain provider from > /soc@0/bus@3000/gpc@303a/pgc/power-domain@a [0.748870] > imx8m-blk-ctrl 3832.blk-ctrl: error -ENODEV: failed to attach bus power > domain [1.265279] PM: Added domain provider from > /soc@0/bus@3000/gpc@303a/pgc/power-domain@5 [1.265861] PM: > Added domain provider from > /soc@0/bus@3000/gpc@303a/pgc/power-domain@6 blk-ctrl@3832 requires the power-domain 'pgc_vpu', which is power-domain@6 in pgc. Best regards, Alexander > > Signed-off-by: Saravana Kannan > > --- > > > > drivers/base/power/domain.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > > index 739e52cd4aba..3e86772d5fac 100644 > > --- a/drivers/base/power/domain.c > > +++ b/drivers/base/power/domain.c > > @@ -2730,7 +2730,7 @@ static int __genpd_dev_pm_attach(struct device *dev, > > struct device *base_dev,> > > mutex_unlock(_list_lock); > > dev_dbg(dev, "%s() failed to find PM domain: %ld\n", > > > > __func__, PTR_ERR(pd)); > > > > - return driver_deferred_probe_check_state(base_dev); > > + return -ENODEV; > > > > } > > > > dev_dbg(dev, "adding to PM domain %s\n", pd->name); ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
On Thu, Jun 23, 2022 at 12:01 AM Tony Lindgren wrote:
>
> * Saravana Kannan [220622 19:05]:
> > On Tue, Jun 21, 2022 at 9:59 PM Tony Lindgren wrote:
> > >
> > > Hi,
> > >
> > > * Saravana Kannan [220621 19:29]:
> > > > On Tue, Jun 21, 2022 at 12:28 AM Tony Lindgren wrote:
> > > > >
> > > > > 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.
> > > >
> > > > Can you please point me to an example DTS I could use for debugging
> > > > this? I'm assuming you are leaving fw_devlink=on and not turning it
> > > > off or putting it in permissive mode.
> > >
> > > Sure, this seems to happen at least with simple-pm-bus as the top
> > > level interconnect with a configured power-domains property:
> > >
> > > $ git grep -A10 "ocp {" arch/arm/boot/dts/*.dtsi | grep -B3 -A4
> > > simple-pm-bus
> >
> > Thanks for the example. I generally start looking from dts (not dtsi)
> > files in case there are some DT property override/additions after the
> > dtsi files are included in the dts file. But I'll assume for now
> > that's not the case. If there's a specific dts file for a board I can
> > look from that'd be helpful to rule out those kinds of issues.
> >
> > For now, I looked at arch/arm/boot/dts/omap4.dtsi.
>
> OK it should be very similar for all the affected SoCs.
>
> > > This issue is no directly related fw_devlink. It is a side effect of
> > > removing driver_deferred_probe_check_state(). We no longer return
> > > -EPROBE_DEFER at the end of driver_deferred_probe_check_state().
> >
> > Yes, I understand the issue. But driver_deferred_probe_check_state()
> > was deleted because fw_devlink=on should have short circuited the
> > probe attempt with an -EPROBE_DEFER before reaching the bus/driver
> > probe function and hitting this -ENOENT failure. That's why I was
> > asking the other questions.
>
> OK. So where is the -EPROBE_DEFER supposed to happen without
> driver_deferred_probe_check_state() then?
device_links_check_suppliers() call inside really_probe() would short
circuit and return an -EPROBE_DEFER if the device links are created as
expected.
>
> > > > > On platform_probe() genpd_get_from_provider() returns
> > > > > -ENOENT.
> > > >
> > > > This error is with the series I assume?
> > >
> > > On the first probe genpd_get_from_provider() will return -ENOENT in
> > > both cases. The list is empty on the first probe and there are no
> > > genpd providers at this point.
> > >
> > > Earlier with driver_deferred_probe_check_state(), the initial -ENOENT
> > > ends up getting changed to -EPROBE_DEFER at the end of
> > > driver_deferred_probe_check_state(), we are now missing that.
> >
> > Right, I was aware -ENOENT would be returned if we got this far. But
> > the point of this series is that you shouldn't have gotten that far
> > before your pm domain device is ready. Hence my questions from the
> > earlier reply.
>
> OK
>
> > Can I get answers to rest of my questions in the first reply please?
> > That should help us figure out why fw_devlink let us get this far.
> > Summarize them here to make it easy:
> > * Are you running with fw_devlink=on?
>
> Yes with the default with no specific kernel params so looks like
> FW_DEVLINK_FLAGS_ON.
>
> > * Is the"ti,omap4-prm-inst"/"ti,omap-prm-inst" built-in in this case?
>
> Yes
>
> > * If it's not built-in, can you please try deferred_probe_timeout=0
> > and deferred_probe_timeout=30 and see if either one of them help?
>
> It's built in so I did not try these.
>
> > * Can I get the output of "ls -d supplier:*" and "cat
> > supplier:*/status" output from the sysfs dir for the ocp device
> > without this series where it boots properly.
>
> Hmm so I'm not seeing any supplier for the top level ocp device in
> the booting case without your patches. I see the suppliers for the
> ocp child device instances only.
Hmmm... this is strange (that the device link isn't there), but this
is what I suspected.
Now we need to figure out why it's missing. There are only a few
things that could cause this and I don't see any of those. I already
checked to make sure the power domain in this instance had a proper
driver with a probe() function -- if it didn't, then that's one thing
that'd could have caused the missing device link. The device does seem
to have a proper driver, so looks like I can rule that out.
Can you point me to the dts file that corresponds to the specific
board you are testing this one? I probably won't find anything, but I
want to rule out some of the possibilities.
All the
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
* Saravana Kannan [220622 19:05]:
> On Tue, Jun 21, 2022 at 9:59 PM Tony Lindgren wrote:
> >
> > Hi,
> >
> > * Saravana Kannan [220621 19:29]:
> > > On Tue, Jun 21, 2022 at 12:28 AM Tony Lindgren wrote:
> > > >
> > > > 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.
> > >
> > > Can you please point me to an example DTS I could use for debugging
> > > this? I'm assuming you are leaving fw_devlink=on and not turning it
> > > off or putting it in permissive mode.
> >
> > Sure, this seems to happen at least with simple-pm-bus as the top
> > level interconnect with a configured power-domains property:
> >
> > $ git grep -A10 "ocp {" arch/arm/boot/dts/*.dtsi | grep -B3 -A4
> > simple-pm-bus
>
> Thanks for the example. I generally start looking from dts (not dtsi)
> files in case there are some DT property override/additions after the
> dtsi files are included in the dts file. But I'll assume for now
> that's not the case. If there's a specific dts file for a board I can
> look from that'd be helpful to rule out those kinds of issues.
>
> For now, I looked at arch/arm/boot/dts/omap4.dtsi.
OK it should be very similar for all the affected SoCs.
> > This issue is no directly related fw_devlink. It is a side effect of
> > removing driver_deferred_probe_check_state(). We no longer return
> > -EPROBE_DEFER at the end of driver_deferred_probe_check_state().
>
> Yes, I understand the issue. But driver_deferred_probe_check_state()
> was deleted because fw_devlink=on should have short circuited the
> probe attempt with an -EPROBE_DEFER before reaching the bus/driver
> probe function and hitting this -ENOENT failure. That's why I was
> asking the other questions.
OK. So where is the -EPROBE_DEFER supposed to happen without
driver_deferred_probe_check_state() then?
> > > > On platform_probe() genpd_get_from_provider() returns
> > > > -ENOENT.
> > >
> > > This error is with the series I assume?
> >
> > On the first probe genpd_get_from_provider() will return -ENOENT in
> > both cases. The list is empty on the first probe and there are no
> > genpd providers at this point.
> >
> > Earlier with driver_deferred_probe_check_state(), the initial -ENOENT
> > ends up getting changed to -EPROBE_DEFER at the end of
> > driver_deferred_probe_check_state(), we are now missing that.
>
> Right, I was aware -ENOENT would be returned if we got this far. But
> the point of this series is that you shouldn't have gotten that far
> before your pm domain device is ready. Hence my questions from the
> earlier reply.
OK
> Can I get answers to rest of my questions in the first reply please?
> That should help us figure out why fw_devlink let us get this far.
> Summarize them here to make it easy:
> * Are you running with fw_devlink=on?
Yes with the default with no specific kernel params so looks like
FW_DEVLINK_FLAGS_ON.
> * Is the"ti,omap4-prm-inst"/"ti,omap-prm-inst" built-in in this case?
Yes
> * If it's not built-in, can you please try deferred_probe_timeout=0
> and deferred_probe_timeout=30 and see if either one of them help?
It's built in so I did not try these.
> * Can I get the output of "ls -d supplier:*" and "cat
> supplier:*/status" output from the sysfs dir for the ocp device
> without this series where it boots properly.
Hmm so I'm not seeing any supplier for the top level ocp device in
the booting case without your patches. I see the suppliers for the
ocp child device instances only.
Without your patches I see simple-pm-bus probe initially with
EPROBE_DEFER like I described earlier, and then simple-pm-bus probes
on the second try.
Regards,
Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
On Tue, Jun 21, 2022 at 9:59 PM Tony Lindgren wrote:
>
> Hi,
>
> * Saravana Kannan [220621 19:29]:
> > On Tue, Jun 21, 2022 at 12:28 AM Tony Lindgren wrote:
> > >
> > > 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.
> >
> > Can you please point me to an example DTS I could use for debugging
> > this? I'm assuming you are leaving fw_devlink=on and not turning it
> > off or putting it in permissive mode.
>
> Sure, this seems to happen at least with simple-pm-bus as the top
> level interconnect with a configured power-domains property:
>
> $ git grep -A10 "ocp {" arch/arm/boot/dts/*.dtsi | grep -B3 -A4 simple-pm-bus
Thanks for the example. I generally start looking from dts (not dtsi)
files in case there are some DT property override/additions after the
dtsi files are included in the dts file. But I'll assume for now
that's not the case. If there's a specific dts file for a board I can
look from that'd be helpful to rule out those kinds of issues.
For now, I looked at arch/arm/boot/dts/omap4.dtsi.
>
> This issue is no directly related fw_devlink. It is a side effect of
> removing driver_deferred_probe_check_state(). We no longer return
> -EPROBE_DEFER at the end of driver_deferred_probe_check_state().
Yes, I understand the issue. But driver_deferred_probe_check_state()
was deleted because fw_devlink=on should have short circuited the
probe attempt with an -EPROBE_DEFER before reaching the bus/driver
probe function and hitting this -ENOENT failure. That's why I was
asking the other questions.
> > > On platform_probe() genpd_get_from_provider() returns
> > > -ENOENT.
> >
> > This error is with the series I assume?
>
> On the first probe genpd_get_from_provider() will return -ENOENT in
> both cases. The list is empty on the first probe and there are no
> genpd providers at this point.
>
> Earlier with driver_deferred_probe_check_state(), the initial -ENOENT
> ends up getting changed to -EPROBE_DEFER at the end of
> driver_deferred_probe_check_state(), we are now missing that.
Right, I was aware -ENOENT would be returned if we got this far. But
the point of this series is that you shouldn't have gotten that far
before your pm domain device is ready. Hence my questions from the
earlier reply.
Can I get answers to rest of my questions in the first reply please?
That should help us figure out why fw_devlink let us get this far.
Summarize them here to make it easy:
* Are you running with fw_devlink=on?
* Is the"ti,omap4-prm-inst"/"ti,omap-prm-inst" built-in in this case?
* If it's not built-in, can you please try deferred_probe_timeout=0
and deferred_probe_timeout=30 and see if either one of them help?
* Can I get the output of "ls -d supplier:*" and "cat
supplier:*/status" output from the sysfs dir for the ocp device
without this series where it boots properly.
Thanks,
Saravana
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
Hi,
* Saravana Kannan [220621 19:29]:
> On Tue, Jun 21, 2022 at 12:28 AM Tony Lindgren wrote:
> >
> > 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.
>
> Can you please point me to an example DTS I could use for debugging
> this? I'm assuming you are leaving fw_devlink=on and not turning it
> off or putting it in permissive mode.
Sure, this seems to happen at least with simple-pm-bus as the top
level interconnect with a configured power-domains property:
$ git grep -A10 "ocp {" arch/arm/boot/dts/*.dtsi | grep -B3 -A4 simple-pm-bus
This issue is no directly related fw_devlink. It is a side effect of
removing driver_deferred_probe_check_state(). We no longer return
-EPROBE_DEFER at the end of driver_deferred_probe_check_state().
> > On platform_probe() genpd_get_from_provider() returns
> > -ENOENT.
>
> This error is with the series I assume?
On the first probe genpd_get_from_provider() will return -ENOENT in
both cases. The list is empty on the first probe and there are no
genpd providers at this point.
Earlier with driver_deferred_probe_check_state(), the initial -ENOENT
ends up getting changed to -EPROBE_DEFER at the end of
driver_deferred_probe_check_state(), we are now missing that.
Regards,
Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
On Tue, Jun 21, 2022 at 12:28 AM Tony Lindgren wrote: > > 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. Can you please point me to an example DTS I could use for debugging this? I'm assuming you are leaving fw_devlink=on and not turning it off or putting it in permissive mode. > With this > simple-pm-bus fails to probe initially as the power-domain is not > yet available. Before we get to late_initcall(), I'd expect this series to not have any impact because both fw_devlink and driver_deferred_probe_check_state() should be causing the device's probe to get deferred until the PM domain device comes up. To double check this, without this series, can you give me the list of "supplier:*" symlinks under a simple-pm-bus device's sysfs folder that's having problems with this series? And for all those symlinks, cat the "status" file under that directory? In the system where this is failing, is the PM domain driver loaded as a module at a later point? Couple of other things to try (with the patches) to narrow this down: * Can you set driver_probe_timeout=0 in the command line and see if that helps? * Can you set it to something high like 30 or even larger and see if it helps? > On platform_probe() genpd_get_from_provider() returns > -ENOENT. This error is with the series I assume? > Seems like other stuff is potentially broken too, any ideas on > how to fix this? I'll want to understand the issue first. It's not yet clear to me why fw_devlink isn't blocking the probe of the simple-pm-bus device until the PM domain device shows up. And if it is not blocking, then why and at what point in boot it's giving up and letting the probe get to this point where there's an error. -Saravana > > > > > > Signed-off-by: Saravana Kannan > > --- > > drivers/base/power/domain.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > > index 739e52cd4aba..3e86772d5fac 100644 > > --- a/drivers/base/power/domain.c > > +++ b/drivers/base/power/domain.c > > @@ -2730,7 +2730,7 @@ static int __genpd_dev_pm_attach(struct device *dev, > > struct device *base_dev, > > mutex_unlock(_list_lock); > > dev_dbg(dev, "%s() failed to find PM domain: %ld\n", > > __func__, PTR_ERR(pd)); > > - return driver_deferred_probe_check_state(base_dev); > > + return -ENODEV; > > } > > > > dev_dbg(dev, "adding to PM domain %s\n", pd->name); > > -- > > 2.36.1.255.ge46751e96f-goog > > > > -- > To unsubscribe from this group and stop receiving emails from it, send an > email to kernel-team+unsubscr...@android.com. > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
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? Regards, Tony > > Signed-off-by: Saravana Kannan > --- > drivers/base/power/domain.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index 739e52cd4aba..3e86772d5fac 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -2730,7 +2730,7 @@ static int __genpd_dev_pm_attach(struct device *dev, > struct device *base_dev, > mutex_unlock(_list_lock); > dev_dbg(dev, "%s() failed to find PM domain: %ld\n", > __func__, PTR_ERR(pd)); > - return driver_deferred_probe_check_state(base_dev); > + return -ENODEV; > } > > dev_dbg(dev, "adding to PM domain %s\n", pd->name); > -- > 2.36.1.255.ge46751e96f-goog > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
On Thu, Jun 9, 2022 at 4:45 AM Ulf Hansson wrote: > > On Wed, 1 Jun 2022 at 09:07, Saravana Kannan wrote: > > > > 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. > > With fw_devlink=on by default - does that mean that the parameter > can't be changed? > > Or perhaps the point is that we don't want to go back, but rather drop > the fw_devlink parameter altogether when moving forward? Good question. For now, keeping fw_devlink=off and fw_devlink=permissive as debugging options that I can ask people to try if some probe is getting blocked. Or maybe if some ultra low memory use case wants to avoid create device links, fwnode links, etc and can build everything in and have init/probe happen in the right order. But in the long run, I see a strong possibility for fw_devlink=off/permissive being removed. I'd still want to keep it for implementing =rpm where it'd also automatically enable PM runtime tracking, but I don't understand that well enough yet to do it by default. > > > > Signed-off-by: Saravana Kannan > > Just a minor nitpick below. Nevertheless, feel free to add: > > Reviewed-by: Ulf Hansson Thanks! > > > --- > > drivers/base/power/domain.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > > index 739e52cd4aba..3e86772d5fac 100644 > > --- a/drivers/base/power/domain.c > > +++ b/drivers/base/power/domain.c > > @@ -2730,7 +2730,7 @@ static int __genpd_dev_pm_attach(struct device *dev, > > struct device *base_dev, > > mutex_unlock(_list_lock); > > dev_dbg(dev, "%s() failed to find PM domain: %ld\n", > > __func__, PTR_ERR(pd)); > > - return driver_deferred_probe_check_state(base_dev); > > Adding a brief comment about why -EPROBE_DEFER doesn't make sense > here, would be nice. Will do once all the reviews comeout/when I send v3. I'm thinking something like: /* fw_devlink will take care of retrying for missing suppliers */ -Saravana > > > + return -ENODEV; > > } > > > > dev_dbg(dev, "adding to PM domain %s\n", pd->name); > > -- > > 2.36.1.255.ge46751e96f-goog > > > > Kind regards > Uffe ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
On Wed, 1 Jun 2022 at 09:07, Saravana Kannan wrote: > > 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. With fw_devlink=on by default - does that mean that the parameter can't be changed? Or perhaps the point is that we don't want to go back, but rather drop the fw_devlink parameter altogether when moving forward? > > Signed-off-by: Saravana Kannan Just a minor nitpick below. Nevertheless, feel free to add: Reviewed-by: Ulf Hansson > --- > drivers/base/power/domain.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index 739e52cd4aba..3e86772d5fac 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -2730,7 +2730,7 @@ static int __genpd_dev_pm_attach(struct device *dev, > struct device *base_dev, > mutex_unlock(_list_lock); > dev_dbg(dev, "%s() failed to find PM domain: %ld\n", > __func__, PTR_ERR(pd)); > - return driver_deferred_probe_check_state(base_dev); Adding a brief comment about why -EPROBE_DEFER doesn't make sense here, would be nice. > + return -ENODEV; > } > > dev_dbg(dev, "adding to PM domain %s\n", pd->name); > -- > 2.36.1.255.ge46751e96f-goog > Kind regards Uffe ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
