linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mfd: rtsx: Add support for RTS5260
@ 2017-09-08  6:57 rui_feng
  2017-09-14  7:57 ` Lee Jones
  0 siblings, 1 reply; 8+ messages in thread
From: rui_feng @ 2017-09-08  6:57 UTC (permalink / raw)
  To: lee.jones, linux-kernel; +Cc: rui_feng

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

Add support for new chip rts5260.

Signed-off-by: Rui Feng <rui_feng@realsil.com.cn>
---
 drivers/mfd/Makefile         |   2 +-
 drivers/mfd/rts5260.c        | 543 +++++++++++++++++++++++++++++++++++++++++++
 drivers/mfd/rtsx_pcr.c       |   8 +
 drivers/mfd/rtsx_pcr.h       |   1 +
 include/linux/mfd/rtsx_pci.h |  81 ++++++-
 5 files changed, 631 insertions(+), 4 deletions(-)
 create mode 100644 drivers/mfd/rts5260.c

diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 080793b..d0f06a3 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -17,7 +17,7 @@ obj-$(CONFIG_MFD_CROS_EC_I2C)	+= cros_ec_i2c.o
 obj-$(CONFIG_MFD_CROS_EC_SPI)	+= cros_ec_spi.o
 obj-$(CONFIG_MFD_EXYNOS_LPASS)	+= exynos-lpass.o
 
-rtsx_pci-objs			:= rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o
+rtsx_pci-objs			:= rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o rts5260.o
 obj-$(CONFIG_MFD_RTSX_PCI)	+= rtsx_pci.o
 obj-$(CONFIG_MFD_RTSX_USB)	+= rtsx_usb.o
 
diff --git a/drivers/mfd/rts5260.c b/drivers/mfd/rts5260.c
new file mode 100644
index 0000000..53f2838
--- /dev/null
+++ b/drivers/mfd/rts5260.c
@@ -0,0 +1,543 @@
+/* Driver for Realtek PCI-Express card reader
+ *
+ * Copyright(c) 2016-2017 Realtek Semiconductor Corp. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ * Author:
+ *   Steven FENG <steven_feng@realsil.com.cn>
+ *   Rui FENG <rui_feng@realsil.com.cn>
+ *   Wei WANG <wei_wang@realsil.com.cn>
+ */
+
+#include <linux/module.h>
+#include <linux/delay.h>
+
+#include "rtsx_pcr.h"
+
+
+static u8 rts5260_get_ic_version(struct rtsx_pcr *pcr)
+{
+	u8 val;
+
+	rtsx_pci_read_register(pcr, DUMMY_REG_RESET_0, &val);
+	return val & IC_VERSION_MASK;
+}
+
+static void rts5260_fill_driving(struct rtsx_pcr *pcr, u8 voltage)
+{
+	u8 driving_3v3[6][3] = {
+		{0x94, 0x94, 0x94},
+		{0x11, 0x11, 0x18},
+		{0x55, 0x55, 0x5C},
+		{0x94, 0x94, 0x94},
+		{0x94, 0x94, 0x94},
+		{0xFF, 0xFF, 0xFF},
+	};
+	u8 driving_1v8[6][3] = {
+		{0x9A, 0x89, 0x89},
+		{0xC4, 0xC4, 0xC4},
+		{0x3C, 0x3C, 0x3C},
+		{0x9A, 0x89, 0x89},
+		{0x9A, 0x89, 0x89},
+		{0xFE, 0xFE, 0xFE},
+	};
+	u8 (*driving)[3], drive_sel;
+
+	if (voltage == OUTPUT_3V3) {
+		driving = driving_3v3;
+		drive_sel = pcr->sd30_drive_sel_3v3;
+	} else {
+		driving = driving_1v8;
+		drive_sel = pcr->sd30_drive_sel_1v8;
+	}
+
+	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, SD30_CLK_DRIVE_SEL,
+			0xFF, driving[drive_sel][0]);
+	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, SD30_CMD_DRIVE_SEL,
+			0xFF, driving[drive_sel][1]);
+	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, SD30_DAT_DRIVE_SEL,
+			0xFF, driving[drive_sel][2]);
+}
+
+static void rtsx_base_fetch_vendor_settings(struct rtsx_pcr *pcr)
+{
+	u32 reg;
+
+	rtsx_pci_read_config_dword(pcr, PCR_SETTING_REG1, &reg);
+	pcr_dbg(pcr, "Cfg 0x%x: 0x%x\n", PCR_SETTING_REG1, reg);
+
+	if (!rtsx_vendor_setting_valid(reg)) {
+		pcr_dbg(pcr, "skip fetch vendor setting\n");
+		return;
+	}
+
+	pcr->aspm_en = rtsx_reg_to_aspm(reg);
+	pcr->sd30_drive_sel_1v8 = rtsx_reg_to_sd30_drive_sel_1v8(reg);
+	pcr->card_drive_sel &= 0x3F;
+	pcr->card_drive_sel |= rtsx_reg_to_card_drive_sel(reg);
+
+	rtsx_pci_read_config_dword(pcr, PCR_SETTING_REG2, &reg);
+	pcr_dbg(pcr, "Cfg 0x%x: 0x%x\n", PCR_SETTING_REG2, reg);
+	pcr->sd30_drive_sel_3v3 = rtsx_reg_to_sd30_drive_sel_3v3(reg);
+	if (rtsx_reg_check_reverse_socket(reg))
+		pcr->flags |= PCR_REVERSE_SOCKET;
+}
+
+static void rtsx_base_force_power_down(struct rtsx_pcr *pcr, u8 pm_state)
+{
+	/* Set relink_time to 0 */
+	rtsx_pci_write_register(pcr, AUTOLOAD_CFG_BASE + 1, MASK_8_BIT_DEF, 0);
+	rtsx_pci_write_register(pcr, AUTOLOAD_CFG_BASE + 2, MASK_8_BIT_DEF, 0);
+	rtsx_pci_write_register(pcr, AUTOLOAD_CFG_BASE + 3,
+			RELINK_TIME_MASK, 0);
+
+	if (pm_state == HOST_ENTER_S3)
+		rtsx_pci_write_register(pcr, pcr->reg_pm_ctrl3,
+			D3_DELINK_MODE_EN, D3_DELINK_MODE_EN);
+
+	rtsx_pci_write_register(pcr, FPDCTL, ALL_POWER_DOWN, ALL_POWER_DOWN);
+}
+
+static int rtsx_base_enable_auto_blink(struct rtsx_pcr *pcr)
+{
+	return rtsx_pci_write_register(pcr, OLT_LED_CTL,
+		LED_SHINE_MASK, LED_SHINE_EN);
+}
+
+static int rtsx_base_disable_auto_blink(struct rtsx_pcr *pcr)
+{
+	return rtsx_pci_write_register(pcr, OLT_LED_CTL,
+		LED_SHINE_MASK, LED_SHINE_DISABLE);
+}
+
+static int rts5260_turn_on_led(struct rtsx_pcr *pcr)
+{
+	return rtsx_pci_write_register(pcr, RTS5260_REG_GPIO_CTL0,
+		RTS5260_REG_GPIO_MASK, RTS5260_REG_GPIO_ON);
+}
+
+static int rts5260_turn_off_led(struct rtsx_pcr *pcr)
+{
+	return rtsx_pci_write_register(pcr, RTS5260_REG_GPIO_CTL0,
+		RTS5260_REG_GPIO_MASK, RTS5260_REG_GPIO_OFF);
+}
+
+/* SD Pull Control Enable:
+ *     SD_DAT[3:0] ==> pull up
+ *     SD_CD       ==> pull up
+ *     SD_WP       ==> pull up
+ *     SD_CMD      ==> pull up
+ *     SD_CLK      ==> pull down
+ */
+static const u32 rts5260_sd_pull_ctl_enable_tbl[] = {
+	RTSX_REG_PAIR(CARD_PULL_CTL1, 0x66),
+	RTSX_REG_PAIR(CARD_PULL_CTL2, 0xAA),
+	RTSX_REG_PAIR(CARD_PULL_CTL3, 0xE9),
+	RTSX_REG_PAIR(CARD_PULL_CTL4, 0xAA),
+	0,
+};
+
+/* SD Pull Control Disable:
+ *     SD_DAT[3:0] ==> pull down
+ *     SD_CD       ==> pull up
+ *     SD_WP       ==> pull down
+ *     SD_CMD      ==> pull down
+ *     SD_CLK      ==> pull down
+ */
+static const u32 rts5260_sd_pull_ctl_disable_tbl[] = {
+	RTSX_REG_PAIR(CARD_PULL_CTL1, 0x66),
+	RTSX_REG_PAIR(CARD_PULL_CTL2, 0x55),
+	RTSX_REG_PAIR(CARD_PULL_CTL3, 0xD5),
+	RTSX_REG_PAIR(CARD_PULL_CTL4, 0x55),
+	0,
+};
+
+/* MS Pull Control Enable:
+ *     MS CD       ==> pull up
+ *     others      ==> pull down
+ */
+static const u32 rts5260_ms_pull_ctl_enable_tbl[] = {
+	RTSX_REG_PAIR(CARD_PULL_CTL4, 0x55),
+	RTSX_REG_PAIR(CARD_PULL_CTL5, 0x55),
+	RTSX_REG_PAIR(CARD_PULL_CTL6, 0x15),
+	0,
+};
+
+/* MS Pull Control Disable:
+ *     MS CD       ==> pull up
+ *     others      ==> pull down
+ */
+static const u32 rts5260_ms_pull_ctl_disable_tbl[] = {
+	RTSX_REG_PAIR(CARD_PULL_CTL4, 0x55),
+	RTSX_REG_PAIR(CARD_PULL_CTL5, 0x55),
+	RTSX_REG_PAIR(CARD_PULL_CTL6, 0x15),
+	0,
+};
+
+static int rts5260_card_power_on(struct rtsx_pcr *pcr, int card)
+{
+	int err = 0;
+
+	rtsx_pci_init_cmd(pcr);
+	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, LDO_CONFIG2,
+			DV331812_VDD1, DV331812_VDD1);
+	err = rtsx_pci_send_cmd(pcr, CMD_TIMEOUT_DEF);
+	if (err < 0)
+		return err;
+
+	rtsx_pci_init_cmd(pcr);
+	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, LDO_VCC_CFG0,
+			RTS5260_DVCC_TUNE_MASK, RTS5260_DVCC_33);
+	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, LDO_VCC_CFG1,
+			LDO_POW_SDVDD1_MASK, LDO_POW_SDVDD1_ON);
+	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, LDO_CONFIG2,
+			DV331812_POWERON, DV331812_POWERON);
+	err = rtsx_pci_send_cmd(pcr, CMD_TIMEOUT_DEF);
+
+	msleep(20);
+
+	return err;
+}
+
+static int rts5260_card_power_off(struct rtsx_pcr *pcr, int card)
+{
+	int err = 0;
+
+	rtsx_pci_init_cmd(pcr);
+	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, LDO_VCC_CFG1,
+			LDO_POW_SDVDD1_MASK, LDO_POW_SDVDD1_OFF);
+	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, LDO_CONFIG2,
+			DV331812_POWERON, DV331812_POWEROFF);
+	err = rtsx_pci_send_cmd(pcr, CMD_TIMEOUT_DEF);
+
+	return err;
+}
+
+static int rts5260_switch_output_voltage(struct rtsx_pcr *pcr, u8 voltage)
+{
+	switch (voltage) {
+	case OUTPUT_3V3:
+		rtsx_pci_write_register(pcr, LDO_CONFIG2,
+			DV331812_VDD1, DV331812_VDD1);
+		rtsx_pci_write_register(pcr, LDO_DV18_CFG,
+			DV331812_MASK, DV331812_33);
+		rtsx_pci_write_register(pcr, SD_PAD_CTL, SD_IO_USING_1V8, 0);
+		break;
+	case OUTPUT_1V8:
+		rtsx_pci_write_register(pcr, LDO_CONFIG2,
+			DV331812_VDD1, DV331812_VDD1);
+		rtsx_pci_write_register(pcr, LDO_DV18_CFG,
+			DV331812_MASK, DV331812_17);
+		rtsx_pci_write_register(pcr, SD_PAD_CTL, SD_IO_USING_1V8,
+			SD_IO_USING_1V8);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* set pad drive */
+	rtsx_pci_init_cmd(pcr);
+	rts5260_fill_driving(pcr, voltage);
+	return rtsx_pci_send_cmd(pcr, CMD_TIMEOUT_DEF);
+}
+
+static void rts5260_stop_cmd(struct rtsx_pcr *pcr)
+{
+	rtsx_pci_writel(pcr, RTSX_HCBCTLR, STOP_CMD);
+	rtsx_pci_writel(pcr, RTSX_HDBCTLR, STOP_DMA);
+	rtsx_pci_write_register(pcr, RTS5260_DMA_RST_CTL_0,
+		RTS5260_DMA_RST | RTS5260_ADMA3_RST,
+		RTS5260_DMA_RST | RTS5260_ADMA3_RST);
+	rtsx_pci_write_register(pcr, RBCTL, RB_FLUSH, RB_FLUSH);
+}
+
+int rts5260_init_hw(struct rtsx_pcr *pcr)
+{
+	int err;
+
+	rtsx_pci_init_cmd(pcr);
+
+	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, L1SUB_CONFIG1,
+		AUX_CLK_ACTIVE_SEL_MASK, MAC_CKSW_DONE);
+	/* Rest L1SUB Config */
+	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, L1SUB_CONFIG3, 0xFF, 0x00);
+	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, PM_CLK_FORCE_CTL,
+		CLK_PM_EN, CLK_PM_EN);
+	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, PWD_SUSPEND_EN, 0xFF, 0xFF);
+	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, PWR_GATE_CTRL,
+		PWR_GATE_EN, PWR_GATE_EN);
+	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, REG_VREF,
+		PWD_SUSPND_EN, PWD_SUSPND_EN);
+	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, RBCTL,
+		U_AUTO_DMA_EN_MASK, U_AUTO_DMA_DISABLE);
+
+	if (pcr->flags & PCR_REVERSE_SOCKET)
+		rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, PETXCFG, 0xB0, 0xB0);
+	else
+		rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, PETXCFG, 0xB0, 0x80);
+
+	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, OBFF_CFG,
+		OBFF_EN_MASK, OBFF_DISABLE);
+
+	err = rtsx_pci_send_cmd(pcr, CMD_TIMEOUT_DEF);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
+static void rts5260_pwr_saving_setting(struct rtsx_pcr *pcr)
+{
+	int lss_l1_1, lss_l1_2;
+
+	lss_l1_1 = rtsx_check_dev_flag(pcr, ASPM_L1_1_EN)
+			|rtsx_check_dev_flag(pcr, PM_L1_1_EN);
+	lss_l1_2 = rtsx_check_dev_flag(pcr, ASPM_L1_2_EN)
+			|rtsx_check_dev_flag(pcr, PM_L1_2_EN);
+
+	if (lss_l1_2) {
+		pcr_dbg(pcr, "Set parameters for L1.2.");
+		rtsx_pci_write_register(pcr, PWR_GLOBAL_CTRL,
+			0xFF, PCIE_L1_2_EN);
+		rtsx_pci_write_register(pcr, PWR_FE_CTL,
+			0xFF, PCIE_L1_2_PD_FE_EN);
+	} else if (lss_l1_1) {
+		pcr_dbg(pcr, "Set parameters for L1.1.");
+		rtsx_pci_write_register(pcr, PWR_GLOBAL_CTRL,
+			0xFF, PCIE_L1_1_EN);
+		rtsx_pci_write_register(pcr, PWR_FE_CTL,
+			0xFF, PCIE_L1_1_PD_FE_EN);
+	} else {
+		pcr_dbg(pcr, "Set parameters for L1.");
+		rtsx_pci_write_register(pcr, PWR_GLOBAL_CTRL,
+			0xFF, PCIE_L1_0_EN);
+		rtsx_pci_write_register(pcr, PWR_FE_CTL,
+			0xFF, PCIE_L1_0_PD_FE_EN);
+	}
+
+	rtsx_pci_write_register(pcr, CFG_L1_0_PCIE_DPHY_RET_VALUE,
+		0xFF, CFG_L1_0_RET_VALUE_DEFAULT);
+	rtsx_pci_write_register(pcr, CFG_L1_0_PCIE_MAC_RET_VALUE,
+		0xFF, CFG_L1_0_RET_VALUE_DEFAULT);
+	rtsx_pci_write_register(pcr, CFG_L1_0_CRC_SD30_RET_VALUE,
+		0xFF, CFG_L1_0_RET_VALUE_DEFAULT);
+	rtsx_pci_write_register(pcr, CFG_L1_0_CRC_SD40_RET_VALUE,
+		0xFF, CFG_L1_0_RET_VALUE_DEFAULT);
+	rtsx_pci_write_register(pcr, CFG_L1_0_SYS_RET_VALUE,
+		0xFF, CFG_L1_0_RET_VALUE_DEFAULT);
+	/*Option cut APHY*/
+	rtsx_pci_write_register(pcr, CFG_PCIE_APHY_OFF_0,
+		0xFF, CFG_PCIE_APHY_OFF_0_DEFAULT);
+	rtsx_pci_write_register(pcr, CFG_PCIE_APHY_OFF_1,
+		0xFF, CFG_PCIE_APHY_OFF_1_DEFAULT);
+	rtsx_pci_write_register(pcr, CFG_PCIE_APHY_OFF_2,
+		0xFF, CFG_PCIE_APHY_OFF_2_DEFAULT);
+	rtsx_pci_write_register(pcr, CFG_PCIE_APHY_OFF_3,
+		0xFF, CFG_PCIE_APHY_OFF_3_DEFAULT);
+	/*CDR DEC*/
+	rtsx_pci_write_register(pcr, PWC_CDR, 0xFF, PWC_CDR_DEFAULT);
+	/*PWMPFM*/
+	rtsx_pci_write_register(pcr, CFG_LP_FPWM_VALUE,
+		0xFF, CFG_LP_FPWM_VALUE_DEFAULT);
+	/*No Power Saving WA*/
+	rtsx_pci_write_register(pcr, CFG_L1_0_CRC_MISC_RET_VALUE,
+		0xFF, CFG_L1_0_CRC_MISC_RET_VALUE_DEFAULT);
+}
+
+static void rts5260_init_from_cfg(struct rtsx_pcr *pcr)
+{
+	struct rtsx_cr_option *option = &(pcr->option);
+	u32 lval;
+
+	rtsx_pci_read_config_dword(pcr, PCR_ASPM_SETTING_5260, &lval);
+
+	if (lval & ASPM_L1_1_EN_MASK)
+		rtsx_set_dev_flag(pcr, ASPM_L1_1_EN);
+
+	if (lval & ASPM_L1_2_EN_MASK)
+		rtsx_set_dev_flag(pcr, ASPM_L1_2_EN);
+
+	if (lval & PM_L1_1_EN_MASK)
+		rtsx_set_dev_flag(pcr, PM_L1_1_EN);
+
+	if (lval & PM_L1_2_EN_MASK)
+		rtsx_set_dev_flag(pcr, PM_L1_2_EN);
+
+	rts5260_pwr_saving_setting(pcr);
+
+	if (option->ltr_en) {
+		u16 val;
+
+		pcie_capability_read_word(pcr->pci, PCI_EXP_DEVCTL2, &val);
+		if (val & PCI_EXP_DEVCTL2_LTR_EN) {
+			option->ltr_enabled = true;
+			option->ltr_active = true;
+			rtsx_set_ltr_latency(pcr, option->ltr_active_latency);
+		} else {
+			option->ltr_enabled = false;
+		}
+	}
+
+	if (rtsx_check_dev_flag(pcr, ASPM_L1_1_EN | ASPM_L1_2_EN
+				| PM_L1_1_EN | PM_L1_2_EN))
+		option->force_clkreq_0 = false;
+	else
+		option->force_clkreq_0 = true;
+}
+
+static int rts5260_extra_init_hw(struct rtsx_pcr *pcr)
+{
+	struct rtsx_cr_option *option = &(pcr->option);
+
+	rts5260_init_from_cfg(pcr);
+
+	/* force no MDIO*/
+	rtsx_pci_write_register(pcr, RTS5260_AUTOLOAD_CFG4,
+		0xFF, RTS5260_MIMO_DISABLE);
+	/*Modify SDVCC Tune Default Parameters!*/
+	rtsx_pci_write_register(pcr, LDO_VCC_CFG0,
+		RTS5260_DVCC_TUNE_MASK, RTS5260_DVCC_33);
+
+	rtsx_pci_write_register(pcr, PCLK_CTL, PCLK_MODE_SEL, PCLK_MODE_SEL);
+
+	rts5260_init_hw(pcr);
+
+	/*
+	 * If u_force_clkreq_0 is enabled, CLKREQ# PIN will be forced
+	 * to drive low, and we forcibly request clock.
+	 */
+	if (option->force_clkreq_0)
+		rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, PETXCFG,
+			FORCE_CLKREQ_DELINK_MASK, FORCE_CLKREQ_LOW);
+	else
+		rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, PETXCFG,
+			FORCE_CLKREQ_DELINK_MASK, FORCE_CLKREQ_HIGH);
+
+	return 0;
+}
+
+void rts5260_set_aspm(struct rtsx_pcr *pcr, bool enable)
+{
+	struct rtsx_cr_option *option = &pcr->option;
+	u8 val = 0;
+
+	if (pcr->aspm_enabled == enable)
+		return;
+
+	if (option->dev_aspm_mode == DEV_ASPM_DYNAMIC) {
+		if (enable)
+			val = pcr->aspm_en;
+		rtsx_pci_update_cfg_byte(pcr,
+			pcr->pcie_cap + PCI_EXP_LNKCTL,
+			ASPM_MASK_NEG, val);
+	} else if (option->dev_aspm_mode == DEV_ASPM_BACKDOOR) {
+		u8 mask = FORCE_ASPM_VAL_MASK | FORCE_ASPM_CTL0;
+
+		if (!enable)
+			val = FORCE_ASPM_CTL0;
+		rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, mask, val);
+	}
+
+	pcr->aspm_enabled = enable;
+}
+
+static void rts5260_set_l1off_cfg_sub_d0(struct rtsx_pcr *pcr, int active)
+{
+	struct rtsx_cr_option *option = &(pcr->option);
+	u32 interrupt = rtsx_pci_readl(pcr, RTSX_BIPR);
+	int card_exist = (interrupt & SD_EXIST) | (interrupt & MS_EXIST);
+	int aspm_L1_1, aspm_L1_2;
+	u8 val = 0;
+
+	aspm_L1_1 = rtsx_check_dev_flag(pcr, ASPM_L1_1_EN);
+	aspm_L1_2 = rtsx_check_dev_flag(pcr, ASPM_L1_2_EN);
+
+	if (active) {
+		/* run, latency: 60us */
+		if (aspm_L1_1)
+			val = option->ltr_l1off_snooze_sspwrgate;
+	} else {
+		/* l1off, latency: 300us */
+		if (aspm_L1_2)
+			val = option->ltr_l1off_sspwrgate;
+	}
+
+	if (aspm_L1_1 || aspm_L1_2) {
+		if (rtsx_check_dev_flag(pcr,
+					LTR_L1SS_PWR_GATE_CHECK_CARD_EN)) {
+			if (card_exist)
+				val &= ~L1OFF_MBIAS2_EN_5250;
+			else
+				val |= L1OFF_MBIAS2_EN_5250;
+		}
+	}
+	rtsx_set_l1off_sub(pcr, val);
+}
+
+static const struct pcr_ops rts5260_pcr_ops = {
+	.fetch_vendor_settings = rtsx_base_fetch_vendor_settings,
+	.turn_on_led = rts5260_turn_on_led,
+	.turn_off_led = rts5260_turn_off_led,
+	.extra_init_hw = rts5260_extra_init_hw,
+	.enable_auto_blink = rtsx_base_enable_auto_blink,
+	.disable_auto_blink = rtsx_base_disable_auto_blink,
+	.card_power_on = rts5260_card_power_on,
+	.card_power_off = rts5260_card_power_off,
+	.switch_output_voltage = rts5260_switch_output_voltage,
+	.force_power_down = rtsx_base_force_power_down,
+	.stop_cmd = rts5260_stop_cmd,
+	.set_aspm = rts5260_set_aspm,
+	.set_l1off_cfg_sub_d0 = rts5260_set_l1off_cfg_sub_d0,
+};
+
+void rts5260_init_params(struct rtsx_pcr *pcr)
+{
+	struct rtsx_cr_option *option = &(pcr->option);
+
+	pcr->extra_caps = EXTRA_CAPS_SD_SDR50 | EXTRA_CAPS_SD_SDR104;
+	pcr->num_slots = 2;
+
+	pcr->flags = 0;
+	pcr->card_drive_sel = RTSX_CARD_DRIVE_DEFAULT;
+	pcr->sd30_drive_sel_1v8 = CFG_DRIVER_TYPE_B;
+	pcr->sd30_drive_sel_3v3 = CFG_DRIVER_TYPE_B;
+	pcr->aspm_en = ASPM_L1_EN;
+	pcr->tx_initial_phase = SET_CLOCK_PHASE(1, 29, 16);
+	pcr->rx_initial_phase = SET_CLOCK_PHASE(24, 6, 5);
+
+	pcr->ic_version = rts5260_get_ic_version(pcr);
+	pcr->sd_pull_ctl_enable_tbl = rts5260_sd_pull_ctl_enable_tbl;
+	pcr->sd_pull_ctl_disable_tbl = rts5260_sd_pull_ctl_disable_tbl;
+	pcr->ms_pull_ctl_enable_tbl = rts5260_ms_pull_ctl_enable_tbl;
+	pcr->ms_pull_ctl_disable_tbl = rts5260_ms_pull_ctl_disable_tbl;
+
+	pcr->reg_pm_ctrl3 = RTS524A_PM_CTRL3;
+
+	pcr->ops = &rts5260_pcr_ops;
+
+	option->dev_flags = (LTR_L1SS_PWR_GATE_CHECK_CARD_EN
+				| LTR_L1SS_PWR_GATE_EN);
+	option->ltr_en = true;
+
+	/* init latency of active, idle, L1OFF to 60us, 300us, 3ms */
+	option->ltr_active_latency = LTR_ACTIVE_LATENCY_DEF;
+	option->ltr_idle_latency = LTR_IDLE_LATENCY_DEF;
+	option->ltr_l1off_latency = LTR_L1OFF_LATENCY_DEF;
+	option->dev_aspm_mode = DEV_ASPM_DYNAMIC;
+	option->l1_snooze_delay = L1_SNOOZE_DELAY_DEF;
+	pcr->option.ltr_l1off_sspwrgate = LTR_L1OFF_SSPWRGATE_5250_DEF;
+	pcr->option.ltr_l1off_snooze_sspwrgate =
+		LTR_L1OFF_SNOOZE_SSPWRGATE_5250_DEF;
+}
diff --git a/drivers/mfd/rtsx_pcr.c b/drivers/mfd/rtsx_pcr.c
index 7c87485..651f19a 100644
--- a/drivers/mfd/rtsx_pcr.c
+++ b/drivers/mfd/rtsx_pcr.c
@@ -62,6 +62,7 @@
 	{ PCI_DEVICE(0x10EC, 0x5286), PCI_CLASS_OTHERS << 16, 0xFF0000 },
 	{ PCI_DEVICE(0x10EC, 0x524A), PCI_CLASS_OTHERS << 16, 0xFF0000 },
 	{ PCI_DEVICE(0x10EC, 0x525A), PCI_CLASS_OTHERS << 16, 0xFF0000 },
+	{ PCI_DEVICE(0x10EC, 0x5260), PCI_CLASS_OTHERS << 16, 0xFF0000 },
 	{ 0, }
 };
 
@@ -334,6 +335,9 @@ int rtsx_pci_read_phy_register(struct rtsx_pcr *pcr, u8 addr, u16 *val)
 
 void rtsx_pci_stop_cmd(struct rtsx_pcr *pcr)
 {
+	if (pcr->ops->stop_cmd)
+		return pcr->ops->stop_cmd(pcr);
+
 	rtsx_pci_writel(pcr, RTSX_HCBCTLR, STOP_CMD);
 	rtsx_pci_writel(pcr, RTSX_HDBCTLR, STOP_DMA);
 
@@ -1189,6 +1193,7 @@ static int rtsx_pci_init_hw(struct rtsx_pcr *pcr)
 	case PID_5250:
 	case PID_524A:
 	case PID_525A:
+	case PID_5260:
 		rtsx_pci_write_register(pcr, PM_CLK_FORCE_CTL, 1, 1);
 		break;
 	default:
@@ -1265,6 +1270,9 @@ static int rtsx_pci_init_chip(struct rtsx_pcr *pcr)
 	case 0x5286:
 		rtl8402_init_params(pcr);
 		break;
+	case 0x5260:
+		rts5260_init_params(pcr);
+		break;
 	}
 
 	pcr_dbg(pcr, "PID: 0x%04x, IC version: 0x%02x\n",
diff --git a/drivers/mfd/rtsx_pcr.h b/drivers/mfd/rtsx_pcr.h
index ec784e0..e55e153 100644
--- a/drivers/mfd/rtsx_pcr.h
+++ b/drivers/mfd/rtsx_pcr.h
@@ -57,6 +57,7 @@
 void rts524a_init_params(struct rtsx_pcr *pcr);
 void rts525a_init_params(struct rtsx_pcr *pcr);
 void rtl8411b_init_params(struct rtsx_pcr *pcr);
+void rts5260_init_params(struct rtsx_pcr *pcr);
 
 static inline u8 map_sd_drive(int idx)
 {
diff --git a/include/linux/mfd/rtsx_pci.h b/include/linux/mfd/rtsx_pci.h
index 57e23ad..a40abaa 100644
--- a/include/linux/mfd/rtsx_pci.h
+++ b/include/linux/mfd/rtsx_pci.h
@@ -452,7 +452,7 @@
 #define FPDCTL				0xFC00
 #define   SSC_POWER_DOWN		0x01
 #define   SD_OC_POWER_DOWN		0x02
-#define   ALL_POWER_DOWN		0x07
+#define   ALL_POWER_DOWN		0x03
 #define   OC_POWER_DOWN			0x06
 #define PDINFO				0xFC01
 
@@ -489,6 +489,9 @@
 
 #define FPGA_PULL_CTL			0xFC1D
 #define OLT_LED_CTL			0xFC1E
+#define   LED_SHINE_MASK		0x08
+#define   LED_SHINE_EN			0x08
+#define   LED_SHINE_DISABLE		0x00
 #define GPIO_CTL			0xFC1F
 
 #define LDO_CTL				0xFC1E
@@ -510,7 +513,11 @@
 #define   BPP_LDO_ON			0x00
 #define   BPP_LDO_SUSPEND		0x02
 #define   BPP_LDO_OFF			0x03
+#define EFUSE_CTL			0xFC30
+#define EFUSE_ADD			0xFC31
 #define SYS_VER				0xFC32
+#define EFUSE_DATAL			0xFC34
+#define EFUSE_DATAH			0xFC35
 
 #define CARD_PULL_CTL1			0xFD60
 #define CARD_PULL_CTL2			0xFD61
@@ -552,6 +559,9 @@
 #define RBBC1				0xFE2F
 #define RBDAT				0xFE30
 #define RBCTL				0xFE34
+#define   U_AUTO_DMA_EN_MASK		0x20
+#define   U_AUTO_DMA_DISABLE		0x00
+#define   RB_FLUSH			0x80
 #define CFGADDR0			0xFE35
 #define CFGADDR1			0xFE36
 #define CFGDATA0			0xFE37
@@ -580,6 +590,8 @@
 #define LTR_LATENCY_MODE_HW		0
 #define LTR_LATENCY_MODE_SW		BIT(6)
 #define OBFF_CFG			0xFE4C
+#define   OBFF_EN_MASK			0x03
+#define   OBFF_DISABLE			0x00
 
 #define CDRESUMECTL			0xFE52
 #define WAKE_SEL_CTL			0xFE54
@@ -594,6 +606,7 @@
 #define   FORCE_ASPM_L0_EN		0x01
 #define   FORCE_ASPM_NO_ASPM		0x00
 #define PM_CLK_FORCE_CTL		0xFE58
+#define   CLK_PM_EN			0x01
 #define FUNC_FORCE_CTL			0xFE59
 #define   FUNC_FORCE_UPME_XMT_DBG	0x02
 #define PERST_GLITCH_WIDTH		0xFE5C
@@ -619,14 +632,23 @@
 #define LDO_PWR_SEL			0xFE78
 
 #define L1SUB_CONFIG1			0xFE8D
+#define   AUX_CLK_ACTIVE_SEL_MASK	0x01
+#define   MAC_CKSW_DONE			0x00
 #define L1SUB_CONFIG2			0xFE8E
 #define   L1SUB_AUTO_CFG		0x02
 #define L1SUB_CONFIG3			0xFE8F
 #define   L1OFF_MBIAS2_EN_5250		BIT(7)
 
 #define DUMMY_REG_RESET_0		0xFE90
+#define   IC_VERSION_MASK		0x0F
 
+#define REG_VREF			0xFE97
+#define   PWD_SUSPND_EN			0x10
+#define RTS5260_DMA_RST_CTL_0		0xFEBF
+#define   RTS5260_DMA_RST		0x80
+#define   RTS5260_ADMA3_RST		0x40
 #define AUTOLOAD_CFG_BASE		0xFF00
+#define RELINK_TIME_MASK		0x01
 #define PETXCFG				0xFF03
 #define FORCE_CLKREQ_DELINK_MASK	BIT(7)
 #define FORCE_CLKREQ_LOW	0x80
@@ -666,15 +688,24 @@
 #define LDO_DV18_CFG			0xFF70
 #define   LDO_DV18_SR_MASK		0xC0
 #define   LDO_DV18_SR_DF		0x40
+#define   DV331812_MASK			0x70
+#define   DV331812_33			0x70
+#define   DV331812_17			0x30
 
 #define LDO_CONFIG2			0xFF71
 #define   LDO_D3318_MASK		0x07
 #define   LDO_D3318_33V			0x07
 #define   LDO_D3318_18V			0x02
+#define   DV331812_VDD1			0x04
+#define   DV331812_POWERON		0x08
+#define   DV331812_POWEROFF		0x00
 
 #define LDO_VCC_CFG0			0xFF72
 #define   LDO_VCC_LMTVTH_MASK		0x30
 #define   LDO_VCC_LMTVTH_2A		0x10
+/*RTS5260*/
+#define   RTS5260_DVCC_TUNE_MASK	0x70
+#define   RTS5260_DVCC_33		0x70
 
 #define LDO_VCC_CFG1			0xFF73
 #define   LDO_VCC_REF_TUNE_MASK		0x30
@@ -683,6 +714,10 @@
 #define   LDO_VCC_1V8			0x04
 #define   LDO_VCC_3V3			0x07
 #define   LDO_VCC_LMT_EN		0x08
+/*RTS5260*/
+#define	  LDO_POW_SDVDD1_MASK		0x08
+#define	  LDO_POW_SDVDD1_ON		0x08
+#define	  LDO_POW_SDVDD1_OFF		0x00
 
 #define LDO_VIO_CFG			0xFF75
 #define   LDO_VIO_SR_MASK		0xC0
@@ -710,6 +745,43 @@
 #define   SD_VIO_LDO_1V8		0x40
 #define   SD_VIO_LDO_3V3		0x70
 
+#define RTS5260_AUTOLOAD_CFG4		0xFF7F
+#define   RTS5260_MIMO_DISABLE		0x8A
+
+#define RTS5260_REG_GPIO_CTL0		0xFC1A
+#define   RTS5260_REG_GPIO_MASK		0x01
+#define   RTS5260_REG_GPIO_ON		0x01
+#define   RTS5260_REG_GPIO_OFF		0x00
+
+#define PWR_GLOBAL_CTRL			0xF200
+#define PCIE_L1_2_EN			0x0C
+#define PCIE_L1_1_EN			0x0A
+#define PCIE_L1_0_EN			0x09
+#define PWR_FE_CTL			0xF201
+#define PCIE_L1_2_PD_FE_EN		0x0C
+#define PCIE_L1_1_PD_FE_EN		0x0A
+#define PCIE_L1_0_PD_FE_EN		0x09
+#define CFG_PCIE_APHY_OFF_0		0xF204
+#define CFG_PCIE_APHY_OFF_0_DEFAULT	0xBF
+#define CFG_PCIE_APHY_OFF_1		0xF205
+#define CFG_PCIE_APHY_OFF_1_DEFAULT	0xFF
+#define CFG_PCIE_APHY_OFF_2		0xF206
+#define CFG_PCIE_APHY_OFF_2_DEFAULT	0x01
+#define CFG_PCIE_APHY_OFF_3		0xF207
+#define CFG_PCIE_APHY_OFF_3_DEFAULT	0x00
+#define CFG_L1_0_PCIE_MAC_RET_VALUE	0xF20C
+#define CFG_L1_0_PCIE_DPHY_RET_VALUE	0xF20E
+#define CFG_L1_0_SYS_RET_VALUE		0xF210
+#define CFG_L1_0_CRC_MISC_RET_VALUE	0xF212
+#define CFG_L1_0_CRC_SD30_RET_VALUE	0xF214
+#define CFG_L1_0_CRC_SD40_RET_VALUE	0xF216
+#define CFG_LP_FPWM_VALUE		0xF219
+#define CFG_LP_FPWM_VALUE_DEFAULT	0x18
+#define PWC_CDR				0xF253
+#define PWC_CDR_DEFAULT			0x03
+#define CFG_L1_0_RET_VALUE_DEFAULT	0x1B
+#define CFG_L1_0_CRC_MISC_RET_VALUE_DEFAULT	0x0C
+
 /* Phy register */
 #define PHY_PCR				0x00
 #define   PHY_PCR_FORCE_CODE		0xB000
@@ -856,6 +928,7 @@
 
 #define PCR_ASPM_SETTING_REG1		0x160
 #define PCR_ASPM_SETTING_REG2		0x168
+#define PCR_ASPM_SETTING_5260		0x178
 
 #define PCR_SETTING_REG1		0x724
 #define PCR_SETTING_REG2		0x814
@@ -889,6 +962,7 @@ struct pcr_ops {
 	int		(*conv_clk_and_div_n)(int clk, int dir);
 	void		(*fetch_vendor_settings)(struct rtsx_pcr *pcr);
 	void		(*force_power_down)(struct rtsx_pcr *pcr, u8 pm_state);
+	void		(*stop_cmd)(struct rtsx_pcr *pcr);
 
 	void (*set_aspm)(struct rtsx_pcr *pcr, bool enable);
 	int (*set_ltr_latency)(struct rtsx_pcr *pcr, u32 latency);
@@ -1044,9 +1118,10 @@ struct rtsx_pcr {
 };
 
 #define PID_524A	0x524A
-#define PID_5249		0x5249
-#define PID_5250		0x5250
+#define PID_5249	0x5249
+#define PID_5250	0x5250
 #define PID_525A	0x525A
+#define PID_5260	0x5260
 
 #define CHK_PCI_PID(pcr, pid)		((pcr)->pci->device == (pid))
 #define PCI_VID(pcr)			((pcr)->pci->vendor)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] mfd: rtsx: Add support for RTS5260
  2017-09-08  6:57 [PATCH] mfd: rtsx: Add support for RTS5260 rui_feng
@ 2017-09-14  7:57 ` Lee Jones
  2017-09-14  9:50   ` 答复: " 冯锐
  0 siblings, 1 reply; 8+ messages in thread
From: Lee Jones @ 2017-09-14  7:57 UTC (permalink / raw)
  To: rui_feng; +Cc: linux-kernel

On Fri, 08 Sep 2017, rui_feng@realsil.com.cn wrote:

> From: rui_feng <rui_feng@realsil.com.cn>
> 
> Add support for new chip rts5260.

You are adding over 600 lines in this patch.  It desearves a more
forthcoming commit message.

What is it?
What functionality does it provide?
What other subsystems are involved?
Could it break something else?
Does it have any dependencies?
Etc etc.

> Signed-off-by: Rui Feng <rui_feng@realsil.com.cn>
> ---
>  drivers/mfd/Makefile         |   2 +-
>  drivers/mfd/rts5260.c        | 543 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/mfd/rtsx_pcr.c       |   8 +
>  drivers/mfd/rtsx_pcr.h       |   1 +
>  include/linux/mfd/rtsx_pci.h |  81 ++++++-
>  5 files changed, 631 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/mfd/rts5260.c
> 
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 080793b..d0f06a3 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -17,7 +17,7 @@ obj-$(CONFIG_MFD_CROS_EC_I2C)	+= cros_ec_i2c.o
>  obj-$(CONFIG_MFD_CROS_EC_SPI)	+= cros_ec_spi.o
>  obj-$(CONFIG_MFD_EXYNOS_LPASS)	+= exynos-lpass.o
>  
> -rtsx_pci-objs			:= rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o
> +rtsx_pci-objs			:= rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o rts5260.o
>  obj-$(CONFIG_MFD_RTSX_PCI)	+= rtsx_pci.o
>  obj-$(CONFIG_MFD_RTSX_USB)	+= rtsx_usb.o
>  
> diff --git a/drivers/mfd/rts5260.c b/drivers/mfd/rts5260.c
> new file mode 100644
> index 0000000..53f2838
> --- /dev/null
> +++ b/drivers/mfd/rts5260.c

There is way too much code in this file to be an MFD driver.

It's a card reader driver (as stated below).

I think this entire file needs relocating somewhere else.

> @@ -0,0 +1,543 @@
> +/* Driver for Realtek PCI-Express card reader
> + *
> + * Copyright(c) 2016-2017 Realtek Semiconductor Corp. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2, or (at your option) any
> + * later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + *
> + * Author:
> + *   Steven FENG <steven_feng@realsil.com.cn>
> + *   Rui FENG <rui_feng@realsil.com.cn>
> + *   Wei WANG <wei_wang@realsil.com.cn>
> + */

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 8+ messages in thread

* 答复: [PATCH] mfd: rtsx: Add support for RTS5260
  2017-09-14  7:57 ` Lee Jones
@ 2017-09-14  9:50   ` 冯锐
  2017-09-14 11:33     ` Lee Jones
  0 siblings, 1 reply; 8+ messages in thread
From: 冯锐 @ 2017-09-14  9:50 UTC (permalink / raw)
  To: Lee Jones; +Cc: linux-kernel

On Fri, 08 Sep 2017, rui_feng@realsil.com.cn wrote:

> From: rui_feng <rui_feng@realsil.com.cn>
> 
> Add support for new chip rts5260.

You are adding over 600 lines in this patch.  It desearves a more forthcoming commit message.

What is it?
What functionality does it provide?
What other subsystems are involved?
Could it break something else?
Does it have any dependencies?
Etc etc.

> Signed-off-by: Rui Feng <rui_feng@realsil.com.cn>
> ---
>  drivers/mfd/Makefile         |   2 +-
>  drivers/mfd/rts5260.c        | 543 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/mfd/rtsx_pcr.c       |   8 +
>  drivers/mfd/rtsx_pcr.h       |   1 +
>  include/linux/mfd/rtsx_pci.h |  81 ++++++-
>  5 files changed, 631 insertions(+), 4 deletions(-)  create mode 
> 100644 drivers/mfd/rts5260.c
> 
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index 
> 080793b..d0f06a3 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -17,7 +17,7 @@ obj-$(CONFIG_MFD_CROS_EC_I2C)	+= cros_ec_i2c.o
>  obj-$(CONFIG_MFD_CROS_EC_SPI)	+= cros_ec_spi.o
>  obj-$(CONFIG_MFD_EXYNOS_LPASS)	+= exynos-lpass.o
>  
> -rtsx_pci-objs			:= rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o
> +rtsx_pci-objs			:= rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o rts5260.o
>  obj-$(CONFIG_MFD_RTSX_PCI)	+= rtsx_pci.o
>  obj-$(CONFIG_MFD_RTSX_USB)	+= rtsx_usb.o
>  
> diff --git a/drivers/mfd/rts5260.c b/drivers/mfd/rts5260.c new file 
> mode 100644 index 0000000..53f2838
> --- /dev/null
> +++ b/drivers/mfd/rts5260.c

There is way too much code in this file to be an MFD driver.

It's a card reader driver (as stated below).

I think this entire file needs relocating somewhere else.

Many other drivers, such as rts5209, rts5227, rts5229, rts5249 and so on, are in the same place.
If I put rts5260 somewhere else, other drivers should also be relocated, this will lead to a much bigger modification.
So I think put rts5260.c here is better suited.

> @@ -0,0 +1,543 @@
> +/* Driver for Realtek PCI-Express card reader
> + *
> + * Copyright(c) 2016-2017 Realtek Semiconductor Corp. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2, or (at your option) any
> + * later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + *
> + * Author:
> + *   Steven FENG <steven_feng@realsil.com.cn>
> + *   Rui FENG <rui_feng@realsil.com.cn>
> + *   Wei WANG <wei_wang@realsil.com.cn>
> + */

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

------Please consider the environment before printing this e-mail.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: 答复: [PATCH] mfd: rtsx: Add support for RTS5260
  2017-09-14  9:50   ` 答复: " 冯锐
@ 2017-09-14 11:33     ` Lee Jones
  2017-09-15  1:56       ` 答复: " 冯锐
  0 siblings, 1 reply; 8+ messages in thread
From: Lee Jones @ 2017-09-14 11:33 UTC (permalink / raw)
  To: 冯锐; +Cc: linux-kernel

On Thu, 14 Sep 2017, 冯锐 wrote:

> On Fri, 08 Sep 2017, rui_feng@realsil.com.cn wrote:
> 
> > From: rui_feng <rui_feng@realsil.com.cn>
> > 
> > Add support for new chip rts5260.
> 
> You are adding over 600 lines in this patch.  It desearves a more forthcoming commit message.
> 
> What is it?
> What functionality does it provide?
> What other subsystems are involved?
> Could it break something else?
> Does it have any dependencies?
> Etc etc.
> 
> > Signed-off-by: Rui Feng <rui_feng@realsil.com.cn>
> > ---
> >  drivers/mfd/Makefile         |   2 +-
> >  drivers/mfd/rts5260.c        | 543 +++++++++++++++++++++++++++++++++++++++++++
> >  drivers/mfd/rtsx_pcr.c       |   8 +
> >  drivers/mfd/rtsx_pcr.h       |   1 +
> >  include/linux/mfd/rtsx_pci.h |  81 ++++++-
> >  5 files changed, 631 insertions(+), 4 deletions(-)  create mode 
> > 100644 drivers/mfd/rts5260.c
> > 
> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index 
> > 080793b..d0f06a3 100644
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -17,7 +17,7 @@ obj-$(CONFIG_MFD_CROS_EC_I2C)	+= cros_ec_i2c.o
> >  obj-$(CONFIG_MFD_CROS_EC_SPI)	+= cros_ec_spi.o
> >  obj-$(CONFIG_MFD_EXYNOS_LPASS)	+= exynos-lpass.o
> >  
> > -rtsx_pci-objs			:= rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o
> > +rtsx_pci-objs			:= rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o rts5260.o
> >  obj-$(CONFIG_MFD_RTSX_PCI)	+= rtsx_pci.o
> >  obj-$(CONFIG_MFD_RTSX_USB)	+= rtsx_usb.o
> >  
> > diff --git a/drivers/mfd/rts5260.c b/drivers/mfd/rts5260.c new file 
> > mode 100644 index 0000000..53f2838
> > --- /dev/null
> > +++ b/drivers/mfd/rts5260.c
> 
> There is way too much code in this file to be an MFD driver.
> 
> It's a card reader driver (as stated below).
> 
> I think this entire file needs relocating somewhere else.

Your reply is confusing, since it does not quote my previous messages.

Please fix your mailer to quote the text you are replying to.

I think I've asked this before.

> Many other drivers, such as rts5209, rts5227, rts5229, rts5249 and so on, are in the same place.
> If I put rts5260 somewhere else, other drivers should also be relocated, this will lead to a much bigger modification.
> So I think put rts5260.c here is better suited.

Just because we made mistakes before, it doesn't mean we should keep
making them.  I think the correct solution is for them all to be
relocated.

> > @@ -0,0 +1,543 @@
> > +/* Driver for Realtek PCI-Express card reader
> > + *
> > + * Copyright(c) 2016-2017 Realtek Semiconductor Corp. All rights reserved.
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License as published by the
> > + * Free Software Foundation; either version 2, or (at your option) any
> > + * later version.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along
> > + * with this program; if not, see <http://www.gnu.org/licenses/>.
> > + *
> > + * Author:
> > + *   Steven FENG <steven_feng@realsil.com.cn>
> > + *   Rui FENG <rui_feng@realsil.com.cn>
> > + *   Wei WANG <wei_wang@realsil.com.cn>
> > + */
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 8+ messages in thread

* 答复: 答复: [PATCH] mfd: rtsx: Add support for RTS5260
  2017-09-14 11:33     ` Lee Jones
@ 2017-09-15  1:56       ` 冯锐
  2017-09-18  8:46         ` Lee Jones
  0 siblings, 1 reply; 8+ messages in thread
From: 冯锐 @ 2017-09-15  1:56 UTC (permalink / raw)
  To: Lee Jones; +Cc: linux-kernel

> On Thu, 14 Sep 2017, 冯锐 wrote:
> 
> > On Fri, 08 Sep 2017, rui_feng@realsil.com.cn wrote:
> >
> > > From: rui_feng <rui_feng@realsil.com.cn>
> > >
> > > Add support for new chip rts5260.
> >
> > You are adding over 600 lines in this patch.  It desearves a more forthcoming
> commit message.
> >
> > What is it?
> > What functionality does it provide?
> > What other subsystems are involved?
> > Could it break something else?
> > Does it have any dependencies?
> > Etc etc.
> >
> > > Signed-off-by: Rui Feng <rui_feng@realsil.com.cn>
> > > ---
> > >  drivers/mfd/Makefile         |   2 +-
> > >  drivers/mfd/rts5260.c        | 543
> +++++++++++++++++++++++++++++++++++++++++++
> > >  drivers/mfd/rtsx_pcr.c       |   8 +
> > >  drivers/mfd/rtsx_pcr.h       |   1 +
> > >  include/linux/mfd/rtsx_pci.h |  81 ++++++-
> > >  5 files changed, 631 insertions(+), 4 deletions(-)  create mode
> > > 100644 drivers/mfd/rts5260.c
> > >
> > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index
> > > 080793b..d0f06a3 100644
> > > --- a/drivers/mfd/Makefile
> > > +++ b/drivers/mfd/Makefile
> > > @@ -17,7 +17,7 @@ obj-$(CONFIG_MFD_CROS_EC_I2C)	+=
> cros_ec_i2c.o
> > >  obj-$(CONFIG_MFD_CROS_EC_SPI)	+= cros_ec_spi.o
> > >  obj-$(CONFIG_MFD_EXYNOS_LPASS)	+= exynos-lpass.o
> > >
> > > -rtsx_pci-objs			:= rtsx_pcr.o rts5209.o rts5229.o rtl8411.o
> rts5227.o rts5249.o
> > > +rtsx_pci-objs			:= rtsx_pcr.o rts5209.o rts5229.o rtl8411.o
> rts5227.o rts5249.o rts5260.o
> > >  obj-$(CONFIG_MFD_RTSX_PCI)	+= rtsx_pci.o
> > >  obj-$(CONFIG_MFD_RTSX_USB)	+= rtsx_usb.o
> > >
> > > diff --git a/drivers/mfd/rts5260.c b/drivers/mfd/rts5260.c new file
> > > mode 100644 index 0000000..53f2838
> > > --- /dev/null
> > > +++ b/drivers/mfd/rts5260.c
> >
> > There is way too much code in this file to be an MFD driver.
> >
> > It's a card reader driver (as stated below).
> >
> > I think this entire file needs relocating somewhere else.
> 
> Your reply is confusing, since it does not quote my previous messages.
> 
> Please fix your mailer to quote the text you are replying to.
> 
> I think I've asked this before.
> 
> > Many other drivers, such as rts5209, rts5227, rts5229, rts5249 and so on,
> are in the same place.
> > If I put rts5260 somewhere else, other drivers should also be relocated, this
> will lead to a much bigger modification.
> > So I think put rts5260.c here is better suited.
> 
> Just because we made mistakes before, it doesn't mean we should keep
> making them.  I think the correct solution is for them all to be relocated.
> 

I'd like to relocate them following two steps:
1. Relocate rts5260.c first and don't move other files.
2. Relocate all other related files.
Each step for one patch, and where do you think is suitable for rts5260.c?

> > > @@ -0,0 +1,543 @@
> > > +/* Driver for Realtek PCI-Express card reader
> > > + *
> > > + * Copyright(c) 2016-2017 Realtek Semiconductor Corp. All rights reserved.
> > > + *
> > > + * This program is free software; you can redistribute it and/or
> > > +modify it
> > > + * under the terms of the GNU General Public License as published
> > > +by the
> > > + * Free Software Foundation; either version 2, or (at your option)
> > > +any
> > > + * later version.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > +but
> > > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > +GNU
> > > + * General Public License for more details.
> > > + *
> > > + * You should have received a copy of the GNU General Public
> > > +License along
> > > + * with this program; if not, see <http://www.gnu.org/licenses/>.
> > > + *
> > > + * Author:
> > > + *   Steven FENG <steven_feng@realsil.com.cn>
> > > + *   Rui FENG <rui_feng@realsil.com.cn>
> > > + *   Wei WANG <wei_wang@realsil.com.cn>
> > > + */
> >
> 
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source
> software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
> 
> ------Please consider the environment before printing this e-mail.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: 答复: 答复: [PATCH] mfd: rtsx: Add support for RTS5260
  2017-09-15  1:56       ` 答复: " 冯锐
@ 2017-09-18  8:46         ` Lee Jones
  0 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2017-09-18  8:46 UTC (permalink / raw)
  To: 冯锐; +Cc: linux-kernel

On Fri, 15 Sep 2017, 冯锐 wrote:

> > On Thu, 14 Sep 2017, 冯锐 wrote:
> > 
> > > On Fri, 08 Sep 2017, rui_feng@realsil.com.cn wrote:
> > >
> > > > From: rui_feng <rui_feng@realsil.com.cn>
> > > >
> > > > Add support for new chip rts5260.
> > >
> > > You are adding over 600 lines in this patch.  It desearves a more forthcoming
> > commit message.
> > >
> > > What is it?
> > > What functionality does it provide?
> > > What other subsystems are involved?
> > > Could it break something else?
> > > Does it have any dependencies?
> > > Etc etc.
> > >
> > > > Signed-off-by: Rui Feng <rui_feng@realsil.com.cn>
> > > > ---
> > > >  drivers/mfd/Makefile         |   2 +-
> > > >  drivers/mfd/rts5260.c        | 543
> > +++++++++++++++++++++++++++++++++++++++++++
> > > >  drivers/mfd/rtsx_pcr.c       |   8 +
> > > >  drivers/mfd/rtsx_pcr.h       |   1 +
> > > >  include/linux/mfd/rtsx_pci.h |  81 ++++++-
> > > >  5 files changed, 631 insertions(+), 4 deletions(-)  create mode
> > > > 100644 drivers/mfd/rts5260.c
> > > >
> > > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index
> > > > 080793b..d0f06a3 100644
> > > > --- a/drivers/mfd/Makefile
> > > > +++ b/drivers/mfd/Makefile
> > > > @@ -17,7 +17,7 @@ obj-$(CONFIG_MFD_CROS_EC_I2C)	+=
> > cros_ec_i2c.o
> > > >  obj-$(CONFIG_MFD_CROS_EC_SPI)	+= cros_ec_spi.o
> > > >  obj-$(CONFIG_MFD_EXYNOS_LPASS)	+= exynos-lpass.o
> > > >
> > > > -rtsx_pci-objs			:= rtsx_pcr.o rts5209.o rts5229.o rtl8411.o
> > rts5227.o rts5249.o
> > > > +rtsx_pci-objs			:= rtsx_pcr.o rts5209.o rts5229.o rtl8411.o
> > rts5227.o rts5249.o rts5260.o
> > > >  obj-$(CONFIG_MFD_RTSX_PCI)	+= rtsx_pci.o
> > > >  obj-$(CONFIG_MFD_RTSX_USB)	+= rtsx_usb.o
> > > >
> > > > diff --git a/drivers/mfd/rts5260.c b/drivers/mfd/rts5260.c new file
> > > > mode 100644 index 0000000..53f2838
> > > > --- /dev/null
> > > > +++ b/drivers/mfd/rts5260.c
> > >
> > > There is way too much code in this file to be an MFD driver.
> > >
> > > It's a card reader driver (as stated below).
> > >
> > > I think this entire file needs relocating somewhere else.
> > 
> > Your reply is confusing, since it does not quote my previous messages.
> > 
> > Please fix your mailer to quote the text you are replying to.
> > 
> > I think I've asked this before.
> > 
> > > Many other drivers, such as rts5209, rts5227, rts5229, rts5249 and so on,
> > are in the same place.
> > > If I put rts5260 somewhere else, other drivers should also be relocated, this
> > will lead to a much bigger modification.
> > > So I think put rts5260.c here is better suited.
> > 
> > Just because we made mistakes before, it doesn't mean we should keep
> > making them.  I think the correct solution is for them all to be relocated.
> > 
> 
> I'd like to relocate them following two steps:
> 1. Relocate rts5260.c first and don't move other files.
> 2. Relocate all other related files.
> Each step for one patch, and where do you think is suitable for rts5260.c?

How about drivers/memstick?
Failing that, drivers/misc is always an option.

> > > > @@ -0,0 +1,543 @@
> > > > +/* Driver for Realtek PCI-Express card reader
> > > > + *
> > > > + * Copyright(c) 2016-2017 Realtek Semiconductor Corp. All rights reserved.
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or
> > > > +modify it
> > > > + * under the terms of the GNU General Public License as published
> > > > +by the
> > > > + * Free Software Foundation; either version 2, or (at your option)
> > > > +any
> > > > + * later version.
> > > > + *
> > > > + * This program is distributed in the hope that it will be useful,
> > > > +but
> > > > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > > +GNU
> > > > + * General Public License for more details.
> > > > + *
> > > > + * You should have received a copy of the GNU General Public
> > > > +License along
> > > > + * with this program; if not, see <http://www.gnu.org/licenses/>.
> > > > + *
> > > > + * Author:
> > > > + *   Steven FENG <steven_feng@realsil.com.cn>
> > > > + *   Rui FENG <rui_feng@realsil.com.cn>
> > > > + *   Wei WANG <wei_wang@realsil.com.cn>
> > > > + */
> > >
> > 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: 答复: [PATCH] mfd: rtsx: Add support for RTS5260
  2017-09-22 10:02   ` 答复: " 冯锐
@ 2017-09-22 12:02     ` Lee Jones
  0 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2017-09-22 12:02 UTC (permalink / raw)
  To: 冯锐; +Cc: Greg KH, arnd, linux-kernel

On Fri, 22 Sep 2017, 冯锐 wrote:

> > On Fri, Sep 22, 2017 at 05:36:24PM +0800, rui_feng@realsil.com.cn wrote:
> > > From: rui_feng <rui_feng@realsil.com.cn>
> > >
> > > Add support for new chip rts5260.
> > > In order to support rts5260,the definitions of some internal registers
> > > and workflow have to be modified and are different from its
> > > predecessors. So we need this patch to ensure RTS5260 can work.
> > >
> > > Signed-off-by: Rui Feng <rui_feng@realsil.com.cn>
> > 
> > Nit, this name needs to match your "From:" line.  It almost does :)
> > 
> > Why is this a drivers/misc/ driver?
> > 
> I want to put this driver in mfd because other drivers like rts5249,rts5227 and so on are in mfd, but lee jones said 
> " There is way too much code in this file to be an MFD driver.

There's more to it than that.

It's not an MFD driver because it's a card reader an nothing else.

MFDs cross subsystem boundaries, whereas this is just a card reader.

MFD isn't a dumping ground for devices which don't belong anywhere
else.

> It's a card reader driver (as stated below).
> I think this entire file needs relocating somewhere else."
> So I put it here.

Without creating a new subsystem, I can't think of a better place for
it either.

> > > --- /dev/null
> > > +++ b/drivers/misc/rts5260.c
> > > @@ -0,0 +1,590 @@
> > > +/* Driver for Realtek PCI-Express card reader
> > > + *
> > > + * Copyright(c) 2016-2017 Realtek Semiconductor Corp. All rights reserved.
> > > + *
> > > + * This program is free software; you can redistribute it and/or
> > > +modify it
> > > + * under the terms of the GNU General Public License as published by
> > > +the
> > > + * Free Software Foundation; either version 2, or (at your option)
> > > +any
> > > + * later version.
> > 
> > Do you really mean "any later version"?  (I have to ask...)
> > 
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + but
> > > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > GNU
> > > + * General Public License for more details.
> > > + *
> > > + * You should have received a copy of the GNU General Public License
> > > + along
> > > + * with this program; if not, see <http://www.gnu.org/licenses/>.
> > > + *
> > > + * Author:
> > > + *   Steven FENG <steven_feng@realsil.com.cn>
> > > + *   Rui FENG <rui_feng@realsil.com.cn>
> > > + *   Wei WANG <wei_wang@realsil.com.cn>
> > > + */
> > > +
> > > +#include <linux/module.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/mfd/rtsx_pci.h>
> > > +
> > > +#include "../mfd/rtsx_pcr.h"
> > 
> > That looks "odd" :(

This is odd.

You should move that file into include/.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 8+ messages in thread

* 答复: [PATCH] mfd: rtsx: Add support for RTS5260
  2017-09-22  9:54 ` Greg KH
@ 2017-09-22 10:02   ` 冯锐
  2017-09-22 12:02     ` Lee Jones
  0 siblings, 1 reply; 8+ messages in thread
From: 冯锐 @ 2017-09-22 10:02 UTC (permalink / raw)
  To: Greg KH; +Cc: lee.jones, arnd, linux-kernel

> On Fri, Sep 22, 2017 at 05:36:24PM +0800, rui_feng@realsil.com.cn wrote:
> > From: rui_feng <rui_feng@realsil.com.cn>
> >
> > Add support for new chip rts5260.
> > In order to support rts5260,the definitions of some internal registers
> > and workflow have to be modified and are different from its
> > predecessors. So we need this patch to ensure RTS5260 can work.
> >
> > Signed-off-by: Rui Feng <rui_feng@realsil.com.cn>
> 
> Nit, this name needs to match your "From:" line.  It almost does :)
> 
> Why is this a drivers/misc/ driver?
> 
I want to put this driver in mfd because other drivers like rts5249,rts5227 and so on are in mfd, but lee jones said 
" There is way too much code in this file to be an MFD driver.
It's a card reader driver (as stated below).
I think this entire file needs relocating somewhere else."
So I put it here.

> > --- /dev/null
> > +++ b/drivers/misc/rts5260.c
> > @@ -0,0 +1,590 @@
> > +/* Driver for Realtek PCI-Express card reader
> > + *
> > + * Copyright(c) 2016-2017 Realtek Semiconductor Corp. All rights reserved.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > +modify it
> > + * under the terms of the GNU General Public License as published by
> > +the
> > + * Free Software Foundation; either version 2, or (at your option)
> > +any
> > + * later version.
> 
> Do you really mean "any later version"?  (I have to ask...)
> 
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU
> > + * General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + along
> > + * with this program; if not, see <http://www.gnu.org/licenses/>.
> > + *
> > + * Author:
> > + *   Steven FENG <steven_feng@realsil.com.cn>
> > + *   Rui FENG <rui_feng@realsil.com.cn>
> > + *   Wei WANG <wei_wang@realsil.com.cn>
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/delay.h>
> > +#include <linux/mfd/rtsx_pci.h>
> > +
> > +#include "../mfd/rtsx_pcr.h"
> 
> That looks "odd" :(
> 
> thanks,
> 
> greg k-h
> 
> ------Please consider the environment before printing this e-mail.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-09-22 12:02 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-08  6:57 [PATCH] mfd: rtsx: Add support for RTS5260 rui_feng
2017-09-14  7:57 ` Lee Jones
2017-09-14  9:50   ` 答复: " 冯锐
2017-09-14 11:33     ` Lee Jones
2017-09-15  1:56       ` 答复: " 冯锐
2017-09-18  8:46         ` Lee Jones
2017-09-22  9:36 rui_feng
2017-09-22  9:54 ` Greg KH
2017-09-22 10:02   ` 答复: " 冯锐
2017-09-22 12:02     ` Lee Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).