[PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[rui_feng]

From: Rui Feng <rui_feng@realsil.com.cn>

RTS5261 support legacy SD mode and SD Express mode.
In SD7.x, SD association introduce SD Express as a new mode.
SD Express mode is distinguished by CMD8.
Therefore, CMD8 has new bit for SD Express.
SD Express is based on PCIe/NVMe.
RTS5261 uses CMD8 to switch to SD Express mode.

Signed-off-by: Rui Feng <rui_feng@realsil.com.cn>
---
 drivers/misc/cardreader/Kconfig    |  8 ++++++
 drivers/misc/cardreader/Makefile   |  3 +++
 drivers/misc/cardreader/rts5261.c  |  7 +++++
 drivers/misc/cardreader/rtsx_pcr.c |  7 +++++
 drivers/mmc/core/sd_ops.c          |  9 ++++++-
 drivers/mmc/host/rtsx_pci_sdmmc.c  | 43 ++++++++++++++++++++++++++++++
 include/linux/mmc/host.h           |  1 +
 include/linux/rtsx_pci.h           | 15 +++++++++++
 8 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/cardreader/Kconfig b/drivers/misc/cardreader/Kconfig
index 022322dfb36e..1ce6bf1d121c 100644
--- a/drivers/misc/cardreader/Kconfig
+++ b/drivers/misc/cardreader/Kconfig
@@ -21,6 +21,14 @@ config MISC_RTSX_PCI
 	  such as Memory Stick, Memory Stick Pro, Secure Digital and
 	  MultiMediaCard.
 
+config MISC_RTSX_PCI_SD_EXPRESS
+        bool "Realtek PCI-E card reader support sd express card"
+        depends on MISC_RTSX_PCI
+        default y
+        help
+          RTS5261 support SD Express card.
+          Say Y here if you want to enable PCIe/NVMe mode.
+
 config MISC_RTSX_USB
 	tristate "Realtek USB card reader"
 	depends on USB
diff --git a/drivers/misc/cardreader/Makefile b/drivers/misc/cardreader/Makefile
index 1f56267ed2f4..589011999f78 100644
--- a/drivers/misc/cardreader/Makefile
+++ b/drivers/misc/cardreader/Makefile
@@ -2,4 +2,7 @@
 obj-$(CONFIG_MISC_ALCOR_PCI)	+= alcor_pci.o
 obj-$(CONFIG_MISC_RTSX_PCI)	+= rtsx_pci.o
 rtsx_pci-objs := rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o rts5260.o rts5261.o
+
+ccflags-$(CONFIG_MISC_RTSX_PCI_SD_EXPRESS) += -DMISC_RTSX_PCI_SD_EXPRESS
+
 obj-$(CONFIG_MISC_RTSX_USB)	+= rtsx_usb.o
diff --git a/drivers/misc/cardreader/rts5261.c b/drivers/misc/cardreader/rts5261.c
index 547db5ffd3f6..68cb0562c278 100644
--- a/drivers/misc/cardreader/rts5261.c
+++ b/drivers/misc/cardreader/rts5261.c
@@ -759,8 +759,14 @@ void rts5261_init_params(struct rtsx_pcr *pcr)
 {
 	struct rtsx_cr_option *option = &pcr->option;
 	struct rtsx_hw_param *hw_param = &pcr->hw_param;
+	u8 val;
 
 	pcr->extra_caps = EXTRA_CAPS_SD_SDR50 | EXTRA_CAPS_SD_SDR104;
+#ifdef MISC_RTSX_PCI_SD_EXPRESS
+	rtsx_pci_read_register(pcr, RTS5261_FW_STATUS, &val);
+	if (!(val & RTS5261_EXPRESS_LINK_FAIL_MASK))
+		pcr->extra_caps |= EXTRA_CAPS_SD_EXPRESS;
+#endif
 	pcr->num_slots = 1;
 	pcr->ops = &rts5261_pcr_ops;
 
@@ -791,6 +797,7 @@ void rts5261_init_params(struct rtsx_pcr *pcr)
 	option->ltr_l1off_snooze_sspwrgate = 0x78;
 	option->dev_aspm_mode = DEV_ASPM_DYNAMIC;
 
+	hw_param->interrupt_en |= DELINK_INT_EN;
 	option->ocp_en = 1;
 	hw_param->interrupt_en |= SD_OC_INT_EN;
 	hw_param->ocp_glitch =  SD_OCP_GLITCH_800U;
diff --git a/drivers/misc/cardreader/rtsx_pcr.c b/drivers/misc/cardreader/rtsx_pcr.c
index 06038b325b02..2127c7b6b2e1 100644
--- a/drivers/misc/cardreader/rtsx_pcr.c
+++ b/drivers/misc/cardreader/rtsx_pcr.c
@@ -1026,6 +1026,13 @@ static irqreturn_t rtsx_pci_isr(int irq, void *dev_id)
 		} else {
 			pcr->card_removed |= SD_EXIST;
 			pcr->card_inserted &= ~SD_EXIST;
+#ifdef MISC_RTSX_PCI_SD_EXPRESS
+			if (PCI_PID(pcr) == PID_5261) {
+				rtsx_pci_write_register(pcr, RTS5261_FW_STATUS,
+					RTS5261_EXPRESS_LINK_FAIL_MASK, 0);
+				pcr->extra_caps |= EXTRA_CAPS_SD_EXPRESS;
+			}
+#endif
 		}
 		pcr->dma_error_count = 0;
 	}
diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c
index 22bf528294b9..7c70d267644b 100644
--- a/drivers/mmc/core/sd_ops.c
+++ b/drivers/mmc/core/sd_ops.c
@@ -171,7 +171,14 @@ 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;
+	/*
+	 * Host asks card's PCIe availability
+	 * if PCIe interface is supported by host.
+	 */
+	if ((ocr & 0xFF8000) && (host->caps2 & MMC_CAP2_SD_EXPRESS))
+		cmd.arg = 0x31 << 8 | test_pattern;
+	else
+		cmd.arg = ((ocr & 0xFF8000) != 0) << 8 | test_pattern;
 	cmd.flags = MMC_RSP_SPI_R7 | MMC_RSP_R7 | MMC_CMD_BCR;
 
 	err = mmc_wait_for_cmd(host, &cmd, 0);
diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c b/drivers/mmc/host/rtsx_pci_sdmmc.c
index bd50935dc37d..87ad75b253eb 100644
--- a/drivers/mmc/host/rtsx_pci_sdmmc.c
+++ b/drivers/mmc/host/rtsx_pci_sdmmc.c
@@ -219,6 +219,7 @@ static void sd_send_cmd_get_rsp(struct realtek_pci_sdmmc *host,
 	int rsp_type;
 	int stat_idx;
 	bool clock_toggled = false;
+	u32 relink_time, val;
 
 	dev_dbg(sdmmc_dev(host), "%s: SD/MMC CMD %d, arg = 0x%08x\n",
 			__func__, cmd_idx, arg);
@@ -322,6 +323,44 @@ static void sd_send_cmd_get_rsp(struct realtek_pci_sdmmc *host,
 	if (err && clock_toggled)
 		rtsx_pci_write_register(pcr, SD_BUS_STAT,
 				SD_CLK_TOGGLE_EN | SD_CLK_FORCE_STOP, 0);
+
+	/*
+	 * If card has PCIe availability and WP if off,
+	 * reader switch to PCIe mode.
+	 */
+	val = rtsx_pci_readl(pcr, RTSX_BIPR);
+	if (cmd->opcode == 8 && ((cmd->resp[0] >> 8) & 0x10)
+		&& !(val & SD_WRITE_PROTECT)) {
+		/* Set relink_time for changing to PCIe card */
+		relink_time = 0x8FFF;
+
+		rtsx_pci_write_register(pcr, 0xFF01, 0xFF, relink_time);
+		rtsx_pci_write_register(pcr, 0xFF02, 0xFF, relink_time >> 8);
+		rtsx_pci_write_register(pcr, 0xFF03, 0x01, relink_time >> 16);
+
+		rtsx_pci_write_register(pcr, PETXCFG, 0x80, 0x80);
+		rtsx_pci_write_register(pcr, LDO_VCC_CFG0,
+			RTS5261_LDO1_OCP_THD_MASK,
+			pcr->option.sd_800mA_ocp_thd);
+
+		if (pcr->ops->disable_auto_blink)
+			pcr->ops->disable_auto_blink(pcr);
+
+		/* For PCIe/NVMe mode can't enter delink issue */
+		pcr->hw_param.interrupt_en &= ~(SD_INT_EN);
+		rtsx_pci_writel(pcr, RTSX_BIER, pcr->hw_param.interrupt_en);
+
+		rtsx_pci_write_register(pcr, RTS5260_AUTOLOAD_CFG4,
+			RTS5261_AUX_CLK_16M_EN, RTS5261_AUX_CLK_16M_EN);
+		rtsx_pci_write_register(pcr, RTS5261_FW_CFG0,
+			RTS5261_FW_ENTER_EXPRESS, RTS5261_FW_ENTER_EXPRESS);
+		rtsx_pci_write_register(pcr, RTS5261_FW_CFG1,
+			RTS5261_MCU_BUS_SEL_MASK | RTS5261_MCU_CLOCK_SEL_MASK
+			| RTS5261_MCU_CLOCK_GATING | RTS5261_DRIVER_ENABLE_FW,
+			RTS5261_MCU_CLOCK_SEL_16M | RTS5261_MCU_CLOCK_GATING
+			| RTS5261_DRIVER_ENABLE_FW);
+		cmd->error = -EPERM;
+	}
 }
 
 static int sd_read_data(struct realtek_pci_sdmmc *host, struct mmc_command *cmd,
@@ -1123,6 +1162,8 @@ static int sdmmc_get_cd(struct mmc_host *mmc)
 	dev_dbg(sdmmc_dev(host), "%s: RTSX_BIPR = 0x%08x\n", __func__, val);
 	if (val & SD_EXIST)
 		cd = 1;
+	if (pcr->extra_caps & EXTRA_CAPS_SD_EXPRESS)
+		mmc->caps2 |= MMC_CAP2_SD_EXPRESS;
 
 	mutex_unlock(&pcr->pcr_mutex);
 
@@ -1333,6 +1374,8 @@ static void init_extra_caps(struct realtek_pci_sdmmc *host)
 		mmc->caps |= MMC_CAP_1_8V_DDR;
 	if (pcr->extra_caps & EXTRA_CAPS_MMC_8BIT)
 		mmc->caps |= MMC_CAP_8_BIT_DATA;
+	if (pcr->extra_caps & EXTRA_CAPS_SD_EXPRESS)
+		mmc->caps2 |= MMC_CAP2_SD_EXPRESS;
 }
 
 static void realtek_init_host(struct realtek_pci_sdmmc *host)
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index ba703384bea0..ac1e3da4ad4f 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -369,6 +369,7 @@ struct mmc_host {
 #define MMC_CAP2_CQE_DCMD	(1 << 24)	/* CQE can issue a direct command */
 #define MMC_CAP2_AVOID_3_3V	(1 << 25)	/* Host must negotiate down from 3.3V */
 #define MMC_CAP2_MERGE_CAPABLE	(1 << 26)	/* Host can merge a segment over the segment size */
+#define MMC_CAP2_SD_EXPRESS	(1 << 27)	/* Host support sd express card */
 
 	int			fixed_drv_type;	/* fixed driver type for non-removable media */
 
diff --git a/include/linux/rtsx_pci.h b/include/linux/rtsx_pci.h
index 65b8142a7fed..2783dc5202ea 100644
--- a/include/linux/rtsx_pci.h
+++ b/include/linux/rtsx_pci.h
@@ -667,6 +667,16 @@
 #define   PM_WAKE_EN			0x01
 #define PM_CTRL4			0xFF47
 
+#define RTS5261_FW_CFG0			0xFF54
+#define RTS5261_FW_ENTER_EXPRESS	(0x01<<0)
+
+#define RTS5261_FW_CFG1			0xFF55
+#define RTS5261_MCU_BUS_SEL_MASK	(0x01<<4)
+#define RTS5261_MCU_CLOCK_SEL_MASK	(0x03<<2)
+#define RTS5261_MCU_CLOCK_SEL_16M	(0x01<<2)
+#define RTS5261_MCU_CLOCK_GATING	(0x01<<1)
+#define RTS5261_DRIVER_ENABLE_FW	(0x01<<0)
+
 /* Memory mapping */
 #define SRAM_BASE			0xE600
 #define RBUF_BASE			0xF400
@@ -704,6 +714,8 @@
 /*RTS5260*/
 #define   RTS5260_DVCC_TUNE_MASK	0x70
 #define   RTS5260_DVCC_33		0x70
+/*RTS5261*/
+#define   RTS5261_LDO1_OCP_THD_MASK	(0x07<<5)
 
 #define LDO_VCC_CFG1			0xFF73
 #define   LDO_VCC_REF_TUNE_MASK		0x30
@@ -745,6 +757,8 @@
 
 #define RTS5260_AUTOLOAD_CFG4		0xFF7F
 #define   RTS5260_MIMO_DISABLE		0x8A
+/*RTS5261*/
+#define   RTS5261_AUX_CLK_16M_EN		(1 << 5)
 
 #define RTS5260_REG_GPIO_CTL0		0xFC1A
 #define   RTS5260_REG_GPIO_MASK		0x01
@@ -1217,6 +1231,7 @@ struct rtsx_pcr {
 #define EXTRA_CAPS_MMC_HSDDR		(1 << 3)
 #define EXTRA_CAPS_MMC_HS200		(1 << 4)
 #define EXTRA_CAPS_MMC_8BIT		(1 << 5)
+#define EXTRA_CAPS_SD_EXPRESS		(1 << 6)
 	u32				extra_caps;
 
 #define IC_VER_A			0
-- 
2.17.1

Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[Christoph Hellwig]

On Sun, Apr 26, 2020 at 09:25:46AM +0800, rui_feng@realsil.com.cn wrote:
> From: Rui Feng <rui_feng@realsil.com.cn>
> 
> RTS5261 support legacy SD mode and SD Express mode.
> In SD7.x, SD association introduce SD Express as a new mode.
> SD Express mode is distinguished by CMD8.
> Therefore, CMD8 has new bit for SD Express.
> SD Express is based on PCIe/NVMe.
> RTS5261 uses CMD8 to switch to SD Express mode.

So how does this bit work?  They way I imagined SD Express to work
is that the actual SD Card just shows up as a real PCIe device, similar
to say Thunderbolt.

答复: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[冯锐]

> On Sun, Apr 26, 2020 at 09:25:46AM +0800, rui_feng@realsil.com.cn wrote:
> > From: Rui Feng <rui_feng@realsil.com.cn>
> >
> > RTS5261 support legacy SD mode and SD Express mode.
> > In SD7.x, SD association introduce SD Express as a new mode.
> > SD Express mode is distinguished by CMD8.
> > Therefore, CMD8 has new bit for SD Express.
> > SD Express is based on PCIe/NVMe.
> > RTS5261 uses CMD8 to switch to SD Express mode.
> 
> So how does this bit work?  They way I imagined SD Express to work is that
> the actual SD Card just shows up as a real PCIe device, similar to say
> Thunderbolt.

New SD Express card has dual mode. One is SD mode and another is PCIe mode.
In PCIe mode, it act as a PCIe device and use PCIe protocol not Thunderbolt protocol.

Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[Arnd Bergmann]

On Mon, Apr 27, 2020 at 11:41 AM 冯锐 <rui_feng@realsil.com.cn> wrote:
>
>
> > On Sun, Apr 26, 2020 at 09:25:46AM +0800, rui_feng@realsil.com.cn wrote:
> > > From: Rui Feng <rui_feng@realsil.com.cn>
> > >
> > > RTS5261 support legacy SD mode and SD Express mode.
> > > In SD7.x, SD association introduce SD Express as a new mode.
> > > SD Express mode is distinguished by CMD8.
> > > Therefore, CMD8 has new bit for SD Express.
> > > SD Express is based on PCIe/NVMe.
> > > RTS5261 uses CMD8 to switch to SD Express mode.
> >
> > So how does this bit work?  They way I imagined SD Express to work is that
> > the actual SD Card just shows up as a real PCIe device, similar to say
> > Thunderbolt.
>
> New SD Express card has dual mode. One is SD mode and another is PCIe mode.
> In PCIe mode, it act as a PCIe device and use PCIe protocol not Thunderbolt protocol.

I think what Christoph was asking about is why you need to issue any commands at
all in SD mode when you want to use PCIe mode instead. What happens if you load
the NVMe driver before loading the rts5261 driver?

       Arnd

答复: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[冯锐]

> 
> On Mon, Apr 27, 2020 at 11:41 AM 冯锐 <rui_feng@realsil.com.cn> wrote:
> >
> >
> > > On Sun, Apr 26, 2020 at 09:25:46AM +0800, rui_feng@realsil.com.cn
> wrote:
> > > > From: Rui Feng <rui_feng@realsil.com.cn>
> > > >
> > > > RTS5261 support legacy SD mode and SD Express mode.
> > > > In SD7.x, SD association introduce SD Express as a new mode.
> > > > SD Express mode is distinguished by CMD8.
> > > > Therefore, CMD8 has new bit for SD Express.
> > > > SD Express is based on PCIe/NVMe.
> > > > RTS5261 uses CMD8 to switch to SD Express mode.
> > >
> > > So how does this bit work?  They way I imagined SD Express to work
> > > is that the actual SD Card just shows up as a real PCIe device,
> > > similar to say Thunderbolt.
> >
> > New SD Express card has dual mode. One is SD mode and another is PCIe
> mode.
> > In PCIe mode, it act as a PCIe device and use PCIe protocol not Thunderbolt
> protocol.
> 
> I think what Christoph was asking about is why you need to issue any
> commands at all in SD mode when you want to use PCIe mode instead. What
> happens if you load the NVMe dthriver before loading the rts5261 driver?
> 
>        Arnd
> 
> ------Please consider the environment before printing this e-mail.

RTS5261 support SD mode and PCIe/NVMe mode. The workflow is as follows.
1.RTS5261 work in SD mode.
2.If card is plugged in, Host send CMD8 to ask card's PCIe availability.
3.If the card has PCIe availability, RTS5261 switch to PCIe/NVMe mode.
4.Mmc driver exit and NVMe driver start working.
5.If card is unplugged, RTS5261 will switch to SD mode.
We should send CMD8 in SD mode to ask card's PCIe availability, and the order of NVMe driver and rts5261 driver doesn't matter.

Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[Greg KH]

On Sun, Apr 26, 2020 at 09:25:46AM +0800, rui_feng@realsil.com.cn wrote:
> From: Rui Feng <rui_feng@realsil.com.cn>
> 
> RTS5261 support legacy SD mode and SD Express mode.
> In SD7.x, SD association introduce SD Express as a new mode.
> SD Express mode is distinguished by CMD8.
> Therefore, CMD8 has new bit for SD Express.
> SD Express is based on PCIe/NVMe.
> RTS5261 uses CMD8 to switch to SD Express mode.
> 
> Signed-off-by: Rui Feng <rui_feng@realsil.com.cn>
> ---
>  drivers/misc/cardreader/Kconfig    |  8 ++++++
>  drivers/misc/cardreader/Makefile   |  3 +++
>  drivers/misc/cardreader/rts5261.c  |  7 +++++
>  drivers/misc/cardreader/rtsx_pcr.c |  7 +++++
>  drivers/mmc/core/sd_ops.c          |  9 ++++++-
>  drivers/mmc/host/rtsx_pci_sdmmc.c  | 43 ++++++++++++++++++++++++++++++
>  include/linux/mmc/host.h           |  1 +
>  include/linux/rtsx_pci.h           | 15 +++++++++++
>  8 files changed, 92 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/cardreader/Kconfig b/drivers/misc/cardreader/Kconfig
> index 022322dfb36e..1ce6bf1d121c 100644
> --- a/drivers/misc/cardreader/Kconfig
> +++ b/drivers/misc/cardreader/Kconfig
> @@ -21,6 +21,14 @@ config MISC_RTSX_PCI
>  	  such as Memory Stick, Memory Stick Pro, Secure Digital and
>  	  MultiMediaCard.
>  
> +config MISC_RTSX_PCI_SD_EXPRESS
> +        bool "Realtek PCI-E card reader support sd express card"
> +        depends on MISC_RTSX_PCI
> +        default y

"default y" is only if the machine will no longer boot if you do not
accept this option.  That's not the case for a driver like this, sorry.

> +        help
> +          RTS5261 support SD Express card.
> +          Say Y here if you want to enable PCIe/NVMe mode.

No hint as to what the module name will be?

> +
>  config MISC_RTSX_USB
>  	tristate "Realtek USB card reader"
>  	depends on USB
> diff --git a/drivers/misc/cardreader/Makefile b/drivers/misc/cardreader/Makefile
> index 1f56267ed2f4..589011999f78 100644
> --- a/drivers/misc/cardreader/Makefile
> +++ b/drivers/misc/cardreader/Makefile
> @@ -2,4 +2,7 @@
>  obj-$(CONFIG_MISC_ALCOR_PCI)	+= alcor_pci.o
>  obj-$(CONFIG_MISC_RTSX_PCI)	+= rtsx_pci.o
>  rtsx_pci-objs := rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o rts5260.o rts5261.o
> +
> +ccflags-$(CONFIG_MISC_RTSX_PCI_SD_EXPRESS) += -DMISC_RTSX_PCI_SD_EXPRESS

Ick, really?  Why?  Why can't you just look at the config option when
you build instead?

> +
>  obj-$(CONFIG_MISC_RTSX_USB)	+= rtsx_usb.o
> diff --git a/drivers/misc/cardreader/rts5261.c b/drivers/misc/cardreader/rts5261.c
> index 547db5ffd3f6..68cb0562c278 100644
> --- a/drivers/misc/cardreader/rts5261.c
> +++ b/drivers/misc/cardreader/rts5261.c
> @@ -759,8 +759,14 @@ void rts5261_init_params(struct rtsx_pcr *pcr)
>  {
>  	struct rtsx_cr_option *option = &pcr->option;
>  	struct rtsx_hw_param *hw_param = &pcr->hw_param;
> +	u8 val;
>  
>  	pcr->extra_caps = EXTRA_CAPS_SD_SDR50 | EXTRA_CAPS_SD_SDR104;
> +#ifdef MISC_RTSX_PCI_SD_EXPRESS
> +	rtsx_pci_read_register(pcr, RTS5261_FW_STATUS, &val);
> +	if (!(val & RTS5261_EXPRESS_LINK_FAIL_MASK))
> +		pcr->extra_caps |= EXTRA_CAPS_SD_EXPRESS;
> +#endif

No #ifdef in .c files please, that is not the Linux kernel way.

>  	pcr->num_slots = 1;
>  	pcr->ops = &rts5261_pcr_ops;
>  
> @@ -791,6 +797,7 @@ void rts5261_init_params(struct rtsx_pcr *pcr)
>  	option->ltr_l1off_snooze_sspwrgate = 0x78;
>  	option->dev_aspm_mode = DEV_ASPM_DYNAMIC;
>  
> +	hw_param->interrupt_en |= DELINK_INT_EN;
>  	option->ocp_en = 1;
>  	hw_param->interrupt_en |= SD_OC_INT_EN;
>  	hw_param->ocp_glitch =  SD_OCP_GLITCH_800U;
> diff --git a/drivers/misc/cardreader/rtsx_pcr.c b/drivers/misc/cardreader/rtsx_pcr.c
> index 06038b325b02..2127c7b6b2e1 100644
> --- a/drivers/misc/cardreader/rtsx_pcr.c
> +++ b/drivers/misc/cardreader/rtsx_pcr.c
> @@ -1026,6 +1026,13 @@ static irqreturn_t rtsx_pci_isr(int irq, void *dev_id)
>  		} else {
>  			pcr->card_removed |= SD_EXIST;
>  			pcr->card_inserted &= ~SD_EXIST;
> +#ifdef MISC_RTSX_PCI_SD_EXPRESS
> +			if (PCI_PID(pcr) == PID_5261) {
> +				rtsx_pci_write_register(pcr, RTS5261_FW_STATUS,
> +					RTS5261_EXPRESS_LINK_FAIL_MASK, 0);
> +				pcr->extra_caps |= EXTRA_CAPS_SD_EXPRESS;
> +			}
> +#endif

Same here, not allowed.

Why do you need the config option at all anyway, shouldn't it be
dynamically detected?

>  		}
>  		pcr->dma_error_count = 0;
>  	}
> diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c
> index 22bf528294b9..7c70d267644b 100644
> --- a/drivers/mmc/core/sd_ops.c
> +++ b/drivers/mmc/core/sd_ops.c
> @@ -171,7 +171,14 @@ 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;
> +	/*
> +	 * Host asks card's PCIe availability
> +	 * if PCIe interface is supported by host.
> +	 */
> +	if ((ocr & 0xFF8000) && (host->caps2 & MMC_CAP2_SD_EXPRESS))
> +		cmd.arg = 0x31 << 8 | test_pattern;
> +	else
> +		cmd.arg = ((ocr & 0xFF8000) != 0) << 8 | test_pattern;

See you detect this dynamically here, so the above config option should
not be needed.

>  	cmd.flags = MMC_RSP_SPI_R7 | MMC_RSP_R7 | MMC_CMD_BCR;
>  
>  	err = mmc_wait_for_cmd(host, &cmd, 0);
> diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c b/drivers/mmc/host/rtsx_pci_sdmmc.c
> index bd50935dc37d..87ad75b253eb 100644
> --- a/drivers/mmc/host/rtsx_pci_sdmmc.c
> +++ b/drivers/mmc/host/rtsx_pci_sdmmc.c
> @@ -219,6 +219,7 @@ static void sd_send_cmd_get_rsp(struct realtek_pci_sdmmc *host,
>  	int rsp_type;
>  	int stat_idx;
>  	bool clock_toggled = false;
> +	u32 relink_time, val;
>  
>  	dev_dbg(sdmmc_dev(host), "%s: SD/MMC CMD %d, arg = 0x%08x\n",
>  			__func__, cmd_idx, arg);
> @@ -322,6 +323,44 @@ static void sd_send_cmd_get_rsp(struct realtek_pci_sdmmc *host,
>  	if (err && clock_toggled)
>  		rtsx_pci_write_register(pcr, SD_BUS_STAT,
>  				SD_CLK_TOGGLE_EN | SD_CLK_FORCE_STOP, 0);
> +
> +	/*
> +	 * If card has PCIe availability and WP if off,
> +	 * reader switch to PCIe mode.
> +	 */
> +	val = rtsx_pci_readl(pcr, RTSX_BIPR);
> +	if (cmd->opcode == 8 && ((cmd->resp[0] >> 8) & 0x10)
> +		&& !(val & SD_WRITE_PROTECT)) {
> +		/* Set relink_time for changing to PCIe card */
> +		relink_time = 0x8FFF;
> +
> +		rtsx_pci_write_register(pcr, 0xFF01, 0xFF, relink_time);
> +		rtsx_pci_write_register(pcr, 0xFF02, 0xFF, relink_time >> 8);
> +		rtsx_pci_write_register(pcr, 0xFF03, 0x01, relink_time >> 16);
> +
> +		rtsx_pci_write_register(pcr, PETXCFG, 0x80, 0x80);
> +		rtsx_pci_write_register(pcr, LDO_VCC_CFG0,
> +			RTS5261_LDO1_OCP_THD_MASK,
> +			pcr->option.sd_800mA_ocp_thd);
> +
> +		if (pcr->ops->disable_auto_blink)
> +			pcr->ops->disable_auto_blink(pcr);
> +
> +		/* For PCIe/NVMe mode can't enter delink issue */
> +		pcr->hw_param.interrupt_en &= ~(SD_INT_EN);
> +		rtsx_pci_writel(pcr, RTSX_BIER, pcr->hw_param.interrupt_en);
> +
> +		rtsx_pci_write_register(pcr, RTS5260_AUTOLOAD_CFG4,
> +			RTS5261_AUX_CLK_16M_EN, RTS5261_AUX_CLK_16M_EN);
> +		rtsx_pci_write_register(pcr, RTS5261_FW_CFG0,
> +			RTS5261_FW_ENTER_EXPRESS, RTS5261_FW_ENTER_EXPRESS);
> +		rtsx_pci_write_register(pcr, RTS5261_FW_CFG1,
> +			RTS5261_MCU_BUS_SEL_MASK | RTS5261_MCU_CLOCK_SEL_MASK
> +			| RTS5261_MCU_CLOCK_GATING | RTS5261_DRIVER_ENABLE_FW,
> +			RTS5261_MCU_CLOCK_SEL_16M | RTS5261_MCU_CLOCK_GATING
> +			| RTS5261_DRIVER_ENABLE_FW);
> +		cmd->error = -EPERM;
> +	}
>  }
>  
>  static int sd_read_data(struct realtek_pci_sdmmc *host, struct mmc_command *cmd,
> @@ -1123,6 +1162,8 @@ static int sdmmc_get_cd(struct mmc_host *mmc)
>  	dev_dbg(sdmmc_dev(host), "%s: RTSX_BIPR = 0x%08x\n", __func__, val);
>  	if (val & SD_EXIST)
>  		cd = 1;
> +	if (pcr->extra_caps & EXTRA_CAPS_SD_EXPRESS)
> +		mmc->caps2 |= MMC_CAP2_SD_EXPRESS;
>  
>  	mutex_unlock(&pcr->pcr_mutex);
>  
> @@ -1333,6 +1374,8 @@ static void init_extra_caps(struct realtek_pci_sdmmc *host)
>  		mmc->caps |= MMC_CAP_1_8V_DDR;
>  	if (pcr->extra_caps & EXTRA_CAPS_MMC_8BIT)
>  		mmc->caps |= MMC_CAP_8_BIT_DATA;
> +	if (pcr->extra_caps & EXTRA_CAPS_SD_EXPRESS)
> +		mmc->caps2 |= MMC_CAP2_SD_EXPRESS;
>  }
>  
>  static void realtek_init_host(struct realtek_pci_sdmmc *host)
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index ba703384bea0..ac1e3da4ad4f 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -369,6 +369,7 @@ struct mmc_host {
>  #define MMC_CAP2_CQE_DCMD	(1 << 24)	/* CQE can issue a direct command */
>  #define MMC_CAP2_AVOID_3_3V	(1 << 25)	/* Host must negotiate down from 3.3V */
>  #define MMC_CAP2_MERGE_CAPABLE	(1 << 26)	/* Host can merge a segment over the segment size */
> +#define MMC_CAP2_SD_EXPRESS	(1 << 27)	/* Host support sd express card */

BIT(27)?


thanks,

greg k-h

Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[Ulf Hansson]

On Tue, 28 Apr 2020 at 05:44, 冯锐 <rui_feng@realsil.com.cn> wrote:
>
> >
> > On Mon, Apr 27, 2020 at 11:41 AM 冯锐 <rui_feng@realsil.com.cn> wrote:
> > >
> > >
> > > > On Sun, Apr 26, 2020 at 09:25:46AM +0800, rui_feng@realsil.com.cn
> > wrote:
> > > > > From: Rui Feng <rui_feng@realsil.com.cn>
> > > > >
> > > > > RTS5261 support legacy SD mode and SD Express mode.
> > > > > In SD7.x, SD association introduce SD Express as a new mode.
> > > > > SD Express mode is distinguished by CMD8.
> > > > > Therefore, CMD8 has new bit for SD Express.
> > > > > SD Express is based on PCIe/NVMe.
> > > > > RTS5261 uses CMD8 to switch to SD Express mode.
> > > >
> > > > So how does this bit work?  They way I imagined SD Express to work
> > > > is that the actual SD Card just shows up as a real PCIe device,
> > > > similar to say Thunderbolt.
> > >
> > > New SD Express card has dual mode. One is SD mode and another is PCIe
> > mode.
> > > In PCIe mode, it act as a PCIe device and use PCIe protocol not Thunderbolt
> > protocol.
> >
> > I think what Christoph was asking about is why you need to issue any
> > commands at all in SD mode when you want to use PCIe mode instead. What
> > happens if you load the NVMe dthriver before loading the rts5261 driver?
> >
> >        Arnd
> >
> > ------Please consider the environment before printing this e-mail.
>
> RTS5261 support SD mode and PCIe/NVMe mode. The workflow is as follows.
> 1.RTS5261 work in SD mode.
> 2.If card is plugged in, Host send CMD8 to ask card's PCIe availability.

This sounds like the card insert/removal needs to be managed by the
rtsx_pci_sdmmc driver (mmc).

> 3.If the card has PCIe availability, RTS5261 switch to PCIe/NVMe mode.

This switch is done by the mmc driver, but how does the PCIe/NVMe
driver know when to take over? Isn't there a synchronization point
needed?

> 4.Mmc driver exit and NVMe driver start working.

Having the mmc driver to exit seems wrong to me. Else how would you
handle a card being removed and inserted again?

In principle you want the mmc core to fail to detect the card and then
do a handover, somehow. No?

Although, to make this work there are a couple of problems you need to
deal with.

1. If the mmc core doesn't successfully detect a card, it will request
the mmc host to power off the card. In this situation, you want to
keep the power to the card, but leave it to be managed by the
PCIe/NVMe driver in some way.

2. During system resume, the mmc core may try to restore power for a
card, especially if it's a removable slot, as to make sure it gets
detected if someone inserted a card while the system was suspended.
Not sure if this plays well with the PCIe/NVMe driver's behaviour.
Again, I think some kind of synchronization is needed.

> 5.If card is unplugged, RTS5261 will switch to SD mode.

Alright, clearly the mmc driver is needed to manage card insert/removal.

> We should send CMD8 in SD mode to ask card's PCIe availability, and the order of NVMe driver and rts5261 driver doesn't matter.

That assumes there's another synchronization mechanism. Maybe there
is, but I don't understand how.

Kind regards
Uffe

答复: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[冯锐]

> On Tue, 28 Apr 2020 at 05:44, 冯锐 <rui_feng@realsil.com.cn> wrote:
> >
> > >
> > > On Mon, Apr 27, 2020 at 11:41 AM 冯锐 <rui_feng@realsil.com.cn>
> wrote:
> > > >
> > > >
> > > > > On Sun, Apr 26, 2020 at 09:25:46AM +0800,
> > > > > rui_feng@realsil.com.cn
> > > wrote:
> > > > > > From: Rui Feng <rui_feng@realsil.com.cn>
> > > > > >
> > > > > > RTS5261 support legacy SD mode and SD Express mode.
> > > > > > In SD7.x, SD association introduce SD Express as a new mode.
> > > > > > SD Express mode is distinguished by CMD8.
> > > > > > Therefore, CMD8 has new bit for SD Express.
> > > > > > SD Express is based on PCIe/NVMe.
> > > > > > RTS5261 uses CMD8 to switch to SD Express mode.
> > > > >
> > > > > So how does this bit work?  They way I imagined SD Express to
> > > > > work is that the actual SD Card just shows up as a real PCIe
> > > > > device, similar to say Thunderbolt.
> > > >
> > > > New SD Express card has dual mode. One is SD mode and another is
> > > > PCIe
> > > mode.
> > > > In PCIe mode, it act as a PCIe device and use PCIe protocol not
> > > > Thunderbolt
> > > protocol.
> > >
> > > I think what Christoph was asking about is why you need to issue any
> > > commands at all in SD mode when you want to use PCIe mode instead.
> > > What happens if you load the NVMe dthriver before loading the rts5261
> driver?
> > >
> > >        Arnd
> > >
> > > ------Please consider the environment before printing this e-mail.
> >
> > RTS5261 support SD mode and PCIe/NVMe mode. The workflow is as follows.
> > 1.RTS5261 work in SD mode.
> > 2.If card is plugged in, Host send CMD8 to ask card's PCIe availability.
> 
> This sounds like the card insert/removal needs to be managed by the
> rtsx_pci_sdmmc driver (mmc).
> 
> > 3.If the card has PCIe availability, RTS5261 switch to PCIe/NVMe mode.
> 
> This switch is done by the mmc driver, but how does the PCIe/NVMe driver
> know when to take over? Isn't there a synchronization point needed?
> 
> > 4.Mmc driver exit and NVMe driver start working.
> 
> Having the mmc driver to exit seems wrong to me. Else how would you handle
> a card being removed and inserted again?
> 
> In principle you want the mmc core to fail to detect the card and then do a
> handover, somehow. No?
> 
> Although, to make this work there are a couple of problems you need to deal
> with.
> 
> 1. If the mmc core doesn't successfully detect a card, it will request the mmc
> host to power off the card. In this situation, you want to keep the power to the
> card, but leave it to be managed by the PCIe/NVMe driver in some way.
> 
> 2. During system resume, the mmc core may try to restore power for a card,
> especially if it's a removable slot, as to make sure it gets detected if someone
> inserted a card while the system was suspended.
> Not sure if this plays well with the PCIe/NVMe driver's behaviour.
> Again, I think some kind of synchronization is needed.
> 
> > 5.If card is unplugged, RTS5261 will switch to SD mode.
> 
> Alright, clearly the mmc driver is needed to manage card insert/removal.
> 
> > We should send CMD8 in SD mode to ask card's PCIe availability, and the
> order of NVMe driver and rts5261 driver doesn't matter.
> 
> That assumes there's another synchronization mechanism. Maybe there is, but
> I don't understand how.
> 
If no card in RTS5261, RTS5261 works in SD mode. If you run command lspci, you can see the RTS5261 device.
When insert a SD Express card, Mmc driver will send CMD8 to ask the card's PCIe availability, because it's a SD EXPRESS card,
RTS5261 will switch to NVMe mode, after switch if you run lspci, you can see RTS5261 disappeared and a NVMe device replaces RTS5261.
In NVMe mode, RTS5261 only provide a bridge between SD Express card and PCIe. For NVMe driver, just like a new NVMe device is inserted.
Mmc core doesn't successfully detect the card and handover to NVMe driver. Because of detect the card failed,
Mmc driver will request the RTS5261 to power off the card, but at that time power off the card will not succeed.
When the card is unplugged, RTS5261 will switch to SD mode by itself and don't need mmc driver to do anything,
If you run lspci, you can see NVMe device disappeared and RTS5261 appeared again.

Kind regards

Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[Ulf Hansson]

On Tue, 19 May 2020 at 11:18, 冯锐 <rui_feng@realsil.com.cn> wrote:
>
> > On Tue, 28 Apr 2020 at 05:44, 冯锐 <rui_feng@realsil.com.cn> wrote:
> > >
> > > >
> > > > On Mon, Apr 27, 2020 at 11:41 AM 冯锐 <rui_feng@realsil.com.cn>
> > wrote:
> > > > >
> > > > >
> > > > > > On Sun, Apr 26, 2020 at 09:25:46AM +0800,
> > > > > > rui_feng@realsil.com.cn
> > > > wrote:
> > > > > > > From: Rui Feng <rui_feng@realsil.com.cn>
> > > > > > >
> > > > > > > RTS5261 support legacy SD mode and SD Express mode.
> > > > > > > In SD7.x, SD association introduce SD Express as a new mode.
> > > > > > > SD Express mode is distinguished by CMD8.
> > > > > > > Therefore, CMD8 has new bit for SD Express.
> > > > > > > SD Express is based on PCIe/NVMe.
> > > > > > > RTS5261 uses CMD8 to switch to SD Express mode.
> > > > > >
> > > > > > So how does this bit work?  They way I imagined SD Express to
> > > > > > work is that the actual SD Card just shows up as a real PCIe
> > > > > > device, similar to say Thunderbolt.
> > > > >
> > > > > New SD Express card has dual mode. One is SD mode and another is
> > > > > PCIe
> > > > mode.
> > > > > In PCIe mode, it act as a PCIe device and use PCIe protocol not
> > > > > Thunderbolt
> > > > protocol.
> > > >
> > > > I think what Christoph was asking about is why you need to issue any
> > > > commands at all in SD mode when you want to use PCIe mode instead.
> > > > What happens if you load the NVMe dthriver before loading the rts5261
> > driver?
> > > >
> > > >        Arnd
> > > >
> > > > ------Please consider the environment before printing this e-mail.
> > >
> > > RTS5261 support SD mode and PCIe/NVMe mode. The workflow is as follows.
> > > 1.RTS5261 work in SD mode.
> > > 2.If card is plugged in, Host send CMD8 to ask card's PCIe availability.
> >
> > This sounds like the card insert/removal needs to be managed by the
> > rtsx_pci_sdmmc driver (mmc).
> >
> > > 3.If the card has PCIe availability, RTS5261 switch to PCIe/NVMe mode.
> >
> > This switch is done by the mmc driver, but how does the PCIe/NVMe driver
> > know when to take over? Isn't there a synchronization point needed?
> >
> > > 4.Mmc driver exit and NVMe driver start working.
> >
> > Having the mmc driver to exit seems wrong to me. Else how would you handle
> > a card being removed and inserted again?
> >
> > In principle you want the mmc core to fail to detect the card and then do a
> > handover, somehow. No?
> >
> > Although, to make this work there are a couple of problems you need to deal
> > with.
> >
> > 1. If the mmc core doesn't successfully detect a card, it will request the mmc
> > host to power off the card. In this situation, you want to keep the power to the
> > card, but leave it to be managed by the PCIe/NVMe driver in some way.
> >
> > 2. During system resume, the mmc core may try to restore power for a card,
> > especially if it's a removable slot, as to make sure it gets detected if someone
> > inserted a card while the system was suspended.
> > Not sure if this plays well with the PCIe/NVMe driver's behaviour.
> > Again, I think some kind of synchronization is needed.
> >
> > > 5.If card is unplugged, RTS5261 will switch to SD mode.
> >
> > Alright, clearly the mmc driver is needed to manage card insert/removal.
> >
> > > We should send CMD8 in SD mode to ask card's PCIe availability, and the
> > order of NVMe driver and rts5261 driver doesn't matter.
> >
> > That assumes there's another synchronization mechanism. Maybe there is, but
> > I don't understand how.
> >
> If no card in RTS5261, RTS5261 works in SD mode. If you run command lspci, you can see the RTS5261 device.

Right.

The rtsx_pci_driver (drivers/misc/cardreader/rtsx_pcr.c) has
registered itself as a pci driver and been probed successfully, I
assume. Then during rtsx_pci_probe() an mfd device is added via
mfd_add_devices(), which corresponds to the rtsx_pci_sdmmc
(drivers/mmc/host/rtsx_pci_sdmmc.c) platform driver.

> When insert a SD Express card, Mmc driver will send CMD8 to ask the card's PCIe availability, because it's a SD EXPRESS card,

Okay, so this will then be a part of the rtsx_pci_sdmmc driver's probe
sequence. Or more exactly, when rtsx_pci_sdmmc_drv_probe() completes
successfully, a mmc rescan work becomes scheduled to try to detect an
SD/MMC card. Then the CMD8 command is sent...

> RTS5261 will switch to NVMe mode, after switch if you run lspci, you can see RTS5261 disappeared and a NVMe device replaces RTS5261.

Can you elaborate more exactly how this managed?

It kind of sounds like the original PCI device is being deleted? How
is this managed?

In any case, the rtsx_pci_driver's ->remove() callback,
rtsx_pci_remove(), should be invoked, I assume?

That would then lead to that mfd_remove_devices() gets called, which
makes the ->remove() callback of the rtsx_pci_sdmmc driver,
rtsx_pci_sdmmc_drv_remove(), to be invoked. Correct?

> In NVMe mode, RTS5261 only provide a bridge between SD Express card and PCIe. For NVMe driver, just like a new NVMe device is inserted.

I don't understand what that means, but I am also not an expert on
PCI/NVMe. Care to explain more?

> Mmc core doesn't successfully detect the card and handover to NVMe driver. Because of detect the card failed,

How do you make sure that the rtsx_pci_sdmmc driver is leaving the
card in the correct state for NVMe?

For example, the mmc core has a loop re-trying with a lower
initialization frequency for the card (400KHz, 300KHz, 200KHz,
100KHz). This will cause additional requests to the rtsx_pci_sdmmc
driver.

> Mmc driver will request the RTS5261 to power off the card, but at that time power off the card will not succeed.

Yes, assuming no card was found, the mmc core calls mmc_power_off().
Ths leads to the rtsx_pci_sdmmc driver's ->set_ios() callback being
invoked, requesting the card to be powered off. I don't see how you
are managing this, what am I missing?

As stated above, I assume you the corresponding platform device for
rtsx_pci_sdmmc being deleted and thus triggering the
rtsx_pci_sdmmc_drv_remove() being called. Correct? If not, how does
the driver manage this?

> When the card is unplugged, RTS5261 will switch to SD mode by itself and don't need mmc driver to do anything,

Okay.

So that means the rtsx_pci_sdmmc driver is being probed again?

> If you run lspci, you can see NVMe device disappeared and RTS5261 appeared again.

I see.

Kind regards
Uffe

答复: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[冯锐]

> 
> On Tue, 19 May 2020 at 11:18, 冯锐 <rui_feng@realsil.com.cn> wrote:
> >
> > > On Tue, 28 Apr 2020 at 05:44, 冯锐 <rui_feng@realsil.com.cn> wrote:
> > > >
> > > > >
> > > > > On Mon, Apr 27, 2020 at 11:41 AM 冯锐 <rui_feng@realsil.com.cn>
> > > wrote:
> > > > > >
> > > > > >
> > > > > > > On Sun, Apr 26, 2020 at 09:25:46AM +0800,
> > > > > > > rui_feng@realsil.com.cn
> > > > > wrote:
> > > > > > > > From: Rui Feng <rui_feng@realsil.com.cn>
> > > > > > > >
> > > > > > > > RTS5261 support legacy SD mode and SD Express mode.
> > > > > > > > In SD7.x, SD association introduce SD Express as a new mode.
> > > > > > > > SD Express mode is distinguished by CMD8.
> > > > > > > > Therefore, CMD8 has new bit for SD Express.
> > > > > > > > SD Express is based on PCIe/NVMe.
> > > > > > > > RTS5261 uses CMD8 to switch to SD Express mode.
> > > > > > >
> > > > > > > So how does this bit work?  They way I imagined SD Express
> > > > > > > to work is that the actual SD Card just shows up as a real
> > > > > > > PCIe device, similar to say Thunderbolt.
> > > > > >
> > > > > > New SD Express card has dual mode. One is SD mode and another
> > > > > > is PCIe
> > > > > mode.
> > > > > > In PCIe mode, it act as a PCIe device and use PCIe protocol
> > > > > > not Thunderbolt
> > > > > protocol.
> > > > >
> > > > > I think what Christoph was asking about is why you need to issue
> > > > > any commands at all in SD mode when you want to use PCIe mode
> instead.
> > > > > What happens if you load the NVMe dthriver before loading the
> > > > > rts5261
> > > driver?
> > > > >
> > > > >        Arnd
> > > > >
> > > > > ------Please consider the environment before printing this e-mail.
> > > >
> > > > RTS5261 support SD mode and PCIe/NVMe mode. The workflow is as
> follows.
> > > > 1.RTS5261 work in SD mode.
> > > > 2.If card is plugged in, Host send CMD8 to ask card's PCIe availability.
> > >
> > > This sounds like the card insert/removal needs to be managed by the
> > > rtsx_pci_sdmmc driver (mmc).
> > >
> > > > 3.If the card has PCIe availability, RTS5261 switch to PCIe/NVMe mode.
> > >
> > > This switch is done by the mmc driver, but how does the PCIe/NVMe
> > > driver know when to take over? Isn't there a synchronization point needed?
> > >
> > > > 4.Mmc driver exit and NVMe driver start working.
> > >
> > > Having the mmc driver to exit seems wrong to me. Else how would you
> > > handle a card being removed and inserted again?
> > >
> > > In principle you want the mmc core to fail to detect the card and
> > > then do a handover, somehow. No?
> > >
> > > Although, to make this work there are a couple of problems you need
> > > to deal with.
> > >
> > > 1. If the mmc core doesn't successfully detect a card, it will
> > > request the mmc host to power off the card. In this situation, you
> > > want to keep the power to the card, but leave it to be managed by the
> PCIe/NVMe driver in some way.
> > >
> > > 2. During system resume, the mmc core may try to restore power for a
> > > card, especially if it's a removable slot, as to make sure it gets
> > > detected if someone inserted a card while the system was suspended.
> > > Not sure if this plays well with the PCIe/NVMe driver's behaviour.
> > > Again, I think some kind of synchronization is needed.
> > >
> > > > 5.If card is unplugged, RTS5261 will switch to SD mode.
> > >
> > > Alright, clearly the mmc driver is needed to manage card insert/removal.
> > >
> > > > We should send CMD8 in SD mode to ask card's PCIe availability,
> > > > and the
> > > order of NVMe driver and rts5261 driver doesn't matter.
> > >
> > > That assumes there's another synchronization mechanism. Maybe there
> > > is, but I don't understand how.
> > >
> > If no card in RTS5261, RTS5261 works in SD mode. If you run command lspci,
> you can see the RTS5261 device.
> 
> Right.
> 
> The rtsx_pci_driver (drivers/misc/cardreader/rtsx_pcr.c) has registered itself
> as a pci driver and been probed successfully, I assume. Then during
> rtsx_pci_probe() an mfd device is added via mfd_add_devices(), which
> corresponds to the rtsx_pci_sdmmc
> (drivers/mmc/host/rtsx_pci_sdmmc.c) platform driver.
> 
> > When insert a SD Express card, Mmc driver will send CMD8 to ask the
> > card's PCIe availability, because it's a SD EXPRESS card,
> 
> Okay, so this will then be a part of the rtsx_pci_sdmmc driver's probe sequence.
> Or more exactly, when rtsx_pci_sdmmc_drv_probe() completes successfully, a
> mmc rescan work becomes scheduled to try to detect an SD/MMC card. Then
> the CMD8 command is sent...
> 
> > RTS5261 will switch to NVMe mode, after switch if you run lspci, you can see
> RTS5261 disappeared and a NVMe device replaces RTS5261.
> 
> Can you elaborate more exactly how this managed?
> 
> It kind of sounds like the original PCI device is being deleted? How is this
> managed?
> 
> In any case, the rtsx_pci_driver's ->remove() callback, rtsx_pci_remove(),
> should be invoked, I assume?
> 
> That would then lead to that mfd_remove_devices() gets called, which makes
> the ->remove() callback of the rtsx_pci_sdmmc driver,
> rtsx_pci_sdmmc_drv_remove(), to be invoked. Correct?
> 
Yes, after RTS5261 switch to NVMe mode, rtsx_pci_remove() and rtsx_pci_sdmmc_drv_remove() will be invoked.

> > In NVMe mode, RTS5261 only provide a bridge between SD Express card and
> PCIe. For NVMe driver, just like a new NVMe device is inserted.
> 
> I don't understand what that means, but I am also not an expert on PCI/NVMe.
> Care to explain more?
> 
In NVMe mode, SD Express card connect the computer via PCIe.
IN SD mode, card connect computer via reader.

> > Mmc core doesn't successfully detect the card and handover to NVMe
> > driver. Because of detect the card failed,
> 
> How do you make sure that the rtsx_pci_sdmmc driver is leaving the card in the
> correct state for NVMe?
> 
> For example, the mmc core has a loop re-trying with a lower initialization
> frequency for the card (400KHz, 300KHz, 200KHz, 100KHz). This will cause
> additional requests to the rtsx_pci_sdmmc driver.
> 
> > Mmc driver will request the RTS5261 to power off the card, but at that time
> power off the card will not succeed.
> 
> Yes, assuming no card was found, the mmc core calls mmc_power_off().
> Ths leads to the rtsx_pci_sdmmc driver's ->set_ios() callback being invoked,
> requesting the card to be powered off. I don't see how you are managing this,
> what am I missing?
> 
Before power off card and re-trying initialization, rtsx driver sets RTS5261 0xFF55 bit4=0.
After set 0xFF55 bit4=0, RTS5261 can't receive any CMD from PCIe and prepare for device disappear.
Therefore, MMC driver can't change card status.

> As stated above, I assume you the corresponding platform device for
> rtsx_pci_sdmmc being deleted and thus triggering the
> rtsx_pci_sdmmc_drv_remove() being called. Correct? If not, how does the
> driver manage this?
> 
Yes.

> > When the card is unplugged, RTS5261 will switch to SD mode by itself
> > and don't need mmc driver to do anything,
> 
> Okay.
> 
> So that means the rtsx_pci_sdmmc driver is being probed again?
> 
Yes.

> > If you run lspci, you can see NVMe device disappeared and RTS5261 appeared
> again.
> 
> I see.
> 
Kind regards

Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[Ulf Hansson]

+linux-mmc

On Mon, 1 Jun 2020 at 09:34, 冯锐 <rui_feng@realsil.com.cn> wrote:
>
> >
> > On Tue, 19 May 2020 at 11:18, 冯锐 <rui_feng@realsil.com.cn> wrote:
> > >
> > > > On Tue, 28 Apr 2020 at 05:44, 冯锐 <rui_feng@realsil.com.cn> wrote:
> > > > >
> > > > > >
> > > > > > On Mon, Apr 27, 2020 at 11:41 AM 冯锐 <rui_feng@realsil.com.cn>
> > > > wrote:
> > > > > > >
> > > > > > >
> > > > > > > > On Sun, Apr 26, 2020 at 09:25:46AM +0800,
> > > > > > > > rui_feng@realsil.com.cn
> > > > > > wrote:
> > > > > > > > > From: Rui Feng <rui_feng@realsil.com.cn>
> > > > > > > > >
> > > > > > > > > RTS5261 support legacy SD mode and SD Express mode.
> > > > > > > > > In SD7.x, SD association introduce SD Express as a new mode.
> > > > > > > > > SD Express mode is distinguished by CMD8.
> > > > > > > > > Therefore, CMD8 has new bit for SD Express.
> > > > > > > > > SD Express is based on PCIe/NVMe.
> > > > > > > > > RTS5261 uses CMD8 to switch to SD Express mode.
> > > > > > > >
> > > > > > > > So how does this bit work?  They way I imagined SD Express
> > > > > > > > to work is that the actual SD Card just shows up as a real
> > > > > > > > PCIe device, similar to say Thunderbolt.
> > > > > > >
> > > > > > > New SD Express card has dual mode. One is SD mode and another
> > > > > > > is PCIe
> > > > > > mode.
> > > > > > > In PCIe mode, it act as a PCIe device and use PCIe protocol
> > > > > > > not Thunderbolt
> > > > > > protocol.
> > > > > >
> > > > > > I think what Christoph was asking about is why you need to issue
> > > > > > any commands at all in SD mode when you want to use PCIe mode
> > instead.
> > > > > > What happens if you load the NVMe dthriver before loading the
> > > > > > rts5261
> > > > driver?
> > > > > >
> > > > > >        Arnd
> > > > > >
> > > > > > ------Please consider the environment before printing this e-mail.
> > > > >
> > > > > RTS5261 support SD mode and PCIe/NVMe mode. The workflow is as
> > follows.
> > > > > 1.RTS5261 work in SD mode.
> > > > > 2.If card is plugged in, Host send CMD8 to ask card's PCIe availability.
> > > >
> > > > This sounds like the card insert/removal needs to be managed by the
> > > > rtsx_pci_sdmmc driver (mmc).
> > > >
> > > > > 3.If the card has PCIe availability, RTS5261 switch to PCIe/NVMe mode.
> > > >
> > > > This switch is done by the mmc driver, but how does the PCIe/NVMe
> > > > driver know when to take over? Isn't there a synchronization point needed?
> > > >
> > > > > 4.Mmc driver exit and NVMe driver start working.
> > > >
> > > > Having the mmc driver to exit seems wrong to me. Else how would you
> > > > handle a card being removed and inserted again?
> > > >
> > > > In principle you want the mmc core to fail to detect the card and
> > > > then do a handover, somehow. No?
> > > >
> > > > Although, to make this work there are a couple of problems you need
> > > > to deal with.
> > > >
> > > > 1. If the mmc core doesn't successfully detect a card, it will
> > > > request the mmc host to power off the card. In this situation, you
> > > > want to keep the power to the card, but leave it to be managed by the
> > PCIe/NVMe driver in some way.
> > > >
> > > > 2. During system resume, the mmc core may try to restore power for a
> > > > card, especially if it's a removable slot, as to make sure it gets
> > > > detected if someone inserted a card while the system was suspended.
> > > > Not sure if this plays well with the PCIe/NVMe driver's behaviour.
> > > > Again, I think some kind of synchronization is needed.
> > > >
> > > > > 5.If card is unplugged, RTS5261 will switch to SD mode.
> > > >
> > > > Alright, clearly the mmc driver is needed to manage card insert/removal.
> > > >
> > > > > We should send CMD8 in SD mode to ask card's PCIe availability,
> > > > > and the
> > > > order of NVMe driver and rts5261 driver doesn't matter.
> > > >
> > > > That assumes there's another synchronization mechanism. Maybe there
> > > > is, but I don't understand how.
> > > >
> > > If no card in RTS5261, RTS5261 works in SD mode. If you run command lspci,
> > you can see the RTS5261 device.
> >
> > Right.
> >
> > The rtsx_pci_driver (drivers/misc/cardreader/rtsx_pcr.c) has registered itself
> > as a pci driver and been probed successfully, I assume. Then during
> > rtsx_pci_probe() an mfd device is added via mfd_add_devices(), which
> > corresponds to the rtsx_pci_sdmmc
> > (drivers/mmc/host/rtsx_pci_sdmmc.c) platform driver.
> >
> > > When insert a SD Express card, Mmc driver will send CMD8 to ask the
> > > card's PCIe availability, because it's a SD EXPRESS card,
> >
> > Okay, so this will then be a part of the rtsx_pci_sdmmc driver's probe sequence.
> > Or more exactly, when rtsx_pci_sdmmc_drv_probe() completes successfully, a
> > mmc rescan work becomes scheduled to try to detect an SD/MMC card. Then
> > the CMD8 command is sent...
> >
> > > RTS5261 will switch to NVMe mode, after switch if you run lspci, you can see
> > RTS5261 disappeared and a NVMe device replaces RTS5261.
> >
> > Can you elaborate more exactly how this managed?
> >
> > It kind of sounds like the original PCI device is being deleted? How is this
> > managed?
> >
> > In any case, the rtsx_pci_driver's ->remove() callback, rtsx_pci_remove(),
> > should be invoked, I assume?
> >
> > That would then lead to that mfd_remove_devices() gets called, which makes
> > the ->remove() callback of the rtsx_pci_sdmmc driver,
> > rtsx_pci_sdmmc_drv_remove(), to be invoked. Correct?
> >
> Yes, after RTS5261 switch to NVMe mode, rtsx_pci_remove() and rtsx_pci_sdmmc_drv_remove() will be invoked.

So, the ->remove() callbacks are invoked because the PCI device that
corresponds to the rtsx_pci_driver is being deleted. Can you explain
who deletes the PCI device and why?

I am not a PCI expert, so apologize for my ignorance - but I really
want to understand how this is supposed to work.

>
> > > In NVMe mode, RTS5261 only provide a bridge between SD Express card and
> > PCIe. For NVMe driver, just like a new NVMe device is inserted.
> >
> > I don't understand what that means, but I am also not an expert on PCI/NVMe.
> > Care to explain more?
> >
> In NVMe mode, SD Express card connect the computer via PCIe.
> IN SD mode, card connect computer via reader.

That didn't make better sense to me, sorry. I do know about the SD
spec and the SD-express card protocol parts. Anyway, let's leave this
for now.

>
> > > Mmc core doesn't successfully detect the card and handover to NVMe
> > > driver. Because of detect the card failed,
> >
> > How do you make sure that the rtsx_pci_sdmmc driver is leaving the card in the
> > correct state for NVMe?
> >
> > For example, the mmc core has a loop re-trying with a lower initialization
> > frequency for the card (400KHz, 300KHz, 200KHz, 100KHz). This will cause
> > additional requests to the rtsx_pci_sdmmc driver.
> >
> > > Mmc driver will request the RTS5261 to power off the card, but at that time
> > power off the card will not succeed.
> >
> > Yes, assuming no card was found, the mmc core calls mmc_power_off().
> > Ths leads to the rtsx_pci_sdmmc driver's ->set_ios() callback being invoked,
> > requesting the card to be powered off. I don't see how you are managing this,
> > what am I missing?
> >
> Before power off card and re-trying initialization, rtsx driver sets RTS5261 0xFF55 bit4=0.
> After set 0xFF55 bit4=0, RTS5261 can't receive any CMD from PCIe and prepare for device disappear.
> Therefore, MMC driver can't change card status.

Okay, so beyond that point - any calls to the interface that is
provided from drivers/misc/cardreader/rtsx_pcr will fail, when invoked
by the rtsx_pci_sdmmc driver?

To me, that sounds a bit fragile and it's also relying on a specific
behaviour of the RTS5261 card reader interface. I wonder if this could
be considered as a common behaviour...??

Perhaps it's better to teach the mmc core *more* about SD express
cards. Maybe add a new host ops for dealing with the specific CMD8
command and make the mmc core to "bail out", rather than keep retrying
the initialization. In principle I think the core should accept that
it may have found an SD express card, then abort further communication
with it. At least until the mmc host indicates that a
re-initialization of the card can be done, which could be through a
remove/re-probe, for example.

>
> > As stated above, I assume you the corresponding platform device for
> > rtsx_pci_sdmmc being deleted and thus triggering the
> > rtsx_pci_sdmmc_drv_remove() being called. Correct? If not, how does the
> > driver manage this?
> >
> Yes.
>
> > > When the card is unplugged, RTS5261 will switch to SD mode by itself
> > > and don't need mmc driver to do anything,
> >
> > Okay.
> >
> > So that means the rtsx_pci_sdmmc driver is being probed again?
> >
> Yes.
>
> > > If you run lspci, you can see NVMe device disappeared and RTS5261 appeared
> > again.
> >
> > I see.
> >

If you need some help on the mmc core parts, I am willing to help out.
However, first, I would like to get some better understanding of who
and why the PCI device is deleted.

Kind regards
Uffe

答复: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[冯锐]

> 
> +linux-mmc
> 
> On Mon, 1 Jun 2020 at 09:34, 冯锐 <rui_feng@realsil.com.cn> wrote:
> >
> > >
> > > On Tue, 19 May 2020 at 11:18, 冯锐 <rui_feng@realsil.com.cn> wrote:
> > > >
> > > > > On Tue, 28 Apr 2020 at 05:44, 冯锐 <rui_feng@realsil.com.cn> wrote:
> > > > > >
> > > > > > >
> > > > > > > On Mon, Apr 27, 2020 at 11:41 AM 冯锐
> > > > > > > <rui_feng@realsil.com.cn>
> > > > > wrote:
> > > > > > > >
> > > > > > > >
> > > > > > > > > On Sun, Apr 26, 2020 at 09:25:46AM +0800,
> > > > > > > > > rui_feng@realsil.com.cn
> > > > > > > wrote:
> > > > > > > > > > From: Rui Feng <rui_feng@realsil.com.cn>
> > > > > > > > > >
> > > > > > > > > > RTS5261 support legacy SD mode and SD Express mode.
> > > > > > > > > > In SD7.x, SD association introduce SD Express as a new mode.
> > > > > > > > > > SD Express mode is distinguished by CMD8.
> > > > > > > > > > Therefore, CMD8 has new bit for SD Express.
> > > > > > > > > > SD Express is based on PCIe/NVMe.
> > > > > > > > > > RTS5261 uses CMD8 to switch to SD Express mode.
> > > > > > > > >
> > > > > > > > > So how does this bit work?  They way I imagined SD
> > > > > > > > > Express to work is that the actual SD Card just shows up
> > > > > > > > > as a real PCIe device, similar to say Thunderbolt.
> > > > > > > >
> > > > > > > > New SD Express card has dual mode. One is SD mode and
> > > > > > > > another is PCIe
> > > > > > > mode.
> > > > > > > > In PCIe mode, it act as a PCIe device and use PCIe
> > > > > > > > protocol not Thunderbolt
> > > > > > > protocol.
> > > > > > >
> > > > > > > I think what Christoph was asking about is why you need to
> > > > > > > issue any commands at all in SD mode when you want to use
> > > > > > > PCIe mode
> > > instead.
> > > > > > > What happens if you load the NVMe dthriver before loading
> > > > > > > the
> > > > > > > rts5261
> > > > > driver?
> > > > > > >
> > > > > > >        Arnd
> > > > > > >
> > > > > > > ------Please consider the environment before printing this e-mail.
> > > > > >
> > > > > > RTS5261 support SD mode and PCIe/NVMe mode. The workflow is as
> > > follows.
> > > > > > 1.RTS5261 work in SD mode.
> > > > > > 2.If card is plugged in, Host send CMD8 to ask card's PCIe availability.
> > > > >
> > > > > This sounds like the card insert/removal needs to be managed by
> > > > > the rtsx_pci_sdmmc driver (mmc).
> > > > >
> > > > > > 3.If the card has PCIe availability, RTS5261 switch to PCIe/NVMe
> mode.
> > > > >
> > > > > This switch is done by the mmc driver, but how does the
> > > > > PCIe/NVMe driver know when to take over? Isn't there a
> synchronization point needed?
> > > > >
> > > > > > 4.Mmc driver exit and NVMe driver start working.
> > > > >
> > > > > Having the mmc driver to exit seems wrong to me. Else how would
> > > > > you handle a card being removed and inserted again?
> > > > >
> > > > > In principle you want the mmc core to fail to detect the card
> > > > > and then do a handover, somehow. No?
> > > > >
> > > > > Although, to make this work there are a couple of problems you
> > > > > need to deal with.
> > > > >
> > > > > 1. If the mmc core doesn't successfully detect a card, it will
> > > > > request the mmc host to power off the card. In this situation,
> > > > > you want to keep the power to the card, but leave it to be
> > > > > managed by the
> > > PCIe/NVMe driver in some way.
> > > > >
> > > > > 2. During system resume, the mmc core may try to restore power
> > > > > for a card, especially if it's a removable slot, as to make sure
> > > > > it gets detected if someone inserted a card while the system was
> suspended.
> > > > > Not sure if this plays well with the PCIe/NVMe driver's behaviour.
> > > > > Again, I think some kind of synchronization is needed.
> > > > >
> > > > > > 5.If card is unplugged, RTS5261 will switch to SD mode.
> > > > >
> > > > > Alright, clearly the mmc driver is needed to manage card
> insert/removal.
> > > > >
> > > > > > We should send CMD8 in SD mode to ask card's PCIe
> > > > > > availability, and the
> > > > > order of NVMe driver and rts5261 driver doesn't matter.
> > > > >
> > > > > That assumes there's another synchronization mechanism. Maybe
> > > > > there is, but I don't understand how.
> > > > >
> > > > If no card in RTS5261, RTS5261 works in SD mode. If you run
> > > > command lspci,
> > > you can see the RTS5261 device.
> > >
> > > Right.
> > >
> > > The rtsx_pci_driver (drivers/misc/cardreader/rtsx_pcr.c) has
> > > registered itself as a pci driver and been probed successfully, I
> > > assume. Then during
> > > rtsx_pci_probe() an mfd device is added via mfd_add_devices(), which
> > > corresponds to the rtsx_pci_sdmmc
> > > (drivers/mmc/host/rtsx_pci_sdmmc.c) platform driver.
> > >
> > > > When insert a SD Express card, Mmc driver will send CMD8 to ask
> > > > the card's PCIe availability, because it's a SD EXPRESS card,
> > >
> > > Okay, so this will then be a part of the rtsx_pci_sdmmc driver's probe
> sequence.
> > > Or more exactly, when rtsx_pci_sdmmc_drv_probe() completes
> > > successfully, a mmc rescan work becomes scheduled to try to detect
> > > an SD/MMC card. Then the CMD8 command is sent...
> > >
> > > > RTS5261 will switch to NVMe mode, after switch if you run lspci,
> > > > you can see
> > > RTS5261 disappeared and a NVMe device replaces RTS5261.
> > >
> > > Can you elaborate more exactly how this managed?
> > >
> > > It kind of sounds like the original PCI device is being deleted? How
> > > is this managed?
> > >
> > > In any case, the rtsx_pci_driver's ->remove() callback,
> > > rtsx_pci_remove(), should be invoked, I assume?
> > >
> > > That would then lead to that mfd_remove_devices() gets called, which
> > > makes the ->remove() callback of the rtsx_pci_sdmmc driver,
> > > rtsx_pci_sdmmc_drv_remove(), to be invoked. Correct?
> > >
> > Yes, after RTS5261 switch to NVMe mode, rtsx_pci_remove() and
> rtsx_pci_sdmmc_drv_remove() will be invoked.
> 
> So, the ->remove() callbacks are invoked because the PCI device that
> corresponds to the rtsx_pci_driver is being deleted. Can you explain who
> deletes the PCI device and why?
> 
> I am not a PCI expert, so apologize for my ignorance - but I really want to
> understand how this is supposed to work.
> 
Rtsx host driver sets RTS5261 0xFF54 bit0=1 and 0xFF55 bit4=0, then RTS5261 will switch MCU and enter SD EXPRESS mode.
Because hardware design is involved, sorry I can't explain much more details about that.

> >
> > > > In NVMe mode, RTS5261 only provide a bridge between SD Express
> > > > card and
> > > PCIe. For NVMe driver, just like a new NVMe device is inserted.
> > >
> > > I don't understand what that means, but I am also not an expert on
> PCI/NVMe.
> > > Care to explain more?
> > >
> > In NVMe mode, SD Express card connect the computer via PCIe.
> > IN SD mode, card connect computer via reader.
> 
> That didn't make better sense to me, sorry. I do know about the SD spec and
> the SD-express card protocol parts. Anyway, let's leave this for now.
> 
> >
> > > > Mmc core doesn't successfully detect the card and handover to NVMe
> > > > driver. Because of detect the card failed,
> > >
> > > How do you make sure that the rtsx_pci_sdmmc driver is leaving the
> > > card in the correct state for NVMe?
> > >
> > > For example, the mmc core has a loop re-trying with a lower
> > > initialization frequency for the card (400KHz, 300KHz, 200KHz,
> > > 100KHz). This will cause additional requests to the rtsx_pci_sdmmc driver.
> > >
> > > > Mmc driver will request the RTS5261 to power off the card, but at
> > > > that time
> > > power off the card will not succeed.
> > >
> > > Yes, assuming no card was found, the mmc core calls mmc_power_off().
> > > Ths leads to the rtsx_pci_sdmmc driver's ->set_ios() callback being
> > > invoked, requesting the card to be powered off. I don't see how you
> > > are managing this, what am I missing?
> > >
> > Before power off card and re-trying initialization, rtsx driver sets RTS5261
> 0xFF55 bit4=0.
> > After set 0xFF55 bit4=0, RTS5261 can't receive any CMD from PCIe and
> prepare for device disappear.
> > Therefore, MMC driver can't change card status.
> 
> Okay, so beyond that point - any calls to the interface that is provided from
> drivers/misc/cardreader/rtsx_pcr will fail, when invoked by the
> rtsx_pci_sdmmc driver?
> 
Yes.

> To me, that sounds a bit fragile and it's also relying on a specific behaviour of
> the RTS5261 card reader interface. I wonder if this could be considered as a
> common behaviour...??
> 
It's a feature proposal by realtek not common.

> Perhaps it's better to teach the mmc core *more* about SD express cards.
> Maybe add a new host ops for dealing with the specific CMD8 command and
> make the mmc core to "bail out", rather than keep retrying the initialization. In
> principle I think the core should accept that it may have found an SD express
> card, then abort further communication with it. At least until the mmc host
> indicates that a re-initialization of the card can be done, which could be through
> a remove/re-probe, for example.
> 
In SD7.x spec host should send CMD8 with bit20=1 and bit21=1 to ask card's PCIe availability.
So the CMD8 is not specific for RTS5261, it's just newly defined in SD7.x spec.
The mmc core will request host to power off card and has a loop re-trying with different initialization frequency for the card (400KHz, 300KHz, 200KHz,
100KHz), if I don't modify mmc core, I can't stop the power off and re-trying, if I modify mmc core, RTS5261 will become a special case for mmc core.
So make the operation fail is the minimum modification in mmc core for me. Do you have any other suggestion?

> >
> > > As stated above, I assume you the corresponding platform device for
> > > rtsx_pci_sdmmc being deleted and thus triggering the
> > > rtsx_pci_sdmmc_drv_remove() being called. Correct? If not, how does
> > > the driver manage this?
> > >
> > Yes.
> >
> > > > When the card is unplugged, RTS5261 will switch to SD mode by
> > > > itself and don't need mmc driver to do anything,
> > >
> > > Okay.
> > >
> > > So that means the rtsx_pci_sdmmc driver is being probed again?
> > >
> > Yes.
> >
> > > > If you run lspci, you can see NVMe device disappeared and RTS5261
> > > > appeared
> > > again.
> > >
> > > I see.
> > >
> 
> If you need some help on the mmc core parts, I am willing to help out.
> However, first, I would like to get some better understanding of who and why
> the PCI device is deleted.
> 
Can I stop the re-trying in host driver other than modify mmc core?
As above, I'm sorry I can't explain much more details about hardware design.


Kind regards

Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[Ulf Hansson]

On Tue, 2 Jun 2020 at 04:41, 冯锐 <rui_feng@realsil.com.cn> wrote:
>
> >
> > +linux-mmc
> >
> > On Mon, 1 Jun 2020 at 09:34, 冯锐 <rui_feng@realsil.com.cn> wrote:
> > >
> > > >
> > > > On Tue, 19 May 2020 at 11:18, 冯锐 <rui_feng@realsil.com.cn> wrote:
> > > > >
> > > > > > On Tue, 28 Apr 2020 at 05:44, 冯锐 <rui_feng@realsil.com.cn> wrote:
> > > > > > >
> > > > > > > >
> > > > > > > > On Mon, Apr 27, 2020 at 11:41 AM 冯锐
> > > > > > > > <rui_feng@realsil.com.cn>
> > > > > > wrote:
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > > On Sun, Apr 26, 2020 at 09:25:46AM +0800,
> > > > > > > > > > rui_feng@realsil.com.cn
> > > > > > > > wrote:
> > > > > > > > > > > From: Rui Feng <rui_feng@realsil.com.cn>
> > > > > > > > > > >
> > > > > > > > > > > RTS5261 support legacy SD mode and SD Express mode.
> > > > > > > > > > > In SD7.x, SD association introduce SD Express as a new mode.
> > > > > > > > > > > SD Express mode is distinguished by CMD8.
> > > > > > > > > > > Therefore, CMD8 has new bit for SD Express.
> > > > > > > > > > > SD Express is based on PCIe/NVMe.
> > > > > > > > > > > RTS5261 uses CMD8 to switch to SD Express mode.
> > > > > > > > > >
> > > > > > > > > > So how does this bit work?  They way I imagined SD
> > > > > > > > > > Express to work is that the actual SD Card just shows up
> > > > > > > > > > as a real PCIe device, similar to say Thunderbolt.
> > > > > > > > >
> > > > > > > > > New SD Express card has dual mode. One is SD mode and
> > > > > > > > > another is PCIe
> > > > > > > > mode.
> > > > > > > > > In PCIe mode, it act as a PCIe device and use PCIe
> > > > > > > > > protocol not Thunderbolt
> > > > > > > > protocol.
> > > > > > > >
> > > > > > > > I think what Christoph was asking about is why you need to
> > > > > > > > issue any commands at all in SD mode when you want to use
> > > > > > > > PCIe mode
> > > > instead.
> > > > > > > > What happens if you load the NVMe dthriver before loading
> > > > > > > > the
> > > > > > > > rts5261
> > > > > > driver?
> > > > > > > >
> > > > > > > >        Arnd
> > > > > > > >
> > > > > > > > ------Please consider the environment before printing this e-mail.
> > > > > > >
> > > > > > > RTS5261 support SD mode and PCIe/NVMe mode. The workflow is as
> > > > follows.
> > > > > > > 1.RTS5261 work in SD mode.
> > > > > > > 2.If card is plugged in, Host send CMD8 to ask card's PCIe availability.
> > > > > >
> > > > > > This sounds like the card insert/removal needs to be managed by
> > > > > > the rtsx_pci_sdmmc driver (mmc).
> > > > > >
> > > > > > > 3.If the card has PCIe availability, RTS5261 switch to PCIe/NVMe
> > mode.
> > > > > >
> > > > > > This switch is done by the mmc driver, but how does the
> > > > > > PCIe/NVMe driver know when to take over? Isn't there a
> > synchronization point needed?
> > > > > >
> > > > > > > 4.Mmc driver exit and NVMe driver start working.
> > > > > >
> > > > > > Having the mmc driver to exit seems wrong to me. Else how would
> > > > > > you handle a card being removed and inserted again?
> > > > > >
> > > > > > In principle you want the mmc core to fail to detect the card
> > > > > > and then do a handover, somehow. No?
> > > > > >
> > > > > > Although, to make this work there are a couple of problems you
> > > > > > need to deal with.
> > > > > >
> > > > > > 1. If the mmc core doesn't successfully detect a card, it will
> > > > > > request the mmc host to power off the card. In this situation,
> > > > > > you want to keep the power to the card, but leave it to be
> > > > > > managed by the
> > > > PCIe/NVMe driver in some way.
> > > > > >
> > > > > > 2. During system resume, the mmc core may try to restore power
> > > > > > for a card, especially if it's a removable slot, as to make sure
> > > > > > it gets detected if someone inserted a card while the system was
> > suspended.
> > > > > > Not sure if this plays well with the PCIe/NVMe driver's behaviour.
> > > > > > Again, I think some kind of synchronization is needed.
> > > > > >
> > > > > > > 5.If card is unplugged, RTS5261 will switch to SD mode.
> > > > > >
> > > > > > Alright, clearly the mmc driver is needed to manage card
> > insert/removal.
> > > > > >
> > > > > > > We should send CMD8 in SD mode to ask card's PCIe
> > > > > > > availability, and the
> > > > > > order of NVMe driver and rts5261 driver doesn't matter.
> > > > > >
> > > > > > That assumes there's another synchronization mechanism. Maybe
> > > > > > there is, but I don't understand how.
> > > > > >
> > > > > If no card in RTS5261, RTS5261 works in SD mode. If you run
> > > > > command lspci,
> > > > you can see the RTS5261 device.
> > > >
> > > > Right.
> > > >
> > > > The rtsx_pci_driver (drivers/misc/cardreader/rtsx_pcr.c) has
> > > > registered itself as a pci driver and been probed successfully, I
> > > > assume. Then during
> > > > rtsx_pci_probe() an mfd device is added via mfd_add_devices(), which
> > > > corresponds to the rtsx_pci_sdmmc
> > > > (drivers/mmc/host/rtsx_pci_sdmmc.c) platform driver.
> > > >
> > > > > When insert a SD Express card, Mmc driver will send CMD8 to ask
> > > > > the card's PCIe availability, because it's a SD EXPRESS card,
> > > >
> > > > Okay, so this will then be a part of the rtsx_pci_sdmmc driver's probe
> > sequence.
> > > > Or more exactly, when rtsx_pci_sdmmc_drv_probe() completes
> > > > successfully, a mmc rescan work becomes scheduled to try to detect
> > > > an SD/MMC card. Then the CMD8 command is sent...
> > > >
> > > > > RTS5261 will switch to NVMe mode, after switch if you run lspci,
> > > > > you can see
> > > > RTS5261 disappeared and a NVMe device replaces RTS5261.
> > > >
> > > > Can you elaborate more exactly how this managed?
> > > >
> > > > It kind of sounds like the original PCI device is being deleted? How
> > > > is this managed?
> > > >
> > > > In any case, the rtsx_pci_driver's ->remove() callback,
> > > > rtsx_pci_remove(), should be invoked, I assume?
> > > >
> > > > That would then lead to that mfd_remove_devices() gets called, which
> > > > makes the ->remove() callback of the rtsx_pci_sdmmc driver,
> > > > rtsx_pci_sdmmc_drv_remove(), to be invoked. Correct?
> > > >
> > > Yes, after RTS5261 switch to NVMe mode, rtsx_pci_remove() and
> > rtsx_pci_sdmmc_drv_remove() will be invoked.
> >
> > So, the ->remove() callbacks are invoked because the PCI device that
> > corresponds to the rtsx_pci_driver is being deleted. Can you explain who
> > deletes the PCI device and why?
> >
> > I am not a PCI expert, so apologize for my ignorance - but I really want to
> > understand how this is supposed to work.
> >
> Rtsx host driver sets RTS5261 0xFF54 bit0=1 and 0xFF55 bit4=0, then RTS5261 will switch MCU and enter SD EXPRESS mode.
> Because hardware design is involved, sorry I can't explain much more details about that.

Okay, so somehow that will trigger the PCI bus to remove the
corresponding PCI device, I guess.

>
> > >
> > > > > In NVMe mode, RTS5261 only provide a bridge between SD Express
> > > > > card and
> > > > PCIe. For NVMe driver, just like a new NVMe device is inserted.
> > > >
> > > > I don't understand what that means, but I am also not an expert on
> > PCI/NVMe.
> > > > Care to explain more?
> > > >
> > > In NVMe mode, SD Express card connect the computer via PCIe.
> > > IN SD mode, card connect computer via reader.
> >
> > That didn't make better sense to me, sorry. I do know about the SD spec and
> > the SD-express card protocol parts. Anyway, let's leave this for now.
> >
> > >
> > > > > Mmc core doesn't successfully detect the card and handover to NVMe
> > > > > driver. Because of detect the card failed,
> > > >
> > > > How do you make sure that the rtsx_pci_sdmmc driver is leaving the
> > > > card in the correct state for NVMe?
> > > >
> > > > For example, the mmc core has a loop re-trying with a lower
> > > > initialization frequency for the card (400KHz, 300KHz, 200KHz,
> > > > 100KHz). This will cause additional requests to the rtsx_pci_sdmmc driver.
> > > >
> > > > > Mmc driver will request the RTS5261 to power off the card, but at
> > > > > that time
> > > > power off the card will not succeed.
> > > >
> > > > Yes, assuming no card was found, the mmc core calls mmc_power_off().
> > > > Ths leads to the rtsx_pci_sdmmc driver's ->set_ios() callback being
> > > > invoked, requesting the card to be powered off. I don't see how you
> > > > are managing this, what am I missing?
> > > >
> > > Before power off card and re-trying initialization, rtsx driver sets RTS5261
> > 0xFF55 bit4=0.
> > > After set 0xFF55 bit4=0, RTS5261 can't receive any CMD from PCIe and
> > prepare for device disappear.
> > > Therefore, MMC driver can't change card status.
> >
> > Okay, so beyond that point - any calls to the interface that is provided from
> > drivers/misc/cardreader/rtsx_pcr will fail, when invoked by the
> > rtsx_pci_sdmmc driver?
> >
> Yes.
>
> > To me, that sounds a bit fragile and it's also relying on a specific behaviour of
> > the RTS5261 card reader interface. I wonder if this could be considered as a
> > common behaviour...??
> >
> It's a feature proposal by realtek not common.

Yes, of course.

>
> > Perhaps it's better to teach the mmc core *more* about SD express cards.
> > Maybe add a new host ops for dealing with the specific CMD8 command and
> > make the mmc core to "bail out", rather than keep retrying the initialization. In
> > principle I think the core should accept that it may have found an SD express
> > card, then abort further communication with it. At least until the mmc host
> > indicates that a re-initialization of the card can be done, which could be through
> > a remove/re-probe, for example.
> >
> In SD7.x spec host should send CMD8 with bit20=1 and bit21=1 to ask card's PCIe availability.
> So the CMD8 is not specific for RTS5261, it's just newly defined in SD7.x spec.

Yes, of course.

So, there are two PCIe modes. 1.8V I/O (mandatory and corresponds to
bit20) and 1.2V I/O (optional and corresponds to bit21). It's
important that the mmc host informs the mmc core about it's
capabilities, so we can set the correct bits when sending CMD8.

What do your host support?

> The mmc core will request host to power off card and has a loop re-trying with different initialization frequency for the card (400KHz, 300KHz, 200KHz,
> 100KHz), if I don't modify mmc core, I can't stop the power off and re-trying, if I modify mmc core, RTS5261 will become a special case for mmc core.
> So make the operation fail is the minimum modification in mmc core for me. Do you have any other suggestion?

Along the lines of what I suggested above. I think the mmc core should
stop sending commands beyond the CMD8, if the card responds to support
PCIe.

>
> > >
> > > > As stated above, I assume you the corresponding platform device for
> > > > rtsx_pci_sdmmc being deleted and thus triggering the
> > > > rtsx_pci_sdmmc_drv_remove() being called. Correct? If not, how does
> > > > the driver manage this?
> > > >
> > > Yes.
> > >
> > > > > When the card is unplugged, RTS5261 will switch to SD mode by
> > > > > itself and don't need mmc driver to do anything,
> > > >
> > > > Okay.
> > > >
> > > > So that means the rtsx_pci_sdmmc driver is being probed again?
> > > >
> > > Yes.
> > >
> > > > > If you run lspci, you can see NVMe device disappeared and RTS5261
> > > > > appeared
> > > > again.
> > > >
> > > > I see.
> > > >
> >
> > If you need some help on the mmc core parts, I am willing to help out.
> > However, first, I would like to get some better understanding of who and why
> > the PCI device is deleted.
> >
> Can I stop the re-trying in host driver other than modify mmc core?

We need to modify the core, but let me try to help in regards to that.
I will post some patches within a couple of days and keep you posted.

Let's see how this goes.

> As above, I'm sorry I can't explain much more details about hardware design.

Sure, it's okay.

Kind regards
Uffe

答复: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[冯锐]

> 
> On Tue, 2 Jun 2020 at 04:41, 冯锐 <rui_feng@realsil.com.cn> wrote:
> >
> > >
> > > +linux-mmc
> > >
> > > On Mon, 1 Jun 2020 at 09:34, 冯锐 <rui_feng@realsil.com.cn> wrote:
> > > >
> > > > >
> > > > > On Tue, 19 May 2020 at 11:18, 冯锐 <rui_feng@realsil.com.cn> wrote:
> > > > > >
> > > > > > > On Tue, 28 Apr 2020 at 05:44, 冯锐 <rui_feng@realsil.com.cn>
> wrote:
> > > > > > > >
> > > > > > > > >
> > > > > > > > > On Mon, Apr 27, 2020 at 11:41 AM 冯锐
> > > > > > > > > <rui_feng@realsil.com.cn>
> > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > On Sun, Apr 26, 2020 at 09:25:46AM +0800,
> > > > > > > > > > > rui_feng@realsil.com.cn
> > > > > > > > > wrote:
> > > > > > > > > > > > From: Rui Feng <rui_feng@realsil.com.cn>
> > > > > > > > > > > >
> > > > > > > > > > > > RTS5261 support legacy SD mode and SD Express mode.
> > > > > > > > > > > > In SD7.x, SD association introduce SD Express as a new
> mode.
> > > > > > > > > > > > SD Express mode is distinguished by CMD8.
> > > > > > > > > > > > Therefore, CMD8 has new bit for SD Express.
> > > > > > > > > > > > SD Express is based on PCIe/NVMe.
> > > > > > > > > > > > RTS5261 uses CMD8 to switch to SD Express mode.
> > > > > > > > > > >
> > > > > > > > > > > So how does this bit work?  They way I imagined SD
> > > > > > > > > > > Express to work is that the actual SD Card just
> > > > > > > > > > > shows up as a real PCIe device, similar to say Thunderbolt.
> > > > > > > > > >
> > > > > > > > > > New SD Express card has dual mode. One is SD mode and
> > > > > > > > > > another is PCIe
> > > > > > > > > mode.
> > > > > > > > > > In PCIe mode, it act as a PCIe device and use PCIe
> > > > > > > > > > protocol not Thunderbolt
> > > > > > > > > protocol.
> > > > > > > > >
> > > > > > > > > I think what Christoph was asking about is why you need
> > > > > > > > > to issue any commands at all in SD mode when you want to
> > > > > > > > > use PCIe mode
> > > > > instead.
> > > > > > > > > What happens if you load the NVMe dthriver before
> > > > > > > > > loading the
> > > > > > > > > rts5261
> > > > > > > driver?
> > > > > > > > >
> > > > > > > > >        Arnd
> > > > > > > > >
> > > > > > > > > ------Please consider the environment before printing this e-mail.
> > > > > > > >
> > > > > > > > RTS5261 support SD mode and PCIe/NVMe mode. The workflow
> > > > > > > > is as
> > > > > follows.
> > > > > > > > 1.RTS5261 work in SD mode.
> > > > > > > > 2.If card is plugged in, Host send CMD8 to ask card's PCIe
> availability.
> > > > > > >
> > > > > > > This sounds like the card insert/removal needs to be managed
> > > > > > > by the rtsx_pci_sdmmc driver (mmc).
> > > > > > >
> > > > > > > > 3.If the card has PCIe availability, RTS5261 switch to
> > > > > > > > PCIe/NVMe
> > > mode.
> > > > > > >
> > > > > > > This switch is done by the mmc driver, but how does the
> > > > > > > PCIe/NVMe driver know when to take over? Isn't there a
> > > synchronization point needed?
> > > > > > >
> > > > > > > > 4.Mmc driver exit and NVMe driver start working.
> > > > > > >
> > > > > > > Having the mmc driver to exit seems wrong to me. Else how
> > > > > > > would you handle a card being removed and inserted again?
> > > > > > >
> > > > > > > In principle you want the mmc core to fail to detect the
> > > > > > > card and then do a handover, somehow. No?
> > > > > > >
> > > > > > > Although, to make this work there are a couple of problems
> > > > > > > you need to deal with.
> > > > > > >
> > > > > > > 1. If the mmc core doesn't successfully detect a card, it
> > > > > > > will request the mmc host to power off the card. In this
> > > > > > > situation, you want to keep the power to the card, but leave
> > > > > > > it to be managed by the
> > > > > PCIe/NVMe driver in some way.
> > > > > > >
> > > > > > > 2. During system resume, the mmc core may try to restore
> > > > > > > power for a card, especially if it's a removable slot, as to
> > > > > > > make sure it gets detected if someone inserted a card while
> > > > > > > the system was
> > > suspended.
> > > > > > > Not sure if this plays well with the PCIe/NVMe driver's behaviour.
> > > > > > > Again, I think some kind of synchronization is needed.
> > > > > > >
> > > > > > > > 5.If card is unplugged, RTS5261 will switch to SD mode.
> > > > > > >
> > > > > > > Alright, clearly the mmc driver is needed to manage card
> > > insert/removal.
> > > > > > >
> > > > > > > > We should send CMD8 in SD mode to ask card's PCIe
> > > > > > > > availability, and the
> > > > > > > order of NVMe driver and rts5261 driver doesn't matter.
> > > > > > >
> > > > > > > That assumes there's another synchronization mechanism.
> > > > > > > Maybe there is, but I don't understand how.
> > > > > > >
> > > > > > If no card in RTS5261, RTS5261 works in SD mode. If you run
> > > > > > command lspci,
> > > > > you can see the RTS5261 device.
> > > > >
> > > > > Right.
> > > > >
> > > > > The rtsx_pci_driver (drivers/misc/cardreader/rtsx_pcr.c) has
> > > > > registered itself as a pci driver and been probed successfully,
> > > > > I assume. Then during
> > > > > rtsx_pci_probe() an mfd device is added via mfd_add_devices(),
> > > > > which corresponds to the rtsx_pci_sdmmc
> > > > > (drivers/mmc/host/rtsx_pci_sdmmc.c) platform driver.
> > > > >
> > > > > > When insert a SD Express card, Mmc driver will send CMD8 to
> > > > > > ask the card's PCIe availability, because it's a SD EXPRESS
> > > > > > card,
> > > > >
> > > > > Okay, so this will then be a part of the rtsx_pci_sdmmc driver's
> > > > > probe
> > > sequence.
> > > > > Or more exactly, when rtsx_pci_sdmmc_drv_probe() completes
> > > > > successfully, a mmc rescan work becomes scheduled to try to
> > > > > detect an SD/MMC card. Then the CMD8 command is sent...
> > > > >
> > > > > > RTS5261 will switch to NVMe mode, after switch if you run
> > > > > > lspci, you can see
> > > > > RTS5261 disappeared and a NVMe device replaces RTS5261.
> > > > >
> > > > > Can you elaborate more exactly how this managed?
> > > > >
> > > > > It kind of sounds like the original PCI device is being deleted?
> > > > > How is this managed?
> > > > >
> > > > > In any case, the rtsx_pci_driver's ->remove() callback,
> > > > > rtsx_pci_remove(), should be invoked, I assume?
> > > > >
> > > > > That would then lead to that mfd_remove_devices() gets called,
> > > > > which makes the ->remove() callback of the rtsx_pci_sdmmc
> > > > > driver, rtsx_pci_sdmmc_drv_remove(), to be invoked. Correct?
> > > > >
> > > > Yes, after RTS5261 switch to NVMe mode, rtsx_pci_remove() and
> > > rtsx_pci_sdmmc_drv_remove() will be invoked.
> > >
> > > So, the ->remove() callbacks are invoked because the PCI device that
> > > corresponds to the rtsx_pci_driver is being deleted. Can you explain
> > > who deletes the PCI device and why?
> > >
> > > I am not a PCI expert, so apologize for my ignorance - but I really
> > > want to understand how this is supposed to work.
> > >
> > Rtsx host driver sets RTS5261 0xFF54 bit0=1 and 0xFF55 bit4=0, then RTS5261
> will switch MCU and enter SD EXPRESS mode.
> > Because hardware design is involved, sorry I can't explain much more details
> about that.
> 
> Okay, so somehow that will trigger the PCI bus to remove the corresponding
> PCI device, I guess.
> 
> >
> > > >
> > > > > > In NVMe mode, RTS5261 only provide a bridge between SD Express
> > > > > > card and
> > > > > PCIe. For NVMe driver, just like a new NVMe device is inserted.
> > > > >
> > > > > I don't understand what that means, but I am also not an expert
> > > > > on
> > > PCI/NVMe.
> > > > > Care to explain more?
> > > > >
> > > > In NVMe mode, SD Express card connect the computer via PCIe.
> > > > IN SD mode, card connect computer via reader.
> > >
> > > That didn't make better sense to me, sorry. I do know about the SD
> > > spec and the SD-express card protocol parts. Anyway, let's leave this for
> now.
> > >
> > > >
> > > > > > Mmc core doesn't successfully detect the card and handover to
> > > > > > NVMe driver. Because of detect the card failed,
> > > > >
> > > > > How do you make sure that the rtsx_pci_sdmmc driver is leaving
> > > > > the card in the correct state for NVMe?
> > > > >
> > > > > For example, the mmc core has a loop re-trying with a lower
> > > > > initialization frequency for the card (400KHz, 300KHz, 200KHz,
> > > > > 100KHz). This will cause additional requests to the rtsx_pci_sdmmc
> driver.
> > > > >
> > > > > > Mmc driver will request the RTS5261 to power off the card, but
> > > > > > at that time
> > > > > power off the card will not succeed.
> > > > >
> > > > > Yes, assuming no card was found, the mmc core calls mmc_power_off().
> > > > > Ths leads to the rtsx_pci_sdmmc driver's ->set_ios() callback
> > > > > being invoked, requesting the card to be powered off. I don't
> > > > > see how you are managing this, what am I missing?
> > > > >
> > > > Before power off card and re-trying initialization, rtsx driver
> > > > sets RTS5261
> > > 0xFF55 bit4=0.
> > > > After set 0xFF55 bit4=0, RTS5261 can't receive any CMD from PCIe
> > > > and
> > > prepare for device disappear.
> > > > Therefore, MMC driver can't change card status.
> > >
> > > Okay, so beyond that point - any calls to the interface that is
> > > provided from drivers/misc/cardreader/rtsx_pcr will fail, when
> > > invoked by the rtsx_pci_sdmmc driver?
> > >
> > Yes.
> >
> > > To me, that sounds a bit fragile and it's also relying on a specific
> > > behaviour of the RTS5261 card reader interface. I wonder if this
> > > could be considered as a common behaviour...??
> > >
> > It's a feature proposal by realtek not common.
> 
> Yes, of course.
> 
> >
> > > Perhaps it's better to teach the mmc core *more* about SD express cards.
> > > Maybe add a new host ops for dealing with the specific CMD8 command
> > > and make the mmc core to "bail out", rather than keep retrying the
> > > initialization. In principle I think the core should accept that it
> > > may have found an SD express card, then abort further communication
> > > with it. At least until the mmc host indicates that a
> > > re-initialization of the card can be done, which could be through a
> remove/re-probe, for example.
> > >
> > In SD7.x spec host should send CMD8 with bit20=1 and bit21=1 to ask card's
> PCIe availability.
> > So the CMD8 is not specific for RTS5261, it's just newly defined in SD7.x spec.
> 
> Yes, of course.
> 
> So, there are two PCIe modes. 1.8V I/O (mandatory and corresponds to
> bit20) and 1.2V I/O (optional and corresponds to bit21). It's important that the
> mmc host informs the mmc core about it's capabilities, so we can set the
> correct bits when sending CMD8.
> 
> What do your host support?
> 
RTS5261 support VDD2(1.8V) and VDD3(1.2V), in rtsx host driver only bit20 is checked,
if bit20 is set in card's response host driver will make RTS5261 switch to SD Express mode
and bit21 is checked by hardware.

> > The mmc core will request host to power off card and has a loop
> > re-trying with different initialization frequency for the card (400KHz, 300KHz,
> 200KHz, 100KHz), if I don't modify mmc core, I can't stop the power off and
> re-trying, if I modify mmc core, RTS5261 will become a special case for mmc
> core.
> > So make the operation fail is the minimum modification in mmc core for me.
> Do you have any other suggestion?
> 
> Along the lines of what I suggested above. I think the mmc core should stop
> sending commands beyond the CMD8, if the card responds to support PCIe.
> 
> >
> > > >
> > > > > As stated above, I assume you the corresponding platform device
> > > > > for rtsx_pci_sdmmc being deleted and thus triggering the
> > > > > rtsx_pci_sdmmc_drv_remove() being called. Correct? If not, how
> > > > > does the driver manage this?
> > > > >
> > > > Yes.
> > > >
> > > > > > When the card is unplugged, RTS5261 will switch to SD mode by
> > > > > > itself and don't need mmc driver to do anything,
> > > > >
> > > > > Okay.
> > > > >
> > > > > So that means the rtsx_pci_sdmmc driver is being probed again?
> > > > >
> > > > Yes.
> > > >
> > > > > > If you run lspci, you can see NVMe device disappeared and
> > > > > > RTS5261 appeared
> > > > > again.
> > > > >
> > > > > I see.
> > > > >
> > >
> > > If you need some help on the mmc core parts, I am willing to help out.
> > > However, first, I would like to get some better understanding of who
> > > and why the PCI device is deleted.
> > >
> > Can I stop the re-trying in host driver other than modify mmc core?
> 
> We need to modify the core, but let me try to help in regards to that.
> I will post some patches within a couple of days and keep you posted.
> 
> Let's see how this goes.
> 
Thanks for your help and looking forward to your patches.

> > As above, I'm sorry I can't explain much more details about hardware design.
> 
> Sure, it's okay.
> 

Kind regards

答复: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[冯锐]

Hi Hansson:

I'm sorry to bother you. One month ago you said you will post some patches and keep my posted,
but I can't found the patches or I miss the patches? Users are looking forward to the patch, If you
are busy, I'll post a patch to let the retry in mmc core to do nothing just return in rtsx host driver.

Kind regards

> On Tue, 2 Jun 2020 at 04:41, 冯锐 <rui_feng@realsil.com.cn> wrote:
> >
> > >
> > > +linux-mmc
> > >
> > > On Mon, 1 Jun 2020 at 09:34, 冯锐 <rui_feng@realsil.com.cn> wrote:
> > > >
> > > > >
> > > > > On Tue, 19 May 2020 at 11:18, 冯锐 <rui_feng@realsil.com.cn> wrote:
> > > > > >
> > > > > > > On Tue, 28 Apr 2020 at 05:44, 冯锐 <rui_feng@realsil.com.cn>
> wrote:
> > > > > > > >
> > > > > > > > >
> > > > > > > > > On Mon, Apr 27, 2020 at 11:41 AM 冯锐
> > > > > > > > > <rui_feng@realsil.com.cn>
> > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > On Sun, Apr 26, 2020 at 09:25:46AM +0800,
> > > > > > > > > > > rui_feng@realsil.com.cn
> > > > > > > > > wrote:
> > > > > > > > > > > > From: Rui Feng <rui_feng@realsil.com.cn>
> > > > > > > > > > > >
> > > > > > > > > > > > RTS5261 support legacy SD mode and SD Express mode.
> > > > > > > > > > > > In SD7.x, SD association introduce SD Express as a new
> mode.
> > > > > > > > > > > > SD Express mode is distinguished by CMD8.
> > > > > > > > > > > > Therefore, CMD8 has new bit for SD Express.
> > > > > > > > > > > > SD Express is based on PCIe/NVMe.
> > > > > > > > > > > > RTS5261 uses CMD8 to switch to SD Express mode.
> > > > > > > > > > >
> > > > > > > > > > > So how does this bit work?  They way I imagined SD
> > > > > > > > > > > Express to work is that the actual SD Card just
> > > > > > > > > > > shows up as a real PCIe device, similar to say Thunderbolt.
> > > > > > > > > >
> > > > > > > > > > New SD Express card has dual mode. One is SD mode and
> > > > > > > > > > another is PCIe
> > > > > > > > > mode.
> > > > > > > > > > In PCIe mode, it act as a PCIe device and use PCIe
> > > > > > > > > > protocol not Thunderbolt
> > > > > > > > > protocol.
> > > > > > > > >
> > > > > > > > > I think what Christoph was asking about is why you need
> > > > > > > > > to issue any commands at all in SD mode when you want to
> > > > > > > > > use PCIe mode
> > > > > instead.
> > > > > > > > > What happens if you load the NVMe dthriver before
> > > > > > > > > loading the
> > > > > > > > > rts5261
> > > > > > > driver?
> > > > > > > > >
> > > > > > > > >        Arnd
> > > > > > > > >
> > > > > > > > > ------Please consider the environment before printing this e-mail.
> > > > > > > >
> > > > > > > > RTS5261 support SD mode and PCIe/NVMe mode. The workflow
> > > > > > > > is as
> > > > > follows.
> > > > > > > > 1.RTS5261 work in SD mode.
> > > > > > > > 2.If card is plugged in, Host send CMD8 to ask card's PCIe
> availability.
> > > > > > >
> > > > > > > This sounds like the card insert/removal needs to be managed
> > > > > > > by the rtsx_pci_sdmmc driver (mmc).
> > > > > > >
> > > > > > > > 3.If the card has PCIe availability, RTS5261 switch to
> > > > > > > > PCIe/NVMe
> > > mode.
> > > > > > >
> > > > > > > This switch is done by the mmc driver, but how does the
> > > > > > > PCIe/NVMe driver know when to take over? Isn't there a
> > > synchronization point needed?
> > > > > > >
> > > > > > > > 4.Mmc driver exit and NVMe driver start working.
> > > > > > >
> > > > > > > Having the mmc driver to exit seems wrong to me. Else how
> > > > > > > would you handle a card being removed and inserted again?
> > > > > > >
> > > > > > > In principle you want the mmc core to fail to detect the
> > > > > > > card and then do a handover, somehow. No?
> > > > > > >
> > > > > > > Although, to make this work there are a couple of problems
> > > > > > > you need to deal with.
> > > > > > >
> > > > > > > 1. If the mmc core doesn't successfully detect a card, it
> > > > > > > will request the mmc host to power off the card. In this
> > > > > > > situation, you want to keep the power to the card, but leave
> > > > > > > it to be managed by the
> > > > > PCIe/NVMe driver in some way.
> > > > > > >
> > > > > > > 2. During system resume, the mmc core may try to restore
> > > > > > > power for a card, especially if it's a removable slot, as to
> > > > > > > make sure it gets detected if someone inserted a card while
> > > > > > > the system was
> > > suspended.
> > > > > > > Not sure if this plays well with the PCIe/NVMe driver's behaviour.
> > > > > > > Again, I think some kind of synchronization is needed.
> > > > > > >
> > > > > > > > 5.If card is unplugged, RTS5261 will switch to SD mode.
> > > > > > >
> > > > > > > Alright, clearly the mmc driver is needed to manage card
> > > insert/removal.
> > > > > > >
> > > > > > > > We should send CMD8 in SD mode to ask card's PCIe
> > > > > > > > availability, and the
> > > > > > > order of NVMe driver and rts5261 driver doesn't matter.
> > > > > > >
> > > > > > > That assumes there's another synchronization mechanism.
> > > > > > > Maybe there is, but I don't understand how.
> > > > > > >
> > > > > > If no card in RTS5261, RTS5261 works in SD mode. If you run
> > > > > > command lspci,
> > > > > you can see the RTS5261 device.
> > > > >
> > > > > Right.
> > > > >
> > > > > The rtsx_pci_driver (drivers/misc/cardreader/rtsx_pcr.c) has
> > > > > registered itself as a pci driver and been probed successfully,
> > > > > I assume. Then during
> > > > > rtsx_pci_probe() an mfd device is added via mfd_add_devices(),
> > > > > which corresponds to the rtsx_pci_sdmmc
> > > > > (drivers/mmc/host/rtsx_pci_sdmmc.c) platform driver.
> > > > >
> > > > > > When insert a SD Express card, Mmc driver will send CMD8 to
> > > > > > ask the card's PCIe availability, because it's a SD EXPRESS
> > > > > > card,
> > > > >
> > > > > Okay, so this will then be a part of the rtsx_pci_sdmmc driver's
> > > > > probe
> > > sequence.
> > > > > Or more exactly, when rtsx_pci_sdmmc_drv_probe() completes
> > > > > successfully, a mmc rescan work becomes scheduled to try to
> > > > > detect an SD/MMC card. Then the CMD8 command is sent...
> > > > >
> > > > > > RTS5261 will switch to NVMe mode, after switch if you run
> > > > > > lspci, you can see
> > > > > RTS5261 disappeared and a NVMe device replaces RTS5261.
> > > > >
> > > > > Can you elaborate more exactly how this managed?
> > > > >
> > > > > It kind of sounds like the original PCI device is being deleted?
> > > > > How is this managed?
> > > > >
> > > > > In any case, the rtsx_pci_driver's ->remove() callback,
> > > > > rtsx_pci_remove(), should be invoked, I assume?
> > > > >
> > > > > That would then lead to that mfd_remove_devices() gets called,
> > > > > which makes the ->remove() callback of the rtsx_pci_sdmmc
> > > > > driver, rtsx_pci_sdmmc_drv_remove(), to be invoked. Correct?
> > > > >
> > > > Yes, after RTS5261 switch to NVMe mode, rtsx_pci_remove() and
> > > rtsx_pci_sdmmc_drv_remove() will be invoked.
> > >
> > > So, the ->remove() callbacks are invoked because the PCI device that
> > > corresponds to the rtsx_pci_driver is being deleted. Can you explain
> > > who deletes the PCI device and why?
> > >
> > > I am not a PCI expert, so apologize for my ignorance - but I really
> > > want to understand how this is supposed to work.
> > >
> > Rtsx host driver sets RTS5261 0xFF54 bit0=1 and 0xFF55 bit4=0, then RTS5261
> will switch MCU and enter SD EXPRESS mode.
> > Because hardware design is involved, sorry I can't explain much more details
> about that.
> 
> Okay, so somehow that will trigger the PCI bus to remove the corresponding
> PCI device, I guess.
> 
> >
> > > >
> > > > > > In NVMe mode, RTS5261 only provide a bridge between SD Express
> > > > > > card and
> > > > > PCIe. For NVMe driver, just like a new NVMe device is inserted.
> > > > >
> > > > > I don't understand what that means, but I am also not an expert
> > > > > on
> > > PCI/NVMe.
> > > > > Care to explain more?
> > > > >
> > > > In NVMe mode, SD Express card connect the computer via PCIe.
> > > > IN SD mode, card connect computer via reader.
> > >
> > > That didn't make better sense to me, sorry. I do know about the SD
> > > spec and the SD-express card protocol parts. Anyway, let's leave this for
> now.
> > >
> > > >
> > > > > > Mmc core doesn't successfully detect the card and handover to
> > > > > > NVMe driver. Because of detect the card failed,
> > > > >
> > > > > How do you make sure that the rtsx_pci_sdmmc driver is leaving
> > > > > the card in the correct state for NVMe?
> > > > >
> > > > > For example, the mmc core has a loop re-trying with a lower
> > > > > initialization frequency for the card (400KHz, 300KHz, 200KHz,
> > > > > 100KHz). This will cause additional requests to the rtsx_pci_sdmmc
> driver.
> > > > >
> > > > > > Mmc driver will request the RTS5261 to power off the card, but
> > > > > > at that time
> > > > > power off the card will not succeed.
> > > > >
> > > > > Yes, assuming no card was found, the mmc core calls mmc_power_off().
> > > > > Ths leads to the rtsx_pci_sdmmc driver's ->set_ios() callback
> > > > > being invoked, requesting the card to be powered off. I don't
> > > > > see how you are managing this, what am I missing?
> > > > >
> > > > Before power off card and re-trying initialization, rtsx driver
> > > > sets RTS5261
> > > 0xFF55 bit4=0.
> > > > After set 0xFF55 bit4=0, RTS5261 can't receive any CMD from PCIe
> > > > and
> > > prepare for device disappear.
> > > > Therefore, MMC driver can't change card status.
> > >
> > > Okay, so beyond that point - any calls to the interface that is
> > > provided from drivers/misc/cardreader/rtsx_pcr will fail, when
> > > invoked by the rtsx_pci_sdmmc driver?
> > >
> > Yes.
> >
> > > To me, that sounds a bit fragile and it's also relying on a specific
> > > behaviour of the RTS5261 card reader interface. I wonder if this
> > > could be considered as a common behaviour...??
> > >
> > It's a feature proposal by realtek not common.
> 
> Yes, of course.
> 
> >
> > > Perhaps it's better to teach the mmc core *more* about SD express cards.
> > > Maybe add a new host ops for dealing with the specific CMD8 command
> > > and make the mmc core to "bail out", rather than keep retrying the
> > > initialization. In principle I think the core should accept that it
> > > may have found an SD express card, then abort further communication
> > > with it. At least until the mmc host indicates that a
> > > re-initialization of the card can be done, which could be through a
> remove/re-probe, for example.
> > >
> > In SD7.x spec host should send CMD8 with bit20=1 and bit21=1 to ask card's
> PCIe availability.
> > So the CMD8 is not specific for RTS5261, it's just newly defined in SD7.x spec.
> 
> Yes, of course.
> 
> So, there are two PCIe modes. 1.8V I/O (mandatory and corresponds to
> bit20) and 1.2V I/O (optional and corresponds to bit21). It's important that the
> mmc host informs the mmc core about it's capabilities, so we can set the
> correct bits when sending CMD8.
> 
> What do your host support?
> 
> > The mmc core will request host to power off card and has a loop
> > re-trying with different initialization frequency for the card (400KHz, 300KHz,
> 200KHz, 100KHz), if I don't modify mmc core, I can't stop the power off and
> re-trying, if I modify mmc core, RTS5261 will become a special case for mmc
> core.
> > So make the operation fail is the minimum modification in mmc core for me.
> Do you have any other suggestion?
> 
> Along the lines of what I suggested above. I think the mmc core should stop
> sending commands beyond the CMD8, if the card responds to support PCIe.
> 
> >
> > > >
> > > > > As stated above, I assume you the corresponding platform device
> > > > > for rtsx_pci_sdmmc being deleted and thus triggering the
> > > > > rtsx_pci_sdmmc_drv_remove() being called. Correct? If not, how
> > > > > does the driver manage this?
> > > > >
> > > > Yes.
> > > >
> > > > > > When the card is unplugged, RTS5261 will switch to SD mode by
> > > > > > itself and don't need mmc driver to do anything,
> > > > >
> > > > > Okay.
> > > > >
> > > > > So that means the rtsx_pci_sdmmc driver is being probed again?
> > > > >
> > > > Yes.
> > > >
> > > > > > If you run lspci, you can see NVMe device disappeared and
> > > > > > RTS5261 appeared
> > > > > again.
> > > > >
> > > > > I see.
> > > > >
> > >
> > > If you need some help on the mmc core parts, I am willing to help out.
> > > However, first, I would like to get some better understanding of who
> > > and why the PCI device is deleted.
> > >
> > Can I stop the re-trying in host driver other than modify mmc core?
> 
> We need to modify the core, but let me try to help in regards to that.
> I will post some patches within a couple of days and keep you posted.
> 
> Let's see how this goes.
> 
Hi 
> > As above, I'm sorry I can't explain much more details about hardware design.
> 
> Sure, it's okay.
> 
> Kind regards
> Uffe

Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[Ulf Hansson]

On Wed, 1 Jul 2020 at 11:52, 冯锐 <rui_feng@realsil.com.cn> wrote:
>
> Hi Hansson:
>
> I'm sorry to bother you. One month ago you said you will post some patches and keep my posted,
> but I can't found the patches or I miss the patches? Users are looking forward to the patch, If you
> are busy, I'll post a patch to let the retry in mmc core to do nothing just return in rtsx host driver.

Apologize for the delay. It turned out to be a little more complex
than I first thought. Also, I got my hands on the "Part A2 SD Host
Controller Specification Ver7.00 Draft", which I would like to have a
closer look at before posting a patch.

I will try my best to get something submitted this week (and I will
keep you in the loop, of course).

Kind regards
Uffe

>
> Kind regards
>
> > On Tue, 2 Jun 2020 at 04:41, 冯锐 <rui_feng@realsil.com.cn> wrote:
> > >
> > > >
> > > > +linux-mmc
> > > >
> > > > On Mon, 1 Jun 2020 at 09:34, 冯锐 <rui_feng@realsil.com.cn> wrote:
> > > > >
> > > > > >
> > > > > > On Tue, 19 May 2020 at 11:18, 冯锐 <rui_feng@realsil.com.cn> wrote:
> > > > > > >
> > > > > > > > On Tue, 28 Apr 2020 at 05:44, 冯锐 <rui_feng@realsil.com.cn>
> > wrote:
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > On Mon, Apr 27, 2020 at 11:41 AM 冯锐
> > > > > > > > > > <rui_feng@realsil.com.cn>
> > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > > On Sun, Apr 26, 2020 at 09:25:46AM +0800,
> > > > > > > > > > > > rui_feng@realsil.com.cn
> > > > > > > > > > wrote:
> > > > > > > > > > > > > From: Rui Feng <rui_feng@realsil.com.cn>
> > > > > > > > > > > > >
> > > > > > > > > > > > > RTS5261 support legacy SD mode and SD Express mode.
> > > > > > > > > > > > > In SD7.x, SD association introduce SD Express as a new
> > mode.
> > > > > > > > > > > > > SD Express mode is distinguished by CMD8.
> > > > > > > > > > > > > Therefore, CMD8 has new bit for SD Express.
> > > > > > > > > > > > > SD Express is based on PCIe/NVMe.
> > > > > > > > > > > > > RTS5261 uses CMD8 to switch to SD Express mode.
> > > > > > > > > > > >
> > > > > > > > > > > > So how does this bit work?  They way I imagined SD
> > > > > > > > > > > > Express to work is that the actual SD Card just
> > > > > > > > > > > > shows up as a real PCIe device, similar to say Thunderbolt.
> > > > > > > > > > >
> > > > > > > > > > > New SD Express card has dual mode. One is SD mode and
> > > > > > > > > > > another is PCIe
> > > > > > > > > > mode.
> > > > > > > > > > > In PCIe mode, it act as a PCIe device and use PCIe
> > > > > > > > > > > protocol not Thunderbolt
> > > > > > > > > > protocol.
> > > > > > > > > >
> > > > > > > > > > I think what Christoph was asking about is why you need
> > > > > > > > > > to issue any commands at all in SD mode when you want to
> > > > > > > > > > use PCIe mode
> > > > > > instead.
> > > > > > > > > > What happens if you load the NVMe dthriver before
> > > > > > > > > > loading the
> > > > > > > > > > rts5261
> > > > > > > > driver?
> > > > > > > > > >
> > > > > > > > > >        Arnd
> > > > > > > > > >
> > > > > > > > > > ------Please consider the environment before printing this e-mail.
> > > > > > > > >
> > > > > > > > > RTS5261 support SD mode and PCIe/NVMe mode. The workflow
> > > > > > > > > is as
> > > > > > follows.
> > > > > > > > > 1.RTS5261 work in SD mode.
> > > > > > > > > 2.If card is plugged in, Host send CMD8 to ask card's PCIe
> > availability.
> > > > > > > >
> > > > > > > > This sounds like the card insert/removal needs to be managed
> > > > > > > > by the rtsx_pci_sdmmc driver (mmc).
> > > > > > > >
> > > > > > > > > 3.If the card has PCIe availability, RTS5261 switch to
> > > > > > > > > PCIe/NVMe
> > > > mode.
> > > > > > > >
> > > > > > > > This switch is done by the mmc driver, but how does the
> > > > > > > > PCIe/NVMe driver know when to take over? Isn't there a
> > > > synchronization point needed?
> > > > > > > >
> > > > > > > > > 4.Mmc driver exit and NVMe driver start working.
> > > > > > > >
> > > > > > > > Having the mmc driver to exit seems wrong to me. Else how
> > > > > > > > would you handle a card being removed and inserted again?
> > > > > > > >
> > > > > > > > In principle you want the mmc core to fail to detect the
> > > > > > > > card and then do a handover, somehow. No?
> > > > > > > >
> > > > > > > > Although, to make this work there are a couple of problems
> > > > > > > > you need to deal with.
> > > > > > > >
> > > > > > > > 1. If the mmc core doesn't successfully detect a card, it
> > > > > > > > will request the mmc host to power off the card. In this
> > > > > > > > situation, you want to keep the power to the card, but leave
> > > > > > > > it to be managed by the
> > > > > > PCIe/NVMe driver in some way.
> > > > > > > >
> > > > > > > > 2. During system resume, the mmc core may try to restore
> > > > > > > > power for a card, especially if it's a removable slot, as to
> > > > > > > > make sure it gets detected if someone inserted a card while
> > > > > > > > the system was
> > > > suspended.
> > > > > > > > Not sure if this plays well with the PCIe/NVMe driver's behaviour.
> > > > > > > > Again, I think some kind of synchronization is needed.
> > > > > > > >
> > > > > > > > > 5.If card is unplugged, RTS5261 will switch to SD mode.
> > > > > > > >
> > > > > > > > Alright, clearly the mmc driver is needed to manage card
> > > > insert/removal.
> > > > > > > >
> > > > > > > > > We should send CMD8 in SD mode to ask card's PCIe
> > > > > > > > > availability, and the
> > > > > > > > order of NVMe driver and rts5261 driver doesn't matter.
> > > > > > > >
> > > > > > > > That assumes there's another synchronization mechanism.
> > > > > > > > Maybe there is, but I don't understand how.
> > > > > > > >
> > > > > > > If no card in RTS5261, RTS5261 works in SD mode. If you run
> > > > > > > command lspci,
> > > > > > you can see the RTS5261 device.
> > > > > >
> > > > > > Right.
> > > > > >
> > > > > > The rtsx_pci_driver (drivers/misc/cardreader/rtsx_pcr.c) has
> > > > > > registered itself as a pci driver and been probed successfully,
> > > > > > I assume. Then during
> > > > > > rtsx_pci_probe() an mfd device is added via mfd_add_devices(),
> > > > > > which corresponds to the rtsx_pci_sdmmc
> > > > > > (drivers/mmc/host/rtsx_pci_sdmmc.c) platform driver.
> > > > > >
> > > > > > > When insert a SD Express card, Mmc driver will send CMD8 to
> > > > > > > ask the card's PCIe availability, because it's a SD EXPRESS
> > > > > > > card,
> > > > > >
> > > > > > Okay, so this will then be a part of the rtsx_pci_sdmmc driver's
> > > > > > probe
> > > > sequence.
> > > > > > Or more exactly, when rtsx_pci_sdmmc_drv_probe() completes
> > > > > > successfully, a mmc rescan work becomes scheduled to try to
> > > > > > detect an SD/MMC card. Then the CMD8 command is sent...
> > > > > >
> > > > > > > RTS5261 will switch to NVMe mode, after switch if you run
> > > > > > > lspci, you can see
> > > > > > RTS5261 disappeared and a NVMe device replaces RTS5261.
> > > > > >
> > > > > > Can you elaborate more exactly how this managed?
> > > > > >
> > > > > > It kind of sounds like the original PCI device is being deleted?
> > > > > > How is this managed?
> > > > > >
> > > > > > In any case, the rtsx_pci_driver's ->remove() callback,
> > > > > > rtsx_pci_remove(), should be invoked, I assume?
> > > > > >
> > > > > > That would then lead to that mfd_remove_devices() gets called,
> > > > > > which makes the ->remove() callback of the rtsx_pci_sdmmc
> > > > > > driver, rtsx_pci_sdmmc_drv_remove(), to be invoked. Correct?
> > > > > >
> > > > > Yes, after RTS5261 switch to NVMe mode, rtsx_pci_remove() and
> > > > rtsx_pci_sdmmc_drv_remove() will be invoked.
> > > >
> > > > So, the ->remove() callbacks are invoked because the PCI device that
> > > > corresponds to the rtsx_pci_driver is being deleted. Can you explain
> > > > who deletes the PCI device and why?
> > > >
> > > > I am not a PCI expert, so apologize for my ignorance - but I really
> > > > want to understand how this is supposed to work.
> > > >
> > > Rtsx host driver sets RTS5261 0xFF54 bit0=1 and 0xFF55 bit4=0, then RTS5261
> > will switch MCU and enter SD EXPRESS mode.
> > > Because hardware design is involved, sorry I can't explain much more details
> > about that.
> >
> > Okay, so somehow that will trigger the PCI bus to remove the corresponding
> > PCI device, I guess.
> >
> > >
> > > > >
> > > > > > > In NVMe mode, RTS5261 only provide a bridge between SD Express
> > > > > > > card and
> > > > > > PCIe. For NVMe driver, just like a new NVMe device is inserted.
> > > > > >
> > > > > > I don't understand what that means, but I am also not an expert
> > > > > > on
> > > > PCI/NVMe.
> > > > > > Care to explain more?
> > > > > >
> > > > > In NVMe mode, SD Express card connect the computer via PCIe.
> > > > > IN SD mode, card connect computer via reader.
> > > >
> > > > That didn't make better sense to me, sorry. I do know about the SD
> > > > spec and the SD-express card protocol parts. Anyway, let's leave this for
> > now.
> > > >
> > > > >
> > > > > > > Mmc core doesn't successfully detect the card and handover to
> > > > > > > NVMe driver. Because of detect the card failed,
> > > > > >
> > > > > > How do you make sure that the rtsx_pci_sdmmc driver is leaving
> > > > > > the card in the correct state for NVMe?
> > > > > >
> > > > > > For example, the mmc core has a loop re-trying with a lower
> > > > > > initialization frequency for the card (400KHz, 300KHz, 200KHz,
> > > > > > 100KHz). This will cause additional requests to the rtsx_pci_sdmmc
> > driver.
> > > > > >
> > > > > > > Mmc driver will request the RTS5261 to power off the card, but
> > > > > > > at that time
> > > > > > power off the card will not succeed.
> > > > > >
> > > > > > Yes, assuming no card was found, the mmc core calls mmc_power_off().
> > > > > > Ths leads to the rtsx_pci_sdmmc driver's ->set_ios() callback
> > > > > > being invoked, requesting the card to be powered off. I don't
> > > > > > see how you are managing this, what am I missing?
> > > > > >
> > > > > Before power off card and re-trying initialization, rtsx driver
> > > > > sets RTS5261
> > > > 0xFF55 bit4=0.
> > > > > After set 0xFF55 bit4=0, RTS5261 can't receive any CMD from PCIe
> > > > > and
> > > > prepare for device disappear.
> > > > > Therefore, MMC driver can't change card status.
> > > >
> > > > Okay, so beyond that point - any calls to the interface that is
> > > > provided from drivers/misc/cardreader/rtsx_pcr will fail, when
> > > > invoked by the rtsx_pci_sdmmc driver?
> > > >
> > > Yes.
> > >
> > > > To me, that sounds a bit fragile and it's also relying on a specific
> > > > behaviour of the RTS5261 card reader interface. I wonder if this
> > > > could be considered as a common behaviour...??
> > > >
> > > It's a feature proposal by realtek not common.
> >
> > Yes, of course.
> >
> > >
> > > > Perhaps it's better to teach the mmc core *more* about SD express cards.
> > > > Maybe add a new host ops for dealing with the specific CMD8 command
> > > > and make the mmc core to "bail out", rather than keep retrying the
> > > > initialization. In principle I think the core should accept that it
> > > > may have found an SD express card, then abort further communication
> > > > with it. At least until the mmc host indicates that a
> > > > re-initialization of the card can be done, which could be through a
> > remove/re-probe, for example.
> > > >
> > > In SD7.x spec host should send CMD8 with bit20=1 and bit21=1 to ask card's
> > PCIe availability.
> > > So the CMD8 is not specific for RTS5261, it's just newly defined in SD7.x spec.
> >
> > Yes, of course.
> >
> > So, there are two PCIe modes. 1.8V I/O (mandatory and corresponds to
> > bit20) and 1.2V I/O (optional and corresponds to bit21). It's important that the
> > mmc host informs the mmc core about it's capabilities, so we can set the
> > correct bits when sending CMD8.
> >
> > What do your host support?
> >
> > > The mmc core will request host to power off card and has a loop
> > > re-trying with different initialization frequency for the card (400KHz, 300KHz,
> > 200KHz, 100KHz), if I don't modify mmc core, I can't stop the power off and
> > re-trying, if I modify mmc core, RTS5261 will become a special case for mmc
> > core.
> > > So make the operation fail is the minimum modification in mmc core for me.
> > Do you have any other suggestion?
> >
> > Along the lines of what I suggested above. I think the mmc core should stop
> > sending commands beyond the CMD8, if the card responds to support PCIe.
> >
> > >
> > > > >
> > > > > > As stated above, I assume you the corresponding platform device
> > > > > > for rtsx_pci_sdmmc being deleted and thus triggering the
> > > > > > rtsx_pci_sdmmc_drv_remove() being called. Correct? If not, how
> > > > > > does the driver manage this?
> > > > > >
> > > > > Yes.
> > > > >
> > > > > > > When the card is unplugged, RTS5261 will switch to SD mode by
> > > > > > > itself and don't need mmc driver to do anything,
> > > > > >
> > > > > > Okay.
> > > > > >
> > > > > > So that means the rtsx_pci_sdmmc driver is being probed again?
> > > > > >
> > > > > Yes.
> > > > >
> > > > > > > If you run lspci, you can see NVMe device disappeared and
> > > > > > > RTS5261 appeared
> > > > > > again.
> > > > > >
> > > > > > I see.
> > > > > >
> > > >
> > > > If you need some help on the mmc core parts, I am willing to help out.
> > > > However, first, I would like to get some better understanding of who
> > > > and why the PCI device is deleted.
> > > >
> > > Can I stop the re-trying in host driver other than modify mmc core?
> >
> > We need to modify the core, but let me try to help in regards to that.
> > I will post some patches within a couple of days and keep you posted.
> >
> > Let's see how this goes.
> >
> Hi
> > > As above, I'm sorry I can't explain much more details about hardware design.
> >
> > Sure, it's okay.
> >
> > Kind regards
> > Uffe

Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[kernel test robot]

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

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on soc/for-next linus/master v5.9-rc6 next-20200924]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/rui_feng-realsil-com-cn/mmc-rtsx-Add-SD-Express-mode-support-for-RTS5261/20200924-154122
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git 26ed5146bd17cbcd0fb84e358902ac244728a3f3
config: riscv-allyesconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=riscv 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/mmc/host/rtsx_pci_sdmmc.c: In function 'sd_power_on':
   drivers/mmc/host/rtsx_pci_sdmmc.c:931:20: error: 'MMC_CAP2_SD_EXP' undeclared (first use in this function); did you mean 'MMC_CAP2_NO_SD'?
     931 |    mmc->caps2 &= ~(MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V);
         |                    ^~~~~~~~~~~~~~~
         |                    MMC_CAP2_NO_SD
   drivers/mmc/host/rtsx_pci_sdmmc.c:931:20: note: each undeclared identifier is reported only once for each function it appears in
   drivers/mmc/host/rtsx_pci_sdmmc.c:931:38: error: 'MMC_CAP2_SD_EXP_1_2V' undeclared (first use in this function); did you mean 'MMC_CAP2_HS400_1_2V'?
     931 |    mmc->caps2 &= ~(MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V);
         |                                      ^~~~~~~~~~~~~~~~~~~~
         |                                      MMC_CAP2_HS400_1_2V
   drivers/mmc/host/rtsx_pci_sdmmc.c: In function 'sdmmc_get_cd':
   drivers/mmc/host/rtsx_pci_sdmmc.c:1141:17: error: 'MMC_CAP2_SD_EXP' undeclared (first use in this function); did you mean 'MMC_CAP2_NO_SD'?
    1141 |   mmc->caps2 |= MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V;
         |                 ^~~~~~~~~~~~~~~
         |                 MMC_CAP2_NO_SD
   drivers/mmc/host/rtsx_pci_sdmmc.c:1141:35: error: 'MMC_CAP2_SD_EXP_1_2V' undeclared (first use in this function); did you mean 'MMC_CAP2_HS400_1_2V'?
    1141 |   mmc->caps2 |= MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V;
         |                                   ^~~~~~~~~~~~~~~~~~~~
         |                                   MMC_CAP2_HS400_1_2V
   drivers/mmc/host/rtsx_pci_sdmmc.c: At top level:
   drivers/mmc/host/rtsx_pci_sdmmc.c:1376:3: error: 'const struct mmc_host_ops' has no member named 'init_sd_express'
    1376 |  .init_sd_express = sdmmc_init_sd_express,
         |   ^~~~~~~~~~~~~~~
>> drivers/mmc/host/rtsx_pci_sdmmc.c:1376:21: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
    1376 |  .init_sd_express = sdmmc_init_sd_express,
         |                     ^~~~~~~~~~~~~~~~~~~~~
   drivers/mmc/host/rtsx_pci_sdmmc.c:1376:21: note: (near initialization for 'realtek_pci_sdmmc_ops')
   drivers/mmc/host/rtsx_pci_sdmmc.c:1376:21: warning: excess elements in struct initializer
   drivers/mmc/host/rtsx_pci_sdmmc.c:1376:21: note: (near initialization for 'realtek_pci_sdmmc_ops')
   drivers/mmc/host/rtsx_pci_sdmmc.c: In function 'init_extra_caps':
   drivers/mmc/host/rtsx_pci_sdmmc.c:1399:17: error: 'MMC_CAP2_SD_EXP' undeclared (first use in this function); did you mean 'MMC_CAP2_NO_SD'?
    1399 |   mmc->caps2 |= MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V;
         |                 ^~~~~~~~~~~~~~~
         |                 MMC_CAP2_NO_SD
   drivers/mmc/host/rtsx_pci_sdmmc.c:1399:35: error: 'MMC_CAP2_SD_EXP_1_2V' undeclared (first use in this function); did you mean 'MMC_CAP2_HS400_1_2V'?
    1399 |   mmc->caps2 |= MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V;
         |                                   ^~~~~~~~~~~~~~~~~~~~
         |                                   MMC_CAP2_HS400_1_2V
   cc1: some warnings being treated as errors

# https://github.com/0day-ci/linux/commit/37daa224f78ef228349cee981d690b735fb9bb2b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review rui_feng-realsil-com-cn/mmc-rtsx-Add-SD-Express-mode-support-for-RTS5261/20200924-154122
git checkout 37daa224f78ef228349cee981d690b735fb9bb2b
vim +1376 drivers/mmc/host/rtsx_pci_sdmmc.c

  1366	
  1367	static const struct mmc_host_ops realtek_pci_sdmmc_ops = {
  1368		.pre_req = sdmmc_pre_req,
  1369		.post_req = sdmmc_post_req,
  1370		.request = sdmmc_request,
  1371		.set_ios = sdmmc_set_ios,
  1372		.get_ro = sdmmc_get_ro,
  1373		.get_cd = sdmmc_get_cd,
  1374		.start_signal_voltage_switch = sdmmc_switch_voltage,
  1375		.execute_tuning = sdmmc_execute_tuning,
> 1376		.init_sd_express = sdmmc_init_sd_express,
  1377	};
  1378	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 66135 bytes --]

Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[Ulf Hansson]

On Thu, 24 Sep 2020 at 12:01, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Thu, 24 Sep 2020 at 11:48, 冯锐 <rui_feng@realsil.com.cn> wrote:
> >
> > Hi Hansson,
> >
> > This patch is based on your patch "mmc: core: Initial support for SD express card/host",
> > If this patch is compiled alone, there must be errors.
> > What should I do in this situation?
>
> You need to pick my patch from the patchtracker or the mailing-list
> and fold into your submission as part of a series. In this way the
> 0-build server will build patches stacked on top of each other.
>
> Moreover, if possible, I would suggest to split cardreader changes
> from mmc host changes. In this way you would get a series along the
> lines of below (not sure what order is best).
>
> PATCH 1/3: mmc core changes.
> PATCH 2/3. cardreader changes.
> PATCH 3/3. mmc host changes.
>
> Kind regards
> Uffe

One more thing, please send the series to linux-mmc as well.

[...]

Kind regards
Uffe

Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[kernel test robot]

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

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on soc/for-next linus/master v5.9-rc6 next-20200923]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/rui_feng-realsil-com-cn/mmc-rtsx-Add-SD-Express-mode-support-for-RTS5261/20200924-154122
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git 26ed5146bd17cbcd0fb84e358902ac244728a3f3
config: s390-randconfig-r003-20200923 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project d6ac649ccda289ecc2d2c0cb51892d57e8ec328c)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   include/uapi/linux/swab.h:19:12: note: expanded from macro '___constant_swab32'
           (((__u32)(x) & (__u32)0x000000ffUL) << 24) |            \
                     ^
   In file included from drivers/mmc/host/rtsx_pci_sdmmc.c:21:
   In file included from include/linux/rtsx_pci.h:14:
   In file included from include/linux/pci.h:39:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:72:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
           ___constant_swab32(x) :                 \
                              ^
   include/uapi/linux/swab.h:20:12: note: expanded from macro '___constant_swab32'
           (((__u32)(x) & (__u32)0x0000ff00UL) <<  8) |            \
                     ^
   In file included from drivers/mmc/host/rtsx_pci_sdmmc.c:21:
   In file included from include/linux/rtsx_pci.h:14:
   In file included from include/linux/pci.h:39:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:72:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
           ___constant_swab32(x) :                 \
                              ^
   include/uapi/linux/swab.h:21:12: note: expanded from macro '___constant_swab32'
           (((__u32)(x) & (__u32)0x00ff0000UL) >>  8) |            \
                     ^
   In file included from drivers/mmc/host/rtsx_pci_sdmmc.c:21:
   In file included from include/linux/rtsx_pci.h:14:
   In file included from include/linux/pci.h:39:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:72:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
           ___constant_swab32(x) :                 \
                              ^
   include/uapi/linux/swab.h:22:12: note: expanded from macro '___constant_swab32'
           (((__u32)(x) & (__u32)0xff000000UL) >> 24)))
                     ^
   In file included from drivers/mmc/host/rtsx_pci_sdmmc.c:21:
   In file included from include/linux/rtsx_pci.h:14:
   In file included from include/linux/pci.h:39:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:72:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:120:12: note: expanded from macro '__swab32'
           __fswab32(x))
                     ^
   In file included from drivers/mmc/host/rtsx_pci_sdmmc.c:21:
   In file included from include/linux/rtsx_pci.h:14:
   In file included from include/linux/pci.h:39:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:72:
   include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsb(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsw(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsl(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesb(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesw(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesl(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
>> drivers/mmc/host/rtsx_pci_sdmmc.c:931:20: error: use of undeclared identifier 'MMC_CAP2_SD_EXP'
                           mmc->caps2 &= ~(MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V);
                                           ^
>> drivers/mmc/host/rtsx_pci_sdmmc.c:931:38: error: use of undeclared identifier 'MMC_CAP2_SD_EXP_1_2V'
                           mmc->caps2 &= ~(MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V);
                                                             ^
   drivers/mmc/host/rtsx_pci_sdmmc.c:1141:17: error: use of undeclared identifier 'MMC_CAP2_SD_EXP'
                   mmc->caps2 |= MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V;
                                 ^
   drivers/mmc/host/rtsx_pci_sdmmc.c:1141:35: error: use of undeclared identifier 'MMC_CAP2_SD_EXP_1_2V'
                   mmc->caps2 |= MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V;
                                                   ^
>> drivers/mmc/host/rtsx_pci_sdmmc.c:1376:3: error: field designator 'init_sd_express' does not refer to any field in type 'const struct mmc_host_ops'
           .init_sd_express = sdmmc_init_sd_express,
            ^
   drivers/mmc/host/rtsx_pci_sdmmc.c:1399:17: error: use of undeclared identifier 'MMC_CAP2_SD_EXP'
                   mmc->caps2 |= MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V;
                                 ^
   drivers/mmc/host/rtsx_pci_sdmmc.c:1399:35: error: use of undeclared identifier 'MMC_CAP2_SD_EXP_1_2V'
                   mmc->caps2 |= MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V;
                                                   ^
   20 warnings and 7 errors generated.

# https://github.com/0day-ci/linux/commit/37daa224f78ef228349cee981d690b735fb9bb2b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review rui_feng-realsil-com-cn/mmc-rtsx-Add-SD-Express-mode-support-for-RTS5261/20200924-154122
git checkout 37daa224f78ef228349cee981d690b735fb9bb2b
vim +/MMC_CAP2_SD_EXP +931 drivers/mmc/host/rtsx_pci_sdmmc.c

   894	
   895	static int sd_power_on(struct realtek_pci_sdmmc *host)
   896	{
   897		struct rtsx_pcr *pcr = host->pcr;
   898		struct mmc_host *mmc = host->mmc;
   899		int err;
   900		u32 val;
   901	
   902		if (host->power_state == SDMMC_POWER_ON)
   903			return 0;
   904	
   905		rtsx_pci_init_cmd(pcr);
   906		rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, CARD_SELECT, 0x07, SD_MOD_SEL);
   907		rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, CARD_SHARE_MODE,
   908				CARD_SHARE_MASK, CARD_SHARE_48_SD);
   909		rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, CARD_CLK_EN,
   910				SD_CLK_EN, SD_CLK_EN);
   911		err = rtsx_pci_send_cmd(pcr, 100);
   912		if (err < 0)
   913			return err;
   914	
   915		err = rtsx_pci_card_pull_ctl_enable(pcr, RTSX_SD_CARD);
   916		if (err < 0)
   917			return err;
   918	
   919		err = rtsx_pci_card_power_on(pcr, RTSX_SD_CARD);
   920		if (err < 0)
   921			return err;
   922	
   923		err = rtsx_pci_write_register(pcr, CARD_OE, SD_OUTPUT_EN, SD_OUTPUT_EN);
   924		if (err < 0)
   925			return err;
   926	
   927		if (PCI_PID(pcr) == PID_5261) {
   928			val = rtsx_pci_readl(pcr, RTSX_BIPR);
   929			if (val & SD_WRITE_PROTECT) {
   930				pcr->extra_caps &= ~EXTRA_CAPS_SD_EXPRESS;
 > 931				mmc->caps2 &= ~(MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V);
   932			}
   933		}
   934	
   935		host->power_state = SDMMC_POWER_ON;
   936		return 0;
   937	}
   938	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 29570 bytes --]

Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[Ulf Hansson]

On Thu, 24 Sep 2020 at 11:48, 冯锐 <rui_feng@realsil.com.cn> wrote:
>
> Hi Hansson,
>
> This patch is based on your patch "mmc: core: Initial support for SD express card/host",
> If this patch is compiled alone, there must be errors.
> What should I do in this situation?

You need to pick my patch from the patchtracker or the mailing-list
and fold into your submission as part of a series. In this way the
0-build server will build patches stacked on top of each other.

Moreover, if possible, I would suggest to split cardreader changes
from mmc host changes. In this way you would get a series along the
lines of below (not sure what order is best).

PATCH 1/3: mmc core changes.
PATCH 2/3. cardreader changes.
PATCH 3/3. mmc host changes.

Kind regards
Uffe

>
> Thanks
>
> >
> > Thank you for the patch! Yet something to improve:
> >
> > [auto build test ERROR on char-misc/char-misc-testing] [also build test ERROR
> > on soc/for-next linus/master v5.9-rc6 next-20200923] [If your patch is applied
> > to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch]
> >
> > url:
> > https://github.com/0day-ci/linux/commits/rui_feng-realsil-com-cn/mmc-rtsx-A
> > dd-SD-Express-mode-support-for-RTS5261/20200924-154122
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
> > 26ed5146bd17cbcd0fb84e358902ac244728a3f3
> > config: arc-allyesconfig (attached as .config)
> > compiler: arceb-elf-gcc (GCC) 9.3.0
> > reproduce (this is a W=1 build):
> >         wget
> > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
> > ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # save the attached .config to linux build tree
> >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0
> > make.cross ARCH=arc
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> >
> > All errors (new ones prefixed by >>):
> >
> >    drivers/mmc/host/rtsx_pci_sdmmc.c: In function 'sd_power_on':
> > >> drivers/mmc/host/rtsx_pci_sdmmc.c:931:20: error: 'MMC_CAP2_SD_EXP'
> > undeclared (first use in this function); did you mean 'MMC_CAP2_NO_SD'?
> >      931 |    mmc->caps2 &= ~(MMC_CAP2_SD_EXP |
> > MMC_CAP2_SD_EXP_1_2V);
> >          |                    ^~~~~~~~~~~~~~~
> >          |                    MMC_CAP2_NO_SD
> >    drivers/mmc/host/rtsx_pci_sdmmc.c:931:20: note: each undeclared
> > identifier is reported only once for each function it appears in
> > >> drivers/mmc/host/rtsx_pci_sdmmc.c:931:38: error:
> > 'MMC_CAP2_SD_EXP_1_2V' undeclared (first use in this function); did you
> > mean 'MMC_CAP2_HS400_1_2V'?
> >      931 |    mmc->caps2 &= ~(MMC_CAP2_SD_EXP |
> > MMC_CAP2_SD_EXP_1_2V);
> >          |
> > ^~~~~~~~~~~~~~~~~~~~
> >          |
> > MMC_CAP2_HS400_1_2V
> >    drivers/mmc/host/rtsx_pci_sdmmc.c: In function 'sdmmc_get_cd':
> >    drivers/mmc/host/rtsx_pci_sdmmc.c:1141:17: error: 'MMC_CAP2_SD_EXP'
> > undeclared (first use in this function); did you mean 'MMC_CAP2_NO_SD'?
> >     1141 |   mmc->caps2 |= MMC_CAP2_SD_EXP |
> > MMC_CAP2_SD_EXP_1_2V;
> >          |                 ^~~~~~~~~~~~~~~
> >          |                 MMC_CAP2_NO_SD
> >    drivers/mmc/host/rtsx_pci_sdmmc.c:1141:35: error:
> > 'MMC_CAP2_SD_EXP_1_2V' undeclared (first use in this function); did you
> > mean 'MMC_CAP2_HS400_1_2V'?
> >     1141 |   mmc->caps2 |= MMC_CAP2_SD_EXP |
> > MMC_CAP2_SD_EXP_1_2V;
> >          |
> > ^~~~~~~~~~~~~~~~~~~~
> >          |
> > MMC_CAP2_HS400_1_2V
> >    drivers/mmc/host/rtsx_pci_sdmmc.c: At top level:
> > >> drivers/mmc/host/rtsx_pci_sdmmc.c:1376:3: error: 'const struct
> > mmc_host_ops' has no member named 'init_sd_express'
> >     1376 |  .init_sd_express = sdmmc_init_sd_express,
> >          |   ^~~~~~~~~~~~~~~
> >    drivers/mmc/host/rtsx_pci_sdmmc.c: In function 'init_extra_caps':
> >    drivers/mmc/host/rtsx_pci_sdmmc.c:1399:17: error: 'MMC_CAP2_SD_EXP'
> > undeclared (first use in this function); did you mean 'MMC_CAP2_NO_SD'?
> >     1399 |   mmc->caps2 |= MMC_CAP2_SD_EXP |
> > MMC_CAP2_SD_EXP_1_2V;
> >          |                 ^~~~~~~~~~~~~~~
> >          |                 MMC_CAP2_NO_SD
> >    drivers/mmc/host/rtsx_pci_sdmmc.c:1399:35: error:
> > 'MMC_CAP2_SD_EXP_1_2V' undeclared (first use in this function); did you
> > mean 'MMC_CAP2_HS400_1_2V'?
> >     1399 |   mmc->caps2 |= MMC_CAP2_SD_EXP |
> > MMC_CAP2_SD_EXP_1_2V;
> >          |
> > ^~~~~~~~~~~~~~~~~~~~
> >          |
> > MMC_CAP2_HS400_1_2V
> >
> > #
> > https://github.com/0day-ci/linux/commit/37daa224f78ef228349cee981d690b7
> > 35fb9bb2b
> > git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags
> > linux-review
> > rui_feng-realsil-com-cn/mmc-rtsx-Add-SD-Express-mode-support-for-RTS5261/
> > 20200924-154122
> > git checkout 37daa224f78ef228349cee981d690b735fb9bb2b
> > vim +931 drivers/mmc/host/rtsx_pci_sdmmc.c
> >
> >    894
> >    895        static int sd_power_on(struct realtek_pci_sdmmc *host)
> >    896        {
> >    897                struct rtsx_pcr *pcr = host->pcr;
> >    898                struct mmc_host *mmc = host->mmc;
> >    899                int err;
> >    900                u32 val;
> >    901
> >    902                if (host->power_state == SDMMC_POWER_ON)
> >    903                        return 0;
> >    904
> >    905                rtsx_pci_init_cmd(pcr);
> >    906                rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, CARD_SELECT, 0x07,
> > SD_MOD_SEL);
> >    907                rtsx_pci_add_cmd(pcr, WRITE_REG_CMD,
> > CARD_SHARE_MODE,
> >    908                                CARD_SHARE_MASK, CARD_SHARE_48_SD);
> >    909                rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, CARD_CLK_EN,
> >    910                                SD_CLK_EN, SD_CLK_EN);
> >    911                err = rtsx_pci_send_cmd(pcr, 100);
> >    912                if (err < 0)
> >    913                        return err;
> >    914
> >    915                err = rtsx_pci_card_pull_ctl_enable(pcr, RTSX_SD_CARD);
> >    916                if (err < 0)
> >    917                        return err;
> >    918
> >    919                err = rtsx_pci_card_power_on(pcr, RTSX_SD_CARD);
> >    920                if (err < 0)
> >    921                        return err;
> >    922
> >    923                err = rtsx_pci_write_register(pcr, CARD_OE, SD_OUTPUT_EN,
> > SD_OUTPUT_EN);
> >    924                if (err < 0)
> >    925                        return err;
> >    926
> >    927                if (PCI_PID(pcr) == PID_5261) {
> >    928                        val = rtsx_pci_readl(pcr, RTSX_BIPR);
> >    929                        if (val & SD_WRITE_PROTECT) {
> >    930                                pcr->extra_caps &= ~EXTRA_CAPS_SD_EXPRESS;
> >  > 931                                mmc->caps2 &= ~(MMC_CAP2_SD_EXP |
> > MMC_CAP2_SD_EXP_1_2V);
> >    932                        }
> >    933                }
> >    934
> >    935                host->power_state = SDMMC_POWER_ON;
> >    936                return 0;
> >    937        }
> >    938
> >
> > ---
> > 0-DAY CI Kernel Test Service, Intel Corporation
> > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> >
> > ------Please consider the environment before printing this e-mail.

Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[kernel test robot]

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

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on soc/for-next linus/master v5.9-rc6 next-20200923]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/rui_feng-realsil-com-cn/mmc-rtsx-Add-SD-Express-mode-support-for-RTS5261/20200924-154122
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git 26ed5146bd17cbcd0fb84e358902ac244728a3f3
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/mmc/host/rtsx_pci_sdmmc.c: In function 'sd_power_on':
>> drivers/mmc/host/rtsx_pci_sdmmc.c:931:20: error: 'MMC_CAP2_SD_EXP' undeclared (first use in this function); did you mean 'MMC_CAP2_NO_SD'?
     931 |    mmc->caps2 &= ~(MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V);
         |                    ^~~~~~~~~~~~~~~
         |                    MMC_CAP2_NO_SD
   drivers/mmc/host/rtsx_pci_sdmmc.c:931:20: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/mmc/host/rtsx_pci_sdmmc.c:931:38: error: 'MMC_CAP2_SD_EXP_1_2V' undeclared (first use in this function); did you mean 'MMC_CAP2_HS400_1_2V'?
     931 |    mmc->caps2 &= ~(MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V);
         |                                      ^~~~~~~~~~~~~~~~~~~~
         |                                      MMC_CAP2_HS400_1_2V
   drivers/mmc/host/rtsx_pci_sdmmc.c: In function 'sdmmc_get_cd':
   drivers/mmc/host/rtsx_pci_sdmmc.c:1141:17: error: 'MMC_CAP2_SD_EXP' undeclared (first use in this function); did you mean 'MMC_CAP2_NO_SD'?
    1141 |   mmc->caps2 |= MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V;
         |                 ^~~~~~~~~~~~~~~
         |                 MMC_CAP2_NO_SD
   drivers/mmc/host/rtsx_pci_sdmmc.c:1141:35: error: 'MMC_CAP2_SD_EXP_1_2V' undeclared (first use in this function); did you mean 'MMC_CAP2_HS400_1_2V'?
    1141 |   mmc->caps2 |= MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V;
         |                                   ^~~~~~~~~~~~~~~~~~~~
         |                                   MMC_CAP2_HS400_1_2V
   drivers/mmc/host/rtsx_pci_sdmmc.c: At top level:
>> drivers/mmc/host/rtsx_pci_sdmmc.c:1376:3: error: 'const struct mmc_host_ops' has no member named 'init_sd_express'
    1376 |  .init_sd_express = sdmmc_init_sd_express,
         |   ^~~~~~~~~~~~~~~
   drivers/mmc/host/rtsx_pci_sdmmc.c: In function 'init_extra_caps':
   drivers/mmc/host/rtsx_pci_sdmmc.c:1399:17: error: 'MMC_CAP2_SD_EXP' undeclared (first use in this function); did you mean 'MMC_CAP2_NO_SD'?
    1399 |   mmc->caps2 |= MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V;
         |                 ^~~~~~~~~~~~~~~
         |                 MMC_CAP2_NO_SD
   drivers/mmc/host/rtsx_pci_sdmmc.c:1399:35: error: 'MMC_CAP2_SD_EXP_1_2V' undeclared (first use in this function); did you mean 'MMC_CAP2_HS400_1_2V'?
    1399 |   mmc->caps2 |= MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V;
         |                                   ^~~~~~~~~~~~~~~~~~~~
         |                                   MMC_CAP2_HS400_1_2V

# https://github.com/0day-ci/linux/commit/37daa224f78ef228349cee981d690b735fb9bb2b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review rui_feng-realsil-com-cn/mmc-rtsx-Add-SD-Express-mode-support-for-RTS5261/20200924-154122
git checkout 37daa224f78ef228349cee981d690b735fb9bb2b
vim +931 drivers/mmc/host/rtsx_pci_sdmmc.c

   894	
   895	static int sd_power_on(struct realtek_pci_sdmmc *host)
   896	{
   897		struct rtsx_pcr *pcr = host->pcr;
   898		struct mmc_host *mmc = host->mmc;
   899		int err;
   900		u32 val;
   901	
   902		if (host->power_state == SDMMC_POWER_ON)
   903			return 0;
   904	
   905		rtsx_pci_init_cmd(pcr);
   906		rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, CARD_SELECT, 0x07, SD_MOD_SEL);
   907		rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, CARD_SHARE_MODE,
   908				CARD_SHARE_MASK, CARD_SHARE_48_SD);
   909		rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, CARD_CLK_EN,
   910				SD_CLK_EN, SD_CLK_EN);
   911		err = rtsx_pci_send_cmd(pcr, 100);
   912		if (err < 0)
   913			return err;
   914	
   915		err = rtsx_pci_card_pull_ctl_enable(pcr, RTSX_SD_CARD);
   916		if (err < 0)
   917			return err;
   918	
   919		err = rtsx_pci_card_power_on(pcr, RTSX_SD_CARD);
   920		if (err < 0)
   921			return err;
   922	
   923		err = rtsx_pci_write_register(pcr, CARD_OE, SD_OUTPUT_EN, SD_OUTPUT_EN);
   924		if (err < 0)
   925			return err;
   926	
   927		if (PCI_PID(pcr) == PID_5261) {
   928			val = rtsx_pci_readl(pcr, RTSX_BIPR);
   929			if (val & SD_WRITE_PROTECT) {
   930				pcr->extra_caps &= ~EXTRA_CAPS_SD_EXPRESS;
 > 931				mmc->caps2 &= ~(MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V);
   932			}
   933		}
   934	
   935		host->power_state = SDMMC_POWER_ON;
   936		return 0;
   937	}
   938	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 65336 bytes --]

Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[kernel test robot]

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

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on soc/for-next linus/master v5.9-rc6 next-20200923]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/rui_feng-realsil-com-cn/mmc-rtsx-Add-SD-Express-mode-support-for-RTS5261/20200924-154122
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git 26ed5146bd17cbcd0fb84e358902ac244728a3f3
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/mmc/host/rtsx_pci_sdmmc.c: In function 'sd_power_on':
>> drivers/mmc/host/rtsx_pci_sdmmc.c:931:20: error: 'MMC_CAP2_SD_EXP' undeclared (first use in this function); did you mean 'MMC_CAP2_NO_SD'?
     931 |    mmc->caps2 &= ~(MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V);
         |                    ^~~~~~~~~~~~~~~
         |                    MMC_CAP2_NO_SD
   drivers/mmc/host/rtsx_pci_sdmmc.c:931:20: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/mmc/host/rtsx_pci_sdmmc.c:931:38: error: 'MMC_CAP2_SD_EXP_1_2V' undeclared (first use in this function); did you mean 'MMC_CAP2_HS400_1_2V'?
     931 |    mmc->caps2 &= ~(MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V);
         |                                      ^~~~~~~~~~~~~~~~~~~~
         |                                      MMC_CAP2_HS400_1_2V
   drivers/mmc/host/rtsx_pci_sdmmc.c: In function 'sdmmc_get_cd':
   drivers/mmc/host/rtsx_pci_sdmmc.c:1141:17: error: 'MMC_CAP2_SD_EXP' undeclared (first use in this function); did you mean 'MMC_CAP2_NO_SD'?
    1141 |   mmc->caps2 |= MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V;
         |                 ^~~~~~~~~~~~~~~
         |                 MMC_CAP2_NO_SD
   drivers/mmc/host/rtsx_pci_sdmmc.c:1141:35: error: 'MMC_CAP2_SD_EXP_1_2V' undeclared (first use in this function); did you mean 'MMC_CAP2_HS400_1_2V'?
    1141 |   mmc->caps2 |= MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V;
         |                                   ^~~~~~~~~~~~~~~~~~~~
         |                                   MMC_CAP2_HS400_1_2V
   drivers/mmc/host/rtsx_pci_sdmmc.c: At top level:
>> drivers/mmc/host/rtsx_pci_sdmmc.c:1376:3: error: 'const struct mmc_host_ops' has no member named 'init_sd_express'
    1376 |  .init_sd_express = sdmmc_init_sd_express,
         |   ^~~~~~~~~~~~~~~
   drivers/mmc/host/rtsx_pci_sdmmc.c: In function 'init_extra_caps':
   drivers/mmc/host/rtsx_pci_sdmmc.c:1399:17: error: 'MMC_CAP2_SD_EXP' undeclared (first use in this function); did you mean 'MMC_CAP2_NO_SD'?
    1399 |   mmc->caps2 |= MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V;
         |                 ^~~~~~~~~~~~~~~
         |                 MMC_CAP2_NO_SD
   drivers/mmc/host/rtsx_pci_sdmmc.c:1399:35: error: 'MMC_CAP2_SD_EXP_1_2V' undeclared (first use in this function); did you mean 'MMC_CAP2_HS400_1_2V'?
    1399 |   mmc->caps2 |= MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V;
         |                                   ^~~~~~~~~~~~~~~~~~~~
         |                                   MMC_CAP2_HS400_1_2V

# https://github.com/0day-ci/linux/commit/37daa224f78ef228349cee981d690b735fb9bb2b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review rui_feng-realsil-com-cn/mmc-rtsx-Add-SD-Express-mode-support-for-RTS5261/20200924-154122
git checkout 37daa224f78ef228349cee981d690b735fb9bb2b
vim +931 drivers/mmc/host/rtsx_pci_sdmmc.c

   894	
   895	static int sd_power_on(struct realtek_pci_sdmmc *host)
   896	{
   897		struct rtsx_pcr *pcr = host->pcr;
   898		struct mmc_host *mmc = host->mmc;
   899		int err;
   900		u32 val;
   901	
   902		if (host->power_state == SDMMC_POWER_ON)
   903			return 0;
   904	
   905		rtsx_pci_init_cmd(pcr);
   906		rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, CARD_SELECT, 0x07, SD_MOD_SEL);
   907		rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, CARD_SHARE_MODE,
   908				CARD_SHARE_MASK, CARD_SHARE_48_SD);
   909		rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, CARD_CLK_EN,
   910				SD_CLK_EN, SD_CLK_EN);
   911		err = rtsx_pci_send_cmd(pcr, 100);
   912		if (err < 0)
   913			return err;
   914	
   915		err = rtsx_pci_card_pull_ctl_enable(pcr, RTSX_SD_CARD);
   916		if (err < 0)
   917			return err;
   918	
   919		err = rtsx_pci_card_power_on(pcr, RTSX_SD_CARD);
   920		if (err < 0)
   921			return err;
   922	
   923		err = rtsx_pci_write_register(pcr, CARD_OE, SD_OUTPUT_EN, SD_OUTPUT_EN);
   924		if (err < 0)
   925			return err;
   926	
   927		if (PCI_PID(pcr) == PID_5261) {
   928			val = rtsx_pci_readl(pcr, RTSX_BIPR);
   929			if (val & SD_WRITE_PROTECT) {
   930				pcr->extra_caps &= ~EXTRA_CAPS_SD_EXPRESS;
 > 931				mmc->caps2 &= ~(MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V);
   932			}
   933		}
   934	
   935		host->power_state = SDMMC_POWER_ON;
   936		return 0;
   937	}
   938	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 65336 bytes --]

[PATCH] mmc: rtsx: Add SD Express mode support for RTS5261

[rui_feng]

From: Rui Feng <rui_feng@realsil.com.cn>

RTS5261 support legacy SD mode and SD Express mode.
In SD7.x, SD association introduce SD Express as a new mode.
This patch makes RTS5261 support SD Express mode,
and this patch is based on patch "mmc: core: Initial support
for SD express card/host" committed by Ulf Hansson.

Signed-off-by: Rui Feng <rui_feng@realsil.com.cn>
---
 drivers/misc/cardreader/rts5261.c  |  4 ++
 drivers/misc/cardreader/rts5261.h  | 23 ------------
 drivers/misc/cardreader/rtsx_pcr.c |  5 +++
 drivers/mmc/host/rtsx_pci_sdmmc.c  | 59 ++++++++++++++++++++++++++++++
 include/linux/rtsx_pci.h           | 28 ++++++++++++++
 5 files changed, 96 insertions(+), 23 deletions(-)

diff --git a/drivers/misc/cardreader/rts5261.c b/drivers/misc/cardreader/rts5261.c
index 471961487ff8..536c90d4fd76 100644
--- a/drivers/misc/cardreader/rts5261.c
+++ b/drivers/misc/cardreader/rts5261.c
@@ -738,8 +738,12 @@ void rts5261_init_params(struct rtsx_pcr *pcr)
 {
 	struct rtsx_cr_option *option = &pcr->option;
 	struct rtsx_hw_param *hw_param = &pcr->hw_param;
+	u8 val;
 
 	pcr->extra_caps = EXTRA_CAPS_SD_SDR50 | EXTRA_CAPS_SD_SDR104;
+	rtsx_pci_read_register(pcr, RTS5261_FW_STATUS, &val);
+	if (!(val & RTS5261_EXPRESS_LINK_FAIL_MASK))
+		pcr->extra_caps |= EXTRA_CAPS_SD_EXPRESS;
 	pcr->num_slots = 1;
 	pcr->ops = &rts5261_pcr_ops;
 
diff --git a/drivers/misc/cardreader/rts5261.h b/drivers/misc/cardreader/rts5261.h
index ebfdd236a553..8d80f0d5d5d6 100644
--- a/drivers/misc/cardreader/rts5261.h
+++ b/drivers/misc/cardreader/rts5261.h
@@ -65,23 +65,6 @@
 #define RTS5261_FW_EXPRESS_TEST_MASK	(0x01<<0)
 #define RTS5261_FW_EA_MODE_MASK		(0x01<<5)
 
-/* FW config register */
-#define RTS5261_FW_CFG0			0xFF54
-#define RTS5261_FW_ENTER_EXPRESS	(0x01<<0)
-
-#define RTS5261_FW_CFG1			0xFF55
-#define RTS5261_SYS_CLK_SEL_MCU_CLK	(0x01<<7)
-#define RTS5261_CRC_CLK_SEL_MCU_CLK	(0x01<<6)
-#define RTS5261_FAKE_MCU_CLOCK_GATING	(0x01<<5)
-/*MCU_bus_mode_sel: 0=real 8051 1=fake mcu*/
-#define RTS5261_MCU_BUS_SEL_MASK	(0x01<<4)
-/*MCU_clock_sel:VerA 00=aux16M 01=aux400K 1x=REFCLK100M*/
-/*MCU_clock_sel:VerB 00=aux400K 01=aux16M 10=REFCLK100M*/
-#define RTS5261_MCU_CLOCK_SEL_MASK	(0x03<<2)
-#define RTS5261_MCU_CLOCK_SEL_16M	(0x01<<2)
-#define RTS5261_MCU_CLOCK_GATING	(0x01<<1)
-#define RTS5261_DRIVER_ENABLE_FW	(0x01<<0)
-
 /* FW status register */
 #define RTS5261_FW_STATUS		0xFF56
 #define RTS5261_EXPRESS_LINK_FAIL_MASK	(0x01<<7)
@@ -121,12 +104,6 @@
 #define RTS5261_DV3318_19		(0x04<<4)
 #define RTS5261_DV3318_33		(0x07<<4)
 
-#define RTS5261_LDO1_CFG0		0xFF72
-#define RTS5261_LDO1_OCP_THD_MASK	(0x07<<5)
-#define RTS5261_LDO1_OCP_EN		(0x01<<4)
-#define RTS5261_LDO1_OCP_LMT_THD_MASK	(0x03<<2)
-#define RTS5261_LDO1_OCP_LMT_EN		(0x01<<1)
-
 /* CRD6603-433 190319 request changed */
 #define RTS5261_LDO1_OCP_THD_740	(0x00<<5)
 #define RTS5261_LDO1_OCP_THD_800	(0x01<<5)
diff --git a/drivers/misc/cardreader/rtsx_pcr.c b/drivers/misc/cardreader/rtsx_pcr.c
index 37ccc67f4914..6e5c16b4b7d1 100644
--- a/drivers/misc/cardreader/rtsx_pcr.c
+++ b/drivers/misc/cardreader/rtsx_pcr.c
@@ -990,6 +990,11 @@ static irqreturn_t rtsx_pci_isr(int irq, void *dev_id)
 		} else {
 			pcr->card_removed |= SD_EXIST;
 			pcr->card_inserted &= ~SD_EXIST;
+			if (PCI_PID(pcr) == PID_5261) {
+				rtsx_pci_write_register(pcr, RTS5261_FW_STATUS,
+					RTS5261_EXPRESS_LINK_FAIL_MASK, 0);
+				pcr->extra_caps |= EXTRA_CAPS_SD_EXPRESS;
+			}
 		}
 		pcr->dma_error_count = 0;
 	}
diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c b/drivers/mmc/host/rtsx_pci_sdmmc.c
index 2763a376b054..efde374a4a5e 100644
--- a/drivers/mmc/host/rtsx_pci_sdmmc.c
+++ b/drivers/mmc/host/rtsx_pci_sdmmc.c
@@ -895,7 +895,9 @@ static int sd_set_bus_width(struct realtek_pci_sdmmc *host,
 static int sd_power_on(struct realtek_pci_sdmmc *host)
 {
 	struct rtsx_pcr *pcr = host->pcr;
+	struct mmc_host *mmc = host->mmc;
 	int err;
+	u32 val;
 
 	if (host->power_state == SDMMC_POWER_ON)
 		return 0;
@@ -922,6 +924,14 @@ static int sd_power_on(struct realtek_pci_sdmmc *host)
 	if (err < 0)
 		return err;
 
+	if (PCI_PID(pcr) == PID_5261) {
+		val = rtsx_pci_readl(pcr, RTSX_BIPR);
+		if (val & SD_WRITE_PROTECT) {
+			pcr->extra_caps &= ~EXTRA_CAPS_SD_EXPRESS;
+			mmc->caps2 &= ~(MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V);
+		}
+	}
+
 	host->power_state = SDMMC_POWER_ON;
 	return 0;
 }
@@ -1127,6 +1137,8 @@ static int sdmmc_get_cd(struct mmc_host *mmc)
 	if (val & SD_EXIST)
 		cd = 1;
 
+	if (pcr->extra_caps & EXTRA_CAPS_SD_EXPRESS)
+		mmc->caps2 |= MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V;
 	mutex_unlock(&pcr->pcr_mutex);
 
 	return cd;
@@ -1308,6 +1320,50 @@ static int sdmmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
 	return err;
 }
 
+static int sdmmc_init_sd_express(struct mmc_host *mmc, struct mmc_ios *ios)
+{
+	u32 relink_time, val;
+	struct realtek_pci_sdmmc *host = mmc_priv(mmc);
+	struct rtsx_pcr *pcr = host->pcr;
+
+	/*
+	 * If card has PCIe availability and WP if off,
+	 * reader switch to PCIe mode.
+	 */
+	val = rtsx_pci_readl(pcr, RTSX_BIPR);
+	if (!(val & SD_WRITE_PROTECT)) {
+		/* Set relink_time for changing to PCIe card */
+		relink_time = 0x8FFF;
+
+		rtsx_pci_write_register(pcr, 0xFF01, 0xFF, relink_time);
+		rtsx_pci_write_register(pcr, 0xFF02, 0xFF, relink_time >> 8);
+		rtsx_pci_write_register(pcr, 0xFF03, 0x01, relink_time >> 16);
+
+		rtsx_pci_write_register(pcr, PETXCFG, 0x80, 0x80);
+		rtsx_pci_write_register(pcr, LDO_VCC_CFG0,
+			RTS5261_LDO1_OCP_THD_MASK,
+			pcr->option.sd_800mA_ocp_thd);
+
+		if (pcr->ops->disable_auto_blink)
+			pcr->ops->disable_auto_blink(pcr);
+
+		/* For PCIe/NVMe mode can't enter delink issue */
+		pcr->hw_param.interrupt_en &= ~(SD_INT_EN);
+		rtsx_pci_writel(pcr, RTSX_BIER, pcr->hw_param.interrupt_en);
+
+		rtsx_pci_write_register(pcr, RTS5260_AUTOLOAD_CFG4,
+			RTS5261_AUX_CLK_16M_EN, RTS5261_AUX_CLK_16M_EN);
+		rtsx_pci_write_register(pcr, RTS5261_FW_CFG0,
+			RTS5261_FW_ENTER_EXPRESS, RTS5261_FW_ENTER_EXPRESS);
+		rtsx_pci_write_register(pcr, RTS5261_FW_CFG1,
+			RTS5261_MCU_BUS_SEL_MASK | RTS5261_MCU_CLOCK_SEL_MASK
+			| RTS5261_MCU_CLOCK_GATING | RTS5261_DRIVER_ENABLE_FW,
+			RTS5261_MCU_CLOCK_SEL_16M | RTS5261_MCU_CLOCK_GATING
+			| RTS5261_DRIVER_ENABLE_FW);
+	}
+	return 0;
+}
+
 static const struct mmc_host_ops realtek_pci_sdmmc_ops = {
 	.pre_req = sdmmc_pre_req,
 	.post_req = sdmmc_post_req,
@@ -1317,6 +1373,7 @@ static const struct mmc_host_ops realtek_pci_sdmmc_ops = {
 	.get_cd = sdmmc_get_cd,
 	.start_signal_voltage_switch = sdmmc_switch_voltage,
 	.execute_tuning = sdmmc_execute_tuning,
+	.init_sd_express = sdmmc_init_sd_express,
 };
 
 static void init_extra_caps(struct realtek_pci_sdmmc *host)
@@ -1338,6 +1395,8 @@ static void init_extra_caps(struct realtek_pci_sdmmc *host)
 		mmc->caps |= MMC_CAP_8_BIT_DATA;
 	if (pcr->extra_caps & EXTRA_CAPS_NO_MMC)
 		mmc->caps2 |= MMC_CAP2_NO_MMC;
+	if (pcr->extra_caps & EXTRA_CAPS_SD_EXPRESS)
+		mmc->caps2 |= MMC_CAP2_SD_EXP | MMC_CAP2_SD_EXP_1_2V;
 }
 
 static void realtek_init_host(struct realtek_pci_sdmmc *host)
diff --git a/include/linux/rtsx_pci.h b/include/linux/rtsx_pci.h
index 745f5e73f99a..cea8147e5992 100644
--- a/include/linux/rtsx_pci.h
+++ b/include/linux/rtsx_pci.h
@@ -658,6 +658,24 @@
 #define   PM_WAKE_EN			0x01
 #define PM_CTRL4			0xFF47
 
+#define RTS5261_FW_CFG0			0xFF54
+#define   RTS5261_FW_ENTER_EXPRESS	(0x01 << 0)
+
+#define RTS5261_FW_CFG1			0xFF55
+#define   RTS5261_SYS_CLK_SEL_MCU_CLK	(0x01 << 7)
+#define   RTS5261_CRC_CLK_SEL_MCU_CLK	(0x01 << 6)
+#define   RTS5261_FAKE_MCU_CLOCK_GATING	(0x01 << 5)
+#define   RTS5261_MCU_BUS_SEL_MASK	(0x01 << 4)
+#define   RTS5261_MCU_BUS_SEL_MASK	(0x01 << 4)
+#define   RTS5261_MCU_CLOCK_SEL_MASK	(0x03 << 2)
+#define   RTS5261_MCU_CLOCK_SEL_16M	(0x01 << 2)
+#define   RTS5261_MCU_CLOCK_GATING	(0x01 << 1)
+#define   RTS5261_DRIVER_ENABLE_FW	(0x01 << 0)
+#define   RTS5261_MCU_CLOCK_SEL_MASK	(0x03 << 2)
+#define   RTS5261_MCU_CLOCK_SEL_16M	(0x01 << 2)
+#define   RTS5261_MCU_CLOCK_GATING	(0x01 << 1)
+#define   RTS5261_DRIVER_ENABLE_FW	(0x01 << 0)
+
 #define REG_CFG_OOBS_OFF_TIMER 0xFEA6
 #define REG_CFG_OOBS_ON_TIMER 0xFEA7
 #define REG_CFG_VCM_ON_TIMER 0xFEA8
@@ -701,6 +719,13 @@
 #define   RTS5260_DVCC_TUNE_MASK	0x70
 #define   RTS5260_DVCC_33		0x70
 
+/*RTS5261*/
+#define RTS5261_LDO1_CFG0		0xFF72
+#define   RTS5261_LDO1_OCP_THD_MASK	(0x07 << 5)
+#define   RTS5261_LDO1_OCP_EN		(0x01 << 4)
+#define   RTS5261_LDO1_OCP_LMT_THD_MASK	(0x03 << 2)
+#define   RTS5261_LDO1_OCP_LMT_EN	(0x01 << 1)
+
 #define LDO_VCC_CFG1			0xFF73
 #define   LDO_VCC_REF_TUNE_MASK		0x30
 #define   LDO_VCC_REF_1V2		0x20
@@ -741,6 +766,8 @@
 
 #define RTS5260_AUTOLOAD_CFG4		0xFF7F
 #define   RTS5260_MIMO_DISABLE		0x8A
+/*RTS5261*/
+#define   RTS5261_AUX_CLK_16M_EN		(1 << 5)
 
 #define RTS5260_REG_GPIO_CTL0		0xFC1A
 #define   RTS5260_REG_GPIO_MASK		0x01
@@ -1191,6 +1218,7 @@ struct rtsx_pcr {
 #define EXTRA_CAPS_MMC_HS200		(1 << 4)
 #define EXTRA_CAPS_MMC_8BIT		(1 << 5)
 #define EXTRA_CAPS_NO_MMC		(1 << 7)
+#define EXTRA_CAPS_SD_EXPRESS		(1 << 8)
 	u32				extra_caps;
 
 #define IC_VER_A			0
-- 
2.17.1