Hi all,
Here's the device tree changes required for USB Host support on Tegra30 and
Tegra114. This enables USB Host on the Cardhu's dock connector port and on the
single built-in A-ports on Dalmore and Beaver,
Mikko Perttunen (1):
ARM: dts: USB for Tegra114 Dalmore
Tuomas Tynkkynen (1):
ARM: DTS: tegra: Add USB entries for Tegra30
arch/arm/boot/dts/tegra114-dalmore.dts | 9 ++++
arch/arm/boot/dts/tegra114.dtsi | 62 ++++++++++++++++++++++++
arch/arm/boot/dts/tegra30-beaver.dts | 23 ++++++++-
arch/arm/boot/dts/tegra30-cardhu.dtsi | 9 ++++
arch/arm/boot/dts/tegra30.dtsi | 86 ++++++++++++++++++++++++++++++++++
5 files changed, 187 insertions(+), 2 deletions(-)
--
1.8.1.5
Add device tree entries for the 3 USB controllers and PHYs and
enable the third controller on Cardhu and Beaver boards.
Fix VBUS regulator entries on Beaver. The GPIO pins were wrong.
Also, a third GPIO is required to power the pullup resistors that drive
the VBUS voltage switches, so add an regulator entry for that.
Signed-off-by: Tuomas Tynkkynen <[email protected]>
---
arch/arm/boot/dts/tegra30-beaver.dts | 23 +++++++++-
arch/arm/boot/dts/tegra30-cardhu.dtsi | 9 ++++
arch/arm/boot/dts/tegra30.dtsi | 86 +++++++++++++++++++++++++++++++++++
3 files changed, 116 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/tegra30-beaver.dts b/arch/arm/boot/dts/tegra30-beaver.dts
index 87c5f7b..414ea02 100644
--- a/arch/arm/boot/dts/tegra30-beaver.dts
+++ b/arch/arm/boot/dts/tegra30-beaver.dts
@@ -285,6 +285,15 @@
non-removable;
};
+ usb@7d008000 {
+ status = "okay";
+ };
+
+ usb-phy@7d008000 {
+ vbus-supply = <&usb3_vbus_reg>;
+ status = "okay";
+ };
+
clocks {
compatible = "simple-bus";
#address-cells = <1>;
@@ -357,7 +366,7 @@
regulator-min-microvolt = <5000000>;
regulator-max-microvolt = <5000000>;
enable-active-high;
- gpio = <&gpio TEGRA_GPIO(I, 4) GPIO_ACTIVE_HIGH>;
+ gpio = <&gpio TEGRA_GPIO(DD, 6) GPIO_ACTIVE_HIGH>;
gpio-open-drain;
vin-supply = <&vdd_5v_in_reg>;
};
@@ -369,7 +378,7 @@
regulator-min-microvolt = <5000000>;
regulator-max-microvolt = <5000000>;
enable-active-high;
- gpio = <&gpio TEGRA_GPIO(H, 7) GPIO_ACTIVE_HIGH>;
+ gpio = <&gpio TEGRA_GPIO(DD, 4) GPIO_ACTIVE_HIGH>;
gpio-open-drain;
vin-supply = <&vdd_5v_in_reg>;
};
@@ -399,6 +408,16 @@
gpio = <&gpio TEGRA_GPIO(L, 7) GPIO_ACTIVE_HIGH>;
vin-supply = <&sys_3v3_reg>;
};
+
+ usb13_vbus_pullup: regulator@8 {
+ compatible = "regulator-fixed";
+ reg = <8>;
+ regulator-name = "usb13_vbus_pullup";
+ regulator-always-on;
+ regulator-boot-on;
+ enable-active-high;
+ gpio = <&gpio TEGRA_GPIO(H, 5) GPIO_ACTIVE_HIGH>;
+ };
};
gpio-leds {
diff --git a/arch/arm/boot/dts/tegra30-cardhu.dtsi b/arch/arm/boot/dts/tegra30-cardhu.dtsi
index f65b53d..7af52e4 100644
--- a/arch/arm/boot/dts/tegra30-cardhu.dtsi
+++ b/arch/arm/boot/dts/tegra30-cardhu.dtsi
@@ -330,6 +330,15 @@
non-removable;
};
+ usb@7d008000 {
+ status = "okay";
+ };
+
+ usb-phy@7d008000 {
+ vbus-supply = <&usb3_vbus_reg>;
+ status = "okay";
+ };
+
clocks {
compatible = "simple-bus";
#address-cells = <1>;
diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index d8783f0..9920797 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -561,6 +561,92 @@
status = "disabled";
};
+ usb@7d000000 {
+ compatible = "nvidia,tegra30-ehci", "usb-ehci";
+ reg = <0x7d000000 0x4000>;
+ interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
+ phy_type = "utmi";
+ clocks = <&tegra_car TEGRA30_CLK_USBD>;
+ nvidia,needs-double-reset;
+ nvidia,phy = <&phy1>;
+ status = "disabled";
+ };
+
+ phy1: usb-phy@7d000000 {
+ compatible = "nvidia,tegra30-usb-phy";
+ reg = <0x7d000000 0x4000 0x7d000000 0x4000>;
+ phy_type = "utmi";
+ clocks = <&tegra_car TEGRA30_CLK_USBD>,
+ <&tegra_car TEGRA30_CLK_PLL_U>,
+ <&tegra_car TEGRA30_CLK_USBD>;
+ clock-names = "reg", "pll_u", "utmi-pads";
+ nvidia,hssync-start-delay = <9>;
+ nvidia,idle-wait-delay = <17>;
+ nvidia,elastic-limit = <16>;
+ nvidia,term-range-adj = <6>;
+ nvidia,xcvr-setup = <51>;
+ nvidia.xcvr-setup-use-fuses;
+ nvidia,xcvr-lsfslew = <1>;
+ nvidia,xcvr-lsrslew = <1>;
+ nvidia,xcvr-hsslew = <32>;
+ nvidia,hssquelch-level = <2>;
+ nvidia,hsdiscon-level = <5>;
+ status = "disabled";
+ };
+
+ usb@7d004000 {
+ compatible = "nvidia,tegra30-ehci", "usb-ehci";
+ reg = <0x7d004000 0x4000>;
+ interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
+ phy_type = "ulpi";
+ clocks = <&tegra_car TEGRA30_CLK_USB2>;
+ nvidia,phy = <&phy2>;
+ status = "disabled";
+ };
+
+ phy2: usb-phy@7d004000 {
+ compatible = "nvidia,tegra30-usb-phy";
+ reg = <0x7d004000 0x4000>;
+ phy_type = "ulpi";
+ clocks = <&tegra_car TEGRA30_CLK_USB2>,
+ <&tegra_car TEGRA30_CLK_PLL_U>,
+ <&tegra_car TEGRA30_CLK_CDEV2>;
+ clock-names = "reg", "pll_u", "ulpi-link";
+ status = "disabled";
+ };
+
+ usb@7d008000 {
+ compatible = "nvidia,tegra30-ehci", "usb-ehci";
+ reg = <0x7d008000 0x4000>;
+ interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>;
+ phy_type = "utmi";
+ clocks = <&tegra_car TEGRA30_CLK_USB3>;
+ nvidia,phy = <&phy3>;
+ status = "disabled";
+ };
+
+ phy3: usb-phy@7d008000 {
+ compatible = "nvidia,tegra30-usb-phy";
+ reg = <0x7d008000 0x4000 0x7d000000 0x4000>;
+ phy_type = "utmi";
+ clocks = <&tegra_car TEGRA30_CLK_USB3>,
+ <&tegra_car TEGRA30_CLK_PLL_U>,
+ <&tegra_car TEGRA30_CLK_USBD>;
+ clock-names = "reg", "pll_u", "utmi-pads";
+ nvidia,hssync-start-delay = <0>;
+ nvidia,idle-wait-delay = <17>;
+ nvidia,elastic-limit = <16>;
+ nvidia,term-range-adj = <6>;
+ nvidia,xcvr-setup = <51>;
+ nvidia.xcvr-setup-use-fuses;
+ nvidia,xcvr-lsfslew = <2>;
+ nvidia,xcvr-lsrslew = <2>;
+ nvidia,xcvr-hsslew = <32>;
+ nvidia,hssquelch-level = <2>;
+ nvidia,hsdiscon-level = <5>;
+ status = "disabled";
+ };
+
cpus {
#address-cells = <1>;
#size-cells = <0>;
--
1.8.1.5
From: Mikko Perttunen <[email protected]>
Device tree entries for the three EHCI controllers on Tegra114.
Enables the the third controller (USB host) on Dalmore.
Signed-off-by: Mikko Perttunen <[email protected]>
---
arch/arm/boot/dts/tegra114-dalmore.dts | 9 +++++
arch/arm/boot/dts/tegra114.dtsi | 62 ++++++++++++++++++++++++++++++++++
2 files changed, 71 insertions(+)
diff --git a/arch/arm/boot/dts/tegra114-dalmore.dts b/arch/arm/boot/dts/tegra114-dalmore.dts
index cb640eb..f3fc2b0 100644
--- a/arch/arm/boot/dts/tegra114-dalmore.dts
+++ b/arch/arm/boot/dts/tegra114-dalmore.dts
@@ -870,6 +870,15 @@
non-removable;
};
+ usb@7d008000 {
+ status = "okay";
+ };
+
+ usb-phy@7d008000 {
+ status = "okay";
+ vbus-supply = <&usb3_vbus_reg>;
+ };
+
clocks {
compatible = "simple-bus";
#address-cells = <1>;
diff --git a/arch/arm/boot/dts/tegra114.dtsi b/arch/arm/boot/dts/tegra114.dtsi
index abf6c40..2905145 100644
--- a/arch/arm/boot/dts/tegra114.dtsi
+++ b/arch/arm/boot/dts/tegra114.dtsi
@@ -430,6 +430,68 @@
status = "disable";
};
+ usb@7d000000 {
+ compatible = "nvidia,tegra30-ehci", "usb-ehci";
+ reg = <0x7d000000 0x4000>;
+ interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
+ phy_type = "utmi";
+ clocks = <&tegra_car TEGRA114_CLK_USBD>;
+ nvidia,phy = <&phy1>;
+ status = "disabled";
+ };
+
+ phy1: usb-phy@7d000000 {
+ compatible = "nvidia,tegra30-usb-phy";
+ reg = <0x7d000000 0x4000 0x7d000000 0x4000>;
+ phy_type = "utmi";
+ clocks = <&tegra_car TEGRA114_CLK_USBD>,
+ <&tegra_car TEGRA114_CLK_PLL_U>,
+ <&tegra_car TEGRA114_CLK_USBD>;
+ clock-names = "reg", "pll_u", "utmi-pads";
+ nvidia,hssync-start-delay = <0>;
+ nvidia,idle-wait-delay = <17>;
+ nvidia,elastic-limit = <16>;
+ nvidia,term-range-adj = <6>;
+ nvidia,xcvr-setup = <9>;
+ nvidia,xcvr-lsfslew = <0>;
+ nvidia,xcvr-lsrslew = <3>;
+ nvidia,hssquelch-level = <2>;
+ nvidia,hsdiscon-level = <5>;
+ nvidia,xcvr-hsslew = <12>;
+ status = "disabled";
+ };
+
+ usb@7d008000 {
+ compatible = "nvidia,tegra30-ehci", "usb-ehci";
+ reg = <0x7d008000 0x4000>;
+ interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>;
+ phy_type = "utmi";
+ clocks = <&tegra_car TEGRA114_CLK_USB3>;
+ nvidia,phy = <&phy3>;
+ status = "disabled";
+ };
+
+ phy3: usb-phy@7d008000 {
+ compatible = "nvidia,tegra30-usb-phy";
+ reg = <0x7d008000 0x4000 0x7d000000 0x4000>;
+ phy_type = "utmi";
+ clocks = <&tegra_car TEGRA114_CLK_USB3>,
+ <&tegra_car TEGRA114_CLK_PLL_U>,
+ <&tegra_car TEGRA114_CLK_USBD>;
+ clock-names = "reg", "pll_u", "utmi-pads";
+ nvidia,hssync-start-delay = <0>;
+ nvidia,idle-wait-delay = <17>;
+ nvidia,elastic-limit = <16>;
+ nvidia,term-range-adj = <6>;
+ nvidia,xcvr-setup = <9>;
+ nvidia,xcvr-lsfslew = <0>;
+ nvidia,xcvr-lsrslew = <3>;
+ nvidia,hssquelch-level = <2>;
+ nvidia,hsdiscon-level = <5>;
+ nvidia,xcvr-hsslew = <12>;
+ status = "disabled";
+ };
+
cpus {
#address-cells = <1>;
#size-cells = <0>;
--
1.8.1.5
Hello.
On 07/31/2013 09:42 PM, Tuomas Tynkkynen wrote:
> From: Mikko Perttunen <[email protected]>
> Device tree entries for the three EHCI controllers on Tegra114.
> Enables the the third controller (USB host) on Dalmore.
I would have done the board patch separately from the SoC one.
> Signed-off-by: Mikko Perttunen <[email protected]>
[...]
> diff --git a/arch/arm/boot/dts/tegra114.dtsi b/arch/arm/boot/dts/tegra114.dtsi
> index abf6c40..2905145 100644
> --- a/arch/arm/boot/dts/tegra114.dtsi
> +++ b/arch/arm/boot/dts/tegra114.dtsi
> @@ -430,6 +430,68 @@
> status = "disable";
> };
>
> + usb@7d000000 {
> + compatible = "nvidia,tegra30-ehci", "usb-ehci";
> + reg = <0x7d000000 0x4000>;
> + interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
> + phy_type = "utmi";
> + clocks = <&tegra_car TEGRA114_CLK_USBD>;
> + nvidia,phy = <&phy1>;
> + status = "disabled";
> + };
> +
> + phy1: usb-phy@7d000000 {
At the same address as the previous node?
> + compatible = "nvidia,tegra30-usb-phy";
> + reg = <0x7d000000 0x4000 0x7d000000 0x4000>;
Hm, there must be some mistake: two similar register ranges.
> + usb@7d008000 {
> + compatible = "nvidia,tegra30-ehci", "usb-ehci";
> + reg = <0x7d008000 0x4000>;
> + interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>;
> + phy_type = "utmi";
> + clocks = <&tegra_car TEGRA114_CLK_USB3>;
> + nvidia,phy = <&phy3>;
> + status = "disabled";
> + };
> +
> + phy3: usb-phy@7d008000 {
Again at the same address as previous node?
> + compatible = "nvidia,tegra30-usb-phy";
> + reg = <0x7d008000 0x4000 0x7d000000 0x4000>;
Second range conflicts with previous nodes.
WBR, Sergei
Hello,
On 31/07/13 21:18, Sergei Shtylyov wrote:
> Hello.
>
> On 07/31/2013 09:42 PM, Tuomas Tynkkynen wrote:
>
>> From: Mikko Perttunen <[email protected]>
>
>> Device tree entries for the three EHCI controllers on Tegra114.
>> Enables the the third controller (USB host) on Dalmore.
>
> I would have done the board patch separately from the SoC one.
>
>> Signed-off-by: Mikko Perttunen <[email protected]>
> [...]
>
>> diff --git a/arch/arm/boot/dts/tegra114.dtsi
>> b/arch/arm/boot/dts/tegra114.dtsi
>> index abf6c40..2905145 100644
>> --- a/arch/arm/boot/dts/tegra114.dtsi
>> +++ b/arch/arm/boot/dts/tegra114.dtsi
>> @@ -430,6 +430,68 @@
>> status = "disable";
>> };
>>
>> + usb@7d000000 {
>> + compatible = "nvidia,tegra30-ehci", "usb-ehci";
>> + reg = <0x7d000000 0x4000>;
>> + interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
>> + phy_type = "utmi";
>> + clocks = <&tegra_car TEGRA114_CLK_USBD>;
>> + nvidia,phy = <&phy1>;
>> + status = "disabled";
>> + };
>> +
>> + phy1: usb-phy@7d000000 {
>
> At the same address as the previous node?
Yes. The first node is for the EHCI driver and the second for the PHY
driver. There is some overlap in the exact registers used, so both
drives map the whole USB controller block.
>
>> + compatible = "nvidia,tegra30-usb-phy";
>> + reg = <0x7d000000 0x4000 0x7d000000 0x4000>;
>
> Hm, there must be some mistake: two similar register ranges.
The second range is used to configure the UTMI pad registers. All the
UTMI pad registers are located in the first USB controller's range.
>> + usb@7d008000 {
>> + compatible = "nvidia,tegra30-ehci", "usb-ehci";
>> + reg = <0x7d008000 0x4000>;
>> + interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>;
>> + phy_type = "utmi";
>> + clocks = <&tegra_car TEGRA114_CLK_USB3>;
>> + nvidia,phy = <&phy3>;
>> + status = "disabled";
>> + };
>> +
>> + phy3: usb-phy@7d008000 {
>
> Again at the same address as previous node?
>
>> + compatible = "nvidia,tegra30-usb-phy";
>> + reg = <0x7d008000 0x4000 0x7d000000 0x4000>;
>
> Second range conflicts with previous nodes.
>
> WBR, Sergei
All these entries mapping the same address are expected. See
arch/arm/boot/dts/tegra20.dtsi for an existing example of Tegra USB
bindings.
Hello.
On 07/31/2013 11:31 PM, Tuomas Tynkkynen wrote:
>>> From: Mikko Perttunen <[email protected]>
>>> Device tree entries for the three EHCI controllers on Tegra114.
>>> Enables the the third controller (USB host) on Dalmore.
>> I would have done the board patch separately from the SoC one.
>>> Signed-off-by: Mikko Perttunen <[email protected]>
>> [...]
>>
>>> diff --git a/arch/arm/boot/dts/tegra114.dtsi
>>> b/arch/arm/boot/dts/tegra114.dtsi
>>> index abf6c40..2905145 100644
>>> --- a/arch/arm/boot/dts/tegra114.dtsi
>>> +++ b/arch/arm/boot/dts/tegra114.dtsi
>>> @@ -430,6 +430,68 @@
>>> status = "disable";
>>> };
>>>
>>> + usb@7d000000 {
>>> + compatible = "nvidia,tegra30-ehci", "usb-ehci";
>>> + reg = <0x7d000000 0x4000>;
>>> + interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
>>> + phy_type = "utmi";
>>> + clocks = <&tegra_car TEGRA114_CLK_USBD>;
>>> + nvidia,phy = <&phy1>;
>>> + status = "disabled";
>>> + };
>>> +
>>> + phy1: usb-phy@7d000000 {
>> At the same address as the previous node?
> Yes. The first node is for the EHCI driver and the second for the PHY driver.
> There is some overlap in the exact registers used, so both drives map the
> whole USB controller block.
That's really horrible design.
>>> + compatible = "nvidia,tegra30-usb-phy";
>>> + reg = <0x7d000000 0x4000 0x7d000000 0x4000>;
>> Hm, there must be some mistake: two similar register ranges.
> The second range is used to configure the UTMI pad registers. All the UTMI pad
> registers are located in the first USB controller's range.
Which second range? This is one and the same range.
>>> + usb@7d008000 {
>>> + compatible = "nvidia,tegra30-ehci", "usb-ehci";
>>> + reg = <0x7d008000 0x4000>;
>>> + interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>;
>>> + phy_type = "utmi";
>>> + clocks = <&tegra_car TEGRA114_CLK_USB3>;
>>> + nvidia,phy = <&phy3>;
>>> + status = "disabled";
>>> + };
>>> +
>>> + phy3: usb-phy@7d008000 {
>> Again at the same address as previous node?
>>> + compatible = "nvidia,tegra30-usb-phy";
>>> + reg = <0x7d008000 0x4000 0x7d000000 0x4000>;
>> Second range conflicts with previous nodes.
Are you sure it's valid? Are you sure it shouldn't be 0x7d008000?
>> WBR, Sergei
> All these entries mapping the same address are expected. See
> arch/arm/boot/dts/tegra20.dtsi for an existing example of Tegra USB bindings.
Don't they cause numerous resource conflicts while device nodes being
instantiated as the platform devices?
WBR, Sergei
On 07/31/2013 01:53 PM, Sergei Shtylyov wrote:
> Hello.
>
> On 07/31/2013 11:31 PM, Tuomas Tynkkynen wrote:
>>>> Device tree entries for the three EHCI controllers on Tegra114.
>>>> Enables the the third controller (USB host) on Dalmore.
>>>> diff --git a/arch/arm/boot/dts/tegra114.dtsi
>>>> b/arch/arm/boot/dts/tegra114.dtsi
>>>> index abf6c40..2905145 100644
>>>> --- a/arch/arm/boot/dts/tegra114.dtsi
>>>> +++ b/arch/arm/boot/dts/tegra114.dtsi
>>>> @@ -430,6 +430,68 @@
>>>> status = "disable";
>>>> };
>>>>
>>>> + usb@7d000000 {
>>>> + compatible = "nvidia,tegra30-ehci", "usb-ehci";
>>>> + reg = <0x7d000000 0x4000>;
>>>> + interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
>>>> + phy_type = "utmi";
>>>> + clocks = <&tegra_car TEGRA114_CLK_USBD>;
>>>> + nvidia,phy = <&phy1>;
>>>> + status = "disabled";
>>>> + };
>>>> +
>>>> + phy1: usb-phy@7d000000 {
>
>>> At the same address as the previous node?
>
>> Yes. The first node is for the EHCI driver and the second for the PHY
>> driver.
>> There is some overlap in the exact registers used, so both drives map the
>> whole USB controller block.
>
> That's really horrible design.
Yup. Both USB PHY and EHCI controller registers really are interleaved
in one range.
>>>> + compatible = "nvidia,tegra30-usb-phy";
>>>> + reg = <0x7d000000 0x4000 0x7d000000 0x4000>;
>
>>> Hm, there must be some mistake: two similar register ranges.
>
>> The second range is used to configure the UTMI pad registers. All the
>> UTMI pad
>> registers are located in the first USB controller's range.
>
> Which second range? This is one and the same range.
Some registers in the USB1 register range actually are "global" and
relevant to all USB controllers. So:
There are two 2-cell entries in that reg property. The first entry
defines the registers for this USB PHY, and hence is unique for each USB
PHY node. The second defines the registers for whichever USB PHY
contains various shared registers across multiple PHYs, so is expected
to be identical across all USB PHY DT nodes.
Yes, the HW design really is this screwy.
> Don't they cause numerous resource conflicts while device nodes being
> instantiated as the platform devices?
No; the driver knows that the HW is screwy and there's lots of
register-range sharing going on, so it simply maps the registers, rather
than reserving the physical address range and mapping it.
On 07/31/2013 11:42 AM, Tuomas Tynkkynen wrote:
> Add device tree entries for the 3 USB controllers and PHYs and
> enable the third controller on Cardhu and Beaver boards.
>
> Fix VBUS regulator entries on Beaver. The GPIO pins were wrong.
That much is correct.
> Also, a third GPIO is required to power the pullup resistors that drive
> the VBUS voltage switches, so add an regulator entry for that.
That is not true. GMI_AD13 / PH5 / 61 is a signal that pulls up all of
USB1 VBUS, USB3 VBUS, and GMI_AD15, which as far as I can tell is
entirely unrelated to USB in any way.
This part of the change should be removed. I know you mentioned that
downstream does this, but that means very little; there are many
mistakes there.
Aside from that, this patch looks fine.
On 08/01/2013 02:06 AM, Stephen Warren wrote:
>>>>> Device tree entries for the three EHCI controllers on Tegra114.
>>>>> Enables the the third controller (USB host) on Dalmore.
>>>>> diff --git a/arch/arm/boot/dts/tegra114.dtsi
>>>>> b/arch/arm/boot/dts/tegra114.dtsi
>>>>> index abf6c40..2905145 100644
>>>>> --- a/arch/arm/boot/dts/tegra114.dtsi
>>>>> +++ b/arch/arm/boot/dts/tegra114.dtsi
>>>>> @@ -430,6 +430,68 @@
>>>>> status = "disable";
>>>>> };
>>>>>
>>>>> + usb@7d000000 {
>>>>> + compatible = "nvidia,tegra30-ehci", "usb-ehci";
>>>>> + reg = <0x7d000000 0x4000>;
>>>>> + interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
>>>>> + phy_type = "utmi";
>>>>> + clocks = <&tegra_car TEGRA114_CLK_USBD>;
>>>>> + nvidia,phy = <&phy1>;
>>>>> + status = "disabled";
>>>>> + };
>>>>> +
>>>>> + phy1: usb-phy@7d000000 {
>>>> At the same address as the previous node?
>>> Yes. The first node is for the EHCI driver and the second for the PHY
>>> driver.
>>> There is some overlap in the exact registers used, so both drives map the
>>> whole USB controller block.
>> That's really horrible design.
> Yup. Both USB PHY and EHCI controller registers really are interleaved
> in one range.
But the standard EHCI register space has no holes IIRC, so they can't be
really that much interleaved as you're describing (unless you have some
non-standard registers of course)...
We just had a case of misinterpreting the EHCI/PHY register spaces as
interleaved (while in fact they weren't) on Renesas R-Car which I had to
resolve for 3.11.
>>>>> + compatible = "nvidia,tegra30-usb-phy";
>>>>> + reg = <0x7d000000 0x4000 0x7d000000 0x4000>;
>>>> Hm, there must be some mistake: two similar register ranges.
>>> The second range is used to configure the UTMI pad registers. All the
>>> UTMI pad
>>> registers are located in the first USB controller's range.
>> Which second range? This is one and the same range.
> Some registers in the USB1 register range actually are "global" and
> relevant to all USB controllers. So:
> There are two 2-cell entries in that reg property. The first entry
> defines the registers for this USB PHY, and hence is unique for each USB
> PHY node. The second defines the registers for whichever USB PHY
> contains various shared registers across multiple PHYs, so is expected
> to be identical across all USB PHY DT nodes.
Hm, couldn't you have those shared registers as a separate device?
> Yes, the HW design really is this screwy.
Ugh...
>> Don't they cause numerous resource conflicts while device nodes being
>> instantiated as the platform devices?
> No; the driver knows that the HW is screwy and there's lots of
> register-range sharing going on, so it simply maps the registers, rather
> than reserving the physical address range and mapping it.
Yes, it's clear that the driver should take special measures, I was asking
about the platform device creation phase. What do you see in /proc/iomem?
WBR, Sergei
On 07/31/2013 04:20 PM, Sergei Shtylyov wrote:
> On 08/01/2013 02:06 AM, Stephen Warren wrote:
...
>>> That's really horrible design.
>>
>> Yup. Both USB PHY and EHCI controller registers really are interleaved
>> in one range.
>
> But the standard EHCI register space has no holes IIRC, so they can't
> be really that much interleaved as you're describing (unless you have
> some non-standard registers of course)...
Yes, there are certainly non-standard registers.
...
>>> Don't they cause numerous resource conflicts while device nodes
>>> being
>>> instantiated as the platform devices?
>
>> No; the driver knows that the HW is screwy and there's lots of
>> register-range sharing going on, so it simply maps the registers, rather
>> than reserving the physical address range and mapping it.
>
> Yes, it's clear that the driver should take special measures, I was
> asking about the platform device creation phase. What do you see in
> /proc/iomem?
The drivers don't request the memory region since doing so would cause
conflicts. Hence, the regions don't show up in /proc/iomem.
This actually isn't that uncommon for DT-based drivers anyway; many use
e.g. of_iomap() which IIRC just looks up the resource and maps it
without registering the usage.
Hello.
On 01-08-2013 3:29, Stephen Warren wrote:
>>>> Don't they cause numerous resource conflicts while device nodes
>>>> being
>>>> instantiated as the platform devices?
>>
>>> No; the driver knows that the HW is screwy and there's lots of
>>> register-range sharing going on, so it simply maps the registers, rather
>>> than reserving the physical address range and mapping it.
>>
>> Yes, it's clear that the driver should take special measures, I was
>> asking about the platform device creation phase. What do you see in
>> /proc/iomem?
> The drivers don't request the memory region since doing so would cause
> conflicts. Hence, the regions don't show up in /proc/iomem.
Regions should show up after the platform device registration. They just
show up as subregions of those when you request them. That's why I asked for
your /proc/iomem.
WBR, Sergei
On 08/01/2013 01:13 AM, Stephen Warren wrote:
> On 07/31/2013 11:42 AM, Tuomas Tynkkynen wrote:
>> Add device tree entries for the 3 USB controllers and PHYs and
>> enable the third controller on Cardhu and Beaver boards.
>>
>> Fix VBUS regulator entries on Beaver. The GPIO pins were wrong.
>
> That much is correct.
>
>> Also, a third GPIO is required to power the pullup resistors that drive
>> the VBUS voltage switches, so add an regulator entry for that.
>
> That is not true. GMI_AD13 / PH5 / 61 is a signal that pulls up all of
> USB1 VBUS, USB3 VBUS, and GMI_AD15, which as far as I can tell is
> entirely unrelated to USB in any way.
I sent a v2 that enables the internal pullups on those pins.
> This part of the change should be removed. I know you mentioned that
> downstream does this, but that means very little; there are many
> mistakes there.
It looks like the pullup resistor from PH5 to GMI_AD15 was added in a later board revision,
so the HW guys are to blame this time.
On Wed, Jul 31, 2013 at 05:29:40PM -0600, Stephen Warren wrote:
> On 07/31/2013 04:20 PM, Sergei Shtylyov wrote:
> > On 08/01/2013 02:06 AM, Stephen Warren wrote:
> ...
> >>> That's really horrible design.
> >>
> >> Yup. Both USB PHY and EHCI controller registers really are interleaved
> >> in one range.
> >
> > But the standard EHCI register space has no holes IIRC, so they can't
> > be really that much interleaved as you're describing (unless you have
> > some non-standard registers of course)...
>
> Yes, there are certainly non-standard registers.
>
> ...
> >>> Don't they cause numerous resource conflicts while device nodes
> >>> being
> >>> instantiated as the platform devices?
> >
> >> No; the driver knows that the HW is screwy and there's lots of
> >> register-range sharing going on, so it simply maps the registers, rather
> >> than reserving the physical address range and mapping it.
> >
> > Yes, it's clear that the driver should take special measures, I was
> > asking about the platform device creation phase. What do you see in
> > /proc/iomem?
>
> The drivers don't request the memory region since doing so would cause
> conflicts. Hence, the regions don't show up in /proc/iomem.
>
> This actually isn't that uncommon for DT-based drivers anyway; many use
> e.g. of_iomap() which IIRC just looks up the resource and maps it
> without registering the usage.
Not being uncommon isn't a good argument. The problem with doing this is
that it sets a bad example and makes it easier for others to do the same
thing. I can see that for some drivers providing a proper abstraction or
encapsulation might be more complicated than necessary. But I've also
seen this kind of shortcut taken quite often lately and especially often
in DT-based drivers.
Am I the only one concerned about this development?
Thierry