LIBATA: Allow devices without IRQ specified to fall back
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/1 LIBATA: Allow devices without IRQ specified to fall back
The Simtec CATS board is an example of an ALI M5229 that is not correclty supported by libata as the pdev->irq field has no valid interrupt set. This is due to the legacy IRQ check in ata_pci_sff_activate_host() only checking to see if the device class is legacy, not that the IRQ has been setup.
Change ata_pci_sff_activate_host() to try the legacy IRQ numbers (which works correctly) when the dev->irq has not been set. The current check for !legacy_mode && dev->irq does not deal with devices that are not in legacy mode but do not have mapped interrupts.
drivers/ata/libata-sff.c +1 -1
@@ -2702,7 +2702,7 @@ int ata_pci_sff_activate_host(struct ataata_port_desc (host->ports[0 ],"irq %d" , pdev->irq);ata_port_desc (host->ports[1 ],"irq %d" , pdev->irq); - }else if (legacy_mode) { + }else if (legacy_mode || !pdev->irq) {if (!ata_port_is_dummy (host->ports[0 ])) { rc = devm_request_irq(dev, ATA_PRIMARY_IRQ(pdev), irq_handler, IRQF_SHARED,
Quoted 5 lines
is not correclty supported by libata as the
pdev->irqfield has no valid interrupt set. This is due to the legacy IRQ check inata_pci_sff_activate_host()only checking to see if the device class is legacy, not that the IRQ has been setup.
It does that check as that is the correct check to do. If the device is in native mode with no assigned IRQ we must poll, we cannot assume that legacy interrupts are in use.
Quoted 5 lines
Change
ata_pci_sff_activate_host()to try the legacy IRQ numbers (which works correctly) when thedev->irqhas not been set. The current check for !legacy_mode &&dev->irqdoes not deal with devices that are not in legacy mode but do not have mapped interrupts.
If they have no mapped interrupt and are not in legacy mode then they have no interrupt we can use and we should poll. Your assumption that if no IRQ is assigned we should use the legacy interrupts is incorrect for PC class systems.
Can you not correct this with a PCI quirk handler for your board instead ?
Alan
Quoted 16 lines · Alan Cox
Quoted 5 lines
is not correclty supported by libata as the
pdev->irqfield has no valid interrupt set. This is due to the legacy IRQ check inata_pci_sff_activate_host()only checking to see if the device class is legacy, not that the IRQ has been setup.It does that check as that is the correct check to do. If the device is in native mode with no assigned IRQ we must poll, we cannot assume that legacy interrupts are in use.
Quoted 5 lines
Change
ata_pci_sff_activate_host()to try the legacy IRQ numbers (which works correctly) when thedev->irqhas not been set. The current check for !legacy_mode &&dev->irqdoes not deal with devices that are not in legacy mode but do not have mapped interrupts.
Quoted 6 lines
If they have no mapped interrupt and are not in legacy mode then they have no interrupt we can use and we should poll. Your assumption that if no IRQ is assigned we should use the legacy interrupts is incorrect for PC class systems.
Can you not correct this with a PCI quirk handler for your board instead ?
Ok, however the drivers/ide/pci/alim15x3.c still works fine on this board, so there's something that something in the libata driver path not working properly. Possibly drivers/ata/pata_ali.c that is missing something
I will check this over tommorow and come up with a new solution.
Signature · 4 lines
Ben Q: What's a light-year? A: One-third less calories than a regular year.
Quoted 4 lines
Ok, however the
drivers/ide/pci/alim15x3.cstill works fine on this board, so there's something that something in the libata driver path not working properly. Possiblydrivers/ata/pata_ali.cthat is missing something
The old driver makes some rather iffy assumptions when dev->irq == 0 in part because the old IDE code predates PCI quirks.
Quoted 9 lines · Alan Cox
Quoted 4 lines
Ok, however the
drivers/ide/pci/alim15x3.cstill works fine on this board, so there's something that something in the libata driver path not working properly. Possiblydrivers/ata/pata_ali.cthat is missing somethingThe old driver makes some rather iffy assumptions when
dev->irq== 0 in part because the old IDE code predates PCI quirks.From reading the ALI M1543 datasheets that the interrupt pin in either
mode (compatibility or native) is routed directly into the inbuilt ISA-PIC, and thus the IRQ field in the PCI child device is not available. This means we need to be able to override the IRQ numbers for these devices.
I had a look, and there doesn't seem to be any way of specifying an IRQ for a host port when registering with libata-sff.c, which is a problem since we need to be able to either pass-in or have a callback to allow our "quirk" to specify a new IRQ for the device.
Does anyone know if the ALi M5229 actually exists outside of one of the M1543 bridges?
Note, this hardly seems to be a board-quirk, it is more of a device quirk...
Signature · 4 lines
Ben Q: What's a light-year? A: One-third less calories than a regular year.
Quoted 4 lines
I had a look, and there doesn't seem to be any way of specifying an IRQ for a host port when registering with
libata-sff.c, which is a problem since we need to be able to either pass-in or have a callback to allow our "quirk" to specify a new IRQ for the device.
Currently there is not. However if the device as a whole is wired to IRQ 14 you want to quirk pdev->irq.
Quoted 7 lines · Alan Cox
Quoted 4 lines
I had a look, and there doesn't seem to be any way of specifying an IRQ for a host port when registering with
libata-sff.c, which is a problem since we need to be able to either pass-in or have a callback to allow our "quirk" to specify a new IRQ for the device.Currently there is not. However if the device as a whole is wired to IRQ 14 you want to quirk
pdev->irq.
That would be great if both channels are mapped to 14, however the mapping set by default is for each channel to have an unique IRQ.
Signature · 4 lines
Ben Q: What's a light-year? A: One-third less calories than a regular year.
Quoted 11 lines · On Thu, 24 Jul 2008 12:50:42 +0100 Ben Dooks <[email protected]> wrote:
Quoted 7 lines · Alan Cox
Quoted 4 lines
I had a look, and there doesn't seem to be any way of specifying an IRQ for a host port when registering with
libata-sff.c, which is a problem since we need to be able to either pass-in or have a callback to allow our "quirk" to specify a new IRQ for the device.Currently there is not. However if the device as a whole is wired to IRQ 14 you want to quirk
pdev->irq.That would be great if both channels are mapped to 14, however the mapping set by default is for each channel to have an unique IRQ.
Ok so you've got a board reporting native mode using the legacy IRQ numbering (14/15 - or platform equivalents) ? In which case may I suggest you rewrite the header to indicate it is in legacy mode as per the BIOS guide ?
Quoted 12 lines · Alan Cox
Quoted 9 lines · On Thu, 24 Jul 2008 12:50:42 +0100 Ben Dooks <[email protected]> wrote:
Quoted 7 lines · Alan Cox
Quoted 4 lines
I had a look, and there doesn't seem to be any way of specifying an IRQ for a host port when registering with
libata-sff.c, which is a problem since we need to be able to either pass-in or have a callback to allow our "quirk" to specify a new IRQ for the device.Currently there is not. However if the device as a whole is wired to IRQ 14 you want to quirk
pdev->irq.
Quoted 7 lines
Quoted 2 lines
That would be great if both channels are mapped to 14, however the mapping set by default is for each channel to have an unique IRQ.
Ok so you've got a board reporting native mode using the legacy IRQ numbering (14/15 - or platform equivalents) ? In which case may I suggest you rewrite the header to indicate it is in legacy mode as per the BIOS guide ?
The drivers/ide/pci/alim15x3.c driver currently has code to correctly set the IRQ fields for anything that isn't SPARC. Does this mean we must disable the libata driver for anything that isn't SPARC? What about other boards where this device combination is present?
Signature · 4 lines
Ben Q: What's a light-year? A: One-third less calories than a regular year.
Quoted 9 lines
Quoted 4 lines
Ok so you've got a board reporting native mode using the legacy IRQ numbering (14/15 - or platform equivalents) ? In which case may I suggest you rewrite the header to indicate it is in legacy mode as per the BIOS guide ?
The
drivers/ide/pci/alim15x3.cdriver currently has code to correctly set the IRQ fields for anything that isn't SPARC. Does this mean we must disable the libata driver for anything that isn't SPARC? What about other boards where this device combination is present?
There should be no boards where this combination is present. IRQ 0 in native mode means "polled". It would therefore be helpful if you would start considering your board as a problem special case - one we need to support yes - rather than trying to argue that we should break support for standard configurations.
Alan
Quoted 10 lines · Alan Cox
Quoted 9 lines
Quoted 4 lines
Ok so you've got a board reporting native mode using the legacy IRQ numbering (14/15 - or platform equivalents) ? In which case may I suggest you rewrite the header to indicate it is in legacy mode as per the BIOS guide ?
The
drivers/ide/pci/alim15x3.cdriver currently has code to correctly set the IRQ fields for anything that isn't SPARC. Does this mean we must disable the libata driver for anything that isn't SPARC? What about other boards where this device combination is present?
Quoted 5 lines
There should be no boards where this combination is present. IRQ 0 in native mode means "polled". It would therefore be helpful if you would start considering your board as a problem special case - one we need to support yes - rather than trying to argue that we should break support for standard configurations.
So just because we fit a chip, we're suddenyly a special case? Moving to libata has ignored the code in the old IDE driver which ensures that the IRQ driver is used. I have no idea how many other systems have this same problems, but the systems we've shipped have had this chip setup for nearly 10 years now.
I admit the original fix is wrong, the change should be handled by some form of callback or a method of passing the interrupt numbers in when registering with the libata-sff.c driver.
Signature · 4 lines
Ben Q: What's a light-year? A: One-third less calories than a regular year.
Quoted 5 lines
So just because we fit a chip, we're suddenyly a special case? Moving to libata has ignored the code in the old IDE driver which ensures that the IRQ driver is used. I have no idea how many other systems have this same problems, but the systems we've shipped have had this chip setup for nearly 10 years now.
Yes and it happened to work by luck. Now it doesn't, instead standards compliant systems work better.
Quoted 3 lines
I admit the original fix is wrong, the change should be handled by some form of callback or a method of passing the interrupt numbers in when registering with the
libata-sff.cdriver.
Why do you want to hack up the libata driver code ? You've said yourself you have put the chip into native mode yet are using the legacy interrupts. Thats misconfiguration and you can undo that by configuring the chip right in the first place - as per the BIOS guide.
Hi,
Quoted 17 lines · Ben Dooks
Quoted 10 lines · Alan Cox
Quoted 9 lines
Quoted 4 lines
Ok so you've got a board reporting native mode using the legacy IRQ numbering (14/15 - or platform equivalents) ? In which case may I suggest you rewrite the header to indicate it is in legacy mode as per the BIOS guide ?
The
drivers/ide/pci/alim15x3.cdriver currently has code to correctly set the IRQ fields for anything that isn't SPARC. Does this mean we must disable the libata driver for anything that isn't SPARC? What about other boards where this device combination is present?Quoted 5 lines
There should be no boards where this combination is present. IRQ 0 in native mode means "polled". It would therefore be helpful if you would start considering your board as a problem special case - one we need to support yes - rather than trying to argue that we should break support for standard configurations.
Quoted 5 lines
So just because we fit a chip, we're suddenyly a special case? Moving to libata has ignored the code in the old IDE driver which ensures that the IRQ driver is used. I have no idea how many other systems have this same problems, but the systems we've shipped have had this chip setup for nearly 10 years now.
Please note that the recommended PATA support in kernel.org kernels is IDE subsystem and libata PATA is still considered experimental.
Quoted 3 lines
I admit the original fix is wrong, the change should be handled by some form of callback or a method of passing the interrupt numbers in when registering with the
libata-sff.cdriver.
Seems like the good solution would be to:
- add PCI HEADER quirk to claim legacy mode (as suggested by Alan)
- move ALi IRQ handling code to PCI layer and then hook it into pci_get_legacy_ide_irq()
Thanks,
Bart
Quoted 2 lines
Please note that the recommended PATA support in kernel.org kernels is IDE subsystem and libata PATA is still considered experimental.
This is false. Most distributions now use libata, and the old IDE code has recently had an enormous amount of patching so is if anything far more experimental than libata.
Quoted 4 lines
- add PCI HEADER quirk to claim legacy mode (as suggested by Alan)
- move ALi IRQ handling code to PCI layer and then hook it into
pci_get_legacy_ide_irq()
The second isn't needed and will happen automatically if the chip is properly configured (again see the BIOS writers guide)
Alan
Quoted 6 lines · Alan Cox
Quoted 2 lines
Please note that the recommended PATA support in kernel.org kernels is IDE subsystem and libata PATA is still considered experimental.
This is false. Most distributions now use libata, and the old IDE code has recently had an enormous amount of patching so is if anything far more experimental than libata.
*) I was talking primarily in ALi PATA context:
config PATA_ALI
tristate "ALi PATA support (Experimental)" depends on PCI && EXPERIMENTAL
...
config BLK_DEV_ALI15X3
tristate "ALI M15x3 chipset support"
select IDE_TIMINGS
select BLK_DEV_IDEDMA_PCI
...
*) Distributions are of course free to do make their own decisions.
*) By using this measurement of 'experimental', x86 would probably won
as the most experimental arch and networking as the most experimental subsystem. I'm proud that you put 'old' IDE code in the same class! :)
Quoted 7 lines
Quoted 4 lines
- add PCI HEADER quirk to claim legacy mode (as suggested by Alan)
- move ALi IRQ handling code to PCI layer and then hook it into
pci_get_legacy_ide_irq()The second isn't needed and will happen automatically if the chip is properly configured (again see the BIOS writers guide)
The second is of course optional for properly configured chips but it is not much of work and better be safe than sorry (also which 'BIOS writers guide' exactly are you referring to?).
Thanks,
Bart
Quoted 4 lines
config
PATA_ALI
tristate "ALi PATA support (Experimental)" depends on PCI && EXPERIMENTAL
...
I should update that now the ATAPI stuff is done.
Quoted 2 lines
The second is of course optional for properly configured chips but it is not much of work and better be safe than sorry (also which 'BIOS writers
That might make sense for the specific chipset.
Quoted 1 line
guide' exactly are you referring to?).
The ALi one for the chipset. You might need to be nice to ULi to get a copy however as I don't think its on the web site 8(
Quoted 2 lines
Does anyone know if the ALi M5229 actually exists outside of one of the M1543 bridges?
Yes - it exists in various forms in all sorts of ALi/ULi chipset products.
Quoted 2 lines
Note, this hardly seems to be a board-quirk, it is more of a device quirk...
Your description so far is of "device not put in a valid configuration according to the spec" - that seems to me to be board problems.
Quoted 10 lines · Alan Cox
Quoted 2 lines
Does anyone know if the ALi M5229 actually exists outside of one of the M1543 bridges?
Yes - it exists in various forms in all sorts of ALi/ULi chipset products.
Quoted 2 lines
Note, this hardly seems to be a board-quirk, it is more of a device quirk...
Your description so far is of "device not put in a valid configuration according to the spec" - that seems to me to be board problems.
No, the configuration is correct, you cannot assign this device an PCI IRQ when it is embedded in the M1543 bridge. The bridge itself has other configuration registers that specify where the IDE IRQs get routed.
Signature · 4 lines
Ben Q: What's a light-year? A: One-third less calories than a regular year.
Quoted 9 lines · On Thu, 24 Jul 2008 12:50:00 +0100 Ben Dooks <[email protected]> wrote:
Quoted 8 lines · Alan Cox
Quoted 2 lines
Does anyone know if the ALi M5229 actually exists outside of one of the M1543 bridges?
Yes - it exists in various forms in all sorts of ALi/ULi chipset products.
Quoted 2 lines
Note, this hardly seems to be a board-quirk, it is more of a device quirk...
Quoted 7 lines
Quoted 2 lines
Your description so far is of "device not put in a valid configuration according to the spec" - that seems to me to be board problems.
No, the configuration is correct, you cannot assign this device an PCI IRQ when it is embedded in the M1543 bridge. The bridge itself has other configuration registers that specify where the IDE IRQs get routed.
You aren't listening: "according to spec" - there is a specification for how legacy and native mode ATA controllers function and should be configured. You are describing a situation which appears to be outside that spec entirely.
Alan
Quoted 20 lines · Alan Cox
Quoted 9 lines · On Thu, 24 Jul 2008 12:50:00 +0100 Ben Dooks <[email protected]> wrote:
Quoted 8 lines · Alan Cox
Quoted 2 lines
Does anyone know if the ALi M5229 actually exists outside of one of the M1543 bridges?
Yes - it exists in various forms in all sorts of ALi/ULi chipset products.
Quoted 2 lines
Note, this hardly seems to be a board-quirk, it is more of a device quirk...
Quoted 7 lines
Quoted 2 lines
Your description so far is of "device not put in a valid configuration according to the spec" - that seems to me to be board problems.
No, the configuration is correct, you cannot assign this device an PCI IRQ when it is embedded in the M1543 bridge. The bridge itself has other configuration registers that specify where the IDE IRQs get routed.
Quoted 4 lines
You aren't listening: "according to spec" - there is a specification for how legacy and native mode ATA controllers function and should be configured. You are describing a situation which appears to be outside that spec entirely.
Ok, so this controller is not performing to the IRQ portion of the PCI IDE specification, but is this any reason to force it back into legacy mode?
Signature · 4 lines
Ben Q: What's a light-year? A: One-third less calories than a regular year.
Quoted 3 lines
Ok, so this controller is not performing to the IRQ portion of the PCI IDE specification, but is this any reason to force it back into legacy mode?
Well the obvious reason to do that is that is what the BIOS guide says and what the standards expect. Given that legacy mode has no performance impact at all as it just changes the IRQ interpretation and the port mapping rules it seems silly not to do that
Alan
Quoted 8 lines · Alan Cox
Quoted 3 lines
Ok, so this controller is not performing to the IRQ portion of the PCI IDE specification, but is this any reason to force it back into legacy mode?
Well the obvious reason to do that is that is what the BIOS guide says and what the standards expect. Given that legacy mode has no performance impact at all as it just changes the IRQ interpretation and the port mapping rules it seems silly not to do that
I've changed drivers/ata/pata_ali.c to change the mode back to compatibility mode, but libata-sff.c is still trying to use the pci configuration BARs to map the control registers. If the copy of the "PCI IDE Controller Specification, Revision 1.0" [1] is to be belived, the values in the PCI BARs should be ignored.
[1] http://suif.stanford.edu/~csapuntz/specs/pciide.ps
Quoted 5 lines
Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 5 lines
-- Ben Q: What's a light-year? A: One-third less calories than a regular year.
Quoted 5 lines
I've changed
drivers/ata/pata_ali.cto change the mode back to compatibility mode, butlibata-sff.cis still trying to use the pci configuration BARs to map the control registers. If the copy of the "PCI IDE Controller Specification, Revision 1.0" [1] is to be belived, the values in the PCI BARs should be ignored.
The PCI code knows about this and fills in BAR0 to BAR3 correctly according to the host platform (defaulting to the usual PC values). We have to do that anyway to keep the PCI resource management straight.
If you switch it in pata_ali that'll break, use a PCI quirk as I said before - switch it with a HEADER quirk for your platform.
Alan