Hi Abdellatif,
On Mon, 3 Jul 2023 at 13:09, Abdellatif El Khlifi
<abdellatif.elkhl...@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].
> > >
> > > [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}
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
