| Message ID | 20231102074916.3280809-5-adeep@lexina.in (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RFC: firmware: meson-sm: add chipid sysfs export | expand |
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
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
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++)
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(-)