On 09/04/2012 04:07 PM, Richard Zhao wrote:
>>>>>>>>>> +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.
>>>>>> That only means current binding is not good enough. Rather not, means
>>>>>> it should not be in common code.
>>>>>> Maybe something like:
>>>>>> usbport0-phys = <&phy0>;
>>>>>> usbport1-phys = <&phy1 &phy2>; /* usb2 & usb3 */
>>>>>
>>>>> Granted. Do we need strings that describe the phys, like in pinctrl or
>>>>> is a index enough? What about this?
>>>>>
>>>>> struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
>>>>>   int index)
>>>>>
>>>>
>>>> Comments? The phandle_name string will be "usbphy".
>>>
>>> I don't think phandle_name should be usbphy. Eventually we want to turn
>>> this into a kernel-wide phy subsystem and if we use usbphy, we will just
>>> have to patch a bunch of dts files once we make the move.
> Coud you please give a link of "kernel-wide phy subsystem" discussion?
>>
>> Is just "phy" better?
> If the property name don't include port number, how do we know what
> port the phy is attached to?

Take this ci13xxx-imx dts snippet for example:

usb@02184000 { /* USB OTG */
        compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
        reg = <0x02184000 0x200>;
        interrupts = <0 43 0x04>;
        phy = <&usbphy1>;
};

The existing "fsl,usbphy" will be renamed into just "phy". If a usb/otg
device needs more than one phy the dts will look like this:

usb@02184000 { /* USB OTG */
        compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
        reg = <0x02184000 0x200>;
        interrupts = <0 43 0x04>;
        phy = <&usbphy1 &usbphy2>;
};

The driver will get a reference to the usb_phy with:

struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
        int index)

Where the index specifies with (usb) phy it wants to use.

This covers TI's usecase with an USB2 and an USB3 phy for one otg device.

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

Reply via email to