On 08/24/2012 08:46 AM, Richard Zhao wrote:
[...]
>> /**
>> + * 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.
I don't think this is a good idea. If we hardcode the phandle name, we
introduce a limit of one phy per usb device. The usb3 controllers
alreadyt use two phys (one for usb2, the othere for usb3) for one
controller. So I think we should not make the same mistake again.
>> +{
>> + 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 think Kishon explained it, Richard are you okay with this?
Marc
--
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 |
Attachment:
signature.asc
Description: OpenPGP digital signature
