phy-core: Don't print an error on probe deferral or nodata
PATCH 1/2 phy-core: Don't print an error on probe deferral or nodata
Printing an error on probe-deferral clearly is not the right thing to do. While at it I've also silenced the error in case of -ENODATA, so that devm_phy_get can be used to get an optional phy without causing errors to be printed. Alternatively a new devm_phy_get_optional method could be added for this.
While at it also factor the error handling out of the if ... else ..., as it is identical in both branches.
--- drivers/phy/phy-core.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
drivers/phy/phy-core.c +6 -8
@@ -371,16 +371,14 @@ struct phy *phy_get(struct device *dev, const char *string) index =of_property_match_string (dev->of_node,"phy-names" , string); phy =of_phy_get (dev, index); -if (IS_ERR (phy)) { -dev_err (dev,"unable to find phy\n" ); -return phy; - } }else { phy =phy_lookup (dev, string); -if (IS_ERR (phy)) { -dev_err (dev,"unable to find phy\n" ); -return phy; - } + } +if (IS_ERR (phy)) { +int err =PTR_ERR (phy); +if (err != -EPROBE_DEFER && err != -ENODATA ) +dev_err (dev,"unable to get phy\n" ); +return phy; }if (!try_module_get (phy->ops->owner))
Signature · 1 line
1.8.4.2
PATCH 2/2 phy-core: Don't propagate -ENOSUPP from phy_pm_runtime_get_sync to caller
The phy-core allows phy_init and phy_power_on to be called multiple times, but before this patch -ENOSUPP from phy_pm_runtime_get_sync would be propagated to the caller for the 2nd and later calls.
--- drivers/phy/phy-core.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/phy/phy-core.c +4 -2
@@ -161,7 +161,8 @@ int phy_init(struct phy *phy)dev_err (&phy->dev,"phy init failed --> %d\n" , ret); goto out; } - } + }else + ret =0 ;/* Override possible ret == -ENOTSUPP */ out:mutex_unlock (&phy->mutex);
@@ -209,7 +210,8 @@ int phy_power_on(struct phy *phy)dev_err (&phy->dev,"phy poweron failed --> %d\n" , ret); goto out; } - } + }else + ret =0 ;/* Override possible ret == -ENOTSUPP */ out:mutex_unlock (&phy->mutex);
Signature · 1 line
1.8.4.2
PATCH 2/2 phy-core: Don't propagate -ENOSUPP from phy_pm_runtime_get_sync to caller
Hello.
Quoted 14 lines · Hans de Goede
The phy-core allows
phy_initandphy_power_onto be called multiple times, but before this patch-ENOSUPPfromphy_pm_runtime_get_syncwould be propagated to the caller for the 2nd and later calls.Signed-off-by Hans de Goede <[email protected]>--- drivers/phy/phy-core.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)drivers/phy/phy-core.c +0
-0
drivers/phy/phy-core.c int phy_init(struct phy *phy) · line 165
Quoted 6 lines
drivers/phy/phy-core.c +2
-1@@ -161,7 +161,8 @@ int phy_init(struct phy *phy)dev_err (&phy->dev,"phy init failed --> %d\n" , ret); goto out; } - } + }else + ret =0 ;/* Override possible ret == -ENOTSUPP */
*else* arm should have {} when the *if* arm has it.
drivers/phy/phy-core.c int phy_power_on(struct phy *phy) · line 214
Quoted 7 lines
drivers/phy/phy-core.c +2
-1@@ -209,7 +210,8 @@ int phy_power_on(struct phy *phy)dev_err (&phy->dev,"phy poweron failed --> %d\n" , ret); goto out; } - } + }else + ret =0 ;/* Override possible ret == -ENOTSUPP */
Same here.
WBR, Sergei
PATCH 2/2 phy-core: Don't propagate -ENOSUPP from phy_pm_runtime_get_sync to caller
In reply to Hans de GoedeHi,
Quoted 3 lines · Hans de Goede
The phy-core allows
phy_initandphy_power_onto be called multiple times, but before this patch-ENOSUPPfromphy_pm_runtime_get_syncwould be propagated to the caller for the 2nd and later calls.
Thanks for fixing this. Have one minor comment below.
Quoted 11 lines
Signed-off-by Hans de Goede <[email protected]>--- drivers/phy/phy-core.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)drivers/phy/phy-core.c +0
-0
drivers/phy/phy-core.c int phy_init(struct phy *phy) · line 165
Quoted 6 lines
drivers/phy/phy-core.c +2
-1@@ -161,7 +161,8 @@ int phy_init(struct phy *phy)dev_err (&phy->dev,"phy init failed --> %d\n" , ret); goto out; } - } + }else + ret =0 ;/* Override possible ret == -ENOTSUPP */
'should use braces in both branches'.
drivers/phy/phy-core.c int phy_power_on(struct phy *phy) · line 214
Quoted 10 lines
drivers/phy/phy-core.c +2
-1out:mutex_unlock (&phy->mutex);@@ -209,7 +210,8 @@ int phy_power_on(struct phy *phy)dev_err (&phy->dev,"phy poweron failed --> %d\n" , ret); goto out; } - } + }else + ret =0 ;/* Override possible ret == -ENOTSUPP */
same here..
Thanks
Kishon
PATCH 2/2 phy-core: Don't propagate -ENOSUPP from phy_pm_runtime_get_sync to caller
Quoted 14 lines · Kishon Vijay Abraham I
Hi,
Quoted 3 lines · Hans de Goede
The phy-core allows
phy_initandphy_power_onto be called multiple times, but before this patch-ENOSUPPfromphy_pm_runtime_get_syncwould be propagated to the caller for the 2nd and later calls.Thanks for fixing this. Have one minor comment below.
Quoted 6 lines
Signed-off-by Hans de Goede <[email protected]>--- drivers/phy/phy-core.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
Quoted 13 lines
drivers/phy/phy-core.c int phy_init(struct phy *phy) · line 165
Quoted 11 lines
drivers/phy/phy-core.c +2
-1@@ -161,7 +161,8 @@ int phy_init(struct phy *phy)dev_err (&phy->dev,"phy init failed --> %d\n" , ret); goto out; } - } + }else + ret =0 ;/* Override possible ret == -ENOTSUPP */ 'should use braces in both branches'.
ah.. Sergei had already commented :-)
Quoted 16 lines
drivers/phy/phy-core.c int phy_power_on(struct phy *phy) · line 214
Quoted 10 lines
drivers/phy/phy-core.c +2
-1out:mutex_unlock (&phy->mutex);@@ -209,7 +210,8 @@ int phy_power_on(struct phy *phy)dev_err (&phy->dev,"phy poweron failed --> %d\n" , ret); goto out; } - } + }else + ret =0 ;/* Override possible ret == -ENOTSUPP */ same here..
Thanks
Kishon
PATCH 2/2 phy-core: Don't propagate -ENOSUPP from phy_pm_runtime_get_sync to caller
In reply to Kishon Vijay Abraham IHi,
Quoted 14 lines · Kishon Vijay Abraham I
Hi,
Quoted 3 lines · Hans de Goede
The phy-core allows
phy_initandphy_power_onto be called multiple times, but before this patch-ENOSUPPfromphy_pm_runtime_get_syncwould be propagated to the caller for the 2nd and later calls.Thanks for fixing this. Have one minor comment below.
Quoted 6 lines
Signed-off-by Hans de Goede <[email protected]>--- drivers/phy/phy-core.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
Quoted 13 lines
drivers/phy/phy-core.c int phy_init(struct phy *phy) · line 165
Quoted 11 lines
drivers/phy/phy-core.c +2
-1@@ -161,7 +161,8 @@ int phy_init(struct phy *phy)dev_err (&phy->dev,"phy init failed --> %d\n" , ret); goto out; } - } + }else + ret =0 ;/* Override possible ret == -ENOTSUPP */ 'should use braces in both branches'.
Ok, I'll do a v2 fixing this soonish, any comments on the first patch?
Regards,
Hans
PATCH 1/2 phy-core: Don't print an error on probe deferral or nodata
In reply to Hans de GoedeHi,
Quoted 3 lines · Hans de Goede
Printing an error on probe-deferral clearly is not the right thing to do. While at it I've also silenced the error in case of
-ENODATA, so thatdevm_phy_getcan be used to get an optional phy without causing errors to
What do you mean by optional phy here?
Thanks
Kishon
Quoted 39 lines
be printed. Alternatively a new
devm_phy_get_optionalmethod could be added for this.While at it also factor the error handling out of the if ... else ..., as it is identical in both branches.
Signed-off-by Hans de Goede <[email protected]>--- drivers/phy/phy-core.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)drivers/phy/phy-core.c +6
-8@@ -371,16 +371,14 @@ struct phy *phy_get(struct device *dev, const char *string) index =of_property_match_string (dev->of_node,"phy-names" , string); phy =of_phy_get (dev, index); -if (IS_ERR (phy)) { -dev_err (dev,"unable to find phy\n" ); -return phy; - } }else { phy =phy_lookup (dev, string); -if (IS_ERR (phy)) { -dev_err (dev,"unable to find phy\n" ); -return phy; - } + } +if (IS_ERR (phy)) { +int err =PTR_ERR (phy); +if (err != -EPROBE_DEFER && err != -ENODATA ) +dev_err (dev,"unable to get phy\n" ); +return phy; }if (!try_module_get (phy->ops->owner))
PATCH 1/2 phy-core: Don't print an error on probe deferral or nodata
Hi,
Quoted 8 lines · Kishon Vijay Abraham I
Hi,
Quoted 3 lines · Hans de Goede
Printing an error on probe-deferral clearly is not the right thing to do. While at it I've also silenced the error in case of
-ENODATA, so thatdevm_phy_getcan be used to get an optional phy without causing errors toWhat do you mean by optional phy here?
Some generic platform drivers, ie drivers/usb/host/ehci-platform.c (with some extensions to make it more generic I'm working on), may take a phy in the devicetree-node, but having a phy is not mandatory, the code using the phy will all be guarded against the phy not being there and only call phy_ functions if it is actually there.
For this use case it would be nice if in this case devm_phy_get would return an error code, but not do a dev_err, so as to not pollute dmesg with irrelevant / wrong error messages.
The regulator core has devm_regulator_get_optional for this, so if you don't want this silent behavior in devm_phy_get by default, I can do write a different patch adding devm_phy_get_optional instead.
Also I've just realized that this check:
Quoted 2 lines
drivers/phy/phy-core.c struct phy *phy_get(struct device *dev, const char *string) · line 380
Quoted 2 lines
drivers/phy/phy-core.c +2
-0+if (err != -EPROBE_DEFER && err != -ENODATA ) +dev_err (dev,"unable to get phy\n" );
Is incomplete to also be silent when no phy-names are specified, it should be:
if (err != -EPROBE_DEFER && err != -ENODATA && err != -EINVAL) dev_err(dev, "unable to get phy\n");
I'll fix this (or add a devm_phy_get_optional) in v2 of this set.
Regards,
Hans