Don't miss the ARM-scsi fix.

Don't miss the ARM-scsi fix.

[Rob Landley]

[-- Attachment #1: Type: text/plain, Size: 454 bytes --]

Just making sure this fix winds up in 2.6.23:

http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=4454/1

It fixes a regression that occurred between 2.6.20 and 2.6.20-rc1.  Without 
it, qemu-system-arm can't use emulated SCSI drives.  It wasn't in -rc7, and 
the attached patch Works For Me (tm).

Signed-off-by: Rob Landley <rob@landley.net>

Rob
-- 
"One of my most productive days was throwing away 1000 lines of code."
  - Ken Thompson.

[-- Attachment #2: linux-armfix.patch --]
[-- Type: text/x-diff, Size: 1131 bytes --]

ARM Versatile PCI config reads of one byte width have the lowest two<br />
bits of the address cleared and result in reading from a wrong place<br />
in the config space.  This change is to use word size accesses like it is done for halfword reads.

Byte reads are used for retrieving the IRQ number of a PCI device and the problem was not exposed until 2.6.20 because the value read was discarded in drivers/pci/setup-irq.c (recently fixed).

Signed-off-by: Andrzej Zaborowski  &lt;(address hidden)&gt;<br />
Acked-by: Paul Brook  &lt;(address hidden)&gt;

Note 2 submitted by Russell King on 02 Jul 2007 12:39:41 (UTC)
Moved to applied
Applied to git-curr.

diff --git a/arch/arm/mach-versatile/pci.c b/arch/arm/mach-versatile/pci.c
index ba58223..ca82901 100644
--- a/arch/arm/mach-versatile/pci.c
+++ b/arch/arm/mach-versatile/pci.c
@@ -117,7 +117,10 @@ static int versatile_read_config(struct pci_bus *bus, unsigned int devfn, int wh
 	} else {
 		switch (size) {
 		case 1:
-			v = __raw_readb(addr);
+			v = __raw_readl(addr);
+			if (where & 2) v >>= 16;
+			if (where & 1) v >>= 8;
+ 			v &= 0xff;
 			break;
 
 		case 2:

Re: Don't miss the ARM-scsi fix.

[Andrew Morton]

On Tue, 3 Jul 2007 15:03:56 -0400
Rob Landley <rob@landley.net> wrote:

> Just making sure this fix winds up in 2.6.23:

2.6.22?

> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=4454/1
> 
> It fixes a regression that occurred between 2.6.20 and 2.6.20-rc1.  Without 
> it, qemu-system-arm can't use emulated SCSI drives.  It wasn't in -rc7, and 
> the attached patch Works For Me (tm).

Should have cc'ed Russell?
 
> Signed-off-by: Rob Landley <rob@landley.net>
> 
> Rob
> -- 
> "One of my most productive days was throwing away 1000 lines of code."
>   - Ken Thompson.
> 
> 
> [linux-armfix.patch  text/x-diff (1.1KB)]
> ARM Versatile PCI config reads of one byte width have the lowest two<br />
> bits of the address cleared and result in reading from a wrong place<br />

html gunk in changelog

> in the config space.  This change is to use word size accesses like it is done for halfword reads.
> 
> Byte reads are used for retrieving the IRQ number of a PCI device and the problem was not exposed until 2.6.20 because the value read was discarded in drivers/pci/setup-irq.c (recently fixed).
> 
> Signed-off-by: Andrzej Zaborowski  &lt;(address hidden)&gt;<br />
> Acked-by: Paul Brook  &lt;(address hidden)&gt;

more
 
> Note 2 submitted by Russell King on 02 Jul 2007 12:39:41 (UTC)
> Moved to applied
> Applied to git-curr.
> 
> diff --git a/arch/arm/mach-versatile/pci.c b/arch/arm/mach-versatile/pci.c
> index ba58223..ca82901 100644
> --- a/arch/arm/mach-versatile/pci.c
> +++ b/arch/arm/mach-versatile/pci.c
> @@ -117,7 +117,10 @@ static int versatile_read_config(struct pci_bus *bus, unsigned int devfn, int wh
>  	} else {
>  		switch (size) {
>  		case 1:
> -			v = __raw_readb(addr);
> +			v = __raw_readl(addr);
> +			if (where & 2) v >>= 16;
> +			if (where & 1) v >>= 8;

Someone's enter key broke?

> + 			v &= 0xff;
>  			break;
>  
>  		case 2:
> 

Re: Don't miss the ARM-scsi fix.

[Russell King - ARM Linux]

On Tue, Jul 03, 2007 at 12:21:45PM -0700, Andrew Morton wrote:
> On Tue, 3 Jul 2007 15:03:56 -0400
> Rob Landley <rob@landley.net> wrote:
> 
> > Just making sure this fix winds up in 2.6.23:
> 
> 2.6.22?
> 
> > http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=4454/1
> > 
> > It fixes a regression that occurred between 2.6.20 and 2.6.20-rc1.  Without 
> > it, qemu-system-arm can't use emulated SCSI drives.  It wasn't in -rc7, and 
> > the attached patch Works For Me (tm).
> 
> Should have cc'ed Russell?

It's been applied and is in my tree since yesterday.  I've no idea why
Rob feels that he needs to pull patches out of the patch system.

> > diff --git a/arch/arm/mach-versatile/pci.c b/arch/arm/mach-versatile/pci.c
> > index ba58223..ca82901 100644
> > --- a/arch/arm/mach-versatile/pci.c
> > +++ b/arch/arm/mach-versatile/pci.c
> > @@ -117,7 +117,10 @@ static int versatile_read_config(struct pci_bus *bus, unsigned int devfn, int wh
> >  	} else {
> >  		switch (size) {
> >  		case 1:
> > -			v = __raw_readb(addr);
> > +			v = __raw_readl(addr);
> > +			if (where & 2) v >>= 16;
> > +			if (where & 1) v >>= 8;
> 
> Someone's enter key broke?

Probably, but I'd rather have the fix in than worry about that at this
stage.

Re: Don't miss the ARM-scsi fix.

[Rob Landley]

On Tuesday 03 July 2007 15:31:04 Russell King - ARM Linux wrote:
> > > http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=4454/1
> > >
> > > It fixes a regression that occurred between 2.6.20 and 2.6.20-rc1. 
> > > Without it, qemu-system-arm can't use emulated SCSI drives.  It wasn't
> > > in -rc7, and the attached patch Works For Me (tm).
> >
> > Should have cc'ed Russell?
>
> It's been applied and is in my tree since yesterday.  I've no idea why
> Rob feels that he needs to pull patches out of the patch system.

Just trying to point at it and make sure that one doesn't miss 2.6.22 final, 
and let you all know that it fixes a real bug I've been seeing and 
complaining about for a while.

Reported in May:
  http://lkml.org/lkml/2007/5/10/670

Reported again in June:
  http://www.uwsg.iu.edu/hypermail/linux/kernel/0706.2/0429.html

Also poked at the qemu guys a few times.

If you've got it under control already, life is good...

Thanks,

Rob
-- 
"One of my most productive days was throwing away 1000 lines of code."
  - Ken Thompson.

Re: Don't miss the ARM-scsi fix.

[andrzej zaborowski]

On 03/07/07, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> On Tue, Jul 03, 2007 at 12:21:45PM -0700, Andrew Morton wrote:
> > On Tue, 3 Jul 2007 15:03:56 -0400
> > Rob Landley <rob@landley.net> wrote:
> > > diff --git a/arch/arm/mach-versatile/pci.c b/arch/arm/mach-versatile/pci.c
> > > index ba58223..ca82901 100644
> > > --- a/arch/arm/mach-versatile/pci.c
> > > +++ b/arch/arm/mach-versatile/pci.c
> > > @@ -117,7 +117,10 @@ static int versatile_read_config(struct pci_bus *bus, unsigned int devfn, int wh
> > >     } else {
> > >             switch (size) {
> > >             case 1:
> > > -                   v = __raw_readb(addr);
> > > +                   v = __raw_readl(addr);
> > > +                   if (where & 2) v >>= 16;
> > > +                   if (where & 1) v >>= 8;
> >
> > Someone's enter key broke?
>
> Probably, but I'd rather have the fix in than worry about that at this
> stage.

It's just the case of being consistent with the already broken style
in a function vs. correct style and inconsistency. I can never decide.

Regards,

Re: Don't miss the ARM-scsi fix.

[Russell King - ARM Linux]

On Wed, Jul 04, 2007 at 02:00:58PM +0200, andrzej zaborowski wrote:
> It's just the case of being consistent with the already broken style
> in a function vs. correct style and inconsistency. I can never decide.

That's always a hard one to call, and the answer is generally to
follow the existing style, even if it's broken.  At a later date,
a patch which _just_ fixes the style can be submitted.

The point of _just_ fixing the style is that it's trivial to prove
that no other changes have happened - you build the file without
the change and save a copy of the object.  Apply the change and
rebuild, and compare the resulting text/data.  They should be the
same, if not there's a bug somewhere.

(There have been instances where reformatting the code has introduced
subtle unexpected changes, so it's always worth performing that check
just in case.)

Re: Don't miss the ARM-scsi fix.

[Rob Landley]

On Tuesday 03 July 2007 15:21:45 Andrew Morton wrote:
> On Tue, 3 Jul 2007 15:03:56 -0400
>
> Rob Landley <rob@landley.net> wrote:
> > Just making sure this fix winds up in 2.6.23:
>
> 2.6.22?

Er, yes.  (After my six red bulls at the Blackthorn party, I've gone cold 
turkey on caffeine for a bit to detox.  I'm surprised I'm still upright.)

> > http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=4454/1
> >
> > It fixes a regression that occurred between 2.6.20 and 2.6.20-rc1. 
> > Without it, qemu-system-arm can't use emulated SCSI drives.  It wasn't in
> > -rc7, and the attached patch Works For Me (tm).
>
> Should have cc'ed Russell?

I cc'd the arm list, which he's on.  Low enough traffic I expected him to get 
it soonish, but I can cc: him directly if you like.

> > Signed-off-by: Rob Landley <rob@landley.net>
> >
> > Rob
> > --
> > "One of my most productive days was throwing away 1000 lines of code."
> >   - Ken Thompson.
> >
> >
> > [linux-armfix.patch  text/x-diff (1.1KB)]
> > ARM Versatile PCI config reads of one byte width have the lowest two<br
> > /> bits of the address cleared and result in reading from a wrong
> > place<br />
>
> html gunk in changelog

Sorry, that's from my quick grab of the web page above.

I expect this to come through the proper arm channels rather than mine, the 
attach was just to be specific about the patch I was talking about.

> > in the config space.  This change is to use word size accesses like it is
> > done for halfword reads.
> >
> > Byte reads are used for retrieving the IRQ number of a PCI device and the
> > problem was not exposed until 2.6.20 because the value read was discarded
> > in drivers/pci/setup-irq.c (recently fixed).
> >
> > Signed-off-by: Andrzej Zaborowski  &lt;(address hidden)&gt;<br />
> > Acked-by: Paul Brook  &lt;(address hidden)&gt;
>
> more

Sorry.

> > Note 2 submitted by Russell King on 02 Jul 2007 12:39:41 (UTC)
> > Moved to applied
> > Applied to git-curr.
> >
> > diff --git a/arch/arm/mach-versatile/pci.c
> > b/arch/arm/mach-versatile/pci.c index ba58223..ca82901 100644
> > --- a/arch/arm/mach-versatile/pci.c
> > +++ b/arch/arm/mach-versatile/pci.c
> > @@ -117,7 +117,10 @@ static int versatile_read_config(struct pci_bus
> > *bus, unsigned int devfn, int wh } else {
> >  		switch (size) {
> >  		case 1:
> > -			v = __raw_readb(addr);
> > +			v = __raw_readl(addr);
> > +			if (where & 2) v >>= 16;
> > +			if (where & 1) v >>= 8;
>
> Someone's enter key broke?

That's what the patch looked like when I got it. :)

I'm just an intrested party who's been trying to fix this particular problem 
on my own (without success) for a couple months now.  I hadn't been able to 
upgrade Firmware Linux past 2.6.20 because ARM doesn't work on the newer 
stuff, and it still doesn't work on 2.6.22-rc7 without this fix.

I found the fix yesterday, and would really really like it not to miss the 
actual release.  This was my attempt to make puppy eyes at subsystem 
maintainers and add a signed-off-by to a small fix that unblocks a project 
I'm working on. :)

Rob
-- 
"One of my most productive days was throwing away 1000 lines of code."
  - Ken Thompson.