On Tue, Sep 04, 2012 at 12:31:14PM +0200, Marc Kleine-Budde wrote: > On 08/29/2012 10:11 PM, Marc Kleine-Budde 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. Kishon ? -- balbi
Attachment:
signature.asc
Description: Digital signature
