Hi, Marco > Hi Anson, > > On 19-06-27 07:01, Anson Huang wrote: > > Hi, Daniel > > > > > On Thu, Jun 27, 2019 at 3:48 AM Anson Huang <anson.hu...@nxp.com> > > > wrote: > > > > > > > > Hi, Daniel > > > > > > > > > On Wed, Jun 26, 2019 at 10:06 AM <anson.hu...@nxp.com> wrote: > > > > > > > > > > > > From: Anson Huang <anson.hu...@nxp.com> > > > > > > > > > > > > Add i.MX SCU SoC's UID(unique identifier) support, user can > > > > > > read it from sysfs: > > > > > > > > > > > > root@imx8qxpmek:~# cat /sys/devices/soc0/soc_uid > > > > > > 7B64280B57AC1898 > > > > > > > > > > > > Signed-off-by: Anson Huang <anson.hu...@nxp.com> > > > > > > --- > > > > > > drivers/soc/imx/soc-imx-scu.c | 35 > > > > > > +++++++++++++++++++++++++++++++++++ > > > > > > 1 file changed, 35 insertions(+) > > > > > > > > > > > > diff --git a/drivers/soc/imx/soc-imx-scu.c > > > > > > b/drivers/soc/imx/soc-imx-scu.c index 676f612..8d322a1 100644 > > > > > > --- a/drivers/soc/imx/soc-imx-scu.c > > > > > > +++ b/drivers/soc/imx/soc-imx-scu.c > > > > > > @@ -27,6 +27,36 @@ struct imx_sc_msg_misc_get_soc_id { > > > > > > } data; > > > > > > } __packed; > > > > > > > > > > > > +struct imx_sc_msg_misc_get_soc_uid { > > > > > > + struct imx_sc_rpc_msg hdr; > > > > > > + u32 uid_low; > > > > > > + u32 uid_high; > > > > > > +} __packed; > > > > > > + > > > > > > +static ssize_t soc_uid_show(struct device *dev, > > > > > > + struct device_attribute *attr, > > > > > > +char > > > > > > +*buf) { > > > > > > + struct imx_sc_msg_misc_get_soc_uid msg; > > > > > > + struct imx_sc_rpc_msg *hdr = &msg.hdr; > > > > > > + u64 soc_uid; > > > > > > + > > > > > > + hdr->ver = IMX_SC_RPC_VERSION; > > > > > > + hdr->svc = IMX_SC_RPC_SVC_MISC; > > > > > > + hdr->func = IMX_SC_MISC_FUNC_UNIQUE_ID; > > > > > > + hdr->size = 1; > > > > > > + > > > > > > + /* the return value of SCU FW is in correct, skip > > > > > > + return value check */ > > > > > > > > > > Why do you mean by "in correct"? > > > > > > > > I made a mistake, it should be "incorrect", the existing SCFW of > > > > this API returns an error value even this API is successfully > > > > called, to make it work with current SCFW, I have to skip the > > > > return value check for this API for now. Will send V2 patch to fix this > typo. > > > > > > Thanks Anson! It makes sense now. It is a little bit sad though > > > because we won't know when there is a "real" error :). > > > > > > Lets update the comment to be more specific: > > > > > > /* SCFW FW API always returns an error even the function is > > > successfully executed, so skip returned value */ > > > > OK, as for external users, the SCFW formally released has this issue, > > so for now I have to skip the return value check for this API, once > > next SCFW release has this issue fixed, I will add a patch to check the > > return > value. > > Do you really keep track of that? Please can you add a FIXME: or TODO: > tag and add the firmware version containing that bug?
Thanks for reminder, I just double checked the SCU FW code, it is actually a mistake, the SCU FW API of sc_misc_unique_id() is actually a void function, which means it does NOT return anything. While in our internal kernel tree, we make SCU IPC call to sc_misc_unique_id() with return value check, and the return value is failure (-5) always. When I clean up the code for upstream, I did NOT notice it. So the correct comment should be, this API does NOT return anything, no need to check the returned value. I will fix the comment in next version. void sc_misc_unique_id(sc_ipc_t ipc, uint32_t *id_l, uint32_t *id_h) Thanks, Anson > > Regards, > Marco > > > Thanks, > > Anson. > > > > > > > > > > > > + imx_scu_call_rpc(soc_ipc_handle, &msg, true); > > > > > > + > > > > > > + soc_uid = msg.uid_high; > > > > > > + soc_uid <<= 32; > > > > > > + soc_uid |= msg.uid_low; > > > > > > + > > > > > > + return sprintf(buf, "%016llX\n", soc_uid); > > > > > > > > > > snprintf? > > > > > > > > The snprintf is to avoid buffer overflow, which in this case, I > > > > don't know the size of "buf", and the value(u64) to be printed is > > > > with fixed length of 64, so I think sprint is just OK. > > > > > > Ok. > > --