All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] phy: Fix use of generic_phy_valid() helper
@ 2023-08-31 22:16 Jonas Karlman
  2023-08-31 22:16 ` [PATCH 1/6] phy: Set phy->dev to NULL when generic_phy_get_by_name() fails Jonas Karlman
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Jonas Karlman @ 2023-08-31 22:16 UTC (permalink / raw)
  To: Simon Glass, Marek Vasut, Patrice Chotard; +Cc: u-boot, Jonas Karlman

The documentation for struct phy state that "The content of the
structure is managed solely by the PHY API and PHY drivers".

Change to use the generic_phy_valid() helper to check if phy is valid.

Patch 1-2 fixes issues where generic_phy_valid() erroneously report a
phy that has failed to be initialized as valid.
Patch 3-6 replace direct access to phy->dev with generic_phy_valid().

Jonas Karlman (6):
  phy: Set phy->dev to NULL when generic_phy_get_by_name() fails
  phy: Set phy->dev to NULL when generic_phy_get_by_index_nodev() fails
  usb: dwc3: Use generic_phy_valid() helper
  scsi: ceva: Use generic_phy_valid() helper
  net: zynq: Use generic_phy_valid() helper
  video: rockchip: dw_mipi_dsi: Use generic_phy_valid() helper

 arch/sandbox/dts/test.dts                     | 11 +++++++++++
 drivers/ata/sata_ceva.c                       |  2 +-
 drivers/net/zynq_gem.c                        |  3 ++-
 drivers/phy/phy-uclass.c                      |  4 ++++
 drivers/usb/dwc3/dwc3-generic.c               |  4 +---
 drivers/video/rockchip/dw_mipi_dsi_rockchip.c |  6 +++---
 test/dm/phy.c                                 | 18 +++++++++++++++++-
 7 files changed, 39 insertions(+), 9 deletions(-)

-- 
2.42.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/6] phy: Set phy->dev to NULL when generic_phy_get_by_name() fails
  2023-08-31 22:16 [PATCH 0/6] phy: Fix use of generic_phy_valid() helper Jonas Karlman
@ 2023-08-31 22:16 ` Jonas Karlman
  2023-09-13 22:04   ` Tom Rini
  2023-08-31 22:16 ` [PATCH 2/6] phy: Set phy->dev to NULL when generic_phy_get_by_index_nodev() fails Jonas Karlman
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Jonas Karlman @ 2023-08-31 22:16 UTC (permalink / raw)
  To: Simon Glass, Marek Vasut, Patrice Chotard, Jan Kiszka
  Cc: u-boot, Jonas Karlman

generic_phy_get_by_name() does not initialize phy->dev to NULL before
returning when dev_read_stringlist_search() fails. This can lead to an
uninitialized or reused struct phy erroneously be report as valid by
generic_phy_valid().

Fix this issue by initializing phy->dev to NULL, also extend the
dm_test_phy_base test with calls to generic_phy_valid().

Fixes: b9688df3cbf4 ("drivers: phy: Set phy->dev to NULL when generic_phy_get_by_index() fails")
Fixes: 868d58f69c7c ("usb: dwc3: Fix non-usb3 configurations")
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/phy/phy-uclass.c | 3 +++
 test/dm/phy.c            | 6 ++++++
 2 files changed, 9 insertions(+)

diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c
index 629ef3aa3de5..0baf314a3471 100644
--- a/drivers/phy/phy-uclass.c
+++ b/drivers/phy/phy-uclass.c
@@ -211,6 +211,9 @@ int generic_phy_get_by_name(struct udevice *dev, const char *phy_name,
 
 	debug("%s(dev=%p, name=%s, phy=%p)\n", __func__, dev, phy_name, phy);
 
+	assert(phy);
+	phy->dev = NULL;
+
 	index = dev_read_stringlist_search(dev, "phy-names", phy_name);
 	if (index < 0) {
 		debug("dev_read_stringlist_search() failed: %d\n", index);
diff --git a/test/dm/phy.c b/test/dm/phy.c
index 4d4a083dd0fd..09329b9f71f6 100644
--- a/test/dm/phy.c
+++ b/test/dm/phy.c
@@ -29,7 +29,9 @@ static int dm_test_phy_base(struct unit_test_state *uts)
 	 * Get the same phy port in 2 different ways and compare.
 	 */
 	ut_assertok(generic_phy_get_by_name(parent, "phy1", &phy1_method1));
+	ut_assert(generic_phy_valid(&phy1_method1));
 	ut_assertok(generic_phy_get_by_index(parent, 0, &phy1_method2));
+	ut_assert(generic_phy_valid(&phy1_method2));
 	ut_asserteq(phy1_method1.id, phy1_method2.id);
 
 	/*
@@ -50,6 +52,10 @@ static int dm_test_phy_base(struct unit_test_state *uts)
 	ut_asserteq(-ENODEV, uclass_get_device(UCLASS_PHY, 4, &dev));
 	ut_asserteq(-ENODATA, generic_phy_get_by_name(parent,
 					"phy_not_existing", &phy1_method1));
+	ut_assert(!generic_phy_valid(&phy1_method1));
+	ut_asserteq(-ENOENT, generic_phy_get_by_index(parent, 3,
+						      &phy1_method2));
+	ut_assert(!generic_phy_valid(&phy1_method2));
 
 	return 0;
 }
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/6] phy: Set phy->dev to NULL when generic_phy_get_by_index_nodev() fails
  2023-08-31 22:16 [PATCH 0/6] phy: Fix use of generic_phy_valid() helper Jonas Karlman
  2023-08-31 22:16 ` [PATCH 1/6] phy: Set phy->dev to NULL when generic_phy_get_by_name() fails Jonas Karlman
@ 2023-08-31 22:16 ` Jonas Karlman
  2023-08-31 22:16 ` [PATCH 3/6] usb: dwc3: Use generic_phy_valid() helper Jonas Karlman
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Jonas Karlman @ 2023-08-31 22:16 UTC (permalink / raw)
  To: Simon Glass, Marek Vasut, Patrice Chotard, Jean-Jacques Hiblot
  Cc: u-boot, Jonas Karlman

Generic phy helpers typically use generic_phy_valid() to determine if
the helper should perform its function on a passed struct phy.
generic_phy_valid() treat any struct phy having phy->dev set as valid.

With generic_phy_get_by_index_nodev() setting phy->dev to a valid struct
udevice early, there can be situations where the struct phy is returned
as valid when initialization in fact failed and returned an error.

Fix this by setting phy->dev back to NULL when any of the calls to
of_xlate ops, device_get_supply_regulator or phy_alloc_counts fail. Also
extend the dm_test_phy_base test with a test where of_xlate ops fail.

Fixes: 72e5016f878d ("drivers: phy: add generic PHY framework")
Fixes: b9688df3cbf4 ("drivers: phy: Set phy->dev to NULL when generic_phy_get_by_index() fails")
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 arch/sandbox/dts/test.dts | 11 +++++++++++
 drivers/phy/phy-uclass.c  |  1 +
 test/dm/phy.c             | 12 +++++++++++-
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index f351d5cb84b0..6be212403473 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -431,6 +431,11 @@
 		#phy-cells = <0>;
 	};
 
+	phy_provider3: gen_phy@3 {
+		compatible = "sandbox,phy";
+		#phy-cells = <2>;
+	};
+
 	gen_phy_user: gen_phy_user {
 		compatible = "simple-bus";
 		phys = <&phy_provider0 0>, <&phy_provider0 1>, <&phy_provider1>;
@@ -443,6 +448,12 @@
 		phy-names = "phy1", "phy2";
 	};
 
+	gen_phy_user2: gen_phy_user2 {
+		compatible = "simple-bus";
+		phys = <&phy_provider3 0 0>;
+		phy-names = "phy1";
+	};
+
 	some-bus {
 		#address-cells = <1>;
 		#size-cells = <0>;
diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c
index 0baf314a3471..7d707c022934 100644
--- a/drivers/phy/phy-uclass.c
+++ b/drivers/phy/phy-uclass.c
@@ -195,6 +195,7 @@ int generic_phy_get_by_index_nodev(ofnode node, int index, struct phy *phy)
 	return 0;
 
 err:
+	phy->dev = NULL;
 	return ret;
 }
 
diff --git a/test/dm/phy.c b/test/dm/phy.c
index 09329b9f71f6..2abd27b22d6d 100644
--- a/test/dm/phy.c
+++ b/test/dm/phy.c
@@ -49,7 +49,7 @@ static int dm_test_phy_base(struct unit_test_state *uts)
 	ut_assert(phy2.dev != phy3.dev);
 
 	/* Try to get a non-existing phy */
-	ut_asserteq(-ENODEV, uclass_get_device(UCLASS_PHY, 4, &dev));
+	ut_asserteq(-ENODEV, uclass_get_device(UCLASS_PHY, 5, &dev));
 	ut_asserteq(-ENODATA, generic_phy_get_by_name(parent,
 					"phy_not_existing", &phy1_method1));
 	ut_assert(!generic_phy_valid(&phy1_method1));
@@ -57,6 +57,16 @@ static int dm_test_phy_base(struct unit_test_state *uts)
 						      &phy1_method2));
 	ut_assert(!generic_phy_valid(&phy1_method2));
 
+	/* Try to get a phy where of_xlate fail */
+	ut_assertok(uclass_get_device_by_name(UCLASS_SIMPLE_BUS,
+					      "gen_phy_user2", &parent));
+	ut_asserteq(-EINVAL, generic_phy_get_by_name(parent, "phy1",
+						     &phy1_method1));
+	ut_assert(!generic_phy_valid(&phy1_method1));
+	ut_asserteq(-EINVAL, generic_phy_get_by_index(parent, 0,
+						      &phy1_method2));
+	ut_assert(!generic_phy_valid(&phy1_method2));
+
 	return 0;
 }
 DM_TEST(dm_test_phy_base, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/6] usb: dwc3: Use generic_phy_valid() helper
  2023-08-31 22:16 [PATCH 0/6] phy: Fix use of generic_phy_valid() helper Jonas Karlman
  2023-08-31 22:16 ` [PATCH 1/6] phy: Set phy->dev to NULL when generic_phy_get_by_name() fails Jonas Karlman
  2023-08-31 22:16 ` [PATCH 2/6] phy: Set phy->dev to NULL when generic_phy_get_by_index_nodev() fails Jonas Karlman
@ 2023-08-31 22:16 ` Jonas Karlman
  2023-08-31 22:16 ` [PATCH 4/6] scsi: ceva: " Jonas Karlman
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Jonas Karlman @ 2023-08-31 22:16 UTC (permalink / raw)
  To: Simon Glass, Marek Vasut, Patrice Chotard, Michal Simek
  Cc: u-boot, Jonas Karlman

The documentation for struct phy state that "The content of the
structure is managed solely by the PHY API and PHY drivers".

Change to use the generic_phy_valid() helper to check if phy is valid.
Also remove setting phy->dev to NULL now that generic_phy_get_by_name()
properly initialize phy->dev to NULL.

Fixes: 142d50fbce7c ("usb: dwc3: Add support for usb3-phy PHY configuration")
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/usb/dwc3/dwc3-generic.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
index 7f0af05855ab..3997b9dbff43 100644
--- a/drivers/usb/dwc3/dwc3-generic.c
+++ b/drivers/usb/dwc3/dwc3-generic.c
@@ -541,8 +541,6 @@ int dwc3_glue_probe(struct udevice *dev)
 	} else if (ret != -ENOENT && ret != -ENODATA) {
 		debug("could not get phy (err %d)\n", ret);
 		return ret;
-	} else {
-		phy.dev = NULL;
 	}
 
 	glue->regs = dev_read_addr_size_index(dev, 0, &glue->size);
@@ -555,7 +553,7 @@ int dwc3_glue_probe(struct udevice *dev)
 	if (ret)
 		return ret;
 
-	if (phy.dev) {
+	if (generic_phy_valid(&phy)) {
 		ret = generic_phy_power_on(&phy);
 		if (ret)
 			return ret;
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/6] scsi: ceva: Use generic_phy_valid() helper
  2023-08-31 22:16 [PATCH 0/6] phy: Fix use of generic_phy_valid() helper Jonas Karlman
                   ` (2 preceding siblings ...)
  2023-08-31 22:16 ` [PATCH 3/6] usb: dwc3: Use generic_phy_valid() helper Jonas Karlman
@ 2023-08-31 22:16 ` Jonas Karlman
  2023-09-01  6:11   ` Michal Simek
  2023-08-31 22:16 ` [PATCH 5/6] net: zynq: " Jonas Karlman
  2023-08-31 22:16 ` [PATCH 6/6] video: rockchip: dw_mipi_dsi: " Jonas Karlman
  5 siblings, 1 reply; 12+ messages in thread
From: Jonas Karlman @ 2023-08-31 22:16 UTC (permalink / raw)
  To: Simon Glass, Marek Vasut, Patrice Chotard, Vladimir Oltean, Michal Simek
  Cc: u-boot, Jonas Karlman

The documentation for struct phy state that "The content of the
structure is managed solely by the PHY API and PHY drivers".

Change to use the generic_phy_valid() helper to check if phy is valid.

Fixes: f6f5451d469b ("scsi: ceva: Enable PHY and reset support")
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/ata/sata_ceva.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ata/sata_ceva.c b/drivers/ata/sata_ceva.c
index 47366438fdfd..7769d4f99efd 100644
--- a/drivers/ata/sata_ceva.c
+++ b/drivers/ata/sata_ceva.c
@@ -217,7 +217,7 @@ static int sata_ceva_probe(struct udevice *dev)
 		}
 	}
 
-	if (phy.dev) {
+	if (generic_phy_valid(&phy)) {
 		dev_dbg(dev, "Perform PHY power on\n");
 		ret = generic_phy_power_on(&phy);
 		if (ret) {
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/6] net: zynq: Use generic_phy_valid() helper
  2023-08-31 22:16 [PATCH 0/6] phy: Fix use of generic_phy_valid() helper Jonas Karlman
                   ` (3 preceding siblings ...)
  2023-08-31 22:16 ` [PATCH 4/6] scsi: ceva: " Jonas Karlman
@ 2023-08-31 22:16 ` Jonas Karlman
  2023-09-01  6:11   ` Michal Simek
  2023-08-31 22:16 ` [PATCH 6/6] video: rockchip: dw_mipi_dsi: " Jonas Karlman
  5 siblings, 1 reply; 12+ messages in thread
From: Jonas Karlman @ 2023-08-31 22:16 UTC (permalink / raw)
  To: Simon Glass, Marek Vasut, Patrice Chotard, Michal Simek,
	Joe Hershberger, Ramon Fried, Sean Anderson
  Cc: u-boot, Jonas Karlman

The documentation for struct phy state that "The content of the
structure is managed solely by the PHY API and PHY drivers".

Change to use the generic_phy_valid() helper to check if phy is valid.

Fixes: 10c50b1facbf ("net: zynq: Add support for PHY configuration in SGMII mode")
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/net/zynq_gem.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c
index f3cdfb0275d0..3377e669f2f6 100644
--- a/drivers/net/zynq_gem.c
+++ b/drivers/net/zynq_gem.c
@@ -890,7 +890,8 @@ static int zynq_gem_probe(struct udevice *dev)
 	if (ret)
 		goto err3;
 
-	if (priv->interface == PHY_INTERFACE_MODE_SGMII && phy.dev) {
+	if (priv->interface == PHY_INTERFACE_MODE_SGMII &&
+	    generic_phy_valid(&phy)) {
 		if (IS_ENABLED(CONFIG_DM_ETH_PHY)) {
 			if (device_is_compatible(dev, "cdns,zynqmp-gem") ||
 			    device_is_compatible(dev, "xlnx,zynqmp-gem")) {
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 6/6] video: rockchip: dw_mipi_dsi: Use generic_phy_valid() helper
  2023-08-31 22:16 [PATCH 0/6] phy: Fix use of generic_phy_valid() helper Jonas Karlman
                   ` (4 preceding siblings ...)
  2023-08-31 22:16 ` [PATCH 5/6] net: zynq: " Jonas Karlman
@ 2023-08-31 22:16 ` Jonas Karlman
  2023-09-01  7:49   ` Kever Yang
  5 siblings, 1 reply; 12+ messages in thread
From: Jonas Karlman @ 2023-08-31 22:16 UTC (permalink / raw)
  To: Simon Glass, Marek Vasut, Patrice Chotard, Anatolij Gustschin,
	Philipp Tomsich, Kever Yang, Ondrej Jirman
  Cc: u-boot, Jonas Karlman

The documentation for struct phy state that "The content of the
structure is managed solely by the PHY API and PHY drivers".

Change to use the generic_phy_valid() helper to check if phy is valid.

Fixes: b7d8d40346f2 ("video: rockchip: dw_mipi_dsi: Fix external phy existence check")
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/video/rockchip/dw_mipi_dsi_rockchip.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/video/rockchip/dw_mipi_dsi_rockchip.c b/drivers/video/rockchip/dw_mipi_dsi_rockchip.c
index 0852b53ebed5..1a5ab781e3f1 100644
--- a/drivers/video/rockchip/dw_mipi_dsi_rockchip.c
+++ b/drivers/video/rockchip/dw_mipi_dsi_rockchip.c
@@ -377,7 +377,7 @@ static int dsi_phy_init(void *priv_data)
 	struct dw_rockchip_dsi_priv *dsi = dev_get_priv(dev);
 	int ret, i, vco;
 
-	if (dsi->phy.dev) {
+	if (generic_phy_valid(&dsi->phy)) {
 		ret = generic_phy_configure(&dsi->phy, &dsi->phy_opts);
 		if (ret) {
 			dev_err(dsi->dsi_host,
@@ -559,7 +559,7 @@ dw_mipi_dsi_get_lane_mbps(void *priv_data, struct display_timing *timings,
 	}
 
 	/* for external phy only the mipi_dphy_config is necessary */
-	if (dsi->phy.dev) {
+	if (generic_phy_valid(&dsi->phy)) {
 		phy_mipi_dphy_get_default_config(timings->pixelclock.typ  * 10 / 8,
 						 bpp, lanes,
 						 &dsi->phy_opts);
@@ -859,7 +859,7 @@ static int dw_mipi_dsi_rockchip_probe(struct udevice *dev)
 	}
 
 	/* Get a ref clock only if not using an external phy. */
-	if (priv->phy.dev) {
+	if (generic_phy_valid(&priv->phy)) {
 		dev_dbg(dev, "setting priv->ref to NULL\n");
 		priv->ref = NULL;
 
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/6] scsi: ceva: Use generic_phy_valid() helper
  2023-08-31 22:16 ` [PATCH 4/6] scsi: ceva: " Jonas Karlman
@ 2023-09-01  6:11   ` Michal Simek
  0 siblings, 0 replies; 12+ messages in thread
From: Michal Simek @ 2023-09-01  6:11 UTC (permalink / raw)
  To: Jonas Karlman, Simon Glass, Marek Vasut, Patrice Chotard,
	Vladimir Oltean
  Cc: u-boot



On 9/1/23 00:16, Jonas Karlman wrote:
> The documentation for struct phy state that "The content of the
> structure is managed solely by the PHY API and PHY drivers".
> 
> Change to use the generic_phy_valid() helper to check if phy is valid.
> 
> Fixes: f6f5451d469b ("scsi: ceva: Enable PHY and reset support")
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
>   drivers/ata/sata_ceva.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/ata/sata_ceva.c b/drivers/ata/sata_ceva.c
> index 47366438fdfd..7769d4f99efd 100644
> --- a/drivers/ata/sata_ceva.c
> +++ b/drivers/ata/sata_ceva.c
> @@ -217,7 +217,7 @@ static int sata_ceva_probe(struct udevice *dev)
>   		}
>   	}
>   
> -	if (phy.dev) {
> +	if (generic_phy_valid(&phy)) {
>   		dev_dbg(dev, "Perform PHY power on\n");
>   		ret = generic_phy_power_on(&phy);
>   		if (ret) {

Applied.
M

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/6] net: zynq: Use generic_phy_valid() helper
  2023-08-31 22:16 ` [PATCH 5/6] net: zynq: " Jonas Karlman
@ 2023-09-01  6:11   ` Michal Simek
  0 siblings, 0 replies; 12+ messages in thread
From: Michal Simek @ 2023-09-01  6:11 UTC (permalink / raw)
  To: Jonas Karlman, Simon Glass, Marek Vasut, Patrice Chotard,
	Joe Hershberger, Ramon Fried, Sean Anderson
  Cc: u-boot



On 9/1/23 00:16, Jonas Karlman wrote:
> The documentation for struct phy state that "The content of the
> structure is managed solely by the PHY API and PHY drivers".
> 
> Change to use the generic_phy_valid() helper to check if phy is valid.
> 
> Fixes: 10c50b1facbf ("net: zynq: Add support for PHY configuration in SGMII mode")
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
>   drivers/net/zynq_gem.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c
> index f3cdfb0275d0..3377e669f2f6 100644
> --- a/drivers/net/zynq_gem.c
> +++ b/drivers/net/zynq_gem.c
> @@ -890,7 +890,8 @@ static int zynq_gem_probe(struct udevice *dev)
>   	if (ret)
>   		goto err3;
>   
> -	if (priv->interface == PHY_INTERFACE_MODE_SGMII && phy.dev) {
> +	if (priv->interface == PHY_INTERFACE_MODE_SGMII &&
> +	    generic_phy_valid(&phy)) {
>   		if (IS_ENABLED(CONFIG_DM_ETH_PHY)) {
>   			if (device_is_compatible(dev, "cdns,zynqmp-gem") ||
>   			    device_is_compatible(dev, "xlnx,zynqmp-gem")) {

Applied.
M

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 6/6] video: rockchip: dw_mipi_dsi: Use generic_phy_valid() helper
  2023-08-31 22:16 ` [PATCH 6/6] video: rockchip: dw_mipi_dsi: " Jonas Karlman
@ 2023-09-01  7:49   ` Kever Yang
  2023-09-01 14:10     ` Jonas Karlman
  0 siblings, 1 reply; 12+ messages in thread
From: Kever Yang @ 2023-09-01  7:49 UTC (permalink / raw)
  To: Jonas Karlman, Simon Glass, Marek Vasut, Patrice Chotard,
	Anatolij Gustschin, Philipp Tomsich, Ondrej Jirman
  Cc: u-boot

Hi Jonas,

     Why is this patch 6/6 and didn't see other patches?

On 2023/9/1 06:16, Jonas Karlman wrote:
> The documentation for struct phy state that "The content of the
> structure is managed solely by the PHY API and PHY drivers".
>
> Change to use the generic_phy_valid() helper to check if phy is valid.
>
> Fixes: b7d8d40346f2 ("video: rockchip: dw_mipi_dsi: Fix external phy existence check")
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
>   drivers/video/rockchip/dw_mipi_dsi_rockchip.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/rockchip/dw_mipi_dsi_rockchip.c b/drivers/video/rockchip/dw_mipi_dsi_rockchip.c
> index 0852b53ebed5..1a5ab781e3f1 100644
> --- a/drivers/video/rockchip/dw_mipi_dsi_rockchip.c
> +++ b/drivers/video/rockchip/dw_mipi_dsi_rockchip.c
> @@ -377,7 +377,7 @@ static int dsi_phy_init(void *priv_data)
>   	struct dw_rockchip_dsi_priv *dsi = dev_get_priv(dev);
>   	int ret, i, vco;
>   
> -	if (dsi->phy.dev) {
> +	if (generic_phy_valid(&dsi->phy)) {
>   		ret = generic_phy_configure(&dsi->phy, &dsi->phy_opts);
>   		if (ret) {
>   			dev_err(dsi->dsi_host,
> @@ -559,7 +559,7 @@ dw_mipi_dsi_get_lane_mbps(void *priv_data, struct display_timing *timings,
>   	}
>   
>   	/* for external phy only the mipi_dphy_config is necessary */
> -	if (dsi->phy.dev) {
> +	if (generic_phy_valid(&dsi->phy)) {
>   		phy_mipi_dphy_get_default_config(timings->pixelclock.typ  * 10 / 8,
>   						 bpp, lanes,
>   						 &dsi->phy_opts);
> @@ -859,7 +859,7 @@ static int dw_mipi_dsi_rockchip_probe(struct udevice *dev)
>   	}
>   
>   	/* Get a ref clock only if not using an external phy. */
> -	if (priv->phy.dev) {
> +	if (generic_phy_valid(&priv->phy)) {
>   		dev_dbg(dev, "setting priv->ref to NULL\n");
>   		priv->ref = NULL;
>   

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 6/6] video: rockchip: dw_mipi_dsi: Use generic_phy_valid() helper
  2023-09-01  7:49   ` Kever Yang
@ 2023-09-01 14:10     ` Jonas Karlman
  0 siblings, 0 replies; 12+ messages in thread
From: Jonas Karlman @ 2023-09-01 14:10 UTC (permalink / raw)
  To: Kever Yang, Simon Glass, Marek Vasut, Patrice Chotard,
	Anatolij Gustschin, Philipp Tomsich, Ondrej Jirman
  Cc: u-boot

Hi Kever,

On 2023-09-01 09:49, Kever Yang wrote:
> Hi Jonas,
> 
>      Why is this patch 6/6 and didn't see other patches?

Patch 1-3 moves a workaround made in usb dwc3 driver into generic phy
core so entire series was sent to usb maintainer and cc mailing list.

Patch 4-6 was trivial and had no real dependencies on prior patches but
was related to the work made in patch 1-3, so sent these to the listed
maintainers and cc mailing list.

Full series can also be found at:
https://patchwork.ozlabs.org/project/uboot/list/?series=371248&state=*

Regards,
Jonas

> 
> On 2023/9/1 06:16, Jonas Karlman wrote:
>> The documentation for struct phy state that "The content of the
>> structure is managed solely by the PHY API and PHY drivers".
>>
>> Change to use the generic_phy_valid() helper to check if phy is valid.
>>
>> Fixes: b7d8d40346f2 ("video: rockchip: dw_mipi_dsi: Fix external phy existence check")
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
> 
> Thanks,
> - Kever
>> ---
>>   drivers/video/rockchip/dw_mipi_dsi_rockchip.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/video/rockchip/dw_mipi_dsi_rockchip.c b/drivers/video/rockchip/dw_mipi_dsi_rockchip.c
>> index 0852b53ebed5..1a5ab781e3f1 100644
>> --- a/drivers/video/rockchip/dw_mipi_dsi_rockchip.c
>> +++ b/drivers/video/rockchip/dw_mipi_dsi_rockchip.c
>> @@ -377,7 +377,7 @@ static int dsi_phy_init(void *priv_data)
>>   	struct dw_rockchip_dsi_priv *dsi = dev_get_priv(dev);
>>   	int ret, i, vco;
>>   
>> -	if (dsi->phy.dev) {
>> +	if (generic_phy_valid(&dsi->phy)) {
>>   		ret = generic_phy_configure(&dsi->phy, &dsi->phy_opts);
>>   		if (ret) {
>>   			dev_err(dsi->dsi_host,
>> @@ -559,7 +559,7 @@ dw_mipi_dsi_get_lane_mbps(void *priv_data, struct display_timing *timings,
>>   	}
>>   
>>   	/* for external phy only the mipi_dphy_config is necessary */
>> -	if (dsi->phy.dev) {
>> +	if (generic_phy_valid(&dsi->phy)) {
>>   		phy_mipi_dphy_get_default_config(timings->pixelclock.typ  * 10 / 8,
>>   						 bpp, lanes,
>>   						 &dsi->phy_opts);
>> @@ -859,7 +859,7 @@ static int dw_mipi_dsi_rockchip_probe(struct udevice *dev)
>>   	}
>>   
>>   	/* Get a ref clock only if not using an external phy. */
>> -	if (priv->phy.dev) {
>> +	if (generic_phy_valid(&priv->phy)) {
>>   		dev_dbg(dev, "setting priv->ref to NULL\n");
>>   		priv->ref = NULL;
>>   


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/6] phy: Set phy->dev to NULL when generic_phy_get_by_name() fails
  2023-08-31 22:16 ` [PATCH 1/6] phy: Set phy->dev to NULL when generic_phy_get_by_name() fails Jonas Karlman
@ 2023-09-13 22:04   ` Tom Rini
  0 siblings, 0 replies; 12+ messages in thread
From: Tom Rini @ 2023-09-13 22:04 UTC (permalink / raw)
  To: Jonas Karlman
  Cc: Simon Glass, Marek Vasut, Patrice Chotard, Jan Kiszka, u-boot

[-- Attachment #1: Type: text/plain, Size: 724 bytes --]

On Thu, Aug 31, 2023 at 10:16:33PM +0000, Jonas Karlman wrote:

> generic_phy_get_by_name() does not initialize phy->dev to NULL before
> returning when dev_read_stringlist_search() fails. This can lead to an
> uninitialized or reused struct phy erroneously be report as valid by
> generic_phy_valid().
> 
> Fix this issue by initializing phy->dev to NULL, also extend the
> dm_test_phy_base test with calls to generic_phy_valid().
> 
> Fixes: b9688df3cbf4 ("drivers: phy: Set phy->dev to NULL when generic_phy_get_by_index() fails")
> Fixes: 868d58f69c7c ("usb: dwc3: Fix non-usb3 configurations")
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>

For the series, applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-09-13 22:04 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-31 22:16 [PATCH 0/6] phy: Fix use of generic_phy_valid() helper Jonas Karlman
2023-08-31 22:16 ` [PATCH 1/6] phy: Set phy->dev to NULL when generic_phy_get_by_name() fails Jonas Karlman
2023-09-13 22:04   ` Tom Rini
2023-08-31 22:16 ` [PATCH 2/6] phy: Set phy->dev to NULL when generic_phy_get_by_index_nodev() fails Jonas Karlman
2023-08-31 22:16 ` [PATCH 3/6] usb: dwc3: Use generic_phy_valid() helper Jonas Karlman
2023-08-31 22:16 ` [PATCH 4/6] scsi: ceva: " Jonas Karlman
2023-09-01  6:11   ` Michal Simek
2023-08-31 22:16 ` [PATCH 5/6] net: zynq: " Jonas Karlman
2023-09-01  6:11   ` Michal Simek
2023-08-31 22:16 ` [PATCH 6/6] video: rockchip: dw_mipi_dsi: " Jonas Karlman
2023-09-01  7:49   ` Kever Yang
2023-09-01 14:10     ` Jonas Karlman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.