Use meson_gx_socinfo variable for chipid compatible call
from meson-gx-socinfo driver if available.
Signed-off-by: Viacheslav Bocharov <adeep@lexina.in>
---
drivers/firmware/meson/meson_sm.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/meson/meson_sm.c b/drivers/firmware/meson/meson_sm.c
index 2820f4ac871b..29b53a8a6941 100644
--- a/drivers/firmware/meson/meson_sm.c
+++ b/drivers/firmware/meson/meson_sm.c
@@ -23,6 +23,10 @@
#include <linux/firmware/meson/meson_sm.h>
+#ifdef CONFIG_MESON_GX_SOCINFO
+extern unsigned int meson_gx_socinfo;
+#endif
+
struct meson_sm_cmd {
unsigned int index;
u32 smc_id;
@@ -310,7 +314,19 @@ static ssize_t chipid_show(struct device *dev, struct device_attribute *attr,
buff = kmalloc(SM_CHIP_ID_LENGTH, GFP_KERNEL);
if (!buff)
return -ENOMEM;
- ((uint32_t *)buff)[0] = 0; // CPU_ID is empty
+#ifdef CONFIG_MESON_GX_SOCINFO
+ uint8_t *ch;
+ int i;
+
+ ((uint32_t *)buff)[0] =
+ ((meson_gx_socinfo & 0xff000000) | // Family ID
+ ((meson_gx_socinfo << 8) & 0xff0000) | // Chip Revision
+ ((meson_gx_socinfo >> 8) & 0xff00)); // Package ID
+
+ ((uint32_t *)buff)[0] = htonl(((uint32_t *)buff)[0]);
+#else
+ ((uint32)t *)buff)[0] = 0;
+#endif
/* Transform into expected order for display */
ch = (uint8_t *)(id_buf + 4);
for (i = 0; i < 12; i++)
--
2.34.1
Hi Viacheslav,
kernel test robot noticed the following build errors:
[auto build test ERROR on soc/for-next]
[also build test ERROR on linus/master v6.6 next-20231102]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Viacheslav-Bocharov/firmware-meson-sm-change-sprintf-to-scnprintf/20231102-172556
base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link: https://lore.kernel.org/r/20231102074916.3280809-5-adeep%40lexina.in
patch subject: [PATCH 4/4] firmware: meson_sm: use meson_gx_socinfo for compatibility
config: arm64-randconfig-003-20231103 (https://download.01.org/0day-ci/archive/20231103/202311030839.2qiIuOUl-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231103/202311030839.2qiIuOUl-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311030839.2qiIuOUl-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/firmware/meson/meson_sm.c: In function 'chipid_show':
>> drivers/firmware/meson/meson_sm.c:328:19: error: 'uint32' undeclared (first use in this function); did you mean 'uint32_t'?
328 | ((uint32)t *)buff)[0] = 0;
| ^~~~~~
| uint32_t
drivers/firmware/meson/meson_sm.c:328:19: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/firmware/meson/meson_sm.c:328:26: error: expected ')' before 't'
328 | ((uint32)t *)buff)[0] = 0;
| ~ ^
| )
>> drivers/firmware/meson/meson_sm.c:328:30: error: expected ';' before 'buff'
328 | ((uint32)t *)buff)[0] = 0;
| ^~~~
| ;
>> drivers/firmware/meson/meson_sm.c:328:34: error: expected statement before ')' token
328 | ((uint32)t *)buff)[0] = 0;
| ^
>> drivers/firmware/meson/meson_sm.c:328:35: error: expected expression before '[' token
328 | ((uint32)t *)buff)[0] = 0;
| ^
drivers/firmware/meson/meson_sm.c:331:17: error: 'ch' undeclared (first use in this function)
331 | ch = (uint8_t *)(id_buf + 4);
| ^~
drivers/firmware/meson/meson_sm.c:332:22: error: 'i' undeclared (first use in this function)
332 | for (i = 0; i < 12; i++)
| ^
vim +328 drivers/firmware/meson/meson_sm.c
281
282 static ssize_t chipid_show(struct device *dev, struct device_attribute *attr,
283 char *buf)
284 {
285 struct platform_device *pdev = to_platform_device(dev);
286 struct meson_sm_firmware *fw;
287 uint8_t *id_buf;
288 int ret;
289
290 fw = platform_get_drvdata(pdev);
291
292 id_buf = kmalloc(SM_CHIP_ID_LENGTH, GFP_KERNEL);
293 if (!id_buf)
294 return -ENOMEM;
295
296 ret = meson_sm_call_read(fw, id_buf, SM_CHIP_ID_LENGTH, SM_GET_CHIP_ID,
297 2, 0, 0, 0, 0);
298 if (ret < 0) {
299 kfree(id_buf);
300 return ret;
301 }
302
303 int version = *((unsigned int *)id_buf);
304
305 if (version == 2)
306 ret = scnprintf(buf, PAGE_SIZE, "%16phN\n", &id_buf[SM_CHIP_ID_OFFSET]);
307 else {
308 /**
309 * Legacy 12-byte chip ID read out, transform data
310 * to expected order format.
311 */
312 uint8_t *buff;
313
314 buff = kmalloc(SM_CHIP_ID_LENGTH, GFP_KERNEL);
315 if (!buff)
316 return -ENOMEM;
317 #ifdef CONFIG_MESON_GX_SOCINFO
318 uint8_t *ch;
319 int i;
320
321 ((uint32_t *)buff)[0] =
322 ((meson_gx_socinfo & 0xff000000) | // Family ID
323 ((meson_gx_socinfo << 8) & 0xff0000) | // Chip Revision
324 ((meson_gx_socinfo >> 8) & 0xff00)); // Package ID
325
326 ((uint32_t *)buff)[0] = htonl(((uint32_t *)buff)[0]);
327 #else
> 328 ((uint32)t *)buff)[0] = 0;
329 #endif
330 /* Transform into expected order for display */
331 ch = (uint8_t *)(id_buf + 4);
332 for (i = 0; i < 12; i++)
333 buff[i + 4] = ch[11 - i];
334 ret = scnprintf(buf, PAGE_SIZE, "%16phN\n", &buff);
335 kfree(buff);
336 }
337
338 kfree(id_buf);
339 return ret;
340 }
341
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On 02/11/2023 08:49, Viacheslav Bocharov wrote: > Use meson_gx_socinfo variable for chipid compatible call > from meson-gx-socinfo driver if available. > So we are back to something like ARMv7 platform/mach-code with drivers tightly coupled between subsystems. But it is not 2007 anymore and we have Devicetree for this. Use it instead. What's more, your commit msg does not explain at all why do you need to do it. This is some "show" callback, which does not exist in current code. Adding code in one patch and then changing it, looks like you add incomplete or buggy feature. Best regards, Krzysztof
В Чт, 02/11/2023 в 10:26 +0100, Krzysztof Kozlowski пишет: > On 02/11/2023 08:49, Viacheslav Bocharov wrote: > > Use meson_gx_socinfo variable for chipid compatible call > > from meson-gx-socinfo driver if available. > > > > So we are back to something like ARMv7 platform/mach-code with > drivers > tightly coupled between subsystems. But it is not 2007 anymore and we > have Devicetree for this. Use it instead. > > What's more, your commit msg does not explain at all why do you need > to > do it. This is some "show" callback, which does not exist in current > code. Adding code in one patch and then changing it, looks like you > add > incomplete or buggy feature. > > Best regards, > Krzysztof > Fair enough. This patch is related to an adjacent one where the socinfo data supplements the result of executing the chipid version 1 function. Indeed, with the introduction of chipid v.2, we now have a second option for obtaining soc info (the first being implemented via register reading). And I'm somewhat contemplative: whether to move the meson-gx- socinfo entirely to the secure monitor or to duplicate the code from there. As a driver, meson-gx-socinfo does not carry practical information apart from outputting status in the boot log, and it cannot be reused without modifications to the driver. -- Viacheslav Bocharov
© 2016 - 2025 Red Hat, Inc.