Hi Abdellatif,
On Mon, 3 Jul 2023 at 13:09, Abdellatif El Khlifi abdellatif.elkhlifi@arm.com wrote:
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].
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=-Wform...;;] 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}
You should be using %lx with map_to_sysmen() since it returns a . Use %p for pointers.
In general in U-Boot we use addresses (ulong) rather than pointers. Unfortunately the phys_addr_t type can be larger than the word size. I suggest creating a printf prefix for that type. See the top of blk.h for an example of how to do it. Perhaps in a follow-up patch?
Regards, Simon