-stable review patch. If anyone has any objections, please let us know.
---------------------
From: Alan Cox <[email protected]>
If you have a controller with one channel disabled and unmapped the new
iomap code blindly tries to iomap unconfigured BARs. Later on the code
does the right thing and checks for unmapped bars but it is done in the
wrong order
Reorder the checks and make the iomap conditional
Tejun: I think the code below is now correct but would appreciate you
giving it a review.
Signed-off-by: Alan Cox <[email protected]>
Acked-by: Tejun Heo <[email protected]>
Signed-off-by: Chris Wright <[email protected]>
---
[chrisw: Why is this not upstream yet?]
drivers/ata/libata-sff.c | 35 +++++++++++++++++++++++------------
1 file changed, 23 insertions(+), 12 deletions(-)
--- linux-2.6.21.1.orig/drivers/ata/libata-sff.c
+++ linux-2.6.21.1/drivers/ata/libata-sff.c
@@ -557,12 +557,30 @@ ata_pci_init_native_mode(struct pci_dev
int i, p = 0;
void __iomem * const *iomap;
+ /* Discard disabled ports. Some controllers show their
+ unused channels this way */
+ if (ata_resources_present(pdev, 0) == 0)
+ ports &= ~ATA_PORT_PRIMARY;
+ if (ata_resources_present(pdev, 1) == 0)
+ ports &= ~ATA_PORT_SECONDARY;
+
/* iomap BARs */
- for (i = 0; i < 4; i++) {
- if (pcim_iomap(pdev, i, 0) == NULL) {
- dev_printk(KERN_ERR, &pdev->dev,
- "failed to iomap PCI BAR %d\n", i);
- return NULL;
+ if (ports & ATA_PORT_PRIMARY) {
+ for (i = 0; i <= 1; i++) {
+ if (pcim_iomap(pdev, i, 0) == NULL) {
+ dev_printk(KERN_ERR, &pdev->dev,
+ "failed to iomap PCI BAR %d\n", i);
+ return NULL;
+ }
+ }
+ }
+ if (ports & ATA_PORT_SECONDARY) {
+ for (i = 2; i <= 3; i++) {
+ if (pcim_iomap(pdev, i, 0) == NULL) {
+ dev_printk(KERN_ERR, &pdev->dev,
+ "failed to iomap PCI BAR %d\n", i);
+ return NULL;
+ }
}
}
@@ -577,13 +595,6 @@ ata_pci_init_native_mode(struct pci_dev
probe_ent->irq = pdev->irq;
probe_ent->irq_flags = IRQF_SHARED;
- /* Discard disabled ports. Some controllers show their
- unused channels this way */
- if (ata_resources_present(pdev, 0) == 0)
- ports &= ~ATA_PORT_PRIMARY;
- if (ata_resources_present(pdev, 1) == 0)
- ports &= ~ATA_PORT_SECONDARY;
-
if (ports & ATA_PORT_PRIMARY) {
probe_ent->port[p].cmd_addr = iomap[0];
probe_ent->port[p].altstatus_addr =
--
On Mon, 21 May 2007, Chris Wright wrote:
> ---
> [chrisw: Why is this not upstream yet?]
And equally importantly, why is it even in the stable queue if it's not
upstream.
It's against stable rules, and it means that we may have stuff that gets
fixed in -stable and not in -upstream, if people don't notice. THAT IS
BAD!
Having fixes in stable that don't exist in upstream make -stable be
worse than pointless - it means that we actively _lose_ bug-reports. It
may sound like a good idea in the short run, but it's a *horrible* thing
for the long run.
IOW, this should not be on the table for -stable, and people involved
should have made sure it's in -upstream first!
And no, I do not think it's languishing in my mailqueue. I'm pretty sure
it's languishing somewhere else.
Linus
Linus Torvalds wrote:
>
> On Mon, 21 May 2007, Chris Wright wrote:
>> ---
>> [chrisw: Why is this not upstream yet?]
>
> And equally importantly, why is it even in the stable queue if it's not
> upstream.
>
> It's against stable rules, and it means that we may have stuff that gets
> fixed in -stable and not in -upstream, if people don't notice. THAT IS
> BAD!
>
> Having fixes in stable that don't exist in upstream make -stable be
> worse than pointless - it means that we actively _lose_ bug-reports. It
> may sound like a good idea in the short run, but it's a *horrible* thing
> for the long run.
>
> IOW, this should not be on the table for -stable, and people involved
> should have made sure it's in -upstream first!
Strongly agreed.
> And no, I do not think it's languishing in my mailqueue. I'm pretty sure
> it's languishing somewhere else.
AFAIK it was obsoleted by another Tejun patch which -is- upstream.
Am I missing something?
Jeff
* Linus Torvalds ([email protected]) wrote:
> On Mon, 21 May 2007, Chris Wright wrote:
> > ---
> > [chrisw: Why is this not upstream yet?]
>
> And equally importantly, why is it even in the stable queue if it's not
> upstream.
>
> It's against stable rules, and it means that we may have stuff that gets
> fixed in -stable and not in -upstream, if people don't notice. THAT IS
> BAD!
It's there specifically to fish out why it was sent to -stable w/out
ever making it upstream. Having sent the same question w/ no response
5 days ago
On Mon, 21 May 2007, Jeff Garzik wrote:
> > And no, I do not think it's languishing in my mailqueue. I'm pretty sure
> > it's languishing somewhere else.
>
> AFAIK it was obsoleted by another Tejun patch which -is- upstream.
>
> Am I missing something?
Ahh, in that case it should hopefully be noted as such. I just got worried
when I saw Chris' note.
Linus
On Mon, 21 May 2007 16:18:25 -0700 (PDT)
Linus Torvalds <[email protected]> wrote:
>
>
> On Mon, 21 May 2007, Chris Wright wrote:
> > ---
> > [chrisw: Why is this not upstream yet?]
>
> And equally importantly, why is it even in the stable queue if it's not
> upstream.
Its not relevant to upstream - upstream has different updates which
removed the bug but not in a clean "backport this" way.
> It's against stable rules, and it means that we may have stuff that gets
> fixed in -stable and not in -upstream, if people don't notice. THAT IS
> BAD
Then the rules are stupid in this specific case.
Alan
> It's there specifically to fish out why it was sent to -stable w/out
> ever making it upstream. Having sent the same question w/ no response
> 5 days ago
Yeah - fix your mailer, you got a reply 5 days ago.
Alan
* Alan Cox ([email protected]) wrote:
> Yeah - fix your mailer, you got a reply 5 days ago.
Sure wouldn't be the first time something broke. I'll take a look.
thanks,
-chris
* Chris Wright ([email protected]) wrote:
> * Alan Cox ([email protected]) wrote:
> > Yeah - fix your mailer, you got a reply 5 days ago.
>
> Sure wouldn't be the first time something broke. I'll take a look.
Thanks for the prod. I found 2 quite stale RBL entries, causing
long connection delay (enough from some MTAs to walk away perhaps).
thanks,
-chris