libata: implement per-dev xfer masks, take#2
PATCH 1/1 libata: implement per-dev xfer masks, take#2
Hello, all.
This is the second take of add-per-dev-pio-mwdma-udma_mask patchset. Changes from the last take[1] are...
* %02 implement-ata_dev_init patch is removed and %03 is updated accordingly.
The patchset is against the current #upstream[2].
I tried to think of a simple way to use single xfer_mask for all masks, but couldn't. The problem is that the masks are used in several different ways affecting specific parts of the mask. When composing transfer xfer_mask, it's best if they can be simply bit-wise or'ed, but we can't use the same mask values when trying to limit trasnfer mode because bits for other transfer modes will be turned off. We're gonna have to use different masks for different purposes or use mask manipulating wrappers.
So, for the time being, LLDD's are left with three separate masks. If anyone can think of a pretty way to convert them, please, go ahead. We also need to sort out transfer mask constants, the naming is somewhat inconsistent.
Thanks.
--
tejun
[1] http://article.gmane.org/gmane.linux.ide/8705
[2] 2cc432eed0491df66e14b578139bba2c75fb3f9a
PATCH 1/3 libata: implement ata_unpack_xfermask()
Implement ata_unpack_xfermask().
---
drivers/scsi/libata-core.c | 23 +++++++++++++++++++++++ 1 files changed, 23 insertions(+), 0 deletions(-)
6ff994d5a30bdaef8a055301f569b781307801a7
drivers/scsi/libata-core.c +23 -0
@@ -252,6 +252,29 @@ static unsigned int ata_pack_xfermask(un ((udma_mask <<ATA_SHIFT_UDMA ) &ATA_MASK_UDMA ); } +/** +* ata_unpack_xfermask - Unpack xfer_mask into pio, mwdma and udma masks +* @xfer_mask: xfer_mask to unpack +* @pio_mask: resulting pio_mask +* @mwdma_mask: resulting mwdma_mask +* @udma_mask: resulting udma_mask +* +* Unpack @xfer_mask into @pio_mask, @mwdma_mask and @udma_mask. +* Any NULL distination masks will be ignored. +*/ +static void ata_unpack_xfermask (unsigned int xfer_mask, +unsigned int *pio_mask, +unsigned int *mwdma_mask, +unsigned int *udma_mask) +{ +if (pio_mask) + *pio_mask = (xfer_mask &ATA_MASK_PIO ) >>ATA_SHIFT_PIO ; +if (mwdma_mask) + *mwdma_mask = (xfer_mask &ATA_MASK_MWDMA ) >>ATA_SHIFT_MWDMA ; +if (udma_mask) + *udma_mask = (xfer_mask &ATA_MASK_UDMA ) >>ATA_SHIFT_UDMA ; +} + static const struct ata_xfer_ent { unsigned int shift, bits; u8 base;
Signature · 1 line
1.2.4
PATCH 2/3 libata: add per-dev pio/mwdma/udma_mask
In reply to Tejun HeoAdd per-dev pio/mwdma/udma_mask. All transfer mode limits used to be applied to ap->*_mask which unnecessarily restricted other devices sharing the port. This change will also benefit later EH speed down and hotplug.
---
drivers/scsi/libata-core.c | 43 +++++++++++++++++++++++++------------------ include/linux/libata.h | 5 +++++ 2 files changed, 30 insertions(+), 18 deletions(-)
31c02ad0eeb7c73243a109a75e41b09511c1576f
drivers/scsi/libata-core.c +25 -18
@@ -65,8 +65,7 @@ static unsigned int ata_dev_init_params( struct ata_device *dev);static void ata_set_mode (struct ata_port *ap);static void ata_dev_set_xfermode (struct ata_port *ap,struct ata_device *dev); -static unsigned int ata_dev_xfermask (struct ata_port *ap, -struct ata_device *dev); +static void ata_dev_xfermask (struct ata_port *ap,struct ata_device *dev);static unsigned int ata_unique_id =1 ;static struct workqueue_struct *ata_wq;
@@ -1801,16 +1800,19 @@ static void ata_set_mode(struct ata_port/* step 1: calculate xfer_mask */ for (i =0 ; i <ATA_MAX_DEVICES ; i++) {struct ata_device *dev = &ap->device[i]; -unsigned int xfer_mask; +unsigned int pio_mask, dma_mask;if (!ata_dev_present (dev))continue ; - xfer_mask =ata_dev_xfermask (ap, dev); +ata_dev_xfermask (ap, dev); - dev->pio_mode =ata_xfer_mask2mode (xfer_mask &ATA_MASK_PIO ); - dev->dma_mode =ata_xfer_mask2mode (xfer_mask & (ATA_MASK_MWDMA | -ATA_MASK_UDMA )); +/* TODO: let LLDD filter dev->*_mask here */ + + pio_mask =ata_pack_xfermask (dev->pio_mask,0 ,0 ); + dma_mask =ata_pack_xfermask (0 , dev->mwdma_mask, dev->udma_mask); + dev->pio_mode =ata_xfer_mask2mode (pio_mask); + dev->dma_mode =ata_xfer_mask2mode (dma_mask); }/* step 2: always set host PIO timings */
@@ -2653,18 +2655,15 @@ static int ata_dma_blacklisted(const str* @ap: Port on which the device to compute xfermask for resides * @dev: Device to compute xfermask for * -* Compute supported xfermask of @dev. This function is -* responsible for applying all known limits including host -* controller limits, device blacklist, etc... +* Compute supported xfermask of @dev and store it in +* dev->*_mask. This function is responsible for applying all +* known limits including host controller limits, device +* blacklist, etc... * * LOCKING: * None. -* -* RETURNS: -* Computed xfermask. */ -static unsigned int ata_dev_xfermask (struct ata_port *ap, -struct ata_device *dev) +static void ata_dev_xfermask (struct ata_port *ap,struct ata_device *dev) {unsigned long xfer_mask;int i;
@@ -2677,6 +2676,8 @@ static unsigned int ata_dev_xfermask(strstruct ata_device *d = &ap->device[i];if (!ata_dev_present (d))continue ; + xfer_mask &=ata_pack_xfermask (d->pio_mask, d->mwdma_mask, + d->udma_mask); xfer_mask &=ata_id_xfermask (d->id);if (ata_dma_blacklisted (d)) xfer_mask &= ~(ATA_MASK_MWDMA |ATA_MASK_UDMA );
@@ -2686,7 +2687,8 @@ static unsigned int ata_dev_xfermask(strprintk (KERN_WARNING "ata%u: dev %u is on DMA blacklist, " "disabling DMA\n" , ap->id, dev->devno); -return xfer_mask; +ata_unpack_xfermask (xfer_mask, &dev->pio_mask, &dev->mwdma_mask, + &dev->udma_mask); }/**
@@ -4436,8 +4438,13 @@ static void ata_host_init(struct ata_porINIT_WORK (&ap->port_task,NULL ,NULL );INIT_LIST_HEAD (&ap->eh_done_q); -for (i =0 ; i <ATA_MAX_DEVICES ; i++) - ap->device[i].devno = i; +for (i =0 ; i <ATA_MAX_DEVICES ; i++) { +struct ata_device *dev = &ap->device[i]; + dev->devno = i; + dev->pio_mask =UINT_MAX ; + dev->mwdma_mask =UINT_MAX ; + dev->udma_mask =UINT_MAX ; + }#ifdef ATA_IRQ_TRAP ap->stats.unhandled_irq =1 ;
include/linux/libata.h +5 -0
@@ -358,6 +358,11 @@ struct ata_device {unsigned int max_sectors;/* per-device max sectors */ unsigned int cdb_len; +/* per-dev xfer mask */ +unsigned int pio_mask; +unsigned int mwdma_mask; +unsigned int udma_mask; +/* for CHS addressing */ u16 cylinders;/* Number of cylinders */ u16 heads;/* Number of heads */
Signature · 1 line
1.2.4
Re: [PATCH 2/3] libata: add per-dev pio/mwdma/udma_mask
Quoted 16 lines · Tejun Heo
drivers/scsi/libata-core.c +3
-2-static unsigned int ata_dev_xfermask (struct ata_port *ap, -struct ata_device *dev) +static void ata_dev_xfermask (struct ata_port *ap,struct ata_device *dev) {unsigned long xfer_mask;int i;@@ -2677,6 +2676,8 @@ static unsigned int ata_dev_xfermask(strstruct ata_device *d = &ap->device[i];if (!ata_dev_present (d))continue ; + xfer_mask &=ata_pack_xfermask (d->pio_mask, d->mwdma_mask, + d->udma_mask); xfer_mask &=ata_id_xfermask (d->id);if (ata_dma_blacklisted (d)) xfer_mask &= ~(ATA_MASK_MWDMA |ATA_MASK_UDMA );
drivers/scsi/libata-core.c static unsigned int ata_dev_xfermask(str · line 2691
Quoted 6 lines
drivers/scsi/libata-core.c +2
-1@@ -2686,7 +2687,8 @@ static unsigned int ata_dev_xfermask(strprintk (KERN_WARNING "ata%u: dev %u is on DMA blacklist, " "disabling DMA\n" , ap->id, dev->devno); -return xfer_mask; +ata_unpack_xfermask (xfer_mask, &dev->pio_mask, &dev->mwdma_mask, + &dev->udma_mask);
applied 1-3. You should add a TODO note somewhere, that notes that the current implementation intentionally limits all devices on a single port to the fastest mode on the slowest device. This is necessary for SOME controllers but not all, and so we shouldn't do it unconditionally.
Jeff
PATCH 1/1 libata: add FIXME above ata_dev_xfermask()
Add FIXME above ata_dev_xfermask noting that the current implementation limits all transfer modes to the fastest of the slowest device on a port which isn't necessary.
drivers/scsi/libata-core.c +4 -0
@@ -2634,6 +2634,10 @@ static int ata_dma_blacklisted(const str* known limits including host controller limits, device * blacklist, etc... * +* FIXME: The current implementation limits all transfer modes to +* the fastest of the lowested device on the port. This is not +* required on most controllers. +* * LOCKING: * None. */
Jeff Garzik
AppliedRe: [PATCH] libata: add FIXME above ata_dev_xfermask()
Quoted 11 lines · Tejun Heo
Add FIXME above
ata_dev_xfermasknoting that the current implementation limits all transfer modes to the fastest of the slowest device on a port which isn't necessary.Signed-off-by Tejun Heo <[email protected]>drivers/scsi/libata-core.c +0
-0
drivers/scsi/libata-core.c static int ata_dma_blacklisted(const str · line 2639
Quoted 6 lines
drivers/scsi/libata-core.c +3
-0@@ -2634,6 +2634,10 @@ static int ata_dma_blacklisted(const str* known limits including host controller limits, device * blacklist, etc... * +* FIXME: The current implementation limits all transfer modes to +* the fastest of the lowested device on the port. This is not +* required on most controllers.
applied, though git complained about...
[jgarzik@pretzel libata-dev]$ git-applymbox /g/tmp/mbox ~/info/signoff.txt 1 patch(es) to process.
Applying 'libata: add FIXME above ata_dev_xfermask()'
Adds trailing whitespace.
.dotest/patch:11: * required on most controllers.
warning: 1 line adds trailing whitespaces. Wrote tree 0fc6b51c6ddfb5d3f759bb88a34d8003386a50d7
Committed: 600511e86babe3727264a0883a3a264f6fb6caf5
PATCH 1/1 libata: kill trailing whitespace
Kill trailing whitespace.
---
Sorry about that.
drivers/scsi/libata-core.c +1 -1
@@ -2635,7 +2635,7 @@ static int ata_dma_blacklisted(const str* * FIXME: The current implementation limits all transfer modes to * the fastest of the lowested device on the port. This is not -* required on most controllers. +* required on most controllers. * * LOCKING: * None.
PATCH 3/3 libata: make per-dev transfer mode limits per-dev
In reply to Tejun HeoNow that each ata_device has xfer masks, per-dev limits can be made per-dev instead of per-port. Make per-dev limits per-dev.
---
drivers/scsi/libata-core.c | 2 +- drivers/scsi/sata_sil.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
d1e4136f124dfae288a5dd382e9d54866a355a44
drivers/scsi/libata-core.c +1 -1
@@ -1327,7 +1327,7 @@ static int ata_dev_configure(struct ata_if (print_info)printk (KERN_INFO "ata%u(%u): applying bridge limits\n" , ap->id, dev->devno); - ap->udma_mask &=ATA_UDMA5 ; + dev->udma_mask &=ATA_UDMA5 ; dev->max_sectors =ATA_MAX_SECTORS ; }
drivers/scsi/sata_sil.c +1 -1
@@ -371,7 +371,7 @@ static void sil_dev_config(struct ata_poif (quirks &SIL_QUIRK_UDMA5MAX ) {printk (KERN_INFO "ata%u(%u): applying Maxtor errata fix %s\n" , ap->id, dev->devno, model_num); - ap->udma_mask &=ATA_UDMA5 ; + dev->udma_mask &=ATA_UDMA5 ;return ; } }
Signature · 1 line
1.2.4