Re: [RFC PATCH 6/6] USB: MUSB: OMAP: get PHY by phandle for dt boot
On 01/21/2013 03:41 PM, kishon wrote: > Hi, > > On Monday 21 January 2013 06:48 PM, Roger Quadros wrote: >> On 01/16/2013 05:01 PM, Kishon Vijay Abraham I wrote: >>> The OMAP glue has been modified to get PHY by phandle for dt boot. >>> >>> Signed-off-by: Kishon Vijay Abraham I >>> --- >>> drivers/usb/musb/omap2430.c |7 ++- >>> 1 file changed, 6 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c >>> index 3628a50..08709cf 100644 >>> --- a/drivers/usb/musb/omap2430.c >>> +++ b/drivers/usb/musb/omap2430.c >>> @@ -346,7 +346,12 @@ static int omap2430_musb_init(struct musb *musb) >>>* up through ULPI. TWL4030-family PMICs include one, >>>* which needs a driver, drivers aren't always needed. >>>*/ >>> -musb->xceiv = devm_usb_get_phy(dev, 0); >>> +if (dev->parent->of_node) >>> +musb->xceiv = devm_usb_get_phy_by_phandle(dev->parent, >>> +"usb_phy", 0); >> >> Why dev->parent and not just dev? > > Right now MUSB core is not converted to dt and hence we don't have > separate dt node for MUSB core. > So the PHY information is added to the glue dt data. > OK. Got it :). -- cheers, -roger ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH 5/6] usb: otg: add device tree support to otg library
On 01/21/2013 03:34 PM, kishon wrote:
> On Monday 21 January 2013 06:51 PM, Roger Quadros wrote:
>> On 01/16/2013 05:01 PM, Kishon Vijay Abraham I wrote:
>>> Added an API devm_usb_get_phy_by_phandle(), to get usb phy by passing a
>>> device node phandle value. This 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.
>>>
>>> Cc: Marc Kleine-Budde
>>> Signed-off-by: Kishon Vijay Abraham I
>>> ---
>>> drivers/usb/otg/otg.c | 77
>>> +++
>>> include/linux/usb/phy.h |8 +
>>> 2 files changed, 85 insertions(+)
>>>
>>> diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
>>> index dbf2043..e9799bb 100644
>>> --- a/drivers/usb/otg/otg.c
>>> +++ b/drivers/usb/otg/otg.c
>>> @@ -13,7 +13,9 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>> #include
>>> +#include
>>>
>>> #include
>>>
>>> @@ -34,6 +36,20 @@ static struct usb_phy *__usb_find_phy(struct
>>> device *dev, u8 index)
>>> return ERR_PTR(-ENODEV);
>>> }
>>>
>>> +static struct usb_phy *__of_usb_find_phy(struct device_node *node)
>>> +{
>>> +struct usb_phy *phy;
>>> +
>>> +list_for_each_entry(phy, &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;
>>> @@ -109,6 +125,67 @@ 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
>>> + * @index - the index of the phy
>>> + *
>>> + * 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, u8 index)
>>> +{
>>> +struct usb_phy*phy = NULL, **ptr;
>>> +unsigned longflags;
>>> +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, index);
>>> +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);
>>> +}
>>
>> I fail to understand why you need ptr at all and why do devres_alloc()
>> for it.
>
> Thats how we create a "managed device resource". You can have a look at
> Documentation/driver-model/devres.txt and drivers/base/devres.c.
OK, I get it now. You might want to update the text file too since you
are adding a automagically managed interface.
regards,
-roger
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH 5/6] usb: otg: add device tree support to otg library
On 01/16/2013 05:01 PM, Kishon Vijay Abraham I wrote:
> Added an API devm_usb_get_phy_by_phandle(), to get usb phy by passing a
> device node phandle value. This 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.
>
> Cc: Marc Kleine-Budde
> Signed-off-by: Kishon Vijay Abraham I
> ---
> drivers/usb/otg/otg.c | 77
> +++
> include/linux/usb/phy.h |8 +
> 2 files changed, 85 insertions(+)
>
> diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
> index dbf2043..e9799bb 100644
> --- a/drivers/usb/otg/otg.c
> +++ b/drivers/usb/otg/otg.c
> @@ -13,7 +13,9 @@
> #include
> #include
> #include
> +#include
> #include
> +#include
>
> #include
>
> @@ -34,6 +36,20 @@ static struct usb_phy *__usb_find_phy(struct device *dev,
> u8 index)
> return ERR_PTR(-ENODEV);
> }
>
> +static struct usb_phy *__of_usb_find_phy(struct device_node *node)
> +{
> + struct usb_phy *phy;
> +
> + list_for_each_entry(phy, &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;
> @@ -109,6 +125,67 @@ 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
> + * @index - the index of the phy
> + *
> + * 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, u8 index)
> +{
> + 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, index);
> + 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);
> + }
I fail to understand why you need ptr at all and why do devres_alloc()
for it.
> +
> + spin_lock_irqsave(&phy_lock, flags);
> +
> + phy = __of_usb_find_phy(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);
> +
--
cheers,
-roger
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH 6/6] USB: MUSB: OMAP: get PHY by phandle for dt boot
On 01/16/2013 05:01 PM, Kishon Vijay Abraham I wrote:
> The OMAP glue has been modified to get PHY by phandle for dt boot.
>
> Signed-off-by: Kishon Vijay Abraham I
> ---
> drivers/usb/musb/omap2430.c |7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> index 3628a50..08709cf 100644
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -346,7 +346,12 @@ static int omap2430_musb_init(struct musb *musb)
>* up through ULPI. TWL4030-family PMICs include one,
>* which needs a driver, drivers aren't always needed.
>*/
> - musb->xceiv = devm_usb_get_phy(dev, 0);
> + if (dev->parent->of_node)
> + musb->xceiv = devm_usb_get_phy_by_phandle(dev->parent,
> + "usb_phy", 0);
Why dev->parent and not just dev?
> + else
> + musb->xceiv = devm_usb_get_phy(dev, 0);
> +
> if (IS_ERR_OR_NULL(musb->xceiv)) {
> pr_err("HS USB OTG: no transceiver configured\n");
> return -ENODEV;
>
--
cheers,
-roger
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH 3/6] usb: otg: utils: change the phy lib to support multiple PHYs of same type
On 01/16/2013 05:00 PM, Kishon Vijay Abraham I wrote: > In order to add support for multipe PHY's of the same type, the API's > for adding PHY and getting PHY has been changed. Now the binding > information of the PHY and controller should be done in platform file > using usb_bind_phy API. And for getting a PHY, the device pointer of the > USB controller and an index should be passed. Based on the binding > information that is added in the platform file, get_phy will return the > approappropriate PHY. > > Signed-off-by: Kishon Vijay Abraham I > --- > arch/arm/mach-shmobile/board-marzen.c |2 +- > drivers/power/ab8500_charger.c|2 +- > drivers/power/isp1704_charger.c |2 +- > drivers/power/pda_power.c |2 +- > drivers/power/twl4030_charger.c |2 +- > drivers/usb/chipidea/udc.c|2 +- > drivers/usb/dwc3/core.c |4 +- > drivers/usb/gadget/fsl_udc_core.c |2 +- > drivers/usb/gadget/mv_udc_core.c |2 +- > drivers/usb/gadget/omap_udc.c |2 +- > drivers/usb/gadget/pxa25x_udc.c |2 +- > drivers/usb/gadget/pxa27x_udc.c |2 +- > drivers/usb/gadget/s3c-hsudc.c|2 +- > drivers/usb/host/ehci-fsl.c |2 +- > drivers/usb/host/ehci-msm.c |2 +- > drivers/usb/host/ehci-mv.c|2 +- > drivers/usb/host/ehci-tegra.c |2 +- > drivers/usb/host/ohci-omap.c |2 +- > drivers/usb/musb/am35x.c |2 +- > drivers/usb/musb/blackfin.c |2 +- > drivers/usb/musb/da8xx.c |2 +- > drivers/usb/musb/davinci.c|2 +- > drivers/usb/musb/musb_dsps.c |2 +- > drivers/usb/musb/omap2430.c |2 +- > drivers/usb/musb/tusb6010.c |2 +- > drivers/usb/musb/ux500.c |2 +- > drivers/usb/otg/ab8500-usb.c |3 +- > drivers/usb/otg/fsl_otg.c |5 ++- > drivers/usb/otg/gpio_vbus.c |3 +- > drivers/usb/otg/isp1301_omap.c|3 +- > drivers/usb/otg/msm_otg.c |3 +- > drivers/usb/otg/mv_otg.c |3 +- > drivers/usb/otg/nop-usb-xceiv.c |3 +- > drivers/usb/otg/otg.c | 67 > +++-- > drivers/usb/otg/twl4030-usb.c |3 +- > drivers/usb/phy/mv_u3d_phy.c |3 +- > drivers/usb/phy/omap-usb2.c | 11 ++ > drivers/usb/phy/rcar-phy.c|3 +- > include/linux/usb/phy.h | 12 +++--- > 39 files changed, 87 insertions(+), 89 deletions(-) I think it better to leave the existing add/get APIs as they are add add new APIs that support multiple PHYs. You could probably mark the old ones as deprecated. That way you don't need to wait till all users are converted and tested. e.g. you could name the new APIs as usb_add_phy_dev(struct usb_phy *phy); usb_get_phy_dev(struct device *dev, int index); -- cheers, -roger ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH 2/6] ARM: OMAP: USB: Add phy binding information
On 01/16/2013 05:00 PM, Kishon Vijay Abraham I wrote:
> This is in preparation for the changes in PHY library to support adding
> and getting multiple PHYs of the same type. In the new design, the
> binding information between the PHY and the USB controller should be
> specified in the platform specific initialization code. So it's been
> done for OMAP platforms here.
>
> Signed-off-by: Kishon Vijay Abraham I
> ---
> This kind-of binding should be done in all the platforms (I've done only
> for OMAP platform).
> arch/arm/mach-omap2/usb-musb.c |7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
> index 9d27e3f..bbe2fa5 100644
> --- a/arch/arm/mach-omap2/usb-musb.c
> +++ b/arch/arm/mach-omap2/usb-musb.c
> @@ -24,6 +24,7 @@
> #include
> #include
> #include
> +#include
>
> #include "omap_device.h"
> #include "soc.h"
> @@ -85,8 +86,12 @@ void __init usb_musb_init(struct omap_musb_board_data
> *musb_board_data)
> musb_plat.mode = board_data->mode;
> musb_plat.extvbus = board_data->extvbus;
>
> - if (cpu_is_omap44xx())
> + if (cpu_is_omap44xx()) {
> musb_plat.has_mailbox = true;
> + usb_bind_phy("musb-hdrc.0.auto", 0, "omap-usb2.1.auto");
> + } else if (cpu_is_omap34xx()) {
> + usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
> + }
Are you sure than these OMAP platforms cannot be wired in any other way
to the PHY?
If they can be then this association must come from the board files or
device tree.
>
> if (soc_is_am35xx()) {
> oh_name = "am35x_otg_hs";
>
--
cheers,
-roger
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH 1/6] usb: otg: Add an API to bind the USB controller and PHY
On 01/16/2013 05:00 PM, Kishon Vijay Abraham I wrote:
> New platforms are added which has multiple PHY's (of same type) and
> which has multiple USB controllers. The binding information has to be
> present in the PHY library (otg.c) in order for it to return the
> appropriate PHY whenever the USB controller request for the PHY. So
> added a new API to pass the binding information. This API should be
> called by platform specific initialization code.
>
> Signed-off-by: Kishon Vijay Abraham I
> ---
> drivers/usb/otg/otg.c | 37 +
> include/linux/usb/phy.h | 22 ++
> 2 files changed, 59 insertions(+)
>
> diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
> index a30c041..492ba2f 100644
> --- a/drivers/usb/otg/otg.c
> +++ b/drivers/usb/otg/otg.c
> @@ -18,6 +18,7 @@
> #include
>
> static LIST_HEAD(phy_list);
> +static LIST_HEAD(phy_bind_list);
> static DEFINE_SPINLOCK(phy_lock);
>
> static struct usb_phy *__usb_find_phy(struct list_head *list,
> @@ -201,6 +202,42 @@ void usb_remove_phy(struct usb_phy *x)
> }
> EXPORT_SYMBOL(usb_remove_phy);
>
> +/**
> + * usb_bind_phy - bind the phy and the controller that uses the phy
> + * @dev_name: the device name of the device that will bind to the phy
> + * @index: index to specify the port number
> + * @phy_dev_name: the device name of the phy
> + *
> + * Fills the phy_bind structure with the dev_name and phy_dev_name. This will
> + * be used when the phy driver registers the phy and when the controller
> + * requests this phy.
> + *
> + * To be used by platform specific initialization code.
> + */
> +struct usb_phy_bind __init *usb_bind_phy(const char *dev_name, u8 index,
> + const char *phy_dev_name)
> +{
> + struct usb_phy_bind *phy_bind;
> + unsigned long flags;
> +
> + phy_bind = kzalloc(sizeof(*phy_bind), GFP_KERNEL);
> + if (!phy_bind) {
> + pr_err("phy_bind(): No memory for phy_bind");
Function name in comment doesn't match the actual.
Instead, you could use
pr_err("%s ...", __func__);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + phy_bind->dev_name = dev_name;
> + phy_bind->phy_dev_name = phy_dev_name;
> + phy_bind->index = index;
> +
> + spin_lock_irqsave(&phy_lock, flags);
> + list_add_tail(&phy_bind->list, &phy_bind_list);
> + spin_unlock_irqrestore(&phy_lock, flags);
> +
> + return phy_bind;
> +}
> +EXPORT_SYMBOL_GPL(usb_bind_phy);
> +
> const char *otg_state_string(enum usb_otg_state state)
> {
> switch (state) {
> diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
> index a29ae1e..fbeab1a 100644
> --- a/include/linux/usb/phy.h
> +++ b/include/linux/usb/phy.h
> @@ -106,6 +106,21 @@ struct usb_phy {
> enum usb_device_speed speed);
> };
>
> +/**
> + * struct usb_phy_bind - represent the binding for the phy
> + * @dev_name: the device name of the device that will bind to the phy
> + * @phy_dev_name: the device name of the phy
> + * @index: used if a single controller uses multiple phys
> + * @phy: reference to the phy
> + * @list: to maintain a linked list of the binding information
> + */
> +struct usb_phy_bind {
> + const char *dev_name;
> + const char *phy_dev_name;
> + u8 index;
> + struct usb_phy *phy;
> + struct list_head list;
> +};
>
> /* for board-specific init logic */
> extern int usb_add_phy(struct usb_phy *, enum usb_phy_type type);
> @@ -151,6 +166,8 @@ extern struct usb_phy *devm_usb_get_phy(struct device
> *dev,
> enum usb_phy_type type);
> extern void usb_put_phy(struct usb_phy *);
> extern void devm_usb_put_phy(struct device *dev, struct usb_phy *x);
> +extern struct usb_phy_bind *usb_bind_phy(const char *dev_name, u8 index,
> + const char *phy_dev_name);
> #else
> static inline struct usb_phy *usb_get_phy(enum usb_phy_type type)
> {
> @@ -171,6 +188,11 @@ static inline void devm_usb_put_phy(struct device *dev,
> struct usb_phy *x)
> {
> }
>
> +static inline struct usb_phy_bind *usb_bind_phy(const char *dev_name, u8
> index,
> + const char *phy_dev_name)
> +{
> + return NULL;
> +}
> #endif
>
> static inline int
>
Controllers like ehci-omap which don't need OTG functionality would
benefit from this API. Can we make these PHY APIs not dependent on OTG /
OTG_UTILS?
cheers,
-roger
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
