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

[Robert Marko]

On Thu, 25 May 2023 at 14:59, Kathiravan T <quic_kathirav@xxxxxxxxxxx> 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@xxxxxxxxx>
> > ---
> > 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