LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] pata_of_platform: fix no irq handling
@ 2008-10-06 17:26 Anton Vorontsov
  2008-10-06 20:41 ` Matt Sealey
  2008-10-13  6:56 ` Tejun Heo
  0 siblings, 2 replies; 23+ messages in thread
From: Anton Vorontsov @ 2008-10-06 17:26 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Li Yang, Wang Jian, Steven A. Falco, linuxppc-dev, linux-ide,
	linux-kernel

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>
---

Resending again...

 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	[flat|nested] 23+ messages in thread

* Re: [PATCH] pata_of_platform: fix no irq handling
  2008-10-06 17:26 [PATCH] pata_of_platform: fix no irq handling Anton Vorontsov
@ 2008-10-06 20:41 ` Matt Sealey
  2008-10-06 21:32   ` Anton Vorontsov
  2008-10-13  6:56 ` Tejun Heo
  1 sibling, 1 reply; 23+ messages in thread
From: Matt Sealey @ 2008-10-06 20:41 UTC (permalink / raw)
  To: avorontsov
  Cc: Jeff Garzik, linuxppc-dev, linux-kernel, linux-ide, Li Yang, Wang Jian

There is a simple problem with the patch which is that an "IRQ 0" can and does
actually exist on a bunch of platforms, at least to the best of my knowledge.

Checking for -1 (which means for definite, no irq at all, because it is
totally unambiguous, as a -1 IRQ numbering is "impossible") is more correct.

The problem is the check against an unsigned value for interrupts (is there
any reason why you would need 4 billion interrupts possible instead of just
2 billion?) although I must say, the patch will work, and probably 99.9999999%
of people will never see a problem with it :)

-- 
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations

Anton Vorontsov wrote:
> 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>
> ---
> 
> Resending again...
> 
>  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;
>  

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] pata_of_platform: fix no irq handling
  2008-10-06 20:41 ` Matt Sealey
@ 2008-10-06 21:32   ` Anton Vorontsov
  2008-10-07  1:30     ` Tejun Heo
  0 siblings, 1 reply; 23+ messages in thread
From: Anton Vorontsov @ 2008-10-06 21:32 UTC (permalink / raw)
  To: Matt Sealey
  Cc: Jeff Garzik, linuxppc-dev, linux-kernel, linux-ide, Li Yang, Wang Jian

On Mon, Oct 06, 2008 at 03:41:19PM -0500, Matt Sealey wrote:
> There is a simple problem with the patch which is that an "IRQ 0" can and does
> actually exist on a bunch of platforms, at least to the best of my knowledge.
> 
> Checking for -1 (which means for definite, no irq at all, because it is
> totally unambiguous, as a -1 IRQ numbering is "impossible") is more correct.

This was discussed years ago.

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

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] pata_of_platform: fix no irq handling
  2008-10-06 21:32   ` Anton Vorontsov
@ 2008-10-07  1:30     ` Tejun Heo
  2008-10-07  9:18       ` Wang Jian
                         ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Tejun Heo @ 2008-10-07  1:30 UTC (permalink / raw)
  To: avorontsov
  Cc: Matt Sealey, Jeff Garzik, linuxppc-dev, linux-kernel, linux-ide,
	Li Yang, Wang Jian

Anton Vorontsov wrote:
> On Mon, Oct 06, 2008 at 03:41:19PM -0500, Matt Sealey wrote:
>> There is a simple problem with the patch which is that an "IRQ 0" can and does
>> actually exist on a bunch of platforms, at least to the best of my knowledge.
>>
>> Checking for -1 (which means for definite, no irq at all, because it is
>> totally unambiguous, as a -1 IRQ numbering is "impossible") is more correct.
> 
> This was discussed years ago.
> 
> http://lkml.org/lkml/2005/11/22/159
> http://lkml.org/lkml/2005/11/22/227
> 

Would this break any existing platforms?  If so, can those be fixed
together or does it become a much bigger problem that way?

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] pata_of_platform: fix no irq handling
  2008-10-07  1:30     ` Tejun Heo
@ 2008-10-07  9:18       ` Wang Jian
  2008-10-07  9:26       ` Anton Vorontsov
  2008-10-07  9:37       ` Alan Cox
  2 siblings, 0 replies; 23+ messages in thread
From: Wang Jian @ 2008-10-07  9:18 UTC (permalink / raw)
  To: Tejun Heo
  Cc: avorontsov, Matt Sealey, Jeff Garzik, linuxppc-dev, linux-kernel,
	linux-ide, Li Yang

Tejun Heo wrote:
> Anton Vorontsov wrote:
>> On Mon, Oct 06, 2008 at 03:41:19PM -0500, Matt Sealey wrote:
>>> There is a simple problem with the patch which is that an "IRQ 0" can and does
>>> actually exist on a bunch of platforms, at least to the best of my knowledge.
>>>
>>> Checking for -1 (which means for definite, no irq at all, because it is
>>> totally unambiguous, as a -1 IRQ numbering is "impossible") is more correct.
>> This was discussed years ago.
>>
>> http://lkml.org/lkml/2005/11/22/159
>> http://lkml.org/lkml/2005/11/22/227
>>
> 
> Would this break any existing platforms?  If so, can those be fixed
> together or does it become a much bigger problem that way?
> 

Pata_of_platform stacks upon pata_platform. This patch fixes problem
concerning definition of "no irq" without touch any other place.  So
far I can't see any new problem.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] pata_of_platform: fix no irq handling
  2008-10-07  1:30     ` Tejun Heo
  2008-10-07  9:18       ` Wang Jian
@ 2008-10-07  9:26       ` Anton Vorontsov
  2008-10-07 10:04         ` Benjamin Herrenschmidt
  2008-10-07  9:37       ` Alan Cox
  2 siblings, 1 reply; 23+ messages in thread
From: Anton Vorontsov @ 2008-10-07  9:26 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Matt Sealey, Jeff Garzik, linuxppc-dev, linux-kernel, linux-ide,
	Li Yang, Wang Jian

On Tue, Oct 07, 2008 at 10:30:59AM +0900, Tejun Heo wrote:
> Anton Vorontsov wrote:
> > On Mon, Oct 06, 2008 at 03:41:19PM -0500, Matt Sealey wrote:
> >> There is a simple problem with the patch which is that an "IRQ 0" can and does
> >> actually exist on a bunch of platforms, at least to the best of my knowledge.
> >>
> >> Checking for -1 (which means for definite, no irq at all, because it is
> >> totally unambiguous, as a -1 IRQ numbering is "impossible") is more correct.
> > 
> > This was discussed years ago.
> > 
> > http://lkml.org/lkml/2005/11/22/159
> > http://lkml.org/lkml/2005/11/22/227
> > 
> 
> Would this break any existing platforms?

Nope.

The driver is only available for PPC platforms.

As time goes by one can change `depend on PPC_OF' to just `depends on
OF', so that the driver will be also available for SPARC. And still
it will work, because SPARC also understands VIRQ0 as invalid VIRQ.


Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] pata_of_platform: fix no irq handling
  2008-10-07  1:30     ` Tejun Heo
  2008-10-07  9:18       ` Wang Jian
  2008-10-07  9:26       ` Anton Vorontsov
@ 2008-10-07  9:37       ` Alan Cox
  2008-10-08  8:40         ` David Woodhouse
  2 siblings, 1 reply; 23+ messages in thread
From: Alan Cox @ 2008-10-07  9:37 UTC (permalink / raw)
  To: Tejun Heo
  Cc: avorontsov, Matt Sealey, Jeff Garzik, linuxppc-dev, linux-kernel,
	linux-ide, Li Yang, Wang Jian

> > This was discussed years ago.
> > 
> > http://lkml.org/lkml/2005/11/22/159
> > http://lkml.org/lkml/2005/11/22/227
> > 
> 
> Would this break any existing platforms?  If so, can those be fixed
> together or does it become a much bigger problem that way?

Zero means no IRQ. Any platform with bits of code left over exposing IRQ
0 is already not supported by lots of driver code including libata.

As IRQs are unsigned using -1 is asking for trouble

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] pata_of_platform: fix no irq handling
  2008-10-07  9:26       ` Anton Vorontsov
@ 2008-10-07 10:04         ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 23+ messages in thread
From: Benjamin Herrenschmidt @ 2008-10-07 10:04 UTC (permalink / raw)
  To: avorontsov
  Cc: Tejun Heo, linuxppc-dev, linux-kernel, linux-ide, Li Yang,
	Jeff Garzik, Wang Jian

On Tue, 2008-10-07 at 13:26 +0400, Anton Vorontsov wrote:
> On Tue, Oct 07, 2008 at 10:30:59AM +0900, Tejun Heo wrote:
> > Anton Vorontsov wrote:
> > > On Mon, Oct 06, 2008 at 03:41:19PM -0500, Matt Sealey wrote:
> > >> There is a simple problem with the patch which is that an "IRQ 0" can and does
> > >> actually exist on a bunch of platforms, at least to the best of my knowledge.
> > >>
> > >> Checking for -1 (which means for definite, no irq at all, because it is
> > >> totally unambiguous, as a -1 IRQ numbering is "impossible") is more correct.
> > > 
> > > This was discussed years ago.
> > > 
> > > http://lkml.org/lkml/2005/11/22/159
> > > http://lkml.org/lkml/2005/11/22/227
> > > 
> > 
> > Would this break any existing platforms?
> 
> Nope.
> 
> The driver is only available for PPC platforms.
> 
> As time goes by one can change `depend on PPC_OF' to just `depends on
> OF', so that the driver will be also available for SPARC. And still
> it will work, because SPARC also understands VIRQ0 as invalid VIRQ.
> 

Yup, I agree. I'll pick it up in my next batch.

Cheers,
Ben.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] pata_of_platform: fix no irq handling
  2008-10-07  9:37       ` Alan Cox
@ 2008-10-08  8:40         ` David Woodhouse
  2008-10-08  9:00           ` Alan Cox
  0 siblings, 1 reply; 23+ messages in thread
From: David Woodhouse @ 2008-10-08  8:40 UTC (permalink / raw)
  To: Alan Cox
  Cc: Tejun Heo, avorontsov, Matt Sealey, Jeff Garzik, linuxppc-dev,
	linux-kernel, linux-ide, Li Yang, Wang Jian

On Tue, 2008-10-07 at 10:37 +0100, Alan Cox wrote:
> Zero means no IRQ. Any platform with bits of code left over exposing IRQ
> 0 is already not supported by lots of driver code including libata.

...and must implement some kind of interrupt remapping crap just to work
around this bogus design decision.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] pata_of_platform: fix no irq handling
  2008-10-08  8:40         ` David Woodhouse
@ 2008-10-08  9:00           ` Alan Cox
  2008-10-08  9:59             ` Geert Uytterhoeven
  0 siblings, 1 reply; 23+ messages in thread
From: Alan Cox @ 2008-10-08  9:00 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Tejun Heo, avorontsov, Matt Sealey, Jeff Garzik, linuxppc-dev,
	linux-kernel, linux-ide, Li Yang, Wang Jian

On Wed, 08 Oct 2008 09:40:54 +0100
David Woodhouse <dwmw2@infradead.org> wrote:

> On Tue, 2008-10-07 at 10:37 +0100, Alan Cox wrote:
> > Zero means no IRQ. Any platform with bits of code left over exposing IRQ
> > 0 is already not supported by lots of driver code including libata.
> 
> ...and must implement some kind of interrupt remapping crap just to work
> around this bogus design decision.

I'll leave you to argue with Linus about that, but since that was the
decision back in 2005 (for good C reasons) we can safely rely on it.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] pata_of_platform: fix no irq handling
  2008-10-08  9:00           ` Alan Cox
@ 2008-10-08  9:59             ` Geert Uytterhoeven
  2008-10-08 10:27               ` Alan Cox
  2008-10-10 17:55               ` Paul Mundt
  0 siblings, 2 replies; 23+ messages in thread
From: Geert Uytterhoeven @ 2008-10-08  9:59 UTC (permalink / raw)
  To: Alan Cox
  Cc: David Woodhouse, Li Yang, linux-ide, linux-kernel, linuxppc-dev,
	Tejun Heo, Jeff Garzik, Wang Jian

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1093 bytes --]

On Wed, 8 Oct 2008, Alan Cox wrote:
> On Wed, 08 Oct 2008 09:40:54 +0100
> David Woodhouse <dwmw2@infradead.org> wrote:
> 
> > On Tue, 2008-10-07 at 10:37 +0100, Alan Cox wrote:
> > > Zero means no IRQ. Any platform with bits of code left over exposing IRQ
> > > 0 is already not supported by lots of driver code including libata.
> > 
> > ...and must implement some kind of interrupt remapping crap just to work
> > around this bogus design decision.
> 
> I'll leave you to argue with Linus about that, but since that was the
> decision back in 2005 (for good C reasons) we can safely rely on it.

`git grep NO_IRQ include arch/*/include' is still quite enlightening...

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] pata_of_platform: fix no irq handling
  2008-10-08  9:59             ` Geert Uytterhoeven
@ 2008-10-08 10:27               ` Alan Cox
  2008-10-10 17:55               ` Paul Mundt
  1 sibling, 0 replies; 23+ messages in thread
From: Alan Cox @ 2008-10-08 10:27 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: David Woodhouse, Li Yang, linux-ide, linux-kernel, linuxppc-dev,
	Tejun Heo, Jeff Garzik, Wang Jian

> > I'll leave you to argue with Linus about that, but since that was the
> > decision back in 2005 (for good C reasons) we can safely rely on it.
> 
> `git grep NO_IRQ include arch/*/include' is still quite enlightening...

Good guide to platform code we should delete really

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] pata_of_platform: fix no irq handling
  2008-10-08  9:59             ` Geert Uytterhoeven
  2008-10-08 10:27               ` Alan Cox
@ 2008-10-10 17:55               ` Paul Mundt
  1 sibling, 0 replies; 23+ messages in thread
From: Paul Mundt @ 2008-10-10 17:55 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Alan Cox, David Woodhouse, Li Yang, linux-ide, linux-kernel,
	linuxppc-dev, Tejun Heo, Jeff Garzik, Wang Jian

On Wed, Oct 08, 2008 at 11:59:59AM +0200, Geert Uytterhoeven wrote:
> On Wed, 8 Oct 2008, Alan Cox wrote:
> > On Wed, 08 Oct 2008 09:40:54 +0100
> > David Woodhouse <dwmw2@infradead.org> wrote:
> > 
> > > On Tue, 2008-10-07 at 10:37 +0100, Alan Cox wrote:
> > > > Zero means no IRQ. Any platform with bits of code left over exposing IRQ
> > > > 0 is already not supported by lots of driver code including libata.
> > > 
> > > ...and must implement some kind of interrupt remapping crap just to work
> > > around this bogus design decision.
> > 
> > I'll leave you to argue with Linus about that, but since that was the
> > decision back in 2005 (for good C reasons) we can safely rely on it.
> 
> `git grep NO_IRQ include arch/*/include' is still quite enlightening...
> 
In the case of PCI where IRQ is unsigned int, that's certainly bogus. The
problem originates on platforms where a 1:1 mapping between vector and
IRQ exists, where 0 is a valid value. This then needs to be remapped in
to an IRQ cookie that has a non-0 value in order to be assigned to
dev->irq. Despite whether this is bogus or not, there's not much to be
done about it. Those of us with vectored IRQ platforms generally have
enough sources that the use of IRQ-0 doesn't matter, and it's not worth
the headache of setting up the remapping crap.

As an example, on SH, IRQ-0 is mapped to vector 0x200. It is '0' for
symbolic reasons only. It's just as easy to pad out irq_desc and treat it
as an off-by-1 to work around all of code that assumes NO_IRQ == 0. There
is enough code in the kernel today that makes the non-zero IRQ cookie
assumption that platforms that do otherwise are simply broken.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] pata_of_platform: fix no irq handling
  2008-10-06 17:26 [PATCH] pata_of_platform: fix no irq handling Anton Vorontsov
  2008-10-06 20:41 ` Matt Sealey
@ 2008-10-13  6:56 ` Tejun Heo
  2008-10-13 13:27   ` Jeff Garzik
  2008-10-13 23:27   ` [PATCH] pata_of_platform: fix no irq handling Benjamin Herrenschmidt
  1 sibling, 2 replies; 23+ messages in thread
From: Tejun Heo @ 2008-10-13  6:56 UTC (permalink / raw)
  To: avorontsov
  Cc: Jeff Garzik, Li Yang, Wang Jian, Steven A. Falco, linuxppc-dev,
	linux-ide, linux-kernel

Anton Vorontsov wrote:
> 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>

applied to #tj-upstream (will soon be sent upstream)

-- 
tejun

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] pata_of_platform: fix no irq handling
  2008-10-13  6:56 ` Tejun Heo
@ 2008-10-13 13:27   ` Jeff Garzik
  2008-10-13 13:53     ` Tejun Heo
                       ` (3 more replies)
  2008-10-13 23:27   ` [PATCH] pata_of_platform: fix no irq handling Benjamin Herrenschmidt
  1 sibling, 4 replies; 23+ messages in thread
From: Jeff Garzik @ 2008-10-13 13:27 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-ide, linux-kernel

Tejun Heo wrote:
> Anton Vorontsov wrote:
>> 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>
> 
> applied to #tj-upstream (will soon be sent upstream)

I have returned!  muhahahahahaha....



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] pata_of_platform: fix no irq handling
  2008-10-13 13:27   ` Jeff Garzik
@ 2008-10-13 13:53     ` Tejun Heo
  2008-10-13 14:02     ` I have returned! Alan Cox
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 23+ messages in thread
From: Tejun Heo @ 2008-10-13 13:53 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: linux-ide, linux-kernel

Jeff Garzik wrote:
> Tejun Heo wrote:
>> Anton Vorontsov wrote:
>>> 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>
>>
>> applied to #tj-upstream (will soon be sent upstream)
> 
> I have returned!  muhahahahahaha....

Ah.. great.  Hope you enjoyed the vacation.  Can you please pull from
#tj-upstream?

 git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata-dev.git tj-upstream
 http://git.kernel.org/?p=linux/kernel/git/tj/libata-dev.git;a=shortlog;h=tj-upstream

Nothing much.  Rafael's skip-spin-off patchset + Elias's small fix +
ATA_HORKAGE_ATAPI_MOD16_DMA + pata_of_platform irq fix.

Gwendal posted patches to enable FIS based switching when NCQ is
disabled on sata_mv but I think this should be put into separate
branch and stay in linux-next till Mark Lord comes back and reviews it
as I don't know much about the driver and don't know of anyone other
than Gwendal and Mark who do.

sata_nv detection problem is still not resolved.  I figured out
generic and ck804 but nf2/3 is still malfunctioning.  I asked for more
tests and am waiting for results.

Eh... I guess that's about it.  Enjoy your INBOX.  :-P

-- 
tejun

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: I have returned!
  2008-10-13 13:27   ` Jeff Garzik
  2008-10-13 13:53     ` Tejun Heo
@ 2008-10-13 14:02     ` Alan Cox
  2008-10-16  9:40       ` Jeff Garzik
  2008-10-13 14:04     ` [PATCH] fusion: remove dead mpt lan code Alan Cox
  2008-10-13 14:05     ` [PATCH] mptsas: remove pointless null check Alan Cox
  3 siblings, 1 reply; 23+ messages in thread
From: Alan Cox @ 2008-10-13 14:02 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Tejun Heo, linux-ide, linux-kernel

> I have returned!  muhahahahahaha....

Oh good..
--

ibmtr: PCMCIA IBMTR is ok on 64bit

From: Alan Cox <alan@redhat.com>

For whatever value of 'OK' can be applied to the use of token ring. Seems
the 32bit to 64bit cleanups missed re-enabling the pcmcia driver

Closes #7133 and also reviewed the code in question

Signed-off-by: Alan Cox <alan@redhat.com>
---

 drivers/net/pcmcia/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


diff --git a/drivers/net/pcmcia/Kconfig b/drivers/net/pcmcia/Kconfig
index e8f55d8..9b8f793 100644
--- a/drivers/net/pcmcia/Kconfig
+++ b/drivers/net/pcmcia/Kconfig
@@ -111,7 +111,7 @@ config ARCNET_COM20020_CS
 
 config PCMCIA_IBMTR
 	tristate "IBM PCMCIA tokenring adapter support"
-	depends on IBMTR!=y && TR && !64BIT
+	depends on IBMTR!=y && TR
 	help
 	  Say Y here if you intend to attach this type of Token Ring PCMCIA
 	  card to your computer. You then also need to say Y to "Token Ring

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH] fusion: remove dead mpt lan code
  2008-10-13 13:27   ` Jeff Garzik
  2008-10-13 13:53     ` Tejun Heo
  2008-10-13 14:02     ` I have returned! Alan Cox
@ 2008-10-13 14:04     ` Alan Cox
  2008-10-13 14:05     ` [PATCH] mptsas: remove pointless null check Alan Cox
  3 siblings, 0 replies; 23+ messages in thread
From: Alan Cox @ 2008-10-13 14:04 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: linux-kernel

mptlan: Kill unused NAA workaround code

From: Alan Cox <alan@redhat.com>

This triggers false bug reports as it does a bogus kmalloc with locks held
but is never really compiled

Closes #8329

Signed-off-by: Alan Cox <alan@redhat.com>
---

 drivers/message/fusion/mptlan.c |  108 ---------------------------------------
 1 files changed, 0 insertions(+), 108 deletions(-)


diff --git a/drivers/message/fusion/mptlan.c b/drivers/message/fusion/mptlan.c
index a1abf95..603ffd0 100644
--- a/drivers/message/fusion/mptlan.c
+++ b/drivers/message/fusion/mptlan.c
@@ -77,12 +77,6 @@ MODULE_VERSION(my_VERSION);
  *  Fusion MPT LAN private structures
  */
 
-struct NAA_Hosed {
-	u16 NAA;
-	u8 ieee[FC_ALEN];
-	struct NAA_Hosed *next;
-};
-
 struct BufferControl {
 	struct sk_buff	*skb;
 	dma_addr_t	dma;
@@ -159,11 +153,6 @@ static u8 LanCtx = MPT_MAX_PROTOCOL_DRIVERS;
 static u32 max_buckets_out = 127;
 static u32 tx_max_out_p = 127 - 16;
 
-#ifdef QLOGIC_NAA_WORKAROUND
-static struct NAA_Hosed *mpt_bad_naa = NULL;
-DEFINE_RWLOCK(bad_naa_lock);
-#endif
-
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
 /**
  *	lan_reply - Handle all data sent from the hardware.
@@ -780,30 +769,6 @@ mpt_lan_sdu_send (struct sk_buff *skb, struct net_device *dev)
 //			ctx, skb, skb->data));
 
 	mac = skb_mac_header(skb);
-#ifdef QLOGIC_NAA_WORKAROUND
-{
-	struct NAA_Hosed *nh;
-
-	/* Munge the NAA for Tx packets to QLogic boards, which don't follow
-	   RFC 2625. The longer I look at this, the more my opinion of Qlogic
-	   drops. */
-	read_lock_irq(&bad_naa_lock);
-	for (nh = mpt_bad_naa; nh != NULL; nh=nh->next) {
-		if ((nh->ieee[0] == mac[0]) &&
-		    (nh->ieee[1] == mac[1]) &&
-		    (nh->ieee[2] == mac[2]) &&
-		    (nh->ieee[3] == mac[3]) &&
-		    (nh->ieee[4] == mac[4]) &&
-		    (nh->ieee[5] == mac[5])) {
-			cur_naa = nh->NAA;
-			dlprintk ((KERN_INFO "mptlan/sdu_send: using NAA value "
-				  "= %04x.\n", cur_naa));
-			break;
-		}
-	}
-	read_unlock_irq(&bad_naa_lock);
-}
-#endif
 
 	pTrans->TransactionDetails[0] = cpu_to_le32((cur_naa         << 16) |
 						    (mac[0] <<  8) |
@@ -1572,79 +1537,6 @@ mpt_lan_type_trans(struct sk_buff *skb, struct net_device *dev)
 
 	fcllc = (struct fcllc *)skb->data;
 
-#ifdef QLOGIC_NAA_WORKAROUND
-{
-	u16 source_naa = fch->stype, found = 0;
-
-	/* Workaround for QLogic not following RFC 2625 in regards to the NAA
-	   value. */
-
-	if ((source_naa & 0xF000) == 0)
-		source_naa = swab16(source_naa);
-
-	if (fcllc->ethertype == htons(ETH_P_ARP))
-	    dlprintk ((KERN_INFO "mptlan/type_trans: got arp req/rep w/ naa of "
-		      "%04x.\n", source_naa));
-
-	if ((fcllc->ethertype == htons(ETH_P_ARP)) &&
-	   ((source_naa >> 12) !=  MPT_LAN_NAA_RFC2625)){
-		struct NAA_Hosed *nh, *prevnh;
-		int i;
-
-		dlprintk ((KERN_INFO "mptlan/type_trans: ARP Req/Rep from "
-			  "system with non-RFC 2625 NAA value (%04x).\n",
-			  source_naa));
-
-		write_lock_irq(&bad_naa_lock);
-		for (prevnh = nh = mpt_bad_naa; nh != NULL;
-		     prevnh=nh, nh=nh->next) {
-			if ((nh->ieee[0] == fch->saddr[0]) &&
-			    (nh->ieee[1] == fch->saddr[1]) &&
-			    (nh->ieee[2] == fch->saddr[2]) &&
-			    (nh->ieee[3] == fch->saddr[3]) &&
-			    (nh->ieee[4] == fch->saddr[4]) &&
-			    (nh->ieee[5] == fch->saddr[5])) {
-				found = 1;
-				dlprintk ((KERN_INFO "mptlan/type_trans: ARP Re"
-					 "q/Rep w/ bad NAA from system already"
-					 " in DB.\n"));
-				break;
-			}
-		}
-
-		if ((!found) && (nh == NULL)) {
-
-			nh = kmalloc(sizeof(struct NAA_Hosed), GFP_KERNEL);
-			dlprintk ((KERN_INFO "mptlan/type_trans: ARP Req/Rep w/"
-				 " bad NAA from system not yet in DB.\n"));
-
-			if (nh != NULL) {
-				nh->next = NULL;
-				if (!mpt_bad_naa)
-					mpt_bad_naa = nh;
-				if (prevnh)
-					prevnh->next = nh;
-
-				nh->NAA = source_naa; /* Set the S_NAA value. */
-				for (i = 0; i < FC_ALEN; i++)
-					nh->ieee[i] = fch->saddr[i];
-				dlprintk ((KERN_INFO "Got ARP from %02x:%02x:%02x:%02x:"
-					  "%02x:%02x with non-compliant S_NAA value.\n",
-					  fch->saddr[0], fch->saddr[1], fch->saddr[2],
-					  fch->saddr[3], fch->saddr[4],fch->saddr[5]));
-			} else {
-				printk (KERN_ERR "mptlan/type_trans: Unable to"
-					" kmalloc a NAA_Hosed struct.\n");
-			}
-		} else if (!found) {
-			printk (KERN_ERR "mptlan/type_trans: found not"
-				" set, but nh isn't null. Evil "
-				"funkiness abounds.\n");
-		}
-		write_unlock_irq(&bad_naa_lock);
-	}
-}
-#endif
 
 	/* Strip the SNAP header from ARP packets since we don't
 	 * pass them through to the 802.2/SNAP layers.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH] mptsas: remove pointless null check
  2008-10-13 13:27   ` Jeff Garzik
                       ` (2 preceding siblings ...)
  2008-10-13 14:04     ` [PATCH] fusion: remove dead mpt lan code Alan Cox
@ 2008-10-13 14:05     ` Alan Cox
  2008-10-13 14:22       ` James Bottomley
  3 siblings, 1 reply; 23+ messages in thread
From: Alan Cox @ 2008-10-13 14:05 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: linux-ide, linux-kernel

mptsas: remove unneeded check

From: Alan Cox <alan@redhat.com>

>From coverity checker. Closes #9675

Signed-off-by: Alan Cox <alan@redhat.com>
---

 drivers/message/fusion/mptsas.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)


diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index 12b7325..a9019f0 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -2279,9 +2279,8 @@ mptsas_delete_expander_phys(MPT_ADAPTER *ioc)
 	mutex_lock(&ioc->sas_topology_mutex);
 	list_for_each_entry_safe(port_info, n, &ioc->sas_topology, list) {
 
-		if (port_info->phy_info &&
-		    (!(port_info->phy_info[0].identify.device_info &
-		    MPI_SAS_DEVICE_INFO_SMP_TARGET)))
+		if (!(port_info->phy_info[0].identify.device_info &
+		    MPI_SAS_DEVICE_INFO_SMP_TARGET))
 			continue;
 
 		if (mptsas_sas_expander_pg0(ioc, &buffer,

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mptsas: remove pointless null check
  2008-10-13 14:05     ` [PATCH] mptsas: remove pointless null check Alan Cox
@ 2008-10-13 14:22       ` James Bottomley
  2008-10-13 14:37         ` Alan Cox
  0 siblings, 1 reply; 23+ messages in thread
From: James Bottomley @ 2008-10-13 14:22 UTC (permalink / raw)
  To: Alan Cox; +Cc: Jeff Garzik, linux-ide, linux-kernel, linux-scsi

On Mon, 2008-10-13 at 15:05 +0100, Alan Cox wrote:
> mptsas: remove unneeded check
> 
> From: Alan Cox <alan@redhat.com>
> 
> >From coverity checker. Closes #9675
> 
> Signed-off-by: Alan Cox <alan@redhat.com>
> ---
> 
>  drivers/message/fusion/mptsas.c |    5 ++---
>  1 files changed, 2 insertions(+), 3 deletions(-)

-EWRONGLIST

This is a SCSI patch (although I admit with fusion sitting in
drivers/message it's hard to tell without looking in the MAINTAINERS
file).

> 
> diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
> index 12b7325..a9019f0 100644
> --- a/drivers/message/fusion/mptsas.c
> +++ b/drivers/message/fusion/mptsas.c
> @@ -2279,9 +2279,8 @@ mptsas_delete_expander_phys(MPT_ADAPTER *ioc)
>  	mutex_lock(&ioc->sas_topology_mutex);
>  	list_for_each_entry_safe(port_info, n, &ioc->sas_topology, list) {
>  
> -		if (port_info->phy_info &&

If I remember rightly this check is necessary because phy_info can be
NULL in certain situations.  Your patch will trip this to oops.  What
your description needs to say is that we no longer need to check this
pointer for NULL because it was checked somewhere else in the stack ...
but I can't see where that is, where is it?

> -		    (!(port_info->phy_info[0].identify.device_info &
> -		    MPI_SAS_DEVICE_INFO_SMP_TARGET)))
> +		if (!(port_info->phy_info[0].identify.device_info &
> +		    MPI_SAS_DEVICE_INFO_SMP_TARGET))
>  			continue;
>  
>  		if (mptsas_sas_expander_pg0(ioc, &buffer,

James



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] mptsas: remove pointless null check
  2008-10-13 14:22       ` James Bottomley
@ 2008-10-13 14:37         ` Alan Cox
  0 siblings, 0 replies; 23+ messages in thread
From: Alan Cox @ 2008-10-13 14:37 UTC (permalink / raw)
  To: James Bottomley; +Cc: Jeff Garzik, linux-ide, linux-kernel, linux-scsi

> If I remember rightly this check is necessary because phy_info can be
> NULL in certain situations.  Your patch will trip this to oops.  What

In which case it will already have crashed

> your description needs to say is that we no longer need to check this
> pointer for NULL because it was checked somewhere else in the stack ...
> but I can't see where that is, where is it?

We don't check: but if it was NULL we then fall straight through and
dereference it unconditionally... so either its a wrong NULL check and
the old code oopses (which a trawl says it doesn't) or the check is bogus.

Alan

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] pata_of_platform: fix no irq handling
  2008-10-13  6:56 ` Tejun Heo
  2008-10-13 13:27   ` Jeff Garzik
@ 2008-10-13 23:27   ` Benjamin Herrenschmidt
  1 sibling, 0 replies; 23+ messages in thread
From: Benjamin Herrenschmidt @ 2008-10-13 23:27 UTC (permalink / raw)
  To: Tejun Heo
  Cc: avorontsov, linuxppc-dev, linux-kernel, linux-ide, Li Yang,
	Jeff Garzik, Wang Jian

On Mon, 2008-10-13 at 15:56 +0900, Tejun Heo wrote:
> Anton Vorontsov wrote:
> > 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>
> 
> applied to #tj-upstream (will soon be sent upstream)

Hrm... I applied it to powerpc.git too :-) Hopefully the merge will sort
it out !

Cheers,
Ben.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: I have returned!
  2008-10-13 14:02     ` I have returned! Alan Cox
@ 2008-10-16  9:40       ` Jeff Garzik
  0 siblings, 0 replies; 23+ messages in thread
From: Jeff Garzik @ 2008-10-16  9:40 UTC (permalink / raw)
  To: Alan Cox; +Cc: Tejun Heo, linux-ide, linux-kernel

Alan Cox wrote:
>> I have returned!  muhahahahahaha....
> 
> Oh good..
> --
> 
> ibmtr: PCMCIA IBMTR is ok on 64bit
> 
> From: Alan Cox <alan@redhat.com>
> 
> For whatever value of 'OK' can be applied to the use of token ring. Seems
> the 32bit to 64bit cleanups missed re-enabling the pcmcia driver
> 
> Closes #7133 and also reviewed the code in question
> 
> Signed-off-by: Alan Cox <alan@redhat.com>
> ---
> 
>  drivers/net/pcmcia/Kconfig |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

applied



^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2008-10-16  9:40 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-10-06 17:26 [PATCH] pata_of_platform: fix no irq handling Anton Vorontsov
2008-10-06 20:41 ` Matt Sealey
2008-10-06 21:32   ` Anton Vorontsov
2008-10-07  1:30     ` Tejun Heo
2008-10-07  9:18       ` Wang Jian
2008-10-07  9:26       ` Anton Vorontsov
2008-10-07 10:04         ` Benjamin Herrenschmidt
2008-10-07  9:37       ` Alan Cox
2008-10-08  8:40         ` David Woodhouse
2008-10-08  9:00           ` Alan Cox
2008-10-08  9:59             ` Geert Uytterhoeven
2008-10-08 10:27               ` Alan Cox
2008-10-10 17:55               ` Paul Mundt
2008-10-13  6:56 ` Tejun Heo
2008-10-13 13:27   ` Jeff Garzik
2008-10-13 13:53     ` Tejun Heo
2008-10-13 14:02     ` I have returned! Alan Cox
2008-10-16  9:40       ` Jeff Garzik
2008-10-13 14:04     ` [PATCH] fusion: remove dead mpt lan code Alan Cox
2008-10-13 14:05     ` [PATCH] mptsas: remove pointless null check Alan Cox
2008-10-13 14:22       ` James Bottomley
2008-10-13 14:37         ` Alan Cox
2008-10-13 23:27   ` [PATCH] pata_of_platform: fix no irq handling Benjamin Herrenschmidt

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).