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.
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.c b/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 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);
--
1.5.6.3
>>>>> "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.
Acked-by: Peter Korsgaard <[email protected]>
--
Bye, Peter Korsgaard
On 8/12/08, Anton Vorontsov <[email protected]> wrote:
> 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
> - if (client->irq <= NO_IRQ)
instead of
> - 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 }
>
> 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.c b/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 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);
>
> --
> 1.5.6.3
>
> _______________________________________________
> Linuxppc-dev mailing list
> [email protected]
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
--
Jon Smirl
[email protected]
On Tue, Aug 19, 2008 at 04:39:09PM -0400, Jon Smirl wrote:
> On 8/12/08, Anton Vorontsov <[email protected]> wrote:
> > 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
>
> > - if (client->irq <= NO_IRQ)
>
> instead of
>
> > - if (client->irq < 0)
> > + if (client->irq <= 0)
>
> Since NO_IRQ can 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.
[...]
> 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 }
irq_of_parse_and_map() returns unsigned type, plus it is defined
only for PowerPC, and for PowerPC NO_IRQ is always 0.
--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
On 8/19/08, Alan Cox <[email protected]> wrote:
> > Shouldn't this be
> >
> > > - if (client->irq <= NO_IRQ)
> >
> > instead of
>
>
> NO_IRQ is 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)
--
Jon Smirl
[email protected]
> Shouldn't this be
>
> > - if (client->irq <= NO_IRQ)
>
> instead of
NO_IRQ is obsolete. client->irq != is the test and IRQ numbers are
unsigned.
Alan
> 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)
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