Hi,
On Thu, Jun 7, 2012 at 1:38 PM, Felipe Balbi <balbi@xxxxxx> wrote:
> On Tue, Jun 05, 2012 at 02:56:36PM +0530, Kishon Vijay Abraham I wrote:
>> Used devres API's to associate the phy with a device so that on
>> driver detach, release function is invoked on the devres data(usb_phy)
>> and devres data(usb_phy) is released.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@xxxxxx>
>> ---
>> drivers/usb/otg/otg.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/usb/otg.h | 13 ++++++++++
>> 2 files changed, 73 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
>> index a230658..23c62b0 100644
>> --- a/drivers/usb/otg/otg.c
>> +++ b/drivers/usb/otg/otg.c
>> @@ -13,6 +13,7 @@
>> #include <linux/export.h>
>> #include <linux/err.h>
>> #include <linux/device.h>
>> +#include <linux/slab.h>
>>
>> #include <linux/usb/otg.h>
>>
>> @@ -34,6 +35,47 @@ static struct usb_phy *__usb_find_phy(struct list_head *list,
>> return ERR_PTR(-ENODEV);
>> }
>>
>> +static void devm_usb_phy_release(struct device *dev, void *res)
>> +{
>> + struct usb_phy *phy = *(struct usb_phy **)res;
>> +
>> + usb_put_phy(phy);
>> +}
>> +
>> +static int devm_usb_phy_match(struct device *dev, void *res, void *match_data)
>> +{
>> + return *(struct usb_phy **)res == match_data;
>
> no need for cast here:
>
> return res == match_data;
>
> should be enough
Ok.
>
>> +}
>> +
>> +/**
>> + * devm_usb_get_phy - find the USB PHY
>> + * @dev - device that requests this phy
>> + * @type - the type of the phy the controller requires
>> + *
>> + * Gets the phy using usb_get_phy(), and associates a device with it 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(struct device *dev, enum usb_phy_type type)
>> +{
>> + struct usb_phy **ptr, *phy;
>> +
>> + ptr = devres_alloc(devm_usb_phy_release, sizeof(*ptr), GFP_KERNEL);
>> + if (!ptr)
>> + return NULL;
>> +
>> + phy = *ptr = usb_get_phy(type);
>
> we avoid multiple assignments. Turn this into:
>
> phy = usb_get_phy(type);
> if (phy) {
> *ptr = phy;
> devres_add(dev, ptr);
> } else {
> devres_free(ptr);
> }
Ok.
>
>> + if (phy)
>> + devres_add(dev, ptr);
>> + else
>> + devres_free(ptr);
>> +
>> + return phy;
>> +}
>> +EXPORT_SYMBOL(devm_usb_get_phy);
>> +
>> /**
>> * usb_get_phy - find the USB PHY
>> * @type - the type of the phy the controller requires
>> @@ -67,6 +109,24 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type)
>> EXPORT_SYMBOL(usb_get_phy);
>>
>> /**
>> + * 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()
>> + *
>> + * destroys the devres associated with this phy and invokes usb_put_phy
>> + * to release the phy.
>> + *
>> + * For use by USB host and peripheral drivers.
>> + */
>> +void devm_usb_put_phy(struct device *dev, struct usb_phy *phy)
>> +{
>> + WARN_ON(devres_destroy(dev, devm_usb_phy_release, devm_usb_phy_match,
>> + phy));
>
> I would break this into two lines:
>
> int r;
>
> r = devres_destroy(dev, devm_usb_phy_release, devm_usb_phy_match, phy);
> dev_WARN_ONCE(dev, r, "couldn't find PHY resource\n");
>
>> + usb_put_phy(phy);
>
> devm_usb_phy_release() all calls usb_put_phy(). This will cause a bug.
oh yeah.. thanks for pointing that.
>
>> +}
>> +EXPORT_SYMBOL(devm_usb_put_phy);
>
> not sure you even need to define this... nothing against it though.
I added that more for completion sake. Other utilities that provide
devm_xxxx API's has API for both acquire and release.
Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
