Message ID | 20190626070706.24930-1-Anson.Huang@nxp.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | soc: imx-scu: Add SoC UID(unique identifier) support | expand |
On Wed, Jun 26, 2019 at 10:06 AM <Anson.Huang@nxp.com> wrote: > > From: Anson Huang <Anson.Huang@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.Huang@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"? > + 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? > +} > + > +static DEVICE_ATTR_RO(soc_uid); > + > static int imx_scu_soc_id(void) > { > struct imx_sc_msg_misc_get_soc_id msg; > @@ -102,6 +132,11 @@ static int imx_scu_soc_probe(struct platform_device *pdev) > goto free_revision; > } > > + ret = device_create_file(soc_device_to_device(soc_dev), > + &dev_attr_soc_uid); > + if (ret) > + goto free_revision; > + > return 0; > > free_revision: > -- > 2.7.4 >
Hi, Daniel > -----Original Message----- > From: Daniel Baluta <daniel.baluta@gmail.com> > Sent: Wednesday, June 26, 2019 8:42 PM > To: Anson Huang <anson.huang@nxp.com> > Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > <s.hauer@pengutronix.de>; Pengutronix Kernel Team > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Aisheng > Dong <aisheng.dong@nxp.com>; Abel Vesa <abel.vesa@nxp.com>; linux- > arm-kernel <linux-arm-kernel@lists.infradead.org>; Linux Kernel Mailing List > <linux-kernel@vger.kernel.org>; dl-linux-imx <linux-imx@nxp.com>; Daniel > Baluta <daniel.baluta@nxp.com> > Subject: Re: [PATCH] soc: imx-scu: Add SoC UID(unique identifier) support > > On Wed, Jun 26, 2019 at 10:06 AM <Anson.Huang@nxp.com> wrote: > > > > From: Anson Huang <Anson.Huang@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.Huang@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. > > + 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. Anson. > > > +} > > + > > +static DEVICE_ATTR_RO(soc_uid); > > + > > static int imx_scu_soc_id(void) > > { > > struct imx_sc_msg_misc_get_soc_id msg; @@ -102,6 +132,11 @@ > > static int imx_scu_soc_probe(struct platform_device *pdev) > > goto free_revision; > > } > > > > + ret = device_create_file(soc_device_to_device(soc_dev), > > + &dev_attr_soc_uid); > > + if (ret) > > + goto free_revision; > > + > > return 0; > > > > free_revision: > > -- > > 2.7.4 > >
On Thu, Jun 27, 2019 at 3:48 AM Anson Huang <anson.huang@nxp.com> wrote: > > Hi, Daniel > > > -----Original Message----- > > From: Daniel Baluta <daniel.baluta@gmail.com> > > Sent: Wednesday, June 26, 2019 8:42 PM > > To: Anson Huang <anson.huang@nxp.com> > > Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > > <s.hauer@pengutronix.de>; Pengutronix Kernel Team > > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Aisheng > > Dong <aisheng.dong@nxp.com>; Abel Vesa <abel.vesa@nxp.com>; linux- > > arm-kernel <linux-arm-kernel@lists.infradead.org>; Linux Kernel Mailing List > > <linux-kernel@vger.kernel.org>; dl-linux-imx <linux-imx@nxp.com>; Daniel > > Baluta <daniel.baluta@nxp.com> > > Subject: Re: [PATCH] soc: imx-scu: Add SoC UID(unique identifier) support > > > > On Wed, Jun 26, 2019 at 10:06 AM <Anson.Huang@nxp.com> wrote: > > > > > > From: Anson Huang <Anson.Huang@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.Huang@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 */ > > > + 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.
Hi, Daniel > -----Original Message----- > From: Daniel Baluta <daniel.baluta@gmail.com> > Sent: Thursday, June 27, 2019 2:44 PM > To: Anson Huang <anson.huang@nxp.com> > Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > <s.hauer@pengutronix.de>; Pengutronix Kernel Team > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Aisheng > Dong <aisheng.dong@nxp.com>; Abel Vesa <abel.vesa@nxp.com>; linux- > arm-kernel <linux-arm-kernel@lists.infradead.org>; Linux Kernel Mailing List > <linux-kernel@vger.kernel.org>; dl-linux-imx <linux-imx@nxp.com>; Daniel > Baluta <daniel.baluta@nxp.com> > Subject: Re: [PATCH] soc: imx-scu: Add SoC UID(unique identifier) support > > On Thu, Jun 27, 2019 at 3:48 AM Anson Huang <anson.huang@nxp.com> > wrote: > > > > Hi, Daniel > > > > > -----Original Message----- > > > From: Daniel Baluta <daniel.baluta@gmail.com> > > > Sent: Wednesday, June 26, 2019 8:42 PM > > > To: Anson Huang <anson.huang@nxp.com> > > > Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > > > <s.hauer@pengutronix.de>; Pengutronix Kernel Team > > > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; > Aisheng > > > Dong <aisheng.dong@nxp.com>; Abel Vesa <abel.vesa@nxp.com>; linux- > > > arm-kernel <linux-arm-kernel@lists.infradead.org>; Linux Kernel > > > Mailing List <linux-kernel@vger.kernel.org>; dl-linux-imx > > > <linux-imx@nxp.com>; Daniel Baluta <daniel.baluta@nxp.com> > > > Subject: Re: [PATCH] soc: imx-scu: Add SoC UID(unique identifier) > > > support > > > > > > On Wed, Jun 26, 2019 at 10:06 AM <Anson.Huang@nxp.com> wrote: > > > > > > > > From: Anson Huang <Anson.Huang@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.Huang@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. 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.
Hi Anson, On 19-06-27 07:01, Anson Huang wrote: > Hi, Daniel > > > -----Original Message----- > > From: Daniel Baluta <daniel.baluta@gmail.com> > > Sent: Thursday, June 27, 2019 2:44 PM > > To: Anson Huang <anson.huang@nxp.com> > > Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > > <s.hauer@pengutronix.de>; Pengutronix Kernel Team > > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Aisheng > > Dong <aisheng.dong@nxp.com>; Abel Vesa <abel.vesa@nxp.com>; linux- > > arm-kernel <linux-arm-kernel@lists.infradead.org>; Linux Kernel Mailing List > > <linux-kernel@vger.kernel.org>; dl-linux-imx <linux-imx@nxp.com>; Daniel > > Baluta <daniel.baluta@nxp.com> > > Subject: Re: [PATCH] soc: imx-scu: Add SoC UID(unique identifier) support > > > > On Thu, Jun 27, 2019 at 3:48 AM Anson Huang <anson.huang@nxp.com> > > wrote: > > > > > > Hi, Daniel > > > > > > > -----Original Message----- > > > > From: Daniel Baluta <daniel.baluta@gmail.com> > > > > Sent: Wednesday, June 26, 2019 8:42 PM > > > > To: Anson Huang <anson.huang@nxp.com> > > > > Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > > > > <s.hauer@pengutronix.de>; Pengutronix Kernel Team > > > > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; > > Aisheng > > > > Dong <aisheng.dong@nxp.com>; Abel Vesa <abel.vesa@nxp.com>; linux- > > > > arm-kernel <linux-arm-kernel@lists.infradead.org>; Linux Kernel > > > > Mailing List <linux-kernel@vger.kernel.org>; dl-linux-imx > > > > <linux-imx@nxp.com>; Daniel Baluta <daniel.baluta@nxp.com> > > > > Subject: Re: [PATCH] soc: imx-scu: Add SoC UID(unique identifier) > > > > support > > > > > > > > On Wed, Jun 26, 2019 at 10:06 AM <Anson.Huang@nxp.com> wrote: > > > > > > > > > > From: Anson Huang <Anson.Huang@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.Huang@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? 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.
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.huang@nxp.com> > > > wrote: > > > > > > > > Hi, Daniel > > > > > > > > > On Wed, Jun 26, 2019 at 10:06 AM <Anson.Huang@nxp.com> wrote: > > > > > > > > > > > > From: Anson Huang <Anson.Huang@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.Huang@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. > > --
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 */ + 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); +} + +static DEVICE_ATTR_RO(soc_uid); + static int imx_scu_soc_id(void) { struct imx_sc_msg_misc_get_soc_id msg; @@ -102,6 +132,11 @@ static int imx_scu_soc_probe(struct platform_device *pdev) goto free_revision; } + ret = device_create_file(soc_device_to_device(soc_dev), + &dev_attr_soc_uid); + if (ret) + goto free_revision; + return 0; free_revision: