| Message ID | 20210831133349.18203-1-wsa+renesas@sang-engineering.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] HACK: mmc: core: also abort tuning with CMD12 for SD | expand |
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 >
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
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
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;