Hi,
On Tue, Sep 4, 2012 at 7:46 PM, Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> wrote:
> 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?
We can use something like "xxxx-phy" as the phandle name. And the
users can get the phy by using
devm_usb_get_phy_by_phandle(dev, "xxxx").
(So the frwrk appends *-phy* to the name and searches). Or we don't
have any restriction on the phandle naming conventions and search for
the phandle by the name the user passes in devm_usb_get_phy_by_phandle
directly.
Btw the regulator framework uses something like "xxxx-supply" and it
took me sometime to figure out the regulator phandle name should be
appended with "-supply".
+ const char *phandle)
>
> 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>;
Will this work? Can we return two phandles for a single phandle name?
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
