Tegra210/Tegra186/Tegra194 has incorrectly enabled SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK from the beginning of their support.
Tegra210 and later SDMMC hardware default uses sdmmc_legacy_tm (TMCLK) all the time for hardware data timeout instead of SDCLK and this TMCLK need to be kept enabled by Tegra sdmmc driver.
This series includes patches to fix this for Tegra210/Tegra186/Tegra194.
These patches need to be manually backported for 4.9, 4.14 and 4.19.
Will send patches to backport separately once these patches are ack'd.
Delta between patch versions: [v6]: v5 is sent out mistakenly with incorrect patches. v6 includes proper patches addressing v4 feedback - updated dt-binding doc to be more clear - updated Tegra sdhci driver to retrieve sdhci and tmclk clocks based on no. of clocks in sdhci device node as old device trees do not use sdhci clock name and this allows proper clock retrival irrespective of sdhci and tmclk clocks order in device tree. - Added separate quirk for identifying SoC's supporting separate timeout clock to be more clear.
[v5]: Include below changes based on v4 feedback - updated dt-binding doc to be more clear - updated Tegra sdhci driver to retrieve sdhci and tmclk clocks based on no. of clocks in sdhci device node as old device trees do not use sdhci clock name and this allows proper clock retrival irrespective of sdhci and tmclk clocks order in device tree. - Added separate quirk for identifying SoC's supporting separate timeout clock to be more clear.
[v4]: Include additional dt-binding patch
[v3]: Same as v2 with fixes tag
[v2]: Includes minor fix - Patch-0006: parentheses around operand of '!'
Sowjanya Komatineni (7): sdhci: tegra: Remove SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK for Tegra210 sdhci: tegra: Remove SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK for Tegra186 dt-bindings: mmc: tegra: Add tmclk for Tegra210 and later arm64: tegra: Add missing timeout clock to Tegra210 SDMMC arm64: tegra: Add missing timeout clock to Tegra186 SDMMC nodes arm64: tegra: Add missing timeout clock to Tegra194 SDMMC nodes sdhci: tegra: Add missing TMCLK for data timeout
.../bindings/mmc/nvidia,tegra20-sdhci.txt | 32 +++++++- arch/arm64/boot/dts/nvidia/tegra186.dtsi | 20 +++-- arch/arm64/boot/dts/nvidia/tegra194.dtsi | 15 ++-- arch/arm64/boot/dts/nvidia/tegra210.dtsi | 20 +++-- drivers/mmc/host/sdhci-tegra.c | 92 +++++++++++++++++++--- 5 files changed, 144 insertions(+), 35 deletions(-)
commit b5a84ecf025a ("mmc: tegra: Add Tegra210 support")
SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK is set for Tegra210 from the beginning of Tegra210 support in the driver.
Tegra210 SDMMC hardware by default uses timeout clock (TMCLK) instead of SDCLK and this quirk should not be set.
So, this patch remove this quirk for Tegra210.
Fixes: b5a84ecf025a ("mmc: tegra: Add Tegra210 support") Cc: stable stable@vger.kernel.org # 5.4 Tested-by: Jon Hunter jonathanh@nvidia.com Reviewed-by: Jon Hunter jonathanh@nvidia.com Acked-by: Adrian Hunter adrian.hunter@intel.com Signed-off-by: Sowjanya Komatineni skomatineni@nvidia.com --- drivers/mmc/host/sdhci-tegra.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c index 0a3f9d0..2be3511 100644 --- a/drivers/mmc/host/sdhci-tegra.c +++ b/drivers/mmc/host/sdhci-tegra.c @@ -1418,7 +1418,6 @@ static const struct sdhci_ops tegra210_sdhci_ops = {
static const struct sdhci_pltfm_data sdhci_tegra210_pdata = { .quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL | - SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | SDHCI_QUIRK_SINGLE_POWER_WRITE | SDHCI_QUIRK_NO_HISPD_BIT | SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
commit 4346b7c7941d ("mmc: tegra: Add Tegra186 support")
SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK is set for Tegra186 from the beginning of its support in driver.
Tegra186 SDMMC hardware by default uses timeout clock (TMCLK) instead of SDCLK and this quirk should not be set.
So, this patch remove this quirk for Tegra186.
Fixes: 4346b7c7941d ("mmc: tegra: Add Tegra186 support") Cc: stable stable@vger.kernel.org # 5.4 Tested-by: Jon Hunter jonathanh@nvidia.com Reviewed-by: Jon Hunter jonathanh@nvidia.com Acked-by: Adrian Hunter adrian.hunter@intel.com Signed-off-by: Sowjanya Komatineni skomatineni@nvidia.com --- drivers/mmc/host/sdhci-tegra.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c index 2be3511..31ed321 100644 --- a/drivers/mmc/host/sdhci-tegra.c +++ b/drivers/mmc/host/sdhci-tegra.c @@ -1455,7 +1455,6 @@ static const struct sdhci_ops tegra186_sdhci_ops = {
static const struct sdhci_pltfm_data sdhci_tegra186_pdata = { .quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL | - SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | SDHCI_QUIRK_SINGLE_POWER_WRITE | SDHCI_QUIRK_NO_HISPD_BIT | SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
Tegra210 and later uses separate SDMMC_LEGACY_TM clock for data timeout.
So, this patch adds "tmclk" to Tegra sdhci clock property in the device tree binding.
Signed-off-by: Sowjanya Komatineni skomatineni@nvidia.com --- .../bindings/mmc/nvidia,tegra20-sdhci.txt | 32 ++++++++++++++++++++-- 1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt index 2cf3aff..96c0b14 100644 --- a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt +++ b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt @@ -15,8 +15,15 @@ Required properties: - "nvidia,tegra210-sdhci": for Tegra210 - "nvidia,tegra186-sdhci": for Tegra186 - "nvidia,tegra194-sdhci": for Tegra194 -- clocks : Must contain one entry, for the module clock. - See ../clocks/clock-bindings.txt for details. +- clocks: For Tegra210, Tegra186 and Tegra194 must contain two entries. + One for the module clock and one for the timeout clock. + For all other Tegra devices, must contain a single entry for + the module clock. See ../clocks/clock-bindings.txt for details. +- clock-names: For Tegra210, Tegra186 and Tegra194 must contain the + strings 'sdhci' and 'tmclk' to represent the module and + the timeout clocks, respectively. + For all other Tegra devices must contain the string 'sdhci' + to represent the module clock. - resets : Must contain an entry for each entry in reset-names. See ../reset/reset.txt for details. - reset-names : Must include the following entries: @@ -99,7 +106,7 @@ Optional properties for Tegra210, Tegra186 and Tegra194:
Example: sdhci@700b0000 { - compatible = "nvidia,tegra210-sdhci", "nvidia,tegra124-sdhci"; + compatible = "nvidia,tegra124-sdhci"; reg = <0x0 0x700b0000 0x0 0x200>; interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>; clocks = <&tegra_car TEGRA210_CLK_SDMMC1>; @@ -115,3 +122,22 @@ sdhci@700b0000 { nvidia,pad-autocal-pull-down-offset-1v8 = <0x7b>; status = "disabled"; }; + +sdhci@700b0000 { + compatible = "nvidia,tegra210-sdhci"; + reg = <0x0 0x700b0000 0x0 0x200>; + interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA210_CLK_SDMMC1>, + <&tegra_car TEGRA210_CLK_SDMMC_LEGACY>; + clock-names = "sdhci", "tmclk"; + resets = <&tegra_car 14>; + reset-names = "sdhci"; + pinctrl-names = "sdmmc-3v3", "sdmmc-1v8"; + pinctrl-0 = <&sdmmc1_3v3>; + pinctrl-1 = <&sdmmc1_1v8>; + nvidia,pad-autocal-pull-up-offset-3v3 = <0x00>; + nvidia,pad-autocal-pull-down-offset-3v3 = <0x7d>; + nvidia,pad-autocal-pull-up-offset-1v8 = <0x7b>; + nvidia,pad-autocal-pull-down-offset-1v8 = <0x7b>; + status = "disabled"; +};
On 27/08/2020 04:49, Sowjanya Komatineni wrote:
Tegra210 and later uses separate SDMMC_LEGACY_TM clock for data timeout.
So, this patch adds "tmclk" to Tegra sdhci clock property in the device tree binding.
Signed-off-by: Sowjanya Komatineni skomatineni@nvidia.com
.../bindings/mmc/nvidia,tegra20-sdhci.txt | 32 ++++++++++++++++++++-- 1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt index 2cf3aff..96c0b14 100644 --- a/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt +++ b/Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt @@ -15,8 +15,15 @@ Required properties:
- "nvidia,tegra210-sdhci": for Tegra210
- "nvidia,tegra186-sdhci": for Tegra186
- "nvidia,tegra194-sdhci": for Tegra194
-- clocks : Must contain one entry, for the module clock.
- See ../clocks/clock-bindings.txt for details.
+- clocks: For Tegra210, Tegra186 and Tegra194 must contain two entries.
One for the module clock and one for the timeout clock.For all other Tegra devices, must contain a single entry forthe module clock. See ../clocks/clock-bindings.txt for details.+- clock-names: For Tegra210, Tegra186 and Tegra194 must contain the
strings 'sdhci' and 'tmclk' to represent the module andthe timeout clocks, respectively.For all other Tegra devices must contain the string 'sdhci'to represent the module clock.
- resets : Must contain an entry for each entry in reset-names. See ../reset/reset.txt for details.
- reset-names : Must include the following entries:
@@ -99,7 +106,7 @@ Optional properties for Tegra210, Tegra186 and Tegra194: Example: sdhci@700b0000 {
- compatible = "nvidia,tegra210-sdhci", "nvidia,tegra124-sdhci";
- compatible = "nvidia,tegra124-sdhci"; reg = <0x0 0x700b0000 0x0 0x200>; interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>; clocks = <&tegra_car TEGRA210_CLK_SDMMC1>;
@@ -115,3 +122,22 @@ sdhci@700b0000 { nvidia,pad-autocal-pull-down-offset-1v8 = <0x7b>; status = "disabled"; };
+sdhci@700b0000 {
- compatible = "nvidia,tegra210-sdhci";
- reg = <0x0 0x700b0000 0x0 0x200>;
- interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&tegra_car TEGRA210_CLK_SDMMC1>,
<&tegra_car TEGRA210_CLK_SDMMC_LEGACY>;- clock-names = "sdhci", "tmclk";
- resets = <&tegra_car 14>;
- reset-names = "sdhci";
- pinctrl-names = "sdmmc-3v3", "sdmmc-1v8";
- pinctrl-0 = <&sdmmc1_3v3>;
- pinctrl-1 = <&sdmmc1_1v8>;
- nvidia,pad-autocal-pull-up-offset-3v3 = <0x00>;
- nvidia,pad-autocal-pull-down-offset-3v3 = <0x7d>;
- nvidia,pad-autocal-pull-up-offset-1v8 = <0x7b>;
- nvidia,pad-autocal-pull-down-offset-1v8 = <0x7b>;
- status = "disabled";
+};
Thanks!
Reviewed-by: Jon Hunter jonathanh@nvidia.com
Cheers Jon
commit 742af7e7a0a1 ("arm64: tegra: Add Tegra210 support")
Tegra210 uses separate SDMMC_LEGACY_TM clock for data timeout and this clock is not enabled currently which is not recommended.
Tegra SDMMC advertises 12Mhz as timeout clock frequency in host capability register.
So, this clock should be kept enabled by SDMMC driver.
Fixes: 742af7e7a0a1 ("arm64: tegra: Add Tegra210 support") Cc: stable stable@vger.kernel.org # 5.4 Tested-by: Jon Hunter jonathanh@nvidia.com Reviewed-by: Jon Hunter jonathanh@nvidia.com Signed-off-by: Sowjanya Komatineni skomatineni@nvidia.com --- arch/arm64/boot/dts/nvidia/tegra210.dtsi | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi index 829f786..8cca216 100644 --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi @@ -1194,8 +1194,9 @@ compatible = "nvidia,tegra210-sdhci"; reg = <0x0 0x700b0000 0x0 0x200>; interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&tegra_car TEGRA210_CLK_SDMMC1>; - clock-names = "sdhci"; + clocks = <&tegra_car TEGRA210_CLK_SDMMC1>, + <&tegra_car TEGRA210_CLK_SDMMC_LEGACY>; + clock-names = "sdhci", "tmclk"; resets = <&tegra_car 14>; reset-names = "sdhci"; pinctrl-names = "sdmmc-3v3", "sdmmc-1v8", @@ -1222,8 +1223,9 @@ compatible = "nvidia,tegra210-sdhci"; reg = <0x0 0x700b0200 0x0 0x200>; interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&tegra_car TEGRA210_CLK_SDMMC2>; - clock-names = "sdhci"; + clocks = <&tegra_car TEGRA210_CLK_SDMMC2>, + <&tegra_car TEGRA210_CLK_SDMMC_LEGACY>; + clock-names = "sdhci", "tmclk"; resets = <&tegra_car 9>; reset-names = "sdhci"; pinctrl-names = "sdmmc-1v8-drv"; @@ -1239,8 +1241,9 @@ compatible = "nvidia,tegra210-sdhci"; reg = <0x0 0x700b0400 0x0 0x200>; interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&tegra_car TEGRA210_CLK_SDMMC3>; - clock-names = "sdhci"; + clocks = <&tegra_car TEGRA210_CLK_SDMMC3>, + <&tegra_car TEGRA210_CLK_SDMMC_LEGACY>; + clock-names = "sdhci", "tmclk"; resets = <&tegra_car 69>; reset-names = "sdhci"; pinctrl-names = "sdmmc-3v3", "sdmmc-1v8", @@ -1262,8 +1265,9 @@ compatible = "nvidia,tegra210-sdhci"; reg = <0x0 0x700b0600 0x0 0x200>; interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&tegra_car TEGRA210_CLK_SDMMC4>; - clock-names = "sdhci"; + clocks = <&tegra_car TEGRA210_CLK_SDMMC4>, + <&tegra_car TEGRA210_CLK_SDMMC_LEGACY>; + clock-names = "sdhci", "tmclk"; resets = <&tegra_car 15>; reset-names = "sdhci"; pinctrl-names = "sdmmc-3v3-drv", "sdmmc-1v8-drv";
commit 39cb62cb8973 ("arm64: tegra: Add Tegra186 support")
Tegra186 uses separate SDMMC_LEGACY_TM clock for data timeout and this clock is not enabled currently which is not recommended.
Tegra186 SDMMC advertises 12Mhz as timeout clock frequency in host capability register and uses it by default.
So, this clock should be kept enabled by the SDMMC driver.
Fixes: 39cb62cb8973 ("arm64: tegra: Add Tegra186 support") Cc: stable stable@vger.kernel.org # 5.4 Tested-by: Jon Hunter jonathanh@nvidia.com Reviewed-by: Jon Hunter jonathanh@nvidia.com Signed-off-by: Sowjanya Komatineni skomatineni@nvidia.com --- arch/arm64/boot/dts/nvidia/tegra186.dtsi | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi index 34d249d..8eb61dd 100644 --- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi +++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi @@ -337,8 +337,9 @@ compatible = "nvidia,tegra186-sdhci"; reg = <0x0 0x03400000 0x0 0x10000>; interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&bpmp TEGRA186_CLK_SDMMC1>; - clock-names = "sdhci"; + clocks = <&bpmp TEGRA186_CLK_SDMMC1>, + <&bpmp TEGRA186_CLK_SDMMC_LEGACY_TM>; + clock-names = "sdhci", "tmclk"; resets = <&bpmp TEGRA186_RESET_SDMMC1>; reset-names = "sdhci"; interconnects = <&mc TEGRA186_MEMORY_CLIENT_SDMMCRA &emc>, @@ -366,8 +367,9 @@ compatible = "nvidia,tegra186-sdhci"; reg = <0x0 0x03420000 0x0 0x10000>; interrupts = <GIC_SPI 63 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&bpmp TEGRA186_CLK_SDMMC2>; - clock-names = "sdhci"; + clocks = <&bpmp TEGRA186_CLK_SDMMC2>, + <&bpmp TEGRA186_CLK_SDMMC_LEGACY_TM>; + clock-names = "sdhci", "tmclk"; resets = <&bpmp TEGRA186_RESET_SDMMC2>; reset-names = "sdhci"; interconnects = <&mc TEGRA186_MEMORY_CLIENT_SDMMCRAA &emc>, @@ -390,8 +392,9 @@ compatible = "nvidia,tegra186-sdhci"; reg = <0x0 0x03440000 0x0 0x10000>; interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&bpmp TEGRA186_CLK_SDMMC3>; - clock-names = "sdhci"; + clocks = <&bpmp TEGRA186_CLK_SDMMC3>, + <&bpmp TEGRA186_CLK_SDMMC_LEGACY_TM>; + clock-names = "sdhci", "tmclk"; resets = <&bpmp TEGRA186_RESET_SDMMC3>; reset-names = "sdhci"; interconnects = <&mc TEGRA186_MEMORY_CLIENT_SDMMCR &emc>, @@ -416,8 +419,9 @@ compatible = "nvidia,tegra186-sdhci"; reg = <0x0 0x03460000 0x0 0x10000>; interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&bpmp TEGRA186_CLK_SDMMC4>; - clock-names = "sdhci"; + clocks = <&bpmp TEGRA186_CLK_SDMMC4>, + <&bpmp TEGRA186_CLK_SDMMC_LEGACY_TM>; + clock-names = "sdhci", "tmclk"; assigned-clocks = <&bpmp TEGRA186_CLK_SDMMC4>, <&bpmp TEGRA186_CLK_PLLC4_VCO>; assigned-clock-parents = <&bpmp TEGRA186_CLK_PLLC4_VCO>;
commit 5425fb15d8ee ("arm64: tegra: Add Tegra194 chip device tree")
Tegra194 uses separate SDMMC_LEGACY_TM clock for data timeout and this clock is not enabled currently which is not recommended.
Tegra194 SDMMC advertises 12Mhz as timeout clock frequency in host capability register.
So, this clock should be kept enabled by SDMMC driver.
Fixes: 5425fb15d8ee ("arm64: tegra: Add Tegra194 chip device tree") Cc: stable stable@vger.kernel.org # 5.4 Tested-by: Jon Hunter jonathanh@nvidia.com Reviewed-by: Jon Hunter jonathanh@nvidia.com Signed-off-by: Sowjanya Komatineni skomatineni@nvidia.com --- arch/arm64/boot/dts/nvidia/tegra194.dtsi | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi index 48160f4..ca5cb6a 100644 --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi @@ -460,8 +460,9 @@ compatible = "nvidia,tegra194-sdhci"; reg = <0x03400000 0x10000>; interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&bpmp TEGRA194_CLK_SDMMC1>; - clock-names = "sdhci"; + clocks = <&bpmp TEGRA194_CLK_SDMMC1>, + <&bpmp TEGRA194_CLK_SDMMC_LEGACY_TM>; + clock-names = "sdhci", "tmclk"; resets = <&bpmp TEGRA194_RESET_SDMMC1>; reset-names = "sdhci"; interconnects = <&mc TEGRA194_MEMORY_CLIENT_SDMMCRA &emc>, @@ -485,8 +486,9 @@ compatible = "nvidia,tegra194-sdhci"; reg = <0x03440000 0x10000>; interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&bpmp TEGRA194_CLK_SDMMC3>; - clock-names = "sdhci"; + clocks = <&bpmp TEGRA194_CLK_SDMMC3>, + <&bpmp TEGRA194_CLK_SDMMC_LEGACY_TM>; + clock-names = "sdhci", "tmclk"; resets = <&bpmp TEGRA194_RESET_SDMMC3>; reset-names = "sdhci"; interconnects = <&mc TEGRA194_MEMORY_CLIENT_SDMMCR &emc>, @@ -511,8 +513,9 @@ compatible = "nvidia,tegra194-sdhci"; reg = <0x03460000 0x10000>; interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&bpmp TEGRA194_CLK_SDMMC4>; - clock-names = "sdhci"; + clocks = <&bpmp TEGRA194_CLK_SDMMC4>, + <&bpmp TEGRA194_CLK_SDMMC_LEGACY_TM>; + clock-names = "sdhci", "tmclk"; assigned-clocks = <&bpmp TEGRA194_CLK_SDMMC4>, <&bpmp TEGRA194_CLK_PLLC4>; assigned-clock-parents =
commit b5a84ecf025a ("mmc: tegra: Add Tegra210 support")
Tegra210 and later has a separate sdmmc_legacy_tm (TMCLK) used by Tegra SDMMC hawdware for data timeout to achive better timeout than using SDCLK and using TMCLK is recommended.
USE_TMCLK_FOR_DATA_TIMEOUT bit in Tegra SDMMC register SDHCI_TEGRA_VENDOR_SYS_SW_CTRL can be used to choose either TMCLK or SDCLK for data timeout.
Default USE_TMCLK_FOR_DATA_TIMEOUT bit is set to 1 and TMCLK is used for data timeout by Tegra SDMMC hardware and having TMCLK not enabled is not recommended.
So, this patch adds quirk NVQUIRK_HAS_TMCLK for SoC having separate timeout clock and keeps TMCLK enabled all the time.
Fixes: b5a84ecf025a ("mmc: tegra: Add Tegra210 support") Cc: stable stable@vger.kernel.org # 5.4 Tested-by: Jon Hunter jonathanh@nvidia.com Reviewed-by: Jon Hunter jonathanh@nvidia.com Acked-by: Adrian Hunter adrian.hunter@intel.com Signed-off-by: Sowjanya Komatineni skomatineni@nvidia.com --- drivers/mmc/host/sdhci-tegra.c | 90 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 82 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c index 31ed321..f69ca8d 100644 --- a/drivers/mmc/host/sdhci-tegra.c +++ b/drivers/mmc/host/sdhci-tegra.c @@ -13,6 +13,7 @@ #include <linux/clk.h> #include <linux/io.h> #include <linux/of.h> +#include <linux/of_clk.h> #include <linux/of_device.h> #include <linux/pinctrl/consumer.h> #include <linux/regulator/consumer.h> @@ -110,6 +111,12 @@ #define NVQUIRK_DIS_CARD_CLK_CONFIG_TAP BIT(8) #define NVQUIRK_CQHCI_DCMD_R1B_CMD_TIMING BIT(9)
+/* + * NVQUIRK_HAS_TMCLK is for SoC's having separate timeout clock for Tegra + * SDMMC hardware data timeout. + */ +#define NVQUIRK_HAS_TMCLK BIT(10) + /* SDMMC CQE Base Address for Tegra Host Ver 4.1 and Higher */ #define SDHCI_TEGRA_CQE_BASE_ADDR 0xF000
@@ -140,6 +147,7 @@ struct sdhci_tegra_autocal_offsets { struct sdhci_tegra { const struct sdhci_tegra_soc_data *soc_data; struct gpio_desc *power_gpio; + struct clk *tmclk; bool ddr_signaling; bool pad_calib_required; bool pad_control_available; @@ -1433,7 +1441,8 @@ static const struct sdhci_tegra_soc_data soc_data_tegra210 = { NVQUIRK_HAS_PADCALIB | NVQUIRK_DIS_CARD_CLK_CONFIG_TAP | NVQUIRK_ENABLE_SDR50 | - NVQUIRK_ENABLE_SDR104, + NVQUIRK_ENABLE_SDR104 | + NVQUIRK_HAS_TMCLK, .min_tap_delay = 106, .max_tap_delay = 185, }; @@ -1471,6 +1480,7 @@ static const struct sdhci_tegra_soc_data soc_data_tegra186 = { NVQUIRK_DIS_CARD_CLK_CONFIG_TAP | NVQUIRK_ENABLE_SDR50 | NVQUIRK_ENABLE_SDR104 | + NVQUIRK_HAS_TMCLK | NVQUIRK_CQHCI_DCMD_R1B_CMD_TIMING, .min_tap_delay = 84, .max_tap_delay = 136, @@ -1483,7 +1493,8 @@ static const struct sdhci_tegra_soc_data soc_data_tegra194 = { NVQUIRK_HAS_PADCALIB | NVQUIRK_DIS_CARD_CLK_CONFIG_TAP | NVQUIRK_ENABLE_SDR50 | - NVQUIRK_ENABLE_SDR104, + NVQUIRK_ENABLE_SDR104 | + NVQUIRK_HAS_TMCLK, .min_tap_delay = 96, .max_tap_delay = 139, }; @@ -1611,15 +1622,76 @@ static int sdhci_tegra_probe(struct platform_device *pdev) goto err_power_req; }
- clk = devm_clk_get(mmc_dev(host->mmc), NULL); - if (IS_ERR(clk)) { - rc = PTR_ERR(clk); + /* + * Tegra210 and later has separate SDMMC_LEGACY_TM clock used for + * hardware data timeout clock and SW can choose TMCLK or SDCLK for + * hardware data timeout through the bit USE_TMCLK_FOR_DATA_TIMEOUT + * of the register SDHCI_TEGRA_VENDOR_SYS_SW_CTRL. + * + * USE_TMCLK_FOR_DATA_TIMEOUT bit default is set to 1 and SDMMC uses + * 12Mhz TMCLK which is advertised in host capability register. + * With TMCLK of 12Mhz provides maximum data timeout period that can + * be achieved is 11s better than using SDCLK for data timeout. + * + * So, TMCLK is set to 12Mhz and kept enabled all the time on SoC's + * supporting separate TMCLK. + * + * Old device tree has single sdhci clock. So with addition of TMCLK, + * retrieving sdhci clock by "sdhci" clock name based on number of + * clocks in sdhci device node. + */ + + if (of_clk_get_parent_count(pdev->dev.of_node) == 1) { + if (soc_data->nvquirks & NVQUIRK_HAS_TMCLK) + dev_warn(&pdev->dev, + "missing tmclk in the device tree\n"); + + clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(clk)) { + rc = PTR_ERR(clk);
- if (rc != -EPROBE_DEFER) - dev_err(&pdev->dev, "failed to get clock: %d\n", rc); + if (rc != -EPROBE_DEFER) + dev_err(&pdev->dev, + "failed to get sdhci clock: %d\n", rc);
- goto err_clk_get; + goto err_power_req; + } + } else { + if (soc_data->nvquirks & NVQUIRK_HAS_TMCLK) { + clk = devm_clk_get(&pdev->dev, "tmclk"); + if (IS_ERR(clk)) { + rc = PTR_ERR(clk); + if (rc == -EPROBE_DEFER) + goto err_power_req; + + dev_warn(&pdev->dev, + "failed to get tmclk: %d\n", rc); + clk = NULL; + } + + clk_set_rate(clk, 12000000); + rc = clk_prepare_enable(clk); + if (rc) { + dev_err(&pdev->dev, + "failed to enable tmclk: %d\n", rc); + goto err_power_req; + } + + tegra_host->tmclk = clk; + } + + clk = devm_clk_get(&pdev->dev, "sdhci"); + if (IS_ERR(clk)) { + rc = PTR_ERR(clk); + + if (rc != -EPROBE_DEFER) + dev_err(&pdev->dev, + "failed to get sdhci clock: %d\n", rc); + + goto err_clk_get; + } } + clk_prepare_enable(clk); pltfm_host->clk = clk;
@@ -1654,6 +1726,7 @@ static int sdhci_tegra_probe(struct platform_device *pdev) err_rst_get: clk_disable_unprepare(pltfm_host->clk); err_clk_get: + clk_disable_unprepare(tegra_host->tmclk); err_power_req: err_parse_dt: sdhci_pltfm_free(pdev); @@ -1671,6 +1744,7 @@ static int sdhci_tegra_remove(struct platform_device *pdev) reset_control_assert(tegra_host->rst); usleep_range(2000, 4000); clk_disable_unprepare(pltfm_host->clk); + clk_disable_unprepare(tegra_host->tmclk);
sdhci_pltfm_free(pdev);
On 27/08/2020 04:50, Sowjanya Komatineni wrote:
commit b5a84ecf025a ("mmc: tegra: Add Tegra210 support")
Tegra210 and later has a separate sdmmc_legacy_tm (TMCLK) used by Tegra SDMMC hawdware for data timeout to achive better timeout than using SDCLK and using TMCLK is recommended.
USE_TMCLK_FOR_DATA_TIMEOUT bit in Tegra SDMMC register SDHCI_TEGRA_VENDOR_SYS_SW_CTRL can be used to choose either TMCLK or SDCLK for data timeout.
Default USE_TMCLK_FOR_DATA_TIMEOUT bit is set to 1 and TMCLK is used for data timeout by Tegra SDMMC hardware and having TMCLK not enabled is not recommended.
So, this patch adds quirk NVQUIRK_HAS_TMCLK for SoC having separate timeout clock and keeps TMCLK enabled all the time.
Fixes: b5a84ecf025a ("mmc: tegra: Add Tegra210 support") Cc: stable stable@vger.kernel.org # 5.4 Tested-by: Jon Hunter jonathanh@nvidia.com Reviewed-by: Jon Hunter jonathanh@nvidia.com Acked-by: Adrian Hunter adrian.hunter@intel.com Signed-off-by: Sowjanya Komatineni skomatineni@nvidia.com
drivers/mmc/host/sdhci-tegra.c | 90 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 82 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c index 31ed321..f69ca8d 100644 --- a/drivers/mmc/host/sdhci-tegra.c +++ b/drivers/mmc/host/sdhci-tegra.c @@ -13,6 +13,7 @@ #include <linux/clk.h> #include <linux/io.h> #include <linux/of.h> +#include <linux/of_clk.h> #include <linux/of_device.h> #include <linux/pinctrl/consumer.h> #include <linux/regulator/consumer.h> @@ -110,6 +111,12 @@ #define NVQUIRK_DIS_CARD_CLK_CONFIG_TAP BIT(8) #define NVQUIRK_CQHCI_DCMD_R1B_CMD_TIMING BIT(9) +/*
- NVQUIRK_HAS_TMCLK is for SoC's having separate timeout clock for Tegra
- SDMMC hardware data timeout.
- */
+#define NVQUIRK_HAS_TMCLK BIT(10)
/* SDMMC CQE Base Address for Tegra Host Ver 4.1 and Higher */ #define SDHCI_TEGRA_CQE_BASE_ADDR 0xF000 @@ -140,6 +147,7 @@ struct sdhci_tegra_autocal_offsets { struct sdhci_tegra { const struct sdhci_tegra_soc_data *soc_data; struct gpio_desc *power_gpio;
- struct clk *tmclk; bool ddr_signaling; bool pad_calib_required; bool pad_control_available;
@@ -1433,7 +1441,8 @@ static const struct sdhci_tegra_soc_data soc_data_tegra210 = { NVQUIRK_HAS_PADCALIB | NVQUIRK_DIS_CARD_CLK_CONFIG_TAP | NVQUIRK_ENABLE_SDR50 |
NVQUIRK_ENABLE_SDR104,
NVQUIRK_ENABLE_SDR104 |NVQUIRK_HAS_TMCLK,}; @@ -1471,6 +1480,7 @@ static const struct sdhci_tegra_soc_data soc_data_tegra186 = { NVQUIRK_DIS_CARD_CLK_CONFIG_TAP | NVQUIRK_ENABLE_SDR50 | NVQUIRK_ENABLE_SDR104 |
NVQUIRK_HAS_TMCLK |@@ -1483,7 +1493,8 @@ static const struct sdhci_tegra_soc_data soc_data_tegra194 = { NVQUIRK_HAS_PADCALIB | NVQUIRK_DIS_CARD_CLK_CONFIG_TAP | NVQUIRK_ENABLE_SDR50 |
NVQUIRK_ENABLE_SDR104,
NVQUIRK_ENABLE_SDR104 |NVQUIRK_HAS_TMCLK,}; @@ -1611,15 +1622,76 @@ static int sdhci_tegra_probe(struct platform_device *pdev) goto err_power_req; }
- clk = devm_clk_get(mmc_dev(host->mmc), NULL);
- if (IS_ERR(clk)) {
rc = PTR_ERR(clk);
- /*
* Tegra210 and later has separate SDMMC_LEGACY_TM clock used for* hardware data timeout clock and SW can choose TMCLK or SDCLK for* hardware data timeout through the bit USE_TMCLK_FOR_DATA_TIMEOUT* of the register SDHCI_TEGRA_VENDOR_SYS_SW_CTRL.** USE_TMCLK_FOR_DATA_TIMEOUT bit default is set to 1 and SDMMC uses* 12Mhz TMCLK which is advertised in host capability register.* With TMCLK of 12Mhz provides maximum data timeout period that can* be achieved is 11s better than using SDCLK for data timeout.** So, TMCLK is set to 12Mhz and kept enabled all the time on SoC's* supporting separate TMCLK.** Old device tree has single sdhci clock. So with addition of TMCLK,* retrieving sdhci clock by "sdhci" clock name based on number of* clocks in sdhci device node.*/- if (of_clk_get_parent_count(pdev->dev.of_node) == 1) {
if (soc_data->nvquirks & NVQUIRK_HAS_TMCLK)dev_warn(&pdev->dev,"missing tmclk in the device tree\n");clk = devm_clk_get(&pdev->dev, NULL);if (IS_ERR(clk)) {rc = PTR_ERR(clk);
if (rc != -EPROBE_DEFER)dev_err(&pdev->dev, "failed to get clock: %d\n", rc);
if (rc != -EPROBE_DEFER)dev_err(&pdev->dev,"failed to get sdhci clock: %d\n", rc);
goto err_clk_get;
goto err_power_req;}- } else {
if (soc_data->nvquirks & NVQUIRK_HAS_TMCLK) {
I think that I would do the inverse of this ...
} else { if (!(soc_data->nvquirks & NVQUIRK_HAS_TMCLK)) { dev_err(&pdev->dev, "Device has unexpected clocks!\n"); rc = -EINVAL; goto_power_req; }
clk = devm_clk_get(&pdev->dev, "tmclk"); ...
If the device does not have a single clock, then we expect it to support the tmclk. If this is not the case, then this is a bug.
Cheers Jon
On 8/27/20 1:40 AM, Jon Hunter wrote:
On 27/08/2020 04:50, Sowjanya Komatineni wrote:
commit b5a84ecf025a ("mmc: tegra: Add Tegra210 support")
Tegra210 and later has a separate sdmmc_legacy_tm (TMCLK) used by Tegra SDMMC hawdware for data timeout to achive better timeout than using SDCLK and using TMCLK is recommended.
USE_TMCLK_FOR_DATA_TIMEOUT bit in Tegra SDMMC register SDHCI_TEGRA_VENDOR_SYS_SW_CTRL can be used to choose either TMCLK or SDCLK for data timeout.
Default USE_TMCLK_FOR_DATA_TIMEOUT bit is set to 1 and TMCLK is used for data timeout by Tegra SDMMC hardware and having TMCLK not enabled is not recommended.
So, this patch adds quirk NVQUIRK_HAS_TMCLK for SoC having separate timeout clock and keeps TMCLK enabled all the time.
Fixes: b5a84ecf025a ("mmc: tegra: Add Tegra210 support") Cc: stable stable@vger.kernel.org # 5.4 Tested-by: Jon Hunter jonathanh@nvidia.com Reviewed-by: Jon Hunter jonathanh@nvidia.com Acked-by: Adrian Hunter adrian.hunter@intel.com Signed-off-by: Sowjanya Komatineni skomatineni@nvidia.com
drivers/mmc/host/sdhci-tegra.c | 90 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 82 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c index 31ed321..f69ca8d 100644 --- a/drivers/mmc/host/sdhci-tegra.c +++ b/drivers/mmc/host/sdhci-tegra.c @@ -13,6 +13,7 @@ #include <linux/clk.h> #include <linux/io.h> #include <linux/of.h> +#include <linux/of_clk.h> #include <linux/of_device.h> #include <linux/pinctrl/consumer.h> #include <linux/regulator/consumer.h> @@ -110,6 +111,12 @@ #define NVQUIRK_DIS_CARD_CLK_CONFIG_TAP BIT(8) #define NVQUIRK_CQHCI_DCMD_R1B_CMD_TIMING BIT(9) +/*
- NVQUIRK_HAS_TMCLK is for SoC's having separate timeout clock for Tegra
- SDMMC hardware data timeout.
- */
+#define NVQUIRK_HAS_TMCLK BIT(10)
- /* SDMMC CQE Base Address for Tegra Host Ver 4.1 and Higher */ #define SDHCI_TEGRA_CQE_BASE_ADDR 0xF000
@@ -140,6 +147,7 @@ struct sdhci_tegra_autocal_offsets { struct sdhci_tegra { const struct sdhci_tegra_soc_data *soc_data; struct gpio_desc *power_gpio;
- struct clk *tmclk; bool ddr_signaling; bool pad_calib_required; bool pad_control_available;
@@ -1433,7 +1441,8 @@ static const struct sdhci_tegra_soc_data soc_data_tegra210 = { NVQUIRK_HAS_PADCALIB | NVQUIRK_DIS_CARD_CLK_CONFIG_TAP | NVQUIRK_ENABLE_SDR50 |
NVQUIRK_ENABLE_SDR104,
NVQUIRK_ENABLE_SDR104 |NVQUIRK_HAS_TMCLK,@@ -1471,6 +1480,7 @@ static const struct sdhci_tegra_soc_data soc_data_tegra186 = { NVQUIRK_DIS_CARD_CLK_CONFIG_TAP | NVQUIRK_ENABLE_SDR50 | NVQUIRK_ENABLE_SDR104 |
NVQUIRK_HAS_TMCLK |@@ -1483,7 +1493,8 @@ static const struct sdhci_tegra_soc_data soc_data_tegra194 = { NVQUIRK_HAS_PADCALIB | NVQUIRK_DIS_CARD_CLK_CONFIG_TAP | NVQUIRK_ENABLE_SDR50 |
NVQUIRK_ENABLE_SDR104,
NVQUIRK_ENABLE_SDR104 |NVQUIRK_HAS_TMCLK,@@ -1611,15 +1622,76 @@ static int sdhci_tegra_probe(struct platform_device *pdev) goto err_power_req; }
- clk = devm_clk_get(mmc_dev(host->mmc), NULL);
- if (IS_ERR(clk)) {
rc = PTR_ERR(clk);
- /*
* Tegra210 and later has separate SDMMC_LEGACY_TM clock used for* hardware data timeout clock and SW can choose TMCLK or SDCLK for* hardware data timeout through the bit USE_TMCLK_FOR_DATA_TIMEOUT* of the register SDHCI_TEGRA_VENDOR_SYS_SW_CTRL.** USE_TMCLK_FOR_DATA_TIMEOUT bit default is set to 1 and SDMMC uses* 12Mhz TMCLK which is advertised in host capability register.* With TMCLK of 12Mhz provides maximum data timeout period that can* be achieved is 11s better than using SDCLK for data timeout.** So, TMCLK is set to 12Mhz and kept enabled all the time on SoC's* supporting separate TMCLK.** Old device tree has single sdhci clock. So with addition of TMCLK,* retrieving sdhci clock by "sdhci" clock name based on number of* clocks in sdhci device node.*/- if (of_clk_get_parent_count(pdev->dev.of_node) == 1) {
if (soc_data->nvquirks & NVQUIRK_HAS_TMCLK)dev_warn(&pdev->dev,"missing tmclk in the device tree\n");clk = devm_clk_get(&pdev->dev, NULL);if (IS_ERR(clk)) {rc = PTR_ERR(clk);
if (rc != -EPROBE_DEFER)dev_err(&pdev->dev, "failed to get clock: %d\n", rc);
if (rc != -EPROBE_DEFER)dev_err(&pdev->dev,"failed to get sdhci clock: %d\n", rc);
goto err_clk_get;
goto err_power_req;}- } else {
if (soc_data->nvquirks & NVQUIRK_HAS_TMCLK) {I think that I would do the inverse of this ...
} else { if (!(soc_data->nvquirks & NVQUIRK_HAS_TMCLK)) { dev_err(&pdev->dev, "Device has unexpected clocks!\n"); rc = -EINVAL; goto_power_req; } clk = devm_clk_get(&pdev->dev, "tmclk"); ...If the device does not have a single clock, then we expect it to support the tmclk. If this is not the case, then this is a bug.
Cheers Jon
I don't see other drivers validating for unexpected device tree entries.
Also only for SoC with quirk HAS_TMCLK, we are retrieving TMCLK with clock name and enabling it.
So for other SoC even if device tree has additional clock entry other than sdhci driver don't use it and also dt-binding do not have any tmclk entry for other SoC. So why would this be a bug?
Can you please correct if I misunderstood you comment?
Thanks
Sowjanya
On 27/08/2020 16:03, Sowjanya Komatineni wrote:
On 8/27/20 1:40 AM, Jon Hunter wrote:
On 27/08/2020 04:50, Sowjanya Komatineni wrote:
commit b5a84ecf025a ("mmc: tegra: Add Tegra210 support")
Tegra210 and later has a separate sdmmc_legacy_tm (TMCLK) used by Tegra SDMMC hawdware for data timeout to achive better timeout than using SDCLK and using TMCLK is recommended.
USE_TMCLK_FOR_DATA_TIMEOUT bit in Tegra SDMMC register SDHCI_TEGRA_VENDOR_SYS_SW_CTRL can be used to choose either TMCLK or SDCLK for data timeout.
Default USE_TMCLK_FOR_DATA_TIMEOUT bit is set to 1 and TMCLK is used for data timeout by Tegra SDMMC hardware and having TMCLK not enabled is not recommended.
So, this patch adds quirk NVQUIRK_HAS_TMCLK for SoC having separate timeout clock and keeps TMCLK enabled all the time.
Fixes: b5a84ecf025a ("mmc: tegra: Add Tegra210 support") Cc: stable stable@vger.kernel.org # 5.4 Tested-by: Jon Hunter jonathanh@nvidia.com Reviewed-by: Jon Hunter jonathanh@nvidia.com Acked-by: Adrian Hunter adrian.hunter@intel.com Signed-off-by: Sowjanya Komatineni skomatineni@nvidia.com
drivers/mmc/host/sdhci-tegra.c | 90 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 82 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c index 31ed321..f69ca8d 100644 --- a/drivers/mmc/host/sdhci-tegra.c +++ b/drivers/mmc/host/sdhci-tegra.c @@ -13,6 +13,7 @@ #include <linux/clk.h> #include <linux/io.h> #include <linux/of.h> +#include <linux/of_clk.h> #include <linux/of_device.h> #include <linux/pinctrl/consumer.h> #include <linux/regulator/consumer.h> @@ -110,6 +111,12 @@ #define NVQUIRK_DIS_CARD_CLK_CONFIG_TAP BIT(8) #define NVQUIRK_CQHCI_DCMD_R1B_CMD_TIMING BIT(9) +/*
- NVQUIRK_HAS_TMCLK is for SoC's having separate timeout clock for
Tegra
- SDMMC hardware data timeout.
- */
+#define NVQUIRK_HAS_TMCLK BIT(10)
/* SDMMC CQE Base Address for Tegra Host Ver 4.1 and Higher */ #define SDHCI_TEGRA_CQE_BASE_ADDR 0xF000 @@ -140,6 +147,7 @@ struct sdhci_tegra_autocal_offsets { struct sdhci_tegra { const struct sdhci_tegra_soc_data *soc_data; struct gpio_desc *power_gpio; + struct clk *tmclk; bool ddr_signaling; bool pad_calib_required; bool pad_control_available; @@ -1433,7 +1441,8 @@ static const struct sdhci_tegra_soc_data soc_data_tegra210 = { NVQUIRK_HAS_PADCALIB | NVQUIRK_DIS_CARD_CLK_CONFIG_TAP | NVQUIRK_ENABLE_SDR50 | - NVQUIRK_ENABLE_SDR104, + NVQUIRK_ENABLE_SDR104 | + NVQUIRK_HAS_TMCLK, .min_tap_delay = 106, .max_tap_delay = 185, }; @@ -1471,6 +1480,7 @@ static const struct sdhci_tegra_soc_data soc_data_tegra186 = { NVQUIRK_DIS_CARD_CLK_CONFIG_TAP | NVQUIRK_ENABLE_SDR50 | NVQUIRK_ENABLE_SDR104 | + NVQUIRK_HAS_TMCLK | NVQUIRK_CQHCI_DCMD_R1B_CMD_TIMING, .min_tap_delay = 84, .max_tap_delay = 136, @@ -1483,7 +1493,8 @@ static const struct sdhci_tegra_soc_data soc_data_tegra194 = { NVQUIRK_HAS_PADCALIB | NVQUIRK_DIS_CARD_CLK_CONFIG_TAP | NVQUIRK_ENABLE_SDR50 | - NVQUIRK_ENABLE_SDR104, + NVQUIRK_ENABLE_SDR104 | + NVQUIRK_HAS_TMCLK, .min_tap_delay = 96, .max_tap_delay = 139, }; @@ -1611,15 +1622,76 @@ static int sdhci_tegra_probe(struct platform_device *pdev) goto err_power_req; } - clk = devm_clk_get(mmc_dev(host->mmc), NULL); - if (IS_ERR(clk)) { - rc = PTR_ERR(clk); + /* + * Tegra210 and later has separate SDMMC_LEGACY_TM clock used for + * hardware data timeout clock and SW can choose TMCLK or SDCLK for + * hardware data timeout through the bit USE_TMCLK_FOR_DATA_TIMEOUT + * of the register SDHCI_TEGRA_VENDOR_SYS_SW_CTRL. + * + * USE_TMCLK_FOR_DATA_TIMEOUT bit default is set to 1 and SDMMC uses + * 12Mhz TMCLK which is advertised in host capability register. + * With TMCLK of 12Mhz provides maximum data timeout period that can + * be achieved is 11s better than using SDCLK for data timeout. + * + * So, TMCLK is set to 12Mhz and kept enabled all the time on SoC's + * supporting separate TMCLK. + * + * Old device tree has single sdhci clock. So with addition of TMCLK, + * retrieving sdhci clock by "sdhci" clock name based on number of + * clocks in sdhci device node. + */
+ if (of_clk_get_parent_count(pdev->dev.of_node) == 1) { + if (soc_data->nvquirks & NVQUIRK_HAS_TMCLK) + dev_warn(&pdev->dev, + "missing tmclk in the device tree\n");
+ clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(clk)) { + rc = PTR_ERR(clk); - if (rc != -EPROBE_DEFER) - dev_err(&pdev->dev, "failed to get clock: %d\n", rc); + if (rc != -EPROBE_DEFER) + dev_err(&pdev->dev, + "failed to get sdhci clock: %d\n", rc); - goto err_clk_get; + goto err_power_req; + } + } else { + if (soc_data->nvquirks & NVQUIRK_HAS_TMCLK) {
I think that I would do the inverse of this ...
} else { if (!(soc_data->nvquirks & NVQUIRK_HAS_TMCLK)) { dev_err(&pdev->dev, "Device has unexpected clocks!\n"); rc = -EINVAL; goto_power_req; }
clk = devm_clk_get(&pdev->dev, "tmclk"); ...
If the device does not have a single clock, then we expect it to support the tmclk. If this is not the case, then this is a bug.
Cheers Jon
I don't see other drivers validating for unexpected device tree entries.
Also only for SoC with quirk HAS_TMCLK, we are retrieving TMCLK with clock name and enabling it.
So for other SoC even if device tree has additional clock entry other than sdhci driver don't use it and also dt-binding do not have any tmclk entry for other SoC. So why would this be a bug?
In the device tree binding doc, we say has two clocks for Tegra210, Tegra186 and Tegra194 and one clock for all other devices. So if we no there is more than 1 but the device does not have this quirk, then the device-tree does not reflect what is stated in the binding doc or the quirk is no populated as it should be. I feel that either case is a bug.
Now of course it could be possible for someone to add a 3rd clock for Tegra210 and we would not detect this but like you said we don't check all conditions. So yes we don't catch all cases, but the ones that matter.
Jon
On 8/27/20 8:14 AM, Jon Hunter wrote:
On 27/08/2020 16:03, Sowjanya Komatineni wrote:
On 8/27/20 1:40 AM, Jon Hunter wrote:
On 27/08/2020 04:50, Sowjanya Komatineni wrote:
commit b5a84ecf025a ("mmc: tegra: Add Tegra210 support")
Tegra210 and later has a separate sdmmc_legacy_tm (TMCLK) used by Tegra SDMMC hawdware for data timeout to achive better timeout than using SDCLK and using TMCLK is recommended.
USE_TMCLK_FOR_DATA_TIMEOUT bit in Tegra SDMMC register SDHCI_TEGRA_VENDOR_SYS_SW_CTRL can be used to choose either TMCLK or SDCLK for data timeout.
Default USE_TMCLK_FOR_DATA_TIMEOUT bit is set to 1 and TMCLK is used for data timeout by Tegra SDMMC hardware and having TMCLK not enabled is not recommended.
So, this patch adds quirk NVQUIRK_HAS_TMCLK for SoC having separate timeout clock and keeps TMCLK enabled all the time.
Fixes: b5a84ecf025a ("mmc: tegra: Add Tegra210 support") Cc: stable stable@vger.kernel.org # 5.4 Tested-by: Jon Hunter jonathanh@nvidia.com Reviewed-by: Jon Hunter jonathanh@nvidia.com Acked-by: Adrian Hunter adrian.hunter@intel.com Signed-off-by: Sowjanya Komatineni skomatineni@nvidia.com
drivers/mmc/host/sdhci-tegra.c | 90 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 82 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c index 31ed321..f69ca8d 100644 --- a/drivers/mmc/host/sdhci-tegra.c +++ b/drivers/mmc/host/sdhci-tegra.c @@ -13,6 +13,7 @@ #include <linux/clk.h> #include <linux/io.h> #include <linux/of.h> +#include <linux/of_clk.h> #include <linux/of_device.h> #include <linux/pinctrl/consumer.h> #include <linux/regulator/consumer.h> @@ -110,6 +111,12 @@ #define NVQUIRK_DIS_CARD_CLK_CONFIG_TAP BIT(8) #define NVQUIRK_CQHCI_DCMD_R1B_CMD_TIMING BIT(9) +/*
- NVQUIRK_HAS_TMCLK is for SoC's having separate timeout clock for
Tegra
- SDMMC hardware data timeout.
- */
+#define NVQUIRK_HAS_TMCLK BIT(10)
/* SDMMC CQE Base Address for Tegra Host Ver 4.1 and Higher */ #define SDHCI_TEGRA_CQE_BASE_ADDR 0xF000 @@ -140,6 +147,7 @@ struct sdhci_tegra_autocal_offsets { struct sdhci_tegra { const struct sdhci_tegra_soc_data *soc_data; struct gpio_desc *power_gpio; + struct clk *tmclk; bool ddr_signaling; bool pad_calib_required; bool pad_control_available; @@ -1433,7 +1441,8 @@ static const struct sdhci_tegra_soc_data soc_data_tegra210 = { NVQUIRK_HAS_PADCALIB | NVQUIRK_DIS_CARD_CLK_CONFIG_TAP | NVQUIRK_ENABLE_SDR50 | - NVQUIRK_ENABLE_SDR104, + NVQUIRK_ENABLE_SDR104 | + NVQUIRK_HAS_TMCLK, .min_tap_delay = 106, .max_tap_delay = 185, }; @@ -1471,6 +1480,7 @@ static const struct sdhci_tegra_soc_data soc_data_tegra186 = { NVQUIRK_DIS_CARD_CLK_CONFIG_TAP | NVQUIRK_ENABLE_SDR50 | NVQUIRK_ENABLE_SDR104 | + NVQUIRK_HAS_TMCLK | NVQUIRK_CQHCI_DCMD_R1B_CMD_TIMING, .min_tap_delay = 84, .max_tap_delay = 136, @@ -1483,7 +1493,8 @@ static const struct sdhci_tegra_soc_data soc_data_tegra194 = { NVQUIRK_HAS_PADCALIB | NVQUIRK_DIS_CARD_CLK_CONFIG_TAP | NVQUIRK_ENABLE_SDR50 | - NVQUIRK_ENABLE_SDR104, + NVQUIRK_ENABLE_SDR104 | + NVQUIRK_HAS_TMCLK, .min_tap_delay = 96, .max_tap_delay = 139, }; @@ -1611,15 +1622,76 @@ static int sdhci_tegra_probe(struct platform_device *pdev) goto err_power_req; } - clk = devm_clk_get(mmc_dev(host->mmc), NULL); - if (IS_ERR(clk)) { - rc = PTR_ERR(clk); + /* + * Tegra210 and later has separate SDMMC_LEGACY_TM clock used for + * hardware data timeout clock and SW can choose TMCLK or SDCLK for + * hardware data timeout through the bit USE_TMCLK_FOR_DATA_TIMEOUT + * of the register SDHCI_TEGRA_VENDOR_SYS_SW_CTRL. + * + * USE_TMCLK_FOR_DATA_TIMEOUT bit default is set to 1 and SDMMC uses + * 12Mhz TMCLK which is advertised in host capability register. + * With TMCLK of 12Mhz provides maximum data timeout period that can + * be achieved is 11s better than using SDCLK for data timeout. + * + * So, TMCLK is set to 12Mhz and kept enabled all the time on SoC's + * supporting separate TMCLK. + * + * Old device tree has single sdhci clock. So with addition of TMCLK, + * retrieving sdhci clock by "sdhci" clock name based on number of + * clocks in sdhci device node. + */
+ if (of_clk_get_parent_count(pdev->dev.of_node) == 1) { + if (soc_data->nvquirks & NVQUIRK_HAS_TMCLK) + dev_warn(&pdev->dev, + "missing tmclk in the device tree\n");
+ clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(clk)) { + rc = PTR_ERR(clk); - if (rc != -EPROBE_DEFER) - dev_err(&pdev->dev, "failed to get clock: %d\n", rc); + if (rc != -EPROBE_DEFER) + dev_err(&pdev->dev, + "failed to get sdhci clock: %d\n", rc); - goto err_clk_get; + goto err_power_req; + } + } else { + if (soc_data->nvquirks & NVQUIRK_HAS_TMCLK) {
I think that I would do the inverse of this ...
} else { if (!(soc_data->nvquirks & NVQUIRK_HAS_TMCLK)) { dev_err(&pdev->dev, "Device has unexpected clocks!\n"); rc = -EINVAL; goto_power_req; }
clk = devm_clk_get(&pdev->dev, "tmclk"); ...
If the device does not have a single clock, then we expect it to support the tmclk. If this is not the case, then this is a bug.
Cheers Jon
I don't see other drivers validating for unexpected device tree entries.
Also only for SoC with quirk HAS_TMCLK, we are retrieving TMCLK with clock name and enabling it.
So for other SoC even if device tree has additional clock entry other than sdhci driver don't use it and also dt-binding do not have any tmclk entry for other SoC. So why would this be a bug?
In the device tree binding doc, we say has two clocks for Tegra210, Tegra186 and Tegra194 and one clock for all other devices. So if we no there is more than 1 but the device does not have this quirk, then the device-tree does not reflect what is stated in the binding doc or the quirk is no populated as it should be. I feel that either case is a bug.
Now of course it could be possible for someone to add a 3rd clock for Tegra210 and we would not detect this but like you said we don't check all conditions. So yes we don't catch all cases, but the ones that matter.
Jon
Based on internal discussion with Thierry we don't need to handle clocks
order in driver. So will revert clock retrieval to same as in v4 and will send v7 series.
Thanks
Sowjanya
On 27/08/2020 16:43, Sowjanya Komatineni wrote:
On 8/27/20 8:14 AM, Jon Hunter wrote:
On 27/08/2020 16:03, Sowjanya Komatineni wrote:
On 8/27/20 1:40 AM, Jon Hunter wrote:
On 27/08/2020 04:50, Sowjanya Komatineni wrote:
commit b5a84ecf025a ("mmc: tegra: Add Tegra210 support")
Tegra210 and later has a separate sdmmc_legacy_tm (TMCLK) used by Tegra SDMMC hawdware for data timeout to achive better timeout than using SDCLK and using TMCLK is recommended.
USE_TMCLK_FOR_DATA_TIMEOUT bit in Tegra SDMMC register SDHCI_TEGRA_VENDOR_SYS_SW_CTRL can be used to choose either TMCLK or SDCLK for data timeout.
Default USE_TMCLK_FOR_DATA_TIMEOUT bit is set to 1 and TMCLK is used for data timeout by Tegra SDMMC hardware and having TMCLK not enabled is not recommended.
So, this patch adds quirk NVQUIRK_HAS_TMCLK for SoC having separate timeout clock and keeps TMCLK enabled all the time.
Fixes: b5a84ecf025a ("mmc: tegra: Add Tegra210 support") Cc: stable stable@vger.kernel.org # 5.4 Tested-by: Jon Hunter jonathanh@nvidia.com Reviewed-by: Jon Hunter jonathanh@nvidia.com Acked-by: Adrian Hunter adrian.hunter@intel.com Signed-off-by: Sowjanya Komatineni skomatineni@nvidia.com
drivers/mmc/host/sdhci-tegra.c | 90 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 82 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c index 31ed321..f69ca8d 100644 --- a/drivers/mmc/host/sdhci-tegra.c +++ b/drivers/mmc/host/sdhci-tegra.c @@ -13,6 +13,7 @@ #include <linux/clk.h> #include <linux/io.h> #include <linux/of.h> +#include <linux/of_clk.h> #include <linux/of_device.h> #include <linux/pinctrl/consumer.h> #include <linux/regulator/consumer.h> @@ -110,6 +111,12 @@ #define NVQUIRK_DIS_CARD_CLK_CONFIG_TAP BIT(8) #define NVQUIRK_CQHCI_DCMD_R1B_CMD_TIMING BIT(9) +/*
- NVQUIRK_HAS_TMCLK is for SoC's having separate timeout clock for
Tegra
- SDMMC hardware data timeout.
- */
+#define NVQUIRK_HAS_TMCLK BIT(10)
/* SDMMC CQE Base Address for Tegra Host Ver 4.1 and Higher */ #define SDHCI_TEGRA_CQE_BASE_ADDR 0xF000 @@ -140,6 +147,7 @@ struct sdhci_tegra_autocal_offsets { struct sdhci_tegra { const struct sdhci_tegra_soc_data *soc_data; struct gpio_desc *power_gpio; + struct clk *tmclk; bool ddr_signaling; bool pad_calib_required; bool pad_control_available; @@ -1433,7 +1441,8 @@ static const struct sdhci_tegra_soc_data soc_data_tegra210 = { NVQUIRK_HAS_PADCALIB | NVQUIRK_DIS_CARD_CLK_CONFIG_TAP | NVQUIRK_ENABLE_SDR50 | - NVQUIRK_ENABLE_SDR104, + NVQUIRK_ENABLE_SDR104 | + NVQUIRK_HAS_TMCLK, .min_tap_delay = 106, .max_tap_delay = 185, }; @@ -1471,6 +1480,7 @@ static const struct sdhci_tegra_soc_data soc_data_tegra186 = { NVQUIRK_DIS_CARD_CLK_CONFIG_TAP | NVQUIRK_ENABLE_SDR50 | NVQUIRK_ENABLE_SDR104 | + NVQUIRK_HAS_TMCLK | NVQUIRK_CQHCI_DCMD_R1B_CMD_TIMING, .min_tap_delay = 84, .max_tap_delay = 136, @@ -1483,7 +1493,8 @@ static const struct sdhci_tegra_soc_data soc_data_tegra194 = { NVQUIRK_HAS_PADCALIB | NVQUIRK_DIS_CARD_CLK_CONFIG_TAP | NVQUIRK_ENABLE_SDR50 | - NVQUIRK_ENABLE_SDR104, + NVQUIRK_ENABLE_SDR104 | + NVQUIRK_HAS_TMCLK, .min_tap_delay = 96, .max_tap_delay = 139, }; @@ -1611,15 +1622,76 @@ static int sdhci_tegra_probe(struct platform_device *pdev) goto err_power_req; } - clk = devm_clk_get(mmc_dev(host->mmc), NULL); - if (IS_ERR(clk)) { - rc = PTR_ERR(clk); + /* + * Tegra210 and later has separate SDMMC_LEGACY_TM clock used for + * hardware data timeout clock and SW can choose TMCLK or SDCLK for + * hardware data timeout through the bit USE_TMCLK_FOR_DATA_TIMEOUT + * of the register SDHCI_TEGRA_VENDOR_SYS_SW_CTRL. + * + * USE_TMCLK_FOR_DATA_TIMEOUT bit default is set to 1 and SDMMC uses + * 12Mhz TMCLK which is advertised in host capability register. + * With TMCLK of 12Mhz provides maximum data timeout period that can + * be achieved is 11s better than using SDCLK for data timeout. + * + * So, TMCLK is set to 12Mhz and kept enabled all the time on SoC's + * supporting separate TMCLK. + * + * Old device tree has single sdhci clock. So with addition of TMCLK, + * retrieving sdhci clock by "sdhci" clock name based on number of + * clocks in sdhci device node. + */
+ if (of_clk_get_parent_count(pdev->dev.of_node) == 1) { + if (soc_data->nvquirks & NVQUIRK_HAS_TMCLK) + dev_warn(&pdev->dev, + "missing tmclk in the device tree\n");
+ clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(clk)) { + rc = PTR_ERR(clk); - if (rc != -EPROBE_DEFER) - dev_err(&pdev->dev, "failed to get clock: %d\n", rc); + if (rc != -EPROBE_DEFER) + dev_err(&pdev->dev, + "failed to get sdhci clock: %d\n", rc); - goto err_clk_get; + goto err_power_req; + } + } else { + if (soc_data->nvquirks & NVQUIRK_HAS_TMCLK) {
I think that I would do the inverse of this ...
} else { if (!(soc_data->nvquirks & NVQUIRK_HAS_TMCLK)) { dev_err(&pdev->dev, "Device has unexpected clocks!\n"); rc = -EINVAL; goto_power_req; }
clk = devm_clk_get(&pdev->dev, "tmclk"); ...
If the device does not have a single clock, then we expect it to support the tmclk. If this is not the case, then this is a bug.
Cheers Jon
I don't see other drivers validating for unexpected device tree entries.
Also only for SoC with quirk HAS_TMCLK, we are retrieving TMCLK with clock name and enabling it.
So for other SoC even if device tree has additional clock entry other than sdhci driver don't use it and also dt-binding do not have any tmclk entry for other SoC. So why would this be a bug?
In the device tree binding doc, we say has two clocks for Tegra210, Tegra186 and Tegra194 and one clock for all other devices. So if we no there is more than 1 but the device does not have this quirk, then the device-tree does not reflect what is stated in the binding doc or the quirk is no populated as it should be. I feel that either case is a bug.
Now of course it could be possible for someone to add a 3rd clock for Tegra210 and we would not detect this but like you said we don't check all conditions. So yes we don't catch all cases, but the ones that matter.
Jon
Based on internal discussion with Thierry we don't need to handle clocks
order in driver. So will revert clock retrieval to same as in v4 and will send v7 series.
Yes OK fine. Maybe I am being too overly cautious as usual!
Jon
linux-stable-mirror@lists.linaro.org
-
Jon Hunter -
Sowjanya Komatineni