[PATCH RFC] HACK: mmc: core: also abort tuning with CMD12 for SD

[PATCH RFC] HACK: mmc: core: also abort tuning with CMD12 for SD

[Wolfram Sang]

We have various SanDisk cards which fail tuning to SDR104 unless we
allow a CMD12 also to be sent to abort a broken tuning. It is true that
the SD specs do not mention that CMD12 is allowed, but they also don't
say it is forbidden. And now reality tells that it is needed to make
some cards work. Other cards I tried did not regress.

If we can agree to allow this for SD, then the problem is now SDIO which
does not support CMD12. mmc_card_sdio() does not work at this stage
because host->card is still NULL. Is there any other way to distinguish
SD and SDIO here?

Not-Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Hey guys,

so, there are two questions here:
1) despite not being mentioned in the spec, do we want to allow CMD12 to
   abort tuning for SD as well?

2) If so, how to make sure not apply it to SDIO but SD only?

Thanks for your input! Kind regards,

   Wolfram


 drivers/mmc/core/mmc_ops.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 973756ed4016..02d378255895 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -704,14 +704,6 @@ int mmc_send_abort_tuning(struct mmc_host *host, u32 opcode)
 {
 	struct mmc_command cmd = {};
 
-	/*
-	 * eMMC specification specifies that CMD12 can be used to stop a tuning
-	 * command, but SD specification does not, so do nothing unless it is
-	 * eMMC.
-	 */
-	if (opcode != MMC_SEND_TUNING_BLOCK_HS200)
-		return 0;
-
 	cmd.opcode = MMC_STOP_TRANSMISSION;
 	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC;
 
-- 
2.30.2

Re: [PATCH RFC] HACK: mmc: core: also abort tuning with CMD12 for SD

[Ulf Hansson]

On Tue, 31 Aug 2021 at 15:34, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> We have various SanDisk cards which fail tuning to SDR104 unless we
> allow a CMD12 also to be sent to abort a broken tuning. It is true that
> the SD specs do not mention that CMD12 is allowed, but they also don't
> say it is forbidden. And now reality tells that it is needed to make
> some cards work. Other cards I tried did not regress.
>
> If we can agree to allow this for SD, then the problem is now SDIO which
> does not support CMD12. mmc_card_sdio() does not work at this stage
> because host->card is still NULL. Is there any other way to distinguish
> SD and SDIO here?
>
> Not-Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>
> Hey guys,
>
> so, there are two questions here:
> 1) despite not being mentioned in the spec, do we want to allow CMD12 to
>    abort tuning for SD as well?

It sounds like we should give it a try with the CMD12 command for SD
cards as well.

>
> 2) If so, how to make sure not apply it to SDIO but SD only?

For now, I am fine with adding a new bus_ops callback
(->abort_tuning()) and then let mmc_send_abort_tuning() to call it.

I have some additional plans to improve life cycle issues for the
bus_ops, but let's ignore that for now. I can deal with that later.

That said, mmc_send_abort_tuning() should no longer need to take the
opcode as an in-parameter, thus some additional cleanup should be
needed in a few host drivers because of that.

Would that work?

>
> Thanks for your input! Kind regards,
>
>    Wolfram

Kind regards
Uffe

>
>
>  drivers/mmc/core/mmc_ops.c | 8 --------
>  1 file changed, 8 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 973756ed4016..02d378255895 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -704,14 +704,6 @@ int mmc_send_abort_tuning(struct mmc_host *host, u32 opcode)
>  {
>         struct mmc_command cmd = {};
>
> -       /*
> -        * eMMC specification specifies that CMD12 can be used to stop a tuning
> -        * command, but SD specification does not, so do nothing unless it is
> -        * eMMC.
> -        */
> -       if (opcode != MMC_SEND_TUNING_BLOCK_HS200)
> -               return 0;
> -
>         cmd.opcode = MMC_STOP_TRANSMISSION;
>         cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC;
>
> --
> 2.30.2
>

Re: [PATCH RFC] HACK: mmc: core: also abort tuning with CMD12 for SD

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1011 bytes --]

Hi Ulf,

> > 1) despite not being mentioned in the spec, do we want to allow CMD12 to
> >    abort tuning for SD as well?
> 
> It sounds like we should give it a try with the CMD12 command for SD
> cards as well.

I think so.

> > 2) If so, how to make sure not apply it to SDIO but SD only?
> 
> For now, I am fine with adding a new bus_ops callback
> (->abort_tuning()) and then let mmc_send_abort_tuning() to call it.

Cool, I like that approach.

> I have some additional plans to improve life cycle issues for the
> bus_ops, but let's ignore that for now. I can deal with that later.

Ok, good.

> That said, mmc_send_abort_tuning() should no longer need to take the
> opcode as an in-parameter, thus some additional cleanup should be
> needed in a few host drivers because of that.
> 
> Would that work?

I think that would work nicely. I will have a go with the above approach
and come back then. Or do you want to implement it?

Thanks and happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH RFC] HACK: mmc: core: also abort tuning with CMD12 for SD

[Ulf Hansson]

On Thu, 9 Sept 2021 at 15:03, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> Hi Ulf,
>
> > > 1) despite not being mentioned in the spec, do we want to allow CMD12 to
> > >    abort tuning for SD as well?
> >
> > It sounds like we should give it a try with the CMD12 command for SD
> > cards as well.
>
> I think so.
>
> > > 2) If so, how to make sure not apply it to SDIO but SD only?
> >
> > For now, I am fine with adding a new bus_ops callback
> > (->abort_tuning()) and then let mmc_send_abort_tuning() to call it.
>
> Cool, I like that approach.
>
> > I have some additional plans to improve life cycle issues for the
> > bus_ops, but let's ignore that for now. I can deal with that later.
>
> Ok, good.
>
> > That said, mmc_send_abort_tuning() should no longer need to take the
> > opcode as an in-parameter, thus some additional cleanup should be
> > needed in a few host drivers because of that.
> >
> > Would that work?
>
> I think that would work nicely. I will have a go with the above approach
> and come back then. Or do you want to implement it?

Please go ahead, I can review it.

>
> Thanks and happy hacking,
>
>    Wolfram
>

Kind regards
Uffe