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

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

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) {

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")) {

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;