On Fri, Jan 26, 2024 at 1:56 AM Tudor Ambarus <tudor.ambarus@xxxxxxxxxx> wrote: > > On 1/25/24 20:43, Sam Protsenko wrote: > > On Thu, Jan 25, 2024 at 8:50 AM Tudor Ambarus <tudor.ambarus@xxxxxxxxxx> wrote: > >> > >> s3c64xx_spi_transfer_one() makes sure that for PIO the xfer->len is > >> always smaller than the fifo size. Since we can't receive more that the > >> FIFO size, droop the loop handling, the code becomes less misleading. > > > > Drop (spelling)? > > oh yeah, thanks. > > > > > For the patch: how exactly it was tested to make sure there is no regression? > > no regression testing for the entire patch set, I have just a gs101 on > my hands. > > However, we shouldn't refrain ourselves on improving things when we > think they're straight forward and they worth it. In this particular This patch clearly brings a functional change. The way I see things, the risk of having a regression outweighs the benefits of this refactoring. I don't think it's even methodologically right to apply such changes without thoroughly testing it first. It might be ok for super-easy one-line cleanups, but that's not one of those. > case, for PIO, s3c64xx_spi_transfer_one() does: > xfer->len = fifo_len - 1; > then in s3c64xx_enable_datapath() we write xfer->len and then in > s3c64xx_wait_for_pio() we code did the following: > loops = xfer->len / FIFO_DEPTH(sdd); > loops is always zero, this is bogus and we shall remove it. > [snip]
