Hi,
On Tue, Aug 28, 2012 at 7:11 PM, Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> wrote:
> 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;
how about having
if (x && !x->dev.of_node && x->type != USB_PHY_TYPE_UNDEFINED) {
dev_err(x->dev, "not accepting initialized PHY %s\n", x->label);
return -EINVAL;
}
By using this we'll return error if the phy device does not have an
of_node. (So it can't get back the phy by phandle).
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
