diff mbox series

[v3,05/10] mmc: host: Always use manual-cmd23 in SDUC

Message ID 20240814072934.2559911-6-avri.altman@wdc.com (mailing list archive)
State New
Headers show
Series Add SDUC Support | expand

Commit Message

Avri Altman Aug. 14, 2024, 7:29 a.m. UTC
In Multi-Block read/write, CMD23 must precede CMD22.  Therefore always
use manual cmd23 so that we'll be able to control the sequence of
commands.  Also, add an applicable mmc_command member for both
mmc_blk_request and mmc_request to accommodate the address extension
command.

Tested-by: Ricky WU <ricky_wu@realtek.com>
Signed-off-by: Avri Altman <avri.altman@wdc.com>
---
 drivers/mmc/core/queue.h | 1 +
 drivers/mmc/host/sdhci.c | 4 ++--
 include/linux/mmc/core.h | 1 +
 3 files changed, 4 insertions(+), 2 deletions(-)

Comments

Ulf Hansson Aug. 22, 2024, 12:33 p.m. UTC | #1
On Wed, 14 Aug 2024 at 09:31, Avri Altman <avri.altman@wdc.com> wrote:
>
> In Multi-Block read/write, CMD23 must precede CMD22.  Therefore always
> use manual cmd23 so that we'll be able to control the sequence of
> commands.  Also, add an applicable mmc_command member for both
> mmc_blk_request and mmc_request to accommodate the address extension
> command.
>
> Tested-by: Ricky WU <ricky_wu@realtek.com>
> Signed-off-by: Avri Altman <avri.altman@wdc.com>
> ---
>  drivers/mmc/core/queue.h | 1 +
>  drivers/mmc/host/sdhci.c | 4 ++--
>  include/linux/mmc/core.h | 1 +
>  3 files changed, 4 insertions(+), 2 deletions(-)

Please split this up. The core changes certainly don't need to be
mixed with changes to sdhci.

>
> diff --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h
> index 1498840a4ea0..7e191d7f0461 100644
> --- a/drivers/mmc/core/queue.h
> +++ b/drivers/mmc/core/queue.h
> @@ -40,6 +40,7 @@ struct mmc_blk_ioc_data;
>  struct mmc_blk_request {
>         struct mmc_request      mrq;
>         struct mmc_command      sbc;
> +       struct mmc_command      ext;
>         struct mmc_command      cmd;
>         struct mmc_command      stop;
>         struct mmc_data         data;
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 4b91c9e96635..f62b489c9e9c 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1399,13 +1399,13 @@ static inline bool sdhci_auto_cmd12(struct sdhci_host *host,
>  static inline bool sdhci_auto_cmd23(struct sdhci_host *host,
>                                     struct mmc_request *mrq)
>  {
> -       return mrq->sbc && (host->flags & SDHCI_AUTO_CMD23);
> +       return mrq->sbc && (host->flags & SDHCI_AUTO_CMD23) && !mrq->ext;
>  }
>
>  static inline bool sdhci_manual_cmd23(struct sdhci_host *host,
>                                       struct mmc_request *mrq)
>  {
> -       return mrq->sbc && !(host->flags & SDHCI_AUTO_CMD23);
> +       return mrq->sbc && (mrq->ext || !(host->flags & SDHCI_AUTO_CMD23));
>  }

I am a bit worried to see that we are going to need updates in host
drivers too, to support CMD22 and SDUC cards. But if that's the case,
so be it.

In any case, at least some more information about why this approach is
needed would be nice.

>
>  static inline void sdhci_auto_cmd_select(struct sdhci_host *host,
> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
> index 2c7928a50907..5560e70cb8d4 100644
> --- a/include/linux/mmc/core.h
> +++ b/include/linux/mmc/core.h
> @@ -142,6 +142,7 @@ struct mmc_data {
>  struct mmc_host;
>  struct mmc_request {
>         struct mmc_command      *sbc;           /* SET_BLOCK_COUNT for multiblock */
> +       struct mmc_command      *ext;           /* SD_ADDR_EXT for SDUC */
>         struct mmc_command      *cmd;
>         struct mmc_data         *data;
>         struct mmc_command      *stop;
> --
> 2.25.1
>

Kind regards
Uffe
Avri Altman Aug. 22, 2024, 1:24 p.m. UTC | #2
> On Wed, 14 Aug 2024 at 09:31, Avri Altman <avri.altman@wdc.com> wrote:
> >
> > In Multi-Block read/write, CMD23 must precede CMD22.  Therefore always
> > use manual cmd23 so that we'll be able to control the sequence of
> > commands.  Also, add an applicable mmc_command member for both
> > mmc_blk_request and mmc_request to accommodate the address extension
> > command.
> >
> > Tested-by: Ricky WU <ricky_wu@realtek.com>
> > Signed-off-by: Avri Altman <avri.altman@wdc.com>
> > ---
> >  drivers/mmc/core/queue.h | 1 +
> >  drivers/mmc/host/sdhci.c | 4 ++--
> >  include/linux/mmc/core.h | 1 +
> >  3 files changed, 4 insertions(+), 2 deletions(-)
> 
> Please split this up. The core changes certainly don't need to be mixed with
> changes to sdhci.
OK.  I think switching 5 & 6 will do.

Thanks,
Avri

> 
> >
> > diff --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h index
> > 1498840a4ea0..7e191d7f0461 100644
> > --- a/drivers/mmc/core/queue.h
> > +++ b/drivers/mmc/core/queue.h
> > @@ -40,6 +40,7 @@ struct mmc_blk_ioc_data;  struct mmc_blk_request {
> >         struct mmc_request      mrq;
> >         struct mmc_command      sbc;
> > +       struct mmc_command      ext;
> >         struct mmc_command      cmd;
> >         struct mmc_command      stop;
> >         struct mmc_data         data;
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index
> > 4b91c9e96635..f62b489c9e9c 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -1399,13 +1399,13 @@ static inline bool sdhci_auto_cmd12(struct
> > sdhci_host *host,  static inline bool sdhci_auto_cmd23(struct sdhci_host
> *host,
> >                                     struct mmc_request *mrq)  {
> > -       return mrq->sbc && (host->flags & SDHCI_AUTO_CMD23);
> > +       return mrq->sbc && (host->flags & SDHCI_AUTO_CMD23) &&
> > + !mrq->ext;
> >  }
> >
> >  static inline bool sdhci_manual_cmd23(struct sdhci_host *host,
> >                                       struct mmc_request *mrq)  {
> > -       return mrq->sbc && !(host->flags & SDHCI_AUTO_CMD23);
> > +       return mrq->sbc && (mrq->ext || !(host->flags &
> > + SDHCI_AUTO_CMD23));
> >  }
> 
> I am a bit worried to see that we are going to need updates in host drivers too,
> to support CMD22 and SDUC cards. But if that's the case, so be it.
> 
> In any case, at least some more information about why this approach is
> needed would be nice.
Done.

Thanks,
Avri

> 
> >
> >  static inline void sdhci_auto_cmd_select(struct sdhci_host *host,
> > diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h index
> > 2c7928a50907..5560e70cb8d4 100644
> > --- a/include/linux/mmc/core.h
> > +++ b/include/linux/mmc/core.h
> > @@ -142,6 +142,7 @@ struct mmc_data {
> >  struct mmc_host;
> >  struct mmc_request {
> >         struct mmc_command      *sbc;           /* SET_BLOCK_COUNT for
> multiblock */
> > +       struct mmc_command      *ext;           /* SD_ADDR_EXT for SDUC */
> >         struct mmc_command      *cmd;
> >         struct mmc_data         *data;
> >         struct mmc_command      *stop;
> > --
> > 2.25.1
> >
> 
> Kind regards
> Uffe
diff mbox series

Patch

diff --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h
index 1498840a4ea0..7e191d7f0461 100644
--- a/drivers/mmc/core/queue.h
+++ b/drivers/mmc/core/queue.h
@@ -40,6 +40,7 @@  struct mmc_blk_ioc_data;
 struct mmc_blk_request {
 	struct mmc_request	mrq;
 	struct mmc_command	sbc;
+	struct mmc_command	ext;
 	struct mmc_command	cmd;
 	struct mmc_command	stop;
 	struct mmc_data		data;
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 4b91c9e96635..f62b489c9e9c 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1399,13 +1399,13 @@  static inline bool sdhci_auto_cmd12(struct sdhci_host *host,
 static inline bool sdhci_auto_cmd23(struct sdhci_host *host,
 				    struct mmc_request *mrq)
 {
-	return mrq->sbc && (host->flags & SDHCI_AUTO_CMD23);
+	return mrq->sbc && (host->flags & SDHCI_AUTO_CMD23) && !mrq->ext;
 }
 
 static inline bool sdhci_manual_cmd23(struct sdhci_host *host,
 				      struct mmc_request *mrq)
 {
-	return mrq->sbc && !(host->flags & SDHCI_AUTO_CMD23);
+	return mrq->sbc && (mrq->ext || !(host->flags & SDHCI_AUTO_CMD23));
 }
 
 static inline void sdhci_auto_cmd_select(struct sdhci_host *host,
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 2c7928a50907..5560e70cb8d4 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -142,6 +142,7 @@  struct mmc_data {
 struct mmc_host;
 struct mmc_request {
 	struct mmc_command	*sbc;		/* SET_BLOCK_COUNT for multiblock */
+	struct mmc_command	*ext;		/* SD_ADDR_EXT for SDUC */
 	struct mmc_command	*cmd;
 	struct mmc_data		*data;
 	struct mmc_command	*stop;