* [PATCH v4] mfd: Add support for RTS5250S power saving
@ 2017-08-24 8:23 rui_feng
2017-09-04 12:56 ` Lee Jones
2017-09-04 13:19 ` Lee Jones
0 siblings, 2 replies; 8+ messages in thread
From: rui_feng @ 2017-08-24 8:23 UTC (permalink / raw)
To: lee.jones, linux-kernel; +Cc: rui_feng
From: rui_feng <rui_feng@realsil.com.cn>
Signed-off-by: Rui Feng <rui_feng@realsil.com.cn>
---
drivers/mfd/rts5249.c | 154 +++++++++++++++++++++++++++++++++++++
drivers/mfd/rtsx_pcr.c | 177 +++++++++++++++++++++++++++++++++++++++++--
drivers/mfd/rtsx_pcr.h | 12 +++
include/linux/mfd/rtsx_pci.h | 85 +++++++++++++++++++++
4 files changed, 423 insertions(+), 5 deletions(-)
diff --git a/drivers/mfd/rts5249.c b/drivers/mfd/rts5249.c
index 40f8bb1..25d25c7f 100644
--- a/drivers/mfd/rts5249.c
+++ b/drivers/mfd/rts5249.c
@@ -103,8 +103,64 @@ static void rtsx_base_force_power_down(struct rtsx_pcr *pcr, u8 pm_state)
rtsx_pci_write_register(pcr, FPDCTL, 0x03, 0x03);
}
+static void 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);
+
+ 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);
+
+ 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;
+ }
+ }
+}
+
+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 +181,17 @@ static int rts5249_extra_init_hw(struct rtsx_pcr *pcr)
else
rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, PETXCFG, 0xB0, 0x80);
+ /*
+ *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 rtsx_pci_send_cmd(pcr, 100);
}
@@ -285,6 +352,31 @@ static int rtsx_base_switch_output_voltage(struct rtsx_pcr *pcr, u8 voltage)
return rtsx_pci_send_cmd(pcr, 100);
}
+static void rts5249_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,
+ 0xFC, 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 const struct pcr_ops rts5249_pcr_ops = {
.fetch_vendor_settings = rtsx_base_fetch_vendor_settings,
.extra_init_hw = rts5249_extra_init_hw,
@@ -297,6 +389,7 @@ static int rtsx_base_switch_output_voltage(struct rtsx_pcr *pcr, u8 voltage)
.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_aspm = rts5249_set_aspm,
};
/* SD Pull Control Enable:
@@ -353,6 +446,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 +467,21 @@ 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 = 0;
+ 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 = L1_SNOOZE_DELAY_DEFAULT;
+ 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 +569,40 @@ 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;
+ 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 rts524a_pcr_ops = {
.write_phy = rts524a_write_phy,
.read_phy = rts524a_read_phy,
@@ -473,11 +617,16 @@ 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,
+ .set_aspm = rts5249_set_aspm,
};
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;
@@ -576,11 +725,16 @@ 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,
+ .set_aspm = rts5249_set_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..50a6e67 100644
--- a/drivers/mfd/rtsx_pcr.c
+++ b/drivers/mfd/rtsx_pcr.c
@@ -79,6 +79,131 @@ 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, LTR_TX_EN_MASK |
+ LTR_LATENCY_MODE_MASK, LTR_TX_EN_1 | LTR_LATENCY_MODE_SW);
+
+ 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);
+}
+
+static void rtsx_comm_set_aspm(struct rtsx_pcr *pcr, bool enable)
+{
+ struct rtsx_cr_option *option = &pcr->option;
+
+ if (pcr->aspm_enabled == enable)
+ return;
+
+ if (option->dev_aspm_mode == DEV_ASPM_DYNAMIC) {
+ if (enable)
+ rtsx_pci_enable_aspm(pcr);
+ else
+ rtsx_pci_disable_aspm(pcr);
+ } else if (option->dev_aspm_mode == DEV_ASPM_BACKDOOR) {
+ u8 mask = FORCE_ASPM_VAL_MASK;
+ u8 val = 0;
+
+ if (enable)
+ val = pcr->aspm_en;
+ rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, mask, val);
+ }
+
+ pcr->aspm_enabled = enable;
+}
+
+static void rtsx_disable_aspm(struct rtsx_pcr *pcr)
+{
+ if (pcr->ops->set_aspm)
+ pcr->ops->set_aspm(pcr, false);
+ else
+ rtsx_comm_set_aspm(pcr, false);
+}
+
+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;
+ 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_COMM;
+ else
+ val |= L1OFF_MBIAS2_EN_COMM;
+ }
+ }
+ rtsx_set_l1off_sub(pcr, val);
+}
+
+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 +214,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 +1081,41 @@ static int rtsx_pci_acquire_irq(struct rtsx_pcr *pcr)
return 0;
}
+static void rtsx_enable_aspm(struct rtsx_pcr *pcr)
+{
+ if (pcr->ops->set_aspm)
+ pcr->ops->set_aspm(pcr, true);
+ else
+ rtsx_comm_set_aspm(pcr, true);
+}
+
+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 +1132,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 +1220,16 @@ static int rtsx_pci_init_hw(struct rtsx_pcr *pcr)
if (err < 0)
return err;
+ switch (PCI_PID(pcr)) {
+ case PID_5250:
+ case PID_524A:
+ case PID_525A:
+ rtsx_pci_write_register(pcr, PM_CLK_FORCE_CTL, 1, 1);
+ break;
+ default:
+ break;
+ }
+
/* 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..39d4372 100644
--- a/drivers/mfd/rtsx_pcr.h
+++ b/drivers/mfd/rtsx_pcr.h
@@ -32,6 +32,16 @@
#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 L1_SNOOZE_DELAY_DEFAULT 1
+#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 +95,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..4e2a9e7 100644
--- a/include/linux/mfd/rtsx_pci.h
+++ b/include/linux/mfd/rtsx_pci.h
@@ -573,6 +573,12 @@
#define MSGTXDATA3 0xFE47
#define MSGTXCTL 0xFE48
#define LTR_CTL 0xFE4A
+#define LTR_TX_EN_MASK BIT(7)
+#define LTR_TX_EN_1 BIT(7)
+#define LTR_TX_EN_0 0
+#define LTR_LATENCY_MODE_MASK BIT(6)
+#define LTR_LATENCY_MODE_HW 0
+#define LTR_LATENCY_MODE_SW BIT(6)
#define OBFF_CFG 0xFE4C
#define CDRESUMECTL 0xFE52
@@ -616,11 +622,16 @@
#define L1SUB_CONFIG2 0xFE8E
#define L1SUB_AUTO_CFG 0x02
#define L1SUB_CONFIG3 0xFE8F
+#define L1OFF_MBIAS2_EN_COMM BIT(4)
+#define L1OFF_MBIAS2_EN_5250 BIT(7)
#define DUMMY_REG_RESET_0 0xFE90
#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 +855,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 +890,79 @@ 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 (*set_aspm)(struct rtsx_pcr *pcr, bool enable);
+ 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 - card reader option
+ * @dev_flags: device flags
+ * @force_clkreq_0: force clock request
+ * @ltr_en: enable ltr mode flag
+ * @ltr_enabled: ltr mode in configure space flag
+ * @ltr_active: ltr mode status
+ * @ltr_active_latency: ltr mode active latency
+ * @ltr_idle_latency: ltr mode idle latency
+ * @ltr_l1off_latency: ltr mode l1off latency
+ * @dev_aspm_mode: device aspm mode
+ * @l1_snooze_delay: l1 snooze delay
+ * @ltr_l1off_sspwrgate: ltr l1off sspwrgate
+ * @ltr_l1off_snooze_sspwrgate: ltr l1off snooze sspwrgate
+ */
+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 +1019,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 +1044,11 @@ struct rtsx_pcr {
u8 dma_error_count;
};
+#define PID_524A 0x524A
+#define PID_5249 0x5249
+#define PID_5250 0x5250
+#define PID_525A 0x525A
+
#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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v4] mfd: Add support for RTS5250S power saving
2017-08-24 8:23 [PATCH v4] mfd: Add support for RTS5250S power saving rui_feng
@ 2017-09-04 12:56 ` Lee Jones
2017-09-04 13:19 ` Lee Jones
1 sibling, 0 replies; 8+ messages in thread
From: Lee Jones @ 2017-09-04 12:56 UTC (permalink / raw)
To: rui_feng; +Cc: linux-kernel
On Thu, 24 Aug 2017, rui_feng@realsil.com.cn wrote:
> From: rui_feng <rui_feng@realsil.com.cn>
You need to write a commit log here.
Why is this change required? What does it achieve? Why have you done
it this way? What effect does it have? Etc ...
> Signed-off-by: Rui Feng <rui_feng@realsil.com.cn>
> ---
> drivers/mfd/rts5249.c | 154 +++++++++++++++++++++++++++++++++++++
> drivers/mfd/rtsx_pcr.c | 177 +++++++++++++++++++++++++++++++++++++++++--
> drivers/mfd/rtsx_pcr.h | 12 +++
> include/linux/mfd/rtsx_pci.h | 85 +++++++++++++++++++++
> 4 files changed, 423 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mfd/rts5249.c b/drivers/mfd/rts5249.c
> index 40f8bb1..25d25c7f 100644
> --- a/drivers/mfd/rts5249.c
> +++ b/drivers/mfd/rts5249.c
> @@ -103,8 +103,64 @@ static void rtsx_base_force_power_down(struct rtsx_pcr *pcr, u8 pm_state)
> rtsx_pci_write_register(pcr, FPDCTL, 0x03, 0x03);
> }
>
> +static void 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);
> +
> + 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);
> +
> + 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;
> + }
> + }
> +}
> +
> +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 +181,17 @@ static int rts5249_extra_init_hw(struct rtsx_pcr *pcr)
> else
> rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, PETXCFG, 0xB0, 0x80);
>
> + /*
> + *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 rtsx_pci_send_cmd(pcr, 100);
> }
>
> @@ -285,6 +352,31 @@ static int rtsx_base_switch_output_voltage(struct rtsx_pcr *pcr, u8 voltage)
> return rtsx_pci_send_cmd(pcr, 100);
> }
>
> +static void rts5249_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,
> + 0xFC, 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 const struct pcr_ops rts5249_pcr_ops = {
> .fetch_vendor_settings = rtsx_base_fetch_vendor_settings,
> .extra_init_hw = rts5249_extra_init_hw,
> @@ -297,6 +389,7 @@ static int rtsx_base_switch_output_voltage(struct rtsx_pcr *pcr, u8 voltage)
> .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_aspm = rts5249_set_aspm,
> };
>
> /* SD Pull Control Enable:
> @@ -353,6 +446,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 +467,21 @@ 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 = 0;
> + 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 = L1_SNOOZE_DELAY_DEFAULT;
> + 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 +569,40 @@ 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;
> + 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 rts524a_pcr_ops = {
> .write_phy = rts524a_write_phy,
> .read_phy = rts524a_read_phy,
> @@ -473,11 +617,16 @@ 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,
> + .set_aspm = rts5249_set_aspm,
> };
>
> 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;
> @@ -576,11 +725,16 @@ 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,
> + .set_aspm = rts5249_set_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..50a6e67 100644
> --- a/drivers/mfd/rtsx_pcr.c
> +++ b/drivers/mfd/rtsx_pcr.c
> @@ -79,6 +79,131 @@ 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, LTR_TX_EN_MASK |
> + LTR_LATENCY_MODE_MASK, LTR_TX_EN_1 | LTR_LATENCY_MODE_SW);
> +
> + 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);
> +}
> +
> +static void rtsx_comm_set_aspm(struct rtsx_pcr *pcr, bool enable)
> +{
> + struct rtsx_cr_option *option = &pcr->option;
> +
> + if (pcr->aspm_enabled == enable)
> + return;
> +
> + if (option->dev_aspm_mode == DEV_ASPM_DYNAMIC) {
> + if (enable)
> + rtsx_pci_enable_aspm(pcr);
> + else
> + rtsx_pci_disable_aspm(pcr);
> + } else if (option->dev_aspm_mode == DEV_ASPM_BACKDOOR) {
> + u8 mask = FORCE_ASPM_VAL_MASK;
> + u8 val = 0;
> +
> + if (enable)
> + val = pcr->aspm_en;
> + rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, mask, val);
> + }
> +
> + pcr->aspm_enabled = enable;
> +}
> +
> +static void rtsx_disable_aspm(struct rtsx_pcr *pcr)
> +{
> + if (pcr->ops->set_aspm)
> + pcr->ops->set_aspm(pcr, false);
> + else
> + rtsx_comm_set_aspm(pcr, false);
> +}
> +
> +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;
> + 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_COMM;
> + else
> + val |= L1OFF_MBIAS2_EN_COMM;
> + }
> + }
> + rtsx_set_l1off_sub(pcr, val);
> +}
> +
> +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 +214,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 +1081,41 @@ static int rtsx_pci_acquire_irq(struct rtsx_pcr *pcr)
> return 0;
> }
>
> +static void rtsx_enable_aspm(struct rtsx_pcr *pcr)
> +{
> + if (pcr->ops->set_aspm)
> + pcr->ops->set_aspm(pcr, true);
> + else
> + rtsx_comm_set_aspm(pcr, true);
> +}
> +
> +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 +1132,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 +1220,16 @@ static int rtsx_pci_init_hw(struct rtsx_pcr *pcr)
> if (err < 0)
> return err;
>
> + switch (PCI_PID(pcr)) {
> + case PID_5250:
> + case PID_524A:
> + case PID_525A:
> + rtsx_pci_write_register(pcr, PM_CLK_FORCE_CTL, 1, 1);
> + break;
> + default:
> + break;
> + }
> +
> /* 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..39d4372 100644
> --- a/drivers/mfd/rtsx_pcr.h
> +++ b/drivers/mfd/rtsx_pcr.h
> @@ -32,6 +32,16 @@
> #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 L1_SNOOZE_DELAY_DEFAULT 1
> +#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 +95,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..4e2a9e7 100644
> --- a/include/linux/mfd/rtsx_pci.h
> +++ b/include/linux/mfd/rtsx_pci.h
> @@ -573,6 +573,12 @@
> #define MSGTXDATA3 0xFE47
> #define MSGTXCTL 0xFE48
> #define LTR_CTL 0xFE4A
> +#define LTR_TX_EN_MASK BIT(7)
> +#define LTR_TX_EN_1 BIT(7)
> +#define LTR_TX_EN_0 0
> +#define LTR_LATENCY_MODE_MASK BIT(6)
> +#define LTR_LATENCY_MODE_HW 0
> +#define LTR_LATENCY_MODE_SW BIT(6)
> #define OBFF_CFG 0xFE4C
>
> #define CDRESUMECTL 0xFE52
> @@ -616,11 +622,16 @@
> #define L1SUB_CONFIG2 0xFE8E
> #define L1SUB_AUTO_CFG 0x02
> #define L1SUB_CONFIG3 0xFE8F
> +#define L1OFF_MBIAS2_EN_COMM BIT(4)
> +#define L1OFF_MBIAS2_EN_5250 BIT(7)
>
> #define DUMMY_REG_RESET_0 0xFE90
>
> #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 +855,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 +890,79 @@ 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 (*set_aspm)(struct rtsx_pcr *pcr, bool enable);
> + 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 - card reader option
> + * @dev_flags: device flags
> + * @force_clkreq_0: force clock request
> + * @ltr_en: enable ltr mode flag
> + * @ltr_enabled: ltr mode in configure space flag
> + * @ltr_active: ltr mode status
> + * @ltr_active_latency: ltr mode active latency
> + * @ltr_idle_latency: ltr mode idle latency
> + * @ltr_l1off_latency: ltr mode l1off latency
> + * @dev_aspm_mode: device aspm mode
> + * @l1_snooze_delay: l1 snooze delay
> + * @ltr_l1off_sspwrgate: ltr l1off sspwrgate
> + * @ltr_l1off_snooze_sspwrgate: ltr l1off snooze sspwrgate
> + */
> +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 +1019,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 +1044,11 @@ struct rtsx_pcr {
> u8 dma_error_count;
> };
>
> +#define PID_524A 0x524A
> +#define PID_5249 0x5249
> +#define PID_5250 0x5250
> +#define PID_525A 0x525A
> +
> #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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] mfd: Add support for RTS5250S power saving
2017-08-24 8:23 [PATCH v4] mfd: Add support for RTS5250S power saving rui_feng
2017-09-04 12:56 ` Lee Jones
@ 2017-09-04 13:19 ` Lee Jones
2017-09-05 2:26 ` 答复: " 冯锐
1 sibling, 1 reply; 8+ messages in thread
From: Lee Jones @ 2017-09-04 13:19 UTC (permalink / raw)
To: rui_feng; +Cc: linux-kernel
On Thu, 24 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 | 154 +++++++++++++++++++++++++++++++++++++
> drivers/mfd/rtsx_pcr.c | 177 +++++++++++++++++++++++++++++++++++++++++--
> drivers/mfd/rtsx_pcr.h | 12 +++
> include/linux/mfd/rtsx_pci.h | 85 +++++++++++++++++++++
> 4 files changed, 423 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mfd/rts5249.c b/drivers/mfd/rts5249.c
> index 40f8bb1..25d25c7f 100644
> --- a/drivers/mfd/rts5249.c
> +++ b/drivers/mfd/rts5249.c
> @@ -103,8 +103,64 @@ static void rtsx_base_force_power_down(struct rtsx_pcr *pcr, u8 pm_state)
> rtsx_pci_write_register(pcr, FPDCTL, 0x03, 0x03);
> }
>
> +static void 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);
> +
> + 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);
> +
> + 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;
> + }
> + }
> +}
> +
> +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 +181,17 @@ static int rts5249_extra_init_hw(struct rtsx_pcr *pcr)
> else
> rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, PETXCFG, 0xB0, 0x80);
>
> + /*
> + *If u_force_clkreq_0 is enabled, CLKREQ# PIN will be forced
> + *to drive low, and we forcibly request clock.
> + */
You need spaces after the '*'s.
> + 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);
What is 100?
> }
>
> @@ -285,6 +352,31 @@ static int rtsx_base_switch_output_voltage(struct rtsx_pcr *pcr, u8 voltage)
> return rtsx_pci_send_cmd(pcr, 100);
> }
>
> +static void rts5249_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,
> + 0xFC, val);
0xFC needs defining.
> + } 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 const struct pcr_ops rts5249_pcr_ops = {
> .fetch_vendor_settings = rtsx_base_fetch_vendor_settings,
> .extra_init_hw = rts5249_extra_init_hw,
> @@ -297,6 +389,7 @@ static int rtsx_base_switch_output_voltage(struct rtsx_pcr *pcr, u8 voltage)
> .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_aspm = rts5249_set_aspm,
> };
>
> /* SD Pull Control Enable:
> @@ -353,6 +446,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 +467,21 @@ 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 = 0;
Remove this line.
> + option->dev_flags |= (LTR_L1SS_PWR_GATE_CHECK_CARD_EN
> + | LTR_L1SS_PWR_GATE_EN);
Don't 'OR', just '='.
> + option->ltr_en = true;
> +
> + /* init latency of active, idle, L1OFF to 60us, 300us, 3ms */
"Init"
> + 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 = L1_SNOOZE_DELAY_DEFAULT;
> + 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 +569,40 @@ 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;
> + u8 val = 0;
No need to pre-allocate.
> + 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 rts524a_pcr_ops = {
> .write_phy = rts524a_write_phy,
> .read_phy = rts524a_read_phy,
> @@ -473,11 +617,16 @@ 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,
> + .set_aspm = rts5249_set_aspm,
> };
>
> 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;
> @@ -576,11 +725,16 @@ 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,
> + .set_aspm = rts5249_set_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..50a6e67 100644
> --- a/drivers/mfd/rtsx_pcr.c
> +++ b/drivers/mfd/rtsx_pcr.c
> @@ -79,6 +79,131 @@ 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));
What is (the first) 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, LTR_TX_EN_MASK |
> + LTR_LATENCY_MODE_MASK, LTR_TX_EN_1 | LTR_LATENCY_MODE_SW);
> +
> + 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);
> +}
> +
> +static void rtsx_comm_set_aspm(struct rtsx_pcr *pcr, bool enable)
> +{
> + struct rtsx_cr_option *option = &pcr->option;
> +
> + if (pcr->aspm_enabled == enable)
> + return;
> +
> + if (option->dev_aspm_mode == DEV_ASPM_DYNAMIC) {
> + if (enable)
> + rtsx_pci_enable_aspm(pcr);
> + else
> + rtsx_pci_disable_aspm(pcr);
> + } else if (option->dev_aspm_mode == DEV_ASPM_BACKDOOR) {
> + u8 mask = FORCE_ASPM_VAL_MASK;
> + u8 val = 0;
> +
> + if (enable)
> + val = pcr->aspm_en;
> + rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, mask, val);
> + }
> +
> + pcr->aspm_enabled = enable;
> +}
> +
> +static void rtsx_disable_aspm(struct rtsx_pcr *pcr)
> +{
> + if (pcr->ops->set_aspm)
> + pcr->ops->set_aspm(pcr, false);
> + else
> + rtsx_comm_set_aspm(pcr, false);
> +}
> +
> +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;
> + u8 val = 0;
No need to pre-initialise.
> + 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_COMM;
> + else
> + val |= L1OFF_MBIAS2_EN_COMM;
> + }
> + }
> + rtsx_set_l1off_sub(pcr, val);
> +}
This function looks very similar to the one above.
Any way you can integrate the two?
> +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 +214,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 +1081,41 @@ static int rtsx_pci_acquire_irq(struct rtsx_pcr *pcr)
> return 0;
> }
>
> +static void rtsx_enable_aspm(struct rtsx_pcr *pcr)
> +{
> + if (pcr->ops->set_aspm)
> + pcr->ops->set_aspm(pcr, true);
> + else
> + rtsx_comm_set_aspm(pcr, true);
> +}
> +
> +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 +1132,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 +1220,16 @@ static int rtsx_pci_init_hw(struct rtsx_pcr *pcr)
> if (err < 0)
> return err;
>
> + switch (PCI_PID(pcr)) {
> + case PID_5250:
> + case PID_524A:
> + case PID_525A:
> + rtsx_pci_write_register(pcr, PM_CLK_FORCE_CTL, 1, 1);
> + break;
> + default:
> + break;
> + }
> +
> /* 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..39d4372 100644
> --- a/drivers/mfd/rtsx_pcr.h
> +++ b/drivers/mfd/rtsx_pcr.h
> @@ -32,6 +32,16 @@
> #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 L1_SNOOZE_DELAY_DEFAULT 1
> +#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
These should be prefixed/namespaced.
Shorten DEFAULT to DEF to save space.
> +
> +
Why 2 '\n'?
> 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 +95,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..4e2a9e7 100644
> --- a/include/linux/mfd/rtsx_pci.h
> +++ b/include/linux/mfd/rtsx_pci.h
> @@ -573,6 +573,12 @@
> #define MSGTXDATA3 0xFE47
> #define MSGTXCTL 0xFE48
> #define LTR_CTL 0xFE4A
> +#define LTR_TX_EN_MASK BIT(7)
> +#define LTR_TX_EN_1 BIT(7)
> +#define LTR_TX_EN_0 0
> +#define LTR_LATENCY_MODE_MASK BIT(6)
> +#define LTR_LATENCY_MODE_HW 0
> +#define LTR_LATENCY_MODE_SW BIT(6)
> #define OBFF_CFG 0xFE4C
>
> #define CDRESUMECTL 0xFE52
> @@ -616,11 +622,16 @@
> #define L1SUB_CONFIG2 0xFE8E
> #define L1SUB_AUTO_CFG 0x02
> #define L1SUB_CONFIG3 0xFE8F
> +#define L1OFF_MBIAS2_EN_COMM BIT(4)
> +#define L1OFF_MBIAS2_EN_5250 BIT(7)
>
> #define DUMMY_REG_RESET_0 0xFE90
>
> #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 +855,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 +890,79 @@ 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 (*set_aspm)(struct rtsx_pcr *pcr, bool enable);
> + 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 - card reader option
> + * @dev_flags: device flags
> + * @force_clkreq_0: force clock request
> + * @ltr_en: enable ltr mode flag
> + * @ltr_enabled: ltr mode in configure space flag
> + * @ltr_active: ltr mode status
> + * @ltr_active_latency: ltr mode active latency
> + * @ltr_idle_latency: ltr mode idle latency
> + * @ltr_l1off_latency: ltr mode l1off latency
> + * @dev_aspm_mode: device aspm mode
> + * @l1_snooze_delay: l1 snooze delay
> + * @ltr_l1off_sspwrgate: ltr l1off sspwrgate
> + * @ltr_l1off_snooze_sspwrgate: ltr l1off snooze sspwrgate
> + */
> +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 +1019,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 +1044,11 @@ struct rtsx_pcr {
> u8 dma_error_count;
> };
>
> +#define PID_524A 0x524A
> +#define PID_5249 0x5249
> +#define PID_5250 0x5250
> +#define PID_525A 0x525A
> +
> #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
^ permalink raw reply [flat|nested] 8+ messages in thread
* 答复: [PATCH v4] mfd: Add support for RTS5250S power saving
2017-09-04 13:19 ` Lee Jones
@ 2017-09-05 2:26 ` 冯锐
2017-09-05 7:21 ` Lee Jones
0 siblings, 1 reply; 8+ messages in thread
From: 冯锐 @ 2017-09-05 2:26 UTC (permalink / raw)
To: Lee Jones; +Cc: linux-kernel
Dear Jones,
> +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;
> + u8 val = 0;
No need to pre-allocate.
If val is not pre-allocated, what will the val be if it is not assigned before using?
> + 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 rts524a_pcr_ops = {
> .write_phy = rts524a_write_phy,
> .read_phy = rts524a_read_phy,
> @@ -473,11 +617,16 @@ 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,
> + .set_aspm = rts5249_set_aspm,
> };
>
> 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;
> @@ -576,11 +725,16 @@ 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,
> + .set_aspm = rts5249_set_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..50a6e67 100644
> --- a/drivers/mfd/rtsx_pcr.c
> +++ b/drivers/mfd/rtsx_pcr.c
> @@ -79,6 +79,131 @@ 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));
What is (the first) 0xFF?
0xFF is just 8 bit mask.
> + 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, LTR_TX_EN_MASK |
> + LTR_LATENCY_MODE_MASK, LTR_TX_EN_1 | LTR_LATENCY_MODE_SW);
> +
> + 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); }
> +
> +static void rtsx_comm_set_aspm(struct rtsx_pcr *pcr, bool enable) {
> + struct rtsx_cr_option *option = &pcr->option;
> +
> + if (pcr->aspm_enabled == enable)
> + return;
> +
> + if (option->dev_aspm_mode == DEV_ASPM_DYNAMIC) {
> + if (enable)
> + rtsx_pci_enable_aspm(pcr);
> + else
> + rtsx_pci_disable_aspm(pcr);
> + } else if (option->dev_aspm_mode == DEV_ASPM_BACKDOOR) {
> + u8 mask = FORCE_ASPM_VAL_MASK;
> + u8 val = 0;
> +
> + if (enable)
> + val = pcr->aspm_en;
> + rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, mask, val);
> + }
> +
> + pcr->aspm_enabled = enable;
> +}
> +
> +static void rtsx_disable_aspm(struct rtsx_pcr *pcr) {
> + if (pcr->ops->set_aspm)
> + pcr->ops->set_aspm(pcr, false);
> + else
> + rtsx_comm_set_aspm(pcr, false);
> +}
> +
> +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;
> + u8 val = 0;
No need to pre-initialise.
Same above.
> + 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_COMM;
> + else
> + val |= L1OFF_MBIAS2_EN_COMM;
> + }
> + }
> + rtsx_set_l1off_sub(pcr, val);
> +}
This function looks very similar to the one above.
Any way you can integrate the two?
The two functions are callback functions in different files for different chips,
in the future maybe need to support other chips, so I think separating them is low coupling.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: 答复: [PATCH v4] mfd: Add support for RTS5250S power saving
2017-09-05 2:26 ` 答复: " 冯锐
@ 2017-09-05 7:21 ` Lee Jones
2017-09-05 7:31 ` 答复: " 冯锐
0 siblings, 1 reply; 8+ messages in thread
From: Lee Jones @ 2017-09-05 7:21 UTC (permalink / raw)
To: 冯锐; +Cc: linux-kernel
On Tue, 05 Sep 2017, 冯锐 wrote:
> Dear Jones,
>
> > +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;
> > + u8 val = 0;
>
> No need to pre-allocate.
All of my responses should now be quotes, but they are not.
This makes replying to your messages difficult.
Please fix your editor.
> If val is not pre-allocated, what will the val be if it is not assigned before using?
You do not use it before assigning it.
> > + 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 rts524a_pcr_ops = {
> > .write_phy = rts524a_write_phy,
> > .read_phy = rts524a_read_phy,
> > @@ -473,11 +617,16 @@ 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,
> > + .set_aspm = rts5249_set_aspm,
> > };
> >
> > 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;
> > @@ -576,11 +725,16 @@ 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,
> > + .set_aspm = rts5249_set_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..50a6e67 100644
> > --- a/drivers/mfd/rtsx_pcr.c
> > +++ b/drivers/mfd/rtsx_pcr.c
> > @@ -79,6 +79,131 @@ 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));
>
> What is (the first) 0xFF?
>
> 0xFF is just 8 bit mask.
I mean the first one.
> > + 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, LTR_TX_EN_MASK |
> > + LTR_LATENCY_MODE_MASK, LTR_TX_EN_1 | LTR_LATENCY_MODE_SW);
> > +
> > + 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); }
> > +
> > +static void rtsx_comm_set_aspm(struct rtsx_pcr *pcr, bool enable) {
> > + struct rtsx_cr_option *option = &pcr->option;
> > +
> > + if (pcr->aspm_enabled == enable)
> > + return;
> > +
> > + if (option->dev_aspm_mode == DEV_ASPM_DYNAMIC) {
> > + if (enable)
> > + rtsx_pci_enable_aspm(pcr);
> > + else
> > + rtsx_pci_disable_aspm(pcr);
> > + } else if (option->dev_aspm_mode == DEV_ASPM_BACKDOOR) {
> > + u8 mask = FORCE_ASPM_VAL_MASK;
> > + u8 val = 0;
> > +
> > + if (enable)
> > + val = pcr->aspm_en;
> > + rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, mask, val);
> > + }
> > +
> > + pcr->aspm_enabled = enable;
> > +}
> > +
> > +static void rtsx_disable_aspm(struct rtsx_pcr *pcr) {
> > + if (pcr->ops->set_aspm)
> > + pcr->ops->set_aspm(pcr, false);
> > + else
> > + rtsx_comm_set_aspm(pcr, false);
> > +}
> > +
> > +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;
> > + u8 val = 0;
>
> No need to pre-initialise.
>
> Same above.
Same answer.
> > + 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_COMM;
> > + else
> > + val |= L1OFF_MBIAS2_EN_COMM;
> > + }
> > + }
> > + rtsx_set_l1off_sub(pcr, val);
> > +}
>
> This function looks very similar to the one above.
>
> Any way you can integrate the two?
>
> The two functions are callback functions in different files for different chips,
> in the future maybe need to support other chips, so I think separating them is low coupling.
Are they identical?
--
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 v4] mfd: Add support for RTS5250S power saving
2017-09-05 7:21 ` Lee Jones
@ 2017-09-05 7:31 ` 冯锐
2017-09-05 8:18 ` Lee Jones
0 siblings, 1 reply; 8+ messages in thread
From: 冯锐 @ 2017-09-05 7:31 UTC (permalink / raw)
To: Lee Jones; +Cc: linux-kernel
> > +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;
> > + u8 val = 0;
>
> No need to pre-allocate.
All of my responses should now be quotes, but they are not.
This makes replying to your messages difficult.
Please fix your editor.
> If val is not pre-allocated, what will the val be if it is not assigned before using?
You do not use it before assigning it.
If aspm_L1_1 == 0 && aspm_L1_2 == 0, the val will be used before assigned.
> > + 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 rts524a_pcr_ops = {
> > .write_phy = rts524a_write_phy,
> > .read_phy = rts524a_read_phy,
> > @@ -473,11 +617,16 @@ 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,
> > + .set_aspm = rts5249_set_aspm,
> > };
> >
> > 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;
> > @@ -576,11 +725,16 @@ 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,
> > + .set_aspm = rts5249_set_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..50a6e67 100644
> > --- a/drivers/mfd/rtsx_pcr.c
> > +++ b/drivers/mfd/rtsx_pcr.c
> > @@ -79,6 +79,131 @@ 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));
>
> What is (the first) 0xFF?
>
> 0xFF is just 8 bit mask.
I mean the first one.
The first 0xFF is also 8 bit mask.
> > + 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, LTR_TX_EN_MASK |
> > + LTR_LATENCY_MODE_MASK, LTR_TX_EN_1 | LTR_LATENCY_MODE_SW);
> > +
> > + 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); }
> > +
> > +static void rtsx_comm_set_aspm(struct rtsx_pcr *pcr, bool enable) {
> > + struct rtsx_cr_option *option = &pcr->option;
> > +
> > + if (pcr->aspm_enabled == enable)
> > + return;
> > +
> > + if (option->dev_aspm_mode == DEV_ASPM_DYNAMIC) {
> > + if (enable)
> > + rtsx_pci_enable_aspm(pcr);
> > + else
> > + rtsx_pci_disable_aspm(pcr);
> > + } else if (option->dev_aspm_mode == DEV_ASPM_BACKDOOR) {
> > + u8 mask = FORCE_ASPM_VAL_MASK;
> > + u8 val = 0;
> > +
> > + if (enable)
> > + val = pcr->aspm_en;
> > + rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, mask, val);
> > + }
> > +
> > + pcr->aspm_enabled = enable;
> > +}
> > +
> > +static void rtsx_disable_aspm(struct rtsx_pcr *pcr) {
> > + if (pcr->ops->set_aspm)
> > + pcr->ops->set_aspm(pcr, false);
> > + else
> > + rtsx_comm_set_aspm(pcr, false);
> > +}
> > +
> > +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;
> > + u8 val = 0;
>
> No need to pre-initialise.
>
> Same above.
Same answer.
Same above.
> > + 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_COMM;
> > + else
> > + val |= L1OFF_MBIAS2_EN_COMM;
> > + }
> > + }
> > + rtsx_set_l1off_sub(pcr, val);
> > +}
>
> This function looks very similar to the one above.
>
> Any way you can integrate the two?
>
> The two functions are callback functions in different files for
> different chips, in the future maybe need to support other chips, so I think separating them is low coupling.
Are they identical?
No. Different chips maybe different.
--
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 v4] mfd: Add support for RTS5250S power saving
2017-09-05 7:31 ` 答复: " 冯锐
@ 2017-09-05 8:18 ` Lee Jones
2017-09-07 7:55 ` Lee Jones
0 siblings, 1 reply; 8+ messages in thread
From: Lee Jones @ 2017-09-05 8:18 UTC (permalink / raw)
To: 冯锐; +Cc: linux-kernel
On Tue, 05 Sep 2017, 冯锐 wrote:
> > > +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;
> > > + u8 val = 0;
> >
> > No need to pre-allocate.
>
> All of my responses should now be quotes, but they are not.
>
> This makes replying to your messages difficult.
>
> Please fix your editor.
>
> > If val is not pre-allocated, what will the val be if it is not assigned before using?
>
> You do not use it before assigning it.
Your mailer is still broken. Please fix it.
> If aspm_L1_1 == 0 && aspm_L1_2 == 0, the val will be used before assigned.
Ah yes, I see. Leave it as it is then.
> > > + 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 rts524a_pcr_ops = {
> > > .write_phy = rts524a_write_phy,
> > > .read_phy = rts524a_read_phy,
> > > @@ -473,11 +617,16 @@ 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,
> > > + .set_aspm = rts5249_set_aspm,
> > > };
> > >
> > > 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;
> > > @@ -576,11 +725,16 @@ 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,
> > > + .set_aspm = rts5249_set_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..50a6e67 100644
> > > --- a/drivers/mfd/rtsx_pcr.c
> > > +++ b/drivers/mfd/rtsx_pcr.c
> > > @@ -79,6 +79,131 @@ 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));
> >
> > What is (the first) 0xFF?
> >
> > 0xFF is just 8 bit mask.
>
> I mean the first one.
>
> The first 0xFF is also 8 bit mask.
>
> > > + 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, LTR_TX_EN_MASK |
> > > + LTR_LATENCY_MODE_MASK, LTR_TX_EN_1 | LTR_LATENCY_MODE_SW);
> > > +
> > > + 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); }
> > > +
> > > +static void rtsx_comm_set_aspm(struct rtsx_pcr *pcr, bool enable) {
> > > + struct rtsx_cr_option *option = &pcr->option;
> > > +
> > > + if (pcr->aspm_enabled == enable)
> > > + return;
> > > +
> > > + if (option->dev_aspm_mode == DEV_ASPM_DYNAMIC) {
> > > + if (enable)
> > > + rtsx_pci_enable_aspm(pcr);
> > > + else
> > > + rtsx_pci_disable_aspm(pcr);
> > > + } else if (option->dev_aspm_mode == DEV_ASPM_BACKDOOR) {
> > > + u8 mask = FORCE_ASPM_VAL_MASK;
> > > + u8 val = 0;
> > > +
> > > + if (enable)
> > > + val = pcr->aspm_en;
> > > + rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, mask, val);
> > > + }
> > > +
> > > + pcr->aspm_enabled = enable;
> > > +}
> > > +
> > > +static void rtsx_disable_aspm(struct rtsx_pcr *pcr) {
> > > + if (pcr->ops->set_aspm)
> > > + pcr->ops->set_aspm(pcr, false);
> > > + else
> > > + rtsx_comm_set_aspm(pcr, false);
> > > +}
> > > +
> > > +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;
> > > + u8 val = 0;
> >
> > No need to pre-initialise.
> >
> > Same above.
>
> Same answer.
>
> Same above.
>
> > > + 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_COMM;
> > > + else
> > > + val |= L1OFF_MBIAS2_EN_COMM;
> > > + }
> > > + }
> > > + rtsx_set_l1off_sub(pcr, val);
> > > +}
> >
> > This function looks very similar to the one above.
> >
> > Any way you can integrate the two?
> >
> > The two functions are callback functions in different files for
> > different chips, in the future maybe need to support other chips, so I think separating them is low coupling.
>
> Are they identical?
>
> No. Different chips maybe different.
>
--
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 v4] mfd: Add support for RTS5250S power saving
2017-09-05 8:18 ` Lee Jones
@ 2017-09-07 7:55 ` Lee Jones
0 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2017-09-07 7:55 UTC (permalink / raw)
To: 冯锐; +Cc: linux-kernel
On Tue, 05 Sep 2017, Lee Jones wrote:
> On Tue, 05 Sep 2017, 冯锐 wrote:
>
> > > > +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;
> > > > + u8 val = 0;
> > >
> > > No need to pre-allocate.
> >
> > All of my responses should now be quotes, but they are not.
> >
> > This makes replying to your messages difficult.
> >
> > Please fix your editor.
> >
> > > If val is not pre-allocated, what will the val be if it is not assigned before using?
> >
> > You do not use it before assigning it.
>
> Your mailer is still broken. Please fix it.
>
> > If aspm_L1_1 == 0 && aspm_L1_2 == 0, the val will be used before assigned.
>
> Ah yes, I see. Leave it as it is then.
>
> > > > + 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 rts524a_pcr_ops = {
> > > > .write_phy = rts524a_write_phy,
> > > > .read_phy = rts524a_read_phy,
> > > > @@ -473,11 +617,16 @@ 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,
> > > > + .set_aspm = rts5249_set_aspm,
> > > > };
> > > >
> > > > 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;
> > > > @@ -576,11 +725,16 @@ 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,
> > > > + .set_aspm = rts5249_set_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..50a6e67 100644
> > > > --- a/drivers/mfd/rtsx_pcr.c
> > > > +++ b/drivers/mfd/rtsx_pcr.c
> > > > @@ -79,6 +79,131 @@ 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));
> > >
> > > What is (the first) 0xFF?
> > >
> > > 0xFF is just 8 bit mask.
> >
> > I mean the first one.
> >
> > The first 0xFF is also 8 bit mask.
Please define them for clarity.
> > > > + 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, LTR_TX_EN_MASK |
> > > > + LTR_LATENCY_MODE_MASK, LTR_TX_EN_1 | LTR_LATENCY_MODE_SW);
> > > > +
> > > > + 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); }
> > > > +
> > > > +static void rtsx_comm_set_aspm(struct rtsx_pcr *pcr, bool enable) {
> > > > + struct rtsx_cr_option *option = &pcr->option;
> > > > +
> > > > + if (pcr->aspm_enabled == enable)
> > > > + return;
> > > > +
> > > > + if (option->dev_aspm_mode == DEV_ASPM_DYNAMIC) {
> > > > + if (enable)
> > > > + rtsx_pci_enable_aspm(pcr);
> > > > + else
> > > > + rtsx_pci_disable_aspm(pcr);
> > > > + } else if (option->dev_aspm_mode == DEV_ASPM_BACKDOOR) {
> > > > + u8 mask = FORCE_ASPM_VAL_MASK;
> > > > + u8 val = 0;
> > > > +
> > > > + if (enable)
> > > > + val = pcr->aspm_en;
> > > > + rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, mask, val);
> > > > + }
> > > > +
> > > > + pcr->aspm_enabled = enable;
> > > > +}
> > > > +
> > > > +static void rtsx_disable_aspm(struct rtsx_pcr *pcr) {
> > > > + if (pcr->ops->set_aspm)
> > > > + pcr->ops->set_aspm(pcr, false);
> > > > + else
> > > > + rtsx_comm_set_aspm(pcr, false);
> > > > +}
> > > > +
> > > > +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;
> > > > + u8 val = 0;
> > >
> > > No need to pre-initialise.
> > >
> > > Same above.
> >
> > Same answer.
> >
> > Same above.
> >
> > > > + 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_COMM;
> > > > + else
> > > > + val |= L1OFF_MBIAS2_EN_COMM;
> > > > + }
> > > > + }
> > > > + rtsx_set_l1off_sub(pcr, val);
> > > > +}
> > >
> > > This function looks very similar to the one above.
> > >
> > > Any way you can integrate the two?
> > >
> > > The two functions are callback functions in different files for
> > > different chips, in the future maybe need to support other chips, so I think separating them is low coupling.
> >
> > Are they identical?
> >
> > No. Different chips maybe different.
But are the functions *currently* identical?
--
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
end of thread, other threads:[~2017-09-07 7:55 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-24 8:23 [PATCH v4] mfd: Add support for RTS5250S power saving rui_feng
2017-09-04 12:56 ` Lee Jones
2017-09-04 13:19 ` Lee Jones
2017-09-05 2:26 ` 答复: " 冯锐
2017-09-05 7:21 ` Lee Jones
2017-09-05 7:31 ` 答复: " 冯锐
2017-09-05 8:18 ` Lee Jones
2017-09-07 7:55 ` 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).