[PATCH v2] ARM: shmobile: r8a7790: link PCI USB devices to USB PHY

[Sergei Shtylyov]

Hello.

On 05/29/2014 01:51 AM, Sergei Shtylyov 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 at codethink.co.uk>.

>>>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov at cogentembedded.com>

>>>>> ---
>>>>> This patch is against 'renesas-devel-v3.15-rc7-20140526' tag of Simon
>>>>> Horman's
>>>>> 'renesas.git' repo plus R8A7790/Lager PCI and USB PHY support patches
>>>>> posted
>>>>> before. The patch requires the internal PCI DT support, USB PHY driver,
>>>>> and
>>>>> USB HCD generic PHY support (also already posted) in order to work.

>>>>> Changes in version 2:
>>>>> - renamed the PCI OHCI/EHCI device nodes to comply with the PCI binding;
>>>>> - changed the PHY specifier in the PCI#2 node to reflect that channel #1
>>>>> support
>>>>>     was dropped;
>>>>> - resolved rejects, refreshed the patch.

>>>>>    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
>>>>> @@ -919,6 +919,20 @@
>>>>>                   interrupt-map = <0x0000 0 0 1 &gic 0 108
>>>>> IRQ_TYPE_LEVEL_HIGH
>>>>>                                    0x0800 0 0 1 &gic 0 108
>>>>> IRQ_TYPE_LEVEL_HIGH
>>>>>                                    0x1000 0 0 2 &gic 0 108
>>>>> IRQ_TYPE_LEVEL_HIGH>;
>>>>> +
>>>>> +               usb at 0,1 {
>>>>> +                       reg = <0x800 0 0 0 0>;
>>>>> +                       device_type = "pci";
>>>>> +                       phys = <&usbphy 0 0>;
>>>>> +                       phy-names = "usb";
>>>>> +               };
>>>>> +
>>>>> +               usb at 0,2 {
>>>>> +                       reg = <0x1000 0 0 0 0>;
>>>>> +                       device_type = "pci";
>>>>> +                       phys = <&usbphy 0 0>;
>>>>> +                       phy-names = "usb";
>>>>> +               };
>>>>>           };
>>>>>
>>>>>           pci1: pci at ee0b0000 {
>>>>> @@ -955,5 +969,19 @@
>>>>>                   interrupt-map = <0x0000 0 0 1 &gic 0 113
>>>>> IRQ_TYPE_LEVEL_HIGH
>>>>>                                    0x0800 0 0 1 &gic 0 113
>>>>> IRQ_TYPE_LEVEL_HIGH
>>>>>                                    0x1000 0 0 2 &gic 0 113
>>>>> IRQ_TYPE_LEVEL_HIGH>;
>>>>> +
>>>>> +               usb at 0,1 {
>>>>> +                       reg = <0x800 0 0 0 0>;
>>>>> +                       device_type = "pci";
>>>>> +                       phys = <&usbphy 1 0>;
>>>>> +                       phy-names = "usb";
>>>>> +               };
>>>>> +
>>>>> +               usb at 0,2 {
>>>>> +                       reg = <0x1000 0 0 0 0>;
>>>>> +                       device_type = "pci";
>>>>> +                       phys = <&usbphy 1 0>;
>>>>> +                       phy-names = "usb";
>>>>> +               };
>>>>>           };
>>>>>    };

[...]

>> I know we've discussed this before, and for USB channel #1 you tend to
>> say that the PHY is not necessary for operation. From my side I am not
>> so sure. Over the months I seen several shortcuts and missing pieces
>> so for now just assume this is another rushed attempt. As usual I
>> would like to make sure the dependencies are correctly described in DT
>> before merging anything.

>> Like I mentioned earlier there are shared bits in UGCTRL register in

>     Did you mean UGCTRL or UGCTRL2 register? My testing didn't prove that the
> UGCTRL bits (CONNECT and PLLRESET) affect anything but the USBHS controller --
> my PHY driver only touches these bits if the PHY selected corresponds to the
> USBHS itself (which is not supported by DT yet)...

>> the PHY. Those may or may not affect USB channel #1. Furthermore, on
>> top of this we have the MSTP bit for the PHY that may or may not be
>> related to USB channel #1.

>     There is no MSTP bit for the PHY, there's MSTP bit for USBHS controller of
> which this PHY is a part.

>> My opinion is that if the USB PHY requires the MSTP clock to be
>> enabled when using USB channel #1 _or_ the UGCTRL bits affect the USB
>> Channel #1 then DT needs to point out the relationship between the PHY
>> and the USB Host

>     I agree and I can conduct some additional tests only linking the USB
> channel #1 to the PHY driver, and hence enabling the USBHS clock only for it...

    OK, I've done additional testing and USB channel #1 on R8A7790 works 
regardless of the USBHS clock being enabled or not.

>> Thanks,
>> / magnus

WBR, Sergei