libata: make both legacy ports share one host_set , take #2
PATCH 1/2 libata: make both legacy ports share one host_set , take #2
Patch 1/2:
- make both legacy ports share one host_set
- host_set->irq is preserved for shared irq case(PCI native mode).
- ap->ioaddr.irq is added for per-port non-shared irq case(legacy mode).
---
The host_set is shared between the 2 legacy ports for the ATA_HOST_SIMPLEX to work.
The per-port irq is also freed for the pata_amd unloading to load. Briefly tested on x86 box.
drivers/scsi/ata_piix.c +1 -1
@@ -249,7 +249,7 @@ static const struct ata_port_operations .error_handler = piix_pata_error_handler, .post_internal_cmd = ata_bmdma_post_internal_cmd, - .irq_handler = ata_interrupt, + .irq_handler = ata_nonshared_interrupt, .irq_clear = ata_bmdma_irq_clear, .port_start = ata_port_start,
drivers/scsi/libata-bmdma.c +43 -54
148 lines not on this page · raw message
drivers/scsi/libata-core.c +81 -19
180 lines not on this page · raw message
include/linux/libata.h +3 -1
@@ -323,6 +323,7 @@ struct ata_ioports {unsigned long ctl_addr;unsigned long bmdma_addr;unsigned long scr_addr; +unsigned long irq; };struct ata_probe_ent {
@@ -356,7 +357,6 @@ struct ata_host_set {unsigned long flags;int simplex_claimed;/* Keep seperate in case we ever need to do this locked */ -struct ata_host_set *next;/* for legacy mode */ struct ata_port *ports[0 ]; };
@@ -688,6 +688,8 @@ extern int ata_port_start (struct ata_poextern void ata_port_stop (struct ata_port *ap);extern void ata_host_stop (struct ata_host_set *host_set);extern irqreturn_t ata_interrupt (int irq,void *dev_instance,struct pt_regs *regs); +extern irqreturn_t ata_nonshared_interrupt (int irq,void *dev_instance, +struct pt_regs *regs);extern void ata_mmio_data_xfer (struct ata_device *adev,unsigned char *buf,unsigned int buflen,int write_data); extern void ata_pio_data_xfer(struct ata_device *adev, unsigned char *buf,
Quoted 32 lines · Unicorn Chang
drivers/scsi/libata-core.c +11
-6@@ -5336,11 +5383,21 @@ int ata_device_add(const struct ata_prob ap->ioaddr.cmd_addr, ap->ioaddr.ctl_addr, ap->ioaddr.bmdma_addr, - ent->irq); + irq);ata_chk_status (ap); host_set->ops->irq_clear (ap);ata_eh_freeze_port (ap);/* freeze port before requesting IRQ */ + +if (ap->ioaddr.irq){ +/* obtain nonshared per-port irq */ + rc =request_irq (irq, ent->port_ops->irq_handler,0 ,DRV_NAME , ap); +if (rc) { +dev_printk (KERN_ERR , dev,"irq %lu request failed: %d\n" , + irq, rc); + goto err_out; + } + } count++; }@@ -5348,12 +5405,14 @@ int ata_device_add(const struct ata_prob goto err_free_ret;/* obtain irq, that is shared between channels */ - rc =request_irq (ent->irq, ent->port_ops->irq_handler, ent->irq_flags, -DRV_NAME , host_set); -if (rc) { -dev_printk (KERN_ERR , dev,"irq %lu request failed: %d\n" , - ent->irq, rc);
drivers/scsi/libata-core.c int ata_device_add(const struct ata_prob · line 5416
Quoted 10 lines
drivers/scsi/libata-core.c +8
-1- goto err_out; +if (ent->irq){ + rc =request_irq (ent->irq, ent->port_ops->irq_handler, ent->irq_flags, +DRV_NAME , host_set); +if (rc) { +dev_printk (KERN_ERR , dev,"irq %lu request failed: %d\n" , + ent->irq, rc); + goto err_out; + } }
I'm sorry, but any patch that adds additional request_irq() calls to libata in this manner will be NAK'd.
The current location of request_irq() inside libata is a design mistake on my part, and this sort of change compounds that mistake.
An improved control flow, which moves request_irq() from libata-core to LLDD, might be:
* LLDD initializes the host controller, in driver.c (as it does today)
* LLDD, perhaps via bmdma helpers, makes certain that no pending interrupt conditions exist (many LLDDs do this today)
* LLDD, perhaps via bmdma helpers, calls request_irq()
- problem: need to pass it a useful pointer
-> move drivers to familiar model where LLDD allocs
host_set and ports, does <stuff>, then registers host_set/ports with system
* LLDD proceeds to register itself with libata core
Overall, the current libata request_irq() design restricts us from doing a better job with legacy ports (your case), and also prevents doing advanced stuff with MSI-X and other approaching technologies.
Jeff