Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1717064 > unrolled thread

[PATCH v3] mfd: Add support for RTS5250S power saving

Started by<rui_feng@realsil.com.cn>
First post2017-08-22 08:40 +0200
Last post2017-08-22 12:40 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v3] mfd: Add support for RTS5250S power saving <rui_feng@realsil.com.cn> - 2017-08-22 08:40 +0200
    Re: [PATCH v3] mfd: Add support for RTS5250S power saving Lee Jones <lee.jones@linaro.org> - 2017-08-22 09:40 +0200
      Re: 答复: [PATCH v3] mfd: Add support  for RTS5250S power saving Lee Jones <lee.jones@linaro.org> - 2017-08-22 12:40 +0200

#1717064 — [PATCH v3] mfd: Add support for RTS5250S power saving

From<rui_feng@realsil.com.cn>
Date2017-08-22 08:40 +0200
Subject[PATCH v3] mfd: Add support for RTS5250S power saving
Message-ID<uhb0Z-6Xp-3@gated-at.bofh.it>
From: Rui Feng <rui_feng@realsil.com.cn>

Signed-off-by: Rui Feng <rui_feng@realsil.com.cn>
---
 drivers/mfd/rts5249.c        | 195 ++++++++++++++++++++++++++++++++++++++++
 drivers/mfd/rtsx_pcr.c       | 205 +++++++++++++++++++++++++++++++++++++++++--
 drivers/mfd/rtsx_pcr.h       |  11 +++
 include/linux/mfd/rtsx_pci.h |  60 +++++++++++++
 4 files changed, 466 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/rts5249.c b/drivers/mfd/rts5249.c
index 40f8bb1..3959ada 100644
--- a/drivers/mfd/rts5249.c
+++ b/drivers/mfd/rts5249.c
@@ -103,8 +103,72 @@ static void rtsx_base_force_power_down(struct rtsx_pcr *pcr, u8 pm_state)
 	rtsx_pci_write_register(pcr, FPDCTL, 0x03, 0x03);
 }
 
+static int rts5249_init_from_cfg(struct rtsx_pcr *pcr)
+{
+	struct rtsx_cr_option *option = &(pcr->option);
+	u32 lval;
+
+	if (CHK_PCI_PID(pcr, PID_524A))
+		rtsx_pci_read_config_dword(pcr,
+			PCR_ASPM_SETTING_REG1, &lval);
+	else
+		rtsx_pci_read_config_dword(pcr,
+			PCR_ASPM_SETTING_REG2, &lval);
+
+	if (lval & ASPM_L1_1_EN_MASK)
+		rtsx_set_dev_flag(pcr, ASPM_L1_1_EN);
+	else
+		rtsx_clear_dev_flag(pcr, ASPM_L1_1_EN);
+	if (lval & ASPM_L1_2_EN_MASK)
+		rtsx_set_dev_flag(pcr, ASPM_L1_2_EN);
+	else
+		rtsx_clear_dev_flag(pcr, ASPM_L1_2_EN);
+
+	if (lval & PM_L1_1_EN_MASK)
+		rtsx_set_dev_flag(pcr, PM_L1_1_EN);
+	else
+		rtsx_clear_dev_flag(pcr, PM_L1_1_EN);
+	if (lval & PM_L1_2_EN_MASK)
+		rtsx_set_dev_flag(pcr, PM_L1_2_EN);
+	else
+		rtsx_clear_dev_flag(pcr, PM_L1_2_EN);
+
+	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;
+		}
+	}
+
+	return 0;
+}
+
+static int rts5249_init_from_hw(struct rtsx_pcr *pcr)
+{
+	struct rtsx_cr_option *option = &(pcr->option);
+
+	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;
+
+	return 0;
+}
+
 static int rts5249_extra_init_hw(struct rtsx_pcr *pcr)
 {
+	struct rtsx_cr_option *option = &(pcr->option);
+
+	rts5249_init_from_cfg(pcr);
+	rts5249_init_from_hw(pcr);
+
 	rtsx_pci_init_cmd(pcr);
 
 	/* Rest L1SUB Config */
@@ -125,6 +189,17 @@ static int rts5249_extra_init_hw(struct rtsx_pcr *pcr)
 	else
 		rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, PETXCFG, 0xB0, 0x80);
 
+	/*
+	 * u_force_clkreq_0
+	 * If true, CLKREQ# PIN will be forced to drive low to 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 rtsx_pci_send_cmd(pcr, 100);
 }
 
@@ -353,6 +428,8 @@ static int rtsx_base_switch_output_voltage(struct rtsx_pcr *pcr, u8 voltage)
 
 void rts5249_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->ops = &rts5249_pcr_ops;
@@ -372,6 +449,20 @@ void rts5249_init_params(struct rtsx_pcr *pcr)
 	pcr->ms_pull_ctl_disable_tbl = rts5249_ms_pull_ctl_disable_tbl;
 
 	pcr->reg_pm_ctrl3 = PM_CTRL3;
+
+	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_DEFAULT;
+	option->ltr_idle_latency = LTR_IDLE_LATENCY_DEFAULT;
+	option->ltr_l1off_latency = LTR_L1OFF_LATENCY_DEFAULT;
+	option->dev_aspm_mode = DEV_ASPM_DYNAMIC;
+	option->l1_snooze_delay = 1;
+	option->ltr_l1off_sspwrgate = LTR_L1OFF_SSPWRGATE_5249_DEFAULT;
+	option->ltr_l1off_snooze_sspwrgate =
+		LTR_L1OFF_SNOOZE_SSPWRGATE_5249_DEFAULT;
 }
 
 static int rts524a_write_phy(struct rtsx_pcr *pcr, u8 addr, u16 val)
@@ -459,6 +550,54 @@ static int rts524a_extra_init_hw(struct rtsx_pcr *pcr)
 	return 0;
 }
 
+static void rts5250_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;
+
+	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) {
+			u8 val = option->ltr_l1off_snooze_sspwrgate;
+
+			if (rtsx_check_dev_flag(pcr,
+					LTR_L1SS_PWR_GATE_CHECK_CARD_EN)) {
+				if (card_exist)
+					val &= ~(1 << 7);
+				else
+					val |= (1 << 7);
+			}
+
+			rtsx_set_l1off_sub(pcr, val);
+		} else {
+			rtsx_set_l1off_sub(pcr, 0);
+		}
+	} else {
+		/* l1off, latency: 300us */
+		if (aspm_L1_2) {
+			u8 val = option->ltr_l1off_sspwrgate;
+
+			if (rtsx_check_dev_flag(pcr,
+					LTR_L1SS_PWR_GATE_CHECK_CARD_EN)) {
+				if (card_exist)
+					val &= ~(1 << 7);
+				else
+					val |= (1 << 7);
+			}
+
+			rtsx_set_l1off_sub(pcr, val);
+		} else {
+			rtsx_set_l1off_sub(pcr, 0);
+		}
+	}
+}
+
 static const struct pcr_ops rts524a_pcr_ops = {
 	.write_phy = rts524a_write_phy,
 	.read_phy = rts524a_read_phy,
@@ -473,11 +612,15 @@ static int rts524a_extra_init_hw(struct rtsx_pcr *pcr)
 	.card_power_off = rtsx_base_card_power_off,
 	.switch_output_voltage = rtsx_base_switch_output_voltage,
 	.force_power_down = rtsx_base_force_power_down,
+	.set_l1off_cfg_sub_d0 = rts5250_set_l1off_cfg_sub_d0,
 };
 
 void rts524a_init_params(struct rtsx_pcr *pcr)
 {
 	rts5249_init_params(pcr);
+	pcr->option.ltr_l1off_sspwrgate = LTR_L1OFF_SSPWRGATE_5250_DEFAULT;
+	pcr->option.ltr_l1off_snooze_sspwrgate =
+		LTR_L1OFF_SNOOZE_SSPWRGATE_5250_DEFAULT;
 
 	pcr->reg_pm_ctrl3 = RTS524A_PM_CTRL3;
 	pcr->ops = &rts524a_pcr_ops;
@@ -564,6 +707,52 @@ static int rts525a_extra_init_hw(struct rtsx_pcr *pcr)
 	return 0;
 }
 
+static void rts5249_enable_aspm(struct rtsx_pcr *pcr)
+{
+	struct rtsx_cr_option *option = &pcr->option;
+
+	if (pcr->aspm_enabled)
+		return;
+
+	if (option->dev_aspm_mode == DEV_ASPM_DISABLE) {
+		pcr->aspm_enabled = true;
+		return;
+	}
+	if (option->dev_aspm_mode == DEV_ASPM_DYNAMIC) {
+		rtsx_pci_update_cfg_byte(pcr,
+			pcr->pcie_cap + PCI_EXP_LNKCTL,
+			0xFC, pcr->aspm_en);
+	} else if (option->dev_aspm_mode == DEV_ASPM_BACKDOOR) {
+		u8 mask = FORCE_ASPM_VAL_MASK | FORCE_ASPM_CTL0;
+
+		rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, mask, 0);
+	}
+	pcr->aspm_enabled = true;
+}
+
+static void rts5249_disable_aspm(struct rtsx_pcr *pcr)
+{
+	struct rtsx_cr_option *option = &pcr->option;
+
+	if (!pcr->aspm_enabled)
+		return;
+	if (option->dev_aspm_mode == DEV_ASPM_DISABLE) {
+		pcr->aspm_enabled = false;
+		return;
+	}
+	if (option->dev_aspm_mode == DEV_ASPM_DYNAMIC) {
+		rtsx_pci_update_cfg_byte(pcr,
+			pcr->pcie_cap + PCI_EXP_LNKCTL,
+			0xFC, 0);
+	} else if (option->dev_aspm_mode == DEV_ASPM_BACKDOOR) {
+		u8 mask = FORCE_ASPM_VAL_MASK | FORCE_ASPM_CTL0;
+
+		rtsx_pci_write_register(pcr, ASPM_FORCE_CTL,
+					mask, FORCE_ASPM_CTL0);
+	}
+	pcr->aspm_enabled = false;
+}
+
 static const struct pcr_ops rts525a_pcr_ops = {
 	.fetch_vendor_settings = rtsx_base_fetch_vendor_settings,
 	.extra_init_hw = rts525a_extra_init_hw,
@@ -576,11 +765,17 @@ static int rts525a_extra_init_hw(struct rtsx_pcr *pcr)
 	.card_power_off = rtsx_base_card_power_off,
 	.switch_output_voltage = rts525a_switch_output_voltage,
 	.force_power_down = rtsx_base_force_power_down,
+	.set_l1off_cfg_sub_d0 = rts5250_set_l1off_cfg_sub_d0,
+	.enable_aspm = rts5249_enable_aspm,
+	.disable_aspm = rts5249_disable_aspm,
 };
 
 void rts525a_init_params(struct rtsx_pcr *pcr)
 {
 	rts5249_init_params(pcr);
+	pcr->option.ltr_l1off_sspwrgate = LTR_L1OFF_SSPWRGATE_5250_DEFAULT;
+	pcr->option.ltr_l1off_snooze_sspwrgate =
+		LTR_L1OFF_SNOOZE_SSPWRGATE_5250_DEFAULT;
 
 	pcr->reg_pm_ctrl3 = RTS524A_PM_CTRL3;
 	pcr->ops = &rts525a_pcr_ops;
diff --git a/drivers/mfd/rtsx_pcr.c b/drivers/mfd/rtsx_pcr.c
index a0ac89d..2ff398d 100644
--- a/drivers/mfd/rtsx_pcr.c
+++ b/drivers/mfd/rtsx_pcr.c
@@ -79,6 +79,142 @@ static inline void rtsx_pci_disable_aspm(struct rtsx_pcr *pcr)
 		0xFC, 0);
 }
 
+int rtsx_comm_set_ltr_latency(struct rtsx_pcr *pcr, u32 latency)
+{
+	rtsx_pci_write_register(pcr, MSGTXDATA0, 0xFF, (u8) (latency & 0xFF));
+	rtsx_pci_write_register(pcr, MSGTXDATA1,
+				0xFF, (u8)((latency >> 8) & 0xFF));
+	rtsx_pci_write_register(pcr, MSGTXDATA2,
+				0xFF, (u8)((latency >> 16) & 0xFF));
+	rtsx_pci_write_register(pcr, MSGTXDATA3,
+				0xFF, (u8)((latency >> 24) & 0xFF));
+	rtsx_pci_write_register(pcr, LTR_CTL, 0xC0, 0xC0);
+
+	return 0;
+}
+
+int rtsx_set_ltr_latency(struct rtsx_pcr *pcr, u32 latency)
+{
+	if (pcr->ops->set_ltr_latency)
+		return pcr->ops->set_ltr_latency(pcr, latency);
+	else
+		return rtsx_comm_set_ltr_latency(pcr, latency);
+
+	return 0;
+}
+
+static void rtsx_comm_disable_aspm(struct rtsx_pcr *pcr)
+{
+	struct rtsx_cr_option *option = &pcr->option;
+
+	if (!pcr->aspm_enabled)
+		return;
+	if (option->dev_aspm_mode == DEV_ASPM_DISABLE) {
+		pcr->aspm_enabled = false;
+		return;
+	}
+	if (option->dev_aspm_mode == DEV_ASPM_DYNAMIC) {
+		rtsx_pci_disable_aspm(pcr);
+	} else if (option->dev_aspm_mode == DEV_ASPM_BACKDOOR) {
+		u8 mask = FORCE_ASPM_VAL_MASK;
+
+		rtsx_pci_write_register(pcr, ASPM_FORCE_CTL,  mask, 0);
+	}
+	msleep(20);
+	pcr->aspm_enabled = false;
+}
+
+static void rtsx_disable_aspm(struct rtsx_pcr *pcr)
+{
+	if (pcr->ops->disable_aspm)
+		pcr->ops->disable_aspm(pcr);
+	else
+		rtsx_comm_disable_aspm(pcr);
+}
+
+int rtsx_set_l1off_sub(struct rtsx_pcr *pcr, u8 val)
+{
+	rtsx_pci_write_register(pcr, L1SUB_CONFIG3, 0xFF, val);
+
+	return 0;
+}
+
+static void rtsx_comm_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;
+
+	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) {
+			u8 val = option->ltr_l1off_snooze_sspwrgate;
+
+			if (rtsx_check_dev_flag(pcr,
+					LTR_L1SS_PWR_GATE_CHECK_CARD_EN)) {
+				if (card_exist)
+					val &= ~(1 << 4);
+				else
+					val |= (1 << 4);
+			}
+
+			rtsx_set_l1off_sub(pcr, val);
+		} else {
+			rtsx_set_l1off_sub(pcr, 0);
+		}
+	} else {
+		/* l1off, latency: 300us */
+		if (aspm_L1_2) {
+			u8 val = option->ltr_l1off_sspwrgate;
+
+			if (rtsx_check_dev_flag(pcr,
+					LTR_L1SS_PWR_GATE_CHECK_CARD_EN)) {
+				if (card_exist)
+					val &= ~(1 << 4);
+				else
+					val |= (1 << 4);
+			}
+
+			rtsx_set_l1off_sub(pcr, val);
+		} else {
+			rtsx_set_l1off_sub(pcr, 0);
+		}
+	}
+}
+
+void rtsx_set_l1off_sub_cfg_d0(struct rtsx_pcr *pcr, int active)
+{
+	if (pcr->ops->set_l1off_cfg_sub_d0)
+		pcr->ops->set_l1off_cfg_sub_d0(pcr, active);
+	else
+		rtsx_comm_set_l1off_cfg_sub_d0(pcr, active);
+}
+
+static void rtsx_comm_pm_full_on(struct rtsx_pcr *pcr)
+{
+	struct rtsx_cr_option *option = &pcr->option;
+
+	rtsx_disable_aspm(pcr);
+
+	if (option->ltr_enabled)
+		rtsx_set_ltr_latency(pcr, option->ltr_active_latency);
+
+	if (rtsx_check_dev_flag(pcr, LTR_L1SS_PWR_GATE_EN))
+		rtsx_set_l1off_sub_cfg_d0(pcr, 1);
+}
+
+void rtsx_pm_full_on(struct rtsx_pcr *pcr)
+{
+	if (pcr->ops->full_on)
+		pcr->ops->full_on(pcr);
+	else
+		rtsx_comm_pm_full_on(pcr);
+}
+
 void rtsx_pci_start_run(struct rtsx_pcr *pcr)
 {
 	/* If pci device removed, don't queue idle work any more */
@@ -89,9 +225,7 @@ void rtsx_pci_start_run(struct rtsx_pcr *pcr)
 		pcr->state = PDEV_STAT_RUN;
 		if (pcr->ops->enable_auto_blink)
 			pcr->ops->enable_auto_blink(pcr);
-
-		if (pcr->aspm_en)
-			rtsx_pci_disable_aspm(pcr);
+		rtsx_pm_full_on(pcr);
 	}
 
 	mod_delayed_work(system_wq, &pcr->idle_work, msecs_to_jiffies(200));
@@ -958,6 +1092,63 @@ static int rtsx_pci_acquire_irq(struct rtsx_pcr *pcr)
 	return 0;
 }
 
+static void rtsx_comm_enable_aspm(struct rtsx_pcr *pcr)
+{
+	struct rtsx_cr_option *option = &pcr->option;
+
+	if (pcr->aspm_enabled)
+		return;
+	if (option->dev_aspm_mode == DEV_ASPM_DISABLE) {
+		pcr->aspm_enabled = true;
+		return;
+	}
+
+	if (option->dev_aspm_mode == DEV_ASPM_DYNAMIC) {
+		rtsx_pci_enable_aspm(pcr);
+	} else if (option->dev_aspm_mode == DEV_ASPM_BACKDOOR) {
+		u8 mask = FORCE_ASPM_VAL_MASK;
+		u8 val = pcr->aspm_en;
+
+		rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, mask, val);
+	}
+	pcr->aspm_enabled = true;
+}
+
+static void rtsx_enable_aspm(struct rtsx_pcr *pcr)
+{
+	if (pcr->ops->enable_aspm)
+		pcr->ops->enable_aspm(pcr);
+	else
+		rtsx_comm_enable_aspm(pcr);
+}
+
+static void rtsx_comm_pm_power_saving(struct rtsx_pcr *pcr)
+{
+	struct rtsx_cr_option *option = &pcr->option;
+
+	if (option->ltr_enabled) {
+		u32 latency = option->ltr_l1off_latency;
+
+		if (rtsx_check_dev_flag(pcr, L1_SNOOZE_TEST_EN))
+			mdelay(option->l1_snooze_delay);
+
+		rtsx_set_ltr_latency(pcr, latency);
+	}
+
+	if (rtsx_check_dev_flag(pcr, LTR_L1SS_PWR_GATE_EN))
+		rtsx_set_l1off_sub_cfg_d0(pcr, 0);
+
+	rtsx_enable_aspm(pcr);
+}
+
+void rtsx_pm_power_saving(struct rtsx_pcr *pcr)
+{
+	if (pcr->ops->power_saving)
+		pcr->ops->power_saving(pcr);
+	else
+		rtsx_comm_pm_power_saving(pcr);
+}
+
 static void rtsx_pci_idle_work(struct work_struct *work)
 {
 	struct delayed_work *dwork = to_delayed_work(work);
@@ -974,8 +1165,7 @@ static void rtsx_pci_idle_work(struct work_struct *work)
 	if (pcr->ops->turn_off_led)
 		pcr->ops->turn_off_led(pcr);
 
-	if (pcr->aspm_en)
-		rtsx_pci_enable_aspm(pcr);
+	rtsx_pm_power_saving(pcr);
 
 	mutex_unlock(&pcr->pcr_mutex);
 }
@@ -1063,6 +1253,11 @@ static int rtsx_pci_init_hw(struct rtsx_pcr *pcr)
 	if (err < 0)
 		return err;
 
+	if (CHK_PCI_PID(pcr, 0x5250) ||
+		CHK_PCI_PID(pcr, 0x524A) ||
+		CHK_PCI_PID(pcr, 0x525A))
+		rtsx_pci_write_register(pcr, PM_CLK_FORCE_CTL, 1, 1);
+
 	/* Enable clk_request_n to enable clock power management */
 	rtsx_pci_write_config_byte(pcr, pcr->pcie_cap + PCI_EXP_LNKCTL + 1, 1);
 	/* Enter L1 when host tx idle */
diff --git a/drivers/mfd/rtsx_pcr.h b/drivers/mfd/rtsx_pcr.h
index 931d1ae..d3baaec 100644
--- a/drivers/mfd/rtsx_pcr.h
+++ b/drivers/mfd/rtsx_pcr.h
@@ -32,6 +32,15 @@
 #define RTS524A_PME_FORCE_CTL		0xFF78
 #define RTS524A_PM_CTRL3		0xFF7E
 
+#define LTR_ACTIVE_LATENCY_DEFAULT	0x883C
+#define LTR_IDLE_LATENCY_DEFAULT		0x892C
+#define LTR_L1OFF_LATENCY_DEFAULT		0x9003
+#define LTR_L1OFF_SSPWRGATE_5249_DEFAULT	0xAF
+#define LTR_L1OFF_SSPWRGATE_5250_DEFAULT	0xFF
+#define LTR_L1OFF_SNOOZE_SSPWRGATE_5249_DEFAULT	0xAC
+#define LTR_L1OFF_SNOOZE_SSPWRGATE_5250_DEFAULT	0xF8
+
+
 int __rtsx_pci_write_phy_register(struct rtsx_pcr *pcr, u8 addr, u16 val);
 int __rtsx_pci_read_phy_register(struct rtsx_pcr *pcr, u8 addr, u16 *val);
 
@@ -85,5 +94,7 @@ static inline u8 map_sd_drive(int idx)
 
 /* generic operations */
 int rtsx_gops_pm_reset(struct rtsx_pcr *pcr);
+int rtsx_set_ltr_latency(struct rtsx_pcr *pcr, u32 latency);
+int rtsx_set_l1off_sub(struct rtsx_pcr *pcr, u8 val);
 
 #endif
diff --git a/include/linux/mfd/rtsx_pci.h b/include/linux/mfd/rtsx_pci.h
index 116816f..62b50e4 100644
--- a/include/linux/mfd/rtsx_pci.h
+++ b/include/linux/mfd/rtsx_pci.h
@@ -621,6 +621,9 @@
 
 #define AUTOLOAD_CFG_BASE		0xFF00
 #define PETXCFG				0xFF03
+#define FORCE_CLKREQ_DELINK_MASK	BIT(7)
+#define FORCE_CLKREQ_LOW	0x80
+#define FORCE_CLKREQ_HIGH	0x00
 
 #define PM_CTRL1			0xFF44
 #define   CD_RESUME_EN_MASK		0xF0
@@ -844,6 +847,9 @@
 #define   PHY_DIG1E_RX_EN_KEEP		0x0001
 #define PHY_DUM_REG			0x1F
 
+#define PCR_ASPM_SETTING_REG1		0x160
+#define PCR_ASPM_SETTING_REG2		0x168
+
 #define PCR_SETTING_REG1		0x724
 #define PCR_SETTING_REG2		0x814
 #define PCR_SETTING_REG3		0x747
@@ -876,14 +882,65 @@ 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 (*enable_aspm)(struct rtsx_pcr *pcr);
+	void (*disable_aspm)(struct rtsx_pcr *pcr);
+	int (*set_ltr_latency)(struct rtsx_pcr *pcr, u32 latency);
+	int (*set_l1off_sub)(struct rtsx_pcr *pcr, u8 val);
+	void (*set_l1off_cfg_sub_d0)(struct rtsx_pcr *pcr, int active);
+	void (*full_on)(struct rtsx_pcr *pcr);
+	void (*power_saving)(struct rtsx_pcr *pcr);
 };
 
 enum PDEV_STAT  {PDEV_STAT_IDLE, PDEV_STAT_RUN};
 
+#define ASPM_L1_1_EN_MASK		BIT(3)
+#define ASPM_L1_2_EN_MASK		BIT(2)
+#define PM_L1_1_EN_MASK		BIT(1)
+#define PM_L1_2_EN_MASK		BIT(0)
+
+#define ASPM_L1_1_EN			BIT(0)
+#define ASPM_L1_2_EN			BIT(1)
+#define PM_L1_1_EN				BIT(2)
+#define PM_L1_2_EN				BIT(3)
+#define LTR_L1SS_PWR_GATE_EN	BIT(4)
+#define L1_SNOOZE_TEST_EN		BIT(5)
+#define LTR_L1SS_PWR_GATE_CHECK_CARD_EN	BIT(6)
+
+enum dev_aspm_mode {
+	DEV_ASPM_DISABLE = 0,
+	DEV_ASPM_DYNAMIC,
+	DEV_ASPM_BACKDOOR,
+	DEV_ASPM_STATIC,
+};
+
+struct rtsx_cr_option {
+	u32 dev_flags;
+	bool force_clkreq_0;
+	bool ltr_en;
+	bool ltr_enabled;
+	bool ltr_active;
+	u32 ltr_active_latency;
+	u32 ltr_idle_latency;
+	u32 ltr_l1off_latency;
+	enum dev_aspm_mode dev_aspm_mode;
+	u32 l1_snooze_delay;
+	u8 ltr_l1off_sspwrgate;
+	u8 ltr_l1off_snooze_sspwrgate;
+};
+
+#define rtsx_set_dev_flag(cr, flag) \
+	((cr)->option.dev_flags |= (flag))
+#define rtsx_clear_dev_flag(cr, flag) \
+	((cr)->option.dev_flags &= ~(flag))
+#define rtsx_check_dev_flag(cr, flag) \
+	((cr)->option.dev_flags & (flag))
+
 struct rtsx_pcr {
 	struct pci_dev			*pci;
 	unsigned int			id;
 	int				pcie_cap;
+	struct rtsx_cr_option	option;
 
 	/* pci resources */
 	unsigned long			addr;
@@ -940,6 +997,7 @@ struct rtsx_pcr {
 	u8				card_drive_sel;
 #define ASPM_L1_EN			0x02
 	u8				aspm_en;
+	bool				aspm_enabled;
 
 #define PCR_MS_PMOS			(1 << 0)
 #define PCR_REVERSE_SOCKET		(1 << 1)
@@ -964,6 +1022,8 @@ struct rtsx_pcr {
 	u8				dma_error_count;
 };
 
+#define PID_524A	0x524A
+
 #define CHK_PCI_PID(pcr, pid)		((pcr)->pci->device == (pid))
 #define PCI_VID(pcr)			((pcr)->pci->vendor)
 #define PCI_PID(pcr)			((pcr)->pci->device)
-- 
1.9.1

[toc] | [next] | [standalone]

#1717093

FromLee Jones <lee.jones@linaro.org>
Date2017-08-22 09:40 +0200
Message-ID<uhbX3-7Cz-3@gated-at.bofh.it>
In reply to#1717064
On Tue, 22 Aug 2017, rui_feng@realsil.com.cn wrote:

> From: Rui Feng <rui_feng@realsil.com.cn>
> 
> Signed-off-by: Rui Feng <rui_feng@realsil.com.cn>
> ---
>  drivers/mfd/rts5249.c        | 195 ++++++++++++++++++++++++++++++++++++++++
>  drivers/mfd/rtsx_pcr.c       | 205 +++++++++++++++++++++++++++++++++++++++++--
>  drivers/mfd/rtsx_pcr.h       |  11 +++
>  include/linux/mfd/rtsx_pci.h |  60 +++++++++++++
>  4 files changed, 466 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mfd/rts5249.c b/drivers/mfd/rts5249.c
> index 40f8bb1..3959ada 100644
> --- a/drivers/mfd/rts5249.c
> +++ b/drivers/mfd/rts5249.c
> @@ -103,8 +103,72 @@ static void rtsx_base_force_power_down(struct rtsx_pcr *pcr, u8 pm_state)
>  	rtsx_pci_write_register(pcr, FPDCTL, 0x03, 0x03);
>  }
>  
> +static int rts5249_init_from_cfg(struct rtsx_pcr *pcr)
> +{
> +	struct rtsx_cr_option *option = &(pcr->option);
> +	u32 lval;
> +
> +	if (CHK_PCI_PID(pcr, PID_524A))
> +		rtsx_pci_read_config_dword(pcr,
> +			PCR_ASPM_SETTING_REG1, &lval);
> +	else
> +		rtsx_pci_read_config_dword(pcr,
> +			PCR_ASPM_SETTING_REG2, &lval);
> +
> +	if (lval & ASPM_L1_1_EN_MASK)
> +		rtsx_set_dev_flag(pcr, ASPM_L1_1_EN);
> +	else
> +		rtsx_clear_dev_flag(pcr, ASPM_L1_1_EN);

'\n' here.

> +	if (lval & ASPM_L1_2_EN_MASK)
> +		rtsx_set_dev_flag(pcr, ASPM_L1_2_EN);
> +	else
> +		rtsx_clear_dev_flag(pcr, ASPM_L1_2_EN);

Why don't you ensure all flags are already clear?  That way you can
drop all of the clear flag calls and only set them when you know they
are available.

> +	if (lval & PM_L1_1_EN_MASK)
> +		rtsx_set_dev_flag(pcr, PM_L1_1_EN);
> +	else
> +		rtsx_clear_dev_flag(pcr, PM_L1_1_EN);

'\n' here.

> +	if (lval & PM_L1_2_EN_MASK)
> +		rtsx_set_dev_flag(pcr, PM_L1_2_EN);
> +	else
> +		rtsx_clear_dev_flag(pcr, PM_L1_2_EN);
> +
> +	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;
> +		}

No need for single line statements to be in {}'s.

> +	}
> +
> +	return 0;

Why does this function return int?

You don't even check the return value.

> +}
> +
> +static int rts5249_init_from_hw(struct rtsx_pcr *pcr)
> +{
> +	struct rtsx_cr_option *option = &(pcr->option);
> +
> +	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;
> +
> +	return 0;
> +}
> +
>  static int rts5249_extra_init_hw(struct rtsx_pcr *pcr)
>  {
> +	struct rtsx_cr_option *option = &(pcr->option);
> +
> +	rts5249_init_from_cfg(pcr);
> +	rts5249_init_from_hw(pcr);
> +
>  	rtsx_pci_init_cmd(pcr);
>  
>  	/* Rest L1SUB Config */
> @@ -125,6 +189,17 @@ static int rts5249_extra_init_hw(struct rtsx_pcr *pcr)
>  	else
>  		rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, PETXCFG, 0xB0, 0x80);
>  
> +	/*
> +	 * u_force_clkreq_0
> +	 * If true, CLKREQ# PIN will be forced to drive low to request clock
> +	 */

"If u_force_clkreq_0 is enabled, we forcibly request the XXX 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 rtsx_pci_send_cmd(pcr, 100);
>  }
>  
> @@ -353,6 +428,8 @@ static int rtsx_base_switch_output_voltage(struct rtsx_pcr *pcr, u8 voltage)
>  
>  void rts5249_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->ops = &rts5249_pcr_ops;
> @@ -372,6 +449,20 @@ void rts5249_init_params(struct rtsx_pcr *pcr)
>  	pcr->ms_pull_ctl_disable_tbl = rts5249_ms_pull_ctl_disable_tbl;
>  
>  	pcr->reg_pm_ctrl3 = PM_CTRL3;
> +
> +	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_DEFAULT;
> +	option->ltr_idle_latency = LTR_IDLE_LATENCY_DEFAULT;
> +	option->ltr_l1off_latency = LTR_L1OFF_LATENCY_DEFAULT;
> +	option->dev_aspm_mode = DEV_ASPM_DYNAMIC;
> +	option->l1_snooze_delay = 1;

1 what?

> +	option->ltr_l1off_sspwrgate = LTR_L1OFF_SSPWRGATE_5249_DEFAULT;
> +	option->ltr_l1off_snooze_sspwrgate =
> +		LTR_L1OFF_SNOOZE_SSPWRGATE_5249_DEFAULT;
>  }
>  
>  static int rts524a_write_phy(struct rtsx_pcr *pcr, u8 addr, u16 val)
> @@ -459,6 +550,54 @@ static int rts524a_extra_init_hw(struct rtsx_pcr *pcr)
>  	return 0;
>  }
>  
> +static void rts5250_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;
> +
> +	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) {
> +			u8 val = option->ltr_l1off_snooze_sspwrgate;
> +
> +			if (rtsx_check_dev_flag(pcr,
> +					LTR_L1SS_PWR_GATE_CHECK_CARD_EN)) {
> +				if (card_exist)
> +					val &= ~(1 << 7);
> +				else
> +					val |= (1 << 7);

These need defining.

> +			}
> +
> +			rtsx_set_l1off_sub(pcr, val);
> +		} else {
> +			rtsx_set_l1off_sub(pcr, 0);
> +		}
> +	} else {
> +		/* l1off, latency: 300us */
> +		if (aspm_L1_2) {
> +			u8 val = option->ltr_l1off_sspwrgate;
> +
> +			if (rtsx_check_dev_flag(pcr,
> +					LTR_L1SS_PWR_GATE_CHECK_CARD_EN)) {
> +				if (card_exist)
> +					val &= ~(1 << 7);
> +				else
> +					val |= (1 << 7);
> +			}
> +
> +			rtsx_set_l1off_sub(pcr, val);
> +		} else {
> +			rtsx_set_l1off_sub(pcr, 0);
> +		}
> +	}
> +}
> +
>  static const struct pcr_ops rts524a_pcr_ops = {
>  	.write_phy = rts524a_write_phy,
>  	.read_phy = rts524a_read_phy,
> @@ -473,11 +612,15 @@ static int rts524a_extra_init_hw(struct rtsx_pcr *pcr)
>  	.card_power_off = rtsx_base_card_power_off,
>  	.switch_output_voltage = rtsx_base_switch_output_voltage,
>  	.force_power_down = rtsx_base_force_power_down,
> +	.set_l1off_cfg_sub_d0 = rts5250_set_l1off_cfg_sub_d0,
>  };
>  
>  void rts524a_init_params(struct rtsx_pcr *pcr)
>  {
>  	rts5249_init_params(pcr);
> +	pcr->option.ltr_l1off_sspwrgate = LTR_L1OFF_SSPWRGATE_5250_DEFAULT;
> +	pcr->option.ltr_l1off_snooze_sspwrgate =
> +		LTR_L1OFF_SNOOZE_SSPWRGATE_5250_DEFAULT;
>  
>  	pcr->reg_pm_ctrl3 = RTS524A_PM_CTRL3;
>  	pcr->ops = &rts524a_pcr_ops;
> @@ -564,6 +707,52 @@ static int rts525a_extra_init_hw(struct rtsx_pcr *pcr)
>  	return 0;
>  }
>  
> +static void rts5249_enable_aspm(struct rtsx_pcr *pcr)
> +{
> +	struct rtsx_cr_option *option = &pcr->option;
> +
> +	if (pcr->aspm_enabled)
> +		return;
> +
> +	if (option->dev_aspm_mode == DEV_ASPM_DISABLE) {
> +		pcr->aspm_enabled = true;
> +		return;
> +	}

'\n' here.

> +	if (option->dev_aspm_mode == DEV_ASPM_DYNAMIC) {
> +		rtsx_pci_update_cfg_byte(pcr,
> +			pcr->pcie_cap + PCI_EXP_LNKCTL,
> +			0xFC, pcr->aspm_en);
> +	} else if (option->dev_aspm_mode == DEV_ASPM_BACKDOOR) {
> +		u8 mask = FORCE_ASPM_VAL_MASK | FORCE_ASPM_CTL0;
> +
> +		rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, mask, 0);
> +	}
> +	pcr->aspm_enabled = true;

Place this above the:

  if (option->dev_aspm_mode == DEV_ASPM_DISABLE)

.. line.  Then you can remove the duplicate.

> +}
> +
> +static void rts5249_disable_aspm(struct rtsx_pcr *pcr)
> +{
> +	struct rtsx_cr_option *option = &pcr->option;
> +
> +	if (!pcr->aspm_enabled)
> +		return;

'\n' here.

> +	if (option->dev_aspm_mode == DEV_ASPM_DISABLE) {
> +		pcr->aspm_enabled = false;
> +		return;
> +	}
> +	if (option->dev_aspm_mode == DEV_ASPM_DYNAMIC) {
> +		rtsx_pci_update_cfg_byte(pcr,
> +			pcr->pcie_cap + PCI_EXP_LNKCTL,
> +			0xFC, 0);
> +	} else if (option->dev_aspm_mode == DEV_ASPM_BACKDOOR) {
> +		u8 mask = FORCE_ASPM_VAL_MASK | FORCE_ASPM_CTL0;
> +
> +		rtsx_pci_write_register(pcr, ASPM_FORCE_CTL,
> +					mask, FORCE_ASPM_CTL0);
> +	}
> +	pcr->aspm_enabled = false;

Place this above the:

  if (option->dev_aspm_mode == DEV_ASPM_DISABLE)

.. line.  Then you can remove the duplicate.

OR, better still.  Since most of this code is the same as the function
above, please amalgamate them and figure a way to get them to share
code.  Whether that's by using a function argument or something else.

Since there are only 2 pieces of information that differ, I would
create a child function which takes 3 arguments (pcr and the
differences between the functions), which both enable_* and disable_*
call.

> +}
> +
>  static const struct pcr_ops rts525a_pcr_ops = {
>  	.fetch_vendor_settings = rtsx_base_fetch_vendor_settings,
>  	.extra_init_hw = rts525a_extra_init_hw,
> @@ -576,11 +765,17 @@ static int rts525a_extra_init_hw(struct rtsx_pcr *pcr)
>  	.card_power_off = rtsx_base_card_power_off,
>  	.switch_output_voltage = rts525a_switch_output_voltage,
>  	.force_power_down = rtsx_base_force_power_down,
> +	.set_l1off_cfg_sub_d0 = rts5250_set_l1off_cfg_sub_d0,
> +	.enable_aspm = rts5249_enable_aspm,
> +	.disable_aspm = rts5249_disable_aspm,
>  };
>  
>  void rts525a_init_params(struct rtsx_pcr *pcr)
>  {
>  	rts5249_init_params(pcr);
> +	pcr->option.ltr_l1off_sspwrgate = LTR_L1OFF_SSPWRGATE_5250_DEFAULT;
> +	pcr->option.ltr_l1off_snooze_sspwrgate =
> +		LTR_L1OFF_SNOOZE_SSPWRGATE_5250_DEFAULT;
>  
>  	pcr->reg_pm_ctrl3 = RTS524A_PM_CTRL3;
>  	pcr->ops = &rts525a_pcr_ops;
> diff --git a/drivers/mfd/rtsx_pcr.c b/drivers/mfd/rtsx_pcr.c
> index a0ac89d..2ff398d 100644
> --- a/drivers/mfd/rtsx_pcr.c
> +++ b/drivers/mfd/rtsx_pcr.c
> @@ -79,6 +79,142 @@ static inline void rtsx_pci_disable_aspm(struct rtsx_pcr *pcr)
>  		0xFC, 0);
>  }
>  
> +int rtsx_comm_set_ltr_latency(struct rtsx_pcr *pcr, u32 latency)
> +{
> +	rtsx_pci_write_register(pcr, MSGTXDATA0, 0xFF, (u8) (latency & 0xFF));
> +	rtsx_pci_write_register(pcr, MSGTXDATA1,
> +				0xFF, (u8)((latency >> 8) & 0xFF));

Define these.

> +	rtsx_pci_write_register(pcr, MSGTXDATA2,
> +				0xFF, (u8)((latency >> 16) & 0xFF));
> +	rtsx_pci_write_register(pcr, MSGTXDATA3,
> +				0xFF, (u8)((latency >> 24) & 0xFF));
> +	rtsx_pci_write_register(pcr, LTR_CTL, 0xC0, 0xC0);
> +
> +	return 0;
> +}
> +
> +int rtsx_set_ltr_latency(struct rtsx_pcr *pcr, u32 latency)
> +{
> +	if (pcr->ops->set_ltr_latency)
> +		return pcr->ops->set_ltr_latency(pcr, latency);
> +	else
> +		return rtsx_comm_set_ltr_latency(pcr, latency);
> +
> +	return 0;

This code is unreachable.

> +}
> +
> +static void rtsx_comm_disable_aspm(struct rtsx_pcr *pcr)
> +{
> +	struct rtsx_cr_option *option = &pcr->option;
> +
> +	if (!pcr->aspm_enabled)
> +		return;

'\n' here.

> +	if (option->dev_aspm_mode == DEV_ASPM_DISABLE) {
> +		pcr->aspm_enabled = false;
> +		return;
> +	}

'\n' here.

> +	if (option->dev_aspm_mode == DEV_ASPM_DYNAMIC) {
> +		rtsx_pci_disable_aspm(pcr);
> +	} else if (option->dev_aspm_mode == DEV_ASPM_BACKDOOR) {
> +		u8 mask = FORCE_ASPM_VAL_MASK;
> +
> +		rtsx_pci_write_register(pcr, ASPM_FORCE_CTL,  mask, 0);
> +	}

'\n' here.

> +	msleep(20);
> +	pcr->aspm_enabled = false;
> +}
> +
> +static void rtsx_disable_aspm(struct rtsx_pcr *pcr)
> +{
> +	if (pcr->ops->disable_aspm)
> +		pcr->ops->disable_aspm(pcr);
> +	else
> +		rtsx_comm_disable_aspm(pcr);
> +}
> +
> +int rtsx_set_l1off_sub(struct rtsx_pcr *pcr, u8 val)
> +{
> +	rtsx_pci_write_register(pcr, L1SUB_CONFIG3, 0xFF, val);
> +
> +	return 0;
> +}
> +
> +static void rtsx_comm_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;
> +
> +	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) {
> +			u8 val = option->ltr_l1off_snooze_sspwrgate;
> +
> +			if (rtsx_check_dev_flag(pcr,
> +					LTR_L1SS_PWR_GATE_CHECK_CARD_EN)) {
> +				if (card_exist)
> +					val &= ~(1 << 4);
> +				else
> +					val |= (1 << 4);

Define these.

> +			}
> +
> +			rtsx_set_l1off_sub(pcr, val);
> +		} else {
> +			rtsx_set_l1off_sub(pcr, 0);
> +		}
> +	} else {
> +		/* l1off, latency: 300us */
> +		if (aspm_L1_2) {
> +			u8 val = option->ltr_l1off_sspwrgate;
> +
> +			if (rtsx_check_dev_flag(pcr,
> +					LTR_L1SS_PWR_GATE_CHECK_CARD_EN)) {
> +				if (card_exist)
> +					val &= ~(1 << 4);
> +				else
> +					val |= (1 << 4);
> +			}
> +
> +			rtsx_set_l1off_sub(pcr, val);
> +		} else {
> +			rtsx_set_l1off_sub(pcr, 0);
> +		}
> +	}
> +}

Again, there is a lot of duplication here.  Any way these can be
amalgamated?

> +void rtsx_set_l1off_sub_cfg_d0(struct rtsx_pcr *pcr, int active)
> +{
> +	if (pcr->ops->set_l1off_cfg_sub_d0)
> +		pcr->ops->set_l1off_cfg_sub_d0(pcr, active);
> +	else
> +		rtsx_comm_set_l1off_cfg_sub_d0(pcr, active);
> +}
> +
> +static void rtsx_comm_pm_full_on(struct rtsx_pcr *pcr)
> +{
> +	struct rtsx_cr_option *option = &pcr->option;
> +
> +	rtsx_disable_aspm(pcr);
> +
> +	if (option->ltr_enabled)
> +		rtsx_set_ltr_latency(pcr, option->ltr_active_latency);
> +
> +	if (rtsx_check_dev_flag(pcr, LTR_L1SS_PWR_GATE_EN))
> +		rtsx_set_l1off_sub_cfg_d0(pcr, 1);
> +}
> +
> +void rtsx_pm_full_on(struct rtsx_pcr *pcr)
> +{
> +	if (pcr->ops->full_on)
> +		pcr->ops->full_on(pcr);
> +	else
> +		rtsx_comm_pm_full_on(pcr);
> +}
> +
>  void rtsx_pci_start_run(struct rtsx_pcr *pcr)
>  {
>  	/* If pci device removed, don't queue idle work any more */
> @@ -89,9 +225,7 @@ void rtsx_pci_start_run(struct rtsx_pcr *pcr)
>  		pcr->state = PDEV_STAT_RUN;
>  		if (pcr->ops->enable_auto_blink)
>  			pcr->ops->enable_auto_blink(pcr);
> -
> -		if (pcr->aspm_en)
> -			rtsx_pci_disable_aspm(pcr);
> +		rtsx_pm_full_on(pcr);
>  	}
>  
>  	mod_delayed_work(system_wq, &pcr->idle_work, msecs_to_jiffies(200));
> @@ -958,6 +1092,63 @@ static int rtsx_pci_acquire_irq(struct rtsx_pcr *pcr)
>  	return 0;
>  }
>  
> +static void rtsx_comm_enable_aspm(struct rtsx_pcr *pcr)
> +{
> +	struct rtsx_cr_option *option = &pcr->option;
> +
> +	if (pcr->aspm_enabled)
> +		return;

'\n' here.

> +	if (option->dev_aspm_mode == DEV_ASPM_DISABLE) {
> +		pcr->aspm_enabled = true;
> +		return;
> +	}
> +
> +	if (option->dev_aspm_mode == DEV_ASPM_DYNAMIC) {
> +		rtsx_pci_enable_aspm(pcr);
> +	} else if (option->dev_aspm_mode == DEV_ASPM_BACKDOOR) {
> +		u8 mask = FORCE_ASPM_VAL_MASK;
> +		u8 val = pcr->aspm_en;
> +
> +		rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, mask, val);
> +	}
> +	pcr->aspm_enabled = true;

Same comments as the code above that looks the same.

> +}
> +
> +static void rtsx_enable_aspm(struct rtsx_pcr *pcr)
> +{
> +	if (pcr->ops->enable_aspm)
> +		pcr->ops->enable_aspm(pcr);
> +	else
> +		rtsx_comm_enable_aspm(pcr);
> +}
> +
> +static void rtsx_comm_pm_power_saving(struct rtsx_pcr *pcr)
> +{
> +	struct rtsx_cr_option *option = &pcr->option;
> +
> +	if (option->ltr_enabled) {
> +		u32 latency = option->ltr_l1off_latency;
> +
> +		if (rtsx_check_dev_flag(pcr, L1_SNOOZE_TEST_EN))
> +			mdelay(option->l1_snooze_delay);
> +
> +		rtsx_set_ltr_latency(pcr, latency);
> +	}
> +
> +	if (rtsx_check_dev_flag(pcr, LTR_L1SS_PWR_GATE_EN))
> +		rtsx_set_l1off_sub_cfg_d0(pcr, 0);
> +
> +	rtsx_enable_aspm(pcr);
> +}
> +
> +void rtsx_pm_power_saving(struct rtsx_pcr *pcr)
> +{
> +	if (pcr->ops->power_saving)
> +		pcr->ops->power_saving(pcr);
> +	else
> +		rtsx_comm_pm_power_saving(pcr);
> +}
> +
>  static void rtsx_pci_idle_work(struct work_struct *work)
>  {
>  	struct delayed_work *dwork = to_delayed_work(work);
> @@ -974,8 +1165,7 @@ static void rtsx_pci_idle_work(struct work_struct *work)
>  	if (pcr->ops->turn_off_led)
>  		pcr->ops->turn_off_led(pcr);
>  
> -	if (pcr->aspm_en)
> -		rtsx_pci_enable_aspm(pcr);
> +	rtsx_pm_power_saving(pcr);
>  
>  	mutex_unlock(&pcr->pcr_mutex);
>  }
> @@ -1063,6 +1253,11 @@ static int rtsx_pci_init_hw(struct rtsx_pcr *pcr)
>  	if (err < 0)
>  		return err;
>  
> +	if (CHK_PCI_PID(pcr, 0x5250) ||
> +		CHK_PCI_PID(pcr, 0x524A) ||
> +		CHK_PCI_PID(pcr, 0x525A))
> +		rtsx_pci_write_register(pcr, PM_CLK_FORCE_CTL, 1, 1);

This should probably be a switch() statement with fall-through.

>  	/* Enable clk_request_n to enable clock power management */
>  	rtsx_pci_write_config_byte(pcr, pcr->pcie_cap + PCI_EXP_LNKCTL + 1, 1);
>  	/* Enter L1 when host tx idle */
> diff --git a/drivers/mfd/rtsx_pcr.h b/drivers/mfd/rtsx_pcr.h
> index 931d1ae..d3baaec 100644
> --- a/drivers/mfd/rtsx_pcr.h
> +++ b/drivers/mfd/rtsx_pcr.h
> @@ -32,6 +32,15 @@
>  #define RTS524A_PME_FORCE_CTL		0xFF78
>  #define RTS524A_PM_CTRL3		0xFF7E
>  
> +#define LTR_ACTIVE_LATENCY_DEFAULT	0x883C
> +#define LTR_IDLE_LATENCY_DEFAULT		0x892C
> +#define LTR_L1OFF_LATENCY_DEFAULT		0x9003
> +#define LTR_L1OFF_SSPWRGATE_5249_DEFAULT	0xAF
> +#define LTR_L1OFF_SSPWRGATE_5250_DEFAULT	0xFF
> +#define LTR_L1OFF_SNOOZE_SSPWRGATE_5249_DEFAULT	0xAC
> +#define LTR_L1OFF_SNOOZE_SSPWRGATE_5250_DEFAULT	0xF8
> +
> +
>  int __rtsx_pci_write_phy_register(struct rtsx_pcr *pcr, u8 addr, u16 val);
>  int __rtsx_pci_read_phy_register(struct rtsx_pcr *pcr, u8 addr, u16 *val);
>  
> @@ -85,5 +94,7 @@ static inline u8 map_sd_drive(int idx)
>  
>  /* generic operations */
>  int rtsx_gops_pm_reset(struct rtsx_pcr *pcr);
> +int rtsx_set_ltr_latency(struct rtsx_pcr *pcr, u32 latency);
> +int rtsx_set_l1off_sub(struct rtsx_pcr *pcr, u8 val);
>  
>  #endif
> diff --git a/include/linux/mfd/rtsx_pci.h b/include/linux/mfd/rtsx_pci.h
> index 116816f..62b50e4 100644
> --- a/include/linux/mfd/rtsx_pci.h
> +++ b/include/linux/mfd/rtsx_pci.h
> @@ -621,6 +621,9 @@
>  
>  #define AUTOLOAD_CFG_BASE		0xFF00
>  #define PETXCFG				0xFF03
> +#define FORCE_CLKREQ_DELINK_MASK	BIT(7)
> +#define FORCE_CLKREQ_LOW	0x80
> +#define FORCE_CLKREQ_HIGH	0x00
>  
>  #define PM_CTRL1			0xFF44
>  #define   CD_RESUME_EN_MASK		0xF0
> @@ -844,6 +847,9 @@
>  #define   PHY_DIG1E_RX_EN_KEEP		0x0001
>  #define PHY_DUM_REG			0x1F
>  
> +#define PCR_ASPM_SETTING_REG1		0x160
> +#define PCR_ASPM_SETTING_REG2		0x168
> +
>  #define PCR_SETTING_REG1		0x724
>  #define PCR_SETTING_REG2		0x814
>  #define PCR_SETTING_REG3		0x747
> @@ -876,14 +882,65 @@ 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 (*enable_aspm)(struct rtsx_pcr *pcr);
> +	void (*disable_aspm)(struct rtsx_pcr *pcr);
> +	int (*set_ltr_latency)(struct rtsx_pcr *pcr, u32 latency);
> +	int (*set_l1off_sub)(struct rtsx_pcr *pcr, u8 val);
> +	void (*set_l1off_cfg_sub_d0)(struct rtsx_pcr *pcr, int active);
> +	void (*full_on)(struct rtsx_pcr *pcr);
> +	void (*power_saving)(struct rtsx_pcr *pcr);
>  };
>  
>  enum PDEV_STAT  {PDEV_STAT_IDLE, PDEV_STAT_RUN};
>  
> +#define ASPM_L1_1_EN_MASK		BIT(3)
> +#define ASPM_L1_2_EN_MASK		BIT(2)
> +#define PM_L1_1_EN_MASK		BIT(1)
> +#define PM_L1_2_EN_MASK		BIT(0)
> +
> +#define ASPM_L1_1_EN			BIT(0)
> +#define ASPM_L1_2_EN			BIT(1)
> +#define PM_L1_1_EN				BIT(2)
> +#define PM_L1_2_EN				BIT(3)
> +#define LTR_L1SS_PWR_GATE_EN	BIT(4)
> +#define L1_SNOOZE_TEST_EN		BIT(5)
> +#define LTR_L1SS_PWR_GATE_CHECK_CARD_EN	BIT(6)
> +
> +enum dev_aspm_mode {
> +	DEV_ASPM_DISABLE = 0,
> +	DEV_ASPM_DYNAMIC,
> +	DEV_ASPM_BACKDOOR,
> +	DEV_ASPM_STATIC,
> +};
> +
> +struct rtsx_cr_option {
> +	u32 dev_flags;
> +	bool force_clkreq_0;
> +	bool ltr_en;
> +	bool ltr_enabled;
> +	bool ltr_active;
> +	u32 ltr_active_latency;
> +	u32 ltr_idle_latency;
> +	u32 ltr_l1off_latency;
> +	enum dev_aspm_mode dev_aspm_mode;
> +	u32 l1_snooze_delay;
> +	u8 ltr_l1off_sspwrgate;
> +	u8 ltr_l1off_snooze_sspwrgate;
> +};

This still needs a kerneldoc header.

> +#define rtsx_set_dev_flag(cr, flag) \
> +	((cr)->option.dev_flags |= (flag))
> +#define rtsx_clear_dev_flag(cr, flag) \
> +	((cr)->option.dev_flags &= ~(flag))
> +#define rtsx_check_dev_flag(cr, flag) \
> +	((cr)->option.dev_flags & (flag))
> +
>  struct rtsx_pcr {
>  	struct pci_dev			*pci;
>  	unsigned int			id;
>  	int				pcie_cap;
> +	struct rtsx_cr_option	option;
>  
>  	/* pci resources */
>  	unsigned long			addr;
> @@ -940,6 +997,7 @@ struct rtsx_pcr {
>  	u8				card_drive_sel;
>  #define ASPM_L1_EN			0x02
>  	u8				aspm_en;
> +	bool				aspm_enabled;
>  
>  #define PCR_MS_PMOS			(1 << 0)
>  #define PCR_REVERSE_SOCKET		(1 << 1)
> @@ -964,6 +1022,8 @@ struct rtsx_pcr {
>  	u8				dma_error_count;
>  };
>  
> +#define PID_524A	0x524A
> +
>  #define CHK_PCI_PID(pcr, pid)		((pcr)->pci->device == (pid))
>  #define PCI_VID(pcr)			((pcr)->pci->vendor)
>  #define PCI_PID(pcr)			((pcr)->pci->device)

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

[toc] | [prev] | [next] | [standalone]

#1717291 — Re: 答复: [PATCH v3] mfd: Add support for RTS5250S power saving

FromLee Jones <lee.jones@linaro.org>
Date2017-08-22 12:40 +0200
SubjectRe: 答复: [PATCH v3] mfd: Add support for RTS5250S power saving
Message-ID<uheLf-14J-11@gated-at.bofh.it>
In reply to#1717093
You have dropped the Mailing List off.

Please don't do that.

On Tue, 22 Aug 2017, 冯锐 wrote:

> Dear Jones:
> 
> > +struct rtsx_cr_option {
> > +	u32 dev_flags;
> > +	bool force_clkreq_0;
> > +	bool ltr_en;
> > +	bool ltr_enabled;
> > +	bool ltr_active;
> > +	u32 ltr_active_latency;
> > +	u32 ltr_idle_latency;
> > +	u32 ltr_l1off_latency;
> > +	enum dev_aspm_mode dev_aspm_mode;
> > +	u32 l1_snooze_delay;
> > +	u8 ltr_l1off_sspwrgate;
> > +	u8 ltr_l1off_snooze_sspwrgate;
> > +};
> 
> This still needs a kerneldoc header.

You need to sort out your email client.

This line should have been quoted.

> Do you mean adding comment for each member of the struct?

Kerneldoc format looks like this:

/**
 * struct ccp_aes_engine - CCP AES operation
 * @type: AES operation key size
 * @mode: AES operation mode
 * @action: AES operation (decrypt/encrypt)
 * @key: key to be used for this AES operation
 * @key_len: length in bytes of key
 * @iv: IV to be used for this AES operation
 * @iv_len: length in bytes of iv
 * @src: data to be used for this operation
 * @dst: data produced by this operation
 * @src_len: length in bytes of data used for this operation
 * @cmac_final: indicates final operation when running in CMAC mode
 * @cmac_key: K1/K2 key used in final CMAC operation
 * @cmac_key_len: length in bytes of cmac_key
 */
 
> > +		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;
> > +		}
> 
> No need for single line statements to be in {}'s.
> 
> But according to the coding-style.rst, the {} is necessary.
> 
> This does not apply if only one branch of a conditional statement is a single
> statement; in the latter case use braces in both branches:
> 
> .. code-block:: c
> 
> 	if (condition) {
> 		do_this();
> 		do_that();
> 	} else {
> 		otherwise();
> 	}

Ah, I thought that was only for Zepher

Never mind then.  Keep that as it is.

> -----邮件原件-----
> 发件人: Lee Jones [mailto:lee.jones@linaro.org] 
> 发送时间: 2017年8月22日 15:33
> 收件人: 冯锐
> 抄送: linux-kernel@vger.kernel.org
> 主题: Re: [PATCH v3] mfd: Add support for RTS5250S power saving

Ah, I see what you've done.

Please just respond in line in future.

Also, these headers should be removed.

> On Tue, 22 Aug 2017, rui_feng@realsil.com.cn wrote:
> 
> > From: Rui Feng <rui_feng@realsil.com.cn>
> > 
> > Signed-off-by: Rui Feng <rui_feng@realsil.com.cn>
> > ---
> >  drivers/mfd/rts5249.c        | 195 ++++++++++++++++++++++++++++++++++++++++
> >  drivers/mfd/rtsx_pcr.c       | 205 +++++++++++++++++++++++++++++++++++++++++--
> >  drivers/mfd/rtsx_pcr.h       |  11 +++
> >  include/linux/mfd/rtsx_pci.h |  60 +++++++++++++
> >  4 files changed, 466 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/mfd/rts5249.c b/drivers/mfd/rts5249.c index 
> > 40f8bb1..3959ada 100644
> > --- a/drivers/mfd/rts5249.c
> > +++ b/drivers/mfd/rts5249.c
> > @@ -103,8 +103,72 @@ static void rtsx_base_force_power_down(struct rtsx_pcr *pcr, u8 pm_state)
> >  	rtsx_pci_write_register(pcr, FPDCTL, 0x03, 0x03);  }
> >  
> > +static int rts5249_init_from_cfg(struct rtsx_pcr *pcr) {
> > +	struct rtsx_cr_option *option = &(pcr->option);
> > +	u32 lval;
> > +
> > +	if (CHK_PCI_PID(pcr, PID_524A))
> > +		rtsx_pci_read_config_dword(pcr,
> > +			PCR_ASPM_SETTING_REG1, &lval);
> > +	else
> > +		rtsx_pci_read_config_dword(pcr,
> > +			PCR_ASPM_SETTING_REG2, &lval);
> > +
> > +	if (lval & ASPM_L1_1_EN_MASK)
> > +		rtsx_set_dev_flag(pcr, ASPM_L1_1_EN);
> > +	else
> > +		rtsx_clear_dev_flag(pcr, ASPM_L1_1_EN);
> 
> '\n' here.
> 
> > +	if (lval & ASPM_L1_2_EN_MASK)
> > +		rtsx_set_dev_flag(pcr, ASPM_L1_2_EN);
> > +	else
> > +		rtsx_clear_dev_flag(pcr, ASPM_L1_2_EN);
> 
> Why don't you ensure all flags are already clear?  That way you can drop all of the clear flag calls and only set them when you know they are available.
> 
> > +	if (lval & PM_L1_1_EN_MASK)
> > +		rtsx_set_dev_flag(pcr, PM_L1_1_EN);
> > +	else
> > +		rtsx_clear_dev_flag(pcr, PM_L1_1_EN);
> 
> '\n' here.
> 
> > +	if (lval & PM_L1_2_EN_MASK)
> > +		rtsx_set_dev_flag(pcr, PM_L1_2_EN);
> > +	else
> > +		rtsx_clear_dev_flag(pcr, PM_L1_2_EN);
> > +
> > +	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;
> > +		}
> 
> No need for single line statements to be in {}'s.
> 
> > +	}
> > +
> > +	return 0;
> 
> Why does this function return int?
> 
> You don't even check the return value.
> 
> > +}
> > +
> > +static int rts5249_init_from_hw(struct rtsx_pcr *pcr) {
> > +	struct rtsx_cr_option *option = &(pcr->option);
> > +
> > +	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;
> > +
> > +	return 0;
> > +}
> > +
> >  static int rts5249_extra_init_hw(struct rtsx_pcr *pcr)  {
> > +	struct rtsx_cr_option *option = &(pcr->option);
> > +
> > +	rts5249_init_from_cfg(pcr);
> > +	rts5249_init_from_hw(pcr);
> > +
> >  	rtsx_pci_init_cmd(pcr);
> >  
> >  	/* Rest L1SUB Config */
> > @@ -125,6 +189,17 @@ static int rts5249_extra_init_hw(struct rtsx_pcr *pcr)
> >  	else
> >  		rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, PETXCFG, 0xB0, 0x80);
> >  
> > +	/*
> > +	 * u_force_clkreq_0
> > +	 * If true, CLKREQ# PIN will be forced to drive low to request clock
> > +	 */
> 
> "If u_force_clkreq_0 is enabled, we forcibly request the XXX 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 rtsx_pci_send_cmd(pcr, 100);
> >  }
> >  
> > @@ -353,6 +428,8 @@ static int rtsx_base_switch_output_voltage(struct 
> > rtsx_pcr *pcr, u8 voltage)
> >  
> >  void rts5249_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->ops = &rts5249_pcr_ops;
> > @@ -372,6 +449,20 @@ void rts5249_init_params(struct rtsx_pcr *pcr)
> >  	pcr->ms_pull_ctl_disable_tbl = rts5249_ms_pull_ctl_disable_tbl;
> >  
> >  	pcr->reg_pm_ctrl3 = PM_CTRL3;
> > +
> > +	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_DEFAULT;
> > +	option->ltr_idle_latency = LTR_IDLE_LATENCY_DEFAULT;
> > +	option->ltr_l1off_latency = LTR_L1OFF_LATENCY_DEFAULT;
> > +	option->dev_aspm_mode = DEV_ASPM_DYNAMIC;
> > +	option->l1_snooze_delay = 1;
> 
> 1 what?
> 
> > +	option->ltr_l1off_sspwrgate = LTR_L1OFF_SSPWRGATE_5249_DEFAULT;
> > +	option->ltr_l1off_snooze_sspwrgate =
> > +		LTR_L1OFF_SNOOZE_SSPWRGATE_5249_DEFAULT;
> >  }
> >  
> >  static int rts524a_write_phy(struct rtsx_pcr *pcr, u8 addr, u16 val) 
> > @@ -459,6 +550,54 @@ static int rts524a_extra_init_hw(struct rtsx_pcr *pcr)
> >  	return 0;
> >  }
> >  
> > +static void rts5250_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;
> > +
> > +	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) {
> > +			u8 val = option->ltr_l1off_snooze_sspwrgate;
> > +
> > +			if (rtsx_check_dev_flag(pcr,
> > +					LTR_L1SS_PWR_GATE_CHECK_CARD_EN)) {
> > +				if (card_exist)
> > +					val &= ~(1 << 7);
> > +				else
> > +					val |= (1 << 7);
> 
> These need defining.
> 
> > +			}
> > +
> > +			rtsx_set_l1off_sub(pcr, val);
> > +		} else {
> > +			rtsx_set_l1off_sub(pcr, 0);
> > +		}
> > +	} else {
> > +		/* l1off, latency: 300us */
> > +		if (aspm_L1_2) {
> > +			u8 val = option->ltr_l1off_sspwrgate;
> > +
> > +			if (rtsx_check_dev_flag(pcr,
> > +					LTR_L1SS_PWR_GATE_CHECK_CARD_EN)) {
> > +				if (card_exist)
> > +					val &= ~(1 << 7);
> > +				else
> > +					val |= (1 << 7);
> > +			}
> > +
> > +			rtsx_set_l1off_sub(pcr, val);
> > +		} else {
> > +			rtsx_set_l1off_sub(pcr, 0);
> > +		}
> > +	}
> > +}
> > +
> >  static const struct pcr_ops rts524a_pcr_ops = {
> >  	.write_phy = rts524a_write_phy,
> >  	.read_phy = rts524a_read_phy,
> > @@ -473,11 +612,15 @@ static int rts524a_extra_init_hw(struct rtsx_pcr *pcr)
> >  	.card_power_off = rtsx_base_card_power_off,
> >  	.switch_output_voltage = rtsx_base_switch_output_voltage,
> >  	.force_power_down = rtsx_base_force_power_down,
> > +	.set_l1off_cfg_sub_d0 = rts5250_set_l1off_cfg_sub_d0,
> >  };
> >  
> >  void rts524a_init_params(struct rtsx_pcr *pcr)  {
> >  	rts5249_init_params(pcr);
> > +	pcr->option.ltr_l1off_sspwrgate = LTR_L1OFF_SSPWRGATE_5250_DEFAULT;
> > +	pcr->option.ltr_l1off_snooze_sspwrgate =
> > +		LTR_L1OFF_SNOOZE_SSPWRGATE_5250_DEFAULT;
> >  
> >  	pcr->reg_pm_ctrl3 = RTS524A_PM_CTRL3;
> >  	pcr->ops = &rts524a_pcr_ops;
> > @@ -564,6 +707,52 @@ static int rts525a_extra_init_hw(struct rtsx_pcr *pcr)
> >  	return 0;
> >  }
> >  
> > +static void rts5249_enable_aspm(struct rtsx_pcr *pcr) {
> > +	struct rtsx_cr_option *option = &pcr->option;
> > +
> > +	if (pcr->aspm_enabled)
> > +		return;
> > +
> > +	if (option->dev_aspm_mode == DEV_ASPM_DISABLE) {
> > +		pcr->aspm_enabled = true;
> > +		return;
> > +	}
> 
> '\n' here.
> 
> > +	if (option->dev_aspm_mode == DEV_ASPM_DYNAMIC) {
> > +		rtsx_pci_update_cfg_byte(pcr,
> > +			pcr->pcie_cap + PCI_EXP_LNKCTL,
> > +			0xFC, pcr->aspm_en);
> > +	} else if (option->dev_aspm_mode == DEV_ASPM_BACKDOOR) {
> > +		u8 mask = FORCE_ASPM_VAL_MASK | FORCE_ASPM_CTL0;
> > +
> > +		rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, mask, 0);
> > +	}
> > +	pcr->aspm_enabled = true;
> 
> Place this above the:
> 
>   if (option->dev_aspm_mode == DEV_ASPM_DISABLE)
> 
> .. line.  Then you can remove the duplicate.
> 
> > +}
> > +
> > +static void rts5249_disable_aspm(struct rtsx_pcr *pcr) {
> > +	struct rtsx_cr_option *option = &pcr->option;
> > +
> > +	if (!pcr->aspm_enabled)
> > +		return;
> 
> '\n' here.
> 
> > +	if (option->dev_aspm_mode == DEV_ASPM_DISABLE) {
> > +		pcr->aspm_enabled = false;
> > +		return;
> > +	}
> > +	if (option->dev_aspm_mode == DEV_ASPM_DYNAMIC) {
> > +		rtsx_pci_update_cfg_byte(pcr,
> > +			pcr->pcie_cap + PCI_EXP_LNKCTL,
> > +			0xFC, 0);
> > +	} else if (option->dev_aspm_mode == DEV_ASPM_BACKDOOR) {
> > +		u8 mask = FORCE_ASPM_VAL_MASK | FORCE_ASPM_CTL0;
> > +
> > +		rtsx_pci_write_register(pcr, ASPM_FORCE_CTL,
> > +					mask, FORCE_ASPM_CTL0);
> > +	}
> > +	pcr->aspm_enabled = false;
> 
> Place this above the:
> 
>   if (option->dev_aspm_mode == DEV_ASPM_DISABLE)
> 
> .. line.  Then you can remove the duplicate.
> 
> OR, better still.  Since most of this code is the same as the function above, please amalgamate them and figure a way to get them to share code.  Whether that's by using a function argument or something else.
> 
> Since there are only 2 pieces of information that differ, I would create a child function which takes 3 arguments (pcr and the differences between the functions), which both enable_* and disable_* call.
> 
> > +}
> > +
> >  static const struct pcr_ops rts525a_pcr_ops = {
> >  	.fetch_vendor_settings = rtsx_base_fetch_vendor_settings,
> >  	.extra_init_hw = rts525a_extra_init_hw, @@ -576,11 +765,17 @@ static 
> > int rts525a_extra_init_hw(struct rtsx_pcr *pcr)
> >  	.card_power_off = rtsx_base_card_power_off,
> >  	.switch_output_voltage = rts525a_switch_output_voltage,
> >  	.force_power_down = rtsx_base_force_power_down,
> > +	.set_l1off_cfg_sub_d0 = rts5250_set_l1off_cfg_sub_d0,
> > +	.enable_aspm = rts5249_enable_aspm,
> > +	.disable_aspm = rts5249_disable_aspm,
> >  };
> >  
> >  void rts525a_init_params(struct rtsx_pcr *pcr)  {
> >  	rts5249_init_params(pcr);
> > +	pcr->option.ltr_l1off_sspwrgate = LTR_L1OFF_SSPWRGATE_5250_DEFAULT;
> > +	pcr->option.ltr_l1off_snooze_sspwrgate =
> > +		LTR_L1OFF_SNOOZE_SSPWRGATE_5250_DEFAULT;
> >  
> >  	pcr->reg_pm_ctrl3 = RTS524A_PM_CTRL3;
> >  	pcr->ops = &rts525a_pcr_ops;
> > diff --git a/drivers/mfd/rtsx_pcr.c b/drivers/mfd/rtsx_pcr.c index 
> > a0ac89d..2ff398d 100644
> > --- a/drivers/mfd/rtsx_pcr.c
> > +++ b/drivers/mfd/rtsx_pcr.c
> > @@ -79,6 +79,142 @@ static inline void rtsx_pci_disable_aspm(struct rtsx_pcr *pcr)
> >  		0xFC, 0);
> >  }
> >  
> > +int rtsx_comm_set_ltr_latency(struct rtsx_pcr *pcr, u32 latency) {
> > +	rtsx_pci_write_register(pcr, MSGTXDATA0, 0xFF, (u8) (latency & 0xFF));
> > +	rtsx_pci_write_register(pcr, MSGTXDATA1,
> > +				0xFF, (u8)((latency >> 8) & 0xFF));
> 
> Define these.
> 
> > +	rtsx_pci_write_register(pcr, MSGTXDATA2,
> > +				0xFF, (u8)((latency >> 16) & 0xFF));
> > +	rtsx_pci_write_register(pcr, MSGTXDATA3,
> > +				0xFF, (u8)((latency >> 24) & 0xFF));
> > +	rtsx_pci_write_register(pcr, LTR_CTL, 0xC0, 0xC0);
> > +
> > +	return 0;
> > +}
> > +
> > +int rtsx_set_ltr_latency(struct rtsx_pcr *pcr, u32 latency) {
> > +	if (pcr->ops->set_ltr_latency)
> > +		return pcr->ops->set_ltr_latency(pcr, latency);
> > +	else
> > +		return rtsx_comm_set_ltr_latency(pcr, latency);
> > +
> > +	return 0;
> 
> This code is unreachable.
> 
> > +}
> > +
> > +static void rtsx_comm_disable_aspm(struct rtsx_pcr *pcr) {
> > +	struct rtsx_cr_option *option = &pcr->option;
> > +
> > +	if (!pcr->aspm_enabled)
> > +		return;
> 
> '\n' here.
> 
> > +	if (option->dev_aspm_mode == DEV_ASPM_DISABLE) {
> > +		pcr->aspm_enabled = false;
> > +		return;
> > +	}
> 
> '\n' here.
> 
> > +	if (option->dev_aspm_mode == DEV_ASPM_DYNAMIC) {
> > +		rtsx_pci_disable_aspm(pcr);
> > +	} else if (option->dev_aspm_mode == DEV_ASPM_BACKDOOR) {
> > +		u8 mask = FORCE_ASPM_VAL_MASK;
> > +
> > +		rtsx_pci_write_register(pcr, ASPM_FORCE_CTL,  mask, 0);
> > +	}
> 
> '\n' here.
> 
> > +	msleep(20);
> > +	pcr->aspm_enabled = false;
> > +}
> > +
> > +static void rtsx_disable_aspm(struct rtsx_pcr *pcr) {
> > +	if (pcr->ops->disable_aspm)
> > +		pcr->ops->disable_aspm(pcr);
> > +	else
> > +		rtsx_comm_disable_aspm(pcr);
> > +}
> > +
> > +int rtsx_set_l1off_sub(struct rtsx_pcr *pcr, u8 val) {
> > +	rtsx_pci_write_register(pcr, L1SUB_CONFIG3, 0xFF, val);
> > +
> > +	return 0;
> > +}
> > +
> > +static void rtsx_comm_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;
> > +
> > +	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) {
> > +			u8 val = option->ltr_l1off_snooze_sspwrgate;
> > +
> > +			if (rtsx_check_dev_flag(pcr,
> > +					LTR_L1SS_PWR_GATE_CHECK_CARD_EN)) {
> > +				if (card_exist)
> > +					val &= ~(1 << 4);
> > +				else
> > +					val |= (1 << 4);
> 
> Define these.
> 
> > +			}
> > +
> > +			rtsx_set_l1off_sub(pcr, val);
> > +		} else {
> > +			rtsx_set_l1off_sub(pcr, 0);
> > +		}
> > +	} else {
> > +		/* l1off, latency: 300us */
> > +		if (aspm_L1_2) {
> > +			u8 val = option->ltr_l1off_sspwrgate;
> > +
> > +			if (rtsx_check_dev_flag(pcr,
> > +					LTR_L1SS_PWR_GATE_CHECK_CARD_EN)) {
> > +				if (card_exist)
> > +					val &= ~(1 << 4);
> > +				else
> > +					val |= (1 << 4);
> > +			}
> > +
> > +			rtsx_set_l1off_sub(pcr, val);
> > +		} else {
> > +			rtsx_set_l1off_sub(pcr, 0);
> > +		}
> > +	}
> > +}
> 
> Again, there is a lot of duplication here.  Any way these can be amalgamated?
> 
> > +void rtsx_set_l1off_sub_cfg_d0(struct rtsx_pcr *pcr, int active) {
> > +	if (pcr->ops->set_l1off_cfg_sub_d0)
> > +		pcr->ops->set_l1off_cfg_sub_d0(pcr, active);
> > +	else
> > +		rtsx_comm_set_l1off_cfg_sub_d0(pcr, active); }
> > +
> > +static void rtsx_comm_pm_full_on(struct rtsx_pcr *pcr) {
> > +	struct rtsx_cr_option *option = &pcr->option;
> > +
> > +	rtsx_disable_aspm(pcr);
> > +
> > +	if (option->ltr_enabled)
> > +		rtsx_set_ltr_latency(pcr, option->ltr_active_latency);
> > +
> > +	if (rtsx_check_dev_flag(pcr, LTR_L1SS_PWR_GATE_EN))
> > +		rtsx_set_l1off_sub_cfg_d0(pcr, 1);
> > +}
> > +
> > +void rtsx_pm_full_on(struct rtsx_pcr *pcr) {
> > +	if (pcr->ops->full_on)
> > +		pcr->ops->full_on(pcr);
> > +	else
> > +		rtsx_comm_pm_full_on(pcr);
> > +}
> > +
> >  void rtsx_pci_start_run(struct rtsx_pcr *pcr)  {
> >  	/* If pci device removed, don't queue idle work any more */ @@ -89,9 
> > +225,7 @@ void rtsx_pci_start_run(struct rtsx_pcr *pcr)
> >  		pcr->state = PDEV_STAT_RUN;
> >  		if (pcr->ops->enable_auto_blink)
> >  			pcr->ops->enable_auto_blink(pcr);
> > -
> > -		if (pcr->aspm_en)
> > -			rtsx_pci_disable_aspm(pcr);
> > +		rtsx_pm_full_on(pcr);
> >  	}
> >  
> >  	mod_delayed_work(system_wq, &pcr->idle_work, msecs_to_jiffies(200)); 
> > @@ -958,6 +1092,63 @@ static int rtsx_pci_acquire_irq(struct rtsx_pcr *pcr)
> >  	return 0;
> >  }
> >  
> > +static void rtsx_comm_enable_aspm(struct rtsx_pcr *pcr) {
> > +	struct rtsx_cr_option *option = &pcr->option;
> > +
> > +	if (pcr->aspm_enabled)
> > +		return;
> 
> '\n' here.
> 
> > +	if (option->dev_aspm_mode == DEV_ASPM_DISABLE) {
> > +		pcr->aspm_enabled = true;
> > +		return;
> > +	}
> > +
> > +	if (option->dev_aspm_mode == DEV_ASPM_DYNAMIC) {
> > +		rtsx_pci_enable_aspm(pcr);
> > +	} else if (option->dev_aspm_mode == DEV_ASPM_BACKDOOR) {
> > +		u8 mask = FORCE_ASPM_VAL_MASK;
> > +		u8 val = pcr->aspm_en;
> > +
> > +		rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, mask, val);
> > +	}
> > +	pcr->aspm_enabled = true;
> 
> Same comments as the code above that looks the same.
> 
> > +}
> > +
> > +static void rtsx_enable_aspm(struct rtsx_pcr *pcr) {
> > +	if (pcr->ops->enable_aspm)
> > +		pcr->ops->enable_aspm(pcr);
> > +	else
> > +		rtsx_comm_enable_aspm(pcr);
> > +}
> > +
> > +static void rtsx_comm_pm_power_saving(struct rtsx_pcr *pcr) {
> > +	struct rtsx_cr_option *option = &pcr->option;
> > +
> > +	if (option->ltr_enabled) {
> > +		u32 latency = option->ltr_l1off_latency;
> > +
> > +		if (rtsx_check_dev_flag(pcr, L1_SNOOZE_TEST_EN))
> > +			mdelay(option->l1_snooze_delay);
> > +
> > +		rtsx_set_ltr_latency(pcr, latency);
> > +	}
> > +
> > +	if (rtsx_check_dev_flag(pcr, LTR_L1SS_PWR_GATE_EN))
> > +		rtsx_set_l1off_sub_cfg_d0(pcr, 0);
> > +
> > +	rtsx_enable_aspm(pcr);
> > +}
> > +
> > +void rtsx_pm_power_saving(struct rtsx_pcr *pcr) {
> > +	if (pcr->ops->power_saving)
> > +		pcr->ops->power_saving(pcr);
> > +	else
> > +		rtsx_comm_pm_power_saving(pcr);
> > +}
> > +
> >  static void rtsx_pci_idle_work(struct work_struct *work)  {
> >  	struct delayed_work *dwork = to_delayed_work(work); @@ -974,8 
> > +1165,7 @@ static void rtsx_pci_idle_work(struct work_struct *work)
> >  	if (pcr->ops->turn_off_led)
> >  		pcr->ops->turn_off_led(pcr);
> >  
> > -	if (pcr->aspm_en)
> > -		rtsx_pci_enable_aspm(pcr);
> > +	rtsx_pm_power_saving(pcr);
> >  
> >  	mutex_unlock(&pcr->pcr_mutex);
> >  }
> > @@ -1063,6 +1253,11 @@ static int rtsx_pci_init_hw(struct rtsx_pcr *pcr)
> >  	if (err < 0)
> >  		return err;
> >  
> > +	if (CHK_PCI_PID(pcr, 0x5250) ||
> > +		CHK_PCI_PID(pcr, 0x524A) ||
> > +		CHK_PCI_PID(pcr, 0x525A))
> > +		rtsx_pci_write_register(pcr, PM_CLK_FORCE_CTL, 1, 1);
> 
> This should probably be a switch() statement with fall-through.
> 
> >  	/* Enable clk_request_n to enable clock power management */
> >  	rtsx_pci_write_config_byte(pcr, pcr->pcie_cap + PCI_EXP_LNKCTL + 1, 1);
> >  	/* Enter L1 when host tx idle */
> > diff --git a/drivers/mfd/rtsx_pcr.h b/drivers/mfd/rtsx_pcr.h index 
> > 931d1ae..d3baaec 100644
> > --- a/drivers/mfd/rtsx_pcr.h
> > +++ b/drivers/mfd/rtsx_pcr.h
> > @@ -32,6 +32,15 @@
> >  #define RTS524A_PME_FORCE_CTL		0xFF78
> >  #define RTS524A_PM_CTRL3		0xFF7E
> >  
> > +#define LTR_ACTIVE_LATENCY_DEFAULT	0x883C
> > +#define LTR_IDLE_LATENCY_DEFAULT		0x892C
> > +#define LTR_L1OFF_LATENCY_DEFAULT		0x9003
> > +#define LTR_L1OFF_SSPWRGATE_5249_DEFAULT	0xAF
> > +#define LTR_L1OFF_SSPWRGATE_5250_DEFAULT	0xFF
> > +#define LTR_L1OFF_SNOOZE_SSPWRGATE_5249_DEFAULT	0xAC
> > +#define LTR_L1OFF_SNOOZE_SSPWRGATE_5250_DEFAULT	0xF8
> > +
> > +
> >  int __rtsx_pci_write_phy_register(struct rtsx_pcr *pcr, u8 addr, u16 
> > val);  int __rtsx_pci_read_phy_register(struct rtsx_pcr *pcr, u8 addr, 
> > u16 *val);
> >  
> > @@ -85,5 +94,7 @@ static inline u8 map_sd_drive(int idx)
> >  
> >  /* generic operations */
> >  int rtsx_gops_pm_reset(struct rtsx_pcr *pcr);
> > +int rtsx_set_ltr_latency(struct rtsx_pcr *pcr, u32 latency); int 
> > +rtsx_set_l1off_sub(struct rtsx_pcr *pcr, u8 val);
> >  
> >  #endif
> > diff --git a/include/linux/mfd/rtsx_pci.h 
> > b/include/linux/mfd/rtsx_pci.h index 116816f..62b50e4 100644
> > --- a/include/linux/mfd/rtsx_pci.h
> > +++ b/include/linux/mfd/rtsx_pci.h
> > @@ -621,6 +621,9 @@
> >  
> >  #define AUTOLOAD_CFG_BASE		0xFF00
> >  #define PETXCFG				0xFF03
> > +#define FORCE_CLKREQ_DELINK_MASK	BIT(7)
> > +#define FORCE_CLKREQ_LOW	0x80
> > +#define FORCE_CLKREQ_HIGH	0x00
> >  
> >  #define PM_CTRL1			0xFF44
> >  #define   CD_RESUME_EN_MASK		0xF0
> > @@ -844,6 +847,9 @@
> >  #define   PHY_DIG1E_RX_EN_KEEP		0x0001
> >  #define PHY_DUM_REG			0x1F
> >  
> > +#define PCR_ASPM_SETTING_REG1		0x160
> > +#define PCR_ASPM_SETTING_REG2		0x168
> > +
> >  #define PCR_SETTING_REG1		0x724
> >  #define PCR_SETTING_REG2		0x814
> >  #define PCR_SETTING_REG3		0x747
> > @@ -876,14 +882,65 @@ 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 (*enable_aspm)(struct rtsx_pcr *pcr);
> > +	void (*disable_aspm)(struct rtsx_pcr *pcr);
> > +	int (*set_ltr_latency)(struct rtsx_pcr *pcr, u32 latency);
> > +	int (*set_l1off_sub)(struct rtsx_pcr *pcr, u8 val);
> > +	void (*set_l1off_cfg_sub_d0)(struct rtsx_pcr *pcr, int active);
> > +	void (*full_on)(struct rtsx_pcr *pcr);
> > +	void (*power_saving)(struct rtsx_pcr *pcr);
> >  };
> >  
> >  enum PDEV_STAT  {PDEV_STAT_IDLE, PDEV_STAT_RUN};
> >  
> > +#define ASPM_L1_1_EN_MASK		BIT(3)
> > +#define ASPM_L1_2_EN_MASK		BIT(2)
> > +#define PM_L1_1_EN_MASK		BIT(1)
> > +#define PM_L1_2_EN_MASK		BIT(0)
> > +
> > +#define ASPM_L1_1_EN			BIT(0)
> > +#define ASPM_L1_2_EN			BIT(1)
> > +#define PM_L1_1_EN				BIT(2)
> > +#define PM_L1_2_EN				BIT(3)
> > +#define LTR_L1SS_PWR_GATE_EN	BIT(4)
> > +#define L1_SNOOZE_TEST_EN		BIT(5)
> > +#define LTR_L1SS_PWR_GATE_CHECK_CARD_EN	BIT(6)
> > +
> > +enum dev_aspm_mode {
> > +	DEV_ASPM_DISABLE = 0,
> > +	DEV_ASPM_DYNAMIC,
> > +	DEV_ASPM_BACKDOOR,
> > +	DEV_ASPM_STATIC,
> > +};
> > +
> > +struct rtsx_cr_option {
> > +	u32 dev_flags;
> > +	bool force_clkreq_0;
> > +	bool ltr_en;
> > +	bool ltr_enabled;
> > +	bool ltr_active;
> > +	u32 ltr_active_latency;
> > +	u32 ltr_idle_latency;
> > +	u32 ltr_l1off_latency;
> > +	enum dev_aspm_mode dev_aspm_mode;
> > +	u32 l1_snooze_delay;
> > +	u8 ltr_l1off_sspwrgate;
> > +	u8 ltr_l1off_snooze_sspwrgate;
> > +};
> 
> This still needs a kerneldoc header.
> 
> > +#define rtsx_set_dev_flag(cr, flag) \
> > +	((cr)->option.dev_flags |= (flag))
> > +#define rtsx_clear_dev_flag(cr, flag) \
> > +	((cr)->option.dev_flags &= ~(flag))
> > +#define rtsx_check_dev_flag(cr, flag) \
> > +	((cr)->option.dev_flags & (flag))
> > +
> >  struct rtsx_pcr {
> >  	struct pci_dev			*pci;
> >  	unsigned int			id;
> >  	int				pcie_cap;
> > +	struct rtsx_cr_option	option;
> >  
> >  	/* pci resources */
> >  	unsigned long			addr;
> > @@ -940,6 +997,7 @@ struct rtsx_pcr {
> >  	u8				card_drive_sel;
> >  #define ASPM_L1_EN			0x02
> >  	u8				aspm_en;
> > +	bool				aspm_enabled;
> >  
> >  #define PCR_MS_PMOS			(1 << 0)
> >  #define PCR_REVERSE_SOCKET		(1 << 1)
> > @@ -964,6 +1022,8 @@ struct rtsx_pcr {
> >  	u8				dma_error_count;
> >  };
> >  
> > +#define PID_524A	0x524A
> > +
> >  #define CHK_PCI_PID(pcr, pid)		((pcr)->pci->device == (pid))
> >  #define PCI_VID(pcr)			((pcr)->pci->vendor)
> >  #define PCI_PID(pcr)			((pcr)->pci->device)
> 

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web