On 8/27/20 7:32 AM, Luca Ceresoli wrote: > In preparation to add error checking for gpiod_get_value(), rework > the loop to avoid the duplication of these lines: > > if (gpiod_get_value(conf->done)) > return xilinx_spi_apply_cclk_cycles(conf); > > There is little advantage in this rework with current code. However > error checking will expand these two lines to five, making code > duplication more annoying. > > Signed-off-by: Luca Ceresoli <luca@xxxxxxxxxxxxxxxx> > > --- > > This patch is new in v2 > --- > drivers/fpga/xilinx-spi.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c > index 01f494172379..cfc933d70f52 100644 > --- a/drivers/fpga/xilinx-spi.c > +++ b/drivers/fpga/xilinx-spi.c > @@ -151,22 +151,19 @@ static int xilinx_spi_write_complete(struct fpga_manager *mgr, > struct fpga_image_info *info) > { > struct xilinx_spi_conf *conf = mgr->priv; > - unsigned long timeout; > + unsigned long timeout = jiffies + usecs_to_jiffies(info->config_complete_timeout_us); > int ret; > > - if (gpiod_get_value(conf->done)) > - return xilinx_spi_apply_cclk_cycles(conf); > - > - timeout = jiffies + usecs_to_jiffies(info->config_complete_timeout_us); > + while (true) { > + if (gpiod_get_value(conf->done)) > + return xilinx_spi_apply_cclk_cycles(conf); > > - while (time_before(jiffies, timeout)) { > + if (time_after(jiffies, timeout)) > + break; > > ret = xilinx_spi_apply_cclk_cycles(conf); > if (ret) > return ret; > - > - if (gpiod_get_value(conf->done)) > - return xilinx_spi_apply_cclk_cycles(conf); > } Do you need another if (gpiod_get_value(conf->done)) return xilinx_spi_apply_cclk_cycles(conf); here to cover the chance of sleeping in the loop ? Tom > > dev_err(&mgr->dev, "Timeout after config data transfer\n");