Prev: use DECLARE_COMPLETION_ONSTACK for non-constant completion
Next: [PATCH 12/12] sched: Simplify set_task_cpu()
From: Anton Vorontsov on 16 Dec 2009 11:40
On Wed, Dec 16, 2009 at 05:10:08PM +0100, Uwe Kleine-König wrote:
> platform_get_irq returns -ENXIO on failure, so !irq was probably
> always true. Better use (int)irq <= 0. Note that a return value of
> zero is still handled as error even though this could mean irq0.
>
> This is a followup to 305b3228f9ff4d59f49e6d34a7034d44ee8ce2f0 that
> changed the return value of platform_get_irq from 0 to -ENXIO on error.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig(a)pengutronix.de>
> ---

Noooooo... :-(

Please revert 305b3228f9ff4d59f49e6d34a7034d44ee8ce2f0 instead,
and fix platforms to remap HWIRQ0 to something that is not VIRQ0.

IRQ0 is invalid for everything that is outside of arch/*.

http://lkml.org/lkml/2005/11/22/159
http://lkml.org/lkml/2005/11/22/213
http://lkml.org/lkml/2005/11/22/227

--
Anton Vorontsov
email: cbouatmailru(a)gmail.com
irc://irc.freenode.net/bd2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Anton Vorontsov on 16 Dec 2009 13:30
On Wed, Dec 16, 2009 at 06:49:04PM +0100, Uwe Kleine-König wrote:
[...]
> > Noooooo... :-(
> >
> > Please revert 305b3228f9ff4d59f49e6d34a7034d44ee8ce2f0 instead,
> > and fix platforms to remap HWIRQ0 to something that is not VIRQ0.
> >
> > IRQ0 is invalid for everything that is outside of arch/*.
> >
> > http://lkml.org/lkml/2005/11/22/159
> > http://lkml.org/lkml/2005/11/22/213
> > http://lkml.org/lkml/2005/11/22/227
> First note that my check is safe with both variants (e.g. it does the
> right thing independent of the error being signaled by 0 or
> -ESOMETHING.)
>
> Then arch/arm/mach-pxa/devices.c has:
>
> static struct resource pxa27x_resource_ssp3[] = {
> ...
> [1] = {
> .start = IRQ_SSP3,
> .end = IRQ_SSP3,
> .flags = IORESOURCE_IRQ,
> },
> ...
> }
>
> with IRQ_SSP3 being zero (sometimes). The driver is implemented in
> arch/arm/mach-pxa/ssp.c and uses platform_get_irq.

So fix this *one* driver? Implement arm-specific platform_get_irq() as
a band-aid. Or better, implement virtual irqs <-> hardware irqs mapping
for ARM.

[...]
> I'm a bit annoyed as this is the third time[1] this month this irq0
> discussion pops up for me. I think people see that irq0 is involved
> somehow, start wailing and stop seeing the issues being fixed.

For this particular driver, there is NO issue whatsoever. It is
only used for PowerPC, which has VIRQ0 == invalid IRQ. And note
that there still could be HWIRQ0 on PowerPC, but it is *never*
mapped to VIRQ0.

[...]
> [1] one is:
> http://thread.gmane.org/gmane.linux.kernel/924739

No wonder the discussion popped up. You're adding some ugly
#ifdef stuff that adds some arch-specific knowledge to a generic
code.

Sure, there's a lot of ugly code even in the kernel/ directly, but
you have to prepare for resistance when you add more of it.

So, if you want to fix the root cause of the issue: revert the
305b3228f9ff4d59f49e6d34a7034d44ee8ce2f0, and try to improve the
ARM land, do not band-aid the whole kernel all over the place.

--
Anton Vorontsov
email: cbouatmailru(a)gmail.com
irc://irc.freenode.net/bd2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: David Vrabel on 16 Dec 2009 13:30
Uwe Kleine-K�nig wrote:
> Hello,
>
> [Note the email address used for David Vrabel isn't valid any more,
> this mail uses his last used address.]

I've not been involved in PXA27x stuff for years. Please drop me from
the CC, thanks.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Anton Vorontsov on 16 Dec 2009 14:40
On Wed, Dec 16, 2009 at 08:18:39PM +0100, Uwe Kleine-König wrote:
[...]
> > > I'm a bit annoyed as this is the third time[1] this month this irq0
> > > discussion pops up for me. I think people see that irq0 is involved
> > > somehow, start wailing and stop seeing the issues being fixed.
> >
> > For this particular driver, there is NO issue whatsoever. It is
> > only used for PowerPC, which has VIRQ0 == invalid IRQ. And note
> > that there still could be HWIRQ0 on PowerPC, but it is *never*
> > mapped to VIRQ0.
> Yes, there is an issue. If the platform device doesn't have a resource
> specifing the irq, platform_get_irq returns -ENXIO. So in the driver
> (unsigned)(-ENXIO) is passed to mpc8xxx_spi_probe as (!(-ENXIO)) is
> false and so the error isn't catched.

If you look into how we create the platform device, you'll notice
that -ENXIO isn't possible.
It's in arch/powerpc/platforms/83xx/mpc832x_rdb.c:of_fsl_spi_probe(),
which is a legacy interface that I'd like to be removed anyway.

Though, if you want to fix the inconsistency in the platform device
API, then I'd suggest to fix the platform_get_irq(). The driver itself
is correct.

--
Anton Vorontsov
email: cbouatmailru(a)gmail.com
irc://irc.freenode.net/bd2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Anton Vorontsov on 16 Dec 2009 15:00
On Wed, Dec 16, 2009 at 10:37:45PM +0300, Anton Vorontsov wrote:
[...]
> which is a legacy interface that I'd like to be removed anyway.

....which means I really don't care that much to nack or ack the
patch. Do whatever you feel the best, but you must realize that
what you're doing is just papering over the real problem, and
maybe even spreading the problem further.

--
Anton Vorontsov
email: cbouatmailru(a)gmail.com
irc://irc.freenode.net/bd2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 |  Next  |  Last
Pages: 1 2
Prev: use DECLARE_COMPLETION_ONSTACK for non-constant completion
Next: [PATCH 12/12] sched: Simplify set_task_cpu()