| Message ID | 20230525210214.78235-3-robimarko@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v4,1/5] soc: qcom: socinfo: move SMEM item struct and defines to a header | expand |
On 25.05.2023 23:02, 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> > --- > Changes in v4: > * Change helper name to qcom_smem_get_soc_id() > * Remove len and just pass NULL, that is sufficient here > > Changes in v3: > * Change export to EXPORT_SYMBOL_GPL > * Use an argument for returning SoC ID > * Update kerneldoc > --- > drivers/soc/qcom/smem.c | 23 +++++++++++++++++++++++ > include/linux/soc/qcom/smem.h | 2 ++ > 2 files changed, 25 insertions(+) > > diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c > index bc98520c4969..78cf79ea4924 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,28 @@ phys_addr_t qcom_smem_virt_to_phys(void *p) > } > EXPORT_SYMBOL_GPL(qcom_smem_virt_to_phys); > > +/** > + * qcom_smem_get_soc_id() - return the SoC ID > + * @id: On success, we return the SoC ID here. > + * > + * Look up SoC ID from HW/SW build ID and return it. > + * > + * Return: 0 on success, negative errno on failure. > + */ > +int qcom_smem_get_soc_id(u32 *id) __le32 *id LGTM otherwise! Konrad > +{ > + struct socinfo *info; > + > + info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, NULL); > + if (IS_ERR(info)) > + return PTR_ERR(info); > + > + *id = info->id; > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(qcom_smem_get_soc_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..223db6a9c733 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_soc_id(u32 *id); > + > #endif
On Fri, May 26, 2023 at 01:18:17AM +0200, Konrad Dybcio wrote: > > > On 25.05.2023 23:02, 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> > > --- > > Changes in v4: > > * Change helper name to qcom_smem_get_soc_id() > > * Remove len and just pass NULL, that is sufficient here > > > > Changes in v3: > > * Change export to EXPORT_SYMBOL_GPL > > * Use an argument for returning SoC ID > > * Update kerneldoc > > --- > > drivers/soc/qcom/smem.c | 23 +++++++++++++++++++++++ > > include/linux/soc/qcom/smem.h | 2 ++ > > 2 files changed, 25 insertions(+) > > > > diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c > > index bc98520c4969..78cf79ea4924 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,28 @@ phys_addr_t qcom_smem_virt_to_phys(void *p) > > } > > EXPORT_SYMBOL_GPL(qcom_smem_virt_to_phys); > > > > +/** > > + * qcom_smem_get_soc_id() - return the SoC ID > > + * @id: On success, we return the SoC ID here. > > + * > > + * Look up SoC ID from HW/SW build ID and return it. > > + * > > + * Return: 0 on success, negative errno on failure. > > + */ > > +int qcom_smem_get_soc_id(u32 *id) > __le32 *id > Why do you want this passed back to the user in little endian? When is it not going to be compared to a cpu-endian constant? > LGTM otherwise! > > Konrad > > +{ > > + struct socinfo *info; > > + > > + info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, NULL); > > + if (IS_ERR(info)) > > + return PTR_ERR(info); > > + > > + *id = info->id; This should be __le32_to_cpu() though... Regards, Bjorn > > + > > + return 0; > > +} > > +EXPORT_SYMBOL_GPL(qcom_smem_get_soc_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..223db6a9c733 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_soc_id(u32 *id); > > + > > #endif
On 26.05.2023 04:33, Bjorn Andersson wrote: > On Fri, May 26, 2023 at 01:18:17AM +0200, Konrad Dybcio wrote: >> >> >> On 25.05.2023 23:02, 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> >>> --- >>> Changes in v4: >>> * Change helper name to qcom_smem_get_soc_id() >>> * Remove len and just pass NULL, that is sufficient here >>> >>> Changes in v3: >>> * Change export to EXPORT_SYMBOL_GPL >>> * Use an argument for returning SoC ID >>> * Update kerneldoc >>> --- >>> drivers/soc/qcom/smem.c | 23 +++++++++++++++++++++++ >>> include/linux/soc/qcom/smem.h | 2 ++ >>> 2 files changed, 25 insertions(+) >>> >>> diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c >>> index bc98520c4969..78cf79ea4924 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,28 @@ phys_addr_t qcom_smem_virt_to_phys(void *p) >>> } >>> EXPORT_SYMBOL_GPL(qcom_smem_virt_to_phys); >>> >>> +/** >>> + * qcom_smem_get_soc_id() - return the SoC ID >>> + * @id: On success, we return the SoC ID here. >>> + * >>> + * Look up SoC ID from HW/SW build ID and return it. >>> + * >>> + * Return: 0 on success, negative errno on failure. >>> + */ >>> +int qcom_smem_get_soc_id(u32 *id) >> __le32 *id >> > > Why do you want this passed back to the user in little endian? When is > it not going to be compared to a cpu-endian constant? Ugh. You're right. This makes no sense. Konrad > >> LGTM otherwise! >> >> Konrad >>> +{ >>> + struct socinfo *info; >>> + >>> + info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, NULL); >>> + if (IS_ERR(info)) >>> + return PTR_ERR(info); >>> + >>> + *id = info->id; > > This should be __le32_to_cpu() though... > > Regards, > Bjorn > >>> + >>> + return 0; >>> +} >>> +EXPORT_SYMBOL_GPL(qcom_smem_get_soc_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..223db6a9c733 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_soc_id(u32 *id); >>> + >>> #endif
On Thu, May 25, 2023 at 11:02:12PM +0200, 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> > --- > Changes in v4: > * Change helper name to qcom_smem_get_soc_id() > * Remove len and just pass NULL, that is sufficient here > > Changes in v3: > * Change export to EXPORT_SYMBOL_GPL > * Use an argument for returning SoC ID > * Update kerneldoc > --- > drivers/soc/qcom/smem.c | 23 +++++++++++++++++++++++ > include/linux/soc/qcom/smem.h | 2 ++ > 2 files changed, 25 insertions(+) > > diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c > index bc98520c4969..78cf79ea4924 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,28 @@ phys_addr_t qcom_smem_virt_to_phys(void *p) > } > EXPORT_SYMBOL_GPL(qcom_smem_virt_to_phys); > > +/** > + * qcom_smem_get_soc_id() - return the SoC ID > + * @id: On success, we return the SoC ID here. > + * > + * Look up SoC ID from HW/SW build ID and return it. > + * > + * Return: 0 on success, negative errno on failure. > + */ > +int qcom_smem_get_soc_id(u32 *id) > +{ > + struct socinfo *info; > + > + info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, NULL); > + if (IS_ERR(info)) > + return PTR_ERR(info); > + > + *id = info->id; So just to make the discussion between Konrad and me clear, please wrap the info->id access in __le32_to_cpu() and the series is ready to go. Thanks, Bjorn > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(qcom_smem_get_soc_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..223db6a9c733 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_soc_id(u32 *id); > + > #endif > -- > 2.40.1 >
diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c index bc98520c4969..78cf79ea4924 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,28 @@ phys_addr_t qcom_smem_virt_to_phys(void *p) } EXPORT_SYMBOL_GPL(qcom_smem_virt_to_phys); +/** + * qcom_smem_get_soc_id() - return the SoC ID + * @id: On success, we return the SoC ID here. + * + * Look up SoC ID from HW/SW build ID and return it. + * + * Return: 0 on success, negative errno on failure. + */ +int qcom_smem_get_soc_id(u32 *id) +{ + struct socinfo *info; + + info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, NULL); + if (IS_ERR(info)) + return PTR_ERR(info); + + *id = info->id; + + return 0; +} +EXPORT_SYMBOL_GPL(qcom_smem_get_soc_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..223db6a9c733 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_soc_id(u32 *id); + #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> --- Changes in v4: * Change helper name to qcom_smem_get_soc_id() * Remove len and just pass NULL, that is sufficient here Changes in v3: * Change export to EXPORT_SYMBOL_GPL * Use an argument for returning SoC ID * Update kerneldoc --- drivers/soc/qcom/smem.c | 23 +++++++++++++++++++++++ include/linux/soc/qcom/smem.h | 2 ++ 2 files changed, 25 insertions(+)