| Message ID | 20230524162329.819770-2-robimarko@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/4] soc: qcom: socinfo: move SMEM item struct and defines to a header | expand |
On 5/24/2023 9:23 AM, Robert Marko wrote: > Introduce a helper to return the SoC SMEM ID, which is used to identify the > exact SoC model as there may be differences in the same SoC family. > > Currently, cpufreq-nvmem does this completely in the driver and there has > been more interest expresed for other drivers to use this information so > lets expose a common helper to prevent redoing it in individual drivers > since this field is present on every SMEM table version. > > Signed-off-by: Robert Marko <robimarko@gmail.com> > --- > drivers/soc/qcom/smem.c | 19 +++++++++++++++++++ > include/linux/soc/qcom/smem.h | 2 ++ > 2 files changed, 21 insertions(+) > > diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c > index 6be7ea93c78c..0d6ba9bce8cb 100644 > --- a/drivers/soc/qcom/smem.c > +++ b/drivers/soc/qcom/smem.c > @@ -14,6 +14,7 @@ > #include <linux/sizes.h> > #include <linux/slab.h> > #include <linux/soc/qcom/smem.h> > +#include <linux/soc/qcom/socinfo.h> > > /* > * The Qualcomm shared memory system is a allocate only heap structure that > @@ -772,6 +773,24 @@ phys_addr_t qcom_smem_virt_to_phys(void *p) > } > EXPORT_SYMBOL(qcom_smem_virt_to_phys); > > +/** > + * qcom_smem_get_msm_id() - return the SoC ID > + * > + * Look up SoC ID from HW/SW build ID and return it. > + */ > +int qcom_smem_get_msm_id(void) > +{ > + size_t len; > + struct socinfo *info; > + > + info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, &len); > + if (IS_ERR(info)) > + return PTR_ERR(info); > + > + return __le32_to_cpu(info->id); > +} > +EXPORT_SYMBOL(qcom_smem_get_msm_id); EXPORT_SYMBOL_GPL please? Please change it for other symbols in the driver as well w/ separate patch. ---Trilok Soni
On 24.05.2023 20:16, Trilok Soni wrote: > On 5/24/2023 9:23 AM, Robert Marko wrote: >> Introduce a helper to return the SoC SMEM ID, which is used to identify the >> exact SoC model as there may be differences in the same SoC family. >> >> Currently, cpufreq-nvmem does this completely in the driver and there has >> been more interest expresed for other drivers to use this information so >> lets expose a common helper to prevent redoing it in individual drivers >> since this field is present on every SMEM table version. >> >> Signed-off-by: Robert Marko <robimarko@gmail.com> >> --- >> drivers/soc/qcom/smem.c | 19 +++++++++++++++++++ >> include/linux/soc/qcom/smem.h | 2 ++ >> 2 files changed, 21 insertions(+) >> >> diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c >> index 6be7ea93c78c..0d6ba9bce8cb 100644 >> --- a/drivers/soc/qcom/smem.c >> +++ b/drivers/soc/qcom/smem.c >> @@ -14,6 +14,7 @@ >> #include <linux/sizes.h> >> #include <linux/slab.h> >> #include <linux/soc/qcom/smem.h> >> +#include <linux/soc/qcom/socinfo.h> >> /* >> * The Qualcomm shared memory system is a allocate only heap structure that >> @@ -772,6 +773,24 @@ phys_addr_t qcom_smem_virt_to_phys(void *p) >> } >> EXPORT_SYMBOL(qcom_smem_virt_to_phys); >> +/** >> + * qcom_smem_get_msm_id() - return the SoC ID >> + * >> + * Look up SoC ID from HW/SW build ID and return it. >> + */ >> +int qcom_smem_get_msm_id(void) On top of Trilok's point, this should return le32, or at least unsigned int. Konrad >> +{ >> + size_t len; >> + struct socinfo *info; >> + >> + info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, &len); >> + if (IS_ERR(info)) >> + return PTR_ERR(info); >> + >> + return __le32_to_cpu(info->id); >> +} >> +EXPORT_SYMBOL(qcom_smem_get_msm_id); > > EXPORT_SYMBOL_GPL please? > > Please change it for other symbols in the driver as well w/ separate patch. > > ---Trilok Soni > >
On Wed, May 24, 2023 at 08:27:03PM +0200, Konrad Dybcio wrote: > > > On 24.05.2023 20:16, Trilok Soni wrote: > > On 5/24/2023 9:23 AM, Robert Marko wrote: > >> Introduce a helper to return the SoC SMEM ID, which is used to identify the > >> exact SoC model as there may be differences in the same SoC family. > >> > >> Currently, cpufreq-nvmem does this completely in the driver and there has > >> been more interest expresed for other drivers to use this information so > >> lets expose a common helper to prevent redoing it in individual drivers > >> since this field is present on every SMEM table version. > >> > >> Signed-off-by: Robert Marko <robimarko@gmail.com> > >> --- > >> drivers/soc/qcom/smem.c | 19 +++++++++++++++++++ > >> include/linux/soc/qcom/smem.h | 2 ++ > >> 2 files changed, 21 insertions(+) > >> > >> diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c > >> index 6be7ea93c78c..0d6ba9bce8cb 100644 > >> --- a/drivers/soc/qcom/smem.c > >> +++ b/drivers/soc/qcom/smem.c > >> @@ -14,6 +14,7 @@ > >> #include <linux/sizes.h> > >> #include <linux/slab.h> > >> #include <linux/soc/qcom/smem.h> > >> +#include <linux/soc/qcom/socinfo.h> > >> /* > >> * The Qualcomm shared memory system is a allocate only heap structure that > >> @@ -772,6 +773,24 @@ phys_addr_t qcom_smem_virt_to_phys(void *p) > >> } > >> EXPORT_SYMBOL(qcom_smem_virt_to_phys); > >> +/** > >> + * qcom_smem_get_msm_id() - return the SoC ID > >> + * > >> + * Look up SoC ID from HW/SW build ID and return it. > >> + */ > >> +int qcom_smem_get_msm_id(void) > On top of Trilok's point, this should return le32, or at least unsigned int. > Mhhh why unsigned? We would lose error and qcom_smem_get can return all sort of errors. Also I think le32 is problematic as we are converting the value with __le32_to_cpu. > >> +{ > >> + size_t len; > >> + struct socinfo *info; > >> + > >> + info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, &len); > >> + if (IS_ERR(info)) > >> + return PTR_ERR(info); > >> + > >> + return __le32_to_cpu(info->id); > >> +} > >> +EXPORT_SYMBOL(qcom_smem_get_msm_id); > > > > EXPORT_SYMBOL_GPL please? > > > > Please change it for other symbols in the driver as well w/ separate patch. > > > > ---Trilok Soni > > > >
On 24.05.2023 21:57, Christian Marangi wrote: > On Wed, May 24, 2023 at 08:27:03PM +0200, Konrad Dybcio wrote: >> >> >> On 24.05.2023 20:16, Trilok Soni wrote: >>> On 5/24/2023 9:23 AM, Robert Marko wrote: >>>> Introduce a helper to return the SoC SMEM ID, which is used to identify the >>>> exact SoC model as there may be differences in the same SoC family. >>>> >>>> Currently, cpufreq-nvmem does this completely in the driver and there has >>>> been more interest expresed for other drivers to use this information so >>>> lets expose a common helper to prevent redoing it in individual drivers >>>> since this field is present on every SMEM table version. >>>> >>>> Signed-off-by: Robert Marko <robimarko@gmail.com> >>>> --- >>>> drivers/soc/qcom/smem.c | 19 +++++++++++++++++++ >>>> include/linux/soc/qcom/smem.h | 2 ++ >>>> 2 files changed, 21 insertions(+) >>>> >>>> diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c >>>> index 6be7ea93c78c..0d6ba9bce8cb 100644 >>>> --- a/drivers/soc/qcom/smem.c >>>> +++ b/drivers/soc/qcom/smem.c >>>> @@ -14,6 +14,7 @@ >>>> #include <linux/sizes.h> >>>> #include <linux/slab.h> >>>> #include <linux/soc/qcom/smem.h> >>>> +#include <linux/soc/qcom/socinfo.h> >>>> /* >>>> * The Qualcomm shared memory system is a allocate only heap structure that >>>> @@ -772,6 +773,24 @@ phys_addr_t qcom_smem_virt_to_phys(void *p) >>>> } >>>> EXPORT_SYMBOL(qcom_smem_virt_to_phys); >>>> +/** >>>> + * qcom_smem_get_msm_id() - return the SoC ID >>>> + * >>>> + * Look up SoC ID from HW/SW build ID and return it. >>>> + */ >>>> +int qcom_smem_get_msm_id(void) >> On top of Trilok's point, this should return le32, or at least unsigned int. >> > > Mhhh why unsigned? We would lose error and qcom_smem_get can return all > sort of errors. Also I think le32 is problematic as we are converting > the value with __le32_to_cpu. Hm right.. Qcom didn't really think this through then, but hopefully they don't randomly jump from e.g. 547 to 1<<31 Konrad > >>>> +{ >>>> + size_t len; >>>> + struct socinfo *info; >>>> + >>>> + info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, &len); >>>> + if (IS_ERR(info)) >>>> + return PTR_ERR(info); >>>> + >>>> + return __le32_to_cpu(info->id); >>>> +} >>>> +EXPORT_SYMBOL(qcom_smem_get_msm_id); >>> >>> EXPORT_SYMBOL_GPL please? >>> >>> Please change it for other symbols in the driver as well w/ separate patch. >>> >>> ---Trilok Soni >>> >>> >
On Wed, May 24, 2023 at 08:27:03PM +0200, Konrad Dybcio wrote: > > > On 24.05.2023 20:16, Trilok Soni wrote: > > On 5/24/2023 9:23 AM, Robert Marko wrote: > >> Introduce a helper to return the SoC SMEM ID, which is used to identify the > >> exact SoC model as there may be differences in the same SoC family. > >> > >> Currently, cpufreq-nvmem does this completely in the driver and there has > >> been more interest expresed for other drivers to use this information so > >> lets expose a common helper to prevent redoing it in individual drivers > >> since this field is present on every SMEM table version. > >> > >> Signed-off-by: Robert Marko <robimarko@gmail.com> > >> --- > >> drivers/soc/qcom/smem.c | 19 +++++++++++++++++++ > >> include/linux/soc/qcom/smem.h | 2 ++ > >> 2 files changed, 21 insertions(+) > >> > >> diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c > >> index 6be7ea93c78c..0d6ba9bce8cb 100644 > >> --- a/drivers/soc/qcom/smem.c > >> +++ b/drivers/soc/qcom/smem.c > >> @@ -14,6 +14,7 @@ > >> #include <linux/sizes.h> > >> #include <linux/slab.h> > >> #include <linux/soc/qcom/smem.h> > >> +#include <linux/soc/qcom/socinfo.h> > >> /* > >> * The Qualcomm shared memory system is a allocate only heap structure that > >> @@ -772,6 +773,24 @@ phys_addr_t qcom_smem_virt_to_phys(void *p) > >> } > >> EXPORT_SYMBOL(qcom_smem_virt_to_phys); > >> +/** > >> + * qcom_smem_get_msm_id() - return the SoC ID > >> + * > >> + * Look up SoC ID from HW/SW build ID and return it. > >> + */ > >> +int qcom_smem_get_msm_id(void) > On top of Trilok's point, this should return le32, or at least unsigned int. > Returning the value in CPU-native endian sounds very reasonable, so we don't need __le32_to_cpu() on the calling side. If we want to worry about this value going beyond 31 bits the appropriate way would be to take, and assign, a unsigned int* argument and return 0/-errno to indicate success/failure. Regards, Bjorn > Konrad > >> +{ > >> + size_t len; > >> + struct socinfo *info; > >> + > >> + info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, &len); > >> + if (IS_ERR(info)) > >> + return PTR_ERR(info); > >> + > >> + return __le32_to_cpu(info->id); > >> +} > >> +EXPORT_SYMBOL(qcom_smem_get_msm_id); > > > > EXPORT_SYMBOL_GPL please? > > > > Please change it for other symbols in the driver as well w/ separate patch. > > > > ---Trilok Soni > > > >
diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c index 6be7ea93c78c..0d6ba9bce8cb 100644 --- a/drivers/soc/qcom/smem.c +++ b/drivers/soc/qcom/smem.c @@ -14,6 +14,7 @@ #include <linux/sizes.h> #include <linux/slab.h> #include <linux/soc/qcom/smem.h> +#include <linux/soc/qcom/socinfo.h> /* * The Qualcomm shared memory system is a allocate only heap structure that @@ -772,6 +773,24 @@ phys_addr_t qcom_smem_virt_to_phys(void *p) } EXPORT_SYMBOL(qcom_smem_virt_to_phys); +/** + * qcom_smem_get_msm_id() - return the SoC ID + * + * Look up SoC ID from HW/SW build ID and return it. + */ +int qcom_smem_get_msm_id(void) +{ + size_t len; + struct socinfo *info; + + info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, &len); + if (IS_ERR(info)) + return PTR_ERR(info); + + return __le32_to_cpu(info->id); +} +EXPORT_SYMBOL(qcom_smem_get_msm_id); + static int qcom_smem_get_sbl_version(struct qcom_smem *smem) { struct smem_header *header; diff --git a/include/linux/soc/qcom/smem.h b/include/linux/soc/qcom/smem.h index 86e1b358688a..6448607239e6 100644 --- a/include/linux/soc/qcom/smem.h +++ b/include/linux/soc/qcom/smem.h @@ -11,4 +11,6 @@ int qcom_smem_get_free_space(unsigned host); phys_addr_t qcom_smem_virt_to_phys(void *p); +int qcom_smem_get_msm_id(void); + #endif
Introduce a helper to return the SoC SMEM ID, which is used to identify the exact SoC model as there may be differences in the same SoC family. Currently, cpufreq-nvmem does this completely in the driver and there has been more interest expresed for other drivers to use this information so lets expose a common helper to prevent redoing it in individual drivers since this field is present on every SMEM table version. Signed-off-by: Robert Marko <robimarko@gmail.com> --- drivers/soc/qcom/smem.c | 19 +++++++++++++++++++ include/linux/soc/qcom/smem.h | 2 ++ 2 files changed, 21 insertions(+)