Hi Jean-Marie,
On 5/15/25 4:01 PM, Verdun, Jean-Marie wrote:
This is dangerous too. How do I know if the returned data is an error
code or actual data?
Right now, xfer returns 0 if successful, -1 otherwise. I would suggest
return ret only if it is < 0.
Or even better, propagate the error code and return data through a
pointer as argument of the function?
e.g.
static int w5500_spi_read(struct udevice *dev, u32 addr, u8 *data)
{
[...]
return xfer(dev, cmd, sizeof(cmd), &data, 1);
}
I could do that but that won’t be the same logic as the linux kernel
driver approach which is mixing up ret and data as return value of
this call and many others too.
Fair enough. You could have the same ternary operator to better match
what's in the kernel.
Can you also reuse the macros that are in the kernel already?
W5500_SPI_BLOCK_SELECT, W5500_SPI_READ_CONTROL and
W5500_SPI_WRITE_CONTROL seems to be of interest?
Considering the only difference between w5500_spi_read16 and
w5500_spi_read is the 2 or 1 as last argument of xfer() call, you
probably want to factor them out into a common function to limit
duplicated code.
If you have access to the info, I would suggest to NOT use a u8[3] array
but create a small struct which specifies what each u8 is for so it's
easier to read and debug later on. This applies to all other functions.
I can do that
If you do that, please also update the kernel code to use that :)
Looks like endianness swap to me? Is this supposed to happen if you have
another endianness for the SoC than the one you're currently using to
test this?
I don’t have access to anything else other than an ARM SoC to test it. It seems
required to swap the bytes from the SPI read process
Yeah the kernel seems to be using __be16 type for that. And the write
part seems to be done by hand for the address and data.
Please use log_debug()/dev_dbg instead of debug().
Neil initially asked for debug, I can swap to any, just let me know which one.
debug() can use the log framework if enabled, but you still need to add
#define DEBUG wherever you want to build the logging messages. Whereas
with log_debug you would only need to raise the log level at compile
time to see the logs. Same, but different.
The benefit of using dev_dbg is when you're having multiple devices from
the same driver, then you can identify from which one the message is
being printed. You could have two Wiznet W5500 for example, how do you
know which one's printing which message?
Up to you,
Cheers,
Quentin
