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 jonas@kwiboo.se --- v2: - Move refactor to own patch - Add unit test

drivers/phy/phy-uclass.c | 2 +- test/dm/phy.c | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c index 7d707c022934..d745e7babc12 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 2abd27b22d6d..4da4841f40f7 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);

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 jonas@kwiboo.se --- v2: - New patch to fix regression introduced in v2023.01-rc1

drivers/phy/phy-uclass.c | 4 ++-- test/dm/phy.c | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c index d745e7babc12..343c23cead0c 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 4da4841f40f7..4f91abca3a02 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);

Refactor generic_{setup,shutdown}_phy() to reduce complexity and indentation. This have no intended functional change.

Signed-off-by: Jonas Karlman jonas@kwiboo.se --- v2: - Split code refactor into own patch

drivers/phy/phy-uclass.c | 41 ++++++++++++++++------------------------ 1 file changed, 16 insertions(+), 25 deletions(-)

diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c index 343c23cead0c..190108e04c7f 100644 --- a/drivers/phy/phy-uclass.c +++ b/drivers/phy/phy-uclass.c @@ -510,44 +510,35 @@ int generic_phy_power_off_bulk(struct phy_bulk *bulk)

int generic_setup_phy(struct udevice *dev, struct phy *phy, int index) { - int ret = 0; - - if (!phy) - return 0; + int ret;

ret = generic_phy_get_by_index(dev, index, phy); - if (ret) { - if (ret == -ENOENT) - return 0; - } else { - ret = generic_phy_init(phy); - if (ret) - return ret; + if (ret) + return ret == -ENOENT ? 0 : ret;

- ret = generic_phy_power_on(phy); - if (ret) - generic_phy_exit(phy); - } + ret = generic_phy_init(phy); + if (ret) + return ret; + + ret = generic_phy_power_on(phy); + if (ret) + generic_phy_exit(phy);

return ret; }

int generic_shutdown_phy(struct phy *phy) { - int ret = 0; + int ret;

- if (!phy) + if (!generic_phy_valid(phy)) return 0;

- if (generic_phy_valid(phy)) { - ret = generic_phy_power_off(phy); - if (ret) - return ret; - - ret = generic_phy_exit(phy); - } + ret = generic_phy_power_off(phy); + if (ret) + return ret;

- return ret; + return generic_phy_exit(phy); }

UCLASS_DRIVER(phy) = {