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
