Re: [PATCH v5 7/7] sdhci: tegra: Add missing TMCLK for data timeout
Hi Sowjanya,
I love your patch! Yet something to improve:
[auto build test ERROR on robh/for-next]
[also build test ERROR on tegra/for-next v5.9-rc2 next-20200826]
[cannot apply to ulf.hansson-mmc/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Sowjanya-Komatineni/Fix-timeout-clock-used-by-hardware-data-timeout/20200827-040814
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
>> drivers/mmc/host/sdhci-tegra.c:1496:24: error: expected '}' before ';' token
1496 | NVQUIRK_HAS_TMCLK;
|^
drivers/mmc/host/sdhci-tegra.c:1488:62: note: to match this '{'
1488 | static const struct sdhci_tegra_soc_data soc_data_tegra194 = {
| ^
drivers/mmc/host/sdhci-tegra.c: In function 'sdhci_tegra_probe':
>> drivers/mmc/host/sdhci-tegra.c:1643:6: error: implicit declaration of
>> function 'of_clk_get_parent_count' [-Werror=implicit-function-declaration]
1643 | if (of_clk_get_parent_count(>dev) == 1) {
| ^~~
>> drivers/mmc/host/sdhci-tegra.c:1648:22: error: 'dev' undeclared (first use
>> in this function); did you mean 'pdev'?
1648 | clk = devm_clk_get(dev, NULL)
| ^~~
| pdev
drivers/mmc/host/sdhci-tegra.c:1648:22: note: each undeclared identifier is
reported only once for each function it appears in
>> drivers/mmc/host/sdhci-tegra.c:1648:32: error: expected ';' before 'if'
1648 | clk = devm_clk_get(dev, NULL)
|^
|;
1649 | if (IS_ERR(clk)) {
| ~~
drivers/mmc/host/sdhci-tegra.c:1682:35: error: expected ';' before 'if'
1682 | clk = devm_clk_get(dev, "sdhci")
| ^
| ;
1683 | if (IS_ERR(clk)) {
| ~~
drivers/mmc/host/sdhci-tegra.c:1727:1: warning: label 'err_clk_get' defined
but not used [-Wunused-label]
1727 | err_clk_get:
| ^~~
cc1: some warnings being treated as errors
#
https://github.com/0day-ci/linux/commit/51ed0e529a10cbce9dba08a11817207acb1b5bcf
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Sowjanya-Komatineni/Fix-timeout-clock-used-by-hardware-data-timeout/20200827-040814
git checkout 51ed0e529a10cbce9dba08a11817207acb1b5bcf
vim +1496 drivers/mmc/host/sdhci-tegra.c
1487
1488 static const struct sdhci_tegra_soc_data soc_data_tegra194 = {
1489 .pdata = _tegra186_pdata,
1490 .dma_mask = DMA_BIT_MASK(39),
1491 .nvquirks = NVQUIRK_NEEDS_PAD_CONTROL |
1492 NVQUIRK_HAS_PADCALIB |
1493 NVQUIRK_DIS_CARD_CLK_CONFIG_TAP |
1494 NVQUIRK_ENABLE_SDR50 |
1495 NVQUIRK_ENABLE_SDR104 |
> 1496 NVQUIRK_HAS_TMCLK;
1497 .min_tap_delay = 96,
1498 .max_tap_delay = 139,
1499 };
1500
1501 static const struct of_device_id sdhci_tegra_dt_match[] = {
1502 { .compatible = "nvidia,tegra194-sdhci", .data =
_data_tegra194 },
1503 { .compatible = "nvidia,tegra186-sdhci", .data =
_data_tegra186 },
1504 { .compatible = "nvidia,tegra210-sdhci", .data =
_data_tegra210 },
1505 { .compatible = "nvidia,tegra124-sdhci", .data =
_data_tegra124 },
1506 { .compatible = "nvidia,tegra114-sdhci", .data =
_data_tegra114 },
1507 { .compatible = "nvidia,tegra30-sdhci", .data =
_data_tegra30 },
1508 { .compatible = "nvidia,tegra20-sdhci", .data =
_data_tegra20 },
1509 {}
1510 };
1511 MODULE_DEVICE_TABLE(of, sdhci_tegra_dt_match);
1512
1513 static int sdhci_tegra_add_host(struct sdhci_host *host)
1514 {
1515 struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
1516 struct sdhci_tegra *tegra_host = sdhci_pltfm_priv(pltfm_host);
1517 struct cqhci_host *cq_host;
1518 bool dma64;
1519 int ret;
1520
1521
[PATCH v5 7/7] sdhci: tegra: Add missing TMCLK for data timeout
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 # 5.4
Tested-by: Jon Hunter
Reviewed-by: Jon Hunter
Acked-by: Adrian Hunter
Signed-off-by: Sowjanya Komatineni
---
drivers/mmc/host/sdhci-tegra.c | 89 ++
1 file changed, 81 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 31ed321..9bcd532 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -110,6 +110,12 @@
#define NVQUIRK_DIS_CARD_CLK_CONFIG_TAPBIT(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 +146,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 +1440,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 +1479,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 +1492,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 +1621,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(>dev) == 1) {
+ if (soc_data->nvquirks & NVQUIRK_HAS_TMCLK)
+ dev_warn(>dev,
+"missing tmclk in the device tree\n");
+
+ clk = devm_clk_get(dev, NULL)
+ if (IS_ERR(clk)) {
+ rc = PTR_ERR(clk);
- if (rc != -EPROBE_DEFER)
- dev_err(>dev, "failed to get clock: %d\n", rc);
+ if (rc != -EPROBE_DEFER)
+ dev_err(>dev,
+ "failed to get sdhci clock: %d\n", rc);
- goto
