* Re: [PATCH] pata_platform struct resource signness fix [not found] ` <48DB51C3.6040504@linux.net.cn> @ 2008-09-25 10:40 ` Li Yang 2008-09-25 10:48 ` Anton Vorontsov 2008-09-25 15:33 ` [PATCH] pata_platform struct resource signness fix Wang Jian 0 siblings, 2 replies; 6+ messages in thread From: Li Yang @ 2008-09-25 10:40 UTC (permalink / raw) To: Wang Jian; +Cc: linuxppc-dev, linux-ide, avorontsov On Thu, Sep 25, 2008 at 4:54 PM, Wang Jian <lark@linux.net.cn> wrote: > The alternative fix can be. This one is better as 0 is defined as 'invalid irq' for all architectures. Added linux-ide and Anton to cc. - Leo > > diff --git a/drivers/ata/pata_of_platform.c b/drivers/ata/pata_of_platform.c > index 408da30..1f18ad9 100644 > --- a/drivers/ata/pata_of_platform.c > +++ b/drivers/ata/pata_of_platform.c > @@ -52,7 +52,7 @@ static int __devinit pata_of_platform_probe(struct > of_device *ofdev, > > ret = of_irq_to_resource(dn, 0, &irq_res); > if (ret == NO_IRQ) > - irq_res.start = irq_res.end = -1; > + irq_res.start = irq_res.end = 0; > else > irq_res.flags = 0; > > I just didn't spend much time to see which is better. > > Wang Jian wrote: >> >> Hi, >> >> This patch is to pata_platform.c but at this time, it's powerpc specific >> because it can only be triggerred using openfirmware, so I post the patch >> here. The patch is against 2.6.26-rc8. >> >> The problem is triggerred when ata device is populated using >> pata_of_platform.c, and no irq is assigned (poll mode, such as CF card). >> >> pata_of_platform_probe() parse device tree and >> >> if (ret == NO_IRQ) >> irq_res.start = irq_res.end = -1; >> >> Then irq is 0xffffffff, not NULL. Probe will fail coz irq can't be >> requested. >> >> >> --- >> (irq_res->start > 0) will be true even when it is (-1). When the device >> has no irq, irq_res->start is assigned (-1). >> >> Signed-off-by: Wang Jian <lark@linux.net.cn> >> --- >> drivers/ata/pata_platform.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/drivers/ata/pata_platform.c b/drivers/ata/pata_platform.c >> index 8f65ad6..b12cd0c 100644 >> --- a/drivers/ata/pata_platform.c >> +++ b/drivers/ata/pata_platform.c >> @@ -123,7 +123,7 @@ int __devinit __pata_platform_probe(struct device >> *dev, >> /* >> * And the IRQ >> */ >> - if (irq_res && irq_res->start > 0) { >> + if (irq_res && irq_res->start != -1) { >> irq = irq_res->start; >> irq_flags = irq_res->flags; >> } > > _______________________________________________ > Linuxppc-dev mailing list > Linuxppc-dev@ozlabs.org > https://ozlabs.org/mailman/listinfo/linuxppc-dev > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pata_platform struct resource signness fix 2008-09-25 10:40 ` [PATCH] pata_platform struct resource signness fix Li Yang @ 2008-09-25 10:48 ` Anton Vorontsov 2008-09-29 4:19 ` Jeff Garzik 2008-09-25 15:33 ` [PATCH] pata_platform struct resource signness fix Wang Jian 1 sibling, 1 reply; 6+ messages in thread From: Anton Vorontsov @ 2008-09-25 10:48 UTC (permalink / raw) To: Li Yang, Jeff Garzik; +Cc: Wang Jian, linuxppc-dev, linux-ide On Thu, Sep 25, 2008 at 06:40:59PM +0800, Li Yang wrote: > On Thu, Sep 25, 2008 at 4:54 PM, Wang Jian <lark@linux.net.cn> wrote: > > The alternative fix can be. > > This one is better as 0 is defined as 'invalid irq' for all > architectures. Added linux-ide and Anton to cc. Thanks for the correct Cc. I've sent a patch to fix the issue more than a month ago. http://www.mail-archive.com/linuxppc-dev@ozlabs.org/msg22851.html Jeff, could you apply the patch? Thanks! -- Anton Vorontsov email: cbouatmailru@gmail.com irc://irc.freenode.net/bd2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pata_platform struct resource signness fix 2008-09-25 10:48 ` Anton Vorontsov @ 2008-09-29 4:19 ` Jeff Garzik 2008-09-29 13:32 ` [PATCH] pata_of_platform: fix no irq handling Anton Vorontsov 0 siblings, 1 reply; 6+ messages in thread From: Jeff Garzik @ 2008-09-29 4:19 UTC (permalink / raw) To: avorontsov; +Cc: Li Yang, Wang Jian, linuxppc-dev, linux-ide Anton Vorontsov wrote: > On Thu, Sep 25, 2008 at 06:40:59PM +0800, Li Yang wrote: >> On Thu, Sep 25, 2008 at 4:54 PM, Wang Jian <lark@linux.net.cn> wrote: >>> The alternative fix can be. >> This one is better as 0 is defined as 'invalid irq' for all >> architectures. Added linux-ide and Anton to cc. > > Thanks for the correct Cc. > > I've sent a patch to fix the issue more than a month ago. > > http://www.mail-archive.com/linuxppc-dev@ozlabs.org/msg22851.html > > Jeff, could you apply the patch? Can you resend, I don't seem to have it... ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] pata_of_platform: fix no irq handling 2008-09-29 4:19 ` Jeff Garzik @ 2008-09-29 13:32 ` Anton Vorontsov 0 siblings, 0 replies; 6+ messages in thread From: Anton Vorontsov @ 2008-09-29 13:32 UTC (permalink / raw) To: Jeff Garzik; +Cc: Li Yang, Wang Jian, Steven A. Falco, linuxppc-dev, linux-ide When no irq specified the pata_of_platform fills the irq_res with -1, which is wrong to do for two reasons: 1. By definition, 'no irq' should be IRQ 0, not some negative integer; 2. pata_platform checks for irq_res.start > 0, but since irq_res.start is unsigned type, the check will be true for `-1'. Reported-by: Steven A. Falco <sfalco@harris.com> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> --- On Mon, Sep 29, 2008 at 12:19:14AM -0400, Jeff Garzik wrote: > Anton Vorontsov wrote: >> On Thu, Sep 25, 2008 at 06:40:59PM +0800, Li Yang wrote: >>> On Thu, Sep 25, 2008 at 4:54 PM, Wang Jian <lark@linux.net.cn> wrote: >>>> The alternative fix can be. >>> This one is better as 0 is defined as 'invalid irq' for all >>> architectures. Added linux-ide and Anton to cc. >> >> Thanks for the correct Cc. >> >> I've sent a patch to fix the issue more than a month ago. >> >> http://www.mail-archive.com/linuxppc-dev@ozlabs.org/msg22851.html >> >> Jeff, could you apply the patch? > > Can you resend, I don't seem to have it... Here it is. Thanks, drivers/ata/pata_of_platform.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/ata/pata_of_platform.c b/drivers/ata/pata_of_platform.c index 408da30..1f18ad9 100644 --- a/drivers/ata/pata_of_platform.c +++ b/drivers/ata/pata_of_platform.c @@ -52,7 +52,7 @@ static int __devinit pata_of_platform_probe(struct of_device *ofdev, ret = of_irq_to_resource(dn, 0, &irq_res); if (ret == NO_IRQ) - irq_res.start = irq_res.end = -1; + irq_res.start = irq_res.end = 0; else irq_res.flags = 0; -- 1.5.6.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] pata_platform struct resource signness fix 2008-09-25 10:40 ` [PATCH] pata_platform struct resource signness fix Li Yang 2008-09-25 10:48 ` Anton Vorontsov @ 2008-09-25 15:33 ` Wang Jian 2008-09-25 15:47 ` Anton Vorontsov 1 sibling, 1 reply; 6+ messages in thread From: Wang Jian @ 2008-09-25 15:33 UTC (permalink / raw) To: Li Yang; +Cc: linuxppc-dev, linux-ide Li Yang wrote: > On Thu, Sep 25, 2008 at 4:54 PM, Wang Jian <lark@linux.net.cn> wrote: >> The alternative fix can be. > > This one is better as 0 is defined as 'invalid irq' for all > architectures. Added linux-ide and Anton to cc. However, this is not very true. Just git grep "#define NO_IRQ" and see. It seems that the NO_IRQ is in transition from (-1) to (0). I happened to code the same 2 patches as Anton Vorontsov <avorontsov@ru.mvista.com> had done without knowing his earlier work. I think he was also confused by (-1) and (0). > > - Leo > >> diff --git a/drivers/ata/pata_of_platform.c b/drivers/ata/pata_of_platform.c >> index 408da30..1f18ad9 100644 >> --- a/drivers/ata/pata_of_platform.c >> +++ b/drivers/ata/pata_of_platform.c >> @@ -52,7 +52,7 @@ static int __devinit pata_of_platform_probe(struct >> of_device *ofdev, >> >> ret = of_irq_to_resource(dn, 0, &irq_res); >> if (ret == NO_IRQ) >> - irq_res.start = irq_res.end = -1; >> + irq_res.start = irq_res.end = 0; >> else >> irq_res.flags = 0; >> >> I just didn't spend much time to see which is better. >> >> Wang Jian wrote: >>> Hi, >>> >>> This patch is to pata_platform.c but at this time, it's powerpc specific >>> because it can only be triggerred using openfirmware, so I post the patch >>> here. The patch is against 2.6.26-rc8. >>> >>> The problem is triggerred when ata device is populated using >>> pata_of_platform.c, and no irq is assigned (poll mode, such as CF card). >>> >>> pata_of_platform_probe() parse device tree and >>> >>> if (ret == NO_IRQ) >>> irq_res.start = irq_res.end = -1; >>> >>> Then irq is 0xffffffff, not NULL. Probe will fail coz irq can't be >>> requested. >>> >>> >>> --- >>> (irq_res->start > 0) will be true even when it is (-1). When the device >>> has no irq, irq_res->start is assigned (-1). >>> >>> Signed-off-by: Wang Jian <lark@linux.net.cn> >>> --- >>> drivers/ata/pata_platform.c | 2 +- >>> 1 files changed, 1 insertions(+), 1 deletions(-) >>> >>> diff --git a/drivers/ata/pata_platform.c b/drivers/ata/pata_platform.c >>> index 8f65ad6..b12cd0c 100644 >>> --- a/drivers/ata/pata_platform.c >>> +++ b/drivers/ata/pata_platform.c >>> @@ -123,7 +123,7 @@ int __devinit __pata_platform_probe(struct device >>> *dev, >>> /* >>> * And the IRQ >>> */ >>> - if (irq_res && irq_res->start > 0) { >>> + if (irq_res && irq_res->start != -1) { >>> irq = irq_res->start; >>> irq_flags = irq_res->flags; >>> } >> _______________________________________________ >> Linuxppc-dev mailing list >> Linuxppc-dev@ozlabs.org >> https://ozlabs.org/mailman/listinfo/linuxppc-dev >> > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pata_platform struct resource signness fix 2008-09-25 15:33 ` [PATCH] pata_platform struct resource signness fix Wang Jian @ 2008-09-25 15:47 ` Anton Vorontsov 0 siblings, 0 replies; 6+ messages in thread From: Anton Vorontsov @ 2008-09-25 15:47 UTC (permalink / raw) To: Wang Jian; +Cc: Li Yang, linuxppc-dev, linux-ide On Thu, Sep 25, 2008 at 11:33:35PM +0800, Wang Jian wrote: > Li Yang wrote: >> On Thu, Sep 25, 2008 at 4:54 PM, Wang Jian <lark@linux.net.cn> wrote: >>> The alternative fix can be. >> >> This one is better as 0 is defined as 'invalid irq' for all >> architectures. Added linux-ide and Anton to cc. > > However, this is not very true. Just git grep "#define NO_IRQ" and see. It > seems that the NO_IRQ is in transition from (-1) to (0). Yeah, there is a mess, but it gets better as time goes by. The platforms/drivers should be fixed, since 0 is the only invalid VIRQ. -- Anton Vorontsov email: cbouatmailru@gmail.com irc://irc.freenode.net/bd2 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-09-29 13:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20080925083622.GA2701@debian>
[not found] ` <48DB51C3.6040504@linux.net.cn>
2008-09-25 10:40 ` [PATCH] pata_platform struct resource signness fix Li Yang
2008-09-25 10:48 ` Anton Vorontsov
2008-09-29 4:19 ` Jeff Garzik
2008-09-29 13:32 ` [PATCH] pata_of_platform: fix no irq handling Anton Vorontsov
2008-09-25 15:33 ` [PATCH] pata_platform struct resource signness fix Wang Jian
2008-09-25 15:47 ` Anton Vorontsov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).