[PATCH 4.19.113 0/3] Fix for long operation cmds busy detection

[PATCH 4.19.113 0/3] Fix for long operation cmds busy detection

[Sowjanya Komatineni]

This series includes manually backported changes that implements Tegra
specific timeout callback to switch between finite and infinite HW busy
detection wait modes.

sdhci-tegra driver patch implements set_timeout callback based on one of
the sdhci host driver patch that refactors sdhci_set_timeout and allows
drivers to call __sdhci_set_timeout with their timeout callback
implementation.

Both of these patches are manually backported in this series.


Sowjanya Komatineni (3):
  mmc: sdhci: Refactor sdhci_set_timeout()
  sdhci: tegra: Implement Tegra specific set_timeout callback
  sdhci: tegra: Enable MMC_CAP_WAIT_WHILE_BUSY host capability

 drivers/mmc/host/sdhci-tegra.c | 32 ++++++++++++++++++++++++++++++++
 drivers/mmc/host/sdhci.c       | 38 ++++++++++++++++++++------------------
 drivers/mmc/host/sdhci.h       |  1 +
 3 files changed, 53 insertions(+), 18 deletions(-)

-- 
2.7.4

[PATCH 4.19.113 1/3] mmc: sdhci: Refactor sdhci_set_timeout()

[Sowjanya Komatineni]

commit 7d76ed77cfbd ("mmc: sdhci: Refactor sdhci_set_timeout()")

Refactor sdhci_set_timeout() such that platform drivers can do some
functionality in a set_timeout() callback and then call
__sdhci_set_timeout() to complete the operation.

Cc: <stable@vger.kernel.org>
Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/mmc/host/sdhci.c | 38 ++++++++++++++++++++------------------
 drivers/mmc/host/sdhci.h |  1 +
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 5a7fd89..c941e81 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -860,27 +860,29 @@ static void sdhci_set_data_timeout_irq(struct sdhci_host *host, bool enable)
 	sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
 }
 
-static void sdhci_set_timeout(struct sdhci_host *host, struct mmc_command *cmd)
+void __sdhci_set_timeout(struct sdhci_host *host, struct mmc_command *cmd)
 {
-	u8 count;
-
-	if (host->ops->set_timeout) {
-		host->ops->set_timeout(host, cmd);
-	} else {
-		bool too_big = false;
-
-		count = sdhci_calc_timeout(host, cmd, &too_big);
+	bool too_big = false;
+	u8 count = sdhci_calc_timeout(host, cmd, &too_big);
+
+	if (too_big &&
+	    host->quirks2 & SDHCI_QUIRK2_DISABLE_HW_TIMEOUT) {
+		sdhci_calc_sw_timeout(host, cmd);
+		sdhci_set_data_timeout_irq(host, false);
+	} else if (!(host->ier & SDHCI_INT_DATA_TIMEOUT)) {
+		sdhci_set_data_timeout_irq(host, true);
+	}
 
-		if (too_big &&
-		    host->quirks2 & SDHCI_QUIRK2_DISABLE_HW_TIMEOUT) {
-			sdhci_calc_sw_timeout(host, cmd);
-			sdhci_set_data_timeout_irq(host, false);
-		} else if (!(host->ier & SDHCI_INT_DATA_TIMEOUT)) {
-			sdhci_set_data_timeout_irq(host, true);
-		}
+	sdhci_writeb(host, count, SDHCI_TIMEOUT_CONTROL);
+}
+EXPORT_SYMBOL_GPL(__sdhci_set_timeout);
 
-		sdhci_writeb(host, count, SDHCI_TIMEOUT_CONTROL);
-	}
+static void sdhci_set_timeout(struct sdhci_host *host, struct mmc_command *cmd)
+{
+	if (host->ops->set_timeout)
+		host->ops->set_timeout(host, cmd);
+	else
+		__sdhci_set_timeout(host, cmd);
 }
 
 static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index c0372e3..15ef9c6 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -759,5 +759,6 @@ void sdhci_start_tuning(struct sdhci_host *host);
 void sdhci_end_tuning(struct sdhci_host *host);
 void sdhci_reset_tuning(struct sdhci_host *host);
 void sdhci_send_tuning(struct sdhci_host *host, u32 opcode);
+void __sdhci_set_timeout(struct sdhci_host *host, struct mmc_command *cmd);
 
 #endif /* __SDHCI_HW_H */
-- 
2.7.4

[PATCH 4.19.113 2/3] sdhci: tegra: Implement Tegra specific set_timeout callback

[Sowjanya Komatineni]

commit 5e958e4aacf4
("sdhci: tegra: Implement Tegra specific set_timeout callback")

Tegra host supports HW busy detection and timeouts based on the
count programmed in SDHCI_TIMEOUT_CONTROL register and max busy
timeout it supports is 11s in finite busy wait mode.

Some operations like SLEEP_AWAKE, ERASE and flush cache through
SWITCH commands take longer than 11s and Tegra host supports
infinite HW busy wait mode where HW waits forever till the card
is busy without HW timeout.

This patch implements Tegra specific set_timeout sdhci_ops to allow
switching between finite and infinite HW busy detection wait modes
based on the device command expected operation time.

Cc: <stable@vger.kernel.org>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 27bdf6d..0f4de73 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -39,6 +39,7 @@
 #define SDHCI_CLOCK_CTRL_SPI_MODE_CLKEN_OVERRIDE	BIT(2)
 
 #define SDHCI_TEGRA_VENDOR_MISC_CTRL		0x120
+#define SDHCI_MISC_CTRL_ERASE_TIMEOUT_LIMIT	BIT(0)
 #define SDHCI_MISC_CTRL_ENABLE_SDR104		0x8
 #define SDHCI_MISC_CTRL_ENABLE_SDR50		0x10
 #define SDHCI_MISC_CTRL_ENABLE_SDHCI_SPEC_300	0x20
@@ -301,6 +302,34 @@ static int tegra_sdhci_execute_tuning(struct sdhci_host *host, u32 opcode)
 	return mmc_send_tuning(host->mmc, opcode, NULL);
 }
 
+static void tegra_sdhci_set_timeout(struct sdhci_host *host,
+				    struct mmc_command *cmd)
+{
+	u32 val;
+
+	/*
+	 * HW busy detection timeout is based on programmed data timeout
+	 * counter and maximum supported timeout is 11s which may not be
+	 * enough for long operations like cache flush, sleep awake, erase.
+	 *
+	 * ERASE_TIMEOUT_LIMIT bit of VENDOR_MISC_CTRL register allows
+	 * host controller to wait for busy state until the card is busy
+	 * without HW timeout.
+	 *
+	 * So, use infinite busy wait mode for operations that may take
+	 * more than maximum HW busy timeout of 11s otherwise use finite
+	 * busy wait mode.
+	 */
+	val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_MISC_CTRL);
+	if (cmd && cmd->busy_timeout >= 11 * HZ)
+		val |= SDHCI_MISC_CTRL_ERASE_TIMEOUT_LIMIT;
+	else
+		val &= ~SDHCI_MISC_CTRL_ERASE_TIMEOUT_LIMIT;
+	sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_MISC_CTRL);
+
+	__sdhci_set_timeout(host, cmd);
+}
+
 static void tegra_sdhci_voltage_switch(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -379,6 +408,7 @@ static const struct sdhci_ops tegra114_sdhci_ops = {
 	.set_uhs_signaling = tegra_sdhci_set_uhs_signaling,
 	.voltage_switch = tegra_sdhci_voltage_switch,
 	.get_max_clock = tegra_sdhci_get_max_clock,
+	.set_timeout = tegra_sdhci_set_timeout,
 };
 
 static const struct sdhci_pltfm_data sdhci_tegra114_pdata = {
-- 
2.7.4

[PATCH 4.19.113 3/3] sdhci: tegra: Enable MMC_CAP_WAIT_WHILE_BUSY host capability

[Sowjanya Komatineni]

commit ff124c31ccd7
("sdhci: tegra: Enable MMC_CAP_WAIT_WHILE_BUSY host capability")

Tegra sdhci host supports HW busy detection of the device busy
signaling over data0 lane.

So, this patch enables host capability MMC_CAP_WAIT_WHILE_BUSY.

Cc: <stable@vger.kernel.org>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 0f4de73..fde1f3b 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -529,6 +529,8 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
 	if (rc)
 		goto err_parse_dt;
 
+	host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;
+
 	if (tegra_host->soc_data->nvquirks & NVQUIRK_ENABLE_DDR50)
 		host->mmc->caps |= MMC_CAP_1_8V_DDR;
 
-- 
2.7.4

Re: [PATCH 4.19.113 0/3] Fix for long operation cmds busy detection

[Greg KH]

On Fri, Apr 17, 2020 at 12:14:01PM -0700, Sowjanya Komatineni wrote:
> This series includes manually backported changes that implements Tegra
> specific timeout callback to switch between finite and infinite HW busy
> detection wait modes.
> 
> sdhci-tegra driver patch implements set_timeout callback based on one of
> the sdhci host driver patch that refactors sdhci_set_timeout and allows
> drivers to call __sdhci_set_timeout with their timeout callback
> implementation.
> 
> Both of these patches are manually backported in this series.

Is this a bugfix or a new feature?  I can't tell, but it feels like it's
a new feature.  What's wrong with just using the 5.4.y kernel tree?

thanks,

greg k-h

Re: [PATCH 4.19.113 0/3] Fix for long operation cmds busy detection

[Sowjanya Komatineni]

On 4/19/20 12:20 AM, Greg KH wrote:
> External email: Use caution opening links or attachments
>
>
> On Fri, Apr 17, 2020 at 12:14:01PM -0700, Sowjanya Komatineni wrote:
>> This series includes manually backported changes that implements Tegra
>> specific timeout callback to switch between finite and infinite HW busy
>> detection wait modes.
>>
>> sdhci-tegra driver patch implements set_timeout callback based on one of
>> the sdhci host driver patch that refactors sdhci_set_timeout and allows
>> drivers to call __sdhci_set_timeout with their timeout callback
>> implementation.
>>
>> Both of these patches are manually backported in this series.
> Is this a bugfix or a new feature?  I can't tell, but it feels like it's
> a new feature.  What's wrong with just using the 5.4.y kernel tree?
>
> thanks,
>
> greg k-h
Ulf recent patches for increased timeout adds capability 
MMC_CAP_NEED_RSP_BUSY for sdhci-tegra as well.

So, it will always use R1B for R1B type commands so there are no known 
bugs or failures with mmc devices we use on our platforms.

So, we can treat this patch as an improvement for long operation 
commands where HW will wait as long as device is busy.

Yes, should be fine to have this from 5.4.y

For 5.4.y and 5.5.y, we can back-port same patch that was applied to 5.6.y

Will send back-port request of that..

Thanks

Sowjanya

Re: [PATCH 4.19.113 0/3] Fix for long operation cmds busy detection

[Greg KH]

On Sun, Apr 19, 2020 at 09:23:39AM -0700, Sowjanya Komatineni wrote:
> 
> On 4/19/20 12:20 AM, Greg KH wrote:
> > External email: Use caution opening links or attachments
> > 
> > 
> > On Fri, Apr 17, 2020 at 12:14:01PM -0700, Sowjanya Komatineni wrote:
> > > This series includes manually backported changes that implements Tegra
> > > specific timeout callback to switch between finite and infinite HW busy
> > > detection wait modes.
> > > 
> > > sdhci-tegra driver patch implements set_timeout callback based on one of
> > > the sdhci host driver patch that refactors sdhci_set_timeout and allows
> > > drivers to call __sdhci_set_timeout with their timeout callback
> > > implementation.
> > > 
> > > Both of these patches are manually backported in this series.
> > Is this a bugfix or a new feature?  I can't tell, but it feels like it's
> > a new feature.  What's wrong with just using the 5.4.y kernel tree?
> > 
> > thanks,
> > 
> > greg k-h
> Ulf recent patches for increased timeout adds capability
> MMC_CAP_NEED_RSP_BUSY for sdhci-tegra as well.
> 
> So, it will always use R1B for R1B type commands so there are no known bugs
> or failures with mmc devices we use on our platforms.

I have no idea what this means, sorry.

> So, we can treat this patch as an improvement for long operation commands
> where HW will wait as long as device is busy.

Ok, so this isn't a regression and can't match the stable kernel rules,
sorry.

greg k-h

Re: [PATCH 4.19.113 0/3] Fix for long operation cmds busy detection

[Ulf Hansson]

On Sun, 19 Apr 2020 at 18:52, Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Sun, Apr 19, 2020 at 09:23:39AM -0700, Sowjanya Komatineni wrote:
> >
> > On 4/19/20 12:20 AM, Greg KH wrote:
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > On Fri, Apr 17, 2020 at 12:14:01PM -0700, Sowjanya Komatineni wrote:
> > > > This series includes manually backported changes that implements Tegra
> > > > specific timeout callback to switch between finite and infinite HW busy
> > > > detection wait modes.
> > > >
> > > > sdhci-tegra driver patch implements set_timeout callback based on one of
> > > > the sdhci host driver patch that refactors sdhci_set_timeout and allows
> > > > drivers to call __sdhci_set_timeout with their timeout callback
> > > > implementation.
> > > >
> > > > Both of these patches are manually backported in this series.
> > > Is this a bugfix or a new feature?  I can't tell, but it feels like it's
> > > a new feature.  What's wrong with just using the 5.4.y kernel tree?
> > >
> > > thanks,
> > >
> > > greg k-h
> > Ulf recent patches for increased timeout adds capability
> > MMC_CAP_NEED_RSP_BUSY for sdhci-tegra as well.
> >
> > So, it will always use R1B for R1B type commands so there are no known bugs
> > or failures with mmc devices we use on our platforms.
>
> I have no idea what this means, sorry.
>
> > So, we can treat this patch as an improvement for long operation commands
> > where HW will wait as long as device is busy.
>
> Ok, so this isn't a regression and can't match the stable kernel rules,
> sorry.

Let me help to clarify. This isn't a regression, correct. However, the
patch fixes a long outstanding bug for sdhci-tegra.

For some SD/MMC commands, the mmc core may provide a long busy timeout
trusting the mmc host to cope with it. It has turned out that
sdhci-tegra didn't, thus it may report a cmd-timeout error, while in
fact it shouldn't.

I believe that is what the small series of patches should be addressing.

Kind regards
Uffe

Re: [PATCH 4.19.113 0/3] Fix for long operation cmds busy detection

[Sowjanya Komatineni]

On 4/20/20 12:01 AM, Ulf Hansson wrote:
> External email: Use caution opening links or attachments
>
>
> On Sun, 19 Apr 2020 at 18:52, Greg KH <gregkh@linuxfoundation.org> wrote:
>> On Sun, Apr 19, 2020 at 09:23:39AM -0700, Sowjanya Komatineni wrote:
>>> On 4/19/20 12:20 AM, Greg KH wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> On Fri, Apr 17, 2020 at 12:14:01PM -0700, Sowjanya Komatineni wrote:
>>>>> This series includes manually backported changes that implements Tegra
>>>>> specific timeout callback to switch between finite and infinite HW busy
>>>>> detection wait modes.
>>>>>
>>>>> sdhci-tegra driver patch implements set_timeout callback based on one of
>>>>> the sdhci host driver patch that refactors sdhci_set_timeout and allows
>>>>> drivers to call __sdhci_set_timeout with their timeout callback
>>>>> implementation.
>>>>>
>>>>> Both of these patches are manually backported in this series.
>>>> Is this a bugfix or a new feature?  I can't tell, but it feels like it's
>>>> a new feature.  What's wrong with just using the 5.4.y kernel tree?
>>>>
>>>> thanks,
>>>>
>>>> greg k-h
>>> Ulf recent patches for increased timeout adds capability
>>> MMC_CAP_NEED_RSP_BUSY for sdhci-tegra as well.
>>>
>>> So, it will always use R1B for R1B type commands so there are no known bugs
>>> or failures with mmc devices we use on our platforms.
>> I have no idea what this means, sorry.
>>
>>> So, we can treat this patch as an improvement for long operation commands
>>> where HW will wait as long as device is busy.
>> Ok, so this isn't a regression and can't match the stable kernel rules,
>> sorry.
> Let me help to clarify. This isn't a regression, correct. However, the
> patch fixes a long outstanding bug for sdhci-tegra.
>
> For some SD/MMC commands, the mmc core may provide a long busy timeout
> trusting the mmc host to cope with it. It has turned out that
> sdhci-tegra didn't, thus it may report a cmd-timeout error, while in
> fact it shouldn't.
>
> I believe that is what the small series of patches should be addressing.
>
> Kind regards
> Uffe

Hi Greg,

Yes, we we don't have failure as of today with mmc devices we are using 
on our platforms.

But this is something thats missing from the beginning to handle 
commands that takes longer than max host timeout and it may fail if any 
operation takes more than max host timeout limit.

So, can we please have this applied to 4.19.113? I will also send 
patches to backport for 5.4 and 5.5?

Thanks

Sowjanya

[PATCH 4.19.113 2/3] sdhci: tegra: Implement Tegra specific set_timeout callback

[Sowjanya Komatineni]

commit 5e958e4aacf4
("sdhci: tegra: Implement Tegra specific set_timeout callback")

Tegra host supports HW busy detection and timeouts based on the
count programmed in SDHCI_TIMEOUT_CONTROL register and max busy
timeout it supports is 11s in finite busy wait mode.

Some operations like SLEEP_AWAKE, ERASE and flush cache through
SWITCH commands take longer than 11s and Tegra host supports
infinite HW busy wait mode where HW waits forever till the card
is busy without HW timeout.

This patch implements Tegra specific set_timeout sdhci_ops to allow
switching between finite and infinite HW busy detection wait modes
based on the device command expected operation time.

Cc: <stable@vger.kernel.org>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 27bdf6d..0f4de73 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -39,6 +39,7 @@
 #define SDHCI_CLOCK_CTRL_SPI_MODE_CLKEN_OVERRIDE	BIT(2)
 
 #define SDHCI_TEGRA_VENDOR_MISC_CTRL		0x120
+#define SDHCI_MISC_CTRL_ERASE_TIMEOUT_LIMIT	BIT(0)
 #define SDHCI_MISC_CTRL_ENABLE_SDR104		0x8
 #define SDHCI_MISC_CTRL_ENABLE_SDR50		0x10
 #define SDHCI_MISC_CTRL_ENABLE_SDHCI_SPEC_300	0x20
@@ -301,6 +302,34 @@ static int tegra_sdhci_execute_tuning(struct sdhci_host *host, u32 opcode)
 	return mmc_send_tuning(host->mmc, opcode, NULL);
 }
 
+static void tegra_sdhci_set_timeout(struct sdhci_host *host,
+				    struct mmc_command *cmd)
+{
+	u32 val;
+
+	/*
+	 * HW busy detection timeout is based on programmed data timeout
+	 * counter and maximum supported timeout is 11s which may not be
+	 * enough for long operations like cache flush, sleep awake, erase.
+	 *
+	 * ERASE_TIMEOUT_LIMIT bit of VENDOR_MISC_CTRL register allows
+	 * host controller to wait for busy state until the card is busy
+	 * without HW timeout.
+	 *
+	 * So, use infinite busy wait mode for operations that may take
+	 * more than maximum HW busy timeout of 11s otherwise use finite
+	 * busy wait mode.
+	 */
+	val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_MISC_CTRL);
+	if (cmd && cmd->busy_timeout >= 11 * HZ)
+		val |= SDHCI_MISC_CTRL_ERASE_TIMEOUT_LIMIT;
+	else
+		val &= ~SDHCI_MISC_CTRL_ERASE_TIMEOUT_LIMIT;
+	sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_MISC_CTRL);
+
+	__sdhci_set_timeout(host, cmd);
+}
+
 static void tegra_sdhci_voltage_switch(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -379,6 +408,7 @@ static const struct sdhci_ops tegra114_sdhci_ops = {
 	.set_uhs_signaling = tegra_sdhci_set_uhs_signaling,
 	.voltage_switch = tegra_sdhci_voltage_switch,
 	.get_max_clock = tegra_sdhci_get_max_clock,
+	.set_timeout = tegra_sdhci_set_timeout,
 };
 
 static const struct sdhci_pltfm_data sdhci_tegra114_pdata = {
-- 
2.7.4