* [PATCH] mmc: core: Initial support for SD express card/host
@ 2020-07-16 14:15 Ulf Hansson
2020-07-16 16:13 ` Greg Kroah-Hartman
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Ulf Hansson @ 2020-07-16 14:15 UTC (permalink / raw)
To: linux-mmc, Christoph Hellwig, Arnd Bergmann, Rui Feng
Cc: linux-nvme, linux-pci, linux-kernel, Adrian Hunter, Ulf Hansson,
Greg Kroah-Hartman
In the SD specification v7.10 the SD express card has been added. This new
type of removable SD card, can be managed via a PCIe/NVMe based interface,
while also allowing backwards compatibility towards the legacy SD
interface.
To keep the backwards compatibility, it's required to start the
initialization through the legacy SD interface. If it turns out that the
mmc host and the SD card, both supports the PCIe/NVMe interface, then a
switch should be allowed.
Therefore, let's introduce some basic support for this type of SD cards to
the mmc core. The mmc host, should set MMC_CAP2_SD_EXP if it supports this
interface and MMC_CAP2_SD_EXP_1_2V, if also 1.2V is supported, as to inform
the core about it.
To deal with the switch to the PCIe/NVMe interface, the mmc host is
required to implement a new host ops, ->init_sd_express(). Based on the
initial communication between the host and the card, host->ios.timing is
set to either MMC_TIMING_SD_EXP or MMC_TIMING_SD_EXP_1_2V, depending on if
1.2V is supported or not. In this way, the mmc host can check these values
in its ->init_sd_express() ops, to know how to proceed with the handover.
Note that, to manage card insert/removal, the mmc core sticks with using
the ->get_cd() callback, which means it's the host's responsibility to make
sure it provides valid data, even if the card may be managed by PCIe/NVMe
at the moment. As long as the card seems to be present, the mmc core keeps
the card powered on.
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Rui Feng <rui_feng@realsil.com.cn>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/core/core.c | 15 ++++++++++--
drivers/mmc/core/host.h | 6 +++++
drivers/mmc/core/sd_ops.c | 49 +++++++++++++++++++++++++++++++++++++--
drivers/mmc/core/sd_ops.h | 1 +
include/linux/mmc/host.h | 7 ++++++
5 files changed, 74 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 8ccae6452b9c..6673c0f33cc7 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2137,8 +2137,12 @@ static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq)
mmc_go_idle(host);
- if (!(host->caps2 & MMC_CAP2_NO_SD))
- mmc_send_if_cond(host, host->ocr_avail);
+ if (!(host->caps2 & MMC_CAP2_NO_SD)) {
+ if (mmc_send_if_cond_pcie(host, host->ocr_avail))
+ goto out;
+ if (mmc_card_sd_express(host))
+ return 0;
+ }
/* Order's important: probe SDIO, then SD, then MMC */
if (!(host->caps2 & MMC_CAP2_NO_SDIO))
@@ -2153,6 +2157,7 @@ static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq)
if (!mmc_attach_mmc(host))
return 0;
+out:
mmc_power_off(host);
return -EIO;
}
@@ -2280,6 +2285,12 @@ void mmc_rescan(struct work_struct *work)
goto out;
}
+ /* If an SD express card is present, then leave it as is. */
+ if (mmc_card_sd_express(host)) {
+ mmc_release_host(host);
+ goto out;
+ }
+
for (i = 0; i < ARRAY_SIZE(freqs); i++) {
unsigned int freq = freqs[i];
if (freq > host->f_max) {
diff --git a/drivers/mmc/core/host.h b/drivers/mmc/core/host.h
index 5e3b9534ffb2..ba407617ed23 100644
--- a/drivers/mmc/core/host.h
+++ b/drivers/mmc/core/host.h
@@ -77,5 +77,11 @@ static inline bool mmc_card_hs400es(struct mmc_card *card)
return card->host->ios.enhanced_strobe;
}
+static inline bool mmc_card_sd_express(struct mmc_host *host)
+{
+ return host->ios.timing == MMC_TIMING_SD_EXP ||
+ host->ios.timing == MMC_TIMING_SD_EXP_1_2V;
+}
+
#endif
diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c
index 22bf528294b9..d61ff811218c 100644
--- a/drivers/mmc/core/sd_ops.c
+++ b/drivers/mmc/core/sd_ops.c
@@ -158,7 +158,8 @@ int mmc_send_app_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr)
return err;
}
-int mmc_send_if_cond(struct mmc_host *host, u32 ocr)
+static int __mmc_send_if_cond(struct mmc_host *host, u32 ocr, u8 pcie_bits,
+ u32 *resp)
{
struct mmc_command cmd = {};
int err;
@@ -171,7 +172,7 @@ int mmc_send_if_cond(struct mmc_host *host, u32 ocr)
* SD 1.0 cards.
*/
cmd.opcode = SD_SEND_IF_COND;
- cmd.arg = ((ocr & 0xFF8000) != 0) << 8 | test_pattern;
+ cmd.arg = ((ocr & 0xFF8000) != 0) << 8 | pcie_bits << 8 | test_pattern;
cmd.flags = MMC_RSP_SPI_R7 | MMC_RSP_R7 | MMC_CMD_BCR;
err = mmc_wait_for_cmd(host, &cmd, 0);
@@ -186,6 +187,50 @@ int mmc_send_if_cond(struct mmc_host *host, u32 ocr)
if (result_pattern != test_pattern)
return -EIO;
+ if (resp)
+ *resp = cmd.resp[0];
+
+ return 0;
+}
+
+int mmc_send_if_cond(struct mmc_host *host, u32 ocr)
+{
+ return __mmc_send_if_cond(host, ocr, 0, NULL);
+}
+
+int mmc_send_if_cond_pcie(struct mmc_host *host, u32 ocr)
+{
+ u32 resp = 0;
+ u8 pcie_bits = 0;
+ int ret;
+
+ if (host->caps2 & MMC_CAP2_SD_EXP) {
+ /* Probe card for SD express support via PCIe. */
+ pcie_bits = 0x10;
+ if (host->caps2 & MMC_CAP2_SD_EXP_1_2V)
+ /* Probe also for 1.2V support. */
+ pcie_bits = 0x30;
+ }
+
+ ret = __mmc_send_if_cond(host, ocr, pcie_bits, &resp);
+ if (ret)
+ return 0;
+
+ /* Continue with the SD express init, if the card supports it. */
+ resp &= 0x3000;
+ if (pcie_bits && resp) {
+ if (resp == 0x3000)
+ host->ios.timing = MMC_TIMING_SD_EXP_1_2V;
+ else
+ host->ios.timing = MMC_TIMING_SD_EXP;
+
+ /*
+ * According to the spec the clock shall also be gated, but
+ * let's leave this to the host driver for more flexibility.
+ */
+ return host->ops->init_sd_express(host, &host->ios);
+ }
+
return 0;
}
diff --git a/drivers/mmc/core/sd_ops.h b/drivers/mmc/core/sd_ops.h
index 2194cabfcfc5..3ba7b3cf4652 100644
--- a/drivers/mmc/core/sd_ops.h
+++ b/drivers/mmc/core/sd_ops.h
@@ -16,6 +16,7 @@ struct mmc_host;
int mmc_app_set_bus_width(struct mmc_card *card, int width);
int mmc_send_app_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr);
int mmc_send_if_cond(struct mmc_host *host, u32 ocr);
+int mmc_send_if_cond_pcie(struct mmc_host *host, u32 ocr);
int mmc_send_relative_addr(struct mmc_host *host, unsigned int *rca);
int mmc_app_send_scr(struct mmc_card *card);
int mmc_sd_switch(struct mmc_card *card, int mode, int group,
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index c5b6e97cb21a..905cddc5e6f3 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -60,6 +60,8 @@ struct mmc_ios {
#define MMC_TIMING_MMC_DDR52 8
#define MMC_TIMING_MMC_HS200 9
#define MMC_TIMING_MMC_HS400 10
+#define MMC_TIMING_SD_EXP 11
+#define MMC_TIMING_SD_EXP_1_2V 12
unsigned char signal_voltage; /* signalling voltage (1.8V or 3.3V) */
@@ -172,6 +174,9 @@ struct mmc_host_ops {
*/
int (*multi_io_quirk)(struct mmc_card *card,
unsigned int direction, int blk_size);
+
+ /* Initialize an SD express card, mandatory for MMC_CAP2_SD_EXP. */
+ int (*init_sd_express)(struct mmc_host *host, struct mmc_ios *ios);
};
struct mmc_cqe_ops {
@@ -357,6 +362,8 @@ struct mmc_host {
#define MMC_CAP2_HS200_1_2V_SDR (1 << 6) /* can support */
#define MMC_CAP2_HS200 (MMC_CAP2_HS200_1_8V_SDR | \
MMC_CAP2_HS200_1_2V_SDR)
+#define MMC_CAP2_SD_EXP (1 << 7) /* SD express via PCIe */
+#define MMC_CAP2_SD_EXP_1_2V (1 << 8) /* SD express 1.2V */
#define MMC_CAP2_CD_ACTIVE_HIGH (1 << 10) /* Card-detect signal active high */
#define MMC_CAP2_RO_ACTIVE_HIGH (1 << 11) /* Write-protect signal active high */
#define MMC_CAP2_NO_PRESCAN_POWERUP (1 << 14) /* Don't power up before scan */
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] mmc: core: Initial support for SD express card/host
2020-07-16 14:15 [PATCH] mmc: core: Initial support for SD express card/host Ulf Hansson
@ 2020-07-16 16:13 ` Greg Kroah-Hartman
2020-07-16 17:04 ` Ulf Hansson
2020-07-16 18:22 ` Arnd Bergmann
2020-08-21 12:40 ` Ulf Hansson
2 siblings, 1 reply; 11+ messages in thread
From: Greg Kroah-Hartman @ 2020-07-16 16:13 UTC (permalink / raw)
To: Ulf Hansson
Cc: linux-mmc, Christoph Hellwig, Arnd Bergmann, Rui Feng,
linux-nvme, linux-pci, linux-kernel, Adrian Hunter
On Thu, Jul 16, 2020 at 04:15:34PM +0200, Ulf Hansson wrote:
> +int mmc_send_if_cond_pcie(struct mmc_host *host, u32 ocr)
> +{
> + u32 resp = 0;
> + u8 pcie_bits = 0;
> + int ret;
> +
> + if (host->caps2 & MMC_CAP2_SD_EXP) {
> + /* Probe card for SD express support via PCIe. */
> + pcie_bits = 0x10;
> + if (host->caps2 & MMC_CAP2_SD_EXP_1_2V)
> + /* Probe also for 1.2V support. */
> + pcie_bits = 0x30;
> + }
> +
> + ret = __mmc_send_if_cond(host, ocr, pcie_bits, &resp);
> + if (ret)
> + return 0;
> +
> + /* Continue with the SD express init, if the card supports it. */
> + resp &= 0x3000;
> + if (pcie_bits && resp) {
> + if (resp == 0x3000)
0x3000 should be some defined value, right? Otherwise it just looks
like magic bits :)
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -60,6 +60,8 @@ struct mmc_ios {
> #define MMC_TIMING_MMC_DDR52 8
> #define MMC_TIMING_MMC_HS200 9
> #define MMC_TIMING_MMC_HS400 10
> +#define MMC_TIMING_SD_EXP 11
> +#define MMC_TIMING_SD_EXP_1_2V 12
>
> unsigned char signal_voltage; /* signalling voltage (1.8V or 3.3V) */
>
> @@ -172,6 +174,9 @@ struct mmc_host_ops {
> */
> int (*multi_io_quirk)(struct mmc_card *card,
> unsigned int direction, int blk_size);
> +
> + /* Initialize an SD express card, mandatory for MMC_CAP2_SD_EXP. */
> + int (*init_sd_express)(struct mmc_host *host, struct mmc_ios *ios);
> };
>
> struct mmc_cqe_ops {
> @@ -357,6 +362,8 @@ struct mmc_host {
> #define MMC_CAP2_HS200_1_2V_SDR (1 << 6) /* can support */
> #define MMC_CAP2_HS200 (MMC_CAP2_HS200_1_8V_SDR | \
> MMC_CAP2_HS200_1_2V_SDR)
> +#define MMC_CAP2_SD_EXP (1 << 7) /* SD express via PCIe */
BIT(7)?
> +#define MMC_CAP2_SD_EXP_1_2V (1 << 8) /* SD express 1.2V */
BIT(8)?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] mmc: core: Initial support for SD express card/host
2020-07-16 16:13 ` Greg Kroah-Hartman
@ 2020-07-16 17:04 ` Ulf Hansson
2020-07-16 17:21 ` Greg Kroah-Hartman
0 siblings, 1 reply; 11+ messages in thread
From: Ulf Hansson @ 2020-07-16 17:04 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-mmc, Christoph Hellwig, Arnd Bergmann, Rui Feng,
linux-nvme, Linux PCI, Linux Kernel Mailing List, Adrian Hunter
On Thu, 16 Jul 2020 at 18:14, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Thu, Jul 16, 2020 at 04:15:34PM +0200, Ulf Hansson wrote:
> > +int mmc_send_if_cond_pcie(struct mmc_host *host, u32 ocr)
> > +{
> > + u32 resp = 0;
> > + u8 pcie_bits = 0;
> > + int ret;
> > +
> > + if (host->caps2 & MMC_CAP2_SD_EXP) {
> > + /* Probe card for SD express support via PCIe. */
> > + pcie_bits = 0x10;
> > + if (host->caps2 & MMC_CAP2_SD_EXP_1_2V)
> > + /* Probe also for 1.2V support. */
> > + pcie_bits = 0x30;
> > + }
> > +
> > + ret = __mmc_send_if_cond(host, ocr, pcie_bits, &resp);
> > + if (ret)
> > + return 0;
> > +
> > + /* Continue with the SD express init, if the card supports it. */
> > + resp &= 0x3000;
> > + if (pcie_bits && resp) {
> > + if (resp == 0x3000)
>
> 0x3000 should be some defined value, right? Otherwise it just looks
> like magic bits :)
Yeah, I was considering that, but there are already lots of magic bits
around here in this code. On top of that, the bits are shifted,
depending on how they are used.
We should probably look into doing a cleanup, so this gets clearer overall.
>
> > --- a/include/linux/mmc/host.h
> > +++ b/include/linux/mmc/host.h
> > @@ -60,6 +60,8 @@ struct mmc_ios {
> > #define MMC_TIMING_MMC_DDR52 8
> > #define MMC_TIMING_MMC_HS200 9
> > #define MMC_TIMING_MMC_HS400 10
> > +#define MMC_TIMING_SD_EXP 11
> > +#define MMC_TIMING_SD_EXP_1_2V 12
> >
> > unsigned char signal_voltage; /* signalling voltage (1.8V or 3.3V) */
> >
> > @@ -172,6 +174,9 @@ struct mmc_host_ops {
> > */
> > int (*multi_io_quirk)(struct mmc_card *card,
> > unsigned int direction, int blk_size);
> > +
> > + /* Initialize an SD express card, mandatory for MMC_CAP2_SD_EXP. */
> > + int (*init_sd_express)(struct mmc_host *host, struct mmc_ios *ios);
> > };
> >
> > struct mmc_cqe_ops {
> > @@ -357,6 +362,8 @@ struct mmc_host {
> > #define MMC_CAP2_HS200_1_2V_SDR (1 << 6) /* can support */
> > #define MMC_CAP2_HS200 (MMC_CAP2_HS200_1_8V_SDR | \
> > MMC_CAP2_HS200_1_2V_SDR)
> > +#define MMC_CAP2_SD_EXP (1 << 7) /* SD express via PCIe */
>
> BIT(7)?
>
> > +#define MMC_CAP2_SD_EXP_1_2V (1 << 8) /* SD express 1.2V */
>
> BIT(8)?
I can change to that, but it wouldn't be consistent with existing
code. Again, probably better targeted as a separate bigger cleanup on
top.
>
> thanks,
>
> greg k-h
Thanks for reviewing!
Kind regards
Uffe
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] mmc: core: Initial support for SD express card/host
2020-07-16 17:04 ` Ulf Hansson
@ 2020-07-16 17:21 ` Greg Kroah-Hartman
0 siblings, 0 replies; 11+ messages in thread
From: Greg Kroah-Hartman @ 2020-07-16 17:21 UTC (permalink / raw)
To: Ulf Hansson
Cc: linux-mmc, Christoph Hellwig, Arnd Bergmann, Rui Feng,
linux-nvme, Linux PCI, Linux Kernel Mailing List, Adrian Hunter
On Thu, Jul 16, 2020 at 07:04:21PM +0200, Ulf Hansson wrote:
> On Thu, 16 Jul 2020 at 18:14, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Thu, Jul 16, 2020 at 04:15:34PM +0200, Ulf Hansson wrote:
> > > +int mmc_send_if_cond_pcie(struct mmc_host *host, u32 ocr)
> > > +{
> > > + u32 resp = 0;
> > > + u8 pcie_bits = 0;
> > > + int ret;
> > > +
> > > + if (host->caps2 & MMC_CAP2_SD_EXP) {
> > > + /* Probe card for SD express support via PCIe. */
> > > + pcie_bits = 0x10;
> > > + if (host->caps2 & MMC_CAP2_SD_EXP_1_2V)
> > > + /* Probe also for 1.2V support. */
> > > + pcie_bits = 0x30;
> > > + }
> > > +
> > > + ret = __mmc_send_if_cond(host, ocr, pcie_bits, &resp);
> > > + if (ret)
> > > + return 0;
> > > +
> > > + /* Continue with the SD express init, if the card supports it. */
> > > + resp &= 0x3000;
> > > + if (pcie_bits && resp) {
> > > + if (resp == 0x3000)
> >
> > 0x3000 should be some defined value, right? Otherwise it just looks
> > like magic bits :)
>
> Yeah, I was considering that, but there are already lots of magic bits
> around here in this code. On top of that, the bits are shifted,
> depending on how they are used.
>
> We should probably look into doing a cleanup, so this gets clearer overall.
>
> >
> > > --- a/include/linux/mmc/host.h
> > > +++ b/include/linux/mmc/host.h
> > > @@ -60,6 +60,8 @@ struct mmc_ios {
> > > #define MMC_TIMING_MMC_DDR52 8
> > > #define MMC_TIMING_MMC_HS200 9
> > > #define MMC_TIMING_MMC_HS400 10
> > > +#define MMC_TIMING_SD_EXP 11
> > > +#define MMC_TIMING_SD_EXP_1_2V 12
> > >
> > > unsigned char signal_voltage; /* signalling voltage (1.8V or 3.3V) */
> > >
> > > @@ -172,6 +174,9 @@ struct mmc_host_ops {
> > > */
> > > int (*multi_io_quirk)(struct mmc_card *card,
> > > unsigned int direction, int blk_size);
> > > +
> > > + /* Initialize an SD express card, mandatory for MMC_CAP2_SD_EXP. */
> > > + int (*init_sd_express)(struct mmc_host *host, struct mmc_ios *ios);
> > > };
> > >
> > > struct mmc_cqe_ops {
> > > @@ -357,6 +362,8 @@ struct mmc_host {
> > > #define MMC_CAP2_HS200_1_2V_SDR (1 << 6) /* can support */
> > > #define MMC_CAP2_HS200 (MMC_CAP2_HS200_1_8V_SDR | \
> > > MMC_CAP2_HS200_1_2V_SDR)
> > > +#define MMC_CAP2_SD_EXP (1 << 7) /* SD express via PCIe */
> >
> > BIT(7)?
> >
> > > +#define MMC_CAP2_SD_EXP_1_2V (1 << 8) /* SD express 1.2V */
> >
> > BIT(8)?
>
> I can change to that, but it wouldn't be consistent with existing
> code. Again, probably better targeted as a separate bigger cleanup on
> top.
Ah, good point.
Ok, no objection from me!
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mmc: core: Initial support for SD express card/host
2020-07-16 14:15 [PATCH] mmc: core: Initial support for SD express card/host Ulf Hansson
2020-07-16 16:13 ` Greg Kroah-Hartman
@ 2020-07-16 18:22 ` Arnd Bergmann
2020-07-24 10:06 ` Ulf Hansson
2020-08-21 12:40 ` Ulf Hansson
2 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2020-07-16 18:22 UTC (permalink / raw)
To: Ulf Hansson
Cc: linux-mmc, Christoph Hellwig, Rui Feng, linux-nvme, linux-pci,
linux-kernel, Adrian Hunter, Greg Kroah-Hartman
On Thu, Jul 16, 2020 at 4:16 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> + /* Continue with the SD express init, if the card supports it. */
> + resp &= 0x3000;
> + if (pcie_bits && resp) {
> + if (resp == 0x3000)
> + host->ios.timing = MMC_TIMING_SD_EXP_1_2V;
> + else
> + host->ios.timing = MMC_TIMING_SD_EXP;
> +
> + /*
> + * According to the spec the clock shall also be gated, but
> + * let's leave this to the host driver for more flexibility.
> + */
> + return host->ops->init_sd_express(host, &host->ios);
> + }
> +
> return 0;
> }
Does this need an additional handshake or timeout to see if the
device was successfully probed by the nvme driver?
It looks like the card would just disappear here if e.g. the nvme driver
is not loaded or runs into a runtime error.
Arnd
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] mmc: core: Initial support for SD express card/host
2020-07-16 18:22 ` Arnd Bergmann
@ 2020-07-24 10:06 ` Ulf Hansson
2020-07-24 13:35 ` Arnd Bergmann
0 siblings, 1 reply; 11+ messages in thread
From: Ulf Hansson @ 2020-07-24 10:06 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-mmc, Christoph Hellwig, Rui Feng, linux-nvme, linux-pci,
linux-kernel, Adrian Hunter, Greg Kroah-Hartman
On Thu, 16 Jul 2020 at 20:23, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Jul 16, 2020 at 4:16 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> > + /* Continue with the SD express init, if the card supports it. */
> > + resp &= 0x3000;
> > + if (pcie_bits && resp) {
> > + if (resp == 0x3000)
> > + host->ios.timing = MMC_TIMING_SD_EXP_1_2V;
> > + else
> > + host->ios.timing = MMC_TIMING_SD_EXP;
> > +
> > + /*
> > + * According to the spec the clock shall also be gated, but
> > + * let's leave this to the host driver for more flexibility.
> > + */
> > + return host->ops->init_sd_express(host, &host->ios);
> > + }
> > +
> > return 0;
> > }
>
> Does this need an additional handshake or timeout to see if the
> device was successfully probed by the nvme driver?
>
> It looks like the card would just disappear here if e.g. the nvme driver
> is not loaded or runs into a runtime error.
You are correct! In principle, the card would not be detected (it
doesn't disappear as it never gets registered). Instead, it's left in
"half-initialized" state, waiting for the nvme driver to take over.
I simply didn't want to go that far, to introduce synchronizations
steps between the nvme driver and mmc driver, but rather started
simple. Perhaps we can discuss these things as improvements on top
instead?
What do you think?
Kind regards
Uffe
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] mmc: core: Initial support for SD express card/host
2020-07-24 10:06 ` Ulf Hansson
@ 2020-07-24 13:35 ` Arnd Bergmann
2020-07-24 13:44 ` Christoph Hellwig
0 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2020-07-24 13:35 UTC (permalink / raw)
To: Ulf Hansson
Cc: linux-mmc, Christoph Hellwig, Rui Feng, linux-nvme, linux-pci,
linux-kernel, Adrian Hunter, Greg Kroah-Hartman
On Fri, Jul 24, 2020 at 12:06 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On Thu, 16 Jul 2020 at 20:23, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thu, Jul 16, 2020 at 4:16 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > > + /* Continue with the SD express init, if the card supports it. */
> > > + resp &= 0x3000;
> > > + if (pcie_bits && resp) {
> > > + if (resp == 0x3000)
> > > + host->ios.timing = MMC_TIMING_SD_EXP_1_2V;
> > > + else
> > > + host->ios.timing = MMC_TIMING_SD_EXP;
> > > +
> > > + /*
> > > + * According to the spec the clock shall also be gated, but
> > > + * let's leave this to the host driver for more flexibility.
> > > + */
> > > + return host->ops->init_sd_express(host, &host->ios);
> > > + }
> > > +
> > > return 0;
> > > }
> >
> > Does this need an additional handshake or timeout to see if the
> > device was successfully probed by the nvme driver?
> >
> > It looks like the card would just disappear here if e.g. the nvme driver
> > is not loaded or runs into a runtime error.
>
> You are correct! In principle, the card would not be detected (it
> doesn't disappear as it never gets registered). Instead, it's left in
> "half-initialized" state, waiting for the nvme driver to take over.
>
> I simply didn't want to go that far, to introduce synchronizations
> steps between the nvme driver and mmc driver, but rather started
> simple. Perhaps we can discuss these things as improvements on top
> instead?
>
> What do you think?
Starting simple is generally a good idea, yes.
It would be good to have feedback from the nvme driver maintainers.
One way I can see the handshake working would be to have
an sdexpress class_driver that provides interfaces for both mmc
and nvme to link against. The mmc core can then create a
class device when it finds an sd-express device and that
class device contains a simple state machine that keeps track of
what either side think is going on, possibly also providing
a way to perform callbacks between the two sides.
Arnd
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] mmc: core: Initial support for SD express card/host
2020-07-24 13:35 ` Arnd Bergmann
@ 2020-07-24 13:44 ` Christoph Hellwig
0 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2020-07-24 13:44 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Ulf Hansson, linux-mmc, Christoph Hellwig, Rui Feng, linux-nvme,
linux-pci, linux-kernel, Adrian Hunter, Greg Kroah-Hartman
On Fri, Jul 24, 2020 at 03:35:47PM +0200, Arnd Bergmann wrote:
> Starting simple is generally a good idea, yes.
>
> It would be good to have feedback from the nvme driver maintainers.
>
> One way I can see the handshake working would be to have
> an sdexpress class_driver that provides interfaces for both mmc
> and nvme to link against. The mmc core can then create a
> class device when it finds an sd-express device and that
> class device contains a simple state machine that keeps track of
> what either side think is going on, possibly also providing
> a way to perform callbacks between the two sides.
None of this is in scope for the NVMe spec, so I don't really want
to deal with it in the NVMe driver in any way. Given that a SD
express card just turns into a normal PCIe link if you really want
to check that something probed I think you'd want to track if any
PCIe driver is bound to the device. Or just wait and see if we really
need anything after all.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mmc: core: Initial support for SD express card/host
2020-07-16 14:15 [PATCH] mmc: core: Initial support for SD express card/host Ulf Hansson
2020-07-16 16:13 ` Greg Kroah-Hartman
2020-07-16 18:22 ` Arnd Bergmann
@ 2020-08-21 12:40 ` Ulf Hansson
2020-08-24 1:04 ` 答复: " 冯锐
2 siblings, 1 reply; 11+ messages in thread
From: Ulf Hansson @ 2020-08-21 12:40 UTC (permalink / raw)
To: Rui Feng
Cc: Christoph Hellwig, Arnd Bergmann, linux-nvme, linux-mmc,
Linux PCI, Linux Kernel Mailing List, Adrian Hunter,
Greg Kroah-Hartman
Rui,
On Thu, 16 Jul 2020 at 16:16, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> In the SD specification v7.10 the SD express card has been added. This new
> type of removable SD card, can be managed via a PCIe/NVMe based interface,
> while also allowing backwards compatibility towards the legacy SD
> interface.
>
> To keep the backwards compatibility, it's required to start the
> initialization through the legacy SD interface. If it turns out that the
> mmc host and the SD card, both supports the PCIe/NVMe interface, then a
> switch should be allowed.
>
> Therefore, let's introduce some basic support for this type of SD cards to
> the mmc core. The mmc host, should set MMC_CAP2_SD_EXP if it supports this
> interface and MMC_CAP2_SD_EXP_1_2V, if also 1.2V is supported, as to inform
> the core about it.
>
> To deal with the switch to the PCIe/NVMe interface, the mmc host is
> required to implement a new host ops, ->init_sd_express(). Based on the
> initial communication between the host and the card, host->ios.timing is
> set to either MMC_TIMING_SD_EXP or MMC_TIMING_SD_EXP_1_2V, depending on if
> 1.2V is supported or not. In this way, the mmc host can check these values
> in its ->init_sd_express() ops, to know how to proceed with the handover.
>
> Note that, to manage card insert/removal, the mmc core sticks with using
> the ->get_cd() callback, which means it's the host's responsibility to make
> sure it provides valid data, even if the card may be managed by PCIe/NVMe
> at the moment. As long as the card seems to be present, the mmc core keeps
> the card powered on.
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Rui Feng <rui_feng@realsil.com.cn>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Rui, did you manage to get some time to look at $subject patch?
If you need some help to understand what's needed to implement the
corresponding support in drivers/mmc/host/rtsx_pci_sdmmc.c, then
please just ask.
I think it would make sense to queue changes for rtsx_pci at the same
point as the mmc core changes. That's because I don't want to maintain
code in the mmc core that's left unused.
Kind regards
Uffe
> ---
> drivers/mmc/core/core.c | 15 ++++++++++--
> drivers/mmc/core/host.h | 6 +++++
> drivers/mmc/core/sd_ops.c | 49 +++++++++++++++++++++++++++++++++++++--
> drivers/mmc/core/sd_ops.h | 1 +
> include/linux/mmc/host.h | 7 ++++++
> 5 files changed, 74 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 8ccae6452b9c..6673c0f33cc7 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2137,8 +2137,12 @@ static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq)
>
> mmc_go_idle(host);
>
> - if (!(host->caps2 & MMC_CAP2_NO_SD))
> - mmc_send_if_cond(host, host->ocr_avail);
> + if (!(host->caps2 & MMC_CAP2_NO_SD)) {
> + if (mmc_send_if_cond_pcie(host, host->ocr_avail))
> + goto out;
> + if (mmc_card_sd_express(host))
> + return 0;
> + }
>
> /* Order's important: probe SDIO, then SD, then MMC */
> if (!(host->caps2 & MMC_CAP2_NO_SDIO))
> @@ -2153,6 +2157,7 @@ static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq)
> if (!mmc_attach_mmc(host))
> return 0;
>
> +out:
> mmc_power_off(host);
> return -EIO;
> }
> @@ -2280,6 +2285,12 @@ void mmc_rescan(struct work_struct *work)
> goto out;
> }
>
> + /* If an SD express card is present, then leave it as is. */
> + if (mmc_card_sd_express(host)) {
> + mmc_release_host(host);
> + goto out;
> + }
> +
> for (i = 0; i < ARRAY_SIZE(freqs); i++) {
> unsigned int freq = freqs[i];
> if (freq > host->f_max) {
> diff --git a/drivers/mmc/core/host.h b/drivers/mmc/core/host.h
> index 5e3b9534ffb2..ba407617ed23 100644
> --- a/drivers/mmc/core/host.h
> +++ b/drivers/mmc/core/host.h
> @@ -77,5 +77,11 @@ static inline bool mmc_card_hs400es(struct mmc_card *card)
> return card->host->ios.enhanced_strobe;
> }
>
> +static inline bool mmc_card_sd_express(struct mmc_host *host)
> +{
> + return host->ios.timing == MMC_TIMING_SD_EXP ||
> + host->ios.timing == MMC_TIMING_SD_EXP_1_2V;
> +}
> +
> #endif
>
> diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c
> index 22bf528294b9..d61ff811218c 100644
> --- a/drivers/mmc/core/sd_ops.c
> +++ b/drivers/mmc/core/sd_ops.c
> @@ -158,7 +158,8 @@ int mmc_send_app_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr)
> return err;
> }
>
> -int mmc_send_if_cond(struct mmc_host *host, u32 ocr)
> +static int __mmc_send_if_cond(struct mmc_host *host, u32 ocr, u8 pcie_bits,
> + u32 *resp)
> {
> struct mmc_command cmd = {};
> int err;
> @@ -171,7 +172,7 @@ int mmc_send_if_cond(struct mmc_host *host, u32 ocr)
> * SD 1.0 cards.
> */
> cmd.opcode = SD_SEND_IF_COND;
> - cmd.arg = ((ocr & 0xFF8000) != 0) << 8 | test_pattern;
> + cmd.arg = ((ocr & 0xFF8000) != 0) << 8 | pcie_bits << 8 | test_pattern;
> cmd.flags = MMC_RSP_SPI_R7 | MMC_RSP_R7 | MMC_CMD_BCR;
>
> err = mmc_wait_for_cmd(host, &cmd, 0);
> @@ -186,6 +187,50 @@ int mmc_send_if_cond(struct mmc_host *host, u32 ocr)
> if (result_pattern != test_pattern)
> return -EIO;
>
> + if (resp)
> + *resp = cmd.resp[0];
> +
> + return 0;
> +}
> +
> +int mmc_send_if_cond(struct mmc_host *host, u32 ocr)
> +{
> + return __mmc_send_if_cond(host, ocr, 0, NULL);
> +}
> +
> +int mmc_send_if_cond_pcie(struct mmc_host *host, u32 ocr)
> +{
> + u32 resp = 0;
> + u8 pcie_bits = 0;
> + int ret;
> +
> + if (host->caps2 & MMC_CAP2_SD_EXP) {
> + /* Probe card for SD express support via PCIe. */
> + pcie_bits = 0x10;
> + if (host->caps2 & MMC_CAP2_SD_EXP_1_2V)
> + /* Probe also for 1.2V support. */
> + pcie_bits = 0x30;
> + }
> +
> + ret = __mmc_send_if_cond(host, ocr, pcie_bits, &resp);
> + if (ret)
> + return 0;
> +
> + /* Continue with the SD express init, if the card supports it. */
> + resp &= 0x3000;
> + if (pcie_bits && resp) {
> + if (resp == 0x3000)
> + host->ios.timing = MMC_TIMING_SD_EXP_1_2V;
> + else
> + host->ios.timing = MMC_TIMING_SD_EXP;
> +
> + /*
> + * According to the spec the clock shall also be gated, but
> + * let's leave this to the host driver for more flexibility.
> + */
> + return host->ops->init_sd_express(host, &host->ios);
> + }
> +
> return 0;
> }
>
> diff --git a/drivers/mmc/core/sd_ops.h b/drivers/mmc/core/sd_ops.h
> index 2194cabfcfc5..3ba7b3cf4652 100644
> --- a/drivers/mmc/core/sd_ops.h
> +++ b/drivers/mmc/core/sd_ops.h
> @@ -16,6 +16,7 @@ struct mmc_host;
> int mmc_app_set_bus_width(struct mmc_card *card, int width);
> int mmc_send_app_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr);
> int mmc_send_if_cond(struct mmc_host *host, u32 ocr);
> +int mmc_send_if_cond_pcie(struct mmc_host *host, u32 ocr);
> int mmc_send_relative_addr(struct mmc_host *host, unsigned int *rca);
> int mmc_app_send_scr(struct mmc_card *card);
> int mmc_sd_switch(struct mmc_card *card, int mode, int group,
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index c5b6e97cb21a..905cddc5e6f3 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -60,6 +60,8 @@ struct mmc_ios {
> #define MMC_TIMING_MMC_DDR52 8
> #define MMC_TIMING_MMC_HS200 9
> #define MMC_TIMING_MMC_HS400 10
> +#define MMC_TIMING_SD_EXP 11
> +#define MMC_TIMING_SD_EXP_1_2V 12
>
> unsigned char signal_voltage; /* signalling voltage (1.8V or 3.3V) */
>
> @@ -172,6 +174,9 @@ struct mmc_host_ops {
> */
> int (*multi_io_quirk)(struct mmc_card *card,
> unsigned int direction, int blk_size);
> +
> + /* Initialize an SD express card, mandatory for MMC_CAP2_SD_EXP. */
> + int (*init_sd_express)(struct mmc_host *host, struct mmc_ios *ios);
> };
>
> struct mmc_cqe_ops {
> @@ -357,6 +362,8 @@ struct mmc_host {
> #define MMC_CAP2_HS200_1_2V_SDR (1 << 6) /* can support */
> #define MMC_CAP2_HS200 (MMC_CAP2_HS200_1_8V_SDR | \
> MMC_CAP2_HS200_1_2V_SDR)
> +#define MMC_CAP2_SD_EXP (1 << 7) /* SD express via PCIe */
> +#define MMC_CAP2_SD_EXP_1_2V (1 << 8) /* SD express 1.2V */
> #define MMC_CAP2_CD_ACTIVE_HIGH (1 << 10) /* Card-detect signal active high */
> #define MMC_CAP2_RO_ACTIVE_HIGH (1 << 11) /* Write-protect signal active high */
> #define MMC_CAP2_NO_PRESCAN_POWERUP (1 << 14) /* Don't power up before scan */
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread* 答复: [PATCH] mmc: core: Initial support for SD express card/host
2020-08-21 12:40 ` Ulf Hansson
@ 2020-08-24 1:04 ` 冯锐
2020-08-24 6:33 ` Ulf Hansson
0 siblings, 1 reply; 11+ messages in thread
From: 冯锐 @ 2020-08-24 1:04 UTC (permalink / raw)
To: Ulf Hansson
Cc: Christoph Hellwig, Arnd Bergmann, linux-nvme, linux-mmc,
Linux PCI, Linux Kernel Mailing List, Adrian Hunter,
Greg Kroah-Hartman
Hi Hansson:
If this patch will not be changed, I will post a patch for rtsx driver according your patch.
>
> Rui,
>
> On Thu, 16 Jul 2020 at 16:16, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > In the SD specification v7.10 the SD express card has been added. This
> > new type of removable SD card, can be managed via a PCIe/NVMe based
> > interface, while also allowing backwards compatibility towards the
> > legacy SD interface.
> >
> > To keep the backwards compatibility, it's required to start the
> > initialization through the legacy SD interface. If it turns out that
> > the mmc host and the SD card, both supports the PCIe/NVMe interface,
> > then a switch should be allowed.
> >
> > Therefore, let's introduce some basic support for this type of SD
> > cards to the mmc core. The mmc host, should set MMC_CAP2_SD_EXP if it
> > supports this interface and MMC_CAP2_SD_EXP_1_2V, if also 1.2V is
> > supported, as to inform the core about it.
> >
> > To deal with the switch to the PCIe/NVMe interface, the mmc host is
> > required to implement a new host ops, ->init_sd_express(). Based on
> > the initial communication between the host and the card,
> > host->ios.timing is set to either MMC_TIMING_SD_EXP or
> > MMC_TIMING_SD_EXP_1_2V, depending on if 1.2V is supported or not. In
> > this way, the mmc host can check these values in its ->init_sd_express() ops,
> to know how to proceed with the handover.
> >
> > Note that, to manage card insert/removal, the mmc core sticks with
> > using the ->get_cd() callback, which means it's the host's
> > responsibility to make sure it provides valid data, even if the card
> > may be managed by PCIe/NVMe at the moment. As long as the card seems
> > to be present, the mmc core keeps the card powered on.
> >
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Rui Feng <rui_feng@realsil.com.cn>
> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>
> Rui, did you manage to get some time to look at $subject patch?
>
> If you need some help to understand what's needed to implement the
> corresponding support in drivers/mmc/host/rtsx_pci_sdmmc.c, then please
> just ask.
>
> I think it would make sense to queue changes for rtsx_pci at the same point as
> the mmc core changes. That's because I don't want to maintain code in the
> mmc core that's left unused.
>
> Kind regards
> Uffe
>
> > ---
> > drivers/mmc/core/core.c | 15 ++++++++++--
> > drivers/mmc/core/host.h | 6 +++++
> > drivers/mmc/core/sd_ops.c | 49
> > +++++++++++++++++++++++++++++++++++++--
> > drivers/mmc/core/sd_ops.h | 1 +
> > include/linux/mmc/host.h | 7 ++++++
> > 5 files changed, 74 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index
> > 8ccae6452b9c..6673c0f33cc7 100644
> > --- a/drivers/mmc/core/core.c
> > +++ b/drivers/mmc/core/core.c
> > @@ -2137,8 +2137,12 @@ static int mmc_rescan_try_freq(struct
> mmc_host
> > *host, unsigned freq)
> >
> > mmc_go_idle(host);
> >
> > - if (!(host->caps2 & MMC_CAP2_NO_SD))
> > - mmc_send_if_cond(host, host->ocr_avail);
> > + if (!(host->caps2 & MMC_CAP2_NO_SD)) {
> > + if (mmc_send_if_cond_pcie(host, host->ocr_avail))
> > + goto out;
> > + if (mmc_card_sd_express(host))
> > + return 0;
> > + }
> >
> > /* Order's important: probe SDIO, then SD, then MMC */
> > if (!(host->caps2 & MMC_CAP2_NO_SDIO)) @@ -2153,6 +2157,7
> @@
> > static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq)
> > if (!mmc_attach_mmc(host))
> > return 0;
> >
> > +out:
> > mmc_power_off(host);
> > return -EIO;
> > }
> > @@ -2280,6 +2285,12 @@ void mmc_rescan(struct work_struct *work)
> > goto out;
> > }
> >
> > + /* If an SD express card is present, then leave it as is. */
> > + if (mmc_card_sd_express(host)) {
> > + mmc_release_host(host);
> > + goto out;
> > + }
> > +
> > for (i = 0; i < ARRAY_SIZE(freqs); i++) {
> > unsigned int freq = freqs[i];
> > if (freq > host->f_max) { diff --git
> > a/drivers/mmc/core/host.h b/drivers/mmc/core/host.h index
> > 5e3b9534ffb2..ba407617ed23 100644
> > --- a/drivers/mmc/core/host.h
> > +++ b/drivers/mmc/core/host.h
> > @@ -77,5 +77,11 @@ static inline bool mmc_card_hs400es(struct
> mmc_card *card)
> > return card->host->ios.enhanced_strobe; }
> >
> > +static inline bool mmc_card_sd_express(struct mmc_host *host) {
> > + return host->ios.timing == MMC_TIMING_SD_EXP ||
> > + host->ios.timing == MMC_TIMING_SD_EXP_1_2V; }
> > +
> > #endif
> >
> > diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c
> > index 22bf528294b9..d61ff811218c 100644
> > --- a/drivers/mmc/core/sd_ops.c
> > +++ b/drivers/mmc/core/sd_ops.c
> > @@ -158,7 +158,8 @@ int mmc_send_app_op_cond(struct mmc_host *host,
> u32 ocr, u32 *rocr)
> > return err;
> > }
> >
> > -int mmc_send_if_cond(struct mmc_host *host, u32 ocr)
> > +static int __mmc_send_if_cond(struct mmc_host *host, u32 ocr, u8
> pcie_bits,
> > + u32 *resp)
> > {
> > struct mmc_command cmd = {};
> > int err;
> > @@ -171,7 +172,7 @@ int mmc_send_if_cond(struct mmc_host *host, u32
> ocr)
> > * SD 1.0 cards.
> > */
> > cmd.opcode = SD_SEND_IF_COND;
> > - cmd.arg = ((ocr & 0xFF8000) != 0) << 8 | test_pattern;
> > + cmd.arg = ((ocr & 0xFF8000) != 0) << 8 | pcie_bits << 8 |
> > + test_pattern;
> > cmd.flags = MMC_RSP_SPI_R7 | MMC_RSP_R7 |
> MMC_CMD_BCR;
> >
> > err = mmc_wait_for_cmd(host, &cmd, 0); @@ -186,6 +187,50
> @@
> > int mmc_send_if_cond(struct mmc_host *host, u32 ocr)
> > if (result_pattern != test_pattern)
> > return -EIO;
> >
> > + if (resp)
> > + *resp = cmd.resp[0];
> > +
> > + return 0;
> > +}
> > +
> > +int mmc_send_if_cond(struct mmc_host *host, u32 ocr) {
> > + return __mmc_send_if_cond(host, ocr, 0, NULL); }
> > +
> > +int mmc_send_if_cond_pcie(struct mmc_host *host, u32 ocr) {
> > + u32 resp = 0;
> > + u8 pcie_bits = 0;
> > + int ret;
> > +
> > + if (host->caps2 & MMC_CAP2_SD_EXP) {
> > + /* Probe card for SD express support via PCIe. */
> > + pcie_bits = 0x10;
> > + if (host->caps2 & MMC_CAP2_SD_EXP_1_2V)
> > + /* Probe also for 1.2V support. */
> > + pcie_bits = 0x30;
> > + }
> > +
> > + ret = __mmc_send_if_cond(host, ocr, pcie_bits, &resp);
> > + if (ret)
> > + return 0;
> > +
> > + /* Continue with the SD express init, if the card supports it. */
> > + resp &= 0x3000;
> > + if (pcie_bits && resp) {
> > + if (resp == 0x3000)
> > + host->ios.timing =
> MMC_TIMING_SD_EXP_1_2V;
> > + else
> > + host->ios.timing = MMC_TIMING_SD_EXP;
> > +
> > + /*
> > + * According to the spec the clock shall also be gated, but
> > + * let's leave this to the host driver for more flexibility.
> > + */
> > + return host->ops->init_sd_express(host, &host->ios);
> > + }
> > +
> > return 0;
> > }
> >
> > diff --git a/drivers/mmc/core/sd_ops.h b/drivers/mmc/core/sd_ops.h
> > index 2194cabfcfc5..3ba7b3cf4652 100644
> > --- a/drivers/mmc/core/sd_ops.h
> > +++ b/drivers/mmc/core/sd_ops.h
> > @@ -16,6 +16,7 @@ struct mmc_host;
> > int mmc_app_set_bus_width(struct mmc_card *card, int width); int
> > mmc_send_app_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr); int
> > mmc_send_if_cond(struct mmc_host *host, u32 ocr);
> > +int mmc_send_if_cond_pcie(struct mmc_host *host, u32 ocr);
> > int mmc_send_relative_addr(struct mmc_host *host, unsigned int *rca);
> > int mmc_app_send_scr(struct mmc_card *card); int mmc_sd_switch(struct
> > mmc_card *card, int mode, int group, diff --git
> > a/include/linux/mmc/host.h b/include/linux/mmc/host.h index
> > c5b6e97cb21a..905cddc5e6f3 100644
> > --- a/include/linux/mmc/host.h
> > +++ b/include/linux/mmc/host.h
> > @@ -60,6 +60,8 @@ struct mmc_ios {
> > #define MMC_TIMING_MMC_DDR52 8
> > #define MMC_TIMING_MMC_HS200 9
> > #define MMC_TIMING_MMC_HS400 10
> > +#define MMC_TIMING_SD_EXP 11
> > +#define MMC_TIMING_SD_EXP_1_2V 12
> >
> > unsigned char signal_voltage; /* signalling voltage
> (1.8V or 3.3V) */
> >
> > @@ -172,6 +174,9 @@ struct mmc_host_ops {
> > */
> > int (*multi_io_quirk)(struct mmc_card *card,
> > unsigned int direction, int
> > blk_size);
> > +
> > + /* Initialize an SD express card, mandatory for MMC_CAP2_SD_EXP.
> */
> > + int (*init_sd_express)(struct mmc_host *host, struct
> mmc_ios *ios);
> > };
> >
> > struct mmc_cqe_ops {
> > @@ -357,6 +362,8 @@ struct mmc_host {
> > #define MMC_CAP2_HS200_1_2V_SDR (1 << 6) /* can
> support */
> > #define MMC_CAP2_HS200 (MMC_CAP2_HS200_1_8V_SDR | \
> > MMC_CAP2_HS200_1_2V_SDR)
> > +#define MMC_CAP2_SD_EXP (1 << 7) /* SD
> express via PCIe */
> > +#define MMC_CAP2_SD_EXP_1_2V (1 << 8) /* SD express 1.2V
> */
> > #define MMC_CAP2_CD_ACTIVE_HIGH (1 << 10) /*
> Card-detect signal active high */
> > #define MMC_CAP2_RO_ACTIVE_HIGH (1 << 11) /*
> Write-protect signal active high */
> > #define MMC_CAP2_NO_PRESCAN_POWERUP (1 << 14) /* Don't power
> up
> > before scan */
> > --
> > 2.20.1
> >
>
> ------Please consider the environment before printing this e-mail.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] mmc: core: Initial support for SD express card/host
2020-08-24 1:04 ` 答复: " 冯锐
@ 2020-08-24 6:33 ` Ulf Hansson
0 siblings, 0 replies; 11+ messages in thread
From: Ulf Hansson @ 2020-08-24 6:33 UTC (permalink / raw)
To: 冯锐
Cc: Christoph Hellwig, Arnd Bergmann, linux-nvme, linux-mmc,
Linux PCI, Linux Kernel Mailing List, Adrian Hunter,
Greg Kroah-Hartman
On Mon, 24 Aug 2020 at 03:04, 冯锐 <rui_feng@realsil.com.cn> wrote:
>
> Hi Hansson:
>
> If this patch will not be changed, I will post a patch for rtsx driver according your patch.
I don't think there is any change needed, unless you think so.
Kind regards
Uffe
>
> >
> > Rui,
> >
> > On Thu, 16 Jul 2020 at 16:16, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > >
> > > In the SD specification v7.10 the SD express card has been added. This
> > > new type of removable SD card, can be managed via a PCIe/NVMe based
> > > interface, while also allowing backwards compatibility towards the
> > > legacy SD interface.
> > >
> > > To keep the backwards compatibility, it's required to start the
> > > initialization through the legacy SD interface. If it turns out that
> > > the mmc host and the SD card, both supports the PCIe/NVMe interface,
> > > then a switch should be allowed.
> > >
> > > Therefore, let's introduce some basic support for this type of SD
> > > cards to the mmc core. The mmc host, should set MMC_CAP2_SD_EXP if it
> > > supports this interface and MMC_CAP2_SD_EXP_1_2V, if also 1.2V is
> > > supported, as to inform the core about it.
> > >
> > > To deal with the switch to the PCIe/NVMe interface, the mmc host is
> > > required to implement a new host ops, ->init_sd_express(). Based on
> > > the initial communication between the host and the card,
> > > host->ios.timing is set to either MMC_TIMING_SD_EXP or
> > > MMC_TIMING_SD_EXP_1_2V, depending on if 1.2V is supported or not. In
> > > this way, the mmc host can check these values in its ->init_sd_express() ops,
> > to know how to proceed with the handover.
> > >
> > > Note that, to manage card insert/removal, the mmc core sticks with
> > > using the ->get_cd() callback, which means it's the host's
> > > responsibility to make sure it provides valid data, even if the card
> > > may be managed by PCIe/NVMe at the moment. As long as the card seems
> > > to be present, the mmc core keeps the card powered on.
> > >
> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Cc: Arnd Bergmann <arnd@arndb.de>
> > > Cc: Christoph Hellwig <hch@lst.de>
> > > Cc: Rui Feng <rui_feng@realsil.com.cn>
> > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> >
> > Rui, did you manage to get some time to look at $subject patch?
> >
> > If you need some help to understand what's needed to implement the
> > corresponding support in drivers/mmc/host/rtsx_pci_sdmmc.c, then please
> > just ask.
> >
> > I think it would make sense to queue changes for rtsx_pci at the same point as
> > the mmc core changes. That's because I don't want to maintain code in the
> > mmc core that's left unused.
> >
> > Kind regards
> > Uffe
> >
> > > ---
> > > drivers/mmc/core/core.c | 15 ++++++++++--
> > > drivers/mmc/core/host.h | 6 +++++
> > > drivers/mmc/core/sd_ops.c | 49
> > > +++++++++++++++++++++++++++++++++++++--
> > > drivers/mmc/core/sd_ops.h | 1 +
> > > include/linux/mmc/host.h | 7 ++++++
> > > 5 files changed, 74 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index
> > > 8ccae6452b9c..6673c0f33cc7 100644
> > > --- a/drivers/mmc/core/core.c
> > > +++ b/drivers/mmc/core/core.c
> > > @@ -2137,8 +2137,12 @@ static int mmc_rescan_try_freq(struct
> > mmc_host
> > > *host, unsigned freq)
> > >
> > > mmc_go_idle(host);
> > >
> > > - if (!(host->caps2 & MMC_CAP2_NO_SD))
> > > - mmc_send_if_cond(host, host->ocr_avail);
> > > + if (!(host->caps2 & MMC_CAP2_NO_SD)) {
> > > + if (mmc_send_if_cond_pcie(host, host->ocr_avail))
> > > + goto out;
> > > + if (mmc_card_sd_express(host))
> > > + return 0;
> > > + }
> > >
> > > /* Order's important: probe SDIO, then SD, then MMC */
> > > if (!(host->caps2 & MMC_CAP2_NO_SDIO)) @@ -2153,6 +2157,7
> > @@
> > > static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq)
> > > if (!mmc_attach_mmc(host))
> > > return 0;
> > >
> > > +out:
> > > mmc_power_off(host);
> > > return -EIO;
> > > }
> > > @@ -2280,6 +2285,12 @@ void mmc_rescan(struct work_struct *work)
> > > goto out;
> > > }
> > >
> > > + /* If an SD express card is present, then leave it as is. */
> > > + if (mmc_card_sd_express(host)) {
> > > + mmc_release_host(host);
> > > + goto out;
> > > + }
> > > +
> > > for (i = 0; i < ARRAY_SIZE(freqs); i++) {
> > > unsigned int freq = freqs[i];
> > > if (freq > host->f_max) { diff --git
> > > a/drivers/mmc/core/host.h b/drivers/mmc/core/host.h index
> > > 5e3b9534ffb2..ba407617ed23 100644
> > > --- a/drivers/mmc/core/host.h
> > > +++ b/drivers/mmc/core/host.h
> > > @@ -77,5 +77,11 @@ static inline bool mmc_card_hs400es(struct
> > mmc_card *card)
> > > return card->host->ios.enhanced_strobe; }
> > >
> > > +static inline bool mmc_card_sd_express(struct mmc_host *host) {
> > > + return host->ios.timing == MMC_TIMING_SD_EXP ||
> > > + host->ios.timing == MMC_TIMING_SD_EXP_1_2V; }
> > > +
> > > #endif
> > >
> > > diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c
> > > index 22bf528294b9..d61ff811218c 100644
> > > --- a/drivers/mmc/core/sd_ops.c
> > > +++ b/drivers/mmc/core/sd_ops.c
> > > @@ -158,7 +158,8 @@ int mmc_send_app_op_cond(struct mmc_host *host,
> > u32 ocr, u32 *rocr)
> > > return err;
> > > }
> > >
> > > -int mmc_send_if_cond(struct mmc_host *host, u32 ocr)
> > > +static int __mmc_send_if_cond(struct mmc_host *host, u32 ocr, u8
> > pcie_bits,
> > > + u32 *resp)
> > > {
> > > struct mmc_command cmd = {};
> > > int err;
> > > @@ -171,7 +172,7 @@ int mmc_send_if_cond(struct mmc_host *host, u32
> > ocr)
> > > * SD 1.0 cards.
> > > */
> > > cmd.opcode = SD_SEND_IF_COND;
> > > - cmd.arg = ((ocr & 0xFF8000) != 0) << 8 | test_pattern;
> > > + cmd.arg = ((ocr & 0xFF8000) != 0) << 8 | pcie_bits << 8 |
> > > + test_pattern;
> > > cmd.flags = MMC_RSP_SPI_R7 | MMC_RSP_R7 |
> > MMC_CMD_BCR;
> > >
> > > err = mmc_wait_for_cmd(host, &cmd, 0); @@ -186,6 +187,50
> > @@
> > > int mmc_send_if_cond(struct mmc_host *host, u32 ocr)
> > > if (result_pattern != test_pattern)
> > > return -EIO;
> > >
> > > + if (resp)
> > > + *resp = cmd.resp[0];
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +int mmc_send_if_cond(struct mmc_host *host, u32 ocr) {
> > > + return __mmc_send_if_cond(host, ocr, 0, NULL); }
> > > +
> > > +int mmc_send_if_cond_pcie(struct mmc_host *host, u32 ocr) {
> > > + u32 resp = 0;
> > > + u8 pcie_bits = 0;
> > > + int ret;
> > > +
> > > + if (host->caps2 & MMC_CAP2_SD_EXP) {
> > > + /* Probe card for SD express support via PCIe. */
> > > + pcie_bits = 0x10;
> > > + if (host->caps2 & MMC_CAP2_SD_EXP_1_2V)
> > > + /* Probe also for 1.2V support. */
> > > + pcie_bits = 0x30;
> > > + }
> > > +
> > > + ret = __mmc_send_if_cond(host, ocr, pcie_bits, &resp);
> > > + if (ret)
> > > + return 0;
> > > +
> > > + /* Continue with the SD express init, if the card supports it. */
> > > + resp &= 0x3000;
> > > + if (pcie_bits && resp) {
> > > + if (resp == 0x3000)
> > > + host->ios.timing =
> > MMC_TIMING_SD_EXP_1_2V;
> > > + else
> > > + host->ios.timing = MMC_TIMING_SD_EXP;
> > > +
> > > + /*
> > > + * According to the spec the clock shall also be gated, but
> > > + * let's leave this to the host driver for more flexibility.
> > > + */
> > > + return host->ops->init_sd_express(host, &host->ios);
> > > + }
> > > +
> > > return 0;
> > > }
> > >
> > > diff --git a/drivers/mmc/core/sd_ops.h b/drivers/mmc/core/sd_ops.h
> > > index 2194cabfcfc5..3ba7b3cf4652 100644
> > > --- a/drivers/mmc/core/sd_ops.h
> > > +++ b/drivers/mmc/core/sd_ops.h
> > > @@ -16,6 +16,7 @@ struct mmc_host;
> > > int mmc_app_set_bus_width(struct mmc_card *card, int width); int
> > > mmc_send_app_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr); int
> > > mmc_send_if_cond(struct mmc_host *host, u32 ocr);
> > > +int mmc_send_if_cond_pcie(struct mmc_host *host, u32 ocr);
> > > int mmc_send_relative_addr(struct mmc_host *host, unsigned int *rca);
> > > int mmc_app_send_scr(struct mmc_card *card); int mmc_sd_switch(struct
> > > mmc_card *card, int mode, int group, diff --git
> > > a/include/linux/mmc/host.h b/include/linux/mmc/host.h index
> > > c5b6e97cb21a..905cddc5e6f3 100644
> > > --- a/include/linux/mmc/host.h
> > > +++ b/include/linux/mmc/host.h
> > > @@ -60,6 +60,8 @@ struct mmc_ios {
> > > #define MMC_TIMING_MMC_DDR52 8
> > > #define MMC_TIMING_MMC_HS200 9
> > > #define MMC_TIMING_MMC_HS400 10
> > > +#define MMC_TIMING_SD_EXP 11
> > > +#define MMC_TIMING_SD_EXP_1_2V 12
> > >
> > > unsigned char signal_voltage; /* signalling voltage
> > (1.8V or 3.3V) */
> > >
> > > @@ -172,6 +174,9 @@ struct mmc_host_ops {
> > > */
> > > int (*multi_io_quirk)(struct mmc_card *card,
> > > unsigned int direction, int
> > > blk_size);
> > > +
> > > + /* Initialize an SD express card, mandatory for MMC_CAP2_SD_EXP.
> > */
> > > + int (*init_sd_express)(struct mmc_host *host, struct
> > mmc_ios *ios);
> > > };
> > >
> > > struct mmc_cqe_ops {
> > > @@ -357,6 +362,8 @@ struct mmc_host {
> > > #define MMC_CAP2_HS200_1_2V_SDR (1 << 6) /* can
> > support */
> > > #define MMC_CAP2_HS200 (MMC_CAP2_HS200_1_8V_SDR | \
> > > MMC_CAP2_HS200_1_2V_SDR)
> > > +#define MMC_CAP2_SD_EXP (1 << 7) /* SD
> > express via PCIe */
> > > +#define MMC_CAP2_SD_EXP_1_2V (1 << 8) /* SD express 1.2V
> > */
> > > #define MMC_CAP2_CD_ACTIVE_HIGH (1 << 10) /*
> > Card-detect signal active high */
> > > #define MMC_CAP2_RO_ACTIVE_HIGH (1 << 11) /*
> > Write-protect signal active high */
> > > #define MMC_CAP2_NO_PRESCAN_POWERUP (1 << 14) /* Don't power
> > up
> > > before scan */
> > > --
> > > 2.20.1
> > >
> >
> > ------Please consider the environment before printing this e-mail.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-08-24 6:34 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-16 14:15 [PATCH] mmc: core: Initial support for SD express card/host Ulf Hansson
2020-07-16 16:13 ` Greg Kroah-Hartman
2020-07-16 17:04 ` Ulf Hansson
2020-07-16 17:21 ` Greg Kroah-Hartman
2020-07-16 18:22 ` Arnd Bergmann
2020-07-24 10:06 ` Ulf Hansson
2020-07-24 13:35 ` Arnd Bergmann
2020-07-24 13:44 ` Christoph Hellwig
2020-08-21 12:40 ` Ulf Hansson
2020-08-24 1:04 ` 答复: " 冯锐
2020-08-24 6:33 ` Ulf Hansson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).