[PATCH 2/3] usb: otg: add device tree support to otg library

ABRAHAM, KISHON VIJAY kishon at ti.com
Mon Aug 27 15:32:25 EST 2012


Hi,

On Sat, Aug 25, 2012 at 11:43 PM, Marc Kleine-Budde <mkl at pengutronix.de> wrote:
> On 08/24/2012 08:46 AM, Richard Zhao wrote:
>> Did you try both enableing and disabing DT pass build?
>
> Impossible on mx28 :) The platform always selects USE_DT, but on imx it
> builds without DT support.
>
>> On Thu, Aug 23, 2012 at 07:22:53PM +0200, Marc Kleine-Budde wrote:
>>> From: Kishon Vijay Abraham I <kishon at ti.com>
>>>
>>> This patch adds an API to get usb phy by passing a device node phandle value.
>>> The new added devm_usb_get_phy_by_phandle() function will return a pointer to
>>> the phy on success, -EPROBE_DEFER if there is a device_node for the phandle,
>>> but the phy has not been added, or a ERR_PTR() otherwise.
>>>
>>> Since it's possible to obtain a phy by phandle, the checks in usb_add_phy() for
>>> a valid phy type is removed (now it's just a debug message if a user tries to
>>> add a phy with undefined type). This also allows to add multiple phys of same
>>> type.
>>>
>>> Cc: Richard Zhao <richard.zhao at freescale.com>
>>> Cc: Marek Vasut <marex at denx.de>
>>> Signed-off-by: Kishon Vijay Abraham I <kishon at ti.com>
>>> Signed-off-by: Marc Kleine-Budde <mkl at pengutronix.de>
>>> ---
>>>  drivers/usb/otg/otg.c   |   96 ++++++++++++++++++++++++++++++++++++++++-------
>>>  include/linux/usb/otg.h |    8 ++++
>>>  2 files changed, 90 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
>>> index 98c430e..23618de 100644
>>> --- a/drivers/usb/otg/otg.c
>>> +++ b/drivers/usb/otg/otg.c
>>> @@ -15,6 +15,7 @@
>>>  #include <linux/device.h>
>>>  #include <linux/module.h>
>>>  #include <linux/slab.h>
>>> +#include <linux/of.h>
>>>
>>>  #include <linux/usb/otg.h>
>>>
>>> @@ -36,6 +37,21 @@ static struct usb_phy *__usb_find_phy(struct list_head *list,
>>>      return ERR_PTR(-ENODEV);
>>>  }
>>>
>>> +static struct usb_phy *__of_usb_find_phy(struct list_head *list,
>>> +    struct device_node *node)
>>> +{
>>> +    struct usb_phy  *phy;
>>> +
>>> +    list_for_each_entry(phy, list, head) {
>>> +            if (node != phy->dev->of_node)
>>> +                    continue;
>>> +
>>> +            return phy;
>>> +    }
>>> +
>>> +    return ERR_PTR(-ENODEV);
>>> +}
>>> +
>>>  static void devm_usb_phy_release(struct device *dev, void *res)
>>>  {
>>>      struct usb_phy *phy = *(struct usb_phy **)res;
>>> @@ -112,6 +128,66 @@ err0:
>>>  EXPORT_SYMBOL(usb_get_phy);
>>>
>>>  /**
>>> + * devm_usb_get_phy_by_phandle - find the USB PHY by phandle
>>> + * @dev - device that requests this phy
>>> + * @phandle - name of the property holding the phy phandle value
>>> + *
>>> + * Returns the phy driver associated with the given phandle value,
>>> + * after getting a refcount to it, -ENODEV if there is no such phy or
>>> + * -EPROBE_DEFER if there is a phandle to the phy, but the device is
>>> + * not yet loaded. While at that, it also associates the device with
>>> + * the phy using devres. On driver detach, release function is invoked
>>> + * on the devres data, then, devres data is freed.
>>> + *
>>> + * For use by USB host and peripheral drivers.
>>> + */
>>> +struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
>>> +    const char *phandle)
>
>> Since it's already a common function, we may give phandler property
>> a common name too. So we will not need phandle argument.
>> Please also don't forget to document the devm_xxx and dt binding.
>
> Good point. This is the device tree snippet from imx28.dtsi:
>
>>         usb0: usb at 80080000 {
>>                 compatible = "fsl,imx28-usb", "fsl,imx27-usb";
>>                 reg = <0x80080000 0x10000>;
>>                 interrupts = <93>;
>>                 fsl,usbphy = <&usbphy0>;
>                   ^^^^
>
> What about removing the "fsl,", so it just would be "usbphy".
>
>>                 status = "disabled";
>>         };
>
>>> +{
>>> +    struct usb_phy  *phy = NULL, **ptr;
>>> +    unsigned long   flags;
>>> +    struct device_node *node;
>>> +
>>> +    if (!dev->of_node) {
>>> +            dev_dbg(dev, "device does not have a device node entry\n");
>>> +            return ERR_PTR(-EINVAL);
>>> +    }
>>> +
>>> +    node = of_parse_phandle(dev->of_node, phandle, 0);
>>> +    if (!node) {
>>> +            dev_dbg(dev, "failed to get %s phandle in %s node\n", phandle,
>>> +                    dev->of_node->full_name);
>>> +            return ERR_PTR(-ENODEV);
>>> +    }
>>> +
>>> +    ptr = devres_alloc(devm_usb_phy_release, sizeof(*ptr), GFP_KERNEL);
>>> +    if (!ptr) {
>>> +            dev_dbg(dev, "failed to allocate memory for devres\n");
>>> +            return ERR_PTR(-ENOMEM);
>>> +    }
>>> +
>>> +    spin_lock_irqsave(&phy_lock, flags);
>>> +
>>> +    phy = __of_usb_find_phy(&phy_list, node);
>>> +    if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
>>> +            phy = ERR_PTR(-EPROBE_DEFER);
>>> +            devres_free(ptr);
>>> +            goto err0;
>>> +    }
>>> +
>>> +    *ptr = phy;
>>> +    devres_add(dev, ptr);
>>> +
>>> +    get_device(phy->dev);
>>> +
>>> +err0:
>>> +    spin_unlock_irqrestore(&phy_lock, flags);
>>> +
>>> +    return phy;
>>> +}
>>> +EXPORT_SYMBOL(devm_usb_get_phy_by_phandle);
>>> +
>>> +/**
>>>   * devm_usb_put_phy - release the USB PHY
>>>   * @dev - device that wants to release this phy
>>>   * @phy - the phy returned by devm_usb_get_phy()
>>> @@ -158,32 +234,24 @@ EXPORT_SYMBOL(usb_put_phy);
>>>   */
>>>  int usb_add_phy(struct usb_phy *x, enum usb_phy_type type)
>>>  {
>>> -    int             ret = 0;
>>>      unsigned long   flags;
>>>      struct usb_phy  *phy;
>>>
>>> -    if (x && x->type != USB_PHY_TYPE_UNDEFINED) {
>>> -            dev_err(x->dev, "not accepting initialized PHY %s\n", x->label);
>>> -            return -EINVAL;
>
>> why do you remove re-intialize check? Maybe you can add a
>> USB_PHY_TYPE_DT and below logic will be more clear.
>
> I don't know why this check is removed in Kishon's original patch. From
> my point of view all these checks can be removed. Kishon can you explain
> the rationale behind this?

I dint remove the check actually. I just avoided the function from
returning -EINVAL if the phy type is UNDEFINED since now the phy can
be obtained by phandle (even if it has undefined phy type). I still
have the check to *warn* if the phy type is undefined (see below).
>
> regards, Marc
>
>>> -    }
>>> +    if (x && x->type != USB_PHY_TYPE_UNDEFINED)
>>> +            dev_dbg(x->dev, "add a phy with undefined type %s\n", x->label);
>>>
>>>      spin_lock_irqsave(&phy_lock, flags);
>>>
>>> -    list_for_each_entry(phy, &phy_list, head) {
>>> -            if (phy->type == type) {
>>> -                    ret = -EBUSY;
>>> -                    dev_err(x->dev, "transceiver type %s already exists\n",
>>> +    list_for_each_entry(phy, &phy_list, head)
>>> +            if (phy->type == type)
>>> +                    dev_dbg(x->dev, "transceiver type %s already exists\n",
>>>                                              usb_phy_type_string(type));
>>> -                    goto out;
>>> -            }
>>> -    }
>>>
>>>      x->type = type;
>>>      list_add_tail(&x->head, &phy_list);
>>>
>>> -out:
>>>      spin_unlock_irqrestore(&phy_lock, flags);
>>> -    return ret;
>>> +    return 0;
>>>  }
>>>  EXPORT_SYMBOL(usb_add_phy);
>>>
>>> diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
>>> index 45824be..602d6b4 100644
>>> --- a/include/linux/usb/otg.h
>>> +++ b/include/linux/usb/otg.h
>>> @@ -190,6 +190,8 @@ usb_phy_shutdown(struct usb_phy *x)
>>>  extern struct usb_phy *usb_get_phy(enum usb_phy_type type);
>>>  extern struct usb_phy *devm_usb_get_phy(struct device *dev,
>>>      enum usb_phy_type type);
>>> +extern struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
>>> +    const char *phandle);
>>>  extern void usb_put_phy(struct usb_phy *);
>>>  extern void devm_usb_put_phy(struct device *dev, struct usb_phy *x);
>>>  extern const char *otg_state_string(enum usb_otg_state state);
>>> @@ -205,6 +207,12 @@ static inline struct usb_phy *devm_usb_get_phy(struct device *dev,
>>>      return NULL;
>>>  }
>>>
>>> +static inline struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
>>> +    const char *phandle)
>>> +{
>>> +    return NULL;
>>> +}
>>> +
>>>  static inline void usb_put_phy(struct usb_phy *x)
>>>  {
>>>  }
>>
>> Thanks
>> Richard
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
>
> --
> Pengutronix e.K.                  | Marc Kleine-Budde           |
> Industrial Linux Solutions        | Phone: +49-231-2826-924     |
> Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |
>

Thanks
Kishon


More information about the devicetree-discuss mailing list