diff mbox series

[v6,7/7] sdhci: tegra: Add missing TMCLK for data timeout

Message ID 1598500201-5987-8-git-send-email-skomatineni@nvidia.com (mailing list archive)
State New, archived
Headers show
Series Fix timeout clock used by hardware data timeout | expand

Commit Message

Sowjanya Komatineni Aug. 27, 2020, 3:50 a.m. UTC
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(-)

Comments

Jon Hunter Aug. 27, 2020, 8:40 a.m. UTC | #1
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
Sowjanya Komatineni Aug. 27, 2020, 3:03 p.m. UTC | #2
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?

Can you please correct if I misunderstood you comment?

Thanks

Sowjanya
Jon Hunter Aug. 27, 2020, 3:14 p.m. UTC | #3
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
Sowjanya Komatineni Aug. 27, 2020, 3:43 p.m. UTC | #4
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
Jon Hunter Aug. 27, 2020, 3:51 p.m. UTC | #5
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
diff mbox series

Patch

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);