Hi Ilias, On Mon, Jul 03, 2023 at 12:59:58PM +0300, Ilias Apalodimas wrote: > > > [...] > > > > +int do_ffa_ping(struct cmd_tbl *cmdtp, int flag, int argc, char *const > > > > argv[]) > > > > +{ > > > > + struct ffa_send_direct_data msg = { > > > > + .data0 = 0xaaaaaaaa, > > > > + .data1 = 0xbbbbbbbb, > > > > + .data2 = 0xcccccccc, > > > > + .data3 = 0xdddddddd, > > > > + .data4 = 0xeeeeeeee, > > > > + }; > > > > + u16 part_id; > > > > + int ret; > > > > + struct udevice *dev; > > > > + > > > > + if (argc != 2) { > > > > + log_err("Missing argument\n"); > > > > + return CMD_RET_USAGE; > > > > + } > > > > + > > > > + errno = 0; > > > > + part_id = strtoul(argv[1], NULL, 16); > > > > + > > > > + if (errno) { > > > > > > Is errno used in strtoul? > > > > Yes, please refer to [1]. > > > > [1]: https://man7.org/linux/man-pages/man3/strtoul.3.html > > > that's what the libc version does. Can you check the u-boot version? >
Short answer: errno not used in strtoul() an I'm gonna remove the errno check. More details: strtoul() is defined as simple_strtoul() in strto.c errno variable is not set by simple_strtoul() or its callees. errno.h is included by strto.c to access the error codes. > > > ... > > > > +int do_ffa_devlist(struct cmd_tbl *cmdtp, int flag, int argc, char > > > > *const argv[]) > > > > +{ > > > > + struct udevice *dev; > > > > + int ret; > > > > + > > > > + ret = ffa_get_dev(&dev); > > > > + if (ret) > > > > + return CMD_RET_FAILURE; > > > > + > > > > + log_info("device name %s, dev %p, driver name %s, ops %p\n", > > > > + dev->name, > > > > + (void *)map_to_sysmem(dev), > > > > + dev->driver->name, > > > > + (void *)map_to_sysmem(dev->driver->ops)); > > > > > > Isn't it more useful to print the physical address map_to_sysmem() retuns? > > > > That's what map_to_sysmem() does, it returns a physical address and it's > > shown in the log. > > I dont have access to u-boot source right, but why do you need all > these void * casts then? Because map_to_sysmem() returns an 'phys_addr_t' (aka 'long long unsigned int') . However, %p expects 'void *'. Compilation warning: cmd/armffa.c:181:18: warning: format ‘%p’ expects argument of type ‘void *’, but argument 3 has type ‘phys_addr_t’ {aka ‘long long unsigned int’} [8; ;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat=-Wformat=8;;] 181 | log_info("device name %s, dev %p, driver name %s, ops %p\n", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 182 | dev->name, 183 | map_to_sysmem(dev), | ~~~~~~~~~~~~~~~~~~ | | | phys_addr_t {aka long long unsigned int} Cheers, Abdellatif