On Mon, Feb 07, 2022 at 01:06:01PM +0300, Dan Carpenter wrote: > > #2 > > In the past, it's been pointed out to me during code review that I tend > > to add code comments which could be omitted. In this case, the for-loop > > seemed a bit odd without explaining why it's in there. Let me know if > > you think I should keep/remove it. > > Remove. Everyone knows what dev_dbg() does and the "read from fifo" > vs "written from[sic] fifo" is built into the function name. > fair enough > > int rf69_read_fifo(struct spi_device *spi, u8 *buffer, unsigned int size) > > { > > -#ifdef DEBUG_FIFO_ACCESS > > int i; > > -#endif > > struct spi_transfer transfer; > > u8 local_buffer[FIFO_SIZE + 1]; > > You did not introduce this but we are potentially printing out > uninitialized data if spi_sync_transfer() fails. Please initialize this > with: > > u8 local_buffer[FIFO_SIZE + 1] = {}; > > Do that in a separate patch, though. > good point, will do. Thanks a lot :) thanks, Paulo Almeida