Re: [PATCH v1 0/9] fw_devlink improvements

From: Saravana Kannan <saravanak@google.com>
To: Alexander Stein <alexander.stein@ew.tq-group.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Bartosz Golaszewski <brgl@bgdev.pl>,
	Rob Herring <robh+dt@kernel.org>,
	Frank Rowand <frowand.list@gmail.com>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Magnus Damm <magnus.damm@gmail.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Daniel Scally <djrscally@gmail.com>,
	Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Len Brown <lenb@kernel.org>, Abel Vesa <abel.vesa@linaro.org>,
	Tony Lindgren <tony@atomide.com>,
	Sudeep Holla <sudeep.holla@arm.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	John Stultz <jstultz@google.com>,
	Doug Anderson <dianders@chromium.org>,
	Guenter Roeck <linux@roeck-us.net>,
	kernel-team@android.com, linux-kernel@vger.kernel.org,
	linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org, linux-acpi@vger.kernel.org
Subject: Re: [PATCH v1 0/9] fw_devlink improvements
Date: Mon, 15 Aug 2022 12:17:36 -0700	[thread overview]
Message-ID: <CAGETcx_tSndU0xerz=DF9JQxYFRC2aaxyOE-bR2JpM0L0ht=sw@mail.gmail.com> (raw)
In-Reply-To: <3601760.iIbC2pHGDl@steina-w>

On Mon, Aug 15, 2022 at 5:39 AM Alexander Stein
<alexander.stein@ew.tq-group.com> wrote:
>
> Hello Saravana,
>
> Am Mittwoch, 10. August 2022, 08:00:29 CEST schrieb Saravana Kannan:
> > Alexander,
> >
> > This should fix your issue where the power domain device not having a
> > compatible property. Can you give it a shot please?
>
> thanks for the update. Unfortunately this does not work:
>
> > [    0.774838] PM: Added domain provider from /soc@0/bus@30000000/
> gpc@303a0000/pgc/power-domain@0
> > [    0.775100] imx-pgc imx-pgc-domain.1: __genpd_dev_pm_attach() failed to
> find PM domain: -2
> > [    0.775324] PM: Added domain provider from /soc@0/bus@30000000/
> gpc@303a0000/pgc/power-domain@2
> > [    0.775601] PM: Added domain provider from /soc@0/bus@30000000/
> gpc@303a0000/pgc/power-domain@3
> > [    0.775842] PM: Added domain provider from /soc@0/bus@30000000/
> gpc@303a0000/pgc/power-domain@4
> > [    0.776642] PM: Added domain provider from /soc@0/bus@30000000/
> gpc@303a0000/pgc/power-domain@7
> > [    0.776897] PM: Added domain provider from /soc@0/bus@30000000/
> gpc@303a0000/pgc/power-domain@8
> > [    0.777158] PM: Added domain provider from /soc@0/bus@30000000/
> gpc@303a0000/pgc/power-domain@9
> > [    0.777405] PM: Added domain provider from /soc@0/bus@30000000/
> gpc@303a0000/pgc/power-domain@a
> > [    0.779342] genpd genpd:0:38320000.blk-ctrl: __genpd_dev_pm_attach()
> failed to find PM domain: -2
> > [    0.779422] imx8m-blk-ctrl 38320000.blk-ctrl: error -ENODEV: failed to
> attach power domain "bus"
> > [    0.848785] etnaviv-gpu 38000000.gpu: __genpd_dev_pm_attach() failed to
> find PM domain: -2
> > [    1.114220] pfuze100-regulator 0-0008: Full layer: 2, Metal layer: 1
> > [    1.122267] pfuze100-regulator 0-0008: FAB: 0, FIN: 0
> > [    1.132970] pfuze100-regulator 0-0008: pfuze100 found.
> > [    1.157011] imx-gpcv2 303a0000.gpc: Failed to create device link with
> 0-0008
> > [    1.164094] imx-gpcv2 303a0000.gpc: Failed to create device link with
> 0-0008
>
> The required power-supply for the power domains is still not yet available.
> Does this series require some other patches as well?

Ah sorry, yeah, this needs additional patches. The one I gave in the
other thread when I debugged this and I also noticed another issue.
Here's the combined diff of what's needed. Can you add this on top of
the series and test it?

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;
 }

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 6383a4edc360..181fbfe5bd4d 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -1513,6 +1513,7 @@ static int imx_gpcv2_probe(struct platform_device *pdev)

                pd_pdev->dev.parent = dev;
                pd_pdev->dev.of_node = np;
+               pd_pdev->dev.fwnode = of_fwnode_handle(np);

                ret = platform_device_add(pd_pdev);
                if (ret) {

With this patch, I'd really expect the power domain dependency to be
handled correctly.

> Whats worse, starting with commit 9/9 [of: property: Simplify
> of_link_to_phandle()], other drivers fail to probe waiting for pinctrl to be
> available.

Heh, Patch 9/9 and all its other dependencies in this series was to
fix your use case. Ironic that it's causing you more issues.

> > $ cat /sys/kernel/debug/devices_deferred
> > gpio-leds       platform: wait for supplier gpioledgrp
> > extcon-usbotg0  platform: wait for supplier usb0congrp
> > gpio-keys       platform: wait for supplier gpiobuttongrp
> > regulator-otg-vbus      platform: wait for supplier reggotgvbusgrp
> > regulator-vdd-arm       platform: wait for supplier dvfsgrp
>
> Apparently for some reason they are not probed again, once the pinctrl driver
> probed.

I'm hoping that this is just some issue due to the missing patch
above, but doesn't sound like it if you say that the pinctrl ended up
probing eventually.

So when device_links_driver_bound() calls
__fw_devlink_pickup_dangling_consumers(), it should have picked up the
consumers of node like gpiobuttongrp and moved it to the pinctrl
device. And right after that we call __fw_devlink_link_to_consumers()
that would have created the device links. And then right after that,
we go through all the consumers and add them to the deferred probe
list. After that deferred probe should have run... either because it's
enabled at late_initcall() or because a new device probed
successfully.

Can you check which one of my expectations isn't true in your case?

Thanks,
Saravana
