| Message ID | 20211014083016.137441-5-y.oudjana@protonmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Add support for MSM8996 Pro | expand |
On 14/10/2021 11:32, Yassine Oudjana wrote: > In preparation for adding a separate device tree for MSM8996 Pro, skip reading > msm-id from smem and just read the speedbin efuse. It would be nice to comment, why is it possible/necessary. For example: MSM8996 Pro has completely different set of frequencies, so it makes no sense to have a single table covering Pro and original SoCs. msm8996pro.dtsi would override frequency table from msm8996.dtsi. > > Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com> > --- > drivers/cpufreq/Kconfig.arm | 1 - > drivers/cpufreq/qcom-cpufreq-nvmem.c | 75 +++------------------------- > 2 files changed, 6 insertions(+), 70 deletions(-) > > diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm > index 954749afb5fe..7d9798bc5753 100644 > --- a/drivers/cpufreq/Kconfig.arm > +++ b/drivers/cpufreq/Kconfig.arm > @@ -154,7 +154,6 @@ config ARM_QCOM_CPUFREQ_NVMEM > tristate "Qualcomm nvmem based CPUFreq" > depends on ARCH_QCOM > depends on QCOM_QFPROM > - depends on QCOM_SMEM > select PM_OPP > help > This adds the CPUFreq driver for Qualcomm Kryo SoC based boards. > diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c > index d1744b5d9619..909f7d97b334 100644 > --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c > +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c > @@ -9,8 +9,8 @@ > * based on the silicon variant in use. Qualcomm Process Voltage Scaling Tables > * defines the voltage and frequency value based on the msm-id in SMEM > * and speedbin blown in the efuse combination. > - * The qcom-cpufreq-nvmem driver reads the msm-id and efuse value from the SoC > - * to provide the OPP framework with required information. > + * The qcom-cpufreq-nvmem driver reads efuse value from the SoC to provide the > + * OPP framework with required information. > * This is used to determine the voltage and frequency value for each OPP of > * operating-points-v2 table when it is parsed by the OPP framework. > */ > @@ -27,22 +27,6 @@ > #include <linux/pm_domain.h> > #include <linux/pm_opp.h> > #include <linux/slab.h> > -#include <linux/soc/qcom/smem.h> > - > -#define MSM_ID_SMEM 137 > - > -enum _msm_id { > - MSM8996V3 = 0xF6ul, > - APQ8096V3 = 0x123ul, > - MSM8996SG = 0x131ul, > - APQ8096SG = 0x138ul, > -}; > - > -enum _msm8996_version { > - MSM8996_V3, > - MSM8996_SG, > - NUM_OF_MSM8996_VERSIONS, > -}; > > struct qcom_cpufreq_drv; > > @@ -142,35 +126,6 @@ static void get_krait_bin_format_b(struct device *cpu_dev, > dev_dbg(cpu_dev, "PVS version: %d\n", *pvs_ver); > } > > -static enum _msm8996_version qcom_cpufreq_get_msm_id(void) > -{ > - size_t len; > - u32 *msm_id; > - enum _msm8996_version version; > - > - msm_id = qcom_smem_get(QCOM_SMEM_HOST_ANY, MSM_ID_SMEM, &len); > - if (IS_ERR(msm_id)) > - return NUM_OF_MSM8996_VERSIONS; > - > - /* The first 4 bytes are format, next to them is the actual msm-id */ > - msm_id++; > - > - switch ((enum _msm_id)*msm_id) { > - case MSM8996V3: > - case APQ8096V3: > - version = MSM8996_V3; > - break; > - case MSM8996SG: > - case APQ8096SG: > - version = MSM8996_SG; > - break; > - default: > - version = NUM_OF_MSM8996_VERSIONS; > - } > - > - return version; > -} > - > static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev, > struct nvmem_cell *speedbin_nvmem, > char **pvs_name, > @@ -178,30 +133,13 @@ static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev, > { > size_t len; > u8 *speedbin; > - enum _msm8996_version msm8996_version; > *pvs_name = NULL; > > - msm8996_version = qcom_cpufreq_get_msm_id(); > - if (NUM_OF_MSM8996_VERSIONS == msm8996_version) { > - dev_err(cpu_dev, "Not Snapdragon 820/821!"); > - return -ENODEV; > - } > - > speedbin = nvmem_cell_read(speedbin_nvmem, &len); > if (IS_ERR(speedbin)) > return PTR_ERR(speedbin); > > - switch (msm8996_version) { > - case MSM8996_V3: > - drv->versions = 1 << (unsigned int)(*speedbin); > - break; > - case MSM8996_SG: > - drv->versions = 1 << ((unsigned int)(*speedbin) + 4); > - break; > - default: > - BUG(); > - break; > - } > + drv->versions = 1 << (unsigned int)(*speedbin); > > kfree(speedbin); > return 0; > @@ -464,10 +402,9 @@ static const struct of_device_id qcom_cpufreq_match_list[] __initconst = { > MODULE_DEVICE_TABLE(of, qcom_cpufreq_match_list); > > /* > - * Since the driver depends on smem and nvmem drivers, which may > - * return EPROBE_DEFER, all the real activity is done in the probe, > - * which may be defered as well. The init here is only registering > - * the driver and the platform device. > + * Since the driver depends on the nvmem driver, which may return EPROBE_DEFER, > + * all the real activity is done in the probe, which may be defered as well. > + * The init here is only registering the driver and the platform device. > */ > static int __init qcom_cpufreq_init(void) > { >
On 14.10.2021 10:32, Yassine Oudjana wrote: > In preparation for adding a separate device tree for MSM8996 Pro, skip reading > msm-id from smem and just read the speedbin efuse. > While I'd really like for this to be merged, it's gonna totally wreck backwards compatibility.. But then, since APCC was not defined properly before commit 0a275a35ceab07 arm64: dts: qcom: msm8996: Make CPUCC actually probe (and work) there's only 5.14/5.15 (both of which were non-LTS) which would *actually* break given somebody decided that "ah yes, pulling in DTs from these specific mainline kernel releases is a good idea"... If I were to judge, it would probably be fine to rid the old mechanism.. Konrad
On Fri 15 Oct 11:58 PDT 2021, Konrad Dybcio wrote: > > On 14.10.2021 10:32, Yassine Oudjana wrote: > > In preparation for adding a separate device tree for MSM8996 Pro, skip reading > > msm-id from smem and just read the speedbin efuse. > > > While I'd really like for this to be merged, it's gonna totally wreck backwards > > compatibility.. But then, since APCC was not defined properly before commit > > 0a275a35ceab07 arm64: dts: qcom: msm8996: Make CPUCC actually probe (and work) > > there's only 5.14/5.15 (both of which were non-LTS) which would *actually* break given > > somebody decided that "ah yes, pulling in DTs from these specific mainline kernel releases > > is a good idea"... > > > If I were to judge, it would probably be fine to rid the old mechanism.. > Given that various people have reported instabilities on db820c in its current form - and prior to that it was too slow - I think it's fine to favour getting this sorted out properly over backwards compatibility. Regards, Bjorn
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index 954749afb5fe..7d9798bc5753 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -154,7 +154,6 @@ config ARM_QCOM_CPUFREQ_NVMEM tristate "Qualcomm nvmem based CPUFreq" depends on ARCH_QCOM depends on QCOM_QFPROM - depends on QCOM_SMEM select PM_OPP help This adds the CPUFreq driver for Qualcomm Kryo SoC based boards. diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c index d1744b5d9619..909f7d97b334 100644 --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c @@ -9,8 +9,8 @@ * based on the silicon variant in use. Qualcomm Process Voltage Scaling Tables * defines the voltage and frequency value based on the msm-id in SMEM * and speedbin blown in the efuse combination. - * The qcom-cpufreq-nvmem driver reads the msm-id and efuse value from the SoC - * to provide the OPP framework with required information. + * The qcom-cpufreq-nvmem driver reads efuse value from the SoC to provide the + * OPP framework with required information. * This is used to determine the voltage and frequency value for each OPP of * operating-points-v2 table when it is parsed by the OPP framework. */ @@ -27,22 +27,6 @@ #include <linux/pm_domain.h> #include <linux/pm_opp.h> #include <linux/slab.h> -#include <linux/soc/qcom/smem.h> - -#define MSM_ID_SMEM 137 - -enum _msm_id { - MSM8996V3 = 0xF6ul, - APQ8096V3 = 0x123ul, - MSM8996SG = 0x131ul, - APQ8096SG = 0x138ul, -}; - -enum _msm8996_version { - MSM8996_V3, - MSM8996_SG, - NUM_OF_MSM8996_VERSIONS, -}; struct qcom_cpufreq_drv; @@ -142,35 +126,6 @@ static void get_krait_bin_format_b(struct device *cpu_dev, dev_dbg(cpu_dev, "PVS version: %d\n", *pvs_ver); } -static enum _msm8996_version qcom_cpufreq_get_msm_id(void) -{ - size_t len; - u32 *msm_id; - enum _msm8996_version version; - - msm_id = qcom_smem_get(QCOM_SMEM_HOST_ANY, MSM_ID_SMEM, &len); - if (IS_ERR(msm_id)) - return NUM_OF_MSM8996_VERSIONS; - - /* The first 4 bytes are format, next to them is the actual msm-id */ - msm_id++; - - switch ((enum _msm_id)*msm_id) { - case MSM8996V3: - case APQ8096V3: - version = MSM8996_V3; - break; - case MSM8996SG: - case APQ8096SG: - version = MSM8996_SG; - break; - default: - version = NUM_OF_MSM8996_VERSIONS; - } - - return version; -} - static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev, struct nvmem_cell *speedbin_nvmem, char **pvs_name, @@ -178,30 +133,13 @@ static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev, { size_t len; u8 *speedbin; - enum _msm8996_version msm8996_version; *pvs_name = NULL; - msm8996_version = qcom_cpufreq_get_msm_id(); - if (NUM_OF_MSM8996_VERSIONS == msm8996_version) { - dev_err(cpu_dev, "Not Snapdragon 820/821!"); - return -ENODEV; - } - speedbin = nvmem_cell_read(speedbin_nvmem, &len); if (IS_ERR(speedbin)) return PTR_ERR(speedbin); - switch (msm8996_version) { - case MSM8996_V3: - drv->versions = 1 << (unsigned int)(*speedbin); - break; - case MSM8996_SG: - drv->versions = 1 << ((unsigned int)(*speedbin) + 4); - break; - default: - BUG(); - break; - } + drv->versions = 1 << (unsigned int)(*speedbin); kfree(speedbin); return 0; @@ -464,10 +402,9 @@ static const struct of_device_id qcom_cpufreq_match_list[] __initconst = { MODULE_DEVICE_TABLE(of, qcom_cpufreq_match_list); /* - * Since the driver depends on smem and nvmem drivers, which may - * return EPROBE_DEFER, all the real activity is done in the probe, - * which may be defered as well. The init here is only registering - * the driver and the platform device. + * Since the driver depends on the nvmem driver, which may return EPROBE_DEFER, + * all the real activity is done in the probe, which may be defered as well. + * The init here is only registering the driver and the platform device. */ static int __init qcom_cpufreq_init(void) {
In preparation for adding a separate device tree for MSM8996 Pro, skip reading msm-id from smem and just read the speedbin efuse. Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com> --- drivers/cpufreq/Kconfig.arm | 1 - drivers/cpufreq/qcom-cpufreq-nvmem.c | 75 +++------------------------- 2 files changed, 6 insertions(+), 70 deletions(-)