diff mbox series

[v4,2/7] iio: mb1232: relax return value check for IRQ get

Message ID 429804dac3b1ea55dd233d1e2fdf94240e2f2b93.1684220962.git.mazziesaccount@gmail.com
State New
Headers show
Series fix fwnode_irq_get[_byname()] returnvalue | expand

Commit Message

Matti Vaittinen May 16, 2023, 7:12 a.m. UTC
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(-)

Comments

Andy Shevchenko May 17, 2023, 4:47 p.m. UTC | #1
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) {
Matti Vaittinen May 19, 2023, 5 a.m. UTC | #2
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
Matti Vaittinen May 19, 2023, 5:26 a.m. UTC | #3
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 mbox series

Patch

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 {