[PATCH v1 0/4] Remove use of fw_devlink_purge_absent_suppliers()

[PATCH v1 0/4] Remove use of fw_devlink_purge_absent_suppliers()

[Saravana Kannan]

Yongqin, Martin, Amelie,

We recent refactor of fw_devlink that ends with commit fb42378dcc7f
("mtd: mtdpart: Don't create platform device that'll never probe"),
fw_devlink is smarter and doesn't depend on compatible property. So, I
don't think these calls are needed anymore. But I don't have these
devices to test on and be sure and the hardware I use to test changes
doesn't have this issue either.

Can you please test these changes on the hardware where you hit the
issue to make sure things work as expected?

Yongqin, If you didn't have the context, this affected hikey960.

Greg,

Let's wait for some tests before we land these.

Thanks,
Saravana

Cc: Yongqin Liu <yongqin.liu@linaro.org>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: Martin Kepplinger <martin.kepplinger@puri.sm>
Cc: Amelie Delaunay <amelie.delaunay@foss.st.com>

Saravana Kannan (4):
  usb: typec: stusb160x: Remove use of
    fw_devlink_purge_absent_suppliers()
  usb: typec: tipd: Remove use of fw_devlink_purge_absent_suppliers()
  usb: typec: tcpm: Remove use of fw_devlink_purge_absent_suppliers()
  driver core: Delete fw_devlink_purge_absent_suppliers()

 drivers/base/core.c           | 16 ----------------
 drivers/usb/typec/stusb160x.c |  9 ---------
 drivers/usb/typec/tcpm/tcpm.c |  9 ---------
 drivers/usb/typec/tipd/core.c |  9 ---------
 include/linux/fwnode.h        |  1 -
 5 files changed, 44 deletions(-)

-- 
2.39.2.722.g9855ee24e9-goog

[PATCH v1 1/4] usb: typec: stusb160x: Remove use of fw_devlink_purge_absent_suppliers()

[Saravana Kannan]

This reverts commit 6b63376722d9 (usb: typec: stusb160x: Don't block
probing of consumer of "connector" nodes, 2021-07-16).

After recent changes to fw_devlink that ended with commit 4a032827daa8
("of: property: Simplify of_link_to_phandle()"), fw_devlink no longer
cares about the "compatible" property and figures out the correct struct
device at runtime. So, we no longer need to call
fw_devlink_purge_absent_suppliers().

Signed-off-by: Saravana Kannan <saravanak@google.com>
Cc: Amelie Delaunay <amelie.delaunay@foss.st.com>
---
 drivers/usb/typec/stusb160x.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/usb/typec/stusb160x.c b/drivers/usb/typec/stusb160x.c
index 494b371151e0..482bffeb8a8f 100644
--- a/drivers/usb/typec/stusb160x.c
+++ b/drivers/usb/typec/stusb160x.c
@@ -685,15 +685,6 @@ static int stusb160x_probe(struct i2c_client *client)
 	if (!fwnode)
 		return -ENODEV;
 
-	/*
-	 * This fwnode has a "compatible" property, but is never populated as a
-	 * struct device. Instead we simply parse it to read the properties.
-	 * This it breaks fw_devlink=on. To maintain backward compatibility
-	 * with existing DT files, we work around this by deleting any
-	 * fwnode_links to/from this fwnode.
-	 */
-	fw_devlink_purge_absent_suppliers(fwnode);
-
 	/*
 	 * When both VDD and VSYS power supplies are present, the low power
 	 * supply VSYS is selected when VSYS voltage is above 3.1 V.
-- 
2.39.2.722.g9855ee24e9-goog

[PATCH v1 2/4] usb: typec: tipd: Remove use of fw_devlink_purge_absent_suppliers()

[Saravana Kannan]

After recent changes to fw_devlink that ended with commit 4a032827daa8
("of: property: Simplify of_link_to_phandle()"), fw_devlink no longer
cares about the "compatible" property and figures out the correct struct
device at runtime. So, we no longer need to call
fw_devlink_purge_absent_suppliers().

Signed-off-by: Saravana Kannan <saravanak@google.com>
Cc: Martin Kepplinger <martin.kepplinger@puri.sm>
---
 drivers/usb/typec/tipd/core.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index 485b90c13078..92401622bc4e 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -764,16 +764,7 @@ static int tps6598x_probe(struct i2c_client *client)
 	if (ret < 0)
 		goto err_clear_mask;
 
-	/*
-	 * This fwnode has a "compatible" property, but is never populated as a
-	 * struct device. Instead we simply parse it to read the properties.
-	 * This breaks fw_devlink=on. To maintain backward compatibility
-	 * with existing DT files, we work around this by deleting any
-	 * fwnode_links to/from this fwnode.
-	 */
 	fwnode = device_get_named_child_node(&client->dev, "connector");
-	if (fwnode)
-		fw_devlink_purge_absent_suppliers(fwnode);
 
 	tps->role_sw = fwnode_usb_role_switch_get(fwnode);
 	if (IS_ERR(tps->role_sw)) {
-- 
2.39.2.722.g9855ee24e9-goog

[PATCH v1 3/4] usb: typec: tcpm: Remove use of fw_devlink_purge_absent_suppliers()

[Saravana Kannan]

After recent changes to fw_devlink that ended with commit 4a032827daa8
("of: property: Simplify of_link_to_phandle()"), fw_devlink no longer
cares about the "compatible" property and figures out the correct struct
device at runtime. So, we no longer need to call
fw_devlink_purge_absent_suppliers().

Signed-off-by: Saravana Kannan <saravanak@google.com>
Cc: Yongqin Liu <yongqin.liu@linaro.org>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
---
 drivers/usb/typec/tcpm/tcpm.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index a0d943d78580..fd131f07020c 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -6112,15 +6112,6 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
 	if (!fwnode)
 		return -EINVAL;
 
-	/*
-	 * This fwnode has a "compatible" property, but is never populated as a
-	 * struct device. Instead we simply parse it to read the properties.
-	 * This it breaks fw_devlink=on. To maintain backward compatibility
-	 * with existing DT files, we work around this by deleting any
-	 * fwnode_links to/from this fwnode.
-	 */
-	fw_devlink_purge_absent_suppliers(fwnode);
-
 	ret = typec_get_fw_cap(&port->typec_caps, fwnode);
 	if (ret < 0)
 		return ret;
-- 
2.39.2.722.g9855ee24e9-goog

[PATCH v1 4/4] driver core: Delete fw_devlink_purge_absent_suppliers()

[Saravana Kannan]

After recent changes to fw_devlink that ended with commit 4a032827daa8
("of: property: Simplify of_link_to_phandle()"), fw_devlink no longer
cares about the "compatible" property and figures out the correct struct
device at runtime.

So, there's no need for any driver or framework to call
fw_devlink_purge_absent_suppliers() anymore and we can delete it.

Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 drivers/base/core.c    | 16 ----------------
 include/linux/fwnode.h |  1 -
 2 files changed, 17 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 6878dfcbf0d6..46364c4d1983 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -186,22 +186,6 @@ void fwnode_links_purge(struct fwnode_handle *fwnode)
 	fwnode_links_purge_consumers(fwnode);
 }
 
-void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode)
-{
-	struct fwnode_handle *child;
-
-	/* Don't purge consumer links of an added child */
-	if (fwnode->dev)
-		return;
-
-	fwnode->flags |= FWNODE_FLAG_NOT_DEVICE;
-	fwnode_links_purge_consumers(fwnode);
-
-	fwnode_for_each_available_child_node(fwnode, child)
-		fw_devlink_purge_absent_suppliers(child);
-}
-EXPORT_SYMBOL_GPL(fw_devlink_purge_absent_suppliers);
-
 /**
  * __fwnode_links_move_consumers - Move consumer from @from to @to fwnode_handle
  * @from: move consumers away from this fwnode
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 5700451b300f..63972c863fcd 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -210,6 +210,5 @@ static inline void fwnode_dev_initialized(struct fwnode_handle *fwnode,
 extern bool fw_devlink_is_strict(void);
 int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup);
 void fwnode_links_purge(struct fwnode_handle *fwnode);
-void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
 
 #endif
-- 
2.39.2.722.g9855ee24e9-goog

Re: [PATCH v1 0/4] Remove use of fw_devlink_purge_absent_suppliers()

[Martin Kepplinger]

Am Mittwoch, dem 01.03.2023 um 13:49 -0800 schrieb Saravana Kannan:
> Yongqin, Martin, Amelie,
> 
> We recent refactor of fw_devlink that ends with commit fb42378dcc7f
> ("mtd: mtdpart: Don't create platform device that'll never probe"),
> fw_devlink is smarter and doesn't depend on compatible property. So,
> I
> don't think these calls are needed anymore. But I don't have these
> devices to test on and be sure and the hardware I use to test changes
> doesn't have this issue either.
> 
> Can you please test these changes on the hardware where you hit the
> issue to make sure things work as expected?
> 
> Yongqin, If you didn't have the context, this affected hikey960.
> 
> Greg,
> 
> Let's wait for some tests before we land these.
> 
> Thanks,
> Saravana

hi Sravana,

I picked the 12 commits leading up to commit fb42378dcc7f ("mtd:
mtdpart: Don't create platform device that'll never probe") (
https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink
) and included the tipd patch below to test it.

With that, I get the following errors:

[    0.237931] imx-uart 30890000.serial: Failed to create device link
with regulator-gnss
[    0.334054] nwl-dsi 30a00000.mipi-dsi: Failed to create device link
with regulator-lcd-1v8
[    0.346964] nwl-dsi 30a00000.mipi-dsi: Failed to create device link
with backlight-dsi

but they are independent of this final tipd patch below. I'll test a
real linux-next tree soon, for completeness, maybe I missed something?

Anyways, on that tree, your tipd removal patch breaks type-c still for
me, imx8mq-librem5.dtsi

just to give a first reply quickly... thanks,

                             martin


> 
> Cc: Yongqin Liu <yongqin.liu@linaro.org>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: Martin Kepplinger <martin.kepplinger@puri.sm>
> Cc: Amelie Delaunay <amelie.delaunay@foss.st.com>
> 
> Saravana Kannan (4):
>   usb: typec: stusb160x: Remove use of
>     fw_devlink_purge_absent_suppliers()
>   usb: typec: tipd: Remove use of fw_devlink_purge_absent_suppliers()
>   usb: typec: tcpm: Remove use of fw_devlink_purge_absent_suppliers()
>   driver core: Delete fw_devlink_purge_absent_suppliers()
> 
>  drivers/base/core.c           | 16 ----------------
>  drivers/usb/typec/stusb160x.c |  9 ---------
>  drivers/usb/typec/tcpm/tcpm.c |  9 ---------
>  drivers/usb/typec/tipd/core.c |  9 ---------
>  include/linux/fwnode.h        |  1 -
>  5 files changed, 44 deletions(-)
> 

Re: [PATCH v1 0/4] Remove use of fw_devlink_purge_absent_suppliers()

[Martin Kepplinger]

Am Donnerstag, dem 02.03.2023 um 10:12 +0100 schrieb Martin Kepplinger:
> Am Mittwoch, dem 01.03.2023 um 13:49 -0800 schrieb Saravana Kannan:
> > Yongqin, Martin, Amelie,
> > 
> > We recent refactor of fw_devlink that ends with commit fb42378dcc7f
> > ("mtd: mtdpart: Don't create platform device that'll never probe"),
> > fw_devlink is smarter and doesn't depend on compatible property.
> > So,
> > I
> > don't think these calls are needed anymore. But I don't have these
> > devices to test on and be sure and the hardware I use to test
> > changes
> > doesn't have this issue either.
> > 
> > Can you please test these changes on the hardware where you hit the
> > issue to make sure things work as expected?
> > 
> > Yongqin, If you didn't have the context, this affected hikey960.
> > 
> > Greg,
> > 
> > Let's wait for some tests before we land these.
> > 
> > Thanks,
> > Saravana
> 
> hi Sravana,
> 
> I picked the 12 commits leading up to commit fb42378dcc7f ("mtd:
> mtdpart: Don't create platform device that'll never probe") (
> https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink
> ) and included the tipd patch below to test it.
> 
> With that, I get the following errors:
> 
> [    0.237931] imx-uart 30890000.serial: Failed to create device link
> with regulator-gnss
> [    0.334054] nwl-dsi 30a00000.mipi-dsi: Failed to create device
> link
> with regulator-lcd-1v8
> [    0.346964] nwl-dsi 30a00000.mipi-dsi: Failed to create device
> link
> with backlight-dsi
> 
> but they are independent of this final tipd patch below. I'll test a
> real linux-next tree soon, for completeness, maybe I missed
> something?
> 
> Anyways, on that tree, your tipd removal patch breaks type-c still
> for
> me, imx8mq-librem5.dtsi
> 
> just to give a first reply quickly... thanks,
> 
>                              martin
> 

just confirming: it's the same as above on next-20230302 + this patch (
https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink_next-20230302
) with the errors already independent from the patch. I should have
tested earlier patches -.-

                        martin

Re: [PATCH v1 1/4] usb: typec: stusb160x: Remove use of fw_devlink_purge_absent_suppliers()

[Heikki Krogerus]

On Wed, Mar 01, 2023 at 01:49:48PM -0800, Saravana Kannan wrote:
> This reverts commit 6b63376722d9 (usb: typec: stusb160x: Don't block
> probing of consumer of "connector" nodes, 2021-07-16).
> 
> After recent changes to fw_devlink that ended with commit 4a032827daa8
> ("of: property: Simplify of_link_to_phandle()"), fw_devlink no longer
> cares about the "compatible" property and figures out the correct struct
> device at runtime. So, we no longer need to call
> fw_devlink_purge_absent_suppliers().
> 
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> Cc: Amelie Delaunay <amelie.delaunay@foss.st.com>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/typec/stusb160x.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/usb/typec/stusb160x.c b/drivers/usb/typec/stusb160x.c
> index 494b371151e0..482bffeb8a8f 100644
> --- a/drivers/usb/typec/stusb160x.c
> +++ b/drivers/usb/typec/stusb160x.c
> @@ -685,15 +685,6 @@ static int stusb160x_probe(struct i2c_client *client)
>  	if (!fwnode)
>  		return -ENODEV;
>  
> -	/*
> -	 * This fwnode has a "compatible" property, but is never populated as a
> -	 * struct device. Instead we simply parse it to read the properties.
> -	 * This it breaks fw_devlink=on. To maintain backward compatibility
> -	 * with existing DT files, we work around this by deleting any
> -	 * fwnode_links to/from this fwnode.
> -	 */
> -	fw_devlink_purge_absent_suppliers(fwnode);
> -
>  	/*
>  	 * When both VDD and VSYS power supplies are present, the low power
>  	 * supply VSYS is selected when VSYS voltage is above 3.1 V.
> -- 
> 2.39.2.722.g9855ee24e9-goog

-- 
heikki

Re: [PATCH v1 2/4] usb: typec: tipd: Remove use of fw_devlink_purge_absent_suppliers()

[Heikki Krogerus]

On Wed, Mar 01, 2023 at 01:49:49PM -0800, Saravana Kannan wrote:
> After recent changes to fw_devlink that ended with commit 4a032827daa8
> ("of: property: Simplify of_link_to_phandle()"), fw_devlink no longer
> cares about the "compatible" property and figures out the correct struct
> device at runtime. So, we no longer need to call
> fw_devlink_purge_absent_suppliers().
> 
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> Cc: Martin Kepplinger <martin.kepplinger@puri.sm>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/typec/tipd/core.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 485b90c13078..92401622bc4e 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -764,16 +764,7 @@ static int tps6598x_probe(struct i2c_client *client)
>  	if (ret < 0)
>  		goto err_clear_mask;
>  
> -	/*
> -	 * This fwnode has a "compatible" property, but is never populated as a
> -	 * struct device. Instead we simply parse it to read the properties.
> -	 * This breaks fw_devlink=on. To maintain backward compatibility
> -	 * with existing DT files, we work around this by deleting any
> -	 * fwnode_links to/from this fwnode.
> -	 */
>  	fwnode = device_get_named_child_node(&client->dev, "connector");
> -	if (fwnode)
> -		fw_devlink_purge_absent_suppliers(fwnode);
>  
>  	tps->role_sw = fwnode_usb_role_switch_get(fwnode);
>  	if (IS_ERR(tps->role_sw)) {
> -- 
> 2.39.2.722.g9855ee24e9-goog

-- 
heikki

Re: [PATCH v1 3/4] usb: typec: tcpm: Remove use of fw_devlink_purge_absent_suppliers()

[Heikki Krogerus]

On Wed, Mar 01, 2023 at 01:49:50PM -0800, Saravana Kannan wrote:
> After recent changes to fw_devlink that ended with commit 4a032827daa8
> ("of: property: Simplify of_link_to_phandle()"), fw_devlink no longer
> cares about the "compatible" property and figures out the correct struct
> device at runtime. So, we no longer need to call
> fw_devlink_purge_absent_suppliers().
> 
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> Cc: Yongqin Liu <yongqin.liu@linaro.org>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/typec/tcpm/tcpm.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index a0d943d78580..fd131f07020c 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -6112,15 +6112,6 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
>  	if (!fwnode)
>  		return -EINVAL;
>  
> -	/*
> -	 * This fwnode has a "compatible" property, but is never populated as a
> -	 * struct device. Instead we simply parse it to read the properties.
> -	 * This it breaks fw_devlink=on. To maintain backward compatibility
> -	 * with existing DT files, we work around this by deleting any
> -	 * fwnode_links to/from this fwnode.
> -	 */
> -	fw_devlink_purge_absent_suppliers(fwnode);
> -
>  	ret = typec_get_fw_cap(&port->typec_caps, fwnode);
>  	if (ret < 0)
>  		return ret;
> -- 
> 2.39.2.722.g9855ee24e9-goog

-- 
heikki

Re: [PATCH v1 4/4] driver core: Delete fw_devlink_purge_absent_suppliers()

[Heikki Krogerus]

On Wed, Mar 01, 2023 at 01:49:51PM -0800, Saravana Kannan wrote:
> After recent changes to fw_devlink that ended with commit 4a032827daa8
> ("of: property: Simplify of_link_to_phandle()"), fw_devlink no longer
> cares about the "compatible" property and figures out the correct struct
> device at runtime.
> 
> So, there's no need for any driver or framework to call
> fw_devlink_purge_absent_suppliers() anymore and we can delete it.
> 
> Signed-off-by: Saravana Kannan <saravanak@google.com>

FWIW:

Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/base/core.c    | 16 ----------------
>  include/linux/fwnode.h |  1 -
>  2 files changed, 17 deletions(-)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 6878dfcbf0d6..46364c4d1983 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -186,22 +186,6 @@ void fwnode_links_purge(struct fwnode_handle *fwnode)
>  	fwnode_links_purge_consumers(fwnode);
>  }
>  
> -void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode)
> -{
> -	struct fwnode_handle *child;
> -
> -	/* Don't purge consumer links of an added child */
> -	if (fwnode->dev)
> -		return;
> -
> -	fwnode->flags |= FWNODE_FLAG_NOT_DEVICE;
> -	fwnode_links_purge_consumers(fwnode);
> -
> -	fwnode_for_each_available_child_node(fwnode, child)
> -		fw_devlink_purge_absent_suppliers(child);
> -}
> -EXPORT_SYMBOL_GPL(fw_devlink_purge_absent_suppliers);
> -
>  /**
>   * __fwnode_links_move_consumers - Move consumer from @from to @to fwnode_handle
>   * @from: move consumers away from this fwnode
> diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> index 5700451b300f..63972c863fcd 100644
> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -210,6 +210,5 @@ static inline void fwnode_dev_initialized(struct fwnode_handle *fwnode,
>  extern bool fw_devlink_is_strict(void);
>  int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup);
>  void fwnode_links_purge(struct fwnode_handle *fwnode);
> -void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
>  
>  #endif
> -- 
> 2.39.2.722.g9855ee24e9-goog

-- 
heikki

Re: [PATCH v1 0/4] Remove use of fw_devlink_purge_absent_suppliers()

[Saravana Kannan]

Greg,

Don't pull in this series please. It needs more testing from the folks
I cc'ed and it's already breaking things for Martin. This needs more
revisions.

-Saravana

On Wed, Mar 1, 2023 at 1:49 PM Saravana Kannan <saravanak@google.com> wrote:
>
> Yongqin, Martin, Amelie,
>
> We recent refactor of fw_devlink that ends with commit fb42378dcc7f
> ("mtd: mtdpart: Don't create platform device that'll never probe"),
> fw_devlink is smarter and doesn't depend on compatible property. So, I
> don't think these calls are needed anymore. But I don't have these
> devices to test on and be sure and the hardware I use to test changes
> doesn't have this issue either.
>
> Can you please test these changes on the hardware where you hit the
> issue to make sure things work as expected?
>
> Yongqin, If you didn't have the context, this affected hikey960.
>
> Greg,
>
> Let's wait for some tests before we land these.
>
> Thanks,
> Saravana
>
> Cc: Yongqin Liu <yongqin.liu@linaro.org>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: Martin Kepplinger <martin.kepplinger@puri.sm>
> Cc: Amelie Delaunay <amelie.delaunay@foss.st.com>
>
> Saravana Kannan (4):
>   usb: typec: stusb160x: Remove use of
>     fw_devlink_purge_absent_suppliers()
>   usb: typec: tipd: Remove use of fw_devlink_purge_absent_suppliers()
>   usb: typec: tcpm: Remove use of fw_devlink_purge_absent_suppliers()
>   driver core: Delete fw_devlink_purge_absent_suppliers()
>
>  drivers/base/core.c           | 16 ----------------
>  drivers/usb/typec/stusb160x.c |  9 ---------
>  drivers/usb/typec/tcpm/tcpm.c |  9 ---------
>  drivers/usb/typec/tipd/core.c |  9 ---------
>  include/linux/fwnode.h        |  1 -
>  5 files changed, 44 deletions(-)
>
> --
> 2.39.2.722.g9855ee24e9-goog
>

Re: [PATCH v1 0/4] Remove use of fw_devlink_purge_absent_suppliers()

[Yongqin Liu]

Hi, Saravana

Sorry for the lateness, I was just aware of this today.

I tested with the ACK android-mainline branch + the 12 commits ending
with fb42378dcc7f
+ the 4 commits of this series + hikey960 AOSP Master userspace.
The hikey960 Android build could boot to the home screen, no stuck there,

Here is the link of the logat in case you want to check some message here:
https://gist.github.com/liuyq/6525af08c547cd2e494af5d1c8b181b5

Thanks,
Yongqin Liu
On Fri, 10 Mar 2023 at 02:05, Saravana Kannan <saravanak@google.com> wrote:
>
> Greg,
>
> Don't pull in this series please. It needs more testing from the folks
> I cc'ed and it's already breaking things for Martin. This needs more
> revisions.
>
> -Saravana
>
> On Wed, Mar 1, 2023 at 1:49 PM Saravana Kannan <saravanak@google.com> wrote:
> >
> > Yongqin, Martin, Amelie,
> >
> > We recent refactor of fw_devlink that ends with commit fb42378dcc7f
> > ("mtd: mtdpart: Don't create platform device that'll never probe"),
> > fw_devlink is smarter and doesn't depend on compatible property. So, I
> > don't think these calls are needed anymore. But I don't have these
> > devices to test on and be sure and the hardware I use to test changes
> > doesn't have this issue either.
> >
> > Can you please test these changes on the hardware where you hit the
> > issue to make sure things work as expected?
> >
> > Yongqin, If you didn't have the context, this affected hikey960.
> >
> > Greg,
> >
> > Let's wait for some tests before we land these.
> >
> > Thanks,
> > Saravana
> >
> > Cc: Yongqin Liu <yongqin.liu@linaro.org>
> > Cc: Sumit Semwal <sumit.semwal@linaro.org>
> > Cc: Martin Kepplinger <martin.kepplinger@puri.sm>
> > Cc: Amelie Delaunay <amelie.delaunay@foss.st.com>
> >
> > Saravana Kannan (4):
> >   usb: typec: stusb160x: Remove use of
> >     fw_devlink_purge_absent_suppliers()
> >   usb: typec: tipd: Remove use of fw_devlink_purge_absent_suppliers()
> >   usb: typec: tcpm: Remove use of fw_devlink_purge_absent_suppliers()
> >   driver core: Delete fw_devlink_purge_absent_suppliers()
> >
> >  drivers/base/core.c           | 16 ----------------
> >  drivers/usb/typec/stusb160x.c |  9 ---------
> >  drivers/usb/typec/tcpm/tcpm.c |  9 ---------
> >  drivers/usb/typec/tipd/core.c |  9 ---------
> >  include/linux/fwnode.h        |  1 -
> >  5 files changed, 44 deletions(-)
> >
> > --
> > 2.39.2.722.g9855ee24e9-goog
> >



-- 
Best Regards,
Yongqin Liu
---------------------------------------------------------------
#mailing list
linaro-android@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-android

Re: [PATCH v1 0/4] Remove use of fw_devlink_purge_absent_suppliers()

[Saravana Kannan]

On Thu, Mar 9, 2023 at 10:53 AM Yongqin Liu <yongqin.liu@linaro.org> wrote:
>
> Hi, Saravana
>
> Sorry for the lateness, I was just aware of this today.

No worries.

> I tested with the ACK android-mainline branch + the 12 commits ending
> with fb42378dcc7f
> + the 4 commits of this series + hikey960 AOSP Master userspace.
> The hikey960 Android build could boot to the home screen, no stuck there,

Thanks for testing! Can you confirm what happens if you drop the "12
commits ending with fb42378dcc7f" ? Does it get stuck at boot or have
some limited functionality?

It's surprising that for the same type of DT node, in your case
fw_devlink is able to handle it
correctly, but no so for Martin's case.

-Saravana

>
> Here is the link of the logat in case you want to check some message here:
> https://gist.github.com/liuyq/6525af08c547cd2e494af5d1c8b181b5
>
> Thanks,
> Yongqin Liu
> On Fri, 10 Mar 2023 at 02:05, Saravana Kannan <saravanak@google.com> wrote:
> >
> > Greg,
> >
> > Don't pull in this series please. It needs more testing from the folks
> > I cc'ed and it's already breaking things for Martin. This needs more
> > revisions.
> >
> > -Saravana
> >
> > On Wed, Mar 1, 2023 at 1:49 PM Saravana Kannan <saravanak@google.com> wrote:
> > >
> > > Yongqin, Martin, Amelie,
> > >
> > > We recent refactor of fw_devlink that ends with commit fb42378dcc7f
> > > ("mtd: mtdpart: Don't create platform device that'll never probe"),
> > > fw_devlink is smarter and doesn't depend on compatible property. So, I
> > > don't think these calls are needed anymore. But I don't have these
> > > devices to test on and be sure and the hardware I use to test changes
> > > doesn't have this issue either.
> > >
> > > Can you please test these changes on the hardware where you hit the
> > > issue to make sure things work as expected?
> > >
> > > Yongqin, If you didn't have the context, this affected hikey960.
> > >
> > > Greg,
> > >
> > > Let's wait for some tests before we land these.
> > >
> > > Thanks,
> > > Saravana
> > >
> > > Cc: Yongqin Liu <yongqin.liu@linaro.org>
> > > Cc: Sumit Semwal <sumit.semwal@linaro.org>
> > > Cc: Martin Kepplinger <martin.kepplinger@puri.sm>
> > > Cc: Amelie Delaunay <amelie.delaunay@foss.st.com>
> > >
> > > Saravana Kannan (4):
> > >   usb: typec: stusb160x: Remove use of
> > >     fw_devlink_purge_absent_suppliers()
> > >   usb: typec: tipd: Remove use of fw_devlink_purge_absent_suppliers()
> > >   usb: typec: tcpm: Remove use of fw_devlink_purge_absent_suppliers()
> > >   driver core: Delete fw_devlink_purge_absent_suppliers()
> > >
> > >  drivers/base/core.c           | 16 ----------------
> > >  drivers/usb/typec/stusb160x.c |  9 ---------
> > >  drivers/usb/typec/tcpm/tcpm.c |  9 ---------
> > >  drivers/usb/typec/tipd/core.c |  9 ---------
> > >  include/linux/fwnode.h        |  1 -
> > >  5 files changed, 44 deletions(-)
> > >
> > > --
> > > 2.39.2.722.g9855ee24e9-goog
> > >
>
>
>
> --
> Best Regards,
> Yongqin Liu
> ---------------------------------------------------------------
> #mailing list
> linaro-android@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-android

Re: [PATCH v1 0/4] Remove use of fw_devlink_purge_absent_suppliers()

[Saravana Kannan]

On Thu, Mar 2, 2023 at 1:41 AM Martin Kepplinger
<martin.kepplinger@puri.sm> wrote:
>
> Am Donnerstag, dem 02.03.2023 um 10:12 +0100 schrieb Martin Kepplinger:
> > Am Mittwoch, dem 01.03.2023 um 13:49 -0800 schrieb Saravana Kannan:
> > > Yongqin, Martin, Amelie,
> > >
> > > We recent refactor of fw_devlink that ends with commit fb42378dcc7f
> > > ("mtd: mtdpart: Don't create platform device that'll never probe"),
> > > fw_devlink is smarter and doesn't depend on compatible property.
> > > So,
> > > I
> > > don't think these calls are needed anymore. But I don't have these
> > > devices to test on and be sure and the hardware I use to test
> > > changes
> > > doesn't have this issue either.
> > >
> > > Can you please test these changes on the hardware where you hit the
> > > issue to make sure things work as expected?
> > >
> > > Yongqin, If you didn't have the context, this affected hikey960.
> > >
> > > Greg,
> > >
> > > Let's wait for some tests before we land these.
> > >
> > > Thanks,
> > > Saravana
> >
> > hi Sravana,
> >
> > I picked the 12 commits leading up to commit fb42378dcc7f ("mtd:
> > mtdpart: Don't create platform device that'll never probe") (
> > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink
> > ) and included the tipd patch below to test it.
> >
> > With that, I get the following errors:
> >
> > [    0.237931] imx-uart 30890000.serial: Failed to create device link
> > with regulator-gnss
> > [    0.334054] nwl-dsi 30a00000.mipi-dsi: Failed to create device
> > link
> > with regulator-lcd-1v8
> > [    0.346964] nwl-dsi 30a00000.mipi-dsi: Failed to create device
> > link
> > with backlight-dsi
> >
> > but they are independent of this final tipd patch below. I'll test a
> > real linux-next tree soon, for completeness, maybe I missed
> > something?
> >
> > Anyways, on that tree, your tipd removal patch breaks type-c still
> > for
> > me, imx8mq-librem5.dtsi
> >
> > just to give a first reply quickly... thanks,
> >
> >                              martin
> >
>
> just confirming: it's the same as above on next-20230302 + this patch (
> https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink_next-20230302
> ) with the errors already independent from the patch. I should have
> tested earlier patches -.-

Thanks a lot for testing Martin!

Your email is a little ambiguous to me. With the 12 refactor commits +
the 4 patches in this series, things are breaking for you. But if you
drop the 4 patches in this series, things work again. Is that right?

Let's ignore the "Failed to create device link" errors for now -- it's
not related to this usb-c-connector series. It's basically pointing
out issues that we ignored silently in the past -- it's basically
pointing out holes in fw_devlink's visibility of devices. I'll get to
them later.

-Saravana

Re: [PATCH v1 0/4] Remove use of fw_devlink_purge_absent_suppliers()

[Greg Kroah-Hartman]

On Thu, Mar 09, 2023 at 10:04:49AM -0800, Saravana Kannan wrote:
> Greg,
> 
> Don't pull in this series please. It needs more testing from the folks
> I cc'ed and it's already breaking things for Martin. This needs more
> revisions.

Ah, missed that, sorry.  I've dropped all 4 of these from my tree now,
please resend a new version when you all have it worked out.

thanks,

greg k-h

Re: [PATCH v1 0/4] Remove use of fw_devlink_purge_absent_suppliers()

[Martin Kepplinger]

Am Donnerstag, dem 09.03.2023 um 16:24 -0800 schrieb Saravana Kannan:
> On Thu, Mar 2, 2023 at 1:41 AM Martin Kepplinger
> <martin.kepplinger@puri.sm> wrote:
> > 
> > Am Donnerstag, dem 02.03.2023 um 10:12 +0100 schrieb Martin
> > Kepplinger:
> > > Am Mittwoch, dem 01.03.2023 um 13:49 -0800 schrieb Saravana
> > > Kannan:
> > > > Yongqin, Martin, Amelie,
> > > > 
> > > > We recent refactor of fw_devlink that ends with commit
> > > > fb42378dcc7f
> > > > ("mtd: mtdpart: Don't create platform device that'll never
> > > > probe"),
> > > > fw_devlink is smarter and doesn't depend on compatible
> > > > property.
> > > > So,
> > > > I
> > > > don't think these calls are needed anymore. But I don't have
> > > > these
> > > > devices to test on and be sure and the hardware I use to test
> > > > changes
> > > > doesn't have this issue either.
> > > > 
> > > > Can you please test these changes on the hardware where you hit
> > > > the
> > > > issue to make sure things work as expected?
> > > > 
> > > > Yongqin, If you didn't have the context, this affected
> > > > hikey960.
> > > > 
> > > > Greg,
> > > > 
> > > > Let's wait for some tests before we land these.
> > > > 
> > > > Thanks,
> > > > Saravana
> > > 
> > > hi Sravana,
> > > 
> > > I picked the 12 commits leading up to commit fb42378dcc7f ("mtd:
> > > mtdpart: Don't create platform device that'll never probe") (
> > > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink
> > > ) and included the tipd patch below to test it.
> > > 
> > > With that, I get the following errors:
> > > 
> > > [    0.237931] imx-uart 30890000.serial: Failed to create device
> > > link
> > > with regulator-gnss
> > > [    0.334054] nwl-dsi 30a00000.mipi-dsi: Failed to create device
> > > link
> > > with regulator-lcd-1v8
> > > [    0.346964] nwl-dsi 30a00000.mipi-dsi: Failed to create device
> > > link
> > > with backlight-dsi
> > > 
> > > but they are independent of this final tipd patch below. I'll
> > > test a
> > > real linux-next tree soon, for completeness, maybe I missed
> > > something?
> > > 
> > > Anyways, on that tree, your tipd removal patch breaks type-c
> > > still
> > > for
> > > me, imx8mq-librem5.dtsi
> > > 
> > > just to give a first reply quickly... thanks,
> > > 
> > >                              martin
> > > 
> > 
> > just confirming: it's the same as above on next-20230302 + this
> > patch (
> > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink_next-20230302
> > ) with the errors already independent from the patch. I should have
> > tested earlier patches -.-
> 
> Thanks a lot for testing Martin!
> 
> Your email is a little ambiguous to me. With the 12 refactor commits
> +
> the 4 patches in this series, things are breaking for you. But if you
> drop the 4 patches in this series, things work again. Is that right?

no. Sorry if I wasn't clear. I can't justify to block these 4 patches.
they *themselves* don't break anything.

Something broke *earlier* than these 4 patches in one of the other 12.

As to *what* broke: The error messages during boot, and the charger and
fuel gauge don't work anymore, I see:

# cat /sys/kernel/debug/devices_deferred 
3-0036	max17042: failed: power supply register
0-003f	
38100000.usb	platform: wait for supplier endpoint
32c00000.hdmi	platform: supplier 0-003f not ready
3-006a	bq25890-charger: registering power supply


I haven't had time to track it down and see where the issue is. Could
be one of the 12 patches, could be a wrong description for my board:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi
or something else (an i2c issue?).

(for completeness, the exact tree I ran:
https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink
)

> 
> Let's ignore the "Failed to create device link" errors for now --
> it's
> not related to this usb-c-connector series. It's basically pointing
> out issues that we ignored silently in the past -- it's basically
> pointing out holes in fw_devlink's visibility of devices. I'll get to
> them later.

ok. good to know. please do.

And since I'm the only one seeing an issue, please move the patches
forward. I'll need to fix this after the fact and hope to have time for
it next week.

> 
> -Saravana


                           martin

Re: [PATCH v1 0/4] Remove use of fw_devlink_purge_absent_suppliers()

[Fabrice Gasnier]

On 3/1/23 22:49, Saravana Kannan wrote:
> Yongqin, Martin, Amelie,
> 
> We recent refactor of fw_devlink that ends with commit fb42378dcc7f
> ("mtd: mtdpart: Don't create platform device that'll never probe"),
> fw_devlink is smarter and doesn't depend on compatible property. So, I
> don't think these calls are needed anymore. But I don't have these
> devices to test on and be sure and the hardware I use to test changes
> doesn't have this issue either.
> 
> Can you please test these changes on the hardware where you hit the
> issue to make sure things work as expected?


Hi Saravana,

Sorry for the late reply,
On behalf of Amelie, I did some testing on STM32MP15 DK2 board, on top
of commit fb42378dcc7f, and also with your series applied.
For reference, it's based on: arch/arm/boot/dts/stm32mp15xx-dkx.dtsi

I noticed some error messages on this board, since the 12 patch series,
around the I2C PMIC device links:

[    3.585514] i2c 1-0033: Failed to create device link with 1-0033
[    3.590115] i2c 1-0033: Failed to create device link with 1-0033
[    3.596278] i2c 1-0033: Failed to create device link with 1-0033
[    3.602188] i2c 1-0033: Failed to create device link with 1-0033
[    3.608165] i2c 1-0033: Failed to create device link with 1-0033
[    3.614278] i2c 1-0033: Failed to create device link with 1-0033
[    3.620256] i2c 1-0033: Failed to create device link with 1-0033
[    3.626253] i2c 1-0033: Failed to create device link with 1-0033
[    3.632252] i2c 1-0033: Failed to create device link with 1-0033
[    3.639001] stpmic1 1-0033: PMIC Chip Version: 0x10
[    3.645398] platform 5c002000.i2c:stpmic@33:regulators: Fixed
dependency cycle(s) with /soc/i2c@5c00200
0/stpmic@33/regulators/boost
[    3.655937] platform 5c002000.i2c:stpmic@33:regulators: Fixed
dependency cycle(s) with /soc/i2c@5c00200
0/stpmic@33/regulators/buck2
[    3.667824] platform 5c002000.i2c:stpmic@33:regulators: Fixed
dependency cycle(s) with /soc/i2c@5c00200
0/stpmic@33/regulators/buck4
[    3.719751] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
Failed to create device link with 1-0033
[    3.728099] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
Failed to create device link with 1-0033
[    3.737576] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
Failed to create device link with 1-0033
[    3.747216] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
Failed to create device link with 1-0033
[    3.756750] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
Failed to create device link with 1-0033
[    3.766382] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
Failed to create device link with 1-0033
[    3.775914] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
Failed to create device link with 1-0033
[    3.785545] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
Failed to create device link with 1-0033

Strangely some of the regulators seems to have "Fixed dependency", but
not all.

Regarding the typec stusb160x I noticed the message below. It seems
correct, right ?

[   15.962771] typec port0: Fixed dependency cycle(s) with
/soc/usb-otg@49000000/port/endpoint

But sometimes (lets say 1/5 times) during boot, when I have a cable
already plugged in, it looks like there's some race condition. The dwc2
driver reports some error logs in a loop, indefinitely, up to the
watchdog resets the platform :-(.

[   16.288458] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently
in Host mode
[   16.288490] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently
in Host mode
[   16.310429] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently
in Host mode

It probably just points some already existing race condition here. Maybe
it isn't even linked to this patch. But I have no evidence at this
stage. I hope I can investigate further on this one, hopefully I can
free up some time for that.

Best Regards,
Fabrice

> 
> Yongqin, If you didn't have the context, this affected hikey960.
> 
> Greg,
> 
> Let's wait for some tests before we land these.
> 
> Thanks,
> Saravana
> 
> Cc: Yongqin Liu <yongqin.liu@linaro.org>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: Martin Kepplinger <martin.kepplinger@puri.sm>
> Cc: Amelie Delaunay <amelie.delaunay@foss.st.com>
> 
> Saravana Kannan (4):
>   usb: typec: stusb160x: Remove use of
>     fw_devlink_purge_absent_suppliers()
>   usb: typec: tipd: Remove use of fw_devlink_purge_absent_suppliers()
>   usb: typec: tcpm: Remove use of fw_devlink_purge_absent_suppliers()
>   driver core: Delete fw_devlink_purge_absent_suppliers()
> 
>  drivers/base/core.c           | 16 ----------------
>  drivers/usb/typec/stusb160x.c |  9 ---------
>  drivers/usb/typec/tcpm/tcpm.c |  9 ---------
>  drivers/usb/typec/tipd/core.c |  9 ---------
>  include/linux/fwnode.h        |  1 -
>  5 files changed, 44 deletions(-)
> 

Re: [PATCH v1 0/4] Remove use of fw_devlink_purge_absent_suppliers()

[Saravana Kannan]

On Fri, Mar 10, 2023 at 9:21 AM Fabrice Gasnier
<fabrice.gasnier@foss.st.com> wrote:
>
> On 3/1/23 22:49, Saravana Kannan wrote:
> > Yongqin, Martin, Amelie,
> >
> > We recent refactor of fw_devlink that ends with commit fb42378dcc7f
> > ("mtd: mtdpart: Don't create platform device that'll never probe"),
> > fw_devlink is smarter and doesn't depend on compatible property. So, I
> > don't think these calls are needed anymore. But I don't have these
> > devices to test on and be sure and the hardware I use to test changes
> > doesn't have this issue either.
> >
> > Can you please test these changes on the hardware where you hit the
> > issue to make sure things work as expected?
>
>
> Hi Saravana,
>
> Sorry for the late reply,

Thanks for testing!

> On behalf of Amelie, I did some testing on STM32MP15 DK2 board, on top
> of commit fb42378dcc7f, and also with your series applied.
> For reference, it's based on: arch/arm/boot/dts/stm32mp15xx-dkx.dtsi
>
> I noticed some error messages on this board, since the 12 patch series,
> around the I2C PMIC device links:
>
> [    3.585514] i2c 1-0033: Failed to create device link with 1-0033
> [    3.590115] i2c 1-0033: Failed to create device link with 1-0033
> [    3.596278] i2c 1-0033: Failed to create device link with 1-0033
> [    3.602188] i2c 1-0033: Failed to create device link with 1-0033
> [    3.608165] i2c 1-0033: Failed to create device link with 1-0033
> [    3.614278] i2c 1-0033: Failed to create device link with 1-0033
> [    3.620256] i2c 1-0033: Failed to create device link with 1-0033
> [    3.626253] i2c 1-0033: Failed to create device link with 1-0033
> [    3.632252] i2c 1-0033: Failed to create device link with 1-0033
> [    3.639001] stpmic1 1-0033: PMIC Chip Version: 0x10
> [    3.645398] platform 5c002000.i2c:stpmic@33:regulators: Fixed
> dependency cycle(s) with /soc/i2c@5c00200
> 0/stpmic@33/regulators/boost
> [    3.655937] platform 5c002000.i2c:stpmic@33:regulators: Fixed
> dependency cycle(s) with /soc/i2c@5c00200
> 0/stpmic@33/regulators/buck2
> [    3.667824] platform 5c002000.i2c:stpmic@33:regulators: Fixed
> dependency cycle(s) with /soc/i2c@5c00200
> 0/stpmic@33/regulators/buck4
> [    3.719751] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
> Failed to create device link with 1-0033
> [    3.728099] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
> Failed to create device link with 1-0033
> [    3.737576] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
> Failed to create device link with 1-0033
> [    3.747216] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
> Failed to create device link with 1-0033
> [    3.756750] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
> Failed to create device link with 1-0033
> [    3.766382] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
> Failed to create device link with 1-0033
> [    3.775914] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
> Failed to create device link with 1-0033
> [    3.785545] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
> Failed to create device link with 1-0033

You can ignore all the "Failed to create device link" errors. They are
just error logs for stuff that was being ignored silently before. So
that's no functional regression AFAIK. I'll fix them separately if
necessary. And I'm sure you'll see these messages even without my
fw_devlink refactor series.

> Strangely some of the regulators seems to have "Fixed dependency", but
> not all.

Yeah, that's fine too -- that's just fw_devlink being verbose about
not enforcing probe ordering between devices in that cycle because it
can't tell which one of the dependencies is not a probe requirement.
Maybe I'll make it a dbg log if it's confusing people.

> Regarding the typec stusb160x I noticed the message below. It seems
> correct, right ?
>
> [   15.962771] typec port0: Fixed dependency cycle(s) with
> /soc/usb-otg@49000000/port/endpoint

I don't know if there is a cyclic dependency in your DT or not. But
this message itself is not an issue.

> But sometimes (lets say 1/5 times) during boot, when I have a cable
> already plugged in, it looks like there's some race condition. The dwc2
> driver reports some error logs in a loop, indefinitely, up to the
> watchdog resets the platform :-(.

Can you try this series (the one you are testing) without my
fw_devlink refactor that ends with commit fb42378dcc7f? Trying to make
sure we can reproduce the issue Amelie was fixing before I claim my
refactor series fixes it.

> [   16.288458] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently
> in Host mode
> [   16.288490] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently
> in Host mode
> [   16.310429] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently
> in Host mode
>
> It probably just points some already existing race condition here. Maybe
> it isn't even linked to this patch. But I have no evidence at this
> stage. I hope I can investigate further on this one, hopefully I can
> free up some time for that.

If you never pick up this series, are you not having any of these 1/5
times boot issues? I wouldn't expect my changes to add any races, but
I'll wait to see what you find here.

Thanks,
Saravana

>
> Best Regards,
> Fabrice
>
> >
> > Yongqin, If you didn't have the context, this affected hikey960.
> >
> > Greg,
> >
> > Let's wait for some tests before we land these.
> >
> > Thanks,
> > Saravana
> >
> > Cc: Yongqin Liu <yongqin.liu@linaro.org>
> > Cc: Sumit Semwal <sumit.semwal@linaro.org>
> > Cc: Martin Kepplinger <martin.kepplinger@puri.sm>
> > Cc: Amelie Delaunay <amelie.delaunay@foss.st.com>
> >
> > Saravana Kannan (4):
> >   usb: typec: stusb160x: Remove use of
> >     fw_devlink_purge_absent_suppliers()
> >   usb: typec: tipd: Remove use of fw_devlink_purge_absent_suppliers()
> >   usb: typec: tcpm: Remove use of fw_devlink_purge_absent_suppliers()
> >   driver core: Delete fw_devlink_purge_absent_suppliers()
> >
> >  drivers/base/core.c           | 16 ----------------
> >  drivers/usb/typec/stusb160x.c |  9 ---------
> >  drivers/usb/typec/tcpm/tcpm.c |  9 ---------
> >  drivers/usb/typec/tipd/core.c |  9 ---------
> >  include/linux/fwnode.h        |  1 -
> >  5 files changed, 44 deletions(-)
> >
>
> --
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
>

Re: [PATCH v1 0/4] Remove use of fw_devlink_purge_absent_suppliers()

[Saravana Kannan]

On Fri, Mar 10, 2023 at 2:07 AM Martin Kepplinger
<martin.kepplinger@puri.sm> wrote:
>
> Am Donnerstag, dem 09.03.2023 um 16:24 -0800 schrieb Saravana Kannan:
> > On Thu, Mar 2, 2023 at 1:41 AM Martin Kepplinger
> > <martin.kepplinger@puri.sm> wrote:
> > >
> > > Am Donnerstag, dem 02.03.2023 um 10:12 +0100 schrieb Martin
> > > Kepplinger:
> > > > Am Mittwoch, dem 01.03.2023 um 13:49 -0800 schrieb Saravana
> > > > Kannan:
> > > > > Yongqin, Martin, Amelie,
> > > > >
> > > > > We recent refactor of fw_devlink that ends with commit
> > > > > fb42378dcc7f
> > > > > ("mtd: mtdpart: Don't create platform device that'll never
> > > > > probe"),
> > > > > fw_devlink is smarter and doesn't depend on compatible
> > > > > property.
> > > > > So,
> > > > > I
> > > > > don't think these calls are needed anymore. But I don't have
> > > > > these
> > > > > devices to test on and be sure and the hardware I use to test
> > > > > changes
> > > > > doesn't have this issue either.
> > > > >
> > > > > Can you please test these changes on the hardware where you hit
> > > > > the
> > > > > issue to make sure things work as expected?
> > > > >
> > > > > Yongqin, If you didn't have the context, this affected
> > > > > hikey960.
> > > > >
> > > > > Greg,
> > > > >
> > > > > Let's wait for some tests before we land these.
> > > > >
> > > > > Thanks,
> > > > > Saravana
> > > >
> > > > hi Sravana,
> > > >
> > > > I picked the 12 commits leading up to commit fb42378dcc7f ("mtd:
> > > > mtdpart: Don't create platform device that'll never probe") (
> > > > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink
> > > > ) and included the tipd patch below to test it.
> > > >
> > > > With that, I get the following errors:
> > > >
> > > > [    0.237931] imx-uart 30890000.serial: Failed to create device
> > > > link
> > > > with regulator-gnss
> > > > [    0.334054] nwl-dsi 30a00000.mipi-dsi: Failed to create device
> > > > link
> > > > with regulator-lcd-1v8
> > > > [    0.346964] nwl-dsi 30a00000.mipi-dsi: Failed to create device
> > > > link
> > > > with backlight-dsi
> > > >
> > > > but they are independent of this final tipd patch below. I'll
> > > > test a
> > > > real linux-next tree soon, for completeness, maybe I missed
> > > > something?
> > > >
> > > > Anyways, on that tree, your tipd removal patch breaks type-c
> > > > still
> > > > for
> > > > me, imx8mq-librem5.dtsi
> > > >
> > > > just to give a first reply quickly... thanks,
> > > >
> > > >                              martin
> > > >
> > >
> > > just confirming: it's the same as above on next-20230302 + this
> > > patch (
> > > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink_next-20230302
> > > ) with the errors already independent from the patch. I should have
> > > tested earlier patches -.-
> >
> > Thanks a lot for testing Martin!
> >
> > Your email is a little ambiguous to me. With the 12 refactor commits
> > +
> > the 4 patches in this series, things are breaking for you. But if you
> > drop the 4 patches in this series, things work again. Is that right?
>
> no. Sorry if I wasn't clear. I can't justify to block these 4 patches.
> they *themselves* don't break anything.
>
> Something broke *earlier* than these 4 patches in one of the other 12.

If you find out it's one of the other 12 patches in the refactor that
broke things for you, can you please reply to the right email in that
series[1] and let me know which patch broke things for you and provide
the debug details there? I don't want to mix issues with unrelated
threads -- I want them to be easy to find in the future.

[1] -  https://lore.kernel.org/lkml/20230207014207.1678715-1-saravanak@google.com/

For all my questions below, you don't need to reply here. Just reply
to the right thread.

> As to *what* broke: The error messages during boot,

The error messages, is it anything other than the "Failed to create
device link"?

> and the charger and
> fuel gauge don't work anymore, I see:
>
> # cat /sys/kernel/debug/devices_deferred
> 3-0036  max17042: failed: power supply register
> 0-003f

This seems to be the main culprit. That 0-003f isn't probing. It's
surprising that the reason is empty too. IIRC that means it didn't do
deferred probing, but rather failed with an error. Wild guess .... Try
increasing the deferred_probe_timeout to see if it helps?

> 38100000.usb    platform: wait for supplier endpoint
> 32c00000.hdmi   platform: supplier 0-003f not ready
> 3-006a  bq25890-charger: registering power supply
>
>
> I haven't had time to track it down and see where the issue is. Could
> be one of the 12 patches, could be a wrong description for my board:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi
> or something else (an i2c issue?).
>
> (for completeness, the exact tree I ran:
> https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink
> )
>
> >
> > Let's ignore the "Failed to create device link" errors for now --
> > it's
> > not related to this usb-c-connector series. It's basically pointing
> > out issues that we ignored silently in the past -- it's basically
> > pointing out holes in fw_devlink's visibility of devices. I'll get to
> > them later.
>
> ok. good to know. please do.

Here's a fix for the Failed to create device link issues.
https://lore.kernel.org/lkml/20230310063910.2474472-1-saravanak@google.com/

> And since I'm the only one seeing an issue, please move the patches
> forward. I'll need to fix this after the fact and hope to have time for
> it next week.

I'd rather wait for you than rush these patches in. The patches are
just removing what I expect to be cruft. I'd rather confirm that first
than break anything.

-Saravana

Re: [PATCH v1 0/4] Remove use of fw_devlink_purge_absent_suppliers()

[Martin Kepplinger]

Am Freitag, dem 10.03.2023 um 14:18 -0800 schrieb Saravana Kannan:
> On Fri, Mar 10, 2023 at 2:07 AM Martin Kepplinger
> <martin.kepplinger@puri.sm> wrote:
> > 
> > Am Donnerstag, dem 09.03.2023 um 16:24 -0800 schrieb Saravana
> > Kannan:
> > > On Thu, Mar 2, 2023 at 1:41 AM Martin Kepplinger
> > > <martin.kepplinger@puri.sm> wrote:
> > > > 
> > > > Am Donnerstag, dem 02.03.2023 um 10:12 +0100 schrieb Martin
> > > > Kepplinger:
> > > > > Am Mittwoch, dem 01.03.2023 um 13:49 -0800 schrieb Saravana
> > > > > Kannan:
> > > > > > Yongqin, Martin, Amelie,
> > > > > > 
> > > > > > We recent refactor of fw_devlink that ends with commit
> > > > > > fb42378dcc7f
> > > > > > ("mtd: mtdpart: Don't create platform device that'll never
> > > > > > probe"),
> > > > > > fw_devlink is smarter and doesn't depend on compatible
> > > > > > property.
> > > > > > So,
> > > > > > I
> > > > > > don't think these calls are needed anymore. But I don't
> > > > > > have
> > > > > > these
> > > > > > devices to test on and be sure and the hardware I use to
> > > > > > test
> > > > > > changes
> > > > > > doesn't have this issue either.
> > > > > > 
> > > > > > Can you please test these changes on the hardware where you
> > > > > > hit
> > > > > > the
> > > > > > issue to make sure things work as expected?
> > > > > > 
> > > > > > Yongqin, If you didn't have the context, this affected
> > > > > > hikey960.
> > > > > > 
> > > > > > Greg,
> > > > > > 
> > > > > > Let's wait for some tests before we land these.
> > > > > > 
> > > > > > Thanks,
> > > > > > Saravana
> > > > > 
> > > > > hi Sravana,
> > > > > 
> > > > > I picked the 12 commits leading up to commit fb42378dcc7f
> > > > > ("mtd:
> > > > > mtdpart: Don't create platform device that'll never probe") (
> > > > > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink
> > > > > ) and included the tipd patch below to test it.
> > > > > 
> > > > > With that, I get the following errors:
> > > > > 
> > > > > [    0.237931] imx-uart 30890000.serial: Failed to create
> > > > > device
> > > > > link
> > > > > with regulator-gnss
> > > > > [    0.334054] nwl-dsi 30a00000.mipi-dsi: Failed to create
> > > > > device
> > > > > link
> > > > > with regulator-lcd-1v8
> > > > > [    0.346964] nwl-dsi 30a00000.mipi-dsi: Failed to create
> > > > > device
> > > > > link
> > > > > with backlight-dsi
> > > > > 
> > > > > but they are independent of this final tipd patch below. I'll
> > > > > test a
> > > > > real linux-next tree soon, for completeness, maybe I missed
> > > > > something?
> > > > > 
> > > > > Anyways, on that tree, your tipd removal patch breaks type-c
> > > > > still
> > > > > for
> > > > > me, imx8mq-librem5.dtsi
> > > > > 
> > > > > just to give a first reply quickly... thanks,
> > > > > 
> > > > >                              martin
> > > > > 
> > > > 
> > > > just confirming: it's the same as above on next-20230302 + this
> > > > patch (
> > > > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink_next-20230302
> > > > ) with the errors already independent from the patch. I should
> > > > have
> > > > tested earlier patches -.-
> > > 
> > > Thanks a lot for testing Martin!
> > > 
> > > Your email is a little ambiguous to me. With the 12 refactor
> > > commits
> > > +
> > > the 4 patches in this series, things are breaking for you. But if
> > > you
> > > drop the 4 patches in this series, things work again. Is that
> > > right?
> > 
> > no. Sorry if I wasn't clear. I can't justify to block these 4
> > patches.
> > they *themselves* don't break anything.
> > 
> > Something broke *earlier* than these 4 patches in one of the other
> > 12.
> 
> If you find out it's one of the other 12 patches in the refactor that
> broke things for you, can you please reply to the right email in that
> series[1] and let me know which patch broke things for you and
> provide
> the debug details there? I don't want to mix issues with unrelated
> threads -- I want them to be easy to find in the future.
> 
> [1] -  
> https://lore.kernel.org/lkml/20230207014207.1678715-1-saravanak@google.com/
> 
> For all my questions below, you don't need to reply here. Just reply
> to the right thread.

Thanks. I'll have to reply here though - I'm puzzled how, but I got it
wrong - I must have seen the "Failed to create device link" messages
without checking broken drivers: The 12 patches you linked above are
fine. (In a way that's good as I saw them in a stable kernel already).

commit ("usb: typec: tipd: Remove use of
fw_devlink_purge_absent_suppliers()") breaks things for me. That is
patch 2 of this series. That's for sure now.

> 
> > As to *what* broke: The error messages during boot,
> 
> The error messages, is it anything other than the "Failed to create
> device link"?
> 
> > and the charger and
> > fuel gauge don't work anymore, I see:
> > 
> > # cat /sys/kernel/debug/devices_deferred
> > 3-0036  max17042: failed: power supply register
> > 0-003f
> 
> This seems to be the main culprit. That 0-003f isn't probing. It's
> surprising that the reason is empty too. IIRC that means it didn't do
> deferred probing, but rather failed with an error. Wild guess ....
> Try
> increasing the deferred_probe_timeout to see if it helps?
> 
> > 38100000.usb    platform: wait for supplier endpoint
> > 32c00000.hdmi   platform: supplier 0-003f not ready
> > 3-006a  bq25890-charger: registering power supply
> > 
> > 
> > I haven't had time to track it down and see where the issue is.
> > Could
> > be one of the 12 patches, could be a wrong description for my
> > board:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi
> > or something else (an i2c issue?).
> > 
> > (for completeness, the exact tree I ran:
> > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink
> > )
> > 
> > > 
> > > Let's ignore the "Failed to create device link" errors for now --
> > > it's
> > > not related to this usb-c-connector series. It's basically
> > > pointing
> > > out issues that we ignored silently in the past -- it's basically
> > > pointing out holes in fw_devlink's visibility of devices. I'll
> > > get to
> > > them later.
> > 
> > ok. good to know. please do.
> 
> Here's a fix for the Failed to create device link issues.
> https://lore.kernel.org/lkml/20230310063910.2474472-1-saravanak@google.com/
> 
> > And since I'm the only one seeing an issue, please move the patches
> > forward. I'll need to fix this after the fact and hope to have time
> > for
> > it next week.
> 
> I'd rather wait for you than rush these patches in. The patches are
> just removing what I expect to be cruft. I'd rather confirm that
> first
> than break anything.
> 
> -Saravana

Re: [PATCH v1 0/4] Remove use of fw_devlink_purge_absent_suppliers()

[Martin Kepplinger]

Am Sonntag, dem 12.03.2023 um 15:41 +0100 schrieb Martin Kepplinger:
> Am Freitag, dem 10.03.2023 um 14:18 -0800 schrieb Saravana Kannan:
> > On Fri, Mar 10, 2023 at 2:07 AM Martin Kepplinger
> > <martin.kepplinger@puri.sm> wrote:
> > > 
> > > Am Donnerstag, dem 09.03.2023 um 16:24 -0800 schrieb Saravana
> > > Kannan:
> > > > On Thu, Mar 2, 2023 at 1:41 AM Martin Kepplinger
> > > > <martin.kepplinger@puri.sm> wrote:
> > > > > 
> > > > > Am Donnerstag, dem 02.03.2023 um 10:12 +0100 schrieb Martin
> > > > > Kepplinger:
> > > > > > Am Mittwoch, dem 01.03.2023 um 13:49 -0800 schrieb Saravana
> > > > > > Kannan:
> > > > > > > Yongqin, Martin, Amelie,
> > > > > > > 
> > > > > > > We recent refactor of fw_devlink that ends with commit
> > > > > > > fb42378dcc7f
> > > > > > > ("mtd: mtdpart: Don't create platform device that'll
> > > > > > > never
> > > > > > > probe"),
> > > > > > > fw_devlink is smarter and doesn't depend on compatible
> > > > > > > property.
> > > > > > > So,
> > > > > > > I
> > > > > > > don't think these calls are needed anymore. But I don't
> > > > > > > have
> > > > > > > these
> > > > > > > devices to test on and be sure and the hardware I use to
> > > > > > > test
> > > > > > > changes
> > > > > > > doesn't have this issue either.
> > > > > > > 
> > > > > > > Can you please test these changes on the hardware where
> > > > > > > you
> > > > > > > hit
> > > > > > > the
> > > > > > > issue to make sure things work as expected?
> > > > > > > 
> > > > > > > Yongqin, If you didn't have the context, this affected
> > > > > > > hikey960.
> > > > > > > 
> > > > > > > Greg,
> > > > > > > 
> > > > > > > Let's wait for some tests before we land these.
> > > > > > > 
> > > > > > > Thanks,
> > > > > > > Saravana
> > > > > > 
> > > > > > hi Sravana,
> > > > > > 
> > > > > > I picked the 12 commits leading up to commit fb42378dcc7f
> > > > > > ("mtd:
> > > > > > mtdpart: Don't create platform device that'll never probe")
> > > > > > (
> > > > > > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink
> > > > > > ) and included the tipd patch below to test it.
> > > > > > 
> > > > > > With that, I get the following errors:
> > > > > > 
> > > > > > [    0.237931] imx-uart 30890000.serial: Failed to create
> > > > > > device
> > > > > > link
> > > > > > with regulator-gnss
> > > > > > [    0.334054] nwl-dsi 30a00000.mipi-dsi: Failed to create
> > > > > > device
> > > > > > link
> > > > > > with regulator-lcd-1v8
> > > > > > [    0.346964] nwl-dsi 30a00000.mipi-dsi: Failed to create
> > > > > > device
> > > > > > link
> > > > > > with backlight-dsi
> > > > > > 
> > > > > > but they are independent of this final tipd patch below.
> > > > > > I'll
> > > > > > test a
> > > > > > real linux-next tree soon, for completeness, maybe I missed
> > > > > > something?
> > > > > > 
> > > > > > Anyways, on that tree, your tipd removal patch breaks type-
> > > > > > c
> > > > > > still
> > > > > > for
> > > > > > me, imx8mq-librem5.dtsi
> > > > > > 
> > > > > > just to give a first reply quickly... thanks,
> > > > > > 
> > > > > >                              martin
> > > > > > 
> > > > > 
> > > > > just confirming: it's the same as above on next-20230302 +
> > > > > this
> > > > > patch (
> > > > > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink_next-20230302
> > > > > ) with the errors already independent from the patch. I
> > > > > should
> > > > > have
> > > > > tested earlier patches -.-
> > > > 
> > > > Thanks a lot for testing Martin!
> > > > 
> > > > Your email is a little ambiguous to me. With the 12 refactor
> > > > commits
> > > > +
> > > > the 4 patches in this series, things are breaking for you. But
> > > > if
> > > > you
> > > > drop the 4 patches in this series, things work again. Is that
> > > > right?
> > > 
> > > no. Sorry if I wasn't clear. I can't justify to block these 4
> > > patches.
> > > they *themselves* don't break anything.
> > > 
> > > Something broke *earlier* than these 4 patches in one of the
> > > other
> > > 12.
> > 
> > If you find out it's one of the other 12 patches in the refactor
> > that
> > broke things for you, can you please reply to the right email in
> > that
> > series[1] and let me know which patch broke things for you and
> > provide
> > the debug details there? I don't want to mix issues with unrelated
> > threads -- I want them to be easy to find in the future.
> > 
> > [1] -  
> > https://lore.kernel.org/lkml/20230207014207.1678715-1-saravanak@google.com/
> > 
> > For all my questions below, you don't need to reply here. Just
> > reply
> > to the right thread.
> 
> Thanks. I'll have to reply here though - I'm puzzled how, but I got
> it
> wrong - I must have seen the "Failed to create device link" messages
> without checking broken drivers: The 12 patches you linked above are
> fine. (In a way that's good as I saw them in a stable kernel
> already).
> 
> commit ("usb: typec: tipd: Remove use of
> fw_devlink_purge_absent_suppliers()") breaks things for me. That is
> patch 2 of this series. That's for sure now.


this thing is this: I have downstream patches against tipd, among
others. And I don't know yet but we might very well do something wrong
in our downstream tree that is now not compatible anymore with this
removal...

I switched to a tree without any downstream stuff: For the 12 earlier
patches, I there see the follwing which is NOT your fault and we need
to fix upstream (we fix that in our downstream tree):

root@pureos:/home/purism# cat /sys/kernel/debug/devices_deferred 
3-0036

And then I add patch 2 of this series (removing the call from tipd), it
becomes:

root@pureos:/home/purism# cat /sys/kernel/debug/devices_deferred 
0-003f	
38100000.usb	platform: wait for supplier endpoint
3-0036	

Re: [PATCH v1 0/4] Remove use of fw_devlink_purge_absent_suppliers()

[Yongqin Liu]

Hi, Saravana

On Fri, 10 Mar 2023 at 08:17, Saravana Kannan <saravanak@google.com> wrote:
>
> On Thu, Mar 9, 2023 at 10:53 AM Yongqin Liu <yongqin.liu@linaro.org> wrote:
> >
> > Hi, Saravana
> >
> > Sorry for the lateness, I was just aware of this today.
>
> No worries.
>
> > I tested with the ACK android-mainline branch + the 12 commits ending
> > with fb42378dcc7f
> > + the 4 commits of this series + hikey960 AOSP Master userspace.
> > The hikey960 Android build could boot to the home screen, no stuck there,
>
> Thanks for testing! Can you confirm what happens if you drop the "12
> commits ending with fb42378dcc7f" ? Does it get stuck at boot or have
> some limited functionality?

I tried to only apply the 4 commits of this series, but that would cause build
error as the commit here:
https://lore.kernel.org/all/20210205222644.2357303-2-saravanak@google.com/

so I need to apply the first commit of the "12 commits ending with fb42378dcc7f"
here as well:
https://lore.kernel.org/all/20230207014207.1678715-2-saravanak@google.com/

With the 5 commits applied on the android-mainline branch, the build could boot
to the home screen, but the adb connection could not be created.
For details please check here https://termbin.com/wf9hj.

Thanks,
Yongqin Liu
>
> It's surprising that for the same type of DT node, in your case
> fw_devlink is able to handle it
> correctly, but no so for Martin's case.
>
> -Saravana
>
> >
> > Here is the link of the logat in case you want to check some message here:
> > https://gist.github.com/liuyq/6525af08c547cd2e494af5d1c8b181b5
> >
> > Thanks,
> > Yongqin Liu
> > On Fri, 10 Mar 2023 at 02:05, Saravana Kannan <saravanak@google.com> wrote:
> > >
> > > Greg,
> > >
> > > Don't pull in this series please. It needs more testing from the folks
> > > I cc'ed and it's already breaking things for Martin. This needs more
> > > revisions.
> > >
> > > -Saravana
> > >
> > > On Wed, Mar 1, 2023 at 1:49 PM Saravana Kannan <saravanak@google.com> wrote:
> > > >
> > > > Yongqin, Martin, Amelie,
> > > >
> > > > We recent refactor of fw_devlink that ends with commit fb42378dcc7f
> > > > ("mtd: mtdpart: Don't create platform device that'll never probe"),
> > > > fw_devlink is smarter and doesn't depend on compatible property. So, I
> > > > don't think these calls are needed anymore. But I don't have these
> > > > devices to test on and be sure and the hardware I use to test changes
> > > > doesn't have this issue either.
> > > >
> > > > Can you please test these changes on the hardware where you hit the
> > > > issue to make sure things work as expected?
> > > >
> > > > Yongqin, If you didn't have the context, this affected hikey960.
> > > >
> > > > Greg,
> > > >
> > > > Let's wait for some tests before we land these.
> > > >
> > > > Thanks,
> > > > Saravana
> > > >
> > > > Cc: Yongqin Liu <yongqin.liu@linaro.org>
> > > > Cc: Sumit Semwal <sumit.semwal@linaro.org>
> > > > Cc: Martin Kepplinger <martin.kepplinger@puri.sm>
> > > > Cc: Amelie Delaunay <amelie.delaunay@foss.st.com>
> > > >
> > > > Saravana Kannan (4):
> > > >   usb: typec: stusb160x: Remove use of
> > > >     fw_devlink_purge_absent_suppliers()
> > > >   usb: typec: tipd: Remove use of fw_devlink_purge_absent_suppliers()
> > > >   usb: typec: tcpm: Remove use of fw_devlink_purge_absent_suppliers()
> > > >   driver core: Delete fw_devlink_purge_absent_suppliers()
> > > >
> > > >  drivers/base/core.c           | 16 ----------------
> > > >  drivers/usb/typec/stusb160x.c |  9 ---------
> > > >  drivers/usb/typec/tcpm/tcpm.c |  9 ---------
> > > >  drivers/usb/typec/tipd/core.c |  9 ---------
> > > >  include/linux/fwnode.h        |  1 -
> > > >  5 files changed, 44 deletions(-)
> > > >
> > > > --
> > > > 2.39.2.722.g9855ee24e9-goog
> > > >
> >
> >
> >
> > --
> > Best Regards,
> > Yongqin Liu
> > ---------------------------------------------------------------
> > #mailing list
> > linaro-android@lists.linaro.org
> > http://lists.linaro.org/mailman/listinfo/linaro-android



--
Best Regards,
Yongqin Liu
---------------------------------------------------------------
#mailing list
linaro-android@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-android

Re: [PATCH v1 0/4] Remove use of fw_devlink_purge_absent_suppliers()

[Fabrice Gasnier]

On 3/10/23 18:40, Saravana Kannan wrote:
> On Fri, Mar 10, 2023 at 9:21 AM Fabrice Gasnier
> <fabrice.gasnier@foss.st.com> wrote:
>>
>> On 3/1/23 22:49, Saravana Kannan wrote:
>>> Yongqin, Martin, Amelie,
>>>
>>> We recent refactor of fw_devlink that ends with commit fb42378dcc7f
>>> ("mtd: mtdpart: Don't create platform device that'll never probe"),
>>> fw_devlink is smarter and doesn't depend on compatible property. So, I
>>> don't think these calls are needed anymore. But I don't have these
>>> devices to test on and be sure and the hardware I use to test changes
>>> doesn't have this issue either.
>>>
>>> Can you please test these changes on the hardware where you hit the
>>> issue to make sure things work as expected?
>>
>>
>> Hi Saravana,
>>
>> Sorry for the late reply,
> 
> Thanks for testing!
> 
>> On behalf of Amelie, I did some testing on STM32MP15 DK2 board, on top
>> of commit fb42378dcc7f, and also with your series applied.
>> For reference, it's based on: arch/arm/boot/dts/stm32mp15xx-dkx.dtsi
>>
>> I noticed some error messages on this board, since the 12 patch series,
>> around the I2C PMIC device links:
>>
>> [    3.585514] i2c 1-0033: Failed to create device link with 1-0033
>> [    3.590115] i2c 1-0033: Failed to create device link with 1-0033
>> [    3.596278] i2c 1-0033: Failed to create device link with 1-0033
>> [    3.602188] i2c 1-0033: Failed to create device link with 1-0033
>> [    3.608165] i2c 1-0033: Failed to create device link with 1-0033
>> [    3.614278] i2c 1-0033: Failed to create device link with 1-0033
>> [    3.620256] i2c 1-0033: Failed to create device link with 1-0033
>> [    3.626253] i2c 1-0033: Failed to create device link with 1-0033
>> [    3.632252] i2c 1-0033: Failed to create device link with 1-0033
>> [    3.639001] stpmic1 1-0033: PMIC Chip Version: 0x10
>> [    3.645398] platform 5c002000.i2c:stpmic@33:regulators: Fixed
>> dependency cycle(s) with /soc/i2c@5c00200
>> 0/stpmic@33/regulators/boost
>> [    3.655937] platform 5c002000.i2c:stpmic@33:regulators: Fixed
>> dependency cycle(s) with /soc/i2c@5c00200
>> 0/stpmic@33/regulators/buck2
>> [    3.667824] platform 5c002000.i2c:stpmic@33:regulators: Fixed
>> dependency cycle(s) with /soc/i2c@5c00200
>> 0/stpmic@33/regulators/buck4
>> [    3.719751] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
>> Failed to create device link with 1-0033
>> [    3.728099] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
>> Failed to create device link with 1-0033
>> [    3.737576] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
>> Failed to create device link with 1-0033
>> [    3.747216] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
>> Failed to create device link with 1-0033
>> [    3.756750] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
>> Failed to create device link with 1-0033
>> [    3.766382] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
>> Failed to create device link with 1-0033
>> [    3.775914] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
>> Failed to create device link with 1-0033
>> [    3.785545] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators:
>> Failed to create device link with 1-0033
> 
> You can ignore all the "Failed to create device link" errors. They are
> just error logs for stuff that was being ignored silently before. So
> that's no functional regression AFAIK. I'll fix them separately if
> necessary. And I'm sure you'll see these messages even without my
> fw_devlink refactor series.

Hi Saravana,

Thanks for the information.

I tested without the 12 patch series, just before commit 3a2dbc510c43
"driver core: fw_devlink: Don't purge child fwnode's consumer links".

I don't see the messages here. But I can see these on top of fb42378dcc7f.


> 
>> Strangely some of the regulators seems to have "Fixed dependency", but
>> not all.
> 
> Yeah, that's fine too -- that's just fw_devlink being verbose about
> not enforcing probe ordering between devices in that cycle because it
> can't tell which one of the dependencies is not a probe requirement.
> Maybe I'll make it a dbg log if it's confusing people.
> 
>> Regarding the typec stusb160x I noticed the message below. It seems
>> correct, right ?
>>
>> [   15.962771] typec port0: Fixed dependency cycle(s) with
>> /soc/usb-otg@49000000/port/endpoint
> 
> I don't know if there is a cyclic dependency in your DT or not. But
> this message itself is not an issue.

Ack,

> 
>> But sometimes (lets say 1/5 times) during boot, when I have a cable
>> already plugged in, it looks like there's some race condition. The dwc2
>> driver reports some error logs in a loop, indefinitely, up to the
>> watchdog resets the platform :-(.
> 
> Can you try this series (the one you are testing) without my
> fw_devlink refactor that ends with commit fb42378dcc7f? Trying to make
> sure we can reproduce the issue Amelie was fixing before I claim my
> refactor series fixes it.

Strangely, I tested without the series, and removed earlier patch from
Amelie. I don't reproduce the issue she used to hit.

> 
>> [   16.288458] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently
>> in Host mode
>> [   16.288490] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently
>> in Host mode
>> [   16.310429] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently
>> in Host mode
>>
>> It probably just points some already existing race condition here. Maybe
>> it isn't even linked to this patch. But I have no evidence at this
>> stage. I hope I can investigate further on this one, hopefully I can
>> free up some time for that.
> 
> If you never pick up this series, are you not having any of these 1/5
> times boot issues? I wouldn't expect my changes to add any races, but
> I'll wait to see what you find here.

Some good news here is, I've identified a recent change [1], that
creates the issue pointed above. I just sent a separate patch [2] for this.
So, it's not related to this series. (I managed to reproduce without
picking it).

[1]
https://lore.kernel.org/r/20221206-dwc2-gadget-dual-role-v1-2-36515e1092cd@theobroma-systems.com
[2]
https://lore.kernel.org/lkml/20230315144433.3095859-1-fabrice.gasnier@foss.st.com/

So for stusb160x: e.g. PATCH 1, feel free to add on my:
Tested-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>

Best Regards,
Fabrice

> 
> Thanks,
> Saravana
> 
>>
>> Best Regards,
>> Fabrice
>>
>>>
>>> Yongqin, If you didn't have the context, this affected hikey960.
>>>
>>> Greg,
>>>
>>> Let's wait for some tests before we land these.
>>>
>>> Thanks,
>>> Saravana
>>>
>>> Cc: Yongqin Liu <yongqin.liu@linaro.org>
>>> Cc: Sumit Semwal <sumit.semwal@linaro.org>
>>> Cc: Martin Kepplinger <martin.kepplinger@puri.sm>
>>> Cc: Amelie Delaunay <amelie.delaunay@foss.st.com>
>>>
>>> Saravana Kannan (4):
>>>   usb: typec: stusb160x: Remove use of
>>>     fw_devlink_purge_absent_suppliers()
>>>   usb: typec: tipd: Remove use of fw_devlink_purge_absent_suppliers()
>>>   usb: typec: tcpm: Remove use of fw_devlink_purge_absent_suppliers()
>>>   driver core: Delete fw_devlink_purge_absent_suppliers()
>>>
>>>  drivers/base/core.c           | 16 ----------------
>>>  drivers/usb/typec/stusb160x.c |  9 ---------
>>>  drivers/usb/typec/tcpm/tcpm.c |  9 ---------
>>>  drivers/usb/typec/tipd/core.c |  9 ---------
>>>  include/linux/fwnode.h        |  1 -
>>>  5 files changed, 44 deletions(-)
>>>
>>
>> --
>> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
>>