Re: [PATCH] usb: phy: add R-Car R8A7779 USB phy driver.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Wed, Sep 5, 2012 at 6:20 AM, Kuninori Morimoto
<kuninori.morimoto.gx@xxxxxxxxxxx> wrote:
>
> Hi Abraham
>
> Thank you for checking patch
>
>
>> > +config USB_RCAR_PHY
>> > +       tristate "Renesas R-Car USB phy support"
>> > +       depends on (USB || USB_GADGET) && ARCH_R8A7779
>> > +       help
>> > +         Say Y here to add support for the Renesas R-Car USB phy driver.
>>
>> Just out of curiosity, which USB contoller will/is using this PHY?
>
> EHCI/OHCI.
> Now, I'm using ohci/ehci-platform.c

Thats a generic ehci driver. I was interested in the specific usb
controller that uses this phy. Anyways now I know its a host only
driver :-)

>
>> > +       reg0 = ioremap_nocache(res0->start, resource_size(res0));
>> > +       reg1 = ioremap_nocache(res1->start, resource_size(res1));
>>
>> Instead use devm_ioremap_nocache?
>
> will do in v2 patch
>
>> > +       /* (1) USB-PHY standby release */
>> > +       iowrite32(0x00000001, (reg0 + USBPCTRL1));
>>
>> Can we have some macros to define the above constant (and for the
>> constants used below)?
>
> will do in v2 patch
>
>> > +
>> > +       /* (2) start USB-PHY internal PLL */
>> > +       iowrite32(0x00000003, (reg0 + USBPCTRL1));
>>
>> No power management stuff added?? Dont we need to stop this PLL during suspend??
> (snip)
>> I see you are doing one time initialization of the phy during probe.
>> But I think this phy will be pointless without the usb controller.
>> Instead how about using the library functions like
>> usb_add_phy/usb_get_phy/usb_phy_init?
>
> I'm not sure detail, but this usb_xxx_phy() is under otg.
> Our system don't support it, but is it normal ?

Yeah. We are in the process of cleaning it up. Indeed it's normal in
the sense quite a few host only drivers use it (ehci-fsl.c,
ehci-msm.c, ehci-mv.c, ehci-tegra.c, ohci-omap.c).
>
> Now, I'm using ehci/ohci-platform.
> When I support this usb_xxx_phy(), do I need to customize these drivers ?

You can't change ehci-platform.c because thats a generic driver. But
you have to do those modifications in your controller specific file.
>
> And, this is extra stuff, but I have 1 question.
> it is ${LINUX}/driver/usb/Makefile's order.
>
> obj-(xxx) += host/
> ...
> obj-(xxx) += phy/
>
> This means that usb phy driver probe function is called
> after host driver probe function().

AFAIK, the order in Makefile will just determine the order in which in
the files are compiled and not the order in which the modules are
loaded/probed. The module loading/probing is dependent on which
*initcall* section you've added your module to and also based on
device creation.
However if two modules add themselves to the same *initcall* section,
the order in which the files are compiled determine the order in which
the modules are probed (I think this is what is happening in your
case).

> Now, I'm using a technique which delays usb host driver registration,
> but why phy driver is called after host driver ?

Does changing the order in Makefile helped you?

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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux