| Message ID | 429804dac3b1ea55dd233d1e2fdf94240e2f2b93.1684220962.git.mazziesaccount@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | fix fwnode_irq_get[_byname()] returnvalue | expand |
On Tue, May 16, 2023 at 10:12:41AM +0300, Matti Vaittinen wrote: > fwnode_irq_get() was changed to not return 0 anymore. > > Drop check for return value 0. ... > - if (data->irqnr <= 0) { > + if (data->irqnr < 0) { > /* usage of interrupt is optional */ > data->irqnr = -1; > } else { After this change I'm not sure we need this branch at all, I mean that -errn is equal to -1 in the code (but needs to be checked for silly checks like == -1). Hence Entire excerpt can be replaced with if (data->irqnr > 0) {
On 5/17/23 19:47, Andy Shevchenko wrote: > On Tue, May 16, 2023 at 10:12:41AM +0300, Matti Vaittinen wrote: >> fwnode_irq_get() was changed to not return 0 anymore. >> >> Drop check for return value 0. > > ... > >> - if (data->irqnr <= 0) { >> + if (data->irqnr < 0) { >> /* usage of interrupt is optional */ >> data->irqnr = -1; >> } else { > > > After this change I'm not sure we need this branch at all, I mean that -errn is > equal to -1 in the code (but needs to be checked for silly checks like == -1). > > Hence > > Entire excerpt can be replaced with > > if (data->irqnr > 0) { > I agree. Furthermore, at a quick glance it seems the whole irqnr could be dropped from the private data, and the private data struct could probably be static. I'd send them as separate clean-ups though as those changes are not really related to this return-value series. Yours, -- Matti
On 5/19/23 08:00, Matti Vaittinen wrote: > On 5/17/23 19:47, Andy Shevchenko wrote: >> On Tue, May 16, 2023 at 10:12:41AM +0300, Matti Vaittinen wrote: >>> fwnode_irq_get() was changed to not return 0 anymore. >>> >>> Drop check for return value 0. >> >> ... >> >>> - if (data->irqnr <= 0) { >>> + if (data->irqnr < 0) { >>> /* usage of interrupt is optional */ >>> data->irqnr = -1; >>> } else { >> >> >> After this change I'm not sure we need this branch at all, I mean that >> -errn is >> equal to -1 in the code (but needs to be checked for silly checks like >> == -1). >> >> Hence >> >> Entire excerpt can be replaced with >> >> if (data->irqnr > 0) { >> > > I agree. Furthermore, at a quick glance it seems the whole irqnr could > be dropped from the private data, and the private data struct could > probably be static. I'd send them as separate clean-ups though as those > changes are not really related to this return-value series. Please, ignore everything I wrote above, except that I agree to your suggestion. I was writing utter nonsense. Sorry for the noise. > > Yours, > -- Matti >
diff --git a/drivers/iio/proximity/mb1232.c b/drivers/iio/proximity/mb1232.c index e70cac8240af..2ab3e3fb2bae 100644 --- a/drivers/iio/proximity/mb1232.c +++ b/drivers/iio/proximity/mb1232.c @@ -76,7 +76,7 @@ static s16 mb1232_read_distance(struct mb1232_data *data) goto error_unlock; } - if (data->irqnr >= 0) { + if (data->irqnr > 0) { /* it cannot take more than 100 ms */ ret = wait_for_completion_killable_timeout(&data->ranging, HZ/10); @@ -212,7 +212,7 @@ static int mb1232_probe(struct i2c_client *client) init_completion(&data->ranging); data->irqnr = fwnode_irq_get(dev_fwnode(&client->dev), 0); - if (data->irqnr <= 0) { + if (data->irqnr < 0) { /* usage of interrupt is optional */ data->irqnr = -1; } else {
fwnode_irq_get() was changed to not return 0 anymore. Drop check for return value 0. Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> --- The first patch of the series changes the fwnode_irq_get() so this depends on the first patch of the series and should not be applied alone. --- drivers/iio/proximity/mb1232.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)