* Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE [not found] ` <fa.AqEKTvguYFAzDFHy/We/8MpOqmo@ifi.uio.no> @ 2008-08-04 20:07 ` Robert Hancock 2008-08-04 19:55 ` Alan Cox 2008-08-04 20:37 ` Jeff Garzik 0 siblings, 2 replies; 21+ messages in thread From: Robert Hancock @ 2008-08-04 20:07 UTC (permalink / raw) To: Alan Cox Cc: Bartlomiej Zolnierkiewicz, James Bottomley, ksummit-2008-discuss, linux-kernel, linux-ide Alan Cox wrote: >> * There are still corner case in libata core - PIO is dead slow >> compared to drivers/ide/, > > There are two there - libata keeps IRQs blocked for longer in PIO mode as > well which is a factor for realtime that needs looking at, as well as > using 16bit not 32bit I/O for most devices (which is trivial to fix). The > IRQ masking stuff is more complex and old IDE handles it far better for > PIO on non shared IRQ interfaces. That is actually probably the most > complicated thing to address of the stuff you'd want to do if you were > going to kill off old IDE. I was looking into the 32-bit PIO issue a bit yesterday. It looks like some of the VLB libata drivers are doing this internally already, so it shouldn't be hard to do this in the core. Only question is how we know generically if the controller can do it or not? It looks like in old IDE, a few controllers explicitly disable it, but it appears that it doesn't default to on for any controller, so it's possible there are others on which it doesn't work. Presumably anything on an actual 16-bit bus (ISA, LPC, etc.) wouldn't like it, to start with. There's also the matter of the identify bit to indicate whether the drive supports 32-bit transfers, which was reallocated to trusted computing in ATA-8 so any drive matching that standard will indicate not supported. I couldn't track down where that bit was actually defined in the first place, all the way back to ATA-1 it seems to be indicated as reserved. Actually, I'm not sure why the drive cares in the first place, it would seem like a pure host controller issue.. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-04 20:07 ` Kernel Summit request for Discussion of future of ATA (libata) and IDE Robert Hancock @ 2008-08-04 19:55 ` Alan Cox 2008-08-04 21:17 ` Robert Hancock ` (2 more replies) 2008-08-04 20:37 ` Jeff Garzik 1 sibling, 3 replies; 21+ messages in thread From: Alan Cox @ 2008-08-04 19:55 UTC (permalink / raw) To: Robert Hancock Cc: Bartlomiej Zolnierkiewicz, James Bottomley, ksummit-2008-discuss, linux-kernel, linux-ide > I was looking into the 32-bit PIO issue a bit yesterday. It looks like > some of the VLB libata drivers are doing this internally already, so it > shouldn't be hard to do this in the core. Only question is how we know > generically if the controller can do it or not? It looks like in old You don't. Basically it is controller dependant. Pretty much all the newer controllers support the 32bit PIO data cycles. Most PCI controllers it makes no speed difference but host bus controllers (especially PIIX/ICH) really benefit. > supported. I couldn't track down where that bit was actually defined in > the first place, all the way back to ATA-1 it seems to be indicated as > reserved. Actually, I'm not sure why the drive cares in the first place, > it would seem like a pure host controller issue.. It goes back before IDE into the depths of the original compaq spec. When you have a device wired basically directly to the ISA bus (original IDE) it mattered. I don't believe it is relevant to any of the PCI controllers. Alan ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-04 19:55 ` Alan Cox @ 2008-08-04 21:17 ` Robert Hancock 2008-08-04 21:06 ` Alan Cox 2008-08-04 21:55 ` Sergei Shtylyov 2008-08-04 22:12 ` Mark Lord 2 siblings, 1 reply; 21+ messages in thread From: Robert Hancock @ 2008-08-04 21:17 UTC (permalink / raw) To: Alan Cox Cc: Bartlomiej Zolnierkiewicz, James Bottomley, ksummit-2008-discuss, linux-kernel, linux-ide, Jeff Garzik Alan Cox wrote: >> I was looking into the 32-bit PIO issue a bit yesterday. It looks like >> some of the VLB libata drivers are doing this internally already, so it >> shouldn't be hard to do this in the core. Only question is how we know >> generically if the controller can do it or not? It looks like in old > > You don't. Basically it is controller dependant. Pretty much all the > newer controllers support the 32bit PIO data cycles. Most PCI controllers > it makes no speed difference but host bus controllers (especially > PIIX/ICH) really benefit. > >> supported. I couldn't track down where that bit was actually defined in >> the first place, all the way back to ATA-1 it seems to be indicated as >> reserved. Actually, I'm not sure why the drive cares in the first place, >> it would seem like a pure host controller issue.. > > It goes back before IDE into the depths of the original compaq spec. When > you have a device wired basically directly to the ISA bus (original IDE) > it mattered. I don't believe it is relevant to any of the PCI controllers. I guess that bit doesn't really make any difference with remotely modern drives, then.. Could we make that ata_id_has_dword_io check always return true if ata_id_is_ata returns true and only check word 48 if not? I saw Willy Tarreau's patch from February for this, I agree that we should likely use a separate data_xfer method for 32-bit transfer (or if enough controllers should support 32-bit, then just make it be the default and make a separate 16-bit only function for those that don't), rather than punting the decision to the user with hdparm. You mentioned in the thread for Willy's patch that "some controllers have quirky rules for 32bit xfers" - any details anywhere? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-04 21:17 ` Robert Hancock @ 2008-08-04 21:06 ` Alan Cox 2008-08-04 21:48 ` Robert Hancock 2008-08-06 0:21 ` Robert Hancock 0 siblings, 2 replies; 21+ messages in thread From: Alan Cox @ 2008-08-04 21:06 UTC (permalink / raw) To: Robert Hancock Cc: Bartlomiej Zolnierkiewicz, James Bottomley, ksummit-2008-discuss, linux-kernel, linux-ide, Jeff Garzik > I guess that bit doesn't really make any difference with remotely modern > drives, then.. Could we make that ata_id_has_dword_io check always > return true if ata_id_is_ata returns true and only check word 48 if not? I suspect (but need to dig further) that ata_id_has_dword_io should only be called by pata_legacy. > I saw Willy Tarreau's patch from February for this, I agree that we > should likely use a separate data_xfer method for 32-bit transfer (or if > enough controllers should support 32-bit, then just make it be the > default and make a separate 16-bit only function for those that don't), > rather than punting the decision to the user with hdparm. Definitely. > You mentioned in the thread for Willy's patch that "some > controllers have quirky rules for 32bit xfers" - any details anywhere? There are two main ones - Some controllers only support 32bit I/O for a multiple of 32bit values [sometimes 'unless the fifo is disabled']. I'd have to go back over the docs but I think the AMD may be one of those - Some controllers (VLB generally) require a magic sequence before the transfer. You'll see that in the pata_legacy bits. Alan ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-04 21:06 ` Alan Cox @ 2008-08-04 21:48 ` Robert Hancock 2008-08-06 0:21 ` Robert Hancock 1 sibling, 0 replies; 21+ messages in thread From: Robert Hancock @ 2008-08-04 21:48 UTC (permalink / raw) To: Alan Cox Cc: Bartlomiej Zolnierkiewicz, James Bottomley, ksummit-2008-discuss, linux-kernel, linux-ide, Jeff Garzik Alan Cox wrote: >> You mentioned in the thread for Willy's patch that "some >> controllers have quirky rules for 32bit xfers" - any details anywhere? > > There are two main ones > > - Some controllers only support 32bit I/O for a multiple of 32bit values > [sometimes 'unless the fifo is disabled']. I'd have to go back over the > docs but I think the AMD may be one of those The AMD-766 doc I have says that when the Secondary Posted Write Buffer or Primary Posted Write Buffer are enabled, only 32-bit writes are allowed to the data port. It doesn't say anything about a restriction with the read prefetch buffer though. I guess it depends if any other controllers could potentially have this restriction. I suspect non-multiple-of-32-bit transfers are rare enough we could just fall back to 16-bit IO always for them, but maybe not. > - Some controllers (VLB generally) require a magic sequence before the > transfer. You'll see that in the pata_legacy bits. > > Alan > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-04 21:06 ` Alan Cox 2008-08-04 21:48 ` Robert Hancock @ 2008-08-06 0:21 ` Robert Hancock 2008-08-06 0:44 ` Tejun Heo 2008-08-06 8:51 ` Alan Cox 1 sibling, 2 replies; 21+ messages in thread From: Robert Hancock @ 2008-08-06 0:21 UTC (permalink / raw) To: Alan Cox Cc: Bartlomiej Zolnierkiewicz, James Bottomley, ksummit-2008-discuss, linux-kernel, linux-ide, Jeff Garzik Alan Cox wrote: >> I guess that bit doesn't really make any difference with remotely modern >> drives, then.. Could we make that ata_id_has_dword_io check always >> return true if ata_id_is_ata returns true and only check word 48 if not? > > I suspect (but need to dig further) that ata_id_has_dword_io should only > be called by pata_legacy. > >> I saw Willy Tarreau's patch from February for this, I agree that we >> should likely use a separate data_xfer method for 32-bit transfer (or if >> enough controllers should support 32-bit, then just make it be the >> default and make a separate 16-bit only function for those that don't), >> rather than punting the decision to the user with hdparm. > > Definitely. > >> You mentioned in the thread for Willy's patch that "some >> controllers have quirky rules for 32bit xfers" - any details anywhere? > > There are two main ones > > - Some controllers only support 32bit I/O for a multiple of 32bit values > [sometimes 'unless the fifo is disabled']. I'd have to go back over the > docs but I think the AMD may be one of those > - Some controllers (VLB generally) require a magic sequence before the > transfer. You'll see that in the pata_legacy bits. Here's my first cut at it. Compile tested only. This sets most controllers to use 32-bit PIO except for those which could potentially be on a real ISA or other 16-bit bus. It's a bit non-obvious what to do with some of the drivers, so input is welcome. This implementation doesn't check the ata_id_has_dword_io at all, since it would only make a difference on controllers where we don't really want to use it anyway. It seems like regardless of whether we do 32-bit by default or not the 32-bit data_xfer function should be added to libata core as we have several drivers which duplicate the same code currently.. Signed-off-by: Robert Hancock <hancockr@shaw.ca> diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c index 304fdc6..acb65a9 100644 --- a/drivers/ata/libata-sff.c +++ b/drivers/ata/libata-sff.c @@ -673,13 +673,14 @@ static inline void ata_tf_to_host(struct ata_port *ap, } /** - * ata_sff_data_xfer - Transfer data by PIO + * ata_sff_data_xfer_16bit - Transfer data by PIO * @dev: device to target * @buf: data buffer * @buflen: buffer length * @rw: read/write * - * Transfer data from/to the device data register by PIO. + * Transfer data from/to the device data register by PIO using only + * 16-bit transfers. * * LOCKING: * Inherited from caller. @@ -687,7 +688,7 @@ static inline void ata_tf_to_host(struct ata_port *ap, * RETURNS: * Bytes consumed. */ -unsigned int ata_sff_data_xfer(struct ata_device *dev, unsigned char *buf, +unsigned int ata_sff_data_xfer_16bit(struct ata_device *dev, unsigned char *buf, unsigned int buflen, int rw) { struct ata_port *ap = dev->link->ap; @@ -719,6 +720,51 @@ unsigned int ata_sff_data_xfer(struct ata_device *dev, unsigned char *buf, } /** + * ata_sff_data_xfer - Transfer data by PIO + * @dev: device to target + * @buf: data buffer + * @buflen: buffer length + * @rw: read/write + * + * Transfer data from/to the device data register by PIO. + * + * LOCKING: + * Inherited from caller. + * + * RETURNS: + * Bytes consumed. + */ +unsigned int ata_sff_data_xfer(struct ata_device *dev, unsigned char *buf, + unsigned int buflen, int rw) +{ + struct ata_port *ap = dev->link->ap; + void __iomem *data_addr = ap->ioaddr.data_addr; + unsigned int words = buflen >> 2; + unsigned int slop = buflen & 3; + + /* Transfer multiple of 4 bytes */ + if (rw == READ) + ioread32_rep(data_addr, buf, words); + else + iowrite32_rep(data_addr, buf, words); + + /* Transfer trailing 1 byte, if any. */ + if (unlikely(slop)) { + __le32 pad = 0; + if (rw == READ) { + pad = cpu_to_le32(ioread32(ap->ioaddr.data_addr)); + memcpy(buf + buflen - slop, &pad, slop); + } else { + memcpy(&pad, buf + buflen - slop, slop); + iowrite32(le32_to_cpu(pad), ap->ioaddr.data_addr); + } + buflen += 4 - slop; + } + + return buflen; +} + +/** * ata_sff_data_xfer_noirq - Transfer data by PIO * @dev: device to target * @buf: data buffer @@ -748,6 +794,36 @@ unsigned int ata_sff_data_xfer_noirq(struct ata_device *dev, unsigned char *buf, } /** + * ata_sff_data_xfer_noirq_16bit - Transfer data by PIO + * @dev: device to target + * @buf: data buffer + * @buflen: buffer length + * @rw: read/write + * + * Transfer data from/to the device data register by PIO. Do the + * transfer with interrupts disabled using only 16-bit transfers. + * + * LOCKING: + * Inherited from caller. + * + * RETURNS: + * Bytes consumed. + */ +unsigned int ata_sff_data_xfer_noirq_16bit(struct ata_device *dev, + unsigned char *buf, + unsigned int buflen, int rw) +{ + unsigned long flags; + unsigned int consumed; + + local_irq_save(flags); + consumed = ata_sff_data_xfer_16bit(dev, buf, buflen, rw); + local_irq_restore(flags); + + return consumed; +} + +/** * ata_pio_sector - Transfer a sector of data. * @qc: Command on going * @@ -2827,7 +2903,9 @@ EXPORT_SYMBOL_GPL(ata_sff_tf_load); EXPORT_SYMBOL_GPL(ata_sff_tf_read); EXPORT_SYMBOL_GPL(ata_sff_exec_command); EXPORT_SYMBOL_GPL(ata_sff_data_xfer); +EXPORT_SYMBOL_GPL(ata_sff_data_xfer_16bit); EXPORT_SYMBOL_GPL(ata_sff_data_xfer_noirq); +EXPORT_SYMBOL_GPL(ata_sff_data_xfer_noirq_16bit); EXPORT_SYMBOL_GPL(ata_sff_irq_on); EXPORT_SYMBOL_GPL(ata_sff_irq_clear); EXPORT_SYMBOL_GPL(ata_sff_hsm_move); diff --git a/drivers/ata/pata_at32.c b/drivers/ata/pata_at32.c index 82fb6e2..aa90b19 100644 --- a/drivers/ata/pata_at32.c +++ b/drivers/ata/pata_at32.c @@ -173,6 +173,7 @@ static struct scsi_host_template at32_sht = { static struct ata_port_operations at32_port_ops = { .inherits = &ata_sff_port_ops, .cable_detect = ata_cable_40wire, + .sff_data_xfer = ata_sff_data_xfer_16bit, .set_piomode = pata_at32_set_piomode, }; diff --git a/drivers/ata/pata_icside.c b/drivers/ata/pata_icside.c index cf9e984..a1f09ca 100644 --- a/drivers/ata/pata_icside.c +++ b/drivers/ata/pata_icside.c @@ -336,7 +336,7 @@ static struct ata_port_operations pata_icside_port_ops = { .inherits = &ata_sff_port_ops, /* no need to build any PRD tables for DMA */ .qc_prep = ata_noop_qc_prep, - .sff_data_xfer = ata_sff_data_xfer_noirq, + .sff_data_xfer = ata_sff_data_xfer_noirq_16bit, .bmdma_setup = pata_icside_bmdma_setup, .bmdma_start = pata_icside_bmdma_start, .bmdma_stop = pata_icside_bmdma_stop, diff --git a/drivers/ata/pata_isapnp.c b/drivers/ata/pata_isapnp.c index 6a111ba..6f38f76 100644 --- a/drivers/ata/pata_isapnp.c +++ b/drivers/ata/pata_isapnp.c @@ -26,6 +26,7 @@ static struct scsi_host_template isapnp_sht = { static struct ata_port_operations isapnp_port_ops = { .inherits = &ata_sff_port_ops, .cable_detect = ata_cable_40wire, + .sff_data_xfer = ata_sff_data_xfer_16bit, }; /** diff --git a/drivers/ata/pata_legacy.c b/drivers/ata/pata_legacy.c index bc037ff..d919934 100644 --- a/drivers/ata/pata_legacy.c +++ b/drivers/ata/pata_legacy.c @@ -226,12 +226,12 @@ static const struct ata_port_operations legacy_base_port_ops = { static struct ata_port_operations simple_port_ops = { .inherits = &legacy_base_port_ops, - .sff_data_xfer = ata_sff_data_xfer_noirq, + .sff_data_xfer = ata_sff_data_xfer_noirq_16bit, }; static struct ata_port_operations legacy_port_ops = { .inherits = &legacy_base_port_ops, - .sff_data_xfer = ata_sff_data_xfer_noirq, + .sff_data_xfer = ata_sff_data_xfer_noirq_16bit, .set_mode = legacy_set_mode, }; @@ -286,39 +286,18 @@ static void pdc20230_set_piomode(struct ata_port *ap, struct ata_device *adev) static unsigned int pdc_data_xfer_vlb(struct ata_device *dev, unsigned char *buf, unsigned int buflen, int rw) { - if (ata_id_has_dword_io(dev->id)) { - struct ata_port *ap = dev->link->ap; - int slop = buflen & 3; - unsigned long flags; + unsigned long flags; - local_irq_save(flags); + local_irq_save(flags); - /* Perform the 32bit I/O synchronization sequence */ - ioread8(ap->ioaddr.nsect_addr); - ioread8(ap->ioaddr.nsect_addr); - ioread8(ap->ioaddr.nsect_addr); + /* Perform the 32bit I/O synchronization sequence */ + ioread8(ap->ioaddr.nsect_addr); + ioread8(ap->ioaddr.nsect_addr); + ioread8(ap->ioaddr.nsect_addr); - /* Now the data */ - if (rw == READ) - ioread32_rep(ap->ioaddr.data_addr, buf, buflen >> 2); - else - iowrite32_rep(ap->ioaddr.data_addr, buf, buflen >> 2); - - if (unlikely(slop)) { - __le32 pad; - if (rw == READ) { - pad = cpu_to_le32(ioread32(ap->ioaddr.data_addr)); - memcpy(buf + buflen - slop, &pad, slop); - } else { - memcpy(&pad, buf + buflen - slop, slop); - iowrite32(le32_to_cpu(pad), ap->ioaddr.data_addr); - } - buflen += 4 - slop; - } - local_irq_restore(flags); - } else - buflen = ata_sff_data_xfer_noirq(dev, buf, buflen, rw); + buflen = ata_sff_data_xfer(dev, buf, buflen, rw); + local_irq_restore(flags); return buflen; } @@ -733,33 +712,6 @@ static unsigned int qdi_qc_issue(struct ata_queued_cmd *qc) return ata_sff_qc_issue(qc); } -static unsigned int vlb32_data_xfer(struct ata_device *adev, unsigned char *buf, - unsigned int buflen, int rw) -{ - struct ata_port *ap = adev->link->ap; - int slop = buflen & 3; - - if (ata_id_has_dword_io(adev->id)) { - if (rw == WRITE) - iowrite32_rep(ap->ioaddr.data_addr, buf, buflen >> 2); - else - ioread32_rep(ap->ioaddr.data_addr, buf, buflen >> 2); - - if (unlikely(slop)) { - __le32 pad; - if (rw == WRITE) { - memcpy(&pad, buf + buflen - slop, slop); - iowrite32(le32_to_cpu(pad), ap->ioaddr.data_addr); - } else { - pad = cpu_to_le32(ioread32(ap->ioaddr.data_addr)); - memcpy(buf + buflen - slop, &pad, slop); - } - } - return (buflen + 3) & ~3; - } else - return ata_sff_data_xfer(adev, buf, buflen, rw); -} - static int qdi_port(struct platform_device *dev, struct legacy_probe *lp, struct legacy_data *ld) { @@ -773,19 +725,19 @@ static struct ata_port_operations qdi6500_port_ops = { .inherits = &legacy_base_port_ops, .set_piomode = qdi6500_set_piomode, .qc_issue = qdi_qc_issue, - .sff_data_xfer = vlb32_data_xfer, + .sff_data_xfer = ata_sff_data_xfer, }; static struct ata_port_operations qdi6580_port_ops = { .inherits = &legacy_base_port_ops, .set_piomode = qdi6580_set_piomode, - .sff_data_xfer = vlb32_data_xfer, + .sff_data_xfer = ata_sff_data_xfer, }; static struct ata_port_operations qdi6580dp_port_ops = { .inherits = &legacy_base_port_ops, .set_piomode = qdi6580dp_set_piomode, - .sff_data_xfer = vlb32_data_xfer, + .sff_data_xfer = ata_sff_data_xfer, }; static DEFINE_SPINLOCK(winbond_lock); @@ -856,7 +808,7 @@ static int winbond_port(struct platform_device *dev, static struct ata_port_operations winbond_port_ops = { .inherits = &legacy_base_port_ops, .set_piomode = winbond_set_piomode, - .sff_data_xfer = vlb32_data_xfer, + .sff_data_xfer = ata_sff_data_xfer, }; static struct legacy_controller controllers[] = { diff --git a/drivers/ata/pata_mpc52xx.c b/drivers/ata/pata_mpc52xx.c index a9e8273..0ab2c8e 100644 --- a/drivers/ata/pata_mpc52xx.c +++ b/drivers/ata/pata_mpc52xx.c @@ -265,6 +265,7 @@ static struct ata_port_operations mpc52xx_ata_port_ops = { .cable_detect = ata_cable_40wire, .set_piomode = mpc52xx_ata_set_piomode, .post_internal_cmd = ATA_OP_NULL, + .sff_data_xfer = ata_sff_data_xfer_16bit, }; static int __devinit diff --git a/drivers/ata/pata_pcmcia.c b/drivers/ata/pata_pcmcia.c index 41b4361..8cb4923 100644 --- a/drivers/ata/pata_pcmcia.c +++ b/drivers/ata/pata_pcmcia.c @@ -133,7 +133,7 @@ static struct scsi_host_template pcmcia_sht = { static struct ata_port_operations pcmcia_port_ops = { .inherits = &ata_sff_port_ops, - .sff_data_xfer = ata_sff_data_xfer_noirq, + .sff_data_xfer = ata_sff_data_xfer_noirq_16bit, .cable_detect = ata_cable_40wire, .set_mode = pcmcia_set_mode, }; diff --git a/drivers/ata/pata_platform.c b/drivers/ata/pata_platform.c index 8f65ad6..3310eb0 100644 --- a/drivers/ata/pata_platform.c +++ b/drivers/ata/pata_platform.c @@ -52,7 +52,7 @@ static struct scsi_host_template pata_platform_sht = { static struct ata_port_operations pata_platform_port_ops = { .inherits = &ata_sff_port_ops, - .sff_data_xfer = ata_sff_data_xfer_noirq, + .sff_data_xfer = ata_sff_data_xfer_noirq_16bit, .cable_detect = ata_cable_unknown, .set_mode = pata_platform_set_mode, .port_start = ATA_OP_NULL, diff --git a/drivers/ata/pata_qdi.c b/drivers/ata/pata_qdi.c index 63b7a1c..36cc778 100644 --- a/drivers/ata/pata_qdi.c +++ b/drivers/ata/pata_qdi.c @@ -124,35 +124,6 @@ static unsigned int qdi_qc_issue(struct ata_queued_cmd *qc) return ata_sff_qc_issue(qc); } -static unsigned int qdi_data_xfer(struct ata_device *dev, unsigned char *buf, - unsigned int buflen, int rw) -{ - if (ata_id_has_dword_io(dev->id)) { - struct ata_port *ap = dev->link->ap; - int slop = buflen & 3; - - if (rw == READ) - ioread32_rep(ap->ioaddr.data_addr, buf, buflen >> 2); - else - iowrite32_rep(ap->ioaddr.data_addr, buf, buflen >> 2); - - if (unlikely(slop)) { - __le32 pad; - if (rw == READ) { - pad = cpu_to_le32(ioread32(ap->ioaddr.data_addr)); - memcpy(buf + buflen - slop, &pad, slop); - } else { - memcpy(&pad, buf + buflen - slop, slop); - iowrite32(le32_to_cpu(pad), ap->ioaddr.data_addr); - } - buflen += 4 - slop; - } - } else - buflen = ata_sff_data_xfer(dev, buf, buflen, rw); - - return buflen; -} - static struct scsi_host_template qdi_sht = { ATA_PIO_SHT(DRV_NAME), }; @@ -160,7 +131,6 @@ static struct scsi_host_template qdi_sht = { static struct ata_port_operations qdi6500_port_ops = { .inherits = &ata_sff_port_ops, .qc_issue = qdi_qc_issue, - .sff_data_xfer = qdi_data_xfer, .cable_detect = ata_cable_40wire, .set_piomode = qdi6500_set_piomode, }; diff --git a/drivers/ata/pata_winbond.c b/drivers/ata/pata_winbond.c index a7606b0..56f5f8e 100644 --- a/drivers/ata/pata_winbond.c +++ b/drivers/ata/pata_winbond.c @@ -92,42 +92,12 @@ static void winbond_set_piomode(struct ata_port *ap, struct ata_device *adev) } -static unsigned int winbond_data_xfer(struct ata_device *dev, - unsigned char *buf, unsigned int buflen, int rw) -{ - struct ata_port *ap = dev->link->ap; - int slop = buflen & 3; - - if (ata_id_has_dword_io(dev->id)) { - if (rw == READ) - ioread32_rep(ap->ioaddr.data_addr, buf, buflen >> 2); - else - iowrite32_rep(ap->ioaddr.data_addr, buf, buflen >> 2); - - if (unlikely(slop)) { - __le32 pad; - if (rw == READ) { - pad = cpu_to_le32(ioread32(ap->ioaddr.data_addr)); - memcpy(buf + buflen - slop, &pad, slop); - } else { - memcpy(&pad, buf + buflen - slop, slop); - iowrite32(le32_to_cpu(pad), ap->ioaddr.data_addr); - } - buflen += 4 - slop; - } - } else - buflen = ata_sff_data_xfer(dev, buf, buflen, rw); - - return buflen; -} - static struct scsi_host_template winbond_sht = { ATA_PIO_SHT(DRV_NAME), }; static struct ata_port_operations winbond_port_ops = { .inherits = &ata_sff_port_ops, - .sff_data_xfer = winbond_data_xfer, .cable_detect = ata_cable_40wire, .set_piomode = winbond_set_piomode, }; diff --git a/include/linux/libata.h b/include/linux/libata.h index 06b8033..643328a 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -1481,8 +1481,12 @@ extern void ata_sff_exec_command(struct ata_port *ap, const struct ata_taskfile *tf); extern unsigned int ata_sff_data_xfer(struct ata_device *dev, unsigned char *buf, unsigned int buflen, int rw); +extern unsigned int ata_sff_data_xfer_16bit(struct ata_device *dev, + unsigned char *buf, unsigned int buflen, int rw); extern unsigned int ata_sff_data_xfer_noirq(struct ata_device *dev, unsigned char *buf, unsigned int buflen, int rw); +extern unsigned int ata_sff_data_xfer_noirq_16bit(struct ata_device *dev, + unsigned char *buf, unsigned int buflen, int rw); extern u8 ata_sff_irq_on(struct ata_port *ap); extern void ata_sff_irq_clear(struct ata_port *ap); extern int ata_sff_hsm_move(struct ata_port *ap, struct ata_queued_cmd *qc, ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-06 0:21 ` Robert Hancock @ 2008-08-06 0:44 ` Tejun Heo 2008-08-06 2:30 ` Robert Hancock 2008-08-06 11:27 ` Sergei Shtylyov 2008-08-06 8:51 ` Alan Cox 1 sibling, 2 replies; 21+ messages in thread From: Tejun Heo @ 2008-08-06 0:44 UTC (permalink / raw) To: Robert Hancock Cc: Alan Cox, Bartlomiej Zolnierkiewicz, James Bottomley, ksummit-2008-discuss, linux-kernel, linux-ide, Jeff Garzik Robert Hancock wrote: > Here's my first cut at it. Compile tested only. This sets most controllers > to use 32-bit PIO except for those which could potentially be on a real ISA > or other 16-bit bus. It's a bit non-obvious what to do with some of the > drivers, so input is welcome. > > This implementation doesn't check the ata_id_has_dword_io at all, since it > would only make a difference on controllers where we don't really want to > use it anyway. > > It seems like regardless of whether we do 32-bit by default or not the 32-bit > data_xfer function should be added to libata core as we have several drivers > which duplicate the same code currently.. Great, just some minor nitpicks as I don't have much idea about 16 bit ones. > +unsigned int ata_sff_data_xfer(struct ata_device *dev, unsigned char *buf, > + unsigned int buflen, int rw) > +{ > + struct ata_port *ap = dev->link->ap; > + void __iomem *data_addr = ap->ioaddr.data_addr; > + unsigned int words = buflen >> 2; dwords maybe? > + unsigned int slop = buflen & 3; > + > + /* Transfer multiple of 4 bytes */ > + if (rw == READ) > + ioread32_rep(data_addr, buf, words); > + else > + iowrite32_rep(data_addr, buf, words); > + > + /* Transfer trailing 1 byte, if any. */ 1byte? Thanks. -- tejun ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-06 0:44 ` Tejun Heo @ 2008-08-06 2:30 ` Robert Hancock 2008-08-06 11:27 ` Sergei Shtylyov 1 sibling, 0 replies; 21+ messages in thread From: Robert Hancock @ 2008-08-06 2:30 UTC (permalink / raw) To: Tejun Heo Cc: Alan Cox, Bartlomiej Zolnierkiewicz, James Bottomley, linux-kernel, linux-ide, Jeff Garzik Tejun Heo wrote: > Robert Hancock wrote: >> Here's my first cut at it. Compile tested only. This sets most controllers >> to use 32-bit PIO except for those which could potentially be on a real ISA >> or other 16-bit bus. It's a bit non-obvious what to do with some of the >> drivers, so input is welcome. >> >> This implementation doesn't check the ata_id_has_dword_io at all, since it >> would only make a difference on controllers where we don't really want to >> use it anyway. >> >> It seems like regardless of whether we do 32-bit by default or not the 32-bit >> data_xfer function should be added to libata core as we have several drivers >> which duplicate the same code currently.. > > Great, just some minor nitpicks as I don't have much idea about 16 bit ones. > >> +unsigned int ata_sff_data_xfer(struct ata_device *dev, unsigned char *buf, >> + unsigned int buflen, int rw) >> +{ >> + struct ata_port *ap = dev->link->ap; >> + void __iomem *data_addr = ap->ioaddr.data_addr; >> + unsigned int words = buflen >> 2; > > dwords maybe? > >> + unsigned int slop = buflen & 3; >> + >> + /* Transfer multiple of 4 bytes */ >> + if (rw == READ) >> + ioread32_rep(data_addr, buf, words); >> + else >> + iowrite32_rep(data_addr, buf, words); >> + >> + /* Transfer trailing 1 byte, if any. */ > > 1byte? Yeah, those are both leftovers from the 16-bit code. I'll fix them up in the next version, if the approach looks good.. > > Thanks. > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-06 0:44 ` Tejun Heo 2008-08-06 2:30 ` Robert Hancock @ 2008-08-06 11:27 ` Sergei Shtylyov 2008-08-06 13:04 ` [Ksummit-2008-discuss] " Tejun Heo 1 sibling, 1 reply; 21+ messages in thread From: Sergei Shtylyov @ 2008-08-06 11:27 UTC (permalink / raw) To: Tejun Heo Cc: Robert Hancock, Alan Cox, Bartlomiej Zolnierkiewicz, James Bottomley, ksummit-2008-discuss, linux-kernel, linux-ide, Jeff Garzik Hello. Tejun Heo wrote: >> +unsigned int ata_sff_data_xfer(struct ata_device *dev, unsigned char *buf, >> + unsigned int buflen, int rw) >> +{ >> + struct ata_port *ap = dev->link->ap; >> + void __iomem *data_addr = ap->ioaddr.data_addr; >> + unsigned int words = buflen >> 2; >> > > dwords maybe? > Isn't word 32-bit in the 32-bit systems? We're not on 80[12]86 (despite Intel/MS preference of calling 32-bit memory cells DWORDS)... MBR, Sergei ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Ksummit-2008-discuss] Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-06 11:27 ` Sergei Shtylyov @ 2008-08-06 13:04 ` Tejun Heo 0 siblings, 0 replies; 21+ messages in thread From: Tejun Heo @ 2008-08-06 13:04 UTC (permalink / raw) To: Sergei Shtylyov Cc: Robert Hancock, Jeff Garzik, Bartlomiej Zolnierkiewicz, ksummit-2008-discuss, linux-kernel, James Bottomley, linux-ide, Alan Cox Sergei Shtylyov wrote: > Hello. > > Tejun Heo wrote: >>> +unsigned int ata_sff_data_xfer(struct ata_device *dev, unsigned char *buf, >>> + unsigned int buflen, int rw) >>> +{ >>> + struct ata_port *ap = dev->link->ap; >>> + void __iomem *data_addr = ap->ioaddr.data_addr; >>> + unsigned int words = buflen >> 2; >>> >> dwords maybe? >> > > Isn't word 32-bit in the 32-bit systems? We're not on 80[12]86 > (despite Intel/MS preference of calling 32-bit memory cells DWORDS)... Well, as most of machines are 64-bit these days, discussing whether a word means 16 or 32 bits is kind of moot. I usually just think byte, word, dword, qword and and libata and pci assume that too - ata_id_has_dword_io() and PCI configuration accessors. It's ultimately a peripheral issue either way tho. Thanks. -- tejun ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-06 0:21 ` Robert Hancock 2008-08-06 0:44 ` Tejun Heo @ 2008-08-06 8:51 ` Alan Cox 1 sibling, 0 replies; 21+ messages in thread From: Alan Cox @ 2008-08-06 8:51 UTC (permalink / raw) To: Robert Hancock Cc: Bartlomiej Zolnierkiewicz, James Bottomley, ksummit-2008-discuss, linux-kernel, linux-ide, Jeff Garzik > Here's my first cut at it. Compile tested only. This sets most controllers > to use 32-bit PIO except for those which could potentially be on a real ISA > or other 16-bit bus. It's a bit non-obvious what to do with some of the > drivers, so input is welcome. Only thing I would do differently and definitely want to do differently is the assumption that we just turn on 32bit and see what occurs. We need to read/test/change controller by controller over to 32bit not just hope. Alan ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-04 19:55 ` Alan Cox 2008-08-04 21:17 ` Robert Hancock @ 2008-08-04 21:55 ` Sergei Shtylyov 2008-08-04 21:43 ` Alan Cox 2008-08-04 22:12 ` Mark Lord 2 siblings, 1 reply; 21+ messages in thread From: Sergei Shtylyov @ 2008-08-04 21:55 UTC (permalink / raw) To: Alan Cox Cc: Robert Hancock, Bartlomiej Zolnierkiewicz, James Bottomley, ksummit-2008-discuss, linux-kernel, linux-ide Hello. Alan Cox wrote: >> I was looking into the 32-bit PIO issue a bit yesterday. It looks like >> some of the VLB libata drivers are doing this internally already, so it >> shouldn't be hard to do this in the core. Only question is how we know >> generically if the controller can do it or not? It looks like in old >> > > You don't. Basically it is controller dependant. Pretty much all the > newer controllers support the 32bit PIO data cycles. Most PCI controllers > it makes no speed difference but host bus controllers (especially > PIIX/ICH) really benefit. > In what way if there's no speed gain? >> supported. I couldn't track down where that bit was actually defined in >> the first place, all the way back to ATA-1 it seems to be indicated as >> reserved. Actually, I'm not sure why the drive cares in the first place, >> it would seem like a pure host controller issue.. >> > > It goes back before IDE into the depths of the original compaq spec. When > you have a device wired basically directly to the ISA bus (original IDE) > ISA has only 8/16-bit data bus, so it could not have mattered there... and I don't think that there ever was a direct connection between the IDE and host bus other than ISA... except maybe EISA. MBR, Sergei ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-04 21:55 ` Sergei Shtylyov @ 2008-08-04 21:43 ` Alan Cox 2008-08-04 22:45 ` Sergei Shtylyov 0 siblings, 1 reply; 21+ messages in thread From: Alan Cox @ 2008-08-04 21:43 UTC (permalink / raw) To: Sergei Shtylyov Cc: Robert Hancock, Bartlomiej Zolnierkiewicz, James Bottomley, ksummit-2008-discuss, linux-kernel, linux-ide > > newer controllers support the 32bit PIO data cycles. Most PCI controllers > > it makes no speed difference but host bus controllers (especially > > PIIX/ICH) really benefit. > > > > In what way if there's no speed gain? As in the numbers are the same before and after. The FIFO on the controller is happily hiding the extra latencies I assume. > >> supported. I couldn't track down where that bit was actually defined in > >> the first place, all the way back to ATA-1 it seems to be indicated as > >> reserved. Actually, I'm not sure why the drive cares in the first place, > >> it would seem like a pure host controller issue.. > >> > > > > It goes back before IDE into the depths of the original compaq spec. When > > you have a device wired basically directly to the ISA bus (original IDE) > > > > ISA has only 8/16-bit data bus, so it could not have mattered > there... Depends what a 32bit I/O looks like on the 16bit bus - timing wise. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-04 21:43 ` Alan Cox @ 2008-08-04 22:45 ` Sergei Shtylyov 2008-08-04 22:52 ` Sergei Shtylyov 0 siblings, 1 reply; 21+ messages in thread From: Sergei Shtylyov @ 2008-08-04 22:45 UTC (permalink / raw) To: Alan Cox Cc: Robert Hancock, Bartlomiej Zolnierkiewicz, James Bottomley, ksummit-2008-discuss, linux-kernel, linux-ide Hello. Alan Cox wrote: >>> newer controllers support the 32bit PIO data cycles. Most PCI controllers >>> it makes no speed difference but host bus controllers (especially >>> PIIX/ICH) really benefit. >>> PIIX was a pure PCI controller, IIRC. ICH is also not a "host bus" controller, it hangs off the I/O hub bus... >>> >>> >> In what way if there's no speed gain? >> > > As in the numbers are the same before and after. The FIFO on the > controller is happily hiding the extra latencies I assume. > Depends on the PIO mode -- in the lower ones, the prefetch reads might really be slower than successive reads on the host bus... >>>> supported. I couldn't track down where that bit was actually defined in >>>> the first place, all the way back to ATA-1 it seems to be indicated as >>>> reserved. Actually, I'm not sure why the drive cares in the first place, >>>> it would seem like a pure host controller issue.. >>>> >>>> >>> It goes back before IDE into the depths of the original compaq spec. When >>> you have a device wired basically directly to the ISA bus (original IDE) >>> >>> >> ISA has only 8/16-bit data bus, so it could not have mattered >> there... >> > > Depends what a 32bit I/O looks like on the 16bit bus - timing wise. > Two 16-bit reads at addresses 0x1x0 and 0x1x2 with the programmed recovery time, IIRC... It's just occured to me that in case of the 16-bit bus it should be how the drive treated the accesses at address 0x1x2 with IOCS16 asserted that could have mattered. If it honored them, 32-bit I/O could have worked even on a dumb ISA "controller", if not -- no way (unless you really had *something* between the ISA and the IDE cable). MBR, Sergei ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-04 22:45 ` Sergei Shtylyov @ 2008-08-04 22:52 ` Sergei Shtylyov 2008-08-06 11:17 ` Sergei Shtylyov 0 siblings, 1 reply; 21+ messages in thread From: Sergei Shtylyov @ 2008-08-04 22:52 UTC (permalink / raw) To: Alan Cox Cc: Robert Hancock, Bartlomiej Zolnierkiewicz, James Bottomley, ksummit-2008-discuss, linux-kernel, linux-ide Hello, I wrote: >>>>> supported. I couldn't track down where that bit was actually >>>>> defined in the first place, all the way back to ATA-1 it seems to >>>>> be indicated as reserved. Actually, I'm not sure why the drive >>>>> cares in the first place, it would seem like a pure host >>>>> controller issue.. >>>>> >>>> It goes back before IDE into the depths of the original compaq >>>> spec. When >>>> you have a device wired basically directly to the ISA bus (original >>>> IDE) >>>> >>> ISA has only 8/16-bit data bus, so it could not have mattered >>> there... >> >> Depends what a 32bit I/O looks like on the 16bit bus - timing wise. >> > > Two 16-bit reads at addresses 0x1x0 and 0x1x2 with the programmed > recovery time, IIRC... It's just occured to me that in case of the > 16-bit bus it should be how the drive treated the accesses at address > 0x1x2 with IOCS16 asserted that could have mattered. If it honored > them, 32-bit I/O could have worked even on a dumb ISA "controller", if > not -- no way (unless you really had *something* between the ISA and > the IDE cable). Oh, -IOCS16 is driven by device, not host. I give up then. :-) MBR, Sergei ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-04 22:52 ` Sergei Shtylyov @ 2008-08-06 11:17 ` Sergei Shtylyov 0 siblings, 0 replies; 21+ messages in thread From: Sergei Shtylyov @ 2008-08-06 11:17 UTC (permalink / raw) To: Sergei Shtylyov Cc: Alan Cox, Robert Hancock, Bartlomiej Zolnierkiewicz, James Bottomley, ksummit-2008-discuss, linux-kernel, linux-ide Hello, I wrote: >>>>>> supported. I couldn't track down where that bit was actually >>>>>> defined in the first place, all the way back to ATA-1 it seems to >>>>>> be indicated as reserved. Actually, I'm not sure why the drive >>>>>> cares in the first place, it would seem like a pure host >>>>>> controller issue.. >>>>>> >>>>> It goes back before IDE into the depths of the original compaq >>>>> spec. When >>>>> you have a device wired basically directly to the ISA bus >>>>> (original IDE) >>>>> >>>> ISA has only 8/16-bit data bus, so it could not have mattered >>>> there... >>> >>> Depends what a 32bit I/O looks like on the 16bit bus - timing wise. >>> >> >> Two 16-bit reads at addresses 0x1x0 and 0x1x2 with the programmed >> recovery time, IIRC... It's just occured to me that in case of the >> 16-bit bus it should be how the drive treated the accesses at address >> 0x1x2 with IOCS16 asserted that could have mattered. If it honored >> them, 32-bit I/O could have worked even on a dumb ISA "controller", >> if not -- no way (unless you really had *something* between the ISA >> and the IDE cable). > > Oh, -IOCS16 is driven by device, not host. I give up then. :-) > OTOH, it definitely could work if the drive asserted it for the I/O port 0x1x2 at least for the data transfer phase (and probably even if it always asserted -IOCS16 for this address). That pre-historic word indeed could have made sense then... MBR, Sergei ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-04 19:55 ` Alan Cox 2008-08-04 21:17 ` Robert Hancock 2008-08-04 21:55 ` Sergei Shtylyov @ 2008-08-04 22:12 ` Mark Lord 2008-08-04 22:00 ` Alan Cox 2 siblings, 1 reply; 21+ messages in thread From: Mark Lord @ 2008-08-04 22:12 UTC (permalink / raw) To: Alan Cox Cc: Robert Hancock, Bartlomiej Zolnierkiewicz, James Bottomley, ksummit-2008-discuss, linux-kernel, linux-ide Alan Cox wrote: > You don't. Basically it is controller dependant. Pretty much all the > newer controllers support the 32bit PIO data cycles. Most PCI controllers > it makes no speed difference but host bus controllers (especially > PIIX/ICH) really benefit. .. By "most PCI controllers", did you really mean "most add-on PCI (card) controllers"? Nearly all onboard "PCI" controllers (integrated into the host chipset) benefit hugely, but I suppose that is what was meant by "host bus controllers". Cheers ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-04 22:12 ` Mark Lord @ 2008-08-04 22:00 ` Alan Cox 0 siblings, 0 replies; 21+ messages in thread From: Alan Cox @ 2008-08-04 22:00 UTC (permalink / raw) To: Mark Lord Cc: Robert Hancock, Bartlomiej Zolnierkiewicz, James Bottomley, ksummit-2008-discuss, linux-kernel, linux-ide On Mon, 04 Aug 2008 18:12:51 -0400 Mark Lord <liml@rtr.ca> wrote: > Alan Cox wrote: > > You don't. Basically it is controller dependant. Pretty much all the > > newer controllers support the 32bit PIO data cycles. Most PCI controllers > > it makes no speed difference but host bus controllers (especially > > PIIX/ICH) really benefit. > .. > > By "most PCI controllers", did you really mean "most add-on PCI (card) controllers"? Yes > Nearly all onboard "PCI" controllers (integrated into the host chipset) benefit hugely, > but I suppose that is what was meant by "host bus controllers". Yes ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-04 20:07 ` Kernel Summit request for Discussion of future of ATA (libata) and IDE Robert Hancock 2008-08-04 19:55 ` Alan Cox @ 2008-08-04 20:37 ` Jeff Garzik 2008-08-04 23:49 ` [Ksummit-2008-discuss] " Tejun Heo 1 sibling, 1 reply; 21+ messages in thread From: Jeff Garzik @ 2008-08-04 20:37 UTC (permalink / raw) To: Robert Hancock Cc: Alan Cox, Bartlomiej Zolnierkiewicz, James Bottomley, ksummit-2008-discuss, linux-kernel, linux-ide Robert Hancock wrote: > Alan Cox wrote: >>> * There are still corner case in libata core - PIO is dead slow >>> compared to drivers/ide/, >> >> There are two there - libata keeps IRQs blocked for longer in PIO mode as >> well which is a factor for realtime that needs looking at, as well as >> using 16bit not 32bit I/O for most devices (which is trivial to fix). The >> IRQ masking stuff is more complex and old IDE handles it far better for >> PIO on non shared IRQ interfaces. That is actually probably the most >> complicated thing to address of the stuff you'd want to do if you were >> going to kill off old IDE. > > I was looking into the 32-bit PIO issue a bit yesterday. It looks like > some of the VLB libata drivers are doing this internally already, so it > shouldn't be hard to do this in the core. Only question is how we know > generically if the controller can do it or not? It looks like in old > IDE, a few controllers explicitly disable it, but it appears that it > doesn't default to on for any controller, so it's possible there are > others on which it doesn't work. Presumably anything on an actual 16-bit > bus (ISA, LPC, etc.) wouldn't like it, to start with. FWIW there is already a patch from Willy Terreau (sp?) to add 32-bit I/O. I queued it for "later" because it had some issues that Alan pointed out, IIRC. I definitely want to push it in, though. Jeff ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Ksummit-2008-discuss] Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-04 20:37 ` Jeff Garzik @ 2008-08-04 23:49 ` Tejun Heo 0 siblings, 0 replies; 21+ messages in thread From: Tejun Heo @ 2008-08-04 23:49 UTC (permalink / raw) To: Jeff Garzik Cc: Robert Hancock, Bartlomiej Zolnierkiewicz, ksummit-2008-discuss, linux-kernel, James Bottomley, linux-ide, Alan Cox Jeff Garzik wrote: > Robert Hancock wrote: >> Alan Cox wrote: >>>> * There are still corner case in libata core - PIO is dead slow >>>> compared to drivers/ide/, >>> There are two there - libata keeps IRQs blocked for longer in PIO mode as >>> well which is a factor for realtime that needs looking at, as well as >>> using 16bit not 32bit I/O for most devices (which is trivial to fix). The >>> IRQ masking stuff is more complex and old IDE handles it far better for >>> PIO on non shared IRQ interfaces. That is actually probably the most >>> complicated thing to address of the stuff you'd want to do if you were >>> going to kill off old IDE. >> I was looking into the 32-bit PIO issue a bit yesterday. It looks like >> some of the VLB libata drivers are doing this internally already, so it >> shouldn't be hard to do this in the core. Only question is how we know >> generically if the controller can do it or not? It looks like in old >> IDE, a few controllers explicitly disable it, but it appears that it >> doesn't default to on for any controller, so it's possible there are >> others on which it doesn't work. Presumably anything on an actual 16-bit >> bus (ISA, LPC, etc.) wouldn't like it, to start with. > > FWIW there is already a patch from Willy Terreau (sp?) to add 32-bit I/O. > > I queued it for "later" because it had some issues that Alan pointed > out, IIRC. I definitely want to push it in, though. Jeff, have your thoughts about PIO IRQ disable handling changed yet? I don't really see any better way than doing it like IDE. Thanks. -- tejun ^ permalink raw reply [flat|nested] 21+ messages in thread
* Kernel Summit request for Discussion of future of ATA (libata) and IDE @ 2008-08-03 15:57 James Bottomley 2008-08-03 16:39 ` Alan Cox 0 siblings, 1 reply; 21+ messages in thread From: James Bottomley @ 2008-08-03 15:57 UTC (permalink / raw) To: ksummit-2008-discuss, linux-kernel, linux-ide Right at the moment, we have two separate subsystems for running IDE type devices: driver/ide and drivers/ata. The claim I've seen is that drivers/ata can do everything drivers/ide can do plus it does sata. I also note that no major distribution seems to enable anything in drivers/ide anymore, so given this is it time to deprecate drivers/ide? A counter argument to the above is that not all drivers (particularly the older ones where hw is scarce) are converted to drivers/ata, so drivers/ide seems to be needed for some legacy systems (in which case it can be deprecated but not removed). I've also noted that some embedded distributions seem to be using drivers/ide, but I'm not really sure whether this is inertia or some overriding need. The proposal is to discuss the future of these two subsystems and arrive at a consensus what's happening to each going forwards. James ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-03 15:57 James Bottomley @ 2008-08-03 16:39 ` Alan Cox 2008-08-03 17:32 ` Willy Tarreau 0 siblings, 1 reply; 21+ messages in thread From: Alan Cox @ 2008-08-03 16:39 UTC (permalink / raw) To: James Bottomley; +Cc: ksummit-2008-discuss, linux-kernel, linux-ide On Sun, 03 Aug 2008 10:57:35 -0500 James Bottomley <James.Bottomley@HansenPartnership.com> wrote: > Right at the moment, we have two separate subsystems for running IDE > type devices: driver/ide and drivers/ata. The claim I've seen is that > drivers/ata can do everything drivers/ide can do plus it does sata. I > also note that no major distribution seems to enable anything in > drivers/ide anymore, so given this is it time to deprecate drivers/ide? > > A counter argument to the above is that not all drivers (particularly > the older ones where hw is scarce) are converted to drivers/ata, so That statement would be false. In fact for old and obscure hw the libata coverage is probably better, not that this in fact is the slighest bit relevant to the real world! The current situation is something like this - For PC class hardware libata covers everything in old IDE and a lot more. - For the older Mac hardware libata does not have pre PCI Macintosh support although it is in progress - For certain embedded PPC boards there are some things to sort out with IRQ routing on non standards sane configurations using pata_ali in particular. There is a trend in new hardware to interfaces that simply won't fit old IDE so that process of libata only drivers is likely to continue. > drivers/ide seems to be needed for some legacy systems (in which case it > can be deprecated but not removed). I've also noted that some embedded > distributions seem to be using drivers/ide, but I'm not really sure > whether this is inertia or some overriding need. Actually what a material number of embedded systems need is a single dumb-as-a-rock CF only PIO driver which doesn't suck in large chunks of midlayer code. I'm just not sure that trend will continue as CF is giving way to other smaller media. > The proposal is to discuss the future of these two subsystems and arrive > at a consensus what's happening to each going forwards. Won't be at KS and I suspect that is true of most of the folks hacking on both sets so the right place for discussion would be the net. Right now the PPC bits need fixing, and the lack of libata Macio support for the moment makes the debate premature. Alan ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-03 16:39 ` Alan Cox @ 2008-08-03 17:32 ` Willy Tarreau 2008-08-03 17:45 ` Rafael J. Wysocki 0 siblings, 1 reply; 21+ messages in thread From: Willy Tarreau @ 2008-08-03 17:32 UTC (permalink / raw) To: Alan Cox; +Cc: James Bottomley, ksummit-2008-discuss, linux-kernel, linux-ide On Sun, Aug 03, 2008 at 05:39:41PM +0100, Alan Cox wrote: > Actually what a material number of embedded systems need is a single > dumb-as-a-rock CF only PIO driver which doesn't suck in large chunks of > midlayer code. I'm just not sure that trend will continue as CF is giving > way to other smaller media. I bet we'll see CF for a long time, because it is the only cheap removable media to support IDE emulation and be bootable from an unmodified PC BIOS. SD is cheaper and will soon provide larger capacities, but it still requires a separate controller and is not bootable with standard BIOSes. Anyway, in my experience, libata supports CF on embedded controllers pretty well. So maybe we should start marking IDE drivers deprecated to encourage people to try libata instead and report breakage if any. The real issue on embedded systems is that the devices names change. On the one hand, it increases portability across models, because you don't have to care anymore about hda/hdc/hdd, your CF is always on sda (major reason why I'm considering a switch BTW). However, for systems which were used to always see system disk on hdX and data on sd* (USB, SATA or SCSI), it might take more time to switch due to automatic numbering. Regards, Willy ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-03 17:32 ` Willy Tarreau @ 2008-08-03 17:45 ` Rafael J. Wysocki 2008-08-03 20:22 ` [Ksummit-2008-discuss] " David Woodhouse 0 siblings, 1 reply; 21+ messages in thread From: Rafael J. Wysocki @ 2008-08-03 17:45 UTC (permalink / raw) To: Willy Tarreau Cc: Alan Cox, James Bottomley, ksummit-2008-discuss, linux-kernel, linux-ide On Sunday, 3 of August 2008, Willy Tarreau wrote: > On Sun, Aug 03, 2008 at 05:39:41PM +0100, Alan Cox wrote: > > Actually what a material number of embedded systems need is a single > > dumb-as-a-rock CF only PIO driver which doesn't suck in large chunks of > > midlayer code. I'm just not sure that trend will continue as CF is giving > > way to other smaller media. > > I bet we'll see CF for a long time, because it is the only cheap > removable media to support IDE emulation and be bootable from an > unmodified PC BIOS. SD is cheaper and will soon provide larger > capacities, but it still requires a separate controller and is > not bootable with standard BIOSes. > > Anyway, in my experience, libata supports CF on embedded controllers > pretty well. So maybe we should start marking IDE drivers deprecated > to encourage people to try libata instead and report breakage if any. Well, I know of at least one case of PC hardware in which an old IDE driver basically works while the libata/pata poeple have no idea why their driver doesn't: http://bugzilla.kernel.org/show_bug.cgi?id=9157 Thanks, Rafael ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Ksummit-2008-discuss] Kernel Summit request for Discussion of future of ATA (libata) and IDE 2008-08-03 17:45 ` Rafael J. Wysocki @ 2008-08-03 20:22 ` David Woodhouse 0 siblings, 0 replies; 21+ messages in thread From: David Woodhouse @ 2008-08-03 20:22 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Willy Tarreau, James Bottomley, ksummit-2008-discuss, linux-kernel, Alan Cox, linux-ide On Sun, 2008-08-03 at 19:45 +0200, Rafael J. Wysocki wrote: > Well, I know of at least one case of PC hardware in which an old IDE driver > basically works while the libata/pata poeple have no idea why their driver > doesn't: I have one of those too (pdc202xx craps itself with the new drivers). -- dwmw2 ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2008-08-06 13:05 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <fa.OGeO7gZvBG4obEzRbVltjSebgTQ@ifi.uio.no>
[not found] ` <fa.KtvYE2B2yrJqUleolhtMPN9ljAQ@ifi.uio.no>
[not found] ` <fa.AqEKTvguYFAzDFHy/We/8MpOqmo@ifi.uio.no>
2008-08-04 20:07 ` Kernel Summit request for Discussion of future of ATA (libata) and IDE Robert Hancock
2008-08-04 19:55 ` Alan Cox
2008-08-04 21:17 ` Robert Hancock
2008-08-04 21:06 ` Alan Cox
2008-08-04 21:48 ` Robert Hancock
2008-08-06 0:21 ` Robert Hancock
2008-08-06 0:44 ` Tejun Heo
2008-08-06 2:30 ` Robert Hancock
2008-08-06 11:27 ` Sergei Shtylyov
2008-08-06 13:04 ` [Ksummit-2008-discuss] " Tejun Heo
2008-08-06 8:51 ` Alan Cox
2008-08-04 21:55 ` Sergei Shtylyov
2008-08-04 21:43 ` Alan Cox
2008-08-04 22:45 ` Sergei Shtylyov
2008-08-04 22:52 ` Sergei Shtylyov
2008-08-06 11:17 ` Sergei Shtylyov
2008-08-04 22:12 ` Mark Lord
2008-08-04 22:00 ` Alan Cox
2008-08-04 20:37 ` Jeff Garzik
2008-08-04 23:49 ` [Ksummit-2008-discuss] " Tejun Heo
2008-08-03 15:57 James Bottomley
2008-08-03 16:39 ` Alan Cox
2008-08-03 17:32 ` Willy Tarreau
2008-08-03 17:45 ` Rafael J. Wysocki
2008-08-03 20:22 ` [Ksummit-2008-discuss] " David Woodhouse
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).