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 |
signature.asc
Description: OpenPGP digital signature
