| Message ID | 20241122075048.2006894-1-quic_yuanjiey@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1] mmc: sdhci-msm: Correctly set the load for the regulator | expand |
On 22/11/24 09:50, Yuanjie Yang wrote: > Qualcomm regulator supports two power supply modes: HPM and LPM. > Currently, the sdhci-msm.c driver does not set the load to adjust > the current for eMMC and SD. Therefore, if the regulator set load > in LPM state, it will lead to the inability to properly initialize > eMMC and SD. > > Set the correct regulator current for eMMC and SD to ensure that the > device can work normally even when the regulator is in LPM. > > Signed-off-by: Yuanjie Yang <quic_yuanjiey@quicinc.com> > --- > drivers/mmc/host/sdhci-msm.c | 91 +++++++++++++++++++++++++++++++++++- > 1 file changed, 89 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c > index e00208535bd1..f2a2260d54c6 100644 > --- a/drivers/mmc/host/sdhci-msm.c > +++ b/drivers/mmc/host/sdhci-msm.c > @@ -134,9 +134,22 @@ > /* Timeout value to avoid infinite waiting for pwr_irq */ > #define MSM_PWR_IRQ_TIMEOUT_MS 5000 > > +/* Max load for eMMC Vdd supply */ > +#define MMC_VMMC_MAX_LOAD_UA 570000 > + > /* Max load for eMMC Vdd-io supply */ > #define MMC_VQMMC_MAX_LOAD_UA 325000 > > +/* Max load for SD Vdd supply */ > +#define SD_VMMC_MAX_LOAD_UA 800000 > + > +/* Max load for SD Vdd-io supply */ > +#define SD_VQMMC_MAX_LOAD_UA 22000 > + > +#define MAX_MMC_SD_VMMC_LOAD_UA max(MMC_VMMC_MAX_LOAD_UA, SD_VMMC_MAX_LOAD_UA) > + > +#define MAX_MMC_SD_VQMMC_LOAD_UA max(MMC_VQMMC_MAX_LOAD_UA, SD_VQMMC_MAX_LOAD_UA) > + > #define msm_host_readl(msm_host, host, offset) \ > msm_host->var_ops->msm_readl_relaxed(host, offset) > > @@ -147,6 +160,11 @@ > #define CQHCI_VENDOR_CFG1 0xA00 > #define CQHCI_VENDOR_DIS_RST_ON_CQ_EN (0x3 << 13) > > +enum { This could be a named type and used instead of 'int' e.g. enum msm_reg_type { > + VMMC_REGULATOR, > + VQMMC_REGULATOR, > +}; > + > struct sdhci_msm_offset { > u32 core_hc_mode; > u32 core_mci_data_cnt; > @@ -1403,11 +1421,70 @@ static int sdhci_msm_set_pincfg(struct sdhci_msm_host *msm_host, bool level) > return ret; > } > > -static int sdhci_msm_set_vmmc(struct mmc_host *mmc) > +static int sdhci_msm_get_regulator_load(struct mmc_host *mmc, int max_current, int type) Then 'int type' could be 'enum msm_reg_type type' > +{ > + int load = 0; > + > + /* > + * When eMMC and SD are powered on for the first time, select a higher > + * current value from the corresponding current for eMMC and SD to > + * ensure that the eMMC and SD cards start up properly and complete > + * initialization. After the initialization process is finished, use > + * the corresponding current to set the eMMC and SD to ensure the > + * normal work of the device. > + */ > + > + if (!mmc->card) > + return max_current; > + > + if (mmc_card_is_removable(mmc) && mmc_card_mmc(mmc->card)) The comment mentions eMMC but here there is 'mmc_card_is_removable()' whereas eMMC's are not removable. If this is right it needs some clarification. > + load = (type == VMMC_REGULATOR) ? MMC_VMMC_MAX_LOAD_UA : MMC_VQMMC_MAX_LOAD_UA; > + else if (mmc_card_sd(mmc->card)) > + load = (type == VMMC_REGULATOR) ? SD_VMMC_MAX_LOAD_UA : SD_VQMMC_MAX_LOAD_UA; > + > + return load; > +} > + > +static int msm_config_regulator_load(struct sdhci_msm_host *msm_host, struct mmc_host *mmc, > + bool hpm, int max_current, int type) Again 'int type' could be 'enum msm_reg_type type' > +{ > + int ret; > + int load = 0; > + > + /* > + * After the initialization process is finished, Once the type of card > + * is determined，only set the corresponding current for SD and eMMC. > + */ > + > + if (mmc->card && !(mmc_card_mmc(mmc->card) || mmc_card_sd(mmc->card))) > + return 0; > + > + if (hpm) > + load = sdhci_msm_get_regulator_load(mmc, max_current, type); > + > + if (type == VMMC_REGULATOR) > + ret = regulator_set_load(mmc->supply.vmmc, load); > + else > + ret = regulator_set_load(mmc->supply.vqmmc, load); > + if (ret) > + dev_err(mmc_dev(mmc), "%s: set load failed: %d\n", > + mmc_hostname(mmc), ret); > + return ret; > +} > + > +static int sdhci_msm_set_vmmc(struct sdhci_msm_host *msm_host, > + struct mmc_host *mmc, bool hpm) > { > + int ret; > + > if (IS_ERR(mmc->supply.vmmc)) > return 0; > > + ret = msm_config_regulator_load(msm_host, mmc, hpm, > + MAX_MMC_SD_VMMC_LOAD_UA, VMMC_REGULATOR); > + if (ret) > + return ret; > + > return mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, mmc->ios.vdd); > } > > @@ -1435,6 +1512,15 @@ static int msm_toggle_vqmmc(struct sdhci_msm_host *msm_host, > goto out; > } > } > + > + ret = msm_config_regulator_load(msm_host, mmc, level, > + MAX_MMC_SD_VQMMC_LOAD_UA, VQMMC_REGULATOR); > + if (ret < 0) { > + dev_err(mmc_dev(mmc), "%s: vqmmc set regulator load failed: %d\n", > + mmc_hostname(mmc), ret); > + goto out; > + } > + > ret = regulator_enable(mmc->supply.vqmmc); > } else { > ret = regulator_disable(mmc->supply.vqmmc); > @@ -1642,7 +1728,8 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq) > } > > if (pwr_state) { > - ret = sdhci_msm_set_vmmc(mmc); > + ret = sdhci_msm_set_vmmc(msm_host, mmc, > + pwr_state & REQ_BUS_ON); > if (!ret) > ret = sdhci_msm_set_vqmmc(msm_host, mmc, > pwr_state & REQ_BUS_ON);
On Tue, Nov 26, 2024 at 12:43:12PM +0200, Adrian Hunter wrote: > On 22/11/24 09:50, Yuanjie Yang wrote: > > Qualcomm regulator supports two power supply modes: HPM and LPM. > > Currently, the sdhci-msm.c driver does not set the load to adjust > > the current for eMMC and SD. Therefore, if the regulator set load > > in LPM state, it will lead to the inability to properly initialize > > eMMC and SD. > > > > Set the correct regulator current for eMMC and SD to ensure that the > > device can work normally even when the regulator is in LPM. > > > > Signed-off-by: Yuanjie Yang <quic_yuanjiey@quicinc.com> > > --- > > drivers/mmc/host/sdhci-msm.c | 91 +++++++++++++++++++++++++++++++++++- > > 1 file changed, 89 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c > > index e00208535bd1..f2a2260d54c6 100644 > > --- a/drivers/mmc/host/sdhci-msm.c > > +++ b/drivers/mmc/host/sdhci-msm.c > > @@ -134,9 +134,22 @@ > > /* Timeout value to avoid infinite waiting for pwr_irq */ > > #define MSM_PWR_IRQ_TIMEOUT_MS 5000 > > > > +/* Max load for eMMC Vdd supply */ > > +#define MMC_VMMC_MAX_LOAD_UA 570000 > > + > > /* Max load for eMMC Vdd-io supply */ > > #define MMC_VQMMC_MAX_LOAD_UA 325000 > > > > +/* Max load for SD Vdd supply */ > > +#define SD_VMMC_MAX_LOAD_UA 800000 > > + > > +/* Max load for SD Vdd-io supply */ > > +#define SD_VQMMC_MAX_LOAD_UA 22000 > > + > > +#define MAX_MMC_SD_VMMC_LOAD_UA max(MMC_VMMC_MAX_LOAD_UA, SD_VMMC_MAX_LOAD_UA) > > + > > +#define MAX_MMC_SD_VQMMC_LOAD_UA max(MMC_VQMMC_MAX_LOAD_UA, SD_VQMMC_MAX_LOAD_UA) > > + > > #define msm_host_readl(msm_host, host, offset) \ > > msm_host->var_ops->msm_readl_relaxed(host, offset) > > > > @@ -147,6 +160,11 @@ > > #define CQHCI_VENDOR_CFG1 0xA00 > > #define CQHCI_VENDOR_DIS_RST_ON_CQ_EN (0x3 << 13) > > > > +enum { > > This could be a named type and used instead of 'int' > e.g. > > enum msm_reg_type { Thanks , I will fix it in next version. > > + VMMC_REGULATOR, > > + VQMMC_REGULATOR, > > +}; > > + > > struct sdhci_msm_offset { > > u32 core_hc_mode; > > u32 core_mci_data_cnt; > > @@ -1403,11 +1421,70 @@ static int sdhci_msm_set_pincfg(struct sdhci_msm_host *msm_host, bool level) > > return ret; > > } > > > > -static int sdhci_msm_set_vmmc(struct mmc_host *mmc) > > +static int sdhci_msm_get_regulator_load(struct mmc_host *mmc, int max_current, int type) > > Then 'int type' could be 'enum msm_reg_type type' Thanks, I will fix it in next version. > > +{ > > + int load = 0; > > + > > + /* > > + * When eMMC and SD are powered on for the first time, select a higher > > + * current value from the corresponding current for eMMC and SD to > > + * ensure that the eMMC and SD cards start up properly and complete > > + * initialization. After the initialization process is finished, use > > + * the corresponding current to set the eMMC and SD to ensure the > > + * normal work of the device. > > + */ > > + > > + if (!mmc->card) > > + return max_current; > > + > > + if (mmc_card_is_removable(mmc) && mmc_card_mmc(mmc->card)) > > The comment mentions eMMC but here there is 'mmc_card_is_removable()' > whereas eMMC's are not removable. If this is right it needs some > clarification. Thanks, I also realize that mmc_card_is_removable(mmc) is an unnecessary judgment. I will delete this judgment in next version. > > + load = (type == VMMC_REGULATOR) ? MMC_VMMC_MAX_LOAD_UA : MMC_VQMMC_MAX_LOAD_UA; > > + else if (mmc_card_sd(mmc->card)) > > + load = (type == VMMC_REGULATOR) ? SD_VMMC_MAX_LOAD_UA : SD_VQMMC_MAX_LOAD_UA; > > + > > + return load; > > +} > > + > > +static int msm_config_regulator_load(struct sdhci_msm_host *msm_host, struct mmc_host *mmc, > > + bool hpm, int max_current, int type) > > Again 'int type' could be 'enum msm_reg_type type' Thanks, I will fix it in next version. > > +{ > > + int ret; > > + int load = 0; > > + > > + /* > > + * After the initialization process is finished, Once the type of card > > + * is determined，only set the corresponding current for SD and eMMC. > > + */ > > + > > + if (mmc->card && !(mmc_card_mmc(mmc->card) || mmc_card_sd(mmc->card))) > > + return 0; > > + > > + if (hpm) > > + load = sdhci_msm_get_regulator_load(mmc, max_current, type); > > + > > + if (type == VMMC_REGULATOR) > > + ret = regulator_set_load(mmc->supply.vmmc, load); > > + else > > + ret = regulator_set_load(mmc->supply.vqmmc, load); > > + if (ret) > > + dev_err(mmc_dev(mmc), "%s: set load failed: %d\n", > > + mmc_hostname(mmc), ret); > > + return ret; > > +} > > + > > +static int sdhci_msm_set_vmmc(struct sdhci_msm_host *msm_host, > > + struct mmc_host *mmc, bool hpm) > > { > > + int ret; > > + > > if (IS_ERR(mmc->supply.vmmc)) > > return 0; > > > > + ret = msm_config_regulator_load(msm_host, mmc, hpm, > > + MAX_MMC_SD_VMMC_LOAD_UA, VMMC_REGULATOR); > > + if (ret) > > + return ret; > > + > > return mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, mmc->ios.vdd); > > } > > > > @@ -1435,6 +1512,15 @@ static int msm_toggle_vqmmc(struct sdhci_msm_host *msm_host, > > goto out; > > } > > } > > + > > + ret = msm_config_regulator_load(msm_host, mmc, level, > > + MAX_MMC_SD_VQMMC_LOAD_UA, VQMMC_REGULATOR); > > + if (ret < 0) { > > + dev_err(mmc_dev(mmc), "%s: vqmmc set regulator load failed: %d\n", > > + mmc_hostname(mmc), ret); > > + goto out; > > + } > > + > > ret = regulator_enable(mmc->supply.vqmmc); > > } else { > > ret = regulator_disable(mmc->supply.vqmmc); > > @@ -1642,7 +1728,8 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq) > > } > > > > if (pwr_state) { > > - ret = sdhci_msm_set_vmmc(mmc); > > + ret = sdhci_msm_set_vmmc(msm_host, mmc, > > + pwr_state & REQ_BUS_ON); > > if (!ret) > > ret = sdhci_msm_set_vqmmc(msm_host, mmc, > > pwr_state & REQ_BUS_ON); >
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c index e00208535bd1..f2a2260d54c6 100644 --- a/drivers/mmc/host/sdhci-msm.c +++ b/drivers/mmc/host/sdhci-msm.c @@ -134,9 +134,22 @@ /* Timeout value to avoid infinite waiting for pwr_irq */ #define MSM_PWR_IRQ_TIMEOUT_MS 5000 +/* Max load for eMMC Vdd supply */ +#define MMC_VMMC_MAX_LOAD_UA 570000 + /* Max load for eMMC Vdd-io supply */ #define MMC_VQMMC_MAX_LOAD_UA 325000 +/* Max load for SD Vdd supply */ +#define SD_VMMC_MAX_LOAD_UA 800000 + +/* Max load for SD Vdd-io supply */ +#define SD_VQMMC_MAX_LOAD_UA 22000 + +#define MAX_MMC_SD_VMMC_LOAD_UA max(MMC_VMMC_MAX_LOAD_UA, SD_VMMC_MAX_LOAD_UA) + +#define MAX_MMC_SD_VQMMC_LOAD_UA max(MMC_VQMMC_MAX_LOAD_UA, SD_VQMMC_MAX_LOAD_UA) + #define msm_host_readl(msm_host, host, offset) \ msm_host->var_ops->msm_readl_relaxed(host, offset) @@ -147,6 +160,11 @@ #define CQHCI_VENDOR_CFG1 0xA00 #define CQHCI_VENDOR_DIS_RST_ON_CQ_EN (0x3 << 13) +enum { + VMMC_REGULATOR, + VQMMC_REGULATOR, +}; + struct sdhci_msm_offset { u32 core_hc_mode; u32 core_mci_data_cnt; @@ -1403,11 +1421,70 @@ static int sdhci_msm_set_pincfg(struct sdhci_msm_host *msm_host, bool level) return ret; } -static int sdhci_msm_set_vmmc(struct mmc_host *mmc) +static int sdhci_msm_get_regulator_load(struct mmc_host *mmc, int max_current, int type) +{ + int load = 0; + + /* + * When eMMC and SD are powered on for the first time, select a higher + * current value from the corresponding current for eMMC and SD to + * ensure that the eMMC and SD cards start up properly and complete + * initialization. After the initialization process is finished, use + * the corresponding current to set the eMMC and SD to ensure the + * normal work of the device. + */ + + if (!mmc->card) + return max_current; + + if (mmc_card_is_removable(mmc) && mmc_card_mmc(mmc->card)) + load = (type == VMMC_REGULATOR) ? MMC_VMMC_MAX_LOAD_UA : MMC_VQMMC_MAX_LOAD_UA; + else if (mmc_card_sd(mmc->card)) + load = (type == VMMC_REGULATOR) ? SD_VMMC_MAX_LOAD_UA : SD_VQMMC_MAX_LOAD_UA; + + return load; +} + +static int msm_config_regulator_load(struct sdhci_msm_host *msm_host, struct mmc_host *mmc, + bool hpm, int max_current, int type) +{ + int ret; + int load = 0; + + /* + * After the initialization process is finished, Once the type of card + * is determined，only set the corresponding current for SD and eMMC. + */ + + if (mmc->card && !(mmc_card_mmc(mmc->card) || mmc_card_sd(mmc->card))) + return 0; + + if (hpm) + load = sdhci_msm_get_regulator_load(mmc, max_current, type); + + if (type == VMMC_REGULATOR) + ret = regulator_set_load(mmc->supply.vmmc, load); + else + ret = regulator_set_load(mmc->supply.vqmmc, load); + if (ret) + dev_err(mmc_dev(mmc), "%s: set load failed: %d\n", + mmc_hostname(mmc), ret); + return ret; +} + +static int sdhci_msm_set_vmmc(struct sdhci_msm_host *msm_host, + struct mmc_host *mmc, bool hpm) { + int ret; + if (IS_ERR(mmc->supply.vmmc)) return 0; + ret = msm_config_regulator_load(msm_host, mmc, hpm, + MAX_MMC_SD_VMMC_LOAD_UA, VMMC_REGULATOR); + if (ret) + return ret; + return mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, mmc->ios.vdd); } @@ -1435,6 +1512,15 @@ static int msm_toggle_vqmmc(struct sdhci_msm_host *msm_host, goto out; } } + + ret = msm_config_regulator_load(msm_host, mmc, level, + MAX_MMC_SD_VQMMC_LOAD_UA, VQMMC_REGULATOR); + if (ret < 0) { + dev_err(mmc_dev(mmc), "%s: vqmmc set regulator load failed: %d\n", + mmc_hostname(mmc), ret); + goto out; + } + ret = regulator_enable(mmc->supply.vqmmc); } else { ret = regulator_disable(mmc->supply.vqmmc); @@ -1642,7 +1728,8 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq) } if (pwr_state) { - ret = sdhci_msm_set_vmmc(mmc); + ret = sdhci_msm_set_vmmc(msm_host, mmc, + pwr_state & REQ_BUS_ON); if (!ret) ret = sdhci_msm_set_vqmmc(msm_host, mmc, pwr_state & REQ_BUS_ON);
Qualcomm regulator supports two power supply modes: HPM and LPM. Currently, the sdhci-msm.c driver does not set the load to adjust the current for eMMC and SD. Therefore, if the regulator set load in LPM state, it will lead to the inability to properly initialize eMMC and SD. Set the correct regulator current for eMMC and SD to ensure that the device can work normally even when the regulator is in LPM. Signed-off-by: Yuanjie Yang <quic_yuanjiey@quicinc.com> --- drivers/mmc/host/sdhci-msm.c | 91 +++++++++++++++++++++++++++++++++++- 1 file changed, 89 insertions(+), 2 deletions(-)