* [PATCH 1/2] phy-core: Don't print an error on probe deferral or nodata
@ 2014-01-05 23:06 Hans de Goede
2014-01-05 23:06 ` [PATCH 2/2] phy-core: Don't propagate -ENOSUPP from phy_pm_runtime_get_sync to caller Hans de Goede
2014-01-07 9:31 ` [PATCH 1/2] phy-core: Don't print an error on probe deferral or nodata Kishon Vijay Abraham I
0 siblings, 2 replies; 8+ messages in thread
From: Hans de Goede @ 2014-01-05 23:06 UTC (permalink / raw)
To: linux-arm-kernel
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.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/phy/phy-core.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 58e0e97..d7b992e 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -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))
--
1.8.4.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] phy-core: Don't propagate -ENOSUPP from phy_pm_runtime_get_sync to caller
2014-01-05 23:06 [PATCH 1/2] phy-core: Don't print an error on probe deferral or nodata Hans de Goede
@ 2014-01-05 23:06 ` Hans de Goede
2014-01-06 17:37 ` Sergei Shtylyov
2014-01-07 8:56 ` Kishon Vijay Abraham I
2014-01-07 9:31 ` [PATCH 1/2] phy-core: Don't print an error on probe deferral or nodata Kishon Vijay Abraham I
1 sibling, 2 replies; 8+ messages in thread
From: Hans de Goede @ 2014-01-05 23:06 UTC (permalink / raw)
To: linux-arm-kernel
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.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/phy/phy-core.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index d7b992e..8ee6157 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -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);
--
1.8.4.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] phy-core: Don't propagate -ENOSUPP from phy_pm_runtime_get_sync to caller
2014-01-05 23:06 ` [PATCH 2/2] phy-core: Don't propagate -ENOSUPP from phy_pm_runtime_get_sync to caller Hans de Goede
@ 2014-01-06 17:37 ` Sergei Shtylyov
2014-01-07 8:56 ` Kishon Vijay Abraham I
1 sibling, 0 replies; 8+ messages in thread
From: Sergei Shtylyov @ 2014-01-06 17:37 UTC (permalink / raw)
To: linux-arm-kernel
Hello.
On 06-01-2014 3:06, Hans de Goede wrote:
> 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.
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/phy/phy-core.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index d7b992e..8ee6157 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -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.
> @@ -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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] phy-core: Don't propagate -ENOSUPP from phy_pm_runtime_get_sync to caller
2014-01-05 23:06 ` [PATCH 2/2] phy-core: Don't propagate -ENOSUPP from phy_pm_runtime_get_sync to caller Hans de Goede
2014-01-06 17:37 ` Sergei Shtylyov
@ 2014-01-07 8:56 ` Kishon Vijay Abraham I
2014-01-07 8:58 ` Kishon Vijay Abraham I
2014-01-07 9:13 ` Hans de Goede
1 sibling, 2 replies; 8+ messages in thread
From: Kishon Vijay Abraham I @ 2014-01-07 8:56 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Monday 06 January 2014 04:36 AM, Hans de Goede wrote:
> 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.
Thanks for fixing this. Have one minor comment below.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/phy/phy-core.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index d7b992e..8ee6157 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -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'.
>
> 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 */
same here..
Thanks
Kishon
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] phy-core: Don't propagate -ENOSUPP from phy_pm_runtime_get_sync to caller
2014-01-07 8:56 ` Kishon Vijay Abraham I
@ 2014-01-07 8:58 ` Kishon Vijay Abraham I
2014-01-07 9:13 ` Hans de Goede
1 sibling, 0 replies; 8+ messages in thread
From: Kishon Vijay Abraham I @ 2014-01-07 8:58 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday 07 January 2014 02:26 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Monday 06 January 2014 04:36 AM, Hans de Goede wrote:
>> 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.
>
> Thanks for fixing this. Have one minor comment below.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> drivers/phy/phy-core.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>> index d7b992e..8ee6157 100644
>> --- a/drivers/phy/phy-core.c
>> +++ b/drivers/phy/phy-core.c
>> @@ -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 :-)
>>
>> 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 */
>
> same here..
>
> Thanks
> Kishon
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] phy-core: Don't propagate -ENOSUPP from phy_pm_runtime_get_sync to caller
2014-01-07 8:56 ` Kishon Vijay Abraham I
2014-01-07 8:58 ` Kishon Vijay Abraham I
@ 2014-01-07 9:13 ` Hans de Goede
1 sibling, 0 replies; 8+ messages in thread
From: Hans de Goede @ 2014-01-07 9:13 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On 01/07/2014 09:56 AM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Monday 06 January 2014 04:36 AM, Hans de Goede wrote:
>> 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.
>
> Thanks for fixing this. Have one minor comment below.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> drivers/phy/phy-core.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>> index d7b992e..8ee6157 100644
>> --- a/drivers/phy/phy-core.c
>> +++ b/drivers/phy/phy-core.c
>> @@ -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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] phy-core: Don't print an error on probe deferral or nodata
2014-01-05 23:06 [PATCH 1/2] phy-core: Don't print an error on probe deferral or nodata Hans de Goede
2014-01-05 23:06 ` [PATCH 2/2] phy-core: Don't propagate -ENOSUPP from phy_pm_runtime_get_sync to caller Hans de Goede
@ 2014-01-07 9:31 ` Kishon Vijay Abraham I
2014-01-07 9:53 ` Hans de Goede
1 sibling, 1 reply; 8+ messages in thread
From: Kishon Vijay Abraham I @ 2014-01-07 9:31 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Monday 06 January 2014 04:36 AM, Hans de Goede wrote:
> 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
What do you mean by optional phy here?
Thanks
Kishon
> 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.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/phy/phy-core.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 58e0e97..d7b992e 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -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))
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] phy-core: Don't print an error on probe deferral or nodata
2014-01-07 9:31 ` [PATCH 1/2] phy-core: Don't print an error on probe deferral or nodata Kishon Vijay Abraham I
@ 2014-01-07 9:53 ` Hans de Goede
0 siblings, 0 replies; 8+ messages in thread
From: Hans de Goede @ 2014-01-07 9:53 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On 01/07/2014 10:31 AM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Monday 06 January 2014 04:36 AM, Hans de Goede wrote:
>> 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
>
> What 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:
>> + 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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-01-07 9:53 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-05 23:06 [PATCH 1/2] phy-core: Don't print an error on probe deferral or nodata Hans de Goede
2014-01-05 23:06 ` [PATCH 2/2] phy-core: Don't propagate -ENOSUPP from phy_pm_runtime_get_sync to caller Hans de Goede
2014-01-06 17:37 ` Sergei Shtylyov
2014-01-07 8:56 ` Kishon Vijay Abraham I
2014-01-07 8:58 ` Kishon Vijay Abraham I
2014-01-07 9:13 ` Hans de Goede
2014-01-07 9:31 ` [PATCH 1/2] phy-core: Don't print an error on probe deferral or nodata Kishon Vijay Abraham I
2014-01-07 9:53 ` Hans de Goede
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).