Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-03 Thread Steve Wise
On Tue, 2007-01-02 at 11:58 +, Alan wrote: ... > > I'm sending this now rather than after running full test suites so that > it can get the maximal testing in a short time. I'll be running tests on > this after lunch. > > Signed-off-by: Alan Cox <[EMAIL PROTECTED]> > > ---

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-03 Thread Steve Wise
On Tue, 2007-01-02 at 11:58 +, Alan wrote: ... I'm sending this now rather than after running full test suites so that it can get the maximal testing in a short time. I'll be running tests on this after lunch. Signed-off-by: Alan Cox [EMAIL PROTECTED] ---

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Jeff Garzik
Alan wrote: Once combined mode is fixed not to abuse resources (and it originally did it that way for a good reason I grant and am not criticising that) the entire management for legacy mode, mixed mode and native mode resources for an ATA device (including 0x170, 0x3F6 and other wacky magic)

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Alan
> Thus, if you avoid calling pci_request_regions (as your patch does), you > must manually provide the same guarantees that pci_request_regions > provides to its callers. pci_request_regions reserves only BAR4/BAR5 in legacy mode because of the fact the resources are mashed and eventually

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Alan
> 1) Programmatically reserve /all/ resources associated with > our PCI device > 2) Manually reserve resources associated with our PCI device, > but are not listed in struct pci_dev. Which it doesn't actually do. You reserve 0x1F0-0x1F7 but forget the other register.

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Jeff Garzik
Or maybe this rephrase helps: Regardless of how the IDE quirks have configured the PCI BARs, libata is written to assume that /all/ struct pci_dev resources for a single PCI device are reserved to the libata driver. Thus, if you avoid calling pci_request_regions (as your patch does), you

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Jeff Garzik
Alan wrote: 2.6.0 - 2.6.19: libata guarantees that all PCI BARs are reserved to the libata driver. Please read the code Jeff. The old IDE quirk code in the PCI layer blanked BAR 0 to BAR 3 of a compatibility mode controller (a) I'm well of aware of this, and (b) that changes nothing. I

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Alan
> 2.6.0 - 2.6.19: libata guarantees that all PCI BARs are reserved to the > libata driver. Please read the code Jeff. The old IDE quirk code in the PCI layer blanked BAR 0 to BAR 3 of a compatibility mode controller You then request_region 0x1f0 and 0x170 (BAR 0 and BAR 2) directly. You never

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Jeff Garzik
Alan wrote: Actually it didn't reserve BAR1 and BAR3 in legacy mode precisely because of the PCI resource mismanagement in the old tree. So you take your pick. I pick old bugs over new regressions. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Jeff Garzik
Alan wrote: We use BAR5 on two devices in legacy mode. Both of those reserve all the other resources. Translation: You want to hand-wave away an obvious regression that YOU have created with your fix-to-a-fix. It's not regressing anything. False: 2.6.0 - 2.6.19: libata guarantees that

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Alan
On Tue, 02 Jan 2007 16:32:01 -0500 Jeff Garzik <[EMAIL PROTECTED]> wrote: > Jeff Garzik wrote: > > * After your patch, the code explicitly calls pci_request_region() for > > BARs 0-4, but never for BAR5. > > Without checking for failures, I might add. The old code didn't reserve 1 or 3 at all

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Alan
> > We use BAR5 on two devices in legacy mode. Both of those reserve all the > > other resources. > > Translation: You want to hand-wave away an obvious regression that YOU > have created with your fix-to-a-fix. It's not regressing anything. > Why INTRODUCE these 2.6.20 Alan-isms, if they are

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Jeff Garzik
Jeff Garzik wrote: * After your patch, the code explicitly calls pci_request_region() for BARs 0-4, but never for BAR5. Without checking for failures, I might add. Let's call that regression/obvious bug #4, because the previous code actually CARED if the resource was reserved.

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Jeff Garzik
Alan wrote: This is a silly complaint because the SFF layer in libata doesn't handle this case yet anyway. Yes, it's "silly" people people use configurations you find inconvenient. At least one embedded x86 case cares, that I know of. They only needed to make two minor changes to make it

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Alan
> > This is a silly complaint because the SFF layer in libata doesn't handle > > this case yet anyway. > > Yes, it's "silly" people people use configurations you find inconvenient. > > At least one embedded x86 case cares, that I know of. They only needed > to make two minor changes to make it

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Jeff Garzik
Alan wrote: This is a slight variant on the patch I posted December 16th to fix libata combined mode handling. The only real change is that we now correctly also reserve BAR1,2,4. That is basically a neatness issue. Jeff was unhappy about two things 1. That it didn't work in the case of one

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Theodore Tso
On Tue, Jan 02, 2007 at 11:58:34AM +, Alan wrote: > This is a slight variant on the patch I posted December 16th to fix > libata combined mode handling. The only real change is that we now > correctly also reserve BAR1,2,4. That is basically a neatness issue. Thanks, I can confirm that with

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Alan
> Appears to work just fine here (compiles, boots and I'm > typing this email :). The build warnings below seem new > to me - but I guess they're harmless... They are harmless. For the 2.6.21 code base they will go away as well. - To unsubscribe from this list: send the line "unsubscribe

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Alessandro Suardi
On 1/2/07, Alan <[EMAIL PROTECTED]> wrote: This is a slight variant on the patch I posted December 16th to fix libata combined mode handling. The only real change is that we now correctly also reserve BAR1,2,4. That is basically a neatness issue. Jeff was unhappy about two things 1. That it

[PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Alan
This is a slight variant on the patch I posted December 16th to fix libata combined mode handling. The only real change is that we now correctly also reserve BAR1,2,4. That is basically a neatness issue. Jeff was unhappy about two things 1. That it didn't work in the case of one channel native

[PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Alan
This is a slight variant on the patch I posted December 16th to fix libata combined mode handling. The only real change is that we now correctly also reserve BAR1,2,4. That is basically a neatness issue. Jeff was unhappy about two things 1. That it didn't work in the case of one channel native

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Alessandro Suardi
On 1/2/07, Alan [EMAIL PROTECTED] wrote: This is a slight variant on the patch I posted December 16th to fix libata combined mode handling. The only real change is that we now correctly also reserve BAR1,2,4. That is basically a neatness issue. Jeff was unhappy about two things 1. That it

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Alan
Appears to work just fine here (compiles, boots and I'm typing this email :). The build warnings below seem new to me - but I guess they're harmless... They are harmless. For the 2.6.21 code base they will go away as well. - To unsubscribe from this list: send the line unsubscribe

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Theodore Tso
On Tue, Jan 02, 2007 at 11:58:34AM +, Alan wrote: This is a slight variant on the patch I posted December 16th to fix libata combined mode handling. The only real change is that we now correctly also reserve BAR1,2,4. That is basically a neatness issue. Thanks, I can confirm that with

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Jeff Garzik
Alan wrote: This is a slight variant on the patch I posted December 16th to fix libata combined mode handling. The only real change is that we now correctly also reserve BAR1,2,4. That is basically a neatness issue. Jeff was unhappy about two things 1. That it didn't work in the case of one

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Alan
This is a silly complaint because the SFF layer in libata doesn't handle this case yet anyway. Yes, it's silly people people use configurations you find inconvenient. At least one embedded x86 case cares, that I know of. They only needed to make two minor changes to make it work. *It

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Jeff Garzik
Alan wrote: This is a silly complaint because the SFF layer in libata doesn't handle this case yet anyway. Yes, it's silly people people use configurations you find inconvenient. At least one embedded x86 case cares, that I know of. They only needed to make two minor changes to make it work.

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Jeff Garzik
Jeff Garzik wrote: * After your patch, the code explicitly calls pci_request_region() for BARs 0-4, but never for BAR5. Without checking for failures, I might add. Let's call that regression/obvious bug #4, because the previous code actually CARED if the resource was reserved.

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Alan
We use BAR5 on two devices in legacy mode. Both of those reserve all the other resources. Translation: You want to hand-wave away an obvious regression that YOU have created with your fix-to-a-fix. It's not regressing anything. Why INTRODUCE these 2.6.20 Alan-isms, if they are going

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Alan
On Tue, 02 Jan 2007 16:32:01 -0500 Jeff Garzik [EMAIL PROTECTED] wrote: Jeff Garzik wrote: * After your patch, the code explicitly calls pci_request_region() for BARs 0-4, but never for BAR5. Without checking for failures, I might add. The old code didn't reserve 1 or 3 at all let alone

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Jeff Garzik
Alan wrote: We use BAR5 on two devices in legacy mode. Both of those reserve all the other resources. Translation: You want to hand-wave away an obvious regression that YOU have created with your fix-to-a-fix. It's not regressing anything. False: 2.6.0 - 2.6.19: libata guarantees that

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Jeff Garzik
Alan wrote: Actually it didn't reserve BAR1 and BAR3 in legacy mode precisely because of the PCI resource mismanagement in the old tree. So you take your pick. I pick old bugs over new regressions. Jeff - To unsubscribe from this list: send the line unsubscribe linux-kernel in the

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Alan
2.6.0 - 2.6.19: libata guarantees that all PCI BARs are reserved to the libata driver. Please read the code Jeff. The old IDE quirk code in the PCI layer blanked BAR 0 to BAR 3 of a compatibility mode controller You then request_region 0x1f0 and 0x170 (BAR 0 and BAR 2) directly. You never

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Jeff Garzik
Alan wrote: 2.6.0 - 2.6.19: libata guarantees that all PCI BARs are reserved to the libata driver. Please read the code Jeff. The old IDE quirk code in the PCI layer blanked BAR 0 to BAR 3 of a compatibility mode controller (a) I'm well of aware of this, and (b) that changes nothing. I

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Jeff Garzik
Or maybe this rephrase helps: Regardless of how the IDE quirks have configured the PCI BARs, libata is written to assume that /all/ struct pci_dev resources for a single PCI device are reserved to the libata driver. Thus, if you avoid calling pci_request_regions (as your patch does), you

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Alan
1) Programmatically reserve /all/ resources associated with our PCI device 2) Manually reserve resources associated with our PCI device, but are not listed in struct pci_dev. Which it doesn't actually do. You reserve 0x1F0-0x1F7 but forget the other register. BTW

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Alan
Thus, if you avoid calling pci_request_regions (as your patch does), you must manually provide the same guarantees that pci_request_regions provides to its callers. pci_request_regions reserves only BAR4/BAR5 in legacy mode because of the fact the resources are mashed and eventually cleaare

Re: [PATCH] libata: fix combined mode (was Re: Happy New Year (and v2.6.20-rc3 released))

2007-01-02 Thread Jeff Garzik
Alan wrote: Once combined mode is fixed not to abuse resources (and it originally did it that way for a good reason I grant and am not criticising that) the entire management for legacy mode, mixed mode and native mode resources for an ATA device (including 0x170, 0x3F6 and other wacky magic)