* Re: [PATCH] ARM: shmobile: r8a7790: link PCI USb devices to USB PHY
@ 2014-04-10 7:07 ` Magnus Damm
0 siblings, 0 replies; 18+ messages in thread
From: Magnus Damm @ 2014-04-10 7:07 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Simon Horman [Horms], SH-Linux, devicetree@vger.kernel.org,
robh+dt, Pawel Moll, Mark Rutland, ijc+devicetree, Kumar Gala,
Russell King - ARM Linux, linux-arm-kernel@lists.infradead.org
Hi Sergei,
Thanks for this patch, good to see that the relationship between the
USB Host and the PHY is described via DT.
This patch seems to cover USB0 and USB2 that both require special
control in the PHY. How about USB1? Can you explain about the reason
why you omit that?
I somehow expected USB1 to be tied to the PHY via DT as well, this so
the PHY driver can be able to disable the hardware if no ports are
registered. In detail, I'm thinking that the register.bit
UGCTL.CONNECT wants to be set to 0 by default but be set to 1 once one
or more USB controllers are hooked up to the PHY.
Cheers,
/ magnus
On Thu, Apr 10, 2014 at 4:18 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Describe the PCI USB devices that are behind the PCI bridges, adding necessary
> links to the USB PHY device.
>
> Based on the original work by Ben Dooks <ben.dooks@codethink.co.uk>.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
> This patch is against 'renesas-devel-v3.14-20140408' tag of Simon Horman's
> 'renesas.git' repo plus R8A7790/Lager PCI and USB PHY support patches posted
> yesterday and day before. The patch requires the internal PCI and USB PHY
> drivers (also already posted) in order to work...
>
> arch/arm/boot/dts/r8a7790.dtsi | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> Index: renesas/arch/arm/boot/dts/r8a7790.dtsi
> ===================================================================
> --- renesas.orig/arch/arm/boot/dts/r8a7790.dtsi
> +++ renesas/arch/arm/boot/dts/r8a7790.dtsi
> @@ -841,6 +841,20 @@
> bus-range = <0 0>;
> #address-cells = <3>;
> #size-cells = <2>;
> +
> + pci@0,1 {
> + reg = <0x800 0 0 0 0>;
> + device_type = "pci";
> + phys = <&usbphy 0 0>;
> + phy-names = "usb";
> + };
> +
> + pci@0,2 {
> + reg = <0x1000 0 0 0 0>;
> + device_type = "pci";
> + phys = <&usbphy 0 0>;
> + phy-names = "usb";
> + };
> };
>
> pci1: pci@ee0b0000 {
> @@ -867,5 +881,19 @@
> bus-range = <2 2>;
> #address-cells = <3>;
> #size-cells = <2>;
> +
> + pci@0,1 {
> + reg = <0x800 0 0 0 0>;
> + device_type = "pci";
> + phys = <&usbphy 2 0>;
> + phy-names = "usb";
> + };
> +
> + pci@0,2 {
> + reg = <0x1000 0 0 0 0>;
> + device_type = "pci";
> + phys = <&usbphy 2 0>;
> + phy-names = "usb";
> + };
> };
> };
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH] ARM: shmobile: r8a7790: link PCI USb devices to USB PHY
@ 2014-04-10 7:07 ` Magnus Damm
0 siblings, 0 replies; 18+ messages in thread
From: Magnus Damm @ 2014-04-10 7:07 UTC (permalink / raw)
To: linux-arm-kernel
Hi Sergei,
Thanks for this patch, good to see that the relationship between the
USB Host and the PHY is described via DT.
This patch seems to cover USB0 and USB2 that both require special
control in the PHY. How about USB1? Can you explain about the reason
why you omit that?
I somehow expected USB1 to be tied to the PHY via DT as well, this so
the PHY driver can be able to disable the hardware if no ports are
registered. In detail, I'm thinking that the register.bit
UGCTL.CONNECT wants to be set to 0 by default but be set to 1 once one
or more USB controllers are hooked up to the PHY.
Cheers,
/ magnus
On Thu, Apr 10, 2014 at 4:18 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Describe the PCI USB devices that are behind the PCI bridges, adding necessary
> links to the USB PHY device.
>
> Based on the original work by Ben Dooks <ben.dooks@codethink.co.uk>.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
> This patch is against 'renesas-devel-v3.14-20140408' tag of Simon Horman's
> 'renesas.git' repo plus R8A7790/Lager PCI and USB PHY support patches posted
> yesterday and day before. The patch requires the internal PCI and USB PHY
> drivers (also already posted) in order to work...
>
> arch/arm/boot/dts/r8a7790.dtsi | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> Index: renesas/arch/arm/boot/dts/r8a7790.dtsi
> ===================================================================
> --- renesas.orig/arch/arm/boot/dts/r8a7790.dtsi
> +++ renesas/arch/arm/boot/dts/r8a7790.dtsi
> @@ -841,6 +841,20 @@
> bus-range = <0 0>;
> #address-cells = <3>;
> #size-cells = <2>;
> +
> + pci at 0,1 {
> + reg = <0x800 0 0 0 0>;
> + device_type = "pci";
> + phys = <&usbphy 0 0>;
> + phy-names = "usb";
> + };
> +
> + pci at 0,2 {
> + reg = <0x1000 0 0 0 0>;
> + device_type = "pci";
> + phys = <&usbphy 0 0>;
> + phy-names = "usb";
> + };
> };
>
> pci1: pci at ee0b0000 {
> @@ -867,5 +881,19 @@
> bus-range = <2 2>;
> #address-cells = <3>;
> #size-cells = <2>;
> +
> + pci at 0,1 {
> + reg = <0x800 0 0 0 0>;
> + device_type = "pci";
> + phys = <&usbphy 2 0>;
> + phy-names = "usb";
> + };
> +
> + pci at 0,2 {
> + reg = <0x1000 0 0 0 0>;
> + device_type = "pci";
> + phys = <&usbphy 2 0>;
> + phy-names = "usb";
> + };
> };
> };
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] ARM: shmobile: r8a7790: link PCI USb devices to USB PHY
2014-04-10 7:07 ` Magnus Damm
(?)
@ 2014-04-10 10:38 ` Sergei Shtylyov
-1 siblings, 0 replies; 18+ messages in thread
From: Sergei Shtylyov @ 2014-04-10 10:38 UTC (permalink / raw)
To: linux-arm-kernel
Hello.
On 10-04-2014 11:07, Magnus Damm wrote:
> Thanks for this patch, good to see that the relationship between the
> USB Host and the PHY is described via DT.
> This patch seems to cover USB0 and USB2 that both require special
> control in the PHY. How about USB1? Can you explain about the reason
> why you omit that?
Because the driver does nothing for USB1 anyway.
> I somehow expected USB1 to be tied to the PHY via DT as well, this so
> the PHY driver can be able to disable the hardware if no ports are
> registered. In detail, I'm thinking that the register.bit
> UGCTL.CONNECT wants to be set to 0 by default but be set to 1 once one
> or more USB controllers are hooked up to the PHY.
I don't think this bit has any sense for non-USBHS controllers. EHCI/OHCI
drivers happily work without setting it.
WBR, Sergei
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] ARM: shmobile: r8a7790: link PCI USb devices to USB PHY
@ 2014-04-10 10:38 ` Sergei Shtylyov
0 siblings, 0 replies; 18+ messages in thread
From: Sergei Shtylyov @ 2014-04-10 10:38 UTC (permalink / raw)
To: Magnus Damm
Cc: Simon Horman [Horms], SH-Linux, devicetree@vger.kernel.org,
robh+dt, Pawel Moll, Mark Rutland, ijc+devicetree, Kumar Gala,
Russell King - ARM Linux, linux-arm-kernel@lists.infradead.org
Hello.
On 10-04-2014 11:07, Magnus Damm wrote:
> Thanks for this patch, good to see that the relationship between the
> USB Host and the PHY is described via DT.
> This patch seems to cover USB0 and USB2 that both require special
> control in the PHY. How about USB1? Can you explain about the reason
> why you omit that?
Because the driver does nothing for USB1 anyway.
> I somehow expected USB1 to be tied to the PHY via DT as well, this so
> the PHY driver can be able to disable the hardware if no ports are
> registered. In detail, I'm thinking that the register.bit
> UGCTL.CONNECT wants to be set to 0 by default but be set to 1 once one
> or more USB controllers are hooked up to the PHY.
I don't think this bit has any sense for non-USBHS controllers. EHCI/OHCI
drivers happily work without setting it.
WBR, Sergei
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH] ARM: shmobile: r8a7790: link PCI USb devices to USB PHY
@ 2014-04-10 10:38 ` Sergei Shtylyov
0 siblings, 0 replies; 18+ messages in thread
From: Sergei Shtylyov @ 2014-04-10 10:38 UTC (permalink / raw)
To: linux-arm-kernel
Hello.
On 10-04-2014 11:07, Magnus Damm wrote:
> Thanks for this patch, good to see that the relationship between the
> USB Host and the PHY is described via DT.
> This patch seems to cover USB0 and USB2 that both require special
> control in the PHY. How about USB1? Can you explain about the reason
> why you omit that?
Because the driver does nothing for USB1 anyway.
> I somehow expected USB1 to be tied to the PHY via DT as well, this so
> the PHY driver can be able to disable the hardware if no ports are
> registered. In detail, I'm thinking that the register.bit
> UGCTL.CONNECT wants to be set to 0 by default but be set to 1 once one
> or more USB controllers are hooked up to the PHY.
I don't think this bit has any sense for non-USBHS controllers. EHCI/OHCI
drivers happily work without setting it.
WBR, Sergei
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] ARM: shmobile: r8a7790: link PCI USb devices to USB PHY
2014-04-10 10:38 ` Sergei Shtylyov
(?)
@ 2014-04-10 18:46 ` Sergei Shtylyov
-1 siblings, 0 replies; 18+ messages in thread
From: Sergei Shtylyov @ 2014-04-10 18:46 UTC (permalink / raw)
To: linux-arm-kernel
On 04/10/2014 02:38 PM, Sergei Shtylyov wrote:
>> Thanks for this patch, good to see that the relationship between the
>> USB Host and the PHY is described via DT.
>> This patch seems to cover USB0 and USB2 that both require special
>> control in the PHY. How about USB1? Can you explain about the reason
>> why you omit that?
> Because the driver does nothing for USB1 anyway.
Looks like I should have tested that last minute change: kernel oopses due
to NULL pointer dereference somewhere in phy_get() once it gets called for
EHCI on the channel #1. At least doesn't seem to be my mistake...
WBR, Sergei
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] ARM: shmobile: r8a7790: link PCI USb devices to USB PHY
@ 2014-04-10 18:46 ` Sergei Shtylyov
0 siblings, 0 replies; 18+ messages in thread
From: Sergei Shtylyov @ 2014-04-10 18:46 UTC (permalink / raw)
To: Magnus Damm
Cc: Simon Horman [Horms], SH-Linux, devicetree@vger.kernel.org,
robh+dt, Pawel Moll, Mark Rutland, ijc+devicetree, Kumar Gala,
Russell King - ARM Linux, linux-arm-kernel@lists.infradead.org
On 04/10/2014 02:38 PM, Sergei Shtylyov wrote:
>> Thanks for this patch, good to see that the relationship between the
>> USB Host and the PHY is described via DT.
>> This patch seems to cover USB0 and USB2 that both require special
>> control in the PHY. How about USB1? Can you explain about the reason
>> why you omit that?
> Because the driver does nothing for USB1 anyway.
Looks like I should have tested that last minute change: kernel oopses due
to NULL pointer dereference somewhere in phy_get() once it gets called for
EHCI on the channel #1. At least doesn't seem to be my mistake...
WBR, Sergei
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH] ARM: shmobile: r8a7790: link PCI USb devices to USB PHY
@ 2014-04-10 18:46 ` Sergei Shtylyov
0 siblings, 0 replies; 18+ messages in thread
From: Sergei Shtylyov @ 2014-04-10 18:46 UTC (permalink / raw)
To: linux-arm-kernel
On 04/10/2014 02:38 PM, Sergei Shtylyov wrote:
>> Thanks for this patch, good to see that the relationship between the
>> USB Host and the PHY is described via DT.
>> This patch seems to cover USB0 and USB2 that both require special
>> control in the PHY. How about USB1? Can you explain about the reason
>> why you omit that?
> Because the driver does nothing for USB1 anyway.
Looks like I should have tested that last minute change: kernel oopses due
to NULL pointer dereference somewhere in phy_get() once it gets called for
EHCI on the channel #1. At least doesn't seem to be my mistake...
WBR, Sergei
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] ARM: shmobile: r8a7790: link PCI USb devices to USB PHY
2014-04-10 18:46 ` Sergei Shtylyov
(?)
@ 2014-04-11 5:48 ` Magnus Damm
-1 siblings, 0 replies; 18+ messages in thread
From: Magnus Damm @ 2014-04-11 5:48 UTC (permalink / raw)
To: linux-arm-kernel
Hi Sergei,
On Fri, Apr 11, 2014 at 3:46 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On 04/10/2014 02:38 PM, Sergei Shtylyov wrote:
>
>>> Thanks for this patch, good to see that the relationship between the
>>> USB Host and the PHY is described via DT.
>
>
>>> This patch seems to cover USB0 and USB2 that both require special
>>> control in the PHY. How about USB1? Can you explain about the reason
>>> why you omit that?
>
>
>> Because the driver does nothing for USB1 anyway.
>
>
> Looks like I should have tested that last minute change: kernel oopses
> due to NULL pointer dereference somewhere in phy_get() once it gets called
> for EHCI on the channel #1. At least doesn't seem to be my mistake...
No worries, thanks for looking into fixing that.
Regarding the USB ports on R-Car Gen2 in general and especially USB1,
it is my impression that even though there is no USB controller
selection available for USB1 I still believe the UGCTL.CONNECT bit
shall be used for power management purpose.
I may of course be wrong, but since the PHY hardware is shared between
USB0, USB1 and USB2 it makes sense to have some kind of usage counter
and manage the hardware enable bit based on registered users somehow.
There is no need to manage this bit at this point IMO, but in the
future we may want to add such handling to improve power management.
And that can only happen if DT is used to connect all USB controllers
with the PHY, so please make sure to describe the complete
dependencies in DT.
Thanks,
/ magnus
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ARM: shmobile: r8a7790: link PCI USb devices to USB PHY
@ 2014-04-11 5:48 ` Magnus Damm
0 siblings, 0 replies; 18+ messages in thread
From: Magnus Damm @ 2014-04-11 5:48 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Simon Horman [Horms], SH-Linux, devicetree@vger.kernel.org,
robh+dt, Pawel Moll, Mark Rutland, ijc+devicetree, Kumar Gala,
Russell King - ARM Linux, linux-arm-kernel@lists.infradead.org
Hi Sergei,
On Fri, Apr 11, 2014 at 3:46 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On 04/10/2014 02:38 PM, Sergei Shtylyov wrote:
>
>>> Thanks for this patch, good to see that the relationship between the
>>> USB Host and the PHY is described via DT.
>
>
>>> This patch seems to cover USB0 and USB2 that both require special
>>> control in the PHY. How about USB1? Can you explain about the reason
>>> why you omit that?
>
>
>> Because the driver does nothing for USB1 anyway.
>
>
> Looks like I should have tested that last minute change: kernel oopses
> due to NULL pointer dereference somewhere in phy_get() once it gets called
> for EHCI on the channel #1. At least doesn't seem to be my mistake...
No worries, thanks for looking into fixing that.
Regarding the USB ports on R-Car Gen2 in general and especially USB1,
it is my impression that even though there is no USB controller
selection available for USB1 I still believe the UGCTL.CONNECT bit
shall be used for power management purpose.
I may of course be wrong, but since the PHY hardware is shared between
USB0, USB1 and USB2 it makes sense to have some kind of usage counter
and manage the hardware enable bit based on registered users somehow.
There is no need to manage this bit at this point IMO, but in the
future we may want to add such handling to improve power management.
And that can only happen if DT is used to connect all USB controllers
with the PHY, so please make sure to describe the complete
dependencies in DT.
Thanks,
/ magnus
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] ARM: shmobile: r8a7790: link PCI USb devices to USB PHY
@ 2014-04-11 5:48 ` Magnus Damm
0 siblings, 0 replies; 18+ messages in thread
From: Magnus Damm @ 2014-04-11 5:48 UTC (permalink / raw)
To: linux-arm-kernel
Hi Sergei,
On Fri, Apr 11, 2014 at 3:46 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On 04/10/2014 02:38 PM, Sergei Shtylyov wrote:
>
>>> Thanks for this patch, good to see that the relationship between the
>>> USB Host and the PHY is described via DT.
>
>
>>> This patch seems to cover USB0 and USB2 that both require special
>>> control in the PHY. How about USB1? Can you explain about the reason
>>> why you omit that?
>
>
>> Because the driver does nothing for USB1 anyway.
>
>
> Looks like I should have tested that last minute change: kernel oopses
> due to NULL pointer dereference somewhere in phy_get() once it gets called
> for EHCI on the channel #1. At least doesn't seem to be my mistake...
No worries, thanks for looking into fixing that.
Regarding the USB ports on R-Car Gen2 in general and especially USB1,
it is my impression that even though there is no USB controller
selection available for USB1 I still believe the UGCTL.CONNECT bit
shall be used for power management purpose.
I may of course be wrong, but since the PHY hardware is shared between
USB0, USB1 and USB2 it makes sense to have some kind of usage counter
and manage the hardware enable bit based on registered users somehow.
There is no need to manage this bit at this point IMO, but in the
future we may want to add such handling to improve power management.
And that can only happen if DT is used to connect all USB controllers
with the PHY, so please make sure to describe the complete
dependencies in DT.
Thanks,
/ magnus
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ARM: shmobile: r8a7790: link PCI USb devices to USB PHY
2014-04-11 5:48 ` Magnus Damm
(?)
@ 2014-04-11 11:39 ` Sergei Shtylyov
-1 siblings, 0 replies; 18+ messages in thread
From: Sergei Shtylyov @ 2014-04-11 11:39 UTC (permalink / raw)
To: linux-arm-kernel
On 11.04.2014 9:48, Magnus Damm wrote:
>>>> Thanks for this patch, good to see that the relationship between the
>>>> USB Host and the PHY is described via DT.
>>>> This patch seems to cover USB0 and USB2 that both require special
>>>> control in the PHY. How about USB1? Can you explain about the reason
>>>> why you omit that?
>>> Because the driver does nothing for USB1 anyway.
>> Looks like I should have tested that last minute change: kernel oopses
>> due to NULL pointer dereference somewhere in phy_get() once it gets called
>> for EHCI on the channel #1. At least doesn't seem to be my mistake...
> No worries, thanks for looking into fixing that.
> Regarding the USB ports on R-Car Gen2 in general and especially USB1,
> it is my impression that even though there is no USB controller
> selection available for USB1 I still believe the UGCTL.CONNECT bit
> shall be used for power management purpose.
I don't think so. As I said the EHCI driver happily works without touching
this bit.
> I may of course be wrong, but since the PHY hardware is shared between
> USB0, USB1 and USB2
I believe it's a wrong impression (which probably my and Valentine's PHY
drivers have created). The PHY actually belongs to USBHS only; UGCTRL2
register is some kind of ad-hockery in this PHY.
> it makes sense to have some kind of usage counter
Generic PHY core already maintains such counters for phy_{init|exit}() and
phy_power_{on|off}() calls.
> and manage the hardware enable bit based on registered users somehow.
> There is no need to manage this bit at this point IMO, but in the
> future we may want to add such handling to improve power management.
I handle this bit but only for USBHS. What is actually necessary for all
USB controllers is enabling the USBHS clocks in order for UGCTRL2 to work.
> And that can only happen if DT is used to connect all USB controllers
> with the PHY, so please make sure to describe the complete
> dependencies in DT.
I cannnot represent USBHS in the device tree yet. I think I can represent
xHCI there but I was told it needs the firmware in order to work -- which we
don't have, so I'm not sure about xHCI testing.
> Thanks,
> / magnus
WBR, Sergei
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] ARM: shmobile: r8a7790: link PCI USb devices to USB PHY
@ 2014-04-11 11:39 ` Sergei Shtylyov
0 siblings, 0 replies; 18+ messages in thread
From: Sergei Shtylyov @ 2014-04-11 11:39 UTC (permalink / raw)
To: Magnus Damm
Cc: Simon Horman [Horms], SH-Linux, devicetree@vger.kernel.org,
robh+dt, Pawel Moll, Mark Rutland, ijc+devicetree, Kumar Gala,
Russell King - ARM Linux, linux-arm-kernel@lists.infradead.org
On 11.04.2014 9:48, Magnus Damm wrote:
>>>> Thanks for this patch, good to see that the relationship between the
>>>> USB Host and the PHY is described via DT.
>>>> This patch seems to cover USB0 and USB2 that both require special
>>>> control in the PHY. How about USB1? Can you explain about the reason
>>>> why you omit that?
>>> Because the driver does nothing for USB1 anyway.
>> Looks like I should have tested that last minute change: kernel oopses
>> due to NULL pointer dereference somewhere in phy_get() once it gets called
>> for EHCI on the channel #1. At least doesn't seem to be my mistake...
> No worries, thanks for looking into fixing that.
> Regarding the USB ports on R-Car Gen2 in general and especially USB1,
> it is my impression that even though there is no USB controller
> selection available for USB1 I still believe the UGCTL.CONNECT bit
> shall be used for power management purpose.
I don't think so. As I said the EHCI driver happily works without touching
this bit.
> I may of course be wrong, but since the PHY hardware is shared between
> USB0, USB1 and USB2
I believe it's a wrong impression (which probably my and Valentine's PHY
drivers have created). The PHY actually belongs to USBHS only; UGCTRL2
register is some kind of ad-hockery in this PHY.
> it makes sense to have some kind of usage counter
Generic PHY core already maintains such counters for phy_{init|exit}() and
phy_power_{on|off}() calls.
> and manage the hardware enable bit based on registered users somehow.
> There is no need to manage this bit at this point IMO, but in the
> future we may want to add such handling to improve power management.
I handle this bit but only for USBHS. What is actually necessary for all
USB controllers is enabling the USBHS clocks in order for UGCTRL2 to work.
> And that can only happen if DT is used to connect all USB controllers
> with the PHY, so please make sure to describe the complete
> dependencies in DT.
I cannnot represent USBHS in the device tree yet. I think I can represent
xHCI there but I was told it needs the firmware in order to work -- which we
don't have, so I'm not sure about xHCI testing.
> Thanks,
> / magnus
WBR, Sergei
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH] ARM: shmobile: r8a7790: link PCI USb devices to USB PHY
@ 2014-04-11 11:39 ` Sergei Shtylyov
0 siblings, 0 replies; 18+ messages in thread
From: Sergei Shtylyov @ 2014-04-11 11:39 UTC (permalink / raw)
To: linux-arm-kernel
On 11.04.2014 9:48, Magnus Damm wrote:
>>>> Thanks for this patch, good to see that the relationship between the
>>>> USB Host and the PHY is described via DT.
>>>> This patch seems to cover USB0 and USB2 that both require special
>>>> control in the PHY. How about USB1? Can you explain about the reason
>>>> why you omit that?
>>> Because the driver does nothing for USB1 anyway.
>> Looks like I should have tested that last minute change: kernel oopses
>> due to NULL pointer dereference somewhere in phy_get() once it gets called
>> for EHCI on the channel #1. At least doesn't seem to be my mistake...
> No worries, thanks for looking into fixing that.
> Regarding the USB ports on R-Car Gen2 in general and especially USB1,
> it is my impression that even though there is no USB controller
> selection available for USB1 I still believe the UGCTL.CONNECT bit
> shall be used for power management purpose.
I don't think so. As I said the EHCI driver happily works without touching
this bit.
> I may of course be wrong, but since the PHY hardware is shared between
> USB0, USB1 and USB2
I believe it's a wrong impression (which probably my and Valentine's PHY
drivers have created). The PHY actually belongs to USBHS only; UGCTRL2
register is some kind of ad-hockery in this PHY.
> it makes sense to have some kind of usage counter
Generic PHY core already maintains such counters for phy_{init|exit}() and
phy_power_{on|off}() calls.
> and manage the hardware enable bit based on registered users somehow.
> There is no need to manage this bit at this point IMO, but in the
> future we may want to add such handling to improve power management.
I handle this bit but only for USBHS. What is actually necessary for all
USB controllers is enabling the USBHS clocks in order for UGCTRL2 to work.
> And that can only happen if DT is used to connect all USB controllers
> with the PHY, so please make sure to describe the complete
> dependencies in DT.
I cannnot represent USBHS in the device tree yet. I think I can represent
xHCI there but I was told it needs the firmware in order to work -- which we
don't have, so I'm not sure about xHCI testing.
> Thanks,
> / magnus
WBR, Sergei
^ permalink raw reply [flat|nested] 18+ messages in thread