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

Groups > linux.kernel > #1726314

答复: [PATCH v4] mfd: Add support for RTS5250S power saving

Show all headers | View raw

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.

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: [PATCH v4] mfd: Add support for RTS5250S power saving Lee Jones <lee.jones@linaro.org> - 2017-09-04 15:20 +0200
  答复: [PATCH v4] mfd: Add support for RTS5250S power saving 冯锐 <rui_feng@realsil.com.cn> - 2017-09-05 04:30 +0200
    Re: 答复: [PATCH v4] mfd: Add support  for RTS5250S power saving Lee Jones <lee.jones@linaro.org> - 2017-09-05 09:30 +0200
      答复: 答复: [PATCH v4] mfd: Add support for RTS5250S power saving 冯锐 <rui_feng@realsil.com.cn> - 2017-09-05 09:40 +0200
        Re: 答复: 答复: [PATCH v4] mfd:  Add support for RTS5250S power saving Lee Jones <lee.jones@linaro.org> - 2017-09-05 10:20 +0200
          Re: 答复: 答复: [PATCH v4] mfd:  Add support for RTS5250S power saving Lee Jones <lee.jones@linaro.org> - 2017-09-07 10:00 +0200

csiph-web