| Message ID | 20230121112947.53433-4-robimarko@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/4] soc: qcom: socinfo: move SMEM item struct and defines to a header | expand |
On Sat, Jan 21, 2023 at 12:29:47PM +0100, Robert Marko wrote: > Currently, qcom_cpufreq_get_msm_id() does not simply return the SoC ID > after getting it via SMEM call but instead uses an enum to encode the > matched SMEM ID to 2 variants of MSM8996 which are then used in > qcom_cpufreq_kryo_name_version() to set the supported version. > > This prevents qcom_cpufreq_get_msm_id() from being universal and its doing > more than its name suggests, so lets make it just return the SoC ID > directly which allows matching directly on the SoC ID and removes the need > for msm8996_version enum which simplifies the driver. > It also allows reusing the qcom_cpufreq_get_msm_id() for new SoC-s. > > Signed-off-by: Robert Marko <robimarko@gmail.com> Reviewed-by: Bjorn Andersson <andersson@kernel.org> > --- > drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++-------------------- > 1 file changed, 12 insertions(+), 32 deletions(-) > > diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c > index da55d2e1925a..9deaf9521d6d 100644 > --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c > +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c > @@ -32,12 +32,6 @@ > > #include <dt-bindings/arm/qcom,ids.h> > > -enum _msm8996_version { > - MSM8996_V3, > - MSM8996_SG, > - NUM_OF_MSM8996_VERSIONS, > -}; > - > struct qcom_cpufreq_drv; > > struct qcom_cpufreq_match_data { > @@ -134,30 +128,16 @@ 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) > +static int qcom_cpufreq_get_msm_id(void) > { > size_t len; > struct socinfo *info; > - enum _msm8996_version version; > > info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, &len); > if (IS_ERR(info)) > - return NUM_OF_MSM8996_VERSIONS; > + return PTR_ERR(info); > > - switch (info->id) { > - case QCOM_ID_MSM8996: > - case QCOM_ID_APQ8096: > - version = MSM8996_V3; > - break; > - case QCOM_ID_MSM8996SG: > - case QCOM_ID_APQ8096SG: > - version = MSM8996_SG; > - break; > - default: > - version = NUM_OF_MSM8996_VERSIONS; > - } > - > - return version; > + return info->id; > } > > static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev, > @@ -166,25 +146,25 @@ static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev, > struct qcom_cpufreq_drv *drv) > { > size_t len; > + int msm_id; > 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; > - } > + msm_id = qcom_cpufreq_get_msm_id(); > + if (msm_id < 0) > + return msm_id; > > speedbin = nvmem_cell_read(speedbin_nvmem, &len); > if (IS_ERR(speedbin)) > return PTR_ERR(speedbin); > > - switch (msm8996_version) { > - case MSM8996_V3: > + switch (msm_id) { > + case QCOM_ID_MSM8996: > + case QCOM_ID_APQ8096: > drv->versions = 1 << (unsigned int)(*speedbin); > break; > - case MSM8996_SG: > + case QCOM_ID_MSM8996SG: > + case QCOM_ID_APQ8096SG: > drv->versions = 1 << ((unsigned int)(*speedbin) + 4); > break; > default: > -- > 2.39.1 >
On 21.01.2023 12:29, Robert Marko wrote: > Currently, qcom_cpufreq_get_msm_id() does not simply return the SoC ID > after getting it via SMEM call but instead uses an enum to encode the > matched SMEM ID to 2 variants of MSM8996 which are then used in > qcom_cpufreq_kryo_name_version() to set the supported version. > > This prevents qcom_cpufreq_get_msm_id() from being universal and its doing > more than its name suggests, so lets make it just return the SoC ID > directly which allows matching directly on the SoC ID and removes the need > for msm8996_version enum which simplifies the driver. > It also allows reusing the qcom_cpufreq_get_msm_id() for new SoC-s. > > Signed-off-by: Robert Marko <robimarko@gmail.com> > --- > drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++-------------------- > 1 file changed, 12 insertions(+), 32 deletions(-) > > diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c > index da55d2e1925a..9deaf9521d6d 100644 > --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c > +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c > @@ -32,12 +32,6 @@ > > #include <dt-bindings/arm/qcom,ids.h> > > -enum _msm8996_version { > - MSM8996_V3, > - MSM8996_SG, > - NUM_OF_MSM8996_VERSIONS, > -}; > - > struct qcom_cpufreq_drv; > > struct qcom_cpufreq_match_data { > @@ -134,30 +128,16 @@ 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) > +static int qcom_cpufreq_get_msm_id(void) This should be u32 as info->id is __le32 And please export this function from socinfo, it'll come in useful for other drivers! Konrad > { > size_t len; > struct socinfo *info; > - enum _msm8996_version version; > > info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, &len); > if (IS_ERR(info)) > - return NUM_OF_MSM8996_VERSIONS; > + return PTR_ERR(info); > > - switch (info->id) { > - case QCOM_ID_MSM8996: > - case QCOM_ID_APQ8096: > - version = MSM8996_V3; > - break; > - case QCOM_ID_MSM8996SG: > - case QCOM_ID_APQ8096SG: > - version = MSM8996_SG; > - break; > - default: > - version = NUM_OF_MSM8996_VERSIONS; > - } > - > - return version; > + return info->id; > } > > static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev, > @@ -166,25 +146,25 @@ static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev, > struct qcom_cpufreq_drv *drv) > { > size_t len; > + int msm_id; > 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; > - } > + msm_id = qcom_cpufreq_get_msm_id(); > + if (msm_id < 0) > + return msm_id; > > speedbin = nvmem_cell_read(speedbin_nvmem, &len); > if (IS_ERR(speedbin)) > return PTR_ERR(speedbin); > > - switch (msm8996_version) { > - case MSM8996_V3: > + switch (msm_id) { > + case QCOM_ID_MSM8996: > + case QCOM_ID_APQ8096: > drv->versions = 1 << (unsigned int)(*speedbin); > break; > - case MSM8996_SG: > + case QCOM_ID_MSM8996SG: > + case QCOM_ID_APQ8096SG: > drv->versions = 1 << ((unsigned int)(*speedbin) + 4); > break; > default:
On Sat, 18 Feb 2023 at 16:43, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > > > On 21.01.2023 12:29, Robert Marko wrote: > > Currently, qcom_cpufreq_get_msm_id() does not simply return the SoC ID > > after getting it via SMEM call but instead uses an enum to encode the > > matched SMEM ID to 2 variants of MSM8996 which are then used in > > qcom_cpufreq_kryo_name_version() to set the supported version. > > > > This prevents qcom_cpufreq_get_msm_id() from being universal and its doing > > more than its name suggests, so lets make it just return the SoC ID > > directly which allows matching directly on the SoC ID and removes the need > > for msm8996_version enum which simplifies the driver. > > It also allows reusing the qcom_cpufreq_get_msm_id() for new SoC-s. > > > > Signed-off-by: Robert Marko <robimarko@gmail.com> > > --- > > drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++-------------------- > > 1 file changed, 12 insertions(+), 32 deletions(-) > > > > diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c > > index da55d2e1925a..9deaf9521d6d 100644 > > --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c > > +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c > > @@ -32,12 +32,6 @@ > > > > #include <dt-bindings/arm/qcom,ids.h> > > > > -enum _msm8996_version { > > - MSM8996_V3, > > - MSM8996_SG, > > - NUM_OF_MSM8996_VERSIONS, > > -}; > > - > > struct qcom_cpufreq_drv; > > > > struct qcom_cpufreq_match_data { > > @@ -134,30 +128,16 @@ 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) > > +static int qcom_cpufreq_get_msm_id(void) > This should be u32 as info->id is __le32 > > And please export this function from socinfo, it'll come in > useful for other drivers! How? In my opinion newer drivers should use compat strings rather than depending on the SoC ID. If we were not bound with the compatibility for msm8996pro device trees already using higher bits, we could have dropped this part too.
On 18.02.2023 21:36, Dmitry Baryshkov wrote: > On Sat, 18 Feb 2023 at 16:43, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >> >> >> >> On 21.01.2023 12:29, Robert Marko wrote: >>> Currently, qcom_cpufreq_get_msm_id() does not simply return the SoC ID >>> after getting it via SMEM call but instead uses an enum to encode the >>> matched SMEM ID to 2 variants of MSM8996 which are then used in >>> qcom_cpufreq_kryo_name_version() to set the supported version. >>> >>> This prevents qcom_cpufreq_get_msm_id() from being universal and its doing >>> more than its name suggests, so lets make it just return the SoC ID >>> directly which allows matching directly on the SoC ID and removes the need >>> for msm8996_version enum which simplifies the driver. >>> It also allows reusing the qcom_cpufreq_get_msm_id() for new SoC-s. >>> >>> Signed-off-by: Robert Marko <robimarko@gmail.com> >>> --- >>> drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++-------------------- >>> 1 file changed, 12 insertions(+), 32 deletions(-) >>> >>> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c >>> index da55d2e1925a..9deaf9521d6d 100644 >>> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c >>> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c >>> @@ -32,12 +32,6 @@ >>> >>> #include <dt-bindings/arm/qcom,ids.h> >>> >>> -enum _msm8996_version { >>> - MSM8996_V3, >>> - MSM8996_SG, >>> - NUM_OF_MSM8996_VERSIONS, >>> -}; >>> - >>> struct qcom_cpufreq_drv; >>> >>> struct qcom_cpufreq_match_data { >>> @@ -134,30 +128,16 @@ 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) >>> +static int qcom_cpufreq_get_msm_id(void) >> This should be u32 as info->id is __le32 >> >> And please export this function from socinfo, it'll come in >> useful for other drivers! > > How? In my opinion newer drivers should use compat strings rather than > depending on the SoC ID. If we were not bound with the compatibility > for msm8996pro device trees already using higher bits, we could have > dropped this part too. Adreno speedbin-to-fuse mapping could use SoC detection.. Konrad >
On Sat, 18 Feb 2023 at 21:40, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > > > On 18.02.2023 21:36, Dmitry Baryshkov wrote: > > On Sat, 18 Feb 2023 at 16:43, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > >> > >> > >> > >> On 21.01.2023 12:29, Robert Marko wrote: > >>> Currently, qcom_cpufreq_get_msm_id() does not simply return the SoC ID > >>> after getting it via SMEM call but instead uses an enum to encode the > >>> matched SMEM ID to 2 variants of MSM8996 which are then used in > >>> qcom_cpufreq_kryo_name_version() to set the supported version. > >>> > >>> This prevents qcom_cpufreq_get_msm_id() from being universal and its doing > >>> more than its name suggests, so lets make it just return the SoC ID > >>> directly which allows matching directly on the SoC ID and removes the need > >>> for msm8996_version enum which simplifies the driver. > >>> It also allows reusing the qcom_cpufreq_get_msm_id() for new SoC-s. > >>> > >>> Signed-off-by: Robert Marko <robimarko@gmail.com> > >>> --- > >>> drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++-------------------- > >>> 1 file changed, 12 insertions(+), 32 deletions(-) > >>> > >>> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c > >>> index da55d2e1925a..9deaf9521d6d 100644 > >>> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c > >>> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c > >>> @@ -32,12 +32,6 @@ > >>> > >>> #include <dt-bindings/arm/qcom,ids.h> > >>> > >>> -enum _msm8996_version { > >>> - MSM8996_V3, > >>> - MSM8996_SG, > >>> - NUM_OF_MSM8996_VERSIONS, > >>> -}; > >>> - > >>> struct qcom_cpufreq_drv; > >>> > >>> struct qcom_cpufreq_match_data { > >>> @@ -134,30 +128,16 @@ 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) > >>> +static int qcom_cpufreq_get_msm_id(void) > >> This should be u32 as info->id is __le32 Nice catch. > >> > >> And please export this function from socinfo, it'll come in > >> useful for other drivers! I intentionally did not do that as socinfo is currently fully optional and I dont really like the idea of making it required for anything using SMEM. Regards, Robert > > Konrad > >
On 3.03.2023 19:38, Robert Marko wrote: > On Sat, 18 Feb 2023 at 21:40, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >> >> >> >> On 18.02.2023 21:36, Dmitry Baryshkov wrote: >>> On Sat, 18 Feb 2023 at 16:43, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >>>> >>>> >>>> >>>> On 21.01.2023 12:29, Robert Marko wrote: >>>>> Currently, qcom_cpufreq_get_msm_id() does not simply return the SoC ID >>>>> after getting it via SMEM call but instead uses an enum to encode the >>>>> matched SMEM ID to 2 variants of MSM8996 which are then used in >>>>> qcom_cpufreq_kryo_name_version() to set the supported version. >>>>> >>>>> This prevents qcom_cpufreq_get_msm_id() from being universal and its doing >>>>> more than its name suggests, so lets make it just return the SoC ID >>>>> directly which allows matching directly on the SoC ID and removes the need >>>>> for msm8996_version enum which simplifies the driver. >>>>> It also allows reusing the qcom_cpufreq_get_msm_id() for new SoC-s. >>>>> >>>>> Signed-off-by: Robert Marko <robimarko@gmail.com> >>>>> --- >>>>> drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++-------------------- >>>>> 1 file changed, 12 insertions(+), 32 deletions(-) >>>>> >>>>> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c >>>>> index da55d2e1925a..9deaf9521d6d 100644 >>>>> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c >>>>> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c >>>>> @@ -32,12 +32,6 @@ >>>>> >>>>> #include <dt-bindings/arm/qcom,ids.h> >>>>> >>>>> -enum _msm8996_version { >>>>> - MSM8996_V3, >>>>> - MSM8996_SG, >>>>> - NUM_OF_MSM8996_VERSIONS, >>>>> -}; >>>>> - >>>>> struct qcom_cpufreq_drv; >>>>> >>>>> struct qcom_cpufreq_match_data { >>>>> @@ -134,30 +128,16 @@ 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) >>>>> +static int qcom_cpufreq_get_msm_id(void) >>>> This should be u32 as info->id is __le32 > > Nice catch. > > >>>> >>>> And please export this function from socinfo, it'll come in >>>> useful for other drivers! > > I intentionally did not do that as socinfo is currently fully optional > and I dont really like > the idea of making it required for anything using SMEM. "anything using SMEM"? As in the drivers, or SoCs? If the former, I don't see how exporting a function from within socid and using it here would make it required for other drivers. If the latter, we're talking non-qcom SoCs. SMEM has been with us forever. I'm planning to reuse this for Adreno speedbin matching. It's one of those blocks that don't have a revision and/or bin reigster within themselves. Konrad > > Regards, > Robert > >> >> Konrad >>>
On Fri, 3 Mar 2023 at 21:46, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > > > On 3.03.2023 19:38, Robert Marko wrote: > > On Sat, 18 Feb 2023 at 21:40, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > >> > >> > >> > >> On 18.02.2023 21:36, Dmitry Baryshkov wrote: > >>> On Sat, 18 Feb 2023 at 16:43, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > >>>> > >>>> > >>>> > >>>> On 21.01.2023 12:29, Robert Marko wrote: > >>>>> Currently, qcom_cpufreq_get_msm_id() does not simply return the SoC ID > >>>>> after getting it via SMEM call but instead uses an enum to encode the > >>>>> matched SMEM ID to 2 variants of MSM8996 which are then used in > >>>>> qcom_cpufreq_kryo_name_version() to set the supported version. > >>>>> > >>>>> This prevents qcom_cpufreq_get_msm_id() from being universal and its doing > >>>>> more than its name suggests, so lets make it just return the SoC ID > >>>>> directly which allows matching directly on the SoC ID and removes the need > >>>>> for msm8996_version enum which simplifies the driver. > >>>>> It also allows reusing the qcom_cpufreq_get_msm_id() for new SoC-s. > >>>>> > >>>>> Signed-off-by: Robert Marko <robimarko@gmail.com> > >>>>> --- > >>>>> drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++-------------------- > >>>>> 1 file changed, 12 insertions(+), 32 deletions(-) > >>>>> > >>>>> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c > >>>>> index da55d2e1925a..9deaf9521d6d 100644 > >>>>> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c > >>>>> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c > >>>>> @@ -32,12 +32,6 @@ > >>>>> > >>>>> #include <dt-bindings/arm/qcom,ids.h> > >>>>> > >>>>> -enum _msm8996_version { > >>>>> - MSM8996_V3, > >>>>> - MSM8996_SG, > >>>>> - NUM_OF_MSM8996_VERSIONS, > >>>>> -}; > >>>>> - > >>>>> struct qcom_cpufreq_drv; > >>>>> > >>>>> struct qcom_cpufreq_match_data { > >>>>> @@ -134,30 +128,16 @@ 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) > >>>>> +static int qcom_cpufreq_get_msm_id(void) > >>>> This should be u32 as info->id is __le32 > > > > Nice catch. > > > > > >>>> > >>>> And please export this function from socinfo, it'll come in > >>>> useful for other drivers! > > > > I intentionally did not do that as socinfo is currently fully optional > > and I dont really like > > the idea of making it required for anything using SMEM. > "anything using SMEM"? As in the drivers, or SoCs? > If the former, I don't see how exporting a function from within > socid and using it here would make it required for other drivers. > If the latter, we're talking non-qcom SoCs. SMEM has been with > us forever. I feel we have a misunderstanding, currently, cpufreq-nvmem does not depend on socinfo being built so I don't want to require it as a dependency in order to get the SMEM ID. Granted, socinfo is useful on any QCA SoC so if adding new dependecies to cpufreq-nvmem is acceptable I am not against exporting it there. > > > I'm planning to reuse this for Adreno speedbin matching. It's one > of those blocks that don't have a revision and/or bin reigster > within themselves. I understand the use case, I am sure it will be required in some other places sooner or later as well. Regards, Robert > > Konrad > > > > Regards, > > Robert > > > >> > >> Konrad > >>>
On 3.03.2023 22:38, Robert Marko wrote: > On Fri, 3 Mar 2023 at 21:46, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >> >> >> >> On 3.03.2023 19:38, Robert Marko wrote: >>> On Sat, 18 Feb 2023 at 21:40, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >>>> >>>> >>>> >>>> On 18.02.2023 21:36, Dmitry Baryshkov wrote: >>>>> On Sat, 18 Feb 2023 at 16:43, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >>>>>> >>>>>> >>>>>> >>>>>> On 21.01.2023 12:29, Robert Marko wrote: >>>>>>> Currently, qcom_cpufreq_get_msm_id() does not simply return the SoC ID >>>>>>> after getting it via SMEM call but instead uses an enum to encode the >>>>>>> matched SMEM ID to 2 variants of MSM8996 which are then used in >>>>>>> qcom_cpufreq_kryo_name_version() to set the supported version. >>>>>>> >>>>>>> This prevents qcom_cpufreq_get_msm_id() from being universal and its doing >>>>>>> more than its name suggests, so lets make it just return the SoC ID >>>>>>> directly which allows matching directly on the SoC ID and removes the need >>>>>>> for msm8996_version enum which simplifies the driver. >>>>>>> It also allows reusing the qcom_cpufreq_get_msm_id() for new SoC-s. >>>>>>> >>>>>>> Signed-off-by: Robert Marko <robimarko@gmail.com> >>>>>>> --- >>>>>>> drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++-------------------- >>>>>>> 1 file changed, 12 insertions(+), 32 deletions(-) >>>>>>> >>>>>>> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c >>>>>>> index da55d2e1925a..9deaf9521d6d 100644 >>>>>>> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c >>>>>>> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c >>>>>>> @@ -32,12 +32,6 @@ >>>>>>> >>>>>>> #include <dt-bindings/arm/qcom,ids.h> >>>>>>> >>>>>>> -enum _msm8996_version { >>>>>>> - MSM8996_V3, >>>>>>> - MSM8996_SG, >>>>>>> - NUM_OF_MSM8996_VERSIONS, >>>>>>> -}; >>>>>>> - >>>>>>> struct qcom_cpufreq_drv; >>>>>>> >>>>>>> struct qcom_cpufreq_match_data { >>>>>>> @@ -134,30 +128,16 @@ 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) >>>>>>> +static int qcom_cpufreq_get_msm_id(void) >>>>>> This should be u32 as info->id is __le32 >>> >>> Nice catch. >>> >>> >>>>>> >>>>>> And please export this function from socinfo, it'll come in >>>>>> useful for other drivers! >>> >>> I intentionally did not do that as socinfo is currently fully optional >>> and I dont really like >>> the idea of making it required for anything using SMEM. >> "anything using SMEM"? As in the drivers, or SoCs? >> If the former, I don't see how exporting a function from within >> socid and using it here would make it required for other drivers. >> If the latter, we're talking non-qcom SoCs. SMEM has been with >> us forever. > > I feel we have a misunderstanding, > currently, cpufreq-nvmem does not depend on socinfo being built > so I don't want to require it as a dependency in order to get the SMEM ID. Okay yeah we simply weren't on the same page. > > Granted, socinfo is useful on any QCA SoC so if adding new dependecies to > cpufreq-nvmem is acceptable I am not against exporting it there. IMO, it would be acceptable. Let's hear if others are on board too. Konrad >> >> >> I'm planning to reuse this for Adreno speedbin matching. It's one >> of those blocks that don't have a revision and/or bin reigster >> within themselves. > > I understand the use case, I am sure it will be required in some other places > sooner or later as well. > > Regards, > Robert >> >> Konrad >>> >>> Regards, >>> Robert >>> >>>> >>>> Konrad >>>>>
On 03/03/2023 22:46, Konrad Dybcio wrote: > > > On 3.03.2023 19:38, Robert Marko wrote: >> On Sat, 18 Feb 2023 at 21:40, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >>> >>> >>> >>> On 18.02.2023 21:36, Dmitry Baryshkov wrote: >>>> On Sat, 18 Feb 2023 at 16:43, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >>>>> >>>>> >>>>> >>>>> On 21.01.2023 12:29, Robert Marko wrote: >>>>>> Currently, qcom_cpufreq_get_msm_id() does not simply return the SoC ID >>>>>> after getting it via SMEM call but instead uses an enum to encode the >>>>>> matched SMEM ID to 2 variants of MSM8996 which are then used in >>>>>> qcom_cpufreq_kryo_name_version() to set the supported version. >>>>>> >>>>>> This prevents qcom_cpufreq_get_msm_id() from being universal and its doing >>>>>> more than its name suggests, so lets make it just return the SoC ID >>>>>> directly which allows matching directly on the SoC ID and removes the need >>>>>> for msm8996_version enum which simplifies the driver. >>>>>> It also allows reusing the qcom_cpufreq_get_msm_id() for new SoC-s. >>>>>> >>>>>> Signed-off-by: Robert Marko <robimarko@gmail.com> >>>>>> --- >>>>>> drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++-------------------- >>>>>> 1 file changed, 12 insertions(+), 32 deletions(-) >>>>>> >>>>>> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c >>>>>> index da55d2e1925a..9deaf9521d6d 100644 >>>>>> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c >>>>>> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c >>>>>> @@ -32,12 +32,6 @@ >>>>>> >>>>>> #include <dt-bindings/arm/qcom,ids.h> >>>>>> >>>>>> -enum _msm8996_version { >>>>>> - MSM8996_V3, >>>>>> - MSM8996_SG, >>>>>> - NUM_OF_MSM8996_VERSIONS, >>>>>> -}; >>>>>> - >>>>>> struct qcom_cpufreq_drv; >>>>>> >>>>>> struct qcom_cpufreq_match_data { >>>>>> @@ -134,30 +128,16 @@ 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) >>>>>> +static int qcom_cpufreq_get_msm_id(void) >>>>> This should be u32 as info->id is __le32 >> >> Nice catch. >> >> >>>>> >>>>> And please export this function from socinfo, it'll come in >>>>> useful for other drivers! >> >> I intentionally did not do that as socinfo is currently fully optional >> and I dont really like >> the idea of making it required for anything using SMEM. > "anything using SMEM"? As in the drivers, or SoCs? > If the former, I don't see how exporting a function from within > socid and using it here would make it required for other drivers. > If the latter, we're talking non-qcom SoCs. SMEM has been with > us forever. > > > I'm planning to reuse this for Adreno speedbin matching. It's one > of those blocks that don't have a revision and/or bin reigster > within themselves. I have mixed feelings towards this. And anyway it might be better to add get_msm_id() function to SCM driver, rather than parsing the data here.
diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c index da55d2e1925a..9deaf9521d6d 100644 --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c @@ -32,12 +32,6 @@ #include <dt-bindings/arm/qcom,ids.h> -enum _msm8996_version { - MSM8996_V3, - MSM8996_SG, - NUM_OF_MSM8996_VERSIONS, -}; - struct qcom_cpufreq_drv; struct qcom_cpufreq_match_data { @@ -134,30 +128,16 @@ 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) +static int qcom_cpufreq_get_msm_id(void) { size_t len; struct socinfo *info; - enum _msm8996_version version; info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, &len); if (IS_ERR(info)) - return NUM_OF_MSM8996_VERSIONS; + return PTR_ERR(info); - switch (info->id) { - case QCOM_ID_MSM8996: - case QCOM_ID_APQ8096: - version = MSM8996_V3; - break; - case QCOM_ID_MSM8996SG: - case QCOM_ID_APQ8096SG: - version = MSM8996_SG; - break; - default: - version = NUM_OF_MSM8996_VERSIONS; - } - - return version; + return info->id; } static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev, @@ -166,25 +146,25 @@ static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev, struct qcom_cpufreq_drv *drv) { size_t len; + int msm_id; 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; - } + msm_id = qcom_cpufreq_get_msm_id(); + if (msm_id < 0) + return msm_id; speedbin = nvmem_cell_read(speedbin_nvmem, &len); if (IS_ERR(speedbin)) return PTR_ERR(speedbin); - switch (msm8996_version) { - case MSM8996_V3: + switch (msm_id) { + case QCOM_ID_MSM8996: + case QCOM_ID_APQ8096: drv->versions = 1 << (unsigned int)(*speedbin); break; - case MSM8996_SG: + case QCOM_ID_MSM8996SG: + case QCOM_ID_APQ8096SG: drv->versions = 1 << ((unsigned int)(*speedbin) + 4); break; default:
Currently, qcom_cpufreq_get_msm_id() does not simply return the SoC ID after getting it via SMEM call but instead uses an enum to encode the matched SMEM ID to 2 variants of MSM8996 which are then used in qcom_cpufreq_kryo_name_version() to set the supported version. This prevents qcom_cpufreq_get_msm_id() from being universal and its doing more than its name suggests, so lets make it just return the SoC ID directly which allows matching directly on the SoC ID and removes the need for msm8996_version enum which simplifies the driver. It also allows reusing the qcom_cpufreq_get_msm_id() for new SoC-s. Signed-off-by: Robert Marko <robimarko@gmail.com> --- drivers/cpufreq/qcom-cpufreq-nvmem.c | 44 ++++++++-------------------- 1 file changed, 12 insertions(+), 32 deletions(-)