On Sat, 30 Jan 2021 10:25:16 -0500 Willem de Bruijn wrote: > > @@ -894,12 +894,16 @@ int gsi_channel_start(struct gsi *gsi, u32 channel_id) > > struct gsi_channel *channel = &gsi->channel[channel_id]; > > int ret; > > > > - /* Enable the completion interrupt */ > > + /* Enable NAPI and the completion interrupt */ > > + napi_enable(&channel->napi); > > gsi_irq_ieob_enable_one(gsi, channel->evt_ring_id); > > > > ret = __gsi_channel_start(channel, true); > > - if (ret) > > - gsi_irq_ieob_disable_one(gsi, channel->evt_ring_id); > > + if (!ret) > > + return 0; > > + > > + gsi_irq_ieob_disable_one(gsi, channel->evt_ring_id); > > + napi_disable(&channel->napi); > > > > return ret; > > } > > subjective, but easier to parse when the normal control flow is linear > and the error path takes a branch (or goto, if reused).
FWIW - fully agreed, I always thought this was part of the kernel coding style.
