[Patch v4 1/3] drivers: phy: Make NULL a valid phy reference

[Patch v4 1/3] drivers: phy: Make NULL a valid phy reference

[Andrew Lunn]

The common clock framework considers NULL a valid clock
reference. This makes handling optional clocks simple, in that if the
optional clock is not available, a NULL reference can be used in the
place of a real clock, simplifying the clock consumer.

Extend this concept to the phy consumer API. A NULL can be passed to
the release calls, the phy_init() and phy_exit() calls, and
phy_power_on() and phy_power_off() and a NOP is performed.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
v2
 No change.
v4
 combine !phy and IS_ERR().
 Add Tested-by
---
 Documentation/phy.txt  |  6 ++++++
 drivers/phy/phy-core.c | 17 ++++++++++++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/Documentation/phy.txt b/Documentation/phy.txt
index 0103e4b15b0e..2e24b993e95f 100644
--- a/Documentation/phy.txt
+++ b/Documentation/phy.txt
@@ -84,6 +84,12 @@ The only difference between the two APIs is that devm_phy_get associates the
 device with the PHY using devres on successful PHY get. On driver detach,
 release function is invoked on the the devres data and devres data is freed.
 
+It should be noted that NULL is a valid phy reference. All phy
+consumer calls on the NULL phy become NOPs. That is the release calls,
+the phy_init() and phy_exit() calls, and phy_power_on() and
+phy_power_off() calls are all NOP when applied to a NULL phy. The NULL
+phy is useful in devices for handling optional phy devices.
+
 5. Releasing a reference to the PHY
 
 When the controller no longer needs the PHY, it has to release the reference
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 645c867c1257..a9cdeee20d91 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -162,6 +162,9 @@ int phy_init(struct phy *phy)
 {
 	int ret;
 
+	if (!phy)
+		return 0;
+
 	ret = phy_pm_runtime_get_sync(phy);
 	if (ret < 0 && ret != -ENOTSUPP)
 		return ret;
@@ -187,6 +190,9 @@ int phy_exit(struct phy *phy)
 {
 	int ret;
 
+	if (!phy)
+		return 0;
+
 	ret = phy_pm_runtime_get_sync(phy);
 	if (ret < 0 && ret != -ENOTSUPP)
 		return ret;
@@ -212,6 +218,9 @@ int phy_power_on(struct phy *phy)
 {
 	int ret;
 
+	if (!phy)
+		return 0;
+
 	ret = phy_pm_runtime_get_sync(phy);
 	if (ret < 0 && ret != -ENOTSUPP)
 		return ret;
@@ -240,6 +249,9 @@ int phy_power_off(struct phy *phy)
 {
 	int ret;
 
+	if (!phy)
+		return 0;
+
 	mutex_lock(&phy->mutex);
 	if (phy->power_count == 1 && phy->ops->power_off) {
 		ret =  phy->ops->power_off(phy);
@@ -308,7 +320,7 @@ err0:
  */
 void phy_put(struct phy *phy)
 {
-	if (IS_ERR(phy))
+	if (!phy || IS_ERR(phy))
 		return;
 
 	module_put(phy->ops->owner);
@@ -328,6 +340,9 @@ void devm_phy_put(struct device *dev, struct phy *phy)
 {
 	int r;
 
+	if (!phy)
+		return;
+
 	r = devres_destroy(dev, devm_phy_release, devm_phy_match, phy);
 	dev_WARN_ONCE(dev, r, "couldn't find PHY resource\n");
 }
-- 
1.8.5.2

[Patch v4 2/3] drivers: phy: Add support for optional phys

[Andrew Lunn]

Add devm_phy_optional_get and phy_optioanl_get, which should be used
when the phy is optional. They does not return an error when the phy
does not exist, rather they returns NULL, which is considered as a valid
phy, but results in NOPs when used with the consumer API.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
v2
 Add phy_optional_get()
 Improve the comments about the optional functions.
 Drop the dev_err()->dev_dbg() changes.
v4
 Add Tested-by
---
 Documentation/phy.txt   | 20 +++++++++++++-------
 drivers/phy/phy-core.c  | 45 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/phy/phy.h | 14 ++++++++++++++
 3 files changed, 72 insertions(+), 7 deletions(-)

diff --git a/Documentation/phy.txt b/Documentation/phy.txt
index 2e24b993e95f..ebff6ee52441 100644
--- a/Documentation/phy.txt
+++ b/Documentation/phy.txt
@@ -75,14 +75,20 @@ Before the controller can make use of the PHY, it has to get a reference to
 it. This framework provides the following APIs to get a reference to the PHY.
 
 struct phy *phy_get(struct device *dev, const char *string);
+struct phy *phy_optional_get(struct device *dev, const char *string);
 struct phy *devm_phy_get(struct device *dev, const char *string);
-
-phy_get and devm_phy_get can be used to get the PHY. In the case of dt boot,
-the string arguments should contain the phy name as given in the dt data and
-in the case of non-dt boot, it should contain the label of the PHY.
-The only difference between the two APIs is that devm_phy_get associates the
-device with the PHY using devres on successful PHY get. On driver detach,
-release function is invoked on the the devres data and devres data is freed.
+struct phy *devm_phy_optional_get(struct device *dev, const char *string);
+
+phy_get, phy_optional_get, devm_phy_get and devm_phy_optional_get can
+be used to get the PHY. In the case of dt boot, the string arguments
+should contain the phy name as given in the dt data and in the case of
+non-dt boot, it should contain the label of the PHY.  The two
+devm_phy_get associates the device with the PHY using devres on
+successful PHY get. On driver detach, release function is invoked on
+the the devres data and devres data is freed. phy_optional_get and
+devm_phy_optional_get should be used when the phy is optional. These
+two functions will never return -ENODEV, but instead returns NULL when
+the phy cannot be found.
 
 It should be noted that NULL is a valid phy reference. All phy
 consumer calls on the NULL phy become NOPs. That is the release calls,
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index a9cdeee20d91..5f5b0f4be5be 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -426,6 +426,27 @@ struct phy *phy_get(struct device *dev, const char *string)
 EXPORT_SYMBOL_GPL(phy_get);
 
 /**
+ * phy_optional_get() - lookup and obtain a reference to an optional phy.
+ * @dev: device that requests this phy
+ * @string: the phy name as given in the dt data or the name of the controller
+ * port for non-dt case
+ *
+ * Returns the phy driver, after getting a refcount to it; or
+ * NULL if there is no such phy.  The caller is responsible for
+ * calling phy_put() to release that count.
+ */
+struct phy *phy_optional_get(struct device *dev, const char *string)
+{
+	struct phy *phy = phy_get(dev, string);
+
+	if (PTR_ERR(phy) == -ENODEV)
+		phy = NULL;
+
+	return phy;
+}
+EXPORT_SYMBOL_GPL(phy_optional_get);
+
+/**
  * devm_phy_get() - lookup and obtain a reference to a phy.
  * @dev: device that requests this phy
  * @string: the phy name as given in the dt data or phy device name
@@ -456,6 +477,30 @@ struct phy *devm_phy_get(struct device *dev, const char *string)
 EXPORT_SYMBOL_GPL(devm_phy_get);
 
 /**
+ * devm_phy_optional_get() - lookup and obtain a reference to an optional phy.
+ * @dev: device that requests this phy
+ * @string: the phy name as given in the dt data or phy device name
+ * for non-dt case
+ *
+ * Gets the phy using phy_get(), and associates a device with it using
+ * devres. On driver detach, release function is invoked on the devres
+ * data, then, devres data is freed. This differs to devm_phy_get() in
+ * that if the phy does not exist, it is not considered an error and
+ * -ENODEV will not be returned. Instead the NULL phy is returned,
+ * which can be passed to all other phy consumer calls.
+ */
+struct phy *devm_phy_optional_get(struct device *dev, const char *string)
+{
+	struct phy *phy = devm_phy_get(dev, string);
+
+	if (PTR_ERR(phy) == -ENODEV)
+		phy = NULL;
+
+	return phy;
+}
+EXPORT_SYMBOL_GPL(devm_phy_optional_get);
+
+/**
  * phy_create() - create a new phy
  * @dev: device that is creating the new phy
  * @ops: function pointers for performing phy operations
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index e273e5ac19c9..3f83459dbb20 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -146,7 +146,9 @@ static inline void phy_set_bus_width(struct phy *phy, int bus_width)
 	phy->attrs.bus_width = bus_width;
 }
 struct phy *phy_get(struct device *dev, const char *string);
+struct phy *phy_optional_get(struct device *dev, const char *string);
 struct phy *devm_phy_get(struct device *dev, const char *string);
+struct phy *devm_phy_optional_get(struct device *dev, const char *string);
 void phy_put(struct phy *phy);
 void devm_phy_put(struct device *dev, struct phy *phy);
 struct phy *of_phy_simple_xlate(struct device *dev,
@@ -232,11 +234,23 @@ static inline struct phy *phy_get(struct device *dev, const char *string)
 	return ERR_PTR(-ENOSYS);
 }
 
+static inline struct phy *phy_optional_get(struct device *dev,
+					   const char *string)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
 static inline struct phy *devm_phy_get(struct device *dev, const char *string)
 {
 	return ERR_PTR(-ENOSYS);
 }
 
+static inline struct phy *devm_phy_optional_get(struct device *dev,
+						const char *string)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
 static inline void phy_put(struct phy *phy)
 {
 }
-- 
1.8.5.2

[Patch v4 3/3] ata: sata_mv: Fix probe failures with optional phys

[Andrew Lunn]

Make use of devm_phy_optional_get() in order to fix probe failures on
Armada 370, XP and others, when there is no phy driver available.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Acked-by: Tejun Heo <tj@kernel.org>
---
v2
 No change
v3
 Fix commit log devm_get_optional->devm_option_get.
v4
 Add Tested-by: & Acked-by:
 Fix comment again.
---
 drivers/ata/sata_mv.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 20a7517bd339..52b8181ddafd 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -4126,12 +4126,14 @@ static int mv_platform_probe(struct platform_device *pdev)
 			clk_prepare_enable(hpriv->port_clks[port]);
 
 		sprintf(port_number, "port%d", port);
-		hpriv->port_phys[port] = devm_phy_get(&pdev->dev, port_number);
+		hpriv->port_phys[port] = devm_phy_optional_get(&pdev->dev,
+							       port_number);
 		if (IS_ERR(hpriv->port_phys[port])) {
 			rc = PTR_ERR(hpriv->port_phys[port]);
 			hpriv->port_phys[port] = NULL;
-			if ((rc != -EPROBE_DEFER) && (rc != -ENODEV))
-				dev_warn(&pdev->dev, "error getting phy");
+			if (rc != -EPROBE_DEFER)
+				dev_warn(&pdev->dev, "error getting phy %d",
+					rc);
 			goto err;
 		} else
 			phy_power_on(hpriv->port_phys[port]);
-- 
1.8.5.2

Re: [Patch v4 1/3] drivers: phy: Make NULL a valid phy reference

[Jason Cooper]

Kishon,

On Tue, Feb 04, 2014 at 06:33:11PM +0100, Andrew Lunn wrote:
> The common clock framework considers NULL a valid clock
> reference. This makes handling optional clocks simple, in that if the
> optional clock is not available, a NULL reference can be used in the
> place of a real clock, simplifying the clock consumer.
> 
> Extend this concept to the phy consumer API. A NULL can be passed to
> the release calls, the phy_init() and phy_exit() calls, and
> phy_power_on() and phy_power_off() and a NOP is performed.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> v2
>  No change.
> v4
>  combine !phy and IS_ERR().
>  Add Tested-by
> ---
>  Documentation/phy.txt  |  6 ++++++
>  drivers/phy/phy-core.c | 17 ++++++++++++++++-
>  2 files changed, 22 insertions(+), 1 deletion(-)

Looking over the overall diffstat (there isn't one, Andrew, please don't
forget the coverletter in the future), this series is predominately
changing the phy subsystem.  However, since it's fixing a boot failure
in the arm-soc bootfarm, we'd like to get it into our tree without
pulling in a bunch of unnecessary changes (eg by pulling in -rc2 or
-rc3).

Could you create a topic branch with this series in it we could base off
of?  Or, could you Ack it so we could take it through arm-soc?

thx,

Jason.

> diff --git a/Documentation/phy.txt b/Documentation/phy.txt
> index 0103e4b15b0e..2e24b993e95f 100644
> --- a/Documentation/phy.txt
> +++ b/Documentation/phy.txt
> @@ -84,6 +84,12 @@ The only difference between the two APIs is that devm_phy_get associates the
>  device with the PHY using devres on successful PHY get. On driver detach,
>  release function is invoked on the the devres data and devres data is freed.
>  
> +It should be noted that NULL is a valid phy reference. All phy
> +consumer calls on the NULL phy become NOPs. That is the release calls,
> +the phy_init() and phy_exit() calls, and phy_power_on() and
> +phy_power_off() calls are all NOP when applied to a NULL phy. The NULL
> +phy is useful in devices for handling optional phy devices.
> +
>  5. Releasing a reference to the PHY
>  
>  When the controller no longer needs the PHY, it has to release the reference
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 645c867c1257..a9cdeee20d91 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -162,6 +162,9 @@ int phy_init(struct phy *phy)
>  {
>  	int ret;
>  
> +	if (!phy)
> +		return 0;
> +
>  	ret = phy_pm_runtime_get_sync(phy);
>  	if (ret < 0 && ret != -ENOTSUPP)
>  		return ret;
> @@ -187,6 +190,9 @@ int phy_exit(struct phy *phy)
>  {
>  	int ret;
>  
> +	if (!phy)
> +		return 0;
> +
>  	ret = phy_pm_runtime_get_sync(phy);
>  	if (ret < 0 && ret != -ENOTSUPP)
>  		return ret;
> @@ -212,6 +218,9 @@ int phy_power_on(struct phy *phy)
>  {
>  	int ret;
>  
> +	if (!phy)
> +		return 0;
> +
>  	ret = phy_pm_runtime_get_sync(phy);
>  	if (ret < 0 && ret != -ENOTSUPP)
>  		return ret;
> @@ -240,6 +249,9 @@ int phy_power_off(struct phy *phy)
>  {
>  	int ret;
>  
> +	if (!phy)
> +		return 0;
> +
>  	mutex_lock(&phy->mutex);
>  	if (phy->power_count == 1 && phy->ops->power_off) {
>  		ret =  phy->ops->power_off(phy);
> @@ -308,7 +320,7 @@ err0:
>   */
>  void phy_put(struct phy *phy)
>  {
> -	if (IS_ERR(phy))
> +	if (!phy || IS_ERR(phy))
>  		return;
>  
>  	module_put(phy->ops->owner);
> @@ -328,6 +340,9 @@ void devm_phy_put(struct device *dev, struct phy *phy)
>  {
>  	int r;
>  
> +	if (!phy)
> +		return;
> +
>  	r = devres_destroy(dev, devm_phy_release, devm_phy_match, phy);
>  	dev_WARN_ONCE(dev, r, "couldn't find PHY resource\n");
>  }
> -- 
> 1.8.5.2
> 

Re: [Patch v4 1/3] drivers: phy: Make NULL a valid phy reference

[Kishon Vijay Abraham I]

On Wednesday 05 February 2014 10:44 AM, Jason Cooper wrote:
> Kishon,
> 
> On Tue, Feb 04, 2014 at 06:33:11PM +0100, Andrew Lunn wrote:
>> The common clock framework considers NULL a valid clock
>> reference. This makes handling optional clocks simple, in that if the
>> optional clock is not available, a NULL reference can be used in the
>> place of a real clock, simplifying the clock consumer.
>>
>> Extend this concept to the phy consumer API. A NULL can be passed to
>> the release calls, the phy_init() and phy_exit() calls, and
>> phy_power_on() and phy_power_off() and a NOP is performed.
>>
>> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
>> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> ---
>> v2
>>  No change.
>> v4
>>  combine !phy and IS_ERR().
>>  Add Tested-by
>> ---
>>  Documentation/phy.txt  |  6 ++++++
>>  drivers/phy/phy-core.c | 17 ++++++++++++++++-
>>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> Looking over the overall diffstat (there isn't one, Andrew, please don't
> forget the coverletter in the future), this series is predominately
> changing the phy subsystem.  However, since it's fixing a boot failure
> in the arm-soc bootfarm, we'd like to get it into our tree without
> pulling in a bunch of unnecessary changes (eg by pulling in -rc2 or
> -rc3).
> 
> Could you create a topic branch with this series in it we could base off
> of?  Or, could you Ack it so we could take it through arm-soc?

I can just ACK it so you can take it through your tree.

Cheers
Kishon

> 
> thx,
> 
> Jason.
> 
>> diff --git a/Documentation/phy.txt b/Documentation/phy.txt
>> index 0103e4b15b0e..2e24b993e95f 100644
>> --- a/Documentation/phy.txt
>> +++ b/Documentation/phy.txt
>> @@ -84,6 +84,12 @@ The only difference between the two APIs is that devm_phy_get associates the
>>  device with the PHY using devres on successful PHY get. On driver detach,
>>  release function is invoked on the the devres data and devres data is freed.
>>  
>> +It should be noted that NULL is a valid phy reference. All phy
>> +consumer calls on the NULL phy become NOPs. That is the release calls,
>> +the phy_init() and phy_exit() calls, and phy_power_on() and
>> +phy_power_off() calls are all NOP when applied to a NULL phy. The NULL
>> +phy is useful in devices for handling optional phy devices.
>> +
>>  5. Releasing a reference to the PHY
>>  
>>  When the controller no longer needs the PHY, it has to release the reference
>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>> index 645c867c1257..a9cdeee20d91 100644
>> --- a/drivers/phy/phy-core.c
>> +++ b/drivers/phy/phy-core.c
>> @@ -162,6 +162,9 @@ int phy_init(struct phy *phy)
>>  {
>>  	int ret;
>>  
>> +	if (!phy)
>> +		return 0;
>> +
>>  	ret = phy_pm_runtime_get_sync(phy);
>>  	if (ret < 0 && ret != -ENOTSUPP)
>>  		return ret;
>> @@ -187,6 +190,9 @@ int phy_exit(struct phy *phy)
>>  {
>>  	int ret;
>>  
>> +	if (!phy)
>> +		return 0;
>> +
>>  	ret = phy_pm_runtime_get_sync(phy);
>>  	if (ret < 0 && ret != -ENOTSUPP)
>>  		return ret;
>> @@ -212,6 +218,9 @@ int phy_power_on(struct phy *phy)
>>  {
>>  	int ret;
>>  
>> +	if (!phy)
>> +		return 0;
>> +
>>  	ret = phy_pm_runtime_get_sync(phy);
>>  	if (ret < 0 && ret != -ENOTSUPP)
>>  		return ret;
>> @@ -240,6 +249,9 @@ int phy_power_off(struct phy *phy)
>>  {
>>  	int ret;
>>  
>> +	if (!phy)
>> +		return 0;
>> +
>>  	mutex_lock(&phy->mutex);
>>  	if (phy->power_count == 1 && phy->ops->power_off) {
>>  		ret =  phy->ops->power_off(phy);
>> @@ -308,7 +320,7 @@ err0:
>>   */
>>  void phy_put(struct phy *phy)
>>  {
>> -	if (IS_ERR(phy))
>> +	if (!phy || IS_ERR(phy))
>>  		return;
>>  
>>  	module_put(phy->ops->owner);
>> @@ -328,6 +340,9 @@ void devm_phy_put(struct device *dev, struct phy *phy)
>>  {
>>  	int r;
>>  
>> +	if (!phy)
>> +		return;
>> +
>>  	r = devres_destroy(dev, devm_phy_release, devm_phy_match, phy);
>>  	dev_WARN_ONCE(dev, r, "couldn't find PHY resource\n");
>>  }
>> -- 
>> 1.8.5.2
>>

Re: [Patch v4 1/3] drivers: phy: Make NULL a valid phy reference

[Kishon Vijay Abraham I]

On Tuesday 04 February 2014 11:03 PM, Andrew Lunn wrote:
> The common clock framework considers NULL a valid clock
> reference. This makes handling optional clocks simple, in that if the
> optional clock is not available, a NULL reference can be used in the
> place of a real clock, simplifying the clock consumer.
> 
> Extend this concept to the phy consumer API. A NULL can be passed to
> the release calls, the phy_init() and phy_exit() calls, and
> phy_power_on() and phy_power_off() and a NOP is performed.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> v2
>  No change.
> v4
>  combine !phy and IS_ERR().
>  Add Tested-by
> ---
>  Documentation/phy.txt  |  6 ++++++
>  drivers/phy/phy-core.c | 17 ++++++++++++++++-
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/phy.txt b/Documentation/phy.txt
> index 0103e4b15b0e..2e24b993e95f 100644
> --- a/Documentation/phy.txt
> +++ b/Documentation/phy.txt
> @@ -84,6 +84,12 @@ The only difference between the two APIs is that devm_phy_get associates the
>  device with the PHY using devres on successful PHY get. On driver detach,
>  release function is invoked on the the devres data and devres data is freed.
>  
> +It should be noted that NULL is a valid phy reference. All phy
> +consumer calls on the NULL phy become NOPs. That is the release calls,
> +the phy_init() and phy_exit() calls, and phy_power_on() and
> +phy_power_off() calls are all NOP when applied to a NULL phy. The NULL
> +phy is useful in devices for handling optional phy devices.
> +
>  5. Releasing a reference to the PHY
>  
>  When the controller no longer needs the PHY, it has to release the reference
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 645c867c1257..a9cdeee20d91 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -162,6 +162,9 @@ int phy_init(struct phy *phy)
>  {
>  	int ret;
>  
> +	if (!phy)
> +		return 0;
> +
>  	ret = phy_pm_runtime_get_sync(phy);
>  	if (ret < 0 && ret != -ENOTSUPP)
>  		return ret;
> @@ -187,6 +190,9 @@ int phy_exit(struct phy *phy)
>  {
>  	int ret;
>  
> +	if (!phy)
> +		return 0;
> +
>  	ret = phy_pm_runtime_get_sync(phy);
>  	if (ret < 0 && ret != -ENOTSUPP)
>  		return ret;
> @@ -212,6 +218,9 @@ int phy_power_on(struct phy *phy)
>  {
>  	int ret;
>  
> +	if (!phy)
> +		return 0;
> +
>  	ret = phy_pm_runtime_get_sync(phy);
>  	if (ret < 0 && ret != -ENOTSUPP)
>  		return ret;
> @@ -240,6 +249,9 @@ int phy_power_off(struct phy *phy)
>  {
>  	int ret;
>  
> +	if (!phy)
> +		return 0;
> +
>  	mutex_lock(&phy->mutex);
>  	if (phy->power_count == 1 && phy->ops->power_off) {
>  		ret =  phy->ops->power_off(phy);
> @@ -308,7 +320,7 @@ err0:
>   */
>  void phy_put(struct phy *phy)
>  {
> -	if (IS_ERR(phy))
> +	if (!phy || IS_ERR(phy))
>  		return;
>  
>  	module_put(phy->ops->owner);
> @@ -328,6 +340,9 @@ void devm_phy_put(struct device *dev, struct phy *phy)
>  {
>  	int r;
>  
> +	if (!phy)
> +		return;
> +
>  	r = devres_destroy(dev, devm_phy_release, devm_phy_match, phy);
>  	dev_WARN_ONCE(dev, r, "couldn't find PHY resource\n");
>  }
> 

Re: [Patch v4 2/3] drivers: phy: Add support for optional phys

[Kishon Vijay Abraham I]

On Tuesday 04 February 2014 11:03 PM, Andrew Lunn wrote:
> Add devm_phy_optional_get and phy_optioanl_get, which should be used
> when the phy is optional. They does not return an error when the phy
> does not exist, rather they returns NULL, which is considered as a valid
> phy, but results in NOPs when used with the consumer API.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> v2
>  Add phy_optional_get()
>  Improve the comments about the optional functions.
>  Drop the dev_err()->dev_dbg() changes.
> v4
>  Add Tested-by
> ---
>  Documentation/phy.txt   | 20 +++++++++++++-------
>  drivers/phy/phy-core.c  | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/phy/phy.h | 14 ++++++++++++++
>  3 files changed, 72 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/phy.txt b/Documentation/phy.txt
> index 2e24b993e95f..ebff6ee52441 100644
> --- a/Documentation/phy.txt
> +++ b/Documentation/phy.txt
> @@ -75,14 +75,20 @@ Before the controller can make use of the PHY, it has to get a reference to
>  it. This framework provides the following APIs to get a reference to the PHY.
>  
>  struct phy *phy_get(struct device *dev, const char *string);
> +struct phy *phy_optional_get(struct device *dev, const char *string);
>  struct phy *devm_phy_get(struct device *dev, const char *string);
> -
> -phy_get and devm_phy_get can be used to get the PHY. In the case of dt boot,
> -the string arguments should contain the phy name as given in the dt data and
> -in the case of non-dt boot, it should contain the label of the PHY.
> -The only difference between the two APIs is that devm_phy_get associates the
> -device with the PHY using devres on successful PHY get. On driver detach,
> -release function is invoked on the the devres data and devres data is freed.
> +struct phy *devm_phy_optional_get(struct device *dev, const char *string);
> +
> +phy_get, phy_optional_get, devm_phy_get and devm_phy_optional_get can
> +be used to get the PHY. In the case of dt boot, the string arguments
> +should contain the phy name as given in the dt data and in the case of
> +non-dt boot, it should contain the label of the PHY.  The two
> +devm_phy_get associates the device with the PHY using devres on
> +successful PHY get. On driver detach, release function is invoked on
> +the the devres data and devres data is freed. phy_optional_get and
> +devm_phy_optional_get should be used when the phy is optional. These
> +two functions will never return -ENODEV, but instead returns NULL when
> +the phy cannot be found.
>  
>  It should be noted that NULL is a valid phy reference. All phy
>  consumer calls on the NULL phy become NOPs. That is the release calls,
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index a9cdeee20d91..5f5b0f4be5be 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -426,6 +426,27 @@ struct phy *phy_get(struct device *dev, const char *string)
>  EXPORT_SYMBOL_GPL(phy_get);
>  
>  /**
> + * phy_optional_get() - lookup and obtain a reference to an optional phy.
> + * @dev: device that requests this phy
> + * @string: the phy name as given in the dt data or the name of the controller
> + * port for non-dt case
> + *
> + * Returns the phy driver, after getting a refcount to it; or
> + * NULL if there is no such phy.  The caller is responsible for
> + * calling phy_put() to release that count.
> + */
> +struct phy *phy_optional_get(struct device *dev, const char *string)
> +{
> +	struct phy *phy = phy_get(dev, string);
> +
> +	if (PTR_ERR(phy) == -ENODEV)
> +		phy = NULL;
> +
> +	return phy;
> +}
> +EXPORT_SYMBOL_GPL(phy_optional_get);
> +
> +/**
>   * devm_phy_get() - lookup and obtain a reference to a phy.
>   * @dev: device that requests this phy
>   * @string: the phy name as given in the dt data or phy device name
> @@ -456,6 +477,30 @@ struct phy *devm_phy_get(struct device *dev, const char *string)
>  EXPORT_SYMBOL_GPL(devm_phy_get);
>  
>  /**
> + * devm_phy_optional_get() - lookup and obtain a reference to an optional phy.
> + * @dev: device that requests this phy
> + * @string: the phy name as given in the dt data or phy device name
> + * for non-dt case
> + *
> + * Gets the phy using phy_get(), and associates a device with it using
> + * devres. On driver detach, release function is invoked on the devres
> + * data, then, devres data is freed. This differs to devm_phy_get() in
> + * that if the phy does not exist, it is not considered an error and
> + * -ENODEV will not be returned. Instead the NULL phy is returned,
> + * which can be passed to all other phy consumer calls.
> + */
> +struct phy *devm_phy_optional_get(struct device *dev, const char *string)
> +{
> +	struct phy *phy = devm_phy_get(dev, string);
> +
> +	if (PTR_ERR(phy) == -ENODEV)
> +		phy = NULL;
> +
> +	return phy;
> +}
> +EXPORT_SYMBOL_GPL(devm_phy_optional_get);
> +
> +/**
>   * phy_create() - create a new phy
>   * @dev: device that is creating the new phy
>   * @ops: function pointers for performing phy operations
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index e273e5ac19c9..3f83459dbb20 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -146,7 +146,9 @@ static inline void phy_set_bus_width(struct phy *phy, int bus_width)
>  	phy->attrs.bus_width = bus_width;
>  }
>  struct phy *phy_get(struct device *dev, const char *string);
> +struct phy *phy_optional_get(struct device *dev, const char *string);
>  struct phy *devm_phy_get(struct device *dev, const char *string);
> +struct phy *devm_phy_optional_get(struct device *dev, const char *string);
>  void phy_put(struct phy *phy);
>  void devm_phy_put(struct device *dev, struct phy *phy);
>  struct phy *of_phy_simple_xlate(struct device *dev,
> @@ -232,11 +234,23 @@ static inline struct phy *phy_get(struct device *dev, const char *string)
>  	return ERR_PTR(-ENOSYS);
>  }
>  
> +static inline struct phy *phy_optional_get(struct device *dev,
> +					   const char *string)
> +{
> +	return ERR_PTR(-ENOSYS);
> +}
> +
>  static inline struct phy *devm_phy_get(struct device *dev, const char *string)
>  {
>  	return ERR_PTR(-ENOSYS);
>  }
>  
> +static inline struct phy *devm_phy_optional_get(struct device *dev,
> +						const char *string)
> +{
> +	return ERR_PTR(-ENOSYS);
> +}
> +
>  static inline void phy_put(struct phy *phy)
>  {
>  }
> 

Re: [Patch v4 1/3] drivers: phy: Make NULL a valid phy reference

[Jason Cooper]

On Wed, Feb 05, 2014 at 10:49:55AM +0530, Kishon Vijay Abraham I wrote:
> On Wednesday 05 February 2014 10:44 AM, Jason Cooper wrote:
> > Kishon,
> > 
> > On Tue, Feb 04, 2014 at 06:33:11PM +0100, Andrew Lunn wrote:
> >> The common clock framework considers NULL a valid clock
> >> reference. This makes handling optional clocks simple, in that if the
> >> optional clock is not available, a NULL reference can be used in the
> >> place of a real clock, simplifying the clock consumer.
> >>
> >> Extend this concept to the phy consumer API. A NULL can be passed to
> >> the release calls, the phy_init() and phy_exit() calls, and
> >> phy_power_on() and phy_power_off() and a NOP is performed.
> >>
> >> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> >> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> >> ---
> >> v2
> >>  No change.
> >> v4
> >>  combine !phy and IS_ERR().
> >>  Add Tested-by
> >> ---
> >>  Documentation/phy.txt  |  6 ++++++
> >>  drivers/phy/phy-core.c | 17 ++++++++++++++++-
> >>  2 files changed, 22 insertions(+), 1 deletion(-)
> > 
> > Looking over the overall diffstat (there isn't one, Andrew, please don't
> > forget the coverletter in the future), this series is predominately
> > changing the phy subsystem.  However, since it's fixing a boot failure
> > in the arm-soc bootfarm, we'd like to get it into our tree without
> > pulling in a bunch of unnecessary changes (eg by pulling in -rc2 or
> > -rc3).
> > 
> > Could you create a topic branch with this series in it we could base off
> > of?  Or, could you Ack it so we could take it through arm-soc?
> 
> I can just ACK it so you can take it through your tree.

Thanks!

Jason.

Re: [Patch v4 3/3] ata: sata_mv: Fix probe failures with optional phys

[Kishon Vijay Abraham I]

On Tuesday 04 February 2014 11:03 PM, Andrew Lunn wrote:
> Make use of devm_phy_optional_get() in order to fix probe failures on
> Armada 370, XP and others, when there is no phy driver available.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Acked-by: Tejun Heo <tj@kernel.org>
Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> v2
>  No change
> v3
>  Fix commit log devm_get_optional->devm_option_get.
> v4
>  Add Tested-by: & Acked-by:
>  Fix comment again.
> ---
>  drivers/ata/sata_mv.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
> index 20a7517bd339..52b8181ddafd 100644
> --- a/drivers/ata/sata_mv.c
> +++ b/drivers/ata/sata_mv.c
> @@ -4126,12 +4126,14 @@ static int mv_platform_probe(struct platform_device *pdev)
>  			clk_prepare_enable(hpriv->port_clks[port]);
>  
>  		sprintf(port_number, "port%d", port);
> -		hpriv->port_phys[port] = devm_phy_get(&pdev->dev, port_number);
> +		hpriv->port_phys[port] = devm_phy_optional_get(&pdev->dev,
> +							       port_number);
>  		if (IS_ERR(hpriv->port_phys[port])) {
>  			rc = PTR_ERR(hpriv->port_phys[port]);
>  			hpriv->port_phys[port] = NULL;
> -			if ((rc != -EPROBE_DEFER) && (rc != -ENODEV))
> -				dev_warn(&pdev->dev, "error getting phy");
> +			if (rc != -EPROBE_DEFER)
> +				dev_warn(&pdev->dev, "error getting phy %d",
> +					rc);
>  			goto err;
>  		} else
>  			phy_power_on(hpriv->port_phys[port]);
> 

Re: [Patch v4 1/3] drivers: phy: Make NULL a valid phy reference

[Jason Cooper]

On Tue, Feb 04, 2014 at 06:33:11PM +0100, Andrew Lunn wrote:
> The common clock framework considers NULL a valid clock
> reference. This makes handling optional clocks simple, in that if the
> optional clock is not available, a NULL reference can be used in the
> place of a real clock, simplifying the clock consumer.
> 
> Extend this concept to the phy consumer API. A NULL can be passed to
> the release calls, the phy_init() and phy_exit() calls, and
> phy_power_on() and phy_power_off() and a NOP is performed.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> v2
>  No change.
> v4
>  combine !phy and IS_ERR().
>  Add Tested-by
> ---
>  Documentation/phy.txt  |  6 ++++++
>  drivers/phy/phy-core.c | 17 ++++++++++++++++-
>  2 files changed, 22 insertions(+), 1 deletion(-)

Whole series applied to mvebu/phy_ata-fixes with Kishon's Acks.

thx,

Jason.