Re: [PATCH] staging: pi433: add rf69_dbg_hex function

[Joe Perches <joe@perches.com>]

On Sat, 2022-02-12 at 08:39 +1300, Paulo Miguel Almeida wrote:
> On Fri, Feb 11, 2022 at 09:14:40AM +0100, Greg KH wrote:
> > 
> > This is a lot of additional complexity for almost no real benefit.
> > 
> 
> you're right. I will no longer pursue this approach. 
> 
> > > -	/* print content read from fifo for debugging purposes */
> > > -	for (i = 0; i < size; i++)
> > > -		dev_dbg(&spi->dev, "%d - 0x%x\n", i, local_buffer[i + 1]);
> > 
> > What is wrong with this simple line?
> > 
> 
> to be honest, I think that 1 register per line isn't the easiest way to
> read them. Given that print_hex_dump_debug existed and had this
> horizontal-style priting format, I thought that it would be a better
> way of visualizing the fifo data.
> 
> the only problems with print_hex_dump_debug was the absense of device
> name and string format... so I saw a couple of drivers implementing
> alternative hex_dump-like functions and thought that pi433 would benefit
> from similar approach.
> 
> > > -	/* print content written from fifo for debugging purposes */
> > > -	for (i = 0; i < size; i++)
> > > -		dev_dbg(&spi->dev, "0x%x\n", buffer[i]);
> 
> if we are keeping this format, I may need to add the register idx to
> dev_dbg: 
> 		dev_dbg(&spi->dev, "%d - 0x%x\n", i, buffer[i]);

You could use %*ph with a buffer length up to 64 bytes.

Multiple times if necessary. 

Something like:

	for (i = 0; i < size; i += 64)
		dev_dbg(&spi->dev, "FIFO buffer: %d: %*ph\n",
			i, min_t(int, 64, size - i), buffer + i);

Or for a more realistic length like 16

	for (i = 0; i < size; i += 16)
		dev_dbg(&spi->dev, "FIFO buffer: %d: %*ph\n",
			i, min_t(int, 16, size - i), buffer + i);