* [PATCH] mmc: usdhi6rol0: Implement card_busy function
@ 2021-08-12 14:50 Mårten Lindahl
2021-08-13 9:25 ` Ulf Hansson
0 siblings, 1 reply; 3+ messages in thread
From: Mårten Lindahl @ 2021-08-12 14:50 UTC (permalink / raw)
To: Jesper Nilsson, Lars Persson, Ulf Hansson
Cc: kernel, linux-arm-kernel, linux-mmc, Mårten Lindahl
When switching card voltage to UHS voltage the mmc framework tries to
check that the card is driving pins CMD, and DAT[0-3] low before the
switch is made. Drivers that does not implement the card_busy function
will manage to do the switch anyway, but the framework will print a
warning about not being able to verify the voltage signal.
Implement card_busy function. Renesas host interface only exposes pins
DAT[0] and DAT[3] for reading the busy state, which is why only these
two pins are checked.
Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com>
---
drivers/mmc/host/usdhi6rol0.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/mmc/host/usdhi6rol0.c b/drivers/mmc/host/usdhi6rol0.c
index b9b79b1089a0..e400a646e675 100644
--- a/drivers/mmc/host/usdhi6rol0.c
+++ b/drivers/mmc/host/usdhi6rol0.c
@@ -77,6 +77,7 @@
#define USDHI6_SD_INFO1_WP BIT(7)
#define USDHI6_SD_INFO1_D3_CARD_OUT BIT(8)
#define USDHI6_SD_INFO1_D3_CARD_IN BIT(9)
+#define USDHI6_SD_INFO1_SDDAT3 BIT(10)
#define USDHI6_SD_INFO2_CMD_ERR BIT(0)
#define USDHI6_SD_INFO2_CRC_ERR BIT(1)
@@ -1186,6 +1187,21 @@ static int usdhi6_sig_volt_switch(struct mmc_host *mmc, struct mmc_ios *ios)
return ret;
}
+static int usdhi6_card_busy(struct mmc_host *mmc)
+{
+ struct usdhi6_host *host = mmc_priv(mmc);
+ u32 info1 = usdhi6_read(host, USDHI6_SD_INFO1);
+ u32 info2 = usdhi6_read(host, USDHI6_SD_INFO2);
+
+ /* Check if the SD bus is processing a command */
+ if (!(info2 & USDHI6_SD_INFO2_SCLKDIVEN))
+ return 0;
+
+ /* Card is busy if it is pulling dat[0] & dat[3] low */
+ return !(info2 & USDHI6_SD_INFO2_SDDAT0 ||
+ info1 & USDHI6_SD_INFO1_SDDAT3);
+}
+
static const struct mmc_host_ops usdhi6_ops = {
.request = usdhi6_request,
.set_ios = usdhi6_set_ios,
@@ -1193,6 +1209,7 @@ static const struct mmc_host_ops usdhi6_ops = {
.get_ro = usdhi6_get_ro,
.enable_sdio_irq = usdhi6_enable_sdio_irq,
.start_signal_voltage_switch = usdhi6_sig_volt_switch,
+ .card_busy = usdhi6_card_busy,
};
/* State machine handlers */
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] mmc: usdhi6rol0: Implement card_busy function
2021-08-12 14:50 [PATCH] mmc: usdhi6rol0: Implement card_busy function Mårten Lindahl
@ 2021-08-13 9:25 ` Ulf Hansson
2021-08-16 12:26 ` Marten Lindahl
0 siblings, 1 reply; 3+ messages in thread
From: Ulf Hansson @ 2021-08-13 9:25 UTC (permalink / raw)
To: Mårten Lindahl
Cc: Jesper Nilsson, Lars Persson, kernel, linux-arm-kernel, linux-mmc
On Thu, 12 Aug 2021 at 16:51, Mårten Lindahl <marten.lindahl@axis.com> wrote:
>
> When switching card voltage to UHS voltage the mmc framework tries to
> check that the card is driving pins CMD, and DAT[0-3] low before the
> switch is made. Drivers that does not implement the card_busy function
> will manage to do the switch anyway, but the framework will print a
> warning about not being able to verify the voltage signal.
>
> Implement card_busy function. Renesas host interface only exposes pins
> DAT[0] and DAT[3] for reading the busy state, which is why only these
> two pins are checked.
>
> Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com>
> ---
> drivers/mmc/host/usdhi6rol0.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/mmc/host/usdhi6rol0.c b/drivers/mmc/host/usdhi6rol0.c
> index b9b79b1089a0..e400a646e675 100644
> --- a/drivers/mmc/host/usdhi6rol0.c
> +++ b/drivers/mmc/host/usdhi6rol0.c
> @@ -77,6 +77,7 @@
> #define USDHI6_SD_INFO1_WP BIT(7)
> #define USDHI6_SD_INFO1_D3_CARD_OUT BIT(8)
> #define USDHI6_SD_INFO1_D3_CARD_IN BIT(9)
> +#define USDHI6_SD_INFO1_SDDAT3 BIT(10)
>
> #define USDHI6_SD_INFO2_CMD_ERR BIT(0)
> #define USDHI6_SD_INFO2_CRC_ERR BIT(1)
> @@ -1186,6 +1187,21 @@ static int usdhi6_sig_volt_switch(struct mmc_host *mmc, struct mmc_ios *ios)
> return ret;
> }
>
> +static int usdhi6_card_busy(struct mmc_host *mmc)
> +{
> + struct usdhi6_host *host = mmc_priv(mmc);
> + u32 info1 = usdhi6_read(host, USDHI6_SD_INFO1);
> + u32 info2 = usdhi6_read(host, USDHI6_SD_INFO2);
> +
> + /* Check if the SD bus is processing a command */
Hmm, this sounds a bit confusing to me. Don't you want to say
something like "the clock logic is enabled".
The point is, from the mmc core point of view, ->card_busy() is called
to poll for busy completion *after* a command has been processed.
> + if (!(info2 & USDHI6_SD_INFO2_SCLKDIVEN))
> + return 0;
> +
> + /* Card is busy if it is pulling dat[0] & dat[3] low */
> + return !(info2 & USDHI6_SD_INFO2_SDDAT0 ||
> + info1 & USDHI6_SD_INFO1_SDDAT3);
In fact, it's sufficient to monitor any one of the DATA lines to check
for busy for the UHS-I switch, according to the SD spec.
However, since we are using ->card_busy() to monitor also other busy
signal scenarios, like for some commands with R1B responses that
assert only DAT0 to signal busy (at least according to specs), I think
it's better to monitor solely DAT0 here.
I realize that the comment for the ->card_busy() callback in
include/linux/mmc/host.h needs to be updated to clarify this as well.
> +}
> +
> static const struct mmc_host_ops usdhi6_ops = {
> .request = usdhi6_request,
> .set_ios = usdhi6_set_ios,
> @@ -1193,6 +1209,7 @@ static const struct mmc_host_ops usdhi6_ops = {
> .get_ro = usdhi6_get_ro,
> .enable_sdio_irq = usdhi6_enable_sdio_irq,
> .start_signal_voltage_switch = usdhi6_sig_volt_switch,
> + .card_busy = usdhi6_card_busy,
> };
>
> /* State machine handlers */
> --
> 2.20.1
>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mmc: usdhi6rol0: Implement card_busy function
2021-08-13 9:25 ` Ulf Hansson
@ 2021-08-16 12:26 ` Marten Lindahl
0 siblings, 0 replies; 3+ messages in thread
From: Marten Lindahl @ 2021-08-16 12:26 UTC (permalink / raw)
To: Ulf Hansson
Cc: Mårten Lindahl, Jesper Nilsson, Lars Persson, kernel,
linux-arm-kernel, linux-mmc
Hi Ulf!
Thank you for your comments! I will update the patch.
On Fri, Aug 13, 2021 at 11:25:35AM +0200, Ulf Hansson wrote:
> On Thu, 12 Aug 2021 at 16:51, Mårten Lindahl <marten.lindahl@axis.com> wrote:
> >
> > When switching card voltage to UHS voltage the mmc framework tries to
> > check that the card is driving pins CMD, and DAT[0-3] low before the
> > switch is made. Drivers that does not implement the card_busy function
> > will manage to do the switch anyway, but the framework will print a
> > warning about not being able to verify the voltage signal.
> >
> > Implement card_busy function. Renesas host interface only exposes pins
> > DAT[0] and DAT[3] for reading the busy state, which is why only these
> > two pins are checked.
> >
> > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com>
> > ---
> > drivers/mmc/host/usdhi6rol0.c | 17 +++++++++++++++++
> > 1 file changed, 17 insertions(+)
> >
> > diff --git a/drivers/mmc/host/usdhi6rol0.c b/drivers/mmc/host/usdhi6rol0.c
> > index b9b79b1089a0..e400a646e675 100644
> > --- a/drivers/mmc/host/usdhi6rol0.c
> > +++ b/drivers/mmc/host/usdhi6rol0.c
> > @@ -77,6 +77,7 @@
> > #define USDHI6_SD_INFO1_WP BIT(7)
> > #define USDHI6_SD_INFO1_D3_CARD_OUT BIT(8)
> > #define USDHI6_SD_INFO1_D3_CARD_IN BIT(9)
> > +#define USDHI6_SD_INFO1_SDDAT3 BIT(10)
> >
> > #define USDHI6_SD_INFO2_CMD_ERR BIT(0)
> > #define USDHI6_SD_INFO2_CRC_ERR BIT(1)
> > @@ -1186,6 +1187,21 @@ static int usdhi6_sig_volt_switch(struct mmc_host *mmc, struct mmc_ios *ios)
> > return ret;
> > }
> >
> > +static int usdhi6_card_busy(struct mmc_host *mmc)
> > +{
> > + struct usdhi6_host *host = mmc_priv(mmc);
> > + u32 info1 = usdhi6_read(host, USDHI6_SD_INFO1);
> > + u32 info2 = usdhi6_read(host, USDHI6_SD_INFO2);
> > +
> > + /* Check if the SD bus is processing a command */
>
> Hmm, this sounds a bit confusing to me. Don't you want to say
> something like "the clock logic is enabled".
>
> The point is, from the mmc core point of view, ->card_busy() is called
> to poll for busy completion *after* a command has been processed.
>
Thanks. After reading the SD specification my understanding is that it
should be enough to solely check DAT0 in this case. I will remove the
check for SCLKDIVEN.
> > + if (!(info2 & USDHI6_SD_INFO2_SCLKDIVEN))
> > + return 0;
> > +
> > + /* Card is busy if it is pulling dat[0] & dat[3] low */
> > + return !(info2 & USDHI6_SD_INFO2_SDDAT0 ||
> > + info1 & USDHI6_SD_INFO1_SDDAT3);
>
> In fact, it's sufficient to monitor any one of the DATA lines to check
> for busy for the UHS-I switch, according to the SD spec.
>
> However, since we are using ->card_busy() to monitor also other busy
> signal scenarios, like for some commands with R1B responses that
> assert only DAT0 to signal busy (at least according to specs), I think
> it's better to monitor solely DAT0 here.
>
I will remove the check for DAT3.
> I realize that the comment for the ->card_busy() callback in
> include/linux/mmc/host.h needs to be updated to clarify this as well.
>
I will update the card_busy callback comment to DAT0 only.
Kind regards
Mårten
> > +}
> > +
> > static const struct mmc_host_ops usdhi6_ops = {
> > .request = usdhi6_request,
> > .set_ios = usdhi6_set_ios,
> > @@ -1193,6 +1209,7 @@ static const struct mmc_host_ops usdhi6_ops = {
> > .get_ro = usdhi6_get_ro,
> > .enable_sdio_irq = usdhi6_enable_sdio_irq,
> > .start_signal_voltage_switch = usdhi6_sig_volt_switch,
> > + .card_busy = usdhi6_card_busy,
> > };
> >
> > /* State machine handlers */
> > --
> > 2.20.1
> >
>
> Kind regards
> Uffe
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-08-16 12:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-12 14:50 [PATCH] mmc: usdhi6rol0: Implement card_busy function Mårten Lindahl
2021-08-13 9:25 ` Ulf Hansson
2021-08-16 12:26 ` Marten Lindahl
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.