Patchwork [U-Boot,1/5] Tegra30: fdt: Add SDMMC (sdhci) nodes for T30 boards (Cardhu for now)

login
register
mail settings
Submitter Tom Warren
Date Feb. 26, 2013, 8:46 p.m.
Message ID <1361911596-16518-2-git-send-email-twarren@nvidia.com>
Download mbox | patch
Permalink /patch/223389/
State Superseded
Delegated to: Tom Warren
Headers show

Comments

Tom Warren - Feb. 26, 2013, 8:46 p.m.
Took these values directly from the kernel dts files.

Signed-off-by: Tom Warren <twarren@nvidia.com>
---
 arch/arm/dts/tegra30.dtsi           |   32 ++++++++++++++++++++++++++++++++
 board/nvidia/dts/tegra30-cardhu.dts |   15 +++++++++++++++
 2 files changed, 47 insertions(+), 0 deletions(-)
Stephen Warren - Feb. 26, 2013, 11:10 p.m.
On 02/26/2013 01:46 PM, Tom Warren wrote:
> Took these values directly from the kernel dts files.

> diff --git a/arch/arm/dts/tegra30.dtsi b/arch/arm/dts/tegra30.dtsi

> +	sdhci@78000000 {
> +		compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";

Looking at this more, I /think/ this should only include the Tegra30
compatible value, since there are new quirks that are required to be
enabled on Tegra30 relative to Tegra20 or the HW won't work. The kernel
DT file is no doubt buggy here.

Cc'ing Rhyland and Pavan to confirm this. (Note: this is
Tegra30-vs-Tegra20, not Tegra114-vs-Tegra30 that we just discussed
downstream).
Tom Warren - Feb. 27, 2013, 4:20 p.m.
Stephen/Rhyland,

On Tue, Feb 26, 2013 at 4:10 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 02/26/2013 01:46 PM, Tom Warren wrote:
>> Took these values directly from the kernel dts files.
>
>> diff --git a/arch/arm/dts/tegra30.dtsi b/arch/arm/dts/tegra30.dtsi
>
>> +     sdhci@78000000 {
>> +             compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
>
> Looking at this more, I /think/ this should only include the Tegra30
> compatible value, since there are new quirks that are required to be
> enabled on Tegra30 relative to Tegra20 or the HW won't work. The kernel
> DT file is no doubt buggy here.
Looking at the SDMMC reg space in the T20 and T30 TRMs, I don't see
anything major that would make the MMC driver not work on T30 as is
(in fact, I know it works just fine w/o modification). Looking at the
sdhci-tegra.c driver source, the only quirk difference is
DATA_TIMEOUT_USES_SDCLK. The U-Boot Tegra MMC driver doesn't reference
the caps Timeout Clock Frequency bits, so this quirk/difference
doesn't matter.

>
> Cc'ing Rhyland and Pavan to confirm this. (Note: this is
> Tegra30-vs-Tegra20, not Tegra114-vs-Tegra30 that we just discussed
> downstream).
Let's see what Rhyland/Pavan have to say before I change this.

Tom
Stephen Warren - Feb. 27, 2013, 6:02 p.m.
On 02/27/2013 09:20 AM, Tom Warren wrote:
> Stephen/Rhyland,
> 
> On Tue, Feb 26, 2013 at 4:10 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 02/26/2013 01:46 PM, Tom Warren wrote:
>>> Took these values directly from the kernel dts files.
>>
>>> diff --git a/arch/arm/dts/tegra30.dtsi b/arch/arm/dts/tegra30.dtsi
>>
>>> +     sdhci@78000000 {
>>> +             compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
>>
>> Looking at this more, I /think/ this should only include the Tegra30
>> compatible value, since there are new quirks that are required to be
>> enabled on Tegra30 relative to Tegra20 or the HW won't work. The kernel
>> DT file is no doubt buggy here.
>
> Looking at the SDMMC reg space in the T20 and T30 TRMs, I don't see
> anything major that would make the MMC driver not work on T30 as is
> (in fact, I know it works just fine w/o modification). Looking at the
> sdhci-tegra.c driver source, the only quirk difference is
> DATA_TIMEOUT_USES_SDCLK. The U-Boot Tegra MMC driver doesn't reference
> the caps Timeout Clock Frequency bits, so this quirk/difference
> doesn't matter.

The compatible value does not document whether the U-Boot driver will
operate on the HW without changes, but rather whether there are
incompatible HW changes.

Now, those changes might only affect some theoretical driver that
doesn't actually exist. However, that is not relevant; compatible is
purely about describing HW compatibility or not.

I'll note that both the current upstream U-Boot MMC driver and likely
the current upstream Linux kernel MMC driver probably don't take
advantage of many of the performance or power-saving features present in
the HW, so it's likely pretty easy for their to be a HW difference that
could affect a driver, but not actually yet affect either of these two
particular drivers.

In particular, the difference in DATA_TIMEOUT_USES_SDCLK would probably
be enough on its own to merit not marking Tegra30 as
backwards-compatible with Tegra20, since it's a bug WAR or issue that a
Tegra30 driver apparently would need to be aware of but not a Tegra20
driver (if that feature is used by the driver).

Patch

diff --git a/arch/arm/dts/tegra30.dtsi b/arch/arm/dts/tegra30.dtsi
index 9483e80..834d617 100644
--- a/arch/arm/dts/tegra30.dtsi
+++ b/arch/arm/dts/tegra30.dtsi
@@ -184,4 +184,36 @@ 
 		clocks = <&tegra_car 105>;
 		status = "disabled";
 	};
+
+	sdhci@78000000 {
+		compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
+		reg = <0x78000000 0x200>;
+		interrupts = <0 14 0x04>;
+		clocks = <&tegra_car 14>;
+		status = "disabled";
+	};
+
+	sdhci@78000200 {
+		compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
+		reg = <0x78000200 0x200>;
+		interrupts = <0 15 0x04>;
+		clocks = <&tegra_car 9>;
+		status = "disabled";
+	};
+
+	sdhci@78000400 {
+		compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
+		reg = <0x78000400 0x200>;
+		interrupts = <0 19 0x04>;
+		clocks = <&tegra_car 69>;
+		status = "disabled";
+	};
+
+	sdhci@78000600 {
+		compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
+		reg = <0x78000600 0x200>;
+		interrupts = <0 31 0x04>;
+		clocks = <&tegra_car 15>;
+		status = "disabled";
+	};
 };
diff --git a/board/nvidia/dts/tegra30-cardhu.dts b/board/nvidia/dts/tegra30-cardhu.dts
index 48039c9..4d22b48 100644
--- a/board/nvidia/dts/tegra30-cardhu.dts
+++ b/board/nvidia/dts/tegra30-cardhu.dts
@@ -12,6 +12,8 @@ 
 		i2c2 = "/i2c@7000c400";
 		i2c3 = "/i2c@7000c500";
 		i2c4 = "/i2c@7000c700";
+		sdhci0 = "/sdhci@78000600";
+		sdhci1 = "/sdhci@78000000";
 	};
 
 	memory {
@@ -48,4 +50,17 @@ 
 		status = "okay";
 		spi-max-frequency = <25000000>;
 	};
+
+	sdhci@78000000 {
+		status = "okay";
+		cd-gpios = <&gpio 69 1>; /* gpio PI5 */
+		wp-gpios = <&gpio 155 0>; /* gpio PT3 */
+		power-gpios = <&gpio 31 0>; /* gpio PD7 */
+		bus-width = <4>;
+	};
+
+	sdhci@78000600 {
+		status = "okay";
+		bus-width = <8>;
+	};
 };