| Message ID | 20200728091057.3.I2a1314232ace4323af96f9981c1e1a4f31f78049@changeid (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | sx9310 iio driver updates | expand |
On Tue, Jul 28, 2020 at 6:14 PM Daniel Campello <campello@chromium.org> wrote: > > Fixes enable/disable irq handling at various points. The driver needs to > only enable/disable irqs if there is an actual irq handler installed. > - enable_irq(data->client->irq); > + if (!ret) > + enable_irq(data->client->irq); > > return ret; > } Can it be a usual pattern? if (ret) return ret; ... return 0;
On Tue, Jul 28, 2020 at 12:08 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Tue, Jul 28, 2020 at 6:14 PM Daniel Campello <campello@chromium.org> wrote: > > > > Fixes enable/disable irq handling at various points. The driver needs to > > only enable/disable irqs if there is an actual irq handler installed. > > > - enable_irq(data->client->irq); > > + if (!ret) > > + enable_irq(data->client->irq); > > > > return ret; > > } > > Can it be a usual pattern? > > if (ret) > return ret; > ... > return 0; I think this way is more readable. The alternative would have to be something like this: .... if (ret) goto out; mutex_unlock(&data->mutex); enable_irq(data->client->irq); return 0; out: mutex_unlock(&data->mutex); return ret; > > -- > With Best Regards, > Andy Shevchenko Regards, Daniel Campello
Quoting Daniel Campello (2020-07-28 13:07:00) > On Tue, Jul 28, 2020 at 12:08 PM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > > > On Tue, Jul 28, 2020 at 6:14 PM Daniel Campello <campello@chromium.org> wrote: > > > > > > Fixes enable/disable irq handling at various points. The driver needs to > > > only enable/disable irqs if there is an actual irq handler installed. > > > > > - enable_irq(data->client->irq); > > > + if (!ret) > > > + enable_irq(data->client->irq); > > > > > > return ret; > > > } > > > > Can it be a usual pattern? > > > > if (ret) > > return ret; > > ... > > return 0; > > I think this way is more readable. The alternative would have to be > something like this: > > .... > if (ret) > goto out; > mutex_unlock(&data->mutex); > enable_irq(data->client->irq); > return 0; > > out: > mutex_unlock(&data->mutex); > return ret; > I think the suggestion is mutex_unlock(&data->mutex); if (ret) return ret; enable_irq(data->client->irq); return 0;
diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c index 07895d4b935d12..76b8bedebeef50 100644 --- a/drivers/iio/proximity/sx9310.c +++ b/drivers/iio/proximity/sx9310.c @@ -376,13 +376,15 @@ static int sx9310_read_proximity(struct sx9310_data *data, if (ret < 0) goto out; - ret = sx9310_enable_irq(data, SX9310_CONVDONE_IRQ); - if (ret < 0) - goto out_put_channel; + if (data->client->irq) { + ret = sx9310_enable_irq(data, SX9310_CONVDONE_IRQ); + if (ret) + goto out_put_channel; + } mutex_unlock(&data->mutex); - if (data->client->irq > 0) { + if (data->client->irq) { ret = wait_for_completion_interruptible(&data->completion); reinit_completion(&data->completion); } else { @@ -401,9 +403,11 @@ static int sx9310_read_proximity(struct sx9310_data *data, *val = sign_extend32(be16_to_cpu(rawval), (chan->address == SX9310_REG_DIFF_MSB ? 11 : 15)); - ret = sx9310_disable_irq(data, SX9310_CONVDONE_IRQ); - if (ret < 0) - goto out_put_channel; + if (data->client->irq) { + ret = sx9310_disable_irq(data, SX9310_CONVDONE_IRQ); + if (ret) + goto out_put_channel; + } ret = sx9310_put_read_channel(data, chan->channel); if (ret < 0) @@ -414,7 +418,8 @@ static int sx9310_read_proximity(struct sx9310_data *data, return IIO_VAL_INT; out_disable_irq: - sx9310_disable_irq(data, SX9310_CONVDONE_IRQ); + if (data->client->irq) + sx9310_disable_irq(data, SX9310_CONVDONE_IRQ); out_put_channel: sx9310_put_read_channel(data, chan->channel); out: @@ -1012,7 +1017,8 @@ static int __maybe_unused sx9310_resume(struct device *dev) out: mutex_unlock(&data->mutex); - enable_irq(data->client->irq); + if (!ret) + enable_irq(data->client->irq); return ret; }
Fixes enable/disable irq handling at various points. The driver needs to only enable/disable irqs if there is an actual irq handler installed. Signed-off-by: Daniel Campello <campello@chromium.org> --- drivers/iio/proximity/sx9310.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-)