rtc: rtc-ds1374: fix 'no irq' case handling
PATCH 1/2 rtc: rtc-ds1374: fix 'no irq' case handling
On a PowerPC board with ds1374 RTC I'm getting this error while RTC tries to probe:
rtc-ds1374 0-0068: unable to request IRQ
This happens because I2C probing code (drivers/of/of_i2c.c) is specifying IRQ0 for 'no irq' case, which is correct.
The driver handles this incorrectly, though. This patch fixes it.
--- drivers/rtc/rtc-ds1374.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)
drivers/rtc/rtc-ds1374.c +5 -5
@@ -173,7 +173,7 @@ static int ds1374_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)int cr, sr;int ret =0 ; -if (client->irq <0 ) +if (client->irq <=0 )return -EINVAL ;mutex_lock (&ds1374->mutex);
@@ -212,7 +212,7 @@ static int ds1374_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)int cr;int ret =0 ; -if (client->irq <0 ) +if (client->irq <=0 )return -EINVAL ; ret =ds1374_read_time (dev, &now);
@@ -381,7 +381,7 @@ static int ds1374_probe(struct i2c_client *client,if (ret) goto out_free; -if (client->irq >=0 ) { +if (client->irq >0 ) { ret =request_irq (client->irq, ds1374_irq,0 ,"ds1374" , client);if (ret) {
@@ -401,7 +401,7 @@ static int ds1374_probe(struct i2c_client *client,return 0 ; out_irq: -if (client->irq >=0 ) +if (client->irq >0 )free_irq (client->irq, client); out_free:
@@ -414,7 +414,7 @@ static int __devexit ds1374_remove(struct i2c_client *client) {struct ds1374 *ds1374 =i2c_get_clientdata (client); -if (client->irq >=0 ) { +if (client->irq >0 ) {mutex_lock (&ds1374->mutex); ds1374->exiting =1 ;mutex_unlock (&ds1374->mutex);
Signature · 1 line
1.5.6.3
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Anton" == Anton Vorontsov <[email protected]> writes:
Anton> On a PowerPC board with ds1374 RTC I'm getting this error while Anton> RTC tries to probe:
Anton> rtc-ds1374 0-0068: unable to request IRQ
Anton> This happens because I2C probing code (drivers/of/of_i2c.c) is Anton> specifying IRQ0 for 'no irq' case, which is correct.
Anton> The driver handles this incorrectly, though. This patch fixes it.
Great, I was just about to send a similar patch. Another advantage of using 0 for 'no irq' is for I2C_BOARD_INFO(). With that you can simply not assign anything to .irq instead of having to set it to -1.
Signature · 1 line
Bye, Peter Korsgaard
Quoted 7 lines · Anton Vorontsov
On a PowerPC board with ds1374 RTC I'm getting this error while
RTC tries to probe:rtc-ds1374 0-0068: unable to request IRQ
This happens because I2C probing code (
drivers/of/of_i2c.c) is specifying IRQ0 for 'no irq' case, which is correct.
Shouldn't this be
Quoted 1 line
- if (
client->irq<=NO_IRQ)
instead of
Quoted 2 lines
- if (
client->irq< 0)
+ if (client->irq<= 0)
Since NO_IRQ can vary by platform (0 or -1)? Work is underway to get everyone onto NO_IRQ=0 but I don't know if it is finished yet. It is much cleaner to use the NO_IRQ define.
In of_i2c.c shouldn't there be an error check?
info.irq = irq_of_parse_and_map(node, 0);
if (info.irq < NO_IRQ) {report error; continue }
Quoted 66 lines
The driver handles this incorrectly, though. This patch fixes it.
Signed-off-by Anton Vorontsov <[email protected]>---
drivers/rtc/rtc-ds1374.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)diff --git
a/drivers/rtc/rtc-ds1374.cb/drivers/rtc/rtc-ds1374.c
index 640acd2..a150418 100644
---a/drivers/rtc/rtc-ds1374.c
+++b/drivers/rtc/rtc-ds1374.c
@@ -173,7 +173,7 @@ static intds1374_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)int cr, sr; int ret = 0;- if (
client->irq< 0)
+ if (client->irq<= 0)return -EINVAL; mutex_lock(&ds1374->mutex); @@ -212,7 +212,7 @@ static int ds1374_set_alarm(struct device *dev, struct rtc_wkalrm *alarm) int cr; int ret = 0;- if (
client->irq< 0)
+ if (client->irq<= 0)return -EINVAL; ret = ds1374_read_time(dev, &now); @@ -381,7 +381,7 @@ static int ds1374_probe(struct i2c_client *client, if (ret) goto out_free;- if (
client->irq>= 0) {+ if (client->irq > 0) { ret = request_irq(client->irq, ds1374_irq, 0, "ds1374", client); if (ret) { @@ -401,7 +401,7 @@ static int ds1374_probe(struct i2c_client *client, return 0;
out_irq:
- if (client->irq>= 0)
+ if (client->irq> 0)free_irq(client->irq, client);
out_free:
@@ -414,7 +414,7 @@ static int__devexitds1374_remove(struct i2c_client *client){ struct ds1374 *ds1374 = i2c_get_clientdata(client);- if (
client->irq>= 0) {+ if (client->irq > 0) { mutex_lock(&ds1374->mutex); ds1374->exiting = 1; mutex_unlock(&ds1374->mutex);--
1.5.6.3_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Signature · 2 lines
Jon Smirl [email protected]
Quoted 5 lines
Shouldn't this be
Quoted 1 line
- if (
client->irq<=NO_IRQ)instead of
NO_IRQ is obsolete. client->irq != is the test and IRQ numbers are unsigned.
Alan
Quoted 9 lines · Alan Cox
Quoted 5 lines
Shouldn't this be
Quoted 1 line
- if (
client->irq<=NO_IRQ)instead of
NO_IRQis obsolete.client->irq!= is the test and IRQ numbers are
unsigned.
I don't know the status of these platforms....
asm-blackfin/irq.h:#define NO_IRQ ((unsigned int)(-1))
asm-mn10300/irq.h:#define NO_IRQ INT_MAX
asm-parisc/irq.h:#define NO_IRQ (-1)
Signature · 2 lines
Jon Smirl [email protected]
Quoted 5 lines
I don't know the status of these platforms....
asm-blackfin/irq.h:#define
NO_IRQ((unsigned int)(-1))
asm-mn10300/irq.h:#defineNO_IRQINT_MAX
asm-parisc/irq.h:#defineNO_IRQ(-1)
In need of fixing, assuming they actually use NO_IRQ for anything - don't be mislead by the fact they may have old defines lying around.
Alan
Quoted 11 lines · Jon Smirl
Quoted 7 lines · Anton Vorontsov
On a PowerPC board with ds1374 RTC I'm getting this error while
RTC tries to probe:rtc-ds1374 0-0068: unable to request IRQ
This happens because I2C probing code (
drivers/of/of_i2c.c) is specifying IRQ0 for 'no irq' case, which is correct.Shouldn't this be
Quoted 8 lines
Quoted 1 line
- if (
client->irq<=NO_IRQ)instead of
Quoted 2 lines
- if (
client->irq< 0)
+ if (client->irq<= 0)Since
NO_IRQcan vary by platform (0 or -1)?
First of all, NO_IRQ is not defined for every architecture. You can't use it for truly cross-platform drivers.
Secondly, "<= 0" will work for both NO_IRQ == 0 and NO_IRQ == -1, since client->irq is signed type.
As for false positives, I don't believe that there is any platform that use IRQ0 for external interrupts.
[...]
Quoted 5 lines
In
of_i2c.cshouldn't there be an error check?info.irq = irq_of_parse_and_map(node, 0);if (info.irq <
NO_IRQ) {report error; continue }
irq_of_parse_and_map() returns unsigned type, plus it is defined only for PowerPC, and for PowerPC NO_IRQ is always 0.
Signature · 3 lines
Anton Vorontsov email: [email protected] irc://irc.freenode.net/bd2