| Message ID | 1533650404-18125-3-git-send-email-avienamo@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Tegra SDHCI support HS400 on Tegra210 and Tegra186 | expand |
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 <avienamo@nvidia.com> > --- > 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 <avienamo@nvidia.com> > --- > 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
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);
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 <avienamo@nvidia.com> --- drivers/mmc/host/sdhci-tegra.c | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-)