Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()

[Saravana Kannan]

On Thu, Jun 23, 2022 at 5:08 AM Alexander Stein
<alexander.stein@xxxxxxxxxxxxxxx> wrote:
>
> Hi,
>
> Am Dienstag, 21. Juni 2022, 09:28:43 CEST schrieb Tony Lindgren:
> > Hi,
> >
> > * Saravana Kannan <saravanak@xxxxxxxxxx> [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 38320000.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@303a0000 {
        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
38320000.blk-ctrl until 303a0000.gpc is initialized. And it stops
blocking the probe of 38320000.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 <saravanak@google.com>
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 = &imx7_access_table,
 	.pgc_regs = &imx7_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 = &imx8m_access_table,
 	.pgc_regs = &imx7_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 +1450,10 @@ static int imx_gpcv2_probe(struct platform_device *pdev)
 	void __iomem *base;
 	int ret;
 
+	ret = imx_gpcv2_irqchip_init(pdev, domain_data->irq_core_num);
+	if (ret)
+		return ret;
+
 	pgc_np = of_get_child_by_name(dev->of_node, "pgc");
 	if (!pgc_np) {
 		dev_err(dev, "No power domains specified in DT\n");
-- 
2.37.0.rc0.161.g10f37bed90-goog