From feb4b919abf39f39faf660ef3d9aedebb54f5db5 Mon Sep 17 00:00:00 2001 From: Jonas Karlman Date: Thu, 31 Aug 2023 22:16:33 +0000 Subject: phy: Set phy->dev to NULL when generic_phy_get_by_name() fails 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 --- test/dm/phy.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'test/dm/phy.c') diff --git a/test/dm/phy.c b/test/dm/phy.c index 4d4a083dd0..09329b9f71 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; } -- cgit v1.2.3 From 14639bf14d824d9fbcfd918f0e7924c7f7065422 Mon Sep 17 00:00:00 2001 From: Jonas Karlman Date: Thu, 31 Aug 2023 22:16:35 +0000 Subject: phy: Set phy->dev to NULL when generic_phy_get_by_index_nodev() fails 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 --- arch/sandbox/dts/test.dts | 11 +++++++++++ drivers/phy/phy-uclass.c | 1 + test/dm/phy.c | 12 +++++++++++- 3 files changed, 23 insertions(+), 1 deletion(-) (limited to 'test/dm/phy.c') diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index b48456aebe..63fda15da7 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -433,6 +433,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>; @@ -445,6 +450,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 0baf314a34..7d707c0229 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 09329b9f71..2abd27b22d 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); -- cgit v1.2.3 From bd78f2c2c28ca93311a7e1cdddefbd08a7cf9f70 Mon Sep 17 00:00:00 2001 From: Jonas Karlman Date: Thu, 31 Aug 2023 23:07:08 +0000 Subject: phy: Fix generic_setup_phy() return value on power on failure generic_phy_exit() typically return 0 for a struct phy that has been initialized with a generic_phy_init() call. generic_setup_phy() returns the value from a generic_phy_exit() call when generic_phy_power_on() fails. This hides the failed state of the power_on ops from the caller of generic_setup_phy(). Fix this by ignoring the return value of the generic_phy_exit() call and return the value from the generic_phy_power_on() call. Fixes: 84e561407a5f ("phy: Add generic_{setup,shutdown}_phy() helpers") Signed-off-by: Jonas Karlman --- drivers/phy/phy-uclass.c | 2 +- test/dm/phy.c | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) (limited to 'test/dm/phy.c') diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c index 7d707c0229..d745e7babc 100644 --- a/drivers/phy/phy-uclass.c +++ b/drivers/phy/phy-uclass.c @@ -526,7 +526,7 @@ int generic_setup_phy(struct udevice *dev, struct phy *phy, int index) ret = generic_phy_power_on(phy); if (ret) - ret = generic_phy_exit(phy); + generic_phy_exit(phy); } return ret; diff --git a/test/dm/phy.c b/test/dm/phy.c index 2abd27b22d..4da4841f40 100644 --- a/test/dm/phy.c +++ b/test/dm/phy.c @@ -234,3 +234,27 @@ static int dm_test_phy_multi_exit(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_phy_multi_exit, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); + +static int dm_test_phy_setup(struct unit_test_state *uts) +{ + struct phy phy; + struct udevice *parent; + + ut_assertok(uclass_get_device_by_name(UCLASS_SIMPLE_BUS, + "gen_phy_user", &parent)); + + /* normal */ + ut_assertok(generic_setup_phy(parent, &phy, 0)); + ut_assertok(generic_shutdown_phy(&phy)); + + /* power_off fail with -EIO */ + ut_assertok(generic_setup_phy(parent, &phy, 1)); + ut_asserteq(-EIO, generic_shutdown_phy(&phy)); + + /* power_on fail with -EIO */ + ut_asserteq(-EIO, generic_setup_phy(parent, &phy, 2)); + ut_assertok(generic_shutdown_phy(&phy)); + + return 0; +} +DM_TEST(dm_test_phy_setup, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); -- cgit v1.2.3 From 1a4293e001d1063401b6f94104684e38d6e401c9 Mon Sep 17 00:00:00 2001 From: Jonas Karlman Date: Thu, 31 Aug 2023 23:07:10 +0000 Subject: phy: Return success from generic_setup_phy() when phy is not found Restore the old behavior of ehci_setup_phy() and ohci_setup_phy() to return success when generic_phy_get_by_index() return -ENOENT. Fixes: 84e561407a5f ("phy: Add generic_{setup,shutdown}_phy() helpers") Fixes: 10005004db73 ("usb: ohci: Make usage of generic_{setup,shutdown}_phy() helpers") Fixes: 083f8aa978a8 ("usb: ehci: Make usage of generic_{setup,shutdown}_phy() helpers") Fixes: 75341e9c16aa ("usb: ehci: Remove unused ehci_{setup,shutdown}_phy() helpers") Signed-off-by: Jonas Karlman --- drivers/phy/phy-uclass.c | 4 ++-- test/dm/phy.c | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) (limited to 'test/dm/phy.c') diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c index d745e7babc..343c23cead 100644 --- a/drivers/phy/phy-uclass.c +++ b/drivers/phy/phy-uclass.c @@ -517,8 +517,8 @@ int generic_setup_phy(struct udevice *dev, struct phy *phy, int index) ret = generic_phy_get_by_index(dev, index, phy); if (ret) { - if (ret != -ENOENT) - return ret; + if (ret == -ENOENT) + return 0; } else { ret = generic_phy_init(phy); if (ret) diff --git a/test/dm/phy.c b/test/dm/phy.c index 4da4841f40..4f91abca3a 100644 --- a/test/dm/phy.c +++ b/test/dm/phy.c @@ -255,6 +255,11 @@ static int dm_test_phy_setup(struct unit_test_state *uts) ut_asserteq(-EIO, generic_setup_phy(parent, &phy, 2)); ut_assertok(generic_shutdown_phy(&phy)); + /* generic_phy_get_by_index fail with -ENOENT */ + ut_asserteq(-ENOENT, generic_phy_get_by_index(parent, 3, &phy)); + ut_assertok(generic_setup_phy(parent, &phy, 3)); + ut_assertok(generic_shutdown_phy(&phy)); + return 0; } DM_TEST(dm_test_phy_setup, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); -- cgit v1.2.3