diff mbox series

[v4,3/5] soc: qcom: smem: introduce qcom_smem_get_soc_id()

Message ID 20230525210214.78235-3-robimarko@gmail.com
State New
Headers
Series [v4,1/5] soc: qcom: socinfo: move SMEM item struct and defines to a header |

Commit Message

Robert Marko May 25, 2023, 9:02 p.m. UTC 
  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(+)

Comments

Konrad Dybcio May 25, 2023, 11:18 p.m. UTC | #1 
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
Bjorn Andersson May 26, 2023, 2:33 a.m. UTC | #2 
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
Konrad Dybcio May 26, 2023, 8:42 a.m. UTC | #3 
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
Bjorn Andersson May 26, 2023, 7:04 p.m. UTC | #4 
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 mbox series

Patch

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