Hi all,
This series implements support for HS400 signaling on Tegra210 and
Tegra186. This includes programming the DQS trimmer values, implementing
enhanced strobe and HS400 delay line calibration.
This series depends on the "Tegra SDHCI add support for HS200 and UHS
signaling" series.
Aapo Vienamo (8):
dt-bindings: mmc: Add DQS trim value to Tegra SDHCI
mmc: tegra: Parse and program DQS trim value
mmc: tegra: Implement HS400 enhanced strobe
mmc: tegra: Implement HS400 delay line calibration
arm64: dts: tegra186: Add SDMMC4 DQS trim value
arm64: dts: tegra210: Add SDMMC4 DQS trim value
arm64: dts: tegra186: Enable HS400
arm64: dts: tegra210: Enable HS400
.../bindings/mmc/nvidia,tegra20-sdhci.txt | 3 +
arch/arm64/boot/dts/nvidia/tegra186.dtsi | 2 +
arch/arm64/boot/dts/nvidia/tegra210.dtsi | 2 +
drivers/mmc/host/sdhci-tegra.c | 83 +++++++++++++++++++++-
4 files changed, 87 insertions(+), 3 deletions(-)
--
2.7.4
Document HS400 DQS trim value device tree property.
Signed-off-by: Aapo Vienamo <[email protected]>
---
Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
index 3c7960a..7d294f3 100644
--- a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
+++ b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
@@ -72,6 +72,7 @@ Optional properties for Tegra210 and Tegra186:
trimmer value for non-tunable modes.
- nvidia,default-trim : Specify the default outbound clock trimmer
value.
+- nvidia,dqs-trim : Specify DQS trim value for HS400 timing
Notes on the pad calibration pull up and pulldown offset values:
- The property values are drive codes which are programmed into the
@@ -88,6 +89,8 @@ Optional properties for Tegra210 and Tegra186:
- The values are programmed to the Vendor Clock Control Register.
Please refer to the reference manual of the SoC for correct
values.
+ - The DQS trim values are only used on controllers which support
+ HS400 timing.
Example:
sdhci@700b0000 {
--
2.7.4
Enable HS400 signaling on Tegra186 SDMMC4 controller.
Signed-off-by: Aapo Vienamo <[email protected]>
---
arch/arm64/boot/dts/nvidia/tegra186.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index 9e07bc6..2f3c8e2 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -314,6 +314,7 @@
nvidia,default-tap = <0x5>;
nvidia,default-trim = <0x9>;
nvidia,dqs-trim = <63>;
+ mmc-hs400-1_8v;
status = "disabled";
};
--
2.7.4
Implement HS400 enhanced strobe.
Signed-off-by: Aapo Vienamo <[email protected]>
---
drivers/mmc/host/sdhci-tegra.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 426f7ea..d81143b 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -43,6 +43,9 @@
#define SDHCI_CLOCK_CTRL_PADPIPE_CLKEN_OVERRIDE BIT(3)
#define SDHCI_CLOCK_CTRL_SPI_MODE_CLKEN_OVERRIDE BIT(2)
+#define SDHCI_TEGRA_VENDOR_SYS_SW_CTRL 0x104
+#define SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE BIT(31)
+
#define SDHCI_TEGRA_VENDOR_CAP_OVERRIDES 0x10c
#define SDHCI_TEGRA_CAP_OVERRIDES_DQS_TRIM_MASK 0x00003f00
#define SDHCI_TEGRA_CAP_OVERRIDES_DQS_TRIM_SHIFT 8
@@ -271,6 +274,22 @@ static void tegra_sdhci_set_tap(struct sdhci_host *host, unsigned int tap)
}
}
+static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
+ struct mmc_ios *ios)
+{
+ struct sdhci_host *host = mmc_priv(mmc);
+ u32 reg;
+
+ reg = sdhci_readl(host, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
+ pr_err("enhanced strobe: %d\n", ios->enhanced_strobe);
+ if (ios->enhanced_strobe)
+ reg |= SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
+ else
+ reg &= ~SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
+ sdhci_writel(host, reg, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
+
+}
+
static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -947,6 +966,9 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
sdhci_tegra_start_signal_voltage_switch;
}
+ host->mmc_host_ops.hs400_enhanced_strobe =
+ tegra_sdhci_hs400_enhanced_strobe;
+
rc = mmc_of_parse(host->mmc);
if (rc)
goto err_parse_dt;
--
2.7.4
Implement HS400 specific delay line calibration procedure.
Signed-off-by: Aapo Vienamo <[email protected]>
---
drivers/mmc/host/sdhci-tegra.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index d81143b..d0b68b7 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -56,6 +56,12 @@
#define SDHCI_MISC_CTRL_ENABLE_SDHCI_SPEC_300 0x20
#define SDHCI_MISC_CTRL_ENABLE_DDR50 0x200
+#define SDHCI_TEGRA_VENDOR_DLLCAL_CFG 0x1b0
+#define SDHCI_TEGRA_DLLCAL_CALIBRATE BIT(31)
+
+#define SDHCI_TEGRA_VENDOR_DLLCAL_STA 0x1bc
+#define SDHCI_TEGRA_DLLCAL_STA_ACTIVE BIT(31)
+
#define SDHCI_VNDR_TUN_CTRL0_0 0x1c0
#define SDHCI_VNDR_TUN_CTRL0_TUN_HW_TAP 0x20000
@@ -584,6 +590,24 @@ static void tegra_sdhci_set_dqs_trim(struct sdhci_host *host, u8 val)
sdhci_writel(host, reg, SDHCI_TEGRA_VENDOR_CAP_OVERRIDES);
}
+static void tegra_sdhci_hs400_dll_cal(struct sdhci_host *host)
+{
+ u32 reg;
+ int err;
+
+ reg = sdhci_readl(host, SDHCI_TEGRA_VENDOR_DLLCAL_CFG);
+ reg |= SDHCI_TEGRA_DLLCAL_CALIBRATE;
+ sdhci_writel(host, reg, SDHCI_TEGRA_VENDOR_DLLCAL_CFG);
+
+ /* 1 ms sleep, 5 ms timeout */
+ err = readl_poll_timeout(host->ioaddr + SDHCI_TEGRA_VENDOR_DLLCAL_STA,
+ reg, !(reg & SDHCI_TEGRA_DLLCAL_STA_ACTIVE),
+ 1000, 5000);
+ if (err)
+ dev_err(mmc_dev(host->mmc),
+ "HS400 delay line calibration timed out\n");
+}
+
static void tegra_sdhci_set_uhs_signaling(struct sdhci_host *host,
unsigned timing)
{
@@ -591,6 +615,7 @@ static void tegra_sdhci_set_uhs_signaling(struct sdhci_host *host,
struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
bool set_default_tap = false;
bool set_dqs_trim = false;
+ bool do_hs400_dll_cal = false;
switch (timing) {
case MMC_TIMING_UHS_SDR50:
@@ -600,6 +625,7 @@ static void tegra_sdhci_set_uhs_signaling(struct sdhci_host *host,
break;
case MMC_TIMING_MMC_HS400:
set_dqs_trim = true;
+ do_hs400_dll_cal = true;
break;
case MMC_TIMING_MMC_DDR52:
case MMC_TIMING_UHS_DDR50:
@@ -620,6 +646,9 @@ static void tegra_sdhci_set_uhs_signaling(struct sdhci_host *host,
if (set_dqs_trim)
tegra_sdhci_set_dqs_trim(host, tegra_host->dqs_trim);
+
+ if (do_hs400_dll_cal)
+ tegra_sdhci_hs400_dll_cal(host);
}
static int tegra_sdhci_execute_tuning(struct sdhci_host *host, u32 opcode)
--
2.7.4
Add the HS400 DQS trim value for Tegra210 SDMMC4.
Signed-off-by: Aapo Vienamo <[email protected]>
---
arch/arm64/boot/dts/nvidia/tegra210.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index 14da98a..f8e5f09 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -1115,6 +1115,7 @@
assigned-clocks = <&tegra_car TEGRA210_CLK_SDMMC4>,
<&tegra_car TEGRA210_CLK_PLL_C4_OUT0>;
assigned-clock-parents = <&tegra_car TEGRA210_CLK_PLL_C4_OUT0>;
+ nvidia,dqs-trim = <40>;
status = "disabled";
};
--
2.7.4
Add the HS400 DQS trim value for Tegra186 SDMMC4.
Signed-off-by: Aapo Vienamo <[email protected]>
---
arch/arm64/boot/dts/nvidia/tegra186.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index 6e9ef26..9e07bc6 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -313,6 +313,7 @@
nvidia,pad-autocal-pull-down-offset-1v8-timeout = <0x0a>;
nvidia,default-tap = <0x5>;
nvidia,default-trim = <0x9>;
+ nvidia,dqs-trim = <63>;
status = "disabled";
};
--
2.7.4
Parse and program the HS400 DQS trim value from dt. Program a fallback
value in case the property is missing.
Signed-off-by: Aapo Vienamo <[email protected]>
---
drivers/mmc/host/sdhci-tegra.c | 32 +++++++++++++++++++++++++++++---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 7f1ac4a..426f7ea 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -43,6 +43,10 @@
#define SDHCI_CLOCK_CTRL_PADPIPE_CLKEN_OVERRIDE BIT(3)
#define SDHCI_CLOCK_CTRL_SPI_MODE_CLKEN_OVERRIDE BIT(2)
+#define SDHCI_TEGRA_VENDOR_CAP_OVERRIDES 0x10c
+#define SDHCI_TEGRA_CAP_OVERRIDES_DQS_TRIM_MASK 0x00003f00
+#define SDHCI_TEGRA_CAP_OVERRIDES_DQS_TRIM_SHIFT 8
+
#define SDHCI_TEGRA_VENDOR_MISC_CTRL 0x120
#define SDHCI_MISC_CTRL_ENABLE_SDR104 0x8
#define SDHCI_MISC_CTRL_ENABLE_SDR50 0x10
@@ -112,6 +116,7 @@ struct sdhci_tegra {
u32 default_tap;
u32 default_trim;
+ u32 dqs_trim;
};
static u16 tegra_sdhci_readw(struct sdhci_host *host, int reg)
@@ -500,7 +505,7 @@ static void tegra_sdhci_parse_pad_autocal_dt(struct sdhci_host *host)
autocal->pull_down_hs400 = autocal->pull_down_1v8;
}
-static void tegra_sdhci_parse_default_tap_and_trim(struct sdhci_host *host)
+static void tegra_sdhci_parse_tap_and_trim(struct sdhci_host *host)
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
@@ -515,6 +520,11 @@ static void tegra_sdhci_parse_default_tap_and_trim(struct sdhci_host *host)
&tegra_host->default_trim);
if (err)
tegra_host->default_trim = 0;
+
+ err = device_property_read_u32(host->mmc->parent, "nvidia,dqs-trim",
+ &tegra_host->dqs_trim);
+ if (err)
+ tegra_host->dqs_trim = 0x11;
}
static void tegra_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
@@ -545,20 +555,33 @@ static unsigned int tegra_sdhci_get_max_clock(struct sdhci_host *host)
return clk_round_rate(pltfm_host->clk, UINT_MAX);
}
+static void tegra_sdhci_set_dqs_trim(struct sdhci_host *host, u8 val)
+{
+ u32 reg;
+
+ reg = sdhci_readl(host, SDHCI_TEGRA_VENDOR_CAP_OVERRIDES);
+ reg &= ~SDHCI_TEGRA_CAP_OVERRIDES_DQS_TRIM_MASK;
+ reg |= val<<SDHCI_TEGRA_CAP_OVERRIDES_DQS_TRIM_SHIFT;
+ sdhci_writel(host, reg, SDHCI_TEGRA_VENDOR_CAP_OVERRIDES);
+}
+
static void tegra_sdhci_set_uhs_signaling(struct sdhci_host *host,
unsigned timing)
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
bool set_default_tap = false;
+ bool set_dqs_trim = false;
switch (timing) {
case MMC_TIMING_UHS_SDR50:
case MMC_TIMING_UHS_SDR104:
case MMC_TIMING_MMC_HS200:
- case MMC_TIMING_MMC_HS400:
/* Don't set default tap on tunable modes. */
break;
+ case MMC_TIMING_MMC_HS400:
+ set_dqs_trim = true;
+ break;
case MMC_TIMING_MMC_DDR52:
case MMC_TIMING_UHS_DDR50:
tegra_host->ddr_signaling = true;
@@ -575,6 +598,9 @@ static void tegra_sdhci_set_uhs_signaling(struct sdhci_host *host,
if (set_default_tap)
tegra_sdhci_set_tap(host, tegra_host->default_tap);
+
+ if (set_dqs_trim)
+ tegra_sdhci_set_dqs_trim(host, tegra_host->dqs_trim);
}
static int tegra_sdhci_execute_tuning(struct sdhci_host *host, u32 opcode)
@@ -930,7 +956,7 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
tegra_sdhci_parse_pad_autocal_dt(host);
- tegra_sdhci_parse_default_tap_and_trim(host);
+ tegra_sdhci_parse_tap_and_trim(host);
tegra_host->power_gpio = devm_gpiod_get_optional(&pdev->dev, "power",
GPIOD_OUT_HIGH);
--
2.7.4
Enable HS400 signaling on Tegra210 SDMMC4 controller.
Signed-off-by: Aapo Vienamo <[email protected]>
---
arch/arm64/boot/dts/nvidia/tegra210.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index f8e5f09..8fe47d6 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -1116,6 +1116,7 @@
<&tegra_car TEGRA210_CLK_PLL_C4_OUT0>;
assigned-clock-parents = <&tegra_car TEGRA210_CLK_PLL_C4_OUT0>;
nvidia,dqs-trim = <40>;
+ mmc-hs400-1_8v;
status = "disabled";
};
--
2.7.4
On Tue, Aug 07, 2018 at 04:59:57PM +0300, Aapo Vienamo wrote:
> Document HS400 DQS trim value device tree property.
>
> Signed-off-by: Aapo Vienamo <[email protected]>
> ---
> Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
> index 3c7960a..7d294f3 100644
> --- a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
> @@ -72,6 +72,7 @@ Optional properties for Tegra210 and Tegra186:
> trimmer value for non-tunable modes.
> - nvidia,default-trim : Specify the default outbound clock trimmer
> value.
> +- nvidia,dqs-trim : Specify DQS trim value for HS400 timing
>
> Notes on the pad calibration pull up and pulldown offset values:
> - The property values are drive codes which are programmed into the
> @@ -88,6 +89,8 @@ Optional properties for Tegra210 and Tegra186:
> - The values are programmed to the Vendor Clock Control Register.
> Please refer to the reference manual of the SoC for correct
> values.
> + - The DQS trim values are only used on controllers which support
> + HS400 timing.
One of these additions says "DQS trim values", the other says "DQS trim
value". It is unclear from the above how many values there are. I think
this should be more explicit. Also, I don't see why the note about which
controllers the DQS trim value(s) applies to is in a separate paragraph.
Couldn't it be moved to the property description?
Also, I think the bindings should specify which generations of Tegra do
support HS400. Where else are people supposed to find that information?
Thierry
On Tue, Aug 07, 2018 at 04:59:58PM +0300, Aapo Vienamo wrote:
> Parse and program the HS400 DQS trim value from dt. Program a fallback
> value in case the property is missing.
>
> Signed-off-by: Aapo Vienamo <[email protected]>
> ---
> drivers/mmc/host/sdhci-tegra.c | 32 +++++++++++++++++++++++++++++---
> 1 file changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index 7f1ac4a..426f7ea 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -43,6 +43,10 @@
> #define SDHCI_CLOCK_CTRL_PADPIPE_CLKEN_OVERRIDE BIT(3)
> #define SDHCI_CLOCK_CTRL_SPI_MODE_CLKEN_OVERRIDE BIT(2)
>
> +#define SDHCI_TEGRA_VENDOR_CAP_OVERRIDES 0x10c
> +#define SDHCI_TEGRA_CAP_OVERRIDES_DQS_TRIM_MASK 0x00003f00
> +#define SDHCI_TEGRA_CAP_OVERRIDES_DQS_TRIM_SHIFT 8
> +
> #define SDHCI_TEGRA_VENDOR_MISC_CTRL 0x120
> #define SDHCI_MISC_CTRL_ENABLE_SDR104 0x8
> #define SDHCI_MISC_CTRL_ENABLE_SDR50 0x10
> @@ -112,6 +116,7 @@ struct sdhci_tegra {
>
> u32 default_tap;
> u32 default_trim;
> + u32 dqs_trim;
> };
>
> static u16 tegra_sdhci_readw(struct sdhci_host *host, int reg)
> @@ -500,7 +505,7 @@ static void tegra_sdhci_parse_pad_autocal_dt(struct sdhci_host *host)
> autocal->pull_down_hs400 = autocal->pull_down_1v8;
> }
>
> -static void tegra_sdhci_parse_default_tap_and_trim(struct sdhci_host *host)
> +static void tegra_sdhci_parse_tap_and_trim(struct sdhci_host *host)
> {
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
> @@ -515,6 +520,11 @@ static void tegra_sdhci_parse_default_tap_and_trim(struct sdhci_host *host)
> &tegra_host->default_trim);
> if (err)
> tegra_host->default_trim = 0;
> +
> + err = device_property_read_u32(host->mmc->parent, "nvidia,dqs-trim",
> + &tegra_host->dqs_trim);
> + if (err)
> + tegra_host->dqs_trim = 0x11;
Okay, so there's only one value. I think that should be clarified in the
bindings documentation. It should mention that a single cell is used for
this. Also, I assume there are lower and upper limits for the valid
range of DQS trim values. Might make sense to specify those in the DT
bindings as well.
> }
>
> static void tegra_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
> @@ -545,20 +555,33 @@ static unsigned int tegra_sdhci_get_max_clock(struct sdhci_host *host)
> return clk_round_rate(pltfm_host->clk, UINT_MAX);
> }
>
> +static void tegra_sdhci_set_dqs_trim(struct sdhci_host *host, u8 val)
> +{
> + u32 reg;
> +
> + reg = sdhci_readl(host, SDHCI_TEGRA_VENDOR_CAP_OVERRIDES);
> + reg &= ~SDHCI_TEGRA_CAP_OVERRIDES_DQS_TRIM_MASK;
> + reg |= val<<SDHCI_TEGRA_CAP_OVERRIDES_DQS_TRIM_SHIFT;
> + sdhci_writel(host, reg, SDHCI_TEGRA_VENDOR_CAP_OVERRIDES);
> +}
Nit: I dislike using "reg" as a variable representing a register value
because I keep interpreting it as designating a register offset. Hence
I tend to use more explicit "offset" for actual register offsets and
"value" for register values.
But maybe that's just me.
Thierry
Two more comments...
On Tue, Aug 07, 2018 at 04:59:58PM +0300, Aapo Vienamo wrote:
> Parse and program the HS400 DQS trim value from dt. Program a fallback
> value in case the property is missing.
"dt" -> "DT" because it is an abbreviation.
>
> Signed-off-by: Aapo Vienamo <[email protected]>
> ---
> drivers/mmc/host/sdhci-tegra.c | 32 +++++++++++++++++++++++++++++---
> 1 file changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
[...]
> @@ -545,20 +555,33 @@ static unsigned int tegra_sdhci_get_max_clock(struct sdhci_host *host)
> return clk_round_rate(pltfm_host->clk, UINT_MAX);
> }
>
> +static void tegra_sdhci_set_dqs_trim(struct sdhci_host *host, u8 val)
> +{
> + u32 reg;
> +
> + reg = sdhci_readl(host, SDHCI_TEGRA_VENDOR_CAP_OVERRIDES);
> + reg &= ~SDHCI_TEGRA_CAP_OVERRIDES_DQS_TRIM_MASK;
> + reg |= val<<SDHCI_TEGRA_CAP_OVERRIDES_DQS_TRIM_SHIFT;
Also, you should add spaces around '<<'.
Thierry
On Tue, Aug 07, 2018 at 04:59:59PM +0300, Aapo Vienamo wrote:
> Implement HS400 enhanced strobe.
Can you provide a little more information about what the impact is of
this? Does this increase throughput? How much?
>
> Signed-off-by: Aapo Vienamo <[email protected]>
> ---
> drivers/mmc/host/sdhci-tegra.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index 426f7ea..d81143b 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -43,6 +43,9 @@
> #define SDHCI_CLOCK_CTRL_PADPIPE_CLKEN_OVERRIDE BIT(3)
> #define SDHCI_CLOCK_CTRL_SPI_MODE_CLKEN_OVERRIDE BIT(2)
>
> +#define SDHCI_TEGRA_VENDOR_SYS_SW_CTRL 0x104
> +#define SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE BIT(31)
> +
> #define SDHCI_TEGRA_VENDOR_CAP_OVERRIDES 0x10c
> #define SDHCI_TEGRA_CAP_OVERRIDES_DQS_TRIM_MASK 0x00003f00
> #define SDHCI_TEGRA_CAP_OVERRIDES_DQS_TRIM_SHIFT 8
> @@ -271,6 +274,22 @@ static void tegra_sdhci_set_tap(struct sdhci_host *host, unsigned int tap)
> }
> }
>
> +static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
> + struct mmc_ios *ios)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> + u32 reg;
> +
> + reg = sdhci_readl(host, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> + pr_err("enhanced strobe: %d\n", ios->enhanced_strobe);
Left-over debug error?
> + if (ios->enhanced_strobe)
> + reg |= SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> + else
> + reg &= ~SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> + sdhci_writel(host, reg, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
You might want to add blank lines around the if ... else ... block for
readability.
Thierry
On Thu, 9 Aug 2018 13:36:09 +0200
Thierry Reding <[email protected]> wrote:
> On Tue, Aug 07, 2018 at 04:59:57PM +0300, Aapo Vienamo wrote:
> > Document HS400 DQS trim value device tree property.
> >
> > Signed-off-by: Aapo Vienamo <[email protected]>
> > ---
> > Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
> > index 3c7960a..7d294f3 100644
> > --- a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
> > +++ b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
> > @@ -72,6 +72,7 @@ Optional properties for Tegra210 and Tegra186:
> > trimmer value for non-tunable modes.
> > - nvidia,default-trim : Specify the default outbound clock trimmer
> > value.
> > +- nvidia,dqs-trim : Specify DQS trim value for HS400 timing
> >
> > Notes on the pad calibration pull up and pulldown offset values:
> > - The property values are drive codes which are programmed into the
> > @@ -88,6 +89,8 @@ Optional properties for Tegra210 and Tegra186:
> > - The values are programmed to the Vendor Clock Control Register.
> > Please refer to the reference manual of the SoC for correct
> > values.
> > + - The DQS trim values are only used on controllers which support
> > + HS400 timing.
>
> One of these additions says "DQS trim values", the other says "DQS trim
> value". It is unclear from the above how many values there are. I think
> this should be more explicit. Also, I don't see why the note about which
> controllers the DQS trim value(s) applies to is in a separate paragraph.
> Couldn't it be moved to the property description?
It's a single value. The plural form is a mistake.
> Also, I think the bindings should specify which generations of Tegra do
> support HS400. Where else are people supposed to find that information?
This property is under the "Optional properties for Tegra210 and
Tegra186" section and it only applies for the said generations.
-Aapo
On Tue, Aug 07, 2018 at 05:00:00PM +0300, Aapo Vienamo wrote:
> Implement HS400 specific delay line calibration procedure.
>
> Signed-off-by: Aapo Vienamo <[email protected]>
> ---
> drivers/mmc/host/sdhci-tegra.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
Should this be before the previous patch in order to make sure the
calibration is performed as soon as the feature is available. This is
counting beans I guess, but it is technically possible for someone to
get everything up to patch 3/8 and then get the corresponding changes
in the DTS files to enable the mode and then have HS400 enabled without
this calibration.
Thierry
>
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index d81143b..d0b68b7 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -56,6 +56,12 @@
> #define SDHCI_MISC_CTRL_ENABLE_SDHCI_SPEC_300 0x20
> #define SDHCI_MISC_CTRL_ENABLE_DDR50 0x200
>
> +#define SDHCI_TEGRA_VENDOR_DLLCAL_CFG 0x1b0
> +#define SDHCI_TEGRA_DLLCAL_CALIBRATE BIT(31)
> +
> +#define SDHCI_TEGRA_VENDOR_DLLCAL_STA 0x1bc
> +#define SDHCI_TEGRA_DLLCAL_STA_ACTIVE BIT(31)
> +
> #define SDHCI_VNDR_TUN_CTRL0_0 0x1c0
> #define SDHCI_VNDR_TUN_CTRL0_TUN_HW_TAP 0x20000
>
> @@ -584,6 +590,24 @@ static void tegra_sdhci_set_dqs_trim(struct sdhci_host *host, u8 val)
> sdhci_writel(host, reg, SDHCI_TEGRA_VENDOR_CAP_OVERRIDES);
> }
>
> +static void tegra_sdhci_hs400_dll_cal(struct sdhci_host *host)
> +{
> + u32 reg;
> + int err;
> +
> + reg = sdhci_readl(host, SDHCI_TEGRA_VENDOR_DLLCAL_CFG);
> + reg |= SDHCI_TEGRA_DLLCAL_CALIBRATE;
> + sdhci_writel(host, reg, SDHCI_TEGRA_VENDOR_DLLCAL_CFG);
Is this self-clearing? Or do we need to clear it manually in order for
a subsequent calibration procedure to succeed?
Thierry
On Tue, Aug 07, 2018 at 05:00:01PM +0300, Aapo Vienamo wrote:
> Add the HS400 DQS trim value for Tegra186 SDMMC4.
>
> Signed-off-by: Aapo Vienamo <[email protected]>
> ---
> arch/arm64/boot/dts/nvidia/tegra186.dtsi | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
> index 6e9ef26..9e07bc6 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
> @@ -313,6 +313,7 @@
> nvidia,pad-autocal-pull-down-offset-1v8-timeout = <0x0a>;
> nvidia,default-tap = <0x5>;
> nvidia,default-trim = <0x9>;
> + nvidia,dqs-trim = <63>;
> status = "disabled";
> };
>
Isn't this technically dependent on the board layout and as such would
belong in the board DTS file? Or does this value work on all existing
Tegra186 platforms?
Thierry
On Thu, 9 Aug 2018 13:49:22 +0200
Thierry Reding <[email protected]> wrote:
> On Tue, Aug 07, 2018 at 05:00:01PM +0300, Aapo Vienamo wrote:
> > Add the HS400 DQS trim value for Tegra186 SDMMC4.
> >
> > Signed-off-by: Aapo Vienamo <[email protected]>
> > ---
> > arch/arm64/boot/dts/nvidia/tegra186.dtsi | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
> > index 6e9ef26..9e07bc6 100644
> > --- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
> > +++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
> > @@ -313,6 +313,7 @@
> > nvidia,pad-autocal-pull-down-offset-1v8-timeout = <0x0a>;
> > nvidia,default-tap = <0x5>;
> > nvidia,default-trim = <0x9>;
> > + nvidia,dqs-trim = <63>;
> > status = "disabled";
> > };
> >
>
> Isn't this technically dependent on the board layout and as such would
> belong in the board DTS file? Or does this value work on all existing
> Tegra186 platforms?
This value is specified as part of the controller initialization
sequence in the TRM. I've understood that this (and other tap and trim)
value(s) are used for compensating the propagation delay differences
that are caused by the internal SoC layout.
-Aapo
On Thu, 9 Aug 2018 13:43:45 +0200
Thierry Reding <[email protected]> wrote:
> On Tue, Aug 07, 2018 at 04:59:59PM +0300, Aapo Vienamo wrote:
> > Implement HS400 enhanced strobe.
>
> Can you provide a little more information about what the impact is of
> this? Does this increase throughput? How much?
The eMMC enhanced strobe is a mechanism that can be used instead of the
HS400 tuning procedure. Note that the delay line calibration implemented
later on in this series is Tegra specific and has to be performed
regardless of which type of HS400 tuning mechanism is used.
> >
> > Signed-off-by: Aapo Vienamo <[email protected]>
> > ---
> > drivers/mmc/host/sdhci-tegra.c | 22 ++++++++++++++++++++++
> > 1 file changed, 22 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> > index 426f7ea..d81143b 100644
> > --- a/drivers/mmc/host/sdhci-tegra.c
> > +++ b/drivers/mmc/host/sdhci-tegra.c
> > @@ -43,6 +43,9 @@
> > #define SDHCI_CLOCK_CTRL_PADPIPE_CLKEN_OVERRIDE BIT(3)
> > #define SDHCI_CLOCK_CTRL_SPI_MODE_CLKEN_OVERRIDE BIT(2)
> >
> > +#define SDHCI_TEGRA_VENDOR_SYS_SW_CTRL 0x104
> > +#define SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE BIT(31)
> > +
> > #define SDHCI_TEGRA_VENDOR_CAP_OVERRIDES 0x10c
> > #define SDHCI_TEGRA_CAP_OVERRIDES_DQS_TRIM_MASK 0x00003f00
> > #define SDHCI_TEGRA_CAP_OVERRIDES_DQS_TRIM_SHIFT 8
> > @@ -271,6 +274,22 @@ static void tegra_sdhci_set_tap(struct sdhci_host *host, unsigned int tap)
> > }
> > }
> >
> > +static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
> > + struct mmc_ios *ios)
> > +{
> > + struct sdhci_host *host = mmc_priv(mmc);
> > + u32 reg;
> > +
> > + reg = sdhci_readl(host, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> > + pr_err("enhanced strobe: %d\n", ios->enhanced_strobe);
>
> Left-over debug error?
Yep.
>
> > + if (ios->enhanced_strobe)
> > + reg |= SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> > + else
> > + reg &= ~SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> > + sdhci_writel(host, reg, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
>
> You might want to add blank lines around the if ... else ... block for
> readability.
True.
-Aapo
On 08/09/2018 08:02 AM, Aapo Vienamo wrote:
> On Thu, 9 Aug 2018 13:49:22 +0200
> Thierry Reding <[email protected]> wrote:
>
>> On Tue, Aug 07, 2018 at 05:00:01PM +0300, Aapo Vienamo wrote:
>>> Add the HS400 DQS trim value for Tegra186 SDMMC4.
>>>
>>> Signed-off-by: Aapo Vienamo <[email protected]>
>>> ---
>>> arch/arm64/boot/dts/nvidia/tegra186.dtsi | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
>>> index 6e9ef26..9e07bc6 100644
>>> --- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
>>> +++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
>>> @@ -313,6 +313,7 @@
>>> nvidia,pad-autocal-pull-down-offset-1v8-timeout = <0x0a>;
>>> nvidia,default-tap = <0x5>;
>>> nvidia,default-trim = <0x9>;
>>> + nvidia,dqs-trim = <63>;
>>> status = "disabled";
>>> };
>>>
>>
>> Isn't this technically dependent on the board layout and as such would
>> belong in the board DTS file? Or does this value work on all existing
>> Tegra186 platforms?
>
> This value is specified as part of the controller initialization
> sequence in the TRM. I've understood that this (and other tap and trim)
> value(s) are used for compensating the propagation delay differences
> that are caused by the internal SoC layout.
>
> -Aapo
> --
The Tegra2 and Tegra3 TRMs also specify recommended DQS values, and I am
working on at least one device that differs in the platform data from
the default value.
I see that you mentioned this is for the newer devices that support
HS200/HS400 modes, but does it enable setting DQS on older devices?
On Thu, 9 Aug 2018 13:48:05 +0200
Thierry Reding <[email protected]> wrote:
> On Tue, Aug 07, 2018 at 05:00:00PM +0300, Aapo Vienamo wrote:
> > Implement HS400 specific delay line calibration procedure.
> >
> > Signed-off-by: Aapo Vienamo <[email protected]>
> > ---
> > drivers/mmc/host/sdhci-tegra.c | 29 +++++++++++++++++++++++++++++
> > 1 file changed, 29 insertions(+)
>
> Should this be before the previous patch in order to make sure the
> calibration is performed as soon as the feature is available. This is
> counting beans I guess, but it is technically possible for someone to
> get everything up to patch 3/8 and then get the corresponding changes
> in the DTS files to enable the mode and then have HS400 enabled without
> this calibration.
True.
>
> >
> > diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> > index d81143b..d0b68b7 100644
> > --- a/drivers/mmc/host/sdhci-tegra.c
> > +++ b/drivers/mmc/host/sdhci-tegra.c
> > @@ -56,6 +56,12 @@
> > #define SDHCI_MISC_CTRL_ENABLE_SDHCI_SPEC_300 0x20
> > #define SDHCI_MISC_CTRL_ENABLE_DDR50 0x200
> >
> > +#define SDHCI_TEGRA_VENDOR_DLLCAL_CFG 0x1b0
> > +#define SDHCI_TEGRA_DLLCAL_CALIBRATE BIT(31)
> > +
> > +#define SDHCI_TEGRA_VENDOR_DLLCAL_STA 0x1bc
> > +#define SDHCI_TEGRA_DLLCAL_STA_ACTIVE BIT(31)
> > +
> > #define SDHCI_VNDR_TUN_CTRL0_0 0x1c0
> > #define SDHCI_VNDR_TUN_CTRL0_TUN_HW_TAP 0x20000
> >
> > @@ -584,6 +590,24 @@ static void tegra_sdhci_set_dqs_trim(struct sdhci_host *host, u8 val)
> > sdhci_writel(host, reg, SDHCI_TEGRA_VENDOR_CAP_OVERRIDES);
> > }
> >
> > +static void tegra_sdhci_hs400_dll_cal(struct sdhci_host *host)
> > +{
> > + u32 reg;
> > + int err;
> > +
> > + reg = sdhci_readl(host, SDHCI_TEGRA_VENDOR_DLLCAL_CFG);
> > + reg |= SDHCI_TEGRA_DLLCAL_CALIBRATE;
> > + sdhci_writel(host, reg, SDHCI_TEGRA_VENDOR_DLLCAL_CFG);
>
> Is this self-clearing? Or do we need to clear it manually in order for
> a subsequent calibration procedure to succeed?
Yes, the TRM states that this bit should not be cleared by software.
-Aapo
On Thu, 9 Aug 2018 08:23:16 -0400
Peter Geis <[email protected]> wrote:
> On 08/09/2018 08:02 AM, Aapo Vienamo wrote:
> > On Thu, 9 Aug 2018 13:49:22 +0200
> > Thierry Reding <[email protected]> wrote:
> >
> >> On Tue, Aug 07, 2018 at 05:00:01PM +0300, Aapo Vienamo wrote:
> >>> Add the HS400 DQS trim value for Tegra186 SDMMC4.
> >>>
> >>> Signed-off-by: Aapo Vienamo <[email protected]>
> >>> ---
> >>> arch/arm64/boot/dts/nvidia/tegra186.dtsi | 1 +
> >>> 1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
> >>> index 6e9ef26..9e07bc6 100644
> >>> --- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
> >>> +++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
> >>> @@ -313,6 +313,7 @@
> >>> nvidia,pad-autocal-pull-down-offset-1v8-timeout = <0x0a>;
> >>> nvidia,default-tap = <0x5>;
> >>> nvidia,default-trim = <0x9>;
> >>> + nvidia,dqs-trim = <63>;
> >>> status = "disabled";
> >>> };
> >>>
> >>
> >> Isn't this technically dependent on the board layout and as such would
> >> belong in the board DTS file? Or does this value work on all existing
> >> Tegra186 platforms?
> >
> > This value is specified as part of the controller initialization
> > sequence in the TRM. I've understood that this (and other tap and trim)
> > value(s) are used for compensating the propagation delay differences
> > that are caused by the internal SoC layout.
> >
> > -Aapo
> > --
>
> The Tegra2 and Tegra3 TRMs also specify recommended DQS values, and I am
> working on at least one device that differs in the platform data from
> the default value.
> I see that you mentioned this is for the newer devices that support
> HS200/HS400 modes, but does it enable setting DQS on older devices?
I can't find any mention of _SDMMC_ DQS trimmer on Tegra2, Tegra3 or
Tegra124 TRMs. As far as I can tell, programming the DQS trimmer value
is only required by HS400 signaling on Tegra210 and Tegra186.
-Aapo
On 08/09/2018 08:37 AM, Aapo Vienamo wrote:
> On Thu, 9 Aug 2018 08:23:16 -0400
> Peter Geis <[email protected]> wrote:
>
>> On 08/09/2018 08:02 AM, Aapo Vienamo wrote:
>>> On Thu, 9 Aug 2018 13:49:22 +0200
>>> Thierry Reding <[email protected]> wrote:
>>>
>>>> On Tue, Aug 07, 2018 at 05:00:01PM +0300, Aapo Vienamo wrote:
>>>>> Add the HS400 DQS trim value for Tegra186 SDMMC4.
>>>>>
>>>>> Signed-off-by: Aapo Vienamo <[email protected]>
>>>>> ---
>>>>> arch/arm64/boot/dts/nvidia/tegra186.dtsi | 1 +
>>>>> 1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
>>>>> index 6e9ef26..9e07bc6 100644
>>>>> --- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
>>>>> +++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
>>>>> @@ -313,6 +313,7 @@
>>>>> nvidia,pad-autocal-pull-down-offset-1v8-timeout = <0x0a>;
>>>>> nvidia,default-tap = <0x5>;
>>>>> nvidia,default-trim = <0x9>;
>>>>> + nvidia,dqs-trim = <63>;
>>>>> status = "disabled";
>>>>> };
>>>>>
>>>>
>>>> Isn't this technically dependent on the board layout and as such would
>>>> belong in the board DTS file? Or does this value work on all existing
>>>> Tegra186 platforms?
>>>
>>> This value is specified as part of the controller initialization
>>> sequence in the TRM. I've understood that this (and other tap and trim)
>>> value(s) are used for compensating the propagation delay differences
>>> that are caused by the internal SoC layout.
>>>
>>> -Aapo
>>> --
>>
>> The Tegra2 and Tegra3 TRMs also specify recommended DQS values, and I am
>> working on at least one device that differs in the platform data from
>> the default value.
>> I see that you mentioned this is for the newer devices that support
>> HS200/HS400 modes, but does it enable setting DQS on older devices?
>
> I can't find any mention of _SDMMC_ DQS trimmer on Tegra2, Tegra3 or
> Tegra124 TRMs. As far as I can tell, programming the DQS trimmer value
> is only required by HS400 signaling on Tegra210 and Tegra186.
>
> -Aapo
>
Apologies, I was referring to the default Tap/Trim values.
DQS found it's way into my brain from the memory controller.
On Thu, Aug 09, 2018 at 02:45:15PM +0300, Aapo Vienamo wrote:
> On Thu, 9 Aug 2018 13:36:09 +0200
> Thierry Reding <[email protected]> wrote:
>
> > On Tue, Aug 07, 2018 at 04:59:57PM +0300, Aapo Vienamo wrote:
> > > Document HS400 DQS trim value device tree property.
> > >
> > > Signed-off-by: Aapo Vienamo <[email protected]>
> > > ---
> > > Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
> > > index 3c7960a..7d294f3 100644
> > > --- a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
> > > +++ b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
> > > @@ -72,6 +72,7 @@ Optional properties for Tegra210 and Tegra186:
> > > trimmer value for non-tunable modes.
> > > - nvidia,default-trim : Specify the default outbound clock trimmer
> > > value.
> > > +- nvidia,dqs-trim : Specify DQS trim value for HS400 timing
> > >
> > > Notes on the pad calibration pull up and pulldown offset values:
> > > - The property values are drive codes which are programmed into the
> > > @@ -88,6 +89,8 @@ Optional properties for Tegra210 and Tegra186:
> > > - The values are programmed to the Vendor Clock Control Register.
> > > Please refer to the reference manual of the SoC for correct
> > > values.
> > > + - The DQS trim values are only used on controllers which support
> > > + HS400 timing.
> >
> > One of these additions says "DQS trim values", the other says "DQS trim
> > value". It is unclear from the above how many values there are. I think
> > this should be more explicit. Also, I don't see why the note about which
> > controllers the DQS trim value(s) applies to is in a separate paragraph.
> > Couldn't it be moved to the property description?
>
> It's a single value. The plural form is a mistake.
>
> > Also, I think the bindings should specify which generations of Tegra do
> > support HS400. Where else are people supposed to find that information?
>
> This property is under the "Optional properties for Tegra210 and
> Tegra186" section and it only applies for the said generations.
What's the point of specifying that they are only used on controllers
which support HS400? Are you saying that only a subset of the SDHCI
controllers on Tegra210 and Tegra186 support HS400?
Thierry
On Thu, Aug 09, 2018 at 03:22:54PM +0300, Aapo Vienamo wrote:
> On Thu, 9 Aug 2018 13:43:45 +0200
> Thierry Reding <[email protected]> wrote:
>
> > On Tue, Aug 07, 2018 at 04:59:59PM +0300, Aapo Vienamo wrote:
> > > Implement HS400 enhanced strobe.
> >
> > Can you provide a little more information about what the impact is of
> > this? Does this increase throughput? How much?
>
> The eMMC enhanced strobe is a mechanism that can be used instead of the
> HS400 tuning procedure. Note that the delay line calibration implemented
> later on in this series is Tegra specific and has to be performed
> regardless of which type of HS400 tuning mechanism is used.
Sounds like a variation of that would be good material for the commit
message.
Thierry
On Thu, Aug 09, 2018 at 03:02:26PM +0300, Aapo Vienamo wrote:
> On Thu, 9 Aug 2018 13:49:22 +0200
> Thierry Reding <[email protected]> wrote:
>
> > On Tue, Aug 07, 2018 at 05:00:01PM +0300, Aapo Vienamo wrote:
> > > Add the HS400 DQS trim value for Tegra186 SDMMC4.
> > >
> > > Signed-off-by: Aapo Vienamo <[email protected]>
> > > ---
> > > arch/arm64/boot/dts/nvidia/tegra186.dtsi | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
> > > index 6e9ef26..9e07bc6 100644
> > > --- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
> > > +++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
> > > @@ -313,6 +313,7 @@
> > > nvidia,pad-autocal-pull-down-offset-1v8-timeout = <0x0a>;
> > > nvidia,default-tap = <0x5>;
> > > nvidia,default-trim = <0x9>;
> > > + nvidia,dqs-trim = <63>;
> > > status = "disabled";
> > > };
> > >
> >
> > Isn't this technically dependent on the board layout and as such would
> > belong in the board DTS file? Or does this value work on all existing
> > Tegra186 platforms?
>
> This value is specified as part of the controller initialization
> sequence in the TRM. I've understood that this (and other tap and trim)
> value(s) are used for compensating the propagation delay differences
> that are caused by the internal SoC layout.
Hmm... it would seem to me like the routing on a board would have a more
significant impact than the SoC internal routing. But perhaps the board-
specific routing is actually what the automatic calibration is used for?
Anyway, if it ever turns out that we need slightly different values on a
given board, we can always override the value in the board DTS file.
Thierry
On Thu, 9 Aug 2018 15:46:48 +0200
Thierry Reding <[email protected]> wrote:
> On Thu, Aug 09, 2018 at 02:45:15PM +0300, Aapo Vienamo wrote:
> > On Thu, 9 Aug 2018 13:36:09 +0200
> > Thierry Reding <[email protected]> wrote:
> >
> > > On Tue, Aug 07, 2018 at 04:59:57PM +0300, Aapo Vienamo wrote:
> > > > Document HS400 DQS trim value device tree property.
> > > >
> > > > Signed-off-by: Aapo Vienamo <[email protected]>
> > > > ---
> > > > Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
> > > > index 3c7960a..7d294f3 100644
> > > > --- a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
> > > > +++ b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
> > > > @@ -72,6 +72,7 @@ Optional properties for Tegra210 and Tegra186:
> > > > trimmer value for non-tunable modes.
> > > > - nvidia,default-trim : Specify the default outbound clock trimmer
> > > > value.
> > > > +- nvidia,dqs-trim : Specify DQS trim value for HS400 timing
> > > >
> > > > Notes on the pad calibration pull up and pulldown offset values:
> > > > - The property values are drive codes which are programmed into the
> > > > @@ -88,6 +89,8 @@ Optional properties for Tegra210 and Tegra186:
> > > > - The values are programmed to the Vendor Clock Control Register.
> > > > Please refer to the reference manual of the SoC for correct
> > > > values.
> > > > + - The DQS trim values are only used on controllers which support
> > > > + HS400 timing.
> > >
> > > One of these additions says "DQS trim values", the other says "DQS trim
> > > value". It is unclear from the above how many values there are. I think
> > > this should be more explicit. Also, I don't see why the note about which
> > > controllers the DQS trim value(s) applies to is in a separate paragraph.
> > > Couldn't it be moved to the property description?
> >
> > It's a single value. The plural form is a mistake.
> >
> > > Also, I think the bindings should specify which generations of Tegra do
> > > support HS400. Where else are people supposed to find that information?
> >
> > This property is under the "Optional properties for Tegra210 and
> > Tegra186" section and it only applies for the said generations.
>
> What's the point of specifying that they are only used on controllers
> which support HS400? Are you saying that only a subset of the SDHCI
> controllers on Tegra210 and Tegra186 support HS400?
Yes, on Tegra210 and Tegra186 only SDMMC4 supports HS400.
-Aapo
On Thu, Aug 09, 2018 at 05:06:04PM +0300, Aapo Vienamo wrote:
> On Thu, 9 Aug 2018 15:46:48 +0200
> Thierry Reding <[email protected]> wrote:
>
> > On Thu, Aug 09, 2018 at 02:45:15PM +0300, Aapo Vienamo wrote:
> > > On Thu, 9 Aug 2018 13:36:09 +0200
> > > Thierry Reding <[email protected]> wrote:
> > >
> > > > On Tue, Aug 07, 2018 at 04:59:57PM +0300, Aapo Vienamo wrote:
> > > > > Document HS400 DQS trim value device tree property.
> > > > >
> > > > > Signed-off-by: Aapo Vienamo <[email protected]>
> > > > > ---
> > > > > Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt | 3 +++
> > > > > 1 file changed, 3 insertions(+)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
> > > > > index 3c7960a..7d294f3 100644
> > > > > --- a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
> > > > > +++ b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
> > > > > @@ -72,6 +72,7 @@ Optional properties for Tegra210 and Tegra186:
> > > > > trimmer value for non-tunable modes.
> > > > > - nvidia,default-trim : Specify the default outbound clock trimmer
> > > > > value.
> > > > > +- nvidia,dqs-trim : Specify DQS trim value for HS400 timing
> > > > >
> > > > > Notes on the pad calibration pull up and pulldown offset values:
> > > > > - The property values are drive codes which are programmed into the
> > > > > @@ -88,6 +89,8 @@ Optional properties for Tegra210 and Tegra186:
> > > > > - The values are programmed to the Vendor Clock Control Register.
> > > > > Please refer to the reference manual of the SoC for correct
> > > > > values.
> > > > > + - The DQS trim values are only used on controllers which support
> > > > > + HS400 timing.
> > > >
> > > > One of these additions says "DQS trim values", the other says "DQS trim
> > > > value". It is unclear from the above how many values there are. I think
> > > > this should be more explicit. Also, I don't see why the note about which
> > > > controllers the DQS trim value(s) applies to is in a separate paragraph.
> > > > Couldn't it be moved to the property description?
> > >
> > > It's a single value. The plural form is a mistake.
> > >
> > > > Also, I think the bindings should specify which generations of Tegra do
> > > > support HS400. Where else are people supposed to find that information?
> > >
> > > This property is under the "Optional properties for Tegra210 and
> > > Tegra186" section and it only applies for the said generations.
> >
> > What's the point of specifying that they are only used on controllers
> > which support HS400? Are you saying that only a subset of the SDHCI
> > controllers on Tegra210 and Tegra186 support HS400?
>
> Yes, on Tegra210 and Tegra186 only SDMMC4 supports HS400.
Good. I think that'd be good to have as part of the DT bindings
documentation.
Thierry