Hi,

We should at least add Grant to the loop, I guess.

On Thu, Sep 06, 2012 at 10:46:13PM +0800, Richard Zhao wrote:
> On Thu, Sep 06, 2012 at 05:30:02PM +0300, Felipe Balbi wrote:
> > On Tue, Sep 04, 2012 at 09:32:06PM +0200, Marc Kleine-Budde wrote:
> > > On 09/04/2012 07:51 PM, ABRAHAM, KISHON VIJAY wrote:
> > > >>>>>>>>>>> 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.
> > > 
> > > Maintainer, I need a Maintainer. Can someone please decide what we want
> > > to have here. I can code all that, but please someone has to make a
> > > decision. Now, please.
> > 
> > Like I said on another reply:
> > 
> > phyN (phy1, phy2, ... phyN) is better since eventually we want to turn
> > this into a kernel-wide PHY layer.
> I think Marc is wondering how to handle the below two case in such way.
>  - how to get the port number the phy is attached to
>  - how to describe it if a port has two phys.

Would something like below work ?

        phy = <&phy1 0 &phy2 1>;

where the first attribute is a phandle to the phy and the second is a
port number ?

-- 
balbi

Attachment: signature.asc
Description: Digital signature

Reply via email to