* [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
@ 2020-09-24 7:40 rui_feng
2020-09-24 9:38 ` kernel test robot
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: rui_feng @ 2020-09-24 7:40 UTC (permalink / raw)
To: arnd, gregkh, ulf.hansson; +Cc: linux-kernel, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
2020-09-24 7:40 [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261 rui_feng
@ 2020-09-24 9:38 ` kernel test robot
2020-09-24 10:48 ` kernel test robot
2020-09-24 20:54 ` kernel test robot
2 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2020-09-24 9:38 UTC (permalink / raw)
To: rui_feng, arnd, gregkh, ulf.hansson; +Cc: kbuild-all, linux-kernel, Rui Feng
[-- 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 --]
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
@ 2020-09-24 9:38 ` kernel test robot
0 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2020-09-24 9:38 UTC (permalink / raw)
To: kbuild-all
[-- 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 --]
^ permalink raw reply [flat|nested] 17+ messages in thread* 答复: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
2020-09-24 9:38 ` kernel test robot
(?)
@ 2020-09-24 9:48 ` 冯锐
2020-09-24 10:01 ` Ulf Hansson
-1 siblings, 1 reply; 17+ messages in thread
From: 冯锐 @ 2020-09-24 9:48 UTC (permalink / raw)
To: ulf.hansson; +Cc: linux-kernel, arnd, gregkh
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?
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.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
2020-09-24 9:48 ` 答复: " 冯锐
@ 2020-09-24 10:01 ` Ulf Hansson
2020-09-24 11:15 ` Ulf Hansson
0 siblings, 1 reply; 17+ messages in thread
From: Ulf Hansson @ 2020-09-24 10:01 UTC (permalink / raw)
To: 冯锐; +Cc: linux-kernel, arnd, gregkh
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.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
2020-09-24 10:01 ` Ulf Hansson
@ 2020-09-24 11:15 ` Ulf Hansson
0 siblings, 0 replies; 17+ messages in thread
From: Ulf Hansson @ 2020-09-24 11:15 UTC (permalink / raw)
To: 冯锐; +Cc: linux-kernel, arnd, gregkh
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
2020-09-24 7:40 [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261 rui_feng
2020-09-24 9:38 ` kernel test robot
@ 2020-09-24 10:48 ` kernel test robot
2020-09-24 20:54 ` kernel test robot
2 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2020-09-24 10:48 UTC (permalink / raw)
To: kbuild-all
[-- 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 --]
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
2020-09-24 7:40 [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261 rui_feng
2020-09-24 9:38 ` kernel test robot
2020-09-24 10:48 ` kernel test robot
@ 2020-09-24 20:54 ` kernel test robot
2 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2020-09-24 20:54 UTC (permalink / raw)
To: kbuild-all
[-- 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 --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
@ 2020-04-26 1:25 rui_feng
2020-04-27 6:14 ` Christoph Hellwig
2020-05-05 14:28 ` Greg KH
0 siblings, 2 replies; 17+ messages in thread
From: rui_feng @ 2020-04-26 1:25 UTC (permalink / raw)
To: arnd, gregkh, ulf.hansson; +Cc: linux-kernel, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
2020-04-26 1:25 rui_feng
@ 2020-04-27 6:14 ` Christoph Hellwig
2020-04-27 9:40 ` 答复: " 冯锐
2020-05-05 14:28 ` Greg KH
1 sibling, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2020-04-27 6:14 UTC (permalink / raw)
To: rui_feng; +Cc: arnd, gregkh, ulf.hansson, linux-kernel, linux-pci
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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* 答复: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
2020-04-27 6:14 ` Christoph Hellwig
@ 2020-04-27 9:40 ` 冯锐
2020-04-27 11:07 ` Arnd Bergmann
0 siblings, 1 reply; 17+ messages in thread
From: 冯锐 @ 2020-04-27 9:40 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: arnd, gregkh, ulf.hansson, linux-kernel, linux-pci
> 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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
2020-04-27 9:40 ` 答复: " 冯锐
@ 2020-04-27 11:07 ` Arnd Bergmann
2020-04-28 3:44 ` 答复: " 冯锐
0 siblings, 1 reply; 17+ messages in thread
From: Arnd Bergmann @ 2020-04-27 11:07 UTC (permalink / raw)
To: 冯锐
Cc: Christoph Hellwig, gregkh, ulf.hansson, linux-kernel, linux-pci
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
^ permalink raw reply [flat|nested] 17+ messages in thread* 答复: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
2020-04-27 11:07 ` Arnd Bergmann
@ 2020-04-28 3:44 ` 冯锐
2020-05-05 18:18 ` Ulf Hansson
0 siblings, 1 reply; 17+ messages in thread
From: 冯锐 @ 2020-04-28 3:44 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Christoph Hellwig, gregkh, ulf.hansson, linux-kernel, linux-pci
>
> 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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
2020-04-28 3:44 ` 答复: " 冯锐
@ 2020-05-05 18:18 ` Ulf Hansson
2020-05-19 9:17 ` 答复: " 冯锐
0 siblings, 1 reply; 17+ messages in thread
From: Ulf Hansson @ 2020-05-05 18:18 UTC (permalink / raw)
To: 冯锐
Cc: Arnd Bergmann, Christoph Hellwig, gregkh, linux-kernel, linux-pci
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* 答复: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
2020-05-05 18:18 ` Ulf Hansson
@ 2020-05-19 9:17 ` 冯锐
2020-05-25 10:27 ` Ulf Hansson
0 siblings, 1 reply; 17+ messages in thread
From: 冯锐 @ 2020-05-19 9:17 UTC (permalink / raw)
To: Ulf Hansson
Cc: Arnd Bergmann, Christoph Hellwig, gregkh, linux-kernel, linux-pci
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
2020-05-19 9:17 ` 答复: " 冯锐
@ 2020-05-25 10:27 ` Ulf Hansson
2020-06-01 7:33 ` 答复: " 冯锐
0 siblings, 1 reply; 17+ messages in thread
From: Ulf Hansson @ 2020-05-25 10:27 UTC (permalink / raw)
To: 冯锐
Cc: Arnd Bergmann, Christoph Hellwig, gregkh, linux-kernel, linux-pci
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* 答复: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
2020-05-25 10:27 ` Ulf Hansson
@ 2020-06-01 7:33 ` 冯锐
2020-06-01 10:19 ` Ulf Hansson
0 siblings, 1 reply; 17+ messages in thread
From: 冯锐 @ 2020-06-01 7:33 UTC (permalink / raw)
To: Ulf Hansson
Cc: Arnd Bergmann, Christoph Hellwig, gregkh, linux-kernel, linux-pci
>
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
2020-06-01 7:33 ` 答复: " 冯锐
@ 2020-06-01 10:19 ` Ulf Hansson
2020-06-02 2:41 ` 答复: " 冯锐
0 siblings, 1 reply; 17+ messages in thread
From: Ulf Hansson @ 2020-06-01 10:19 UTC (permalink / raw)
To: 冯锐
Cc: Arnd Bergmann, Christoph Hellwig, gregkh, linux-kernel,
linux-pci, linux-mmc
+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
^ permalink raw reply [flat|nested] 17+ messages in thread
* 答复: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
2020-06-01 10:19 ` Ulf Hansson
@ 2020-06-02 2:41 ` 冯锐
2020-06-02 8:36 ` Ulf Hansson
0 siblings, 1 reply; 17+ messages in thread
From: 冯锐 @ 2020-06-02 2:41 UTC (permalink / raw)
To: Ulf Hansson
Cc: Arnd Bergmann, Christoph Hellwig, gregkh, linux-kernel,
linux-pci, linux-mmc
>
> +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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
2020-06-02 2:41 ` 答复: " 冯锐
@ 2020-06-02 8:36 ` Ulf Hansson
2020-07-01 9:52 ` 答复: " 冯锐
0 siblings, 1 reply; 17+ messages in thread
From: Ulf Hansson @ 2020-06-02 8:36 UTC (permalink / raw)
To: 冯锐
Cc: Arnd Bergmann, Christoph Hellwig, gregkh, linux-kernel,
linux-pci, linux-mmc
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* 答复: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
2020-06-02 8:36 ` Ulf Hansson
@ 2020-07-01 9:52 ` 冯锐
2020-07-06 14:42 ` Ulf Hansson
0 siblings, 1 reply; 17+ messages in thread
From: 冯锐 @ 2020-07-01 9:52 UTC (permalink / raw)
To: Ulf Hansson; +Cc: Arnd Bergmann, gregkh, linux-kernel, linux-pci, linux-mmc
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
2020-07-01 9:52 ` 答复: " 冯锐
@ 2020-07-06 14:42 ` Ulf Hansson
0 siblings, 0 replies; 17+ messages in thread
From: Ulf Hansson @ 2020-07-06 14:42 UTC (permalink / raw)
To: 冯锐
Cc: Arnd Bergmann, gregkh, linux-kernel, linux-pci, linux-mmc
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261
2020-04-26 1:25 rui_feng
2020-04-27 6:14 ` Christoph Hellwig
@ 2020-05-05 14:28 ` Greg KH
1 sibling, 0 replies; 17+ messages in thread
From: Greg KH @ 2020-05-05 14:28 UTC (permalink / raw)
To: rui_feng; +Cc: arnd, ulf.hansson, linux-kernel
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
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-09-24 20:54 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-24 7:40 [PATCH] mmc: rtsx: Add SD Express mode support for RTS5261 rui_feng
2020-09-24 9:38 ` kernel test robot
2020-09-24 9:38 ` kernel test robot
2020-09-24 9:48 ` 答复: " 冯锐
2020-09-24 10:01 ` Ulf Hansson
2020-09-24 11:15 ` Ulf Hansson
2020-09-24 10:48 ` kernel test robot
2020-09-24 20:54 ` kernel test robot
-- strict thread matches above, loose matches on Subject: below --
2020-04-26 1:25 rui_feng
2020-04-27 6:14 ` Christoph Hellwig
2020-04-27 9:40 ` 答复: " 冯锐
2020-04-27 11:07 ` Arnd Bergmann
2020-04-28 3:44 ` 答复: " 冯锐
2020-05-05 18:18 ` Ulf Hansson
2020-05-19 9:17 ` 答复: " 冯锐
2020-05-25 10:27 ` Ulf Hansson
2020-06-01 7:33 ` 答复: " 冯锐
2020-06-01 10:19 ` Ulf Hansson
2020-06-02 2:41 ` 答复: " 冯锐
2020-06-02 8:36 ` Ulf Hansson
2020-07-01 9:52 ` 答复: " 冯锐
2020-07-06 14:42 ` Ulf Hansson
2020-05-05 14:28 ` Greg KH
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.