| Message ID | 20210618180907.258149-3-angelogioacchino.delregno@somainline.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Implement SPM/SAW for MSM8998 and SDM6xx | expand |
On Fri, Jun 18, 2021 at 08:09:06PM +0200, AngeloGioacchino Del Regno wrote: > Implement the support for SAW v4.1, used in at least MSM8998, > SDM630, SDM660 and APQ variants and, while at it, also add the > configuration for the SDM630/660 Silver and Gold cluster L2 > Adaptive Voltage Scaler: this is also one of the prerequisites > to allow the OSM controller to perform DCVS. > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org> > --- > drivers/soc/qcom/spm.c | 28 +++++++++++++++++++++++++++- > 1 file changed, 27 insertions(+), 1 deletion(-) > > diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c > index 0c8aa9240c41..843732d12c54 100644 > --- a/drivers/soc/qcom/spm.c > +++ b/drivers/soc/qcom/spm.c > @@ -32,9 +32,28 @@ enum spm_reg { > SPM_REG_SEQ_ENTRY, > SPM_REG_SPM_STS, > SPM_REG_PMIC_STS, > + SPM_REG_AVS_CTL, > + SPM_REG_AVS_LIMIT, > SPM_REG_NR, > }; > > +static const u16 spm_reg_offset_v4_1[SPM_REG_NR] = { > + [SPM_REG_AVS_CTL] = 0x904, > + [SPM_REG_AVS_LIMIT] = 0x908, > +}; > + > +static const struct spm_reg_data spm_reg_660_gold_l2 = { > + .reg_offset = spm_reg_offset_v4_1, > + .avs_ctl = 0x1010031, > + .avs_limit = 0x4580458, > +}; > + > +static const struct spm_reg_data spm_reg_660_silver_l2 = { > + .reg_offset = spm_reg_offset_v4_1, > + .avs_ctl = 0x101c031, I was just randomly looking for the same value in downstream and it looks like Qualcomm reverted something here to the same value as for the gold cluster, claiming "stability issues": https://source.codeaurora.org/quic/la/kernel/msm-4.4/commit/?h=LA.UM.8.2.r2-04600-sdm660.0&id=5a07b7336a1b3fa6a3ac67470805259c5026206e The commit seems still present in recent qcom tags. I cannot say anything about this, but could you confirm if you are intentionally not also doing the same as qcom did in that commit? Thanks, Stephan
Il 19/06/21 00:17, Stephan Gerhold ha scritto: > On Fri, Jun 18, 2021 at 08:09:06PM +0200, AngeloGioacchino Del Regno wrote: >> Implement the support for SAW v4.1, used in at least MSM8998, >> SDM630, SDM660 and APQ variants and, while at it, also add the >> configuration for the SDM630/660 Silver and Gold cluster L2 >> Adaptive Voltage Scaler: this is also one of the prerequisites >> to allow the OSM controller to perform DCVS. >> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org> >> --- >> drivers/soc/qcom/spm.c | 28 +++++++++++++++++++++++++++- >> 1 file changed, 27 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c >> index 0c8aa9240c41..843732d12c54 100644 >> --- a/drivers/soc/qcom/spm.c >> +++ b/drivers/soc/qcom/spm.c >> @@ -32,9 +32,28 @@ enum spm_reg { >> SPM_REG_SEQ_ENTRY, >> SPM_REG_SPM_STS, >> SPM_REG_PMIC_STS, >> + SPM_REG_AVS_CTL, >> + SPM_REG_AVS_LIMIT, >> SPM_REG_NR, >> }; >> >> +static const u16 spm_reg_offset_v4_1[SPM_REG_NR] = { >> + [SPM_REG_AVS_CTL] = 0x904, >> + [SPM_REG_AVS_LIMIT] = 0x908, >> +}; >> + >> +static const struct spm_reg_data spm_reg_660_gold_l2 = { >> + .reg_offset = spm_reg_offset_v4_1, >> + .avs_ctl = 0x1010031, >> + .avs_limit = 0x4580458, >> +}; >> + >> +static const struct spm_reg_data spm_reg_660_silver_l2 = { >> + .reg_offset = spm_reg_offset_v4_1, >> + .avs_ctl = 0x101c031, > > I was just randomly looking for the same value in downstream and it > looks like Qualcomm reverted something here to the same value as for > the gold cluster, claiming "stability issues": > > https://source.codeaurora.org/quic/la/kernel/msm-4.4/commit/?h=LA.UM.8.2.r2-04600-sdm660.0&id=5a07b7336a1b3fa6a3ac67470805259c5026206e > > The commit seems still present in recent qcom tags. I cannot say > anything about this, but could you confirm if you are intentionally > not also doing the same as qcom did in that commit? > I am intentionally not doing the same as that commit; 4 out of 6 devices experienced random lockups with the values you mentioned (4x SDM630, 2x SDM636, of which all SDM630 and one SDM636 device are affected). > Thanks, > Stephan >
On Sat, Jun 19, 2021 at 12:39:00AM +0200, AngeloGioacchino Del Regno wrote: > Il 19/06/21 00:17, Stephan Gerhold ha scritto: > > On Fri, Jun 18, 2021 at 08:09:06PM +0200, AngeloGioacchino Del Regno wrote: > > > Implement the support for SAW v4.1, used in at least MSM8998, > > > SDM630, SDM660 and APQ variants and, while at it, also add the > > > configuration for the SDM630/660 Silver and Gold cluster L2 > > > Adaptive Voltage Scaler: this is also one of the prerequisites > > > to allow the OSM controller to perform DCVS. > > > > > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org> > > > --- > > > drivers/soc/qcom/spm.c | 28 +++++++++++++++++++++++++++- > > > 1 file changed, 27 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c > > > index 0c8aa9240c41..843732d12c54 100644 > > > --- a/drivers/soc/qcom/spm.c > > > +++ b/drivers/soc/qcom/spm.c > > > @@ -32,9 +32,28 @@ enum spm_reg { > > > SPM_REG_SEQ_ENTRY, > > > SPM_REG_SPM_STS, > > > SPM_REG_PMIC_STS, > > > + SPM_REG_AVS_CTL, > > > + SPM_REG_AVS_LIMIT, > > > SPM_REG_NR, > > > }; > > > +static const u16 spm_reg_offset_v4_1[SPM_REG_NR] = { > > > + [SPM_REG_AVS_CTL] = 0x904, > > > + [SPM_REG_AVS_LIMIT] = 0x908, > > > +}; > > > + > > > +static const struct spm_reg_data spm_reg_660_gold_l2 = { > > > + .reg_offset = spm_reg_offset_v4_1, > > > + .avs_ctl = 0x1010031, > > > + .avs_limit = 0x4580458, > > > +}; > > > + > > > +static const struct spm_reg_data spm_reg_660_silver_l2 = { > > > + .reg_offset = spm_reg_offset_v4_1, > > > + .avs_ctl = 0x101c031, > > > > I was just randomly looking for the same value in downstream and it > > looks like Qualcomm reverted something here to the same value as for > > the gold cluster, claiming "stability issues": > > > > https://source.codeaurora.org/quic/la/kernel/msm-4.4/commit/?h=LA.UM.8.2.r2-04600-sdm660.0&id=5a07b7336a1b3fa6a3ac67470805259c5026206e > > > > The commit seems still present in recent qcom tags. I cannot say > > anything about this, but could you confirm if you are intentionally > > not also doing the same as qcom did in that commit? > > > > I am intentionally not doing the same as that commit; 4 out of 6 devices > experienced random lockups with the values you mentioned (4x SDM630, > 2x SDM636, of which all SDM630 and one SDM636 device are affected). > Might be worth a short comment in the file or commit message? Just in case someone is wondering the same in the future. You probably don't want someone else to refer to that commit in the future and suddenly your devices will experience "random lockups". :)
diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c index 0c8aa9240c41..843732d12c54 100644 --- a/drivers/soc/qcom/spm.c +++ b/drivers/soc/qcom/spm.c @@ -32,9 +32,28 @@ enum spm_reg { SPM_REG_SEQ_ENTRY, SPM_REG_SPM_STS, SPM_REG_PMIC_STS, + SPM_REG_AVS_CTL, + SPM_REG_AVS_LIMIT, SPM_REG_NR, }; +static const u16 spm_reg_offset_v4_1[SPM_REG_NR] = { + [SPM_REG_AVS_CTL] = 0x904, + [SPM_REG_AVS_LIMIT] = 0x908, +}; + +static const struct spm_reg_data spm_reg_660_gold_l2 = { + .reg_offset = spm_reg_offset_v4_1, + .avs_ctl = 0x1010031, + .avs_limit = 0x4580458, +}; + +static const struct spm_reg_data spm_reg_660_silver_l2 = { + .reg_offset = spm_reg_offset_v4_1, + .avs_ctl = 0x101c031, + .avs_limit = 0x4580458, +}; + static const u16 spm_reg_offset_v2_1[SPM_REG_NR] = { [SPM_REG_CFG] = 0x08, [SPM_REG_SPM_CTL] = 0x30, @@ -126,6 +145,10 @@ void spm_set_low_power_mode(struct spm_driver_data *drv, } static const struct of_device_id spm_match_table[] = { + { .compatible = "qcom,sdm660-gold-saw2-v4.1-l2", + .data = &spm_reg_660_gold_l2 }, + { .compatible = "qcom,sdm660-silver-saw2-v4.1-l2", + .data = &spm_reg_660_silver_l2 }, { .compatible = "qcom,msm8974-saw2-v2.1-cpu", .data = &spm_reg_8974_8084_cpu }, { .compatible = "qcom,apq8084-saw2-v2.1-cpu", @@ -169,6 +192,8 @@ static int spm_dev_probe(struct platform_device *pdev) * CPU was held in reset, the reset signal could trigger the SPM state * machine, before the sequences are completely written. */ + spm_register_write(drv, SPM_REG_AVS_CTL, drv->reg_data->avs_ctl); + spm_register_write(drv, SPM_REG_AVS_LIMIT, drv->reg_data->avs_limit); spm_register_write(drv, SPM_REG_CFG, drv->reg_data->spm_cfg); spm_register_write(drv, SPM_REG_DLY, drv->reg_data->spm_dly); spm_register_write(drv, SPM_REG_PMIC_DLY, drv->reg_data->pmic_dly); @@ -178,7 +203,8 @@ static int spm_dev_probe(struct platform_device *pdev) drv->reg_data->pmic_data[1]); /* Set up Standby as the default low power mode */ - spm_set_low_power_mode(drv, PM_SLEEP_MODE_STBY); + if (drv->reg_data->reg_offset[SPM_REG_SPM_CTL]) + spm_set_low_power_mode(drv, PM_SLEEP_MODE_STBY); return 0; }
Implement the support for SAW v4.1, used in at least MSM8998, SDM630, SDM660 and APQ variants and, while at it, also add the configuration for the SDM630/660 Silver and Gold cluster L2 Adaptive Voltage Scaler: this is also one of the prerequisites to allow the OSM controller to perform DCVS. Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org> --- drivers/soc/qcom/spm.c | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-)