[v3,3/5] soc: qcom: smem: introduce qcom_smem_get_msm_id()

  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 v3:
* Change export to EXPORT_SYMBOL_GPL
* Use an argument for returning SoC ID
* Update kerneldoc
---
 drivers/soc/qcom/smem.c       | 24 ++++++++++++++++++++++++
 include/linux/soc/qcom/smem.h |  2 ++
 2 files changed, 26 insertions(+)
  

On 5/25/2023 5:39 PM, 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 v3:
> * Change export to EXPORT_SYMBOL_GPL
> * Use an argument for returning SoC ID
> * Update kerneldoc
> ---
>   drivers/soc/qcom/smem.c       | 24 ++++++++++++++++++++++++
>   include/linux/soc/qcom/smem.h |  2 ++
>   2 files changed, 26 insertions(+)
>
> diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
> index bc98520c4969..185ed0da11a1 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,29 @@ phys_addr_t qcom_smem_virt_to_phys(void *p)
>   }
>   EXPORT_SYMBOL_GPL(qcom_smem_virt_to_phys);
>   
> +/**
> + * qcom_smem_get_msm_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_msm_id(u32 *id)


I think, MSM  is not the only platform which will leverage this API. 
qcom_smem_get_soc_id() / qcom_smem_get_cpu_id() would make more sense 
than qcom_smem_get_msm_id() ?


> +{
> +	size_t len;
> +	struct socinfo *info;
> +
> +	info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, &len);


len is unused after this, can we just pass NULL? Did a quick check on 
the code, if we pass the address, size of the item will be updated, else no.


> +	if (IS_ERR(info))
> +		return PTR_ERR(info);
> +
> +	*id = info->id;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(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..cb204ad6373c 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(u32 *id);
> +
>   #endif
  

On Thu, 25 May 2023 at 14:59, Kathiravan T <quic_kathirav@quicinc.com> wrote:
>
>
> On 5/25/2023 5:39 PM, 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 v3:
> > * Change export to EXPORT_SYMBOL_GPL
> > * Use an argument for returning SoC ID
> > * Update kerneldoc
> > ---
> >   drivers/soc/qcom/smem.c       | 24 ++++++++++++++++++++++++
> >   include/linux/soc/qcom/smem.h |  2 ++
> >   2 files changed, 26 insertions(+)
> >
> > diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
> > index bc98520c4969..185ed0da11a1 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,29 @@ phys_addr_t qcom_smem_virt_to_phys(void *p)
> >   }
> >   EXPORT_SYMBOL_GPL(qcom_smem_virt_to_phys);
> >
> > +/**
> > + * qcom_smem_get_msm_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_msm_id(u32 *id)
>
>
> I think, MSM  is not the only platform which will leverage this API.
> qcom_smem_get_soc_id() / qcom_smem_get_cpu_id() would make more sense
> than qcom_smem_get_msm_id() ?

I agree, qcom_smem_get_soc_id() sounds better to me as its not just MSM parts.
>
>
> > +{
> > +     size_t len;
> > +     struct socinfo *info;
> > +
> > +     info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, &len);
>
>
> len is unused after this, can we just pass NULL? Did a quick check on
> the code, if we pass the address, size of the item will be updated, else no.

Yes, indeed passing NULL works here for the simple case this helper is handling.
Will address in v4.

Regards,
Robert
>
>
> > +     if (IS_ERR(info))
> > +             return PTR_ERR(info);
> > +
> > +     *id = info->id;
> > +
> > +     return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(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..cb204ad6373c 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(u32 *id);
> > +
> >   #endif
  

On 25.05.2023 17:03, Robert Marko wrote:
> On Thu, 25 May 2023 at 14:59, Kathiravan T <quic_kathirav@quicinc.com> wrote:
>>
>>
>> On 5/25/2023 5:39 PM, 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 v3:
>>> * Change export to EXPORT_SYMBOL_GPL
>>> * Use an argument for returning SoC ID
>>> * Update kerneldoc
>>> ---
>>>   drivers/soc/qcom/smem.c       | 24 ++++++++++++++++++++++++
>>>   include/linux/soc/qcom/smem.h |  2 ++
>>>   2 files changed, 26 insertions(+)
>>>
>>> diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
>>> index bc98520c4969..185ed0da11a1 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,29 @@ phys_addr_t qcom_smem_virt_to_phys(void *p)
>>>   }
>>>   EXPORT_SYMBOL_GPL(qcom_smem_virt_to_phys);
>>>
>>> +/**
>>> + * qcom_smem_get_msm_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_msm_id(u32 *id)
>>
>>
>> I think, MSM  is not the only platform which will leverage this API.
>> qcom_smem_get_soc_id() / qcom_smem_get_cpu_id() would make more sense
>> than qcom_smem_get_msm_id() ?
> 
> I agree, qcom_smem_get_soc_id() sounds better to me as its not just MSM parts.
>>
>>
>>> +{
>>> +     size_t len;
>>> +     struct socinfo *info;
>>> +
>>> +     info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, &len);
>>
>>
>> len is unused after this, can we just pass NULL? Did a quick check on
>> the code, if we pass the address, size of the item will be updated, else no.
> 
> Yes, indeed passing NULL works here for the simple case this helper is handling.
> Will address in v4.
Please also consider Bjorn's suggestion of using PTR_ERR

Konrad
> 
> Regards,
> Robert
>>
>>
>>> +     if (IS_ERR(info))
>>> +             return PTR_ERR(info);
>>> +
>>> +     *id = info->id;
>>> +
>>> +     return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(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..cb204ad6373c 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(u32 *id);
>>> +
>>>   #endif
  

On 25.05.2023 23:21, Konrad Dybcio wrote:
> 
> 
> On 25.05.2023 17:03, Robert Marko wrote:
>> On Thu, 25 May 2023 at 14:59, Kathiravan T <quic_kathirav@quicinc.com> wrote:
>>>
>>>
>>> On 5/25/2023 5:39 PM, 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 v3:
>>>> * Change export to EXPORT_SYMBOL_GPL
>>>> * Use an argument for returning SoC ID
>>>> * Update kerneldoc
>>>> ---
>>>>   drivers/soc/qcom/smem.c       | 24 ++++++++++++++++++++++++
>>>>   include/linux/soc/qcom/smem.h |  2 ++
>>>>   2 files changed, 26 insertions(+)
>>>>
>>>> diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
>>>> index bc98520c4969..185ed0da11a1 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,29 @@ phys_addr_t qcom_smem_virt_to_phys(void *p)
>>>>   }
>>>>   EXPORT_SYMBOL_GPL(qcom_smem_virt_to_phys);
>>>>
>>>> +/**
>>>> + * qcom_smem_get_msm_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_msm_id(u32 *id)
>>>
>>>
>>> I think, MSM  is not the only platform which will leverage this API.
>>> qcom_smem_get_soc_id() / qcom_smem_get_cpu_id() would make more sense
>>> than qcom_smem_get_msm_id() ?
>>
>> I agree, qcom_smem_get_soc_id() sounds better to me as its not just MSM parts.
>>>
>>>
>>>> +{
>>>> +     size_t len;
>>>> +     struct socinfo *info;
>>>> +
>>>> +     info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, &len);
>>>
>>>
>>> len is unused after this, can we just pass NULL? Did a quick check on
>>> the code, if we pass the address, size of the item will be updated, else no.
>>
>> Yes, indeed passing NULL works here for the simple case this helper is handling.
>> Will address in v4.
> Please also consider Bjorn's suggestion of using PTR_ERR
Nevermind, brain too laggy ;)

Konrad
> 
> Konrad
>>
>> Regards,
>> Robert
>>>
>>>
>>>> +     if (IS_ERR(info))
>>>> +             return PTR_ERR(info);
>>>> +
>>>> +     *id = info->id;
>>>> +
>>>> +     return 0;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(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..cb204ad6373c 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(u32 *id);
>>>> +
>>>>   #endif