diff mbox

[RESEND,v2,1/2] phy: core: Add an exported of_phy_get function

Message ID 1386943380-28596-1-git-send-email-k.debski@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kamil Debski Dec. 13, 2013, 2:02 p.m. UTC
Previously the of_phy_get function took a struct device * and
was declared static. It was impossible to call it from
another driver and thus it was impossible to get phy defined
for a given node. The old function was renamed to _of_phy_get
and was left for internal use. of_phy_get function was added
and it was exported. The function enables to get a phy for
a given device tree node.

Signed-off-by: Kamil Debski <k.debski@samsung.com>
---
It seems that my git send-email is playing up and sent the previous emails
without from. This is a resend. Sorry for any confusion.
---
 drivers/phy/phy-core.c  |   41 ++++++++++++++++++++++++++++++++---------
 include/linux/phy/phy.h |    1 +
 2 files changed, 33 insertions(+), 9 deletions(-)

Comments

Kishon Vijay Abraham I Dec. 13, 2013, 2:45 p.m. UTC | #1
Hi,

On Friday 13 December 2013 07:32 PM, Kamil Debski wrote:
> Previously the of_phy_get function took a struct device * and
> was declared static. It was impossible to call it from
> another driver and thus it was impossible to get phy defined
> for a given node. The old function was renamed to _of_phy_get
> and was left for internal use. of_phy_get function was added
> and it was exported. The function enables to get a phy for
> a given device tree node.
> 
> Signed-off-by: Kamil Debski <k.debski@samsung.com>
> ---
> It seems that my git send-email is playing up and sent the previous emails
> without from. This is a resend. Sorry for any confusion.
> ---
>  drivers/phy/phy-core.c  |   41 ++++++++++++++++++++++++++++++++---------
>  include/linux/phy/phy.h |    1 +
>  2 files changed, 33 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 03cf8fb..f0dbd42 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -240,8 +240,8 @@ out:
>  EXPORT_SYMBOL_GPL(phy_power_off);
>  
>  /**
> - * of_phy_get() - lookup and obtain a reference to a phy by phandle
> - * @dev: device that requests this phy
> + * _of_phy_get() - lookup and obtain a reference to a phy by phandle
> + * @np: device_node for which to get the phy
>   * @index: the index of the phy
>   *
>   * Returns the phy associated with the given phandle value,
> @@ -250,20 +250,17 @@ EXPORT_SYMBOL_GPL(phy_power_off);
>   * not yet loaded. This function uses of_xlate call back function provided
>   * while registering the phy_provider to find the phy instance.
>   */
> -static struct phy *of_phy_get(struct device *dev, int index)
> +static struct phy *_of_phy_get(struct device_node *np, int index)
>  {
>  	int ret;
>  	struct phy_provider *phy_provider;
>  	struct phy *phy = NULL;
>  	struct of_phandle_args args;
>  
> -	ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-cells",
> +	ret = of_parse_phandle_with_args(np, "phys", "#phy-cells",
>  		index, &args);
> -	if (ret) {
> -		dev_dbg(dev, "failed to get phy in %s node\n",
> -			dev->of_node->full_name);
> +	if (ret)
>  		return ERR_PTR(-ENODEV);
> -	}
>  
>  	mutex_lock(&phy_provider_mutex);
>  	phy_provider = of_phy_provider_lookup(args.np);
> @@ -283,6 +280,32 @@ err0:
>  }
>  
>  /**
> + * of_phy_get() - lookup and obtain a reference to a phy using a device_node.
> + * @np: device_node for which to get the phy
> + * @index: the index of the phy

Would be better if the user can get the PHY by string instead of index.
> + *
> + * Returns the phy driver, after getting a refcount to it; or
> + * -ENODEV if there is no such phy.  The caller is responsible for
> + * calling phy_put() to release that count.
> + */
> +struct phy *of_phy_get(struct device_node *np, int index)
> +{
> +	struct phy *phy = NULL;
> +
> +	phy = _of_phy_get(np, index);
> +	if (IS_ERR(phy))
dev_err here?

Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kamil Debski Dec. 13, 2013, 3:26 p.m. UTC | #2
Hi Kishon,

> From: Kishon Vijay Abraham I [mailto:kishon@ti.com]
> Sent: Friday, December 13, 2013 3:45 PM
> 
> Hi,
> 
> On Friday 13 December 2013 07:32 PM, Kamil Debski wrote:
> > Previously the of_phy_get function took a struct device * and was
> > declared static. It was impossible to call it from another driver and
> > thus it was impossible to get phy defined for a given node. The old
> > function was renamed to _of_phy_get and was left for internal use.
> > of_phy_get function was added and it was exported. The function
> > enables to get a phy for a given device tree node.
> >
> > Signed-off-by: Kamil Debski <k.debski@samsung.com>
> > ---
> > It seems that my git send-email is playing up and sent the previous
> > emails without from. This is a resend. Sorry for any confusion.
> > ---
> >  drivers/phy/phy-core.c  |   41 ++++++++++++++++++++++++++++++++-----
> ----
> >  include/linux/phy/phy.h |    1 +
> >  2 files changed, 33 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c index
> > 03cf8fb..f0dbd42 100644
> > --- a/drivers/phy/phy-core.c
> > +++ b/drivers/phy/phy-core.c
> > @@ -240,8 +240,8 @@ out:
> >  EXPORT_SYMBOL_GPL(phy_power_off);
> >
> >  /**
> > - * of_phy_get() - lookup and obtain a reference to a phy by phandle
> > - * @dev: device that requests this phy
> > + * _of_phy_get() - lookup and obtain a reference to a phy by phandle
> > + * @np: device_node for which to get the phy
> >   * @index: the index of the phy
> >   *
> >   * Returns the phy associated with the given phandle value, @@
> > -250,20 +250,17 @@ EXPORT_SYMBOL_GPL(phy_power_off);
> >   * not yet loaded. This function uses of_xlate call back function
> provided
> >   * while registering the phy_provider to find the phy instance.
> >   */
> > -static struct phy *of_phy_get(struct device *dev, int index)
> > +static struct phy *_of_phy_get(struct device_node *np, int index)
> >  {
> >  	int ret;
> >  	struct phy_provider *phy_provider;
> >  	struct phy *phy = NULL;
> >  	struct of_phandle_args args;
> >
> > -	ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-
> cells",
> > +	ret = of_parse_phandle_with_args(np, "phys", "#phy-cells",
> >  		index, &args);
> > -	if (ret) {
> > -		dev_dbg(dev, "failed to get phy in %s node\n",
> > -			dev->of_node->full_name);
> > +	if (ret)
> >  		return ERR_PTR(-ENODEV);
> > -	}
> >
> >  	mutex_lock(&phy_provider_mutex);
> >  	phy_provider = of_phy_provider_lookup(args.np); @@ -283,6 +280,32
> @@
> > err0:
> >  }
> >
> >  /**
> > + * of_phy_get() - lookup and obtain a reference to a phy using a
> device_node.
> > + * @np: device_node for which to get the phy
> > + * @index: the index of the phy
> 
> Would be better if the user can get the PHY by string instead of index.

Ok, this sounds doable. I will add of_phy_get_by_name analogous to the clk
framework.

> > + *
> > + * Returns the phy driver, after getting a refcount to it; or
> > + * -ENODEV if there is no such phy.  The caller is responsible for
> > + * calling phy_put() to release that count.
> > + */
> > +struct phy *of_phy_get(struct device_node *np, int index) {
> > +	struct phy *phy = NULL;
> > +
> > +	phy = _of_phy_get(np, index);
> > +	if (IS_ERR(phy))
> dev_err here?

dev_err requires a dev, so it is not possible with the arguments used.
And we do not want to add any extra parameters to keep the function call
analogous to other of_*_get (as for example of_clk_get).

Best wishes,
Kishon Vijay Abraham I Dec. 16, 2013, 9 a.m. UTC | #3
On Friday 13 December 2013 08:56 PM, Kamil Debski wrote:
> Hi Kishon,
> 
>> From: Kishon Vijay Abraham I [mailto:kishon@ti.com]
>> Sent: Friday, December 13, 2013 3:45 PM
>>
>> Hi,
>>
>> On Friday 13 December 2013 07:32 PM, Kamil Debski wrote:
>>> Previously the of_phy_get function took a struct device * and was
>>> declared static. It was impossible to call it from another driver and
>>> thus it was impossible to get phy defined for a given node. The old
>>> function was renamed to _of_phy_get and was left for internal use.
>>> of_phy_get function was added and it was exported. The function
>>> enables to get a phy for a given device tree node.
>>>
>>> Signed-off-by: Kamil Debski <k.debski@samsung.com>
>>> ---
>>> It seems that my git send-email is playing up and sent the previous
>>> emails without from. This is a resend. Sorry for any confusion.
>>> ---
>>>  drivers/phy/phy-core.c  |   41 ++++++++++++++++++++++++++++++++-----
>> ----
>>>  include/linux/phy/phy.h |    1 +
>>>  2 files changed, 33 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c index
>>> 03cf8fb..f0dbd42 100644
>>> --- a/drivers/phy/phy-core.c
>>> +++ b/drivers/phy/phy-core.c
>>> @@ -240,8 +240,8 @@ out:
>>>  EXPORT_SYMBOL_GPL(phy_power_off);
>>>
>>>  /**
>>> - * of_phy_get() - lookup and obtain a reference to a phy by phandle
>>> - * @dev: device that requests this phy
>>> + * _of_phy_get() - lookup and obtain a reference to a phy by phandle
>>> + * @np: device_node for which to get the phy
>>>   * @index: the index of the phy
>>>   *
>>>   * Returns the phy associated with the given phandle value, @@
>>> -250,20 +250,17 @@ EXPORT_SYMBOL_GPL(phy_power_off);
>>>   * not yet loaded. This function uses of_xlate call back function
>> provided
>>>   * while registering the phy_provider to find the phy instance.
>>>   */
>>> -static struct phy *of_phy_get(struct device *dev, int index)
>>> +static struct phy *_of_phy_get(struct device_node *np, int index)
>>>  {
>>>  	int ret;
>>>  	struct phy_provider *phy_provider;
>>>  	struct phy *phy = NULL;
>>>  	struct of_phandle_args args;
>>>
>>> -	ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-
>> cells",
>>> +	ret = of_parse_phandle_with_args(np, "phys", "#phy-cells",
>>>  		index, &args);
>>> -	if (ret) {
>>> -		dev_dbg(dev, "failed to get phy in %s node\n",
>>> -			dev->of_node->full_name);
>>> +	if (ret)
>>>  		return ERR_PTR(-ENODEV);
>>> -	}
>>>
>>>  	mutex_lock(&phy_provider_mutex);
>>>  	phy_provider = of_phy_provider_lookup(args.np); @@ -283,6 +280,32
>> @@
>>> err0:
>>>  }
>>>
>>>  /**
>>> + * of_phy_get() - lookup and obtain a reference to a phy using a
>> device_node.
>>> + * @np: device_node for which to get the phy
>>> + * @index: the index of the phy
>>
>> Would be better if the user can get the PHY by string instead of index.
> 
> Ok, this sounds doable. I will add of_phy_get_by_name analogous to the clk
> framework.

Nope. It was decided initially to have minimal no of APIs exported to get PHYs.
So you can just get string as argument instead of index and leave the name of
the API to just of_phy_get.

Thanks
Kishon

> 
>>> + *
>>> + * Returns the phy driver, after getting a refcount to it; or
>>> + * -ENODEV if there is no such phy.  The caller is responsible for
>>> + * calling phy_put() to release that count.
>>> + */
>>> +struct phy *of_phy_get(struct device_node *np, int index) {
>>> +	struct phy *phy = NULL;
>>> +
>>> +	phy = _of_phy_get(np, index);
>>> +	if (IS_ERR(phy))
>> dev_err here?
> 
> dev_err requires a dev, so it is not possible with the arguments used.
> And we do not want to add any extra parameters to keep the function call
> analogous to other of_*_get (as for example of_clk_get).
> 
> Best wishes,
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kamil Debski Dec. 16, 2013, 12:55 p.m. UTC | #4
Hi,

> From: Kishon Vijay Abraham I [mailto:kishon@ti.com]
> Sent: Monday, December 16, 2013 10:01 AM
> 
> On Friday 13 December 2013 08:56 PM, Kamil Debski wrote:
> > Hi Kishon,
> >
> >> From: Kishon Vijay Abraham I [mailto:kishon@ti.com]
> >> Sent: Friday, December 13, 2013 3:45 PM
> >>
> >> Hi,
> >>
> >> On Friday 13 December 2013 07:32 PM, Kamil Debski wrote:
> >>> Previously the of_phy_get function took a struct device * and was
> >>> declared static. It was impossible to call it from another driver
> >>> and thus it was impossible to get phy defined for a given node. The
> >>> old function was renamed to _of_phy_get and was left for internal
> use.
> >>> of_phy_get function was added and it was exported. The function
> >>> enables to get a phy for a given device tree node.
> >>>
> >>> Signed-off-by: Kamil Debski <k.debski@samsung.com>
> >>> ---
> >>> It seems that my git send-email is playing up and sent the previous
> >>> emails without from. This is a resend. Sorry for any confusion.
> >>> ---
> >>>  drivers/phy/phy-core.c  |   41 ++++++++++++++++++++++++++++++++---
> --
> >> ----
> >>>  include/linux/phy/phy.h |    1 +
> >>>  2 files changed, 33 insertions(+), 9 deletions(-)
> >>>
> >>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c index
> >>> 03cf8fb..f0dbd42 100644
> >>> --- a/drivers/phy/phy-core.c
> >>> +++ b/drivers/phy/phy-core.c
> >>> @@ -240,8 +240,8 @@ out:
> >>>  EXPORT_SYMBOL_GPL(phy_power_off);
> >>>
> >>>  /**
> >>> - * of_phy_get() - lookup and obtain a reference to a phy by
> phandle
> >>> - * @dev: device that requests this phy
> >>> + * _of_phy_get() - lookup and obtain a reference to a phy by
> >>> + phandle
> >>> + * @np: device_node for which to get the phy
> >>>   * @index: the index of the phy
> >>>   *
> >>>   * Returns the phy associated with the given phandle value, @@
> >>> -250,20 +250,17 @@ EXPORT_SYMBOL_GPL(phy_power_off);
> >>>   * not yet loaded. This function uses of_xlate call back function
> >> provided
> >>>   * while registering the phy_provider to find the phy instance.
> >>>   */
> >>> -static struct phy *of_phy_get(struct device *dev, int index)
> >>> +static struct phy *_of_phy_get(struct device_node *np, int index)
> >>>  {
> >>>  	int ret;
> >>>  	struct phy_provider *phy_provider;
> >>>  	struct phy *phy = NULL;
> >>>  	struct of_phandle_args args;
> >>>
> >>> -	ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-
> >> cells",
> >>> +	ret = of_parse_phandle_with_args(np, "phys", "#phy-cells",
> >>>  		index, &args);
> >>> -	if (ret) {
> >>> -		dev_dbg(dev, "failed to get phy in %s node\n",
> >>> -			dev->of_node->full_name);
> >>> +	if (ret)
> >>>  		return ERR_PTR(-ENODEV);
> >>> -	}
> >>>
> >>>  	mutex_lock(&phy_provider_mutex);
> >>>  	phy_provider = of_phy_provider_lookup(args.np); @@ -283,6 +280,32
> >> @@
> >>> err0:
> >>>  }
> >>>
> >>>  /**
> >>> + * of_phy_get() - lookup and obtain a reference to a phy using a
> >> device_node.
> >>> + * @np: device_node for which to get the phy
> >>> + * @index: the index of the phy
> >>
> >> Would be better if the user can get the PHY by string instead of
> index.
> >
> > Ok, this sounds doable. I will add of_phy_get_by_name analogous to
> the
> > clk framework.
> 
> Nope. It was decided initially to have minimal no of APIs exported to
> get PHYs.
> So you can just get string as argument instead of index and leave the
> name of the API to just of_phy_get.

If you say so. I just sent a new version taking a string id instead of
an index. I hope you find it satisfying.

> Thanks
> Kishon
> 
> >
> >>> + *
> >>> + * Returns the phy driver, after getting a refcount to it; or
> >>> + * -ENODEV if there is no such phy.  The caller is responsible for
> >>> + * calling phy_put() to release that count.
> >>> + */
> >>> +struct phy *of_phy_get(struct device_node *np, int index) {
> >>> +	struct phy *phy = NULL;
> >>> +
> >>> +	phy = _of_phy_get(np, index);
> >>> +	if (IS_ERR(phy))
> >> dev_err here?
> >
> > dev_err requires a dev, so it is not possible with the arguments used.
> > And we do not want to add any extra parameters to keep the function
> > call analogous to other of_*_get (as for example of_clk_get).
> >
> > Best wishes,
> >

Best wishes,
diff mbox

Patch

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 03cf8fb..f0dbd42 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -240,8 +240,8 @@  out:
 EXPORT_SYMBOL_GPL(phy_power_off);
 
 /**
- * of_phy_get() - lookup and obtain a reference to a phy by phandle
- * @dev: device that requests this phy
+ * _of_phy_get() - lookup and obtain a reference to a phy by phandle
+ * @np: device_node for which to get the phy
  * @index: the index of the phy
  *
  * Returns the phy associated with the given phandle value,
@@ -250,20 +250,17 @@  EXPORT_SYMBOL_GPL(phy_power_off);
  * not yet loaded. This function uses of_xlate call back function provided
  * while registering the phy_provider to find the phy instance.
  */
-static struct phy *of_phy_get(struct device *dev, int index)
+static struct phy *_of_phy_get(struct device_node *np, int index)
 {
 	int ret;
 	struct phy_provider *phy_provider;
 	struct phy *phy = NULL;
 	struct of_phandle_args args;
 
-	ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-cells",
+	ret = of_parse_phandle_with_args(np, "phys", "#phy-cells",
 		index, &args);
-	if (ret) {
-		dev_dbg(dev, "failed to get phy in %s node\n",
-			dev->of_node->full_name);
+	if (ret)
 		return ERR_PTR(-ENODEV);
-	}
 
 	mutex_lock(&phy_provider_mutex);
 	phy_provider = of_phy_provider_lookup(args.np);
@@ -283,6 +280,32 @@  err0:
 }
 
 /**
+ * of_phy_get() - lookup and obtain a reference to a phy using a device_node.
+ * @np: device_node for which to get the phy
+ * @index: the index of the phy
+ *
+ * Returns the phy driver, after getting a refcount to it; or
+ * -ENODEV if there is no such phy.  The caller is responsible for
+ * calling phy_put() to release that count.
+ */
+struct phy *of_phy_get(struct device_node *np, int index)
+{
+	struct phy *phy = NULL;
+
+	phy = _of_phy_get(np, index);
+	if (IS_ERR(phy))
+		return phy;
+
+	if (!try_module_get(phy->ops->owner))
+		return ERR_PTR(-EPROBE_DEFER);
+
+	get_device(&phy->dev);
+
+	return phy;
+}
+EXPORT_SYMBOL_GPL(of_phy_get);
+
+/**
  * phy_put() - release the PHY
  * @phy: the phy returned by phy_get()
  *
@@ -370,7 +393,7 @@  struct phy *phy_get(struct device *dev, const char *string)
 	if (dev->of_node) {
 		index = of_property_match_string(dev->of_node, "phy-names",
 			string);
-		phy = of_phy_get(dev, index);
+		phy = _of_phy_get(dev->of_node, index);
 		if (IS_ERR(phy)) {
 			dev_err(dev, "unable to find phy\n");
 			return phy;
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 6d72269..169f572 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -131,6 +131,7 @@  struct phy *phy_get(struct device *dev, const char *string);
 struct phy *devm_phy_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_get(struct device_node *np, int index);
 struct phy *of_phy_simple_xlate(struct device *dev,
 	struct of_phandle_args *args);
 struct phy *phy_create(struct device *dev, const struct phy_ops *ops,