regarding xfer mode representation in dev, ap and other places

regarding xfer mode representation in dev, ap and other places

[Tejun Heo]

Hello, all.

Now that new reset mechanism is in tree and configuration changes are 
submitted, I'm about to send out transfer mode setting changes.  Two 
major goals of the changes are...

* Give upper layer the power to determine whether devices/ports go 
offline or not.

* Add per-device xfermode limit mask such that EH can limit transfer 
mode later.

One thing that bothers me is various representations of xfermode in 
ata_device, ata_port and other places.

* In ata_device, the current pio and dma transfer mode are recorded in 
->pio_mode and ->dma_mode using SETFEATURES XFERMODE constants.  Of the 
two, currently active mode is copied in ->xfer_mode and ->xfer_shift 
indicates which one is copied.

* In ata_port, supported transfer modes are recorded in three unsigned 
int fields, ->pio_mask, ->mwdma_mask and ->udma_mask.

* Yet another representation is used for printing supported transfer 
mode.  This is a combined mask of ->pio_mask, ->mwdma_mask and 
->udma_mask shifted by respective ATA_SHIFT_XXX.

IMHO, storing SETFEATURES XFERMODE constants and ATA_SHIFT_xxx isn't a 
very good idea.  Except for writing directly to the device, this value 
is just cumbersome to deal with.  e.g. we currently have awkward code 
involving base_from_shift() and offset arithmetics to reverse map and 
print configured transfer mode in ata_dev_set_mode().  Logical transfer 
mode -> constants is easy, the other way around, not.  Also, multiple 
transfer modes cannot be represented in this way.

Of the left two representations, IMHO, the packed one is easier to deal 
with.  It can be easily passed, returned and masked.  Storing as a 
single mask and unpacking when needed is much more convenient then 
carrying around three values, and it isn't like we're gonna need more 
than 32 bits in any foreseeable future; also, we always have u64.

So, I'm proposing to convert all transfer mode representations to one 
packed bit mask which would be u32 for now.  How does it sound?

Thanks.

-- 
tejun

[PATCH 1/3] libata: fix sata_sil24 mwdma_mask setting

[Tejun Heo]

mwdma_mask was not copied from port_info to probe_ent.  This bug is
probably introduced by following ahci.c without thinking.  Fix it.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/sata_sil24.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

bc7de87cd24b47b5480d928755dd7c08f4d43b03
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index a0e35a2..1c54736 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -891,6 +891,7 @@ static int sil24_init_one(struct pci_dev
 	probe_ent->sht		= pinfo->sht;
 	probe_ent->host_flags	= pinfo->host_flags;
 	probe_ent->pio_mask	= pinfo->pio_mask;
+	probe_ent->mwdma_mask	= pinfo->mwdma_mask;
 	probe_ent->udma_mask	= pinfo->udma_mask;
 	probe_ent->port_ops	= pinfo->port_ops;
 	probe_ent->n_ports	= SIL24_FLAG2NPORTS(pinfo->host_flags);
-- 
1.1.5

[PATCH 3/3] libata: make ata_port_info and ata_probe_ent use xfer_mask

[Tejun Heo]

Make ata_port_info and ata_probe_ent use single unsigned int xfer_mask
instead of three masks for pio, mwdma and udma.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/ahci.c         |    7 +++----
 drivers/scsi/ata_piix.c     |   37 ++++++++++++++++---------------------
 drivers/scsi/libata-bmdma.c |    4 +---
 drivers/scsi/libata-core.c  |    4 +---
 drivers/scsi/pdc_adma.c     |    8 +++-----
 drivers/scsi/sata_mv.c      |   27 +++++++++++----------------
 drivers/scsi/sata_nv.c      |    8 ++------
 drivers/scsi/sata_promise.c |   31 ++++++++++---------------------
 drivers/scsi/sata_qstor.c   |    8 +++-----
 drivers/scsi/sata_sil.c     |   19 +++++++------------
 drivers/scsi/sata_sil24.c   |   19 +++++++------------
 drivers/scsi/sata_sis.c     |    4 +---
 drivers/scsi/sata_svw.c     |    4 +---
 drivers/scsi/sata_sx4.c     |    9 +++------
 drivers/scsi/sata_uli.c     |    3 +--
 drivers/scsi/sata_via.c     |    8 ++------
 drivers/scsi/sata_vsc.c     |    4 +---
 include/linux/libata.h      |    8 ++------
 18 files changed, 75 insertions(+), 137 deletions(-)

45b6b098d03ba8570c51d20ad41804b32b0378ed
diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index 6b36345..cc6c8ea 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -252,8 +252,8 @@ static const struct ata_port_info ahci_p
 		.sht		= &ahci_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA,
-		.pio_mask	= 0x1f, /* pio0-4 */
-		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
+		.xfer_mask	= ata_pack_xfermask(0x1f, 0, 0x7f),
+				  /* pio0-4, udma0-6 ; FIXME */
 		.port_ops	= &ahci_ops,
 	},
 };
@@ -1246,8 +1246,7 @@ static int ahci_init_one (struct pci_dev
 
 	probe_ent->sht		= ahci_port_info[board_idx].sht;
 	probe_ent->host_flags	= ahci_port_info[board_idx].host_flags;
-	probe_ent->pio_mask	= ahci_port_info[board_idx].pio_mask;
-	probe_ent->udma_mask	= ahci_port_info[board_idx].udma_mask;
+	probe_ent->xfer_mask	= ahci_port_info[board_idx].xfer_mask;
 	probe_ent->port_ops	= ahci_port_info[board_idx].port_ops;
 
        	probe_ent->irq = pdev->irq;
diff --git a/drivers/scsi/ata_piix.c b/drivers/scsi/ata_piix.c
index f806772..27d3998 100644
--- a/drivers/scsi/ata_piix.c
+++ b/drivers/scsi/ata_piix.c
@@ -126,6 +126,9 @@ enum {
 	ich6_sata_ahci		= 4,
 
 	PIIX_AHCI_DEVICE	= 6,
+
+	PIIX_SATA_XFERMASK	= ata_pack_xfermask(0x1f, 0x07, 0x7f),
+				  /* pio0-4, mwdma0-2, udma0-6 */
 };
 
 static int piix_init_one (struct pci_dev *pdev,
@@ -260,13 +263,13 @@ static struct ata_port_info piix_port_in
 		.sht		= &piix_sht,
 		.host_flags	= ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST |
 				  PIIX_FLAG_CHECKINTR,
-		.pio_mask	= 0x1f,	/* pio0-4 */
+		.xfer_mask	= ata_pack_xfermask(0x1f,  /* pio0-4 */
 #if 0
-		.mwdma_mask	= 0x06, /* mwdma1-2 */
+						    0x06,  /* mwdma1-2 */
 #else
-		.mwdma_mask	= 0x00, /* mwdma broken */
+						    0x00,  /* mwdma broken */
 #endif
-		.udma_mask	= 0x3f, /* udma0-5 */
+						    0x3f), /* udma0-5 */
 		.port_ops	= &piix_pata_ops,
 	},
 
@@ -275,9 +278,7 @@ static struct ata_port_info piix_port_in
 		.sht		= &piix_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_SRST |
 				  PIIX_FLAG_COMBINED | PIIX_FLAG_CHECKINTR,
-		.pio_mask	= 0x1f,	/* pio0-4 */
-		.mwdma_mask	= 0x07, /* mwdma0-2 */
-		.udma_mask	= 0x7f,	/* udma0-6 */
+		.xfer_mask	= PIIX_SATA_XFERMASK,
 		.port_ops	= &piix_sata_ops,
 	},
 
@@ -285,13 +286,13 @@ static struct ata_port_info piix_port_in
 	{
 		.sht		= &piix_sht,
 		.host_flags	= ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST,
-		.pio_mask	= 0x1f,	/* pio0-4 */
+		.xfer_mask	= ata_pack_xfermask(0x1f,  /* pio0-4 */
 #if 0
-		.mwdma_mask	= 0x06, /* mwdma1-2 */
+						    0x06,  /* mwdma1-2 */
 #else
-		.mwdma_mask	= 0x00, /* mwdma broken */
+						    0x00,  /* mwdma broken */
 #endif
-		.udma_mask	= ATA_UDMA_MASK_40C,
+						    ATA_UDMA_MASK_40C),
 		.port_ops	= &piix_pata_ops,
 	},
 
@@ -301,9 +302,7 @@ static struct ata_port_info piix_port_in
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_SRST |
 				  PIIX_FLAG_COMBINED_ICH6 |
 				  PIIX_FLAG_CHECKINTR | ATA_FLAG_SLAVE_POSS,
-		.pio_mask	= 0x1f,	/* pio0-4 */
-		.mwdma_mask	= 0x07, /* mwdma0-2 */
-		.udma_mask	= 0x7f,	/* udma0-6 */
+		.xfer_mask	= PIIX_SATA_XFERMASK,
 		.port_ops	= &piix_sata_ops,
 	},
 
@@ -314,9 +313,7 @@ static struct ata_port_info piix_port_in
 				  PIIX_FLAG_COMBINED_ICH6 |
 				  PIIX_FLAG_CHECKINTR | ATA_FLAG_SLAVE_POSS |
 				  PIIX_FLAG_AHCI,
-		.pio_mask	= 0x1f,	/* pio0-4 */
-		.mwdma_mask	= 0x07, /* mwdma0-2 */
-		.udma_mask	= 0x7f,	/* udma0-6 */
+		.xfer_mask	= PIIX_SATA_XFERMASK,
 		.port_ops	= &piix_sata_ops,
 	},
 };
@@ -759,10 +756,8 @@ static int piix_init_one (struct pci_dev
 		/* This writes into the master table but it does not
 		   really matter for this errata as we will apply it to
 		   all the PIIX devices on the board */
-		port_info[0]->mwdma_mask = 0;
-		port_info[0]->udma_mask = 0;
-		port_info[1]->mwdma_mask = 0;
-		port_info[1]->udma_mask = 0;
+		port_info[0]->xfer_mask &= ATA_MASK_PIO;
+		port_info[1]->xfer_mask &= ATA_MASK_PIO;
 	}
 	return ata_pci_init_one(pdev, port_info, 2);
 }
diff --git a/drivers/scsi/libata-bmdma.c b/drivers/scsi/libata-bmdma.c
index a93336a..20b30b1 100644
--- a/drivers/scsi/libata-bmdma.c
+++ b/drivers/scsi/libata-bmdma.c
@@ -434,9 +434,7 @@ ata_probe_ent_alloc(struct device *dev, 
 
 	probe_ent->sht = port->sht;
 	probe_ent->host_flags = port->host_flags;
-	probe_ent->pio_mask = port->pio_mask;
-	probe_ent->mwdma_mask = port->mwdma_mask;
-	probe_ent->udma_mask = port->udma_mask;
+	probe_ent->xfer_mask = port->xfer_mask;
 	probe_ent->port_ops = port->port_ops;
 
 	return probe_ent;
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 3286df1..07bd232 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -4503,9 +4503,7 @@ static void ata_host_init(struct ata_por
 	ap->port_no = port_no;
 	ap->hard_port_no =
 		ent->legacy_mode ? ent->hard_port_no : port_no;
-	ap->xfer_mask = ata_pack_xfermask(ent->pio_mask,
-					  ent->mwdma_mask,
-					  ent->udma_mask);
+	ap->xfer_mask = ent->xfer_mask;
 	ap->flags |= ent->host_flags;
 	ap->ops = ent->port_ops;
 	ap->cbl = ATA_CBL_NONE;
diff --git a/drivers/scsi/pdc_adma.c b/drivers/scsi/pdc_adma.c
index 9f9b4d5..f4920af 100644
--- a/drivers/scsi/pdc_adma.c
+++ b/drivers/scsi/pdc_adma.c
@@ -184,8 +184,8 @@ static struct ata_port_info adma_port_in
 		.sht		= &adma_ata_sht,
 		.host_flags	= ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST |
 				  ATA_FLAG_NO_LEGACY | ATA_FLAG_MMIO,
-		.pio_mask	= 0x10, /* pio4 */
-		.udma_mask	= 0x1f, /* udma0-4 */
+		.xfer_mask	= ata_pack_xfermask(0x10, 0, 0x1f),
+				  /* pio4, udma0-4 */
 		.port_ops	= &adma_ata_ops,
 	},
 };
@@ -684,9 +684,7 @@ static int adma_ata_init_one(struct pci_
 
 	probe_ent->sht		= adma_port_info[board_idx].sht;
 	probe_ent->host_flags	= adma_port_info[board_idx].host_flags;
-	probe_ent->pio_mask	= adma_port_info[board_idx].pio_mask;
-	probe_ent->mwdma_mask	= adma_port_info[board_idx].mwdma_mask;
-	probe_ent->udma_mask	= adma_port_info[board_idx].udma_mask;
+	probe_ent->xfer_mask	= adma_port_info[board_idx].xfer_mask;
 	probe_ent->port_ops	= adma_port_info[board_idx].port_ops;
 
 	probe_ent->irq		= pdev->irq;
diff --git a/drivers/scsi/sata_mv.c b/drivers/scsi/sata_mv.c
index c158de2..e44267c 100644
--- a/drivers/scsi/sata_mv.c
+++ b/drivers/scsi/sata_mv.c
@@ -235,6 +235,9 @@ enum {
 	/* Port private flags (pp_flags) */
 	MV_PP_FLAG_EDMA_EN	= (1 << 0),
 	MV_PP_FLAG_EDMA_DS_ACT	= (1 << 1),
+
+	/* xfer_mask: pio0-4, udma0-6 */
+	MV_XFER_MASK		= ata_pack_xfermask(0x1f, 0, 0x7f),
 };
 
 #define IS_50XX(hpriv) ((hpriv)->hp_flags & MV_HP_50XX)
@@ -477,52 +480,45 @@ static const struct ata_port_info mv_por
 	{  /* chip_504x */
 		.sht		= &mv_sht,
 		.host_flags	= MV_COMMON_FLAGS,
-		.pio_mask	= 0x1f,	/* pio0-4 */
-		.udma_mask	= 0x7f,	/* udma0-6 */
+		.xfer_mask	= MV_XFER_MASK,
 		.port_ops	= &mv5_ops,
 	},
 	{  /* chip_508x */
 		.sht		= &mv_sht,
 		.host_flags	= (MV_COMMON_FLAGS | MV_FLAG_DUAL_HC),
-		.pio_mask	= 0x1f,	/* pio0-4 */
-		.udma_mask	= 0x7f,	/* udma0-6 */
+		.xfer_mask	= MV_XFER_MASK,
 		.port_ops	= &mv5_ops,
 	},
 	{  /* chip_5080 */
 		.sht		= &mv_sht,
 		.host_flags	= (MV_COMMON_FLAGS | MV_FLAG_DUAL_HC),
-		.pio_mask	= 0x1f,	/* pio0-4 */
-		.udma_mask	= 0x7f,	/* udma0-6 */
+		.xfer_mask	= MV_XFER_MASK,
 		.port_ops	= &mv5_ops,
 	},
 	{  /* chip_604x */
 		.sht		= &mv_sht,
 		.host_flags	= (MV_COMMON_FLAGS | MV_6XXX_FLAGS),
-		.pio_mask	= 0x1f,	/* pio0-4 */
-		.udma_mask	= 0x7f,	/* udma0-6 */
+		.xfer_mask	= MV_XFER_MASK,
 		.port_ops	= &mv6_ops,
 	},
 	{  /* chip_608x */
 		.sht		= &mv_sht,
 		.host_flags	= (MV_COMMON_FLAGS | MV_6XXX_FLAGS |
 				   MV_FLAG_DUAL_HC),
-		.pio_mask	= 0x1f,	/* pio0-4 */
-		.udma_mask	= 0x7f,	/* udma0-6 */
+		.xfer_mask	= MV_XFER_MASK,
 		.port_ops	= &mv6_ops,
 	},
 	{  /* chip_6042 */
 		.sht		= &mv_sht,
 		.host_flags	= (MV_COMMON_FLAGS | MV_6XXX_FLAGS),
-		.pio_mask	= 0x1f,	/* pio0-4 */
-		.udma_mask	= 0x7f,	/* udma0-6 */
+		.xfer_mask	= MV_XFER_MASK,
 		.port_ops	= &mv_iie_ops,
 	},
 	{  /* chip_7042 */
 		.sht		= &mv_sht,
 		.host_flags	= (MV_COMMON_FLAGS | MV_6XXX_FLAGS |
 				   MV_FLAG_DUAL_HC),
-		.pio_mask	= 0x1f,	/* pio0-4 */
-		.udma_mask	= 0x7f,	/* udma0-6 */
+		.xfer_mask	= MV_XFER_MASK,
 		.port_ops	= &mv_iie_ops,
 	},
 };
@@ -2354,8 +2350,7 @@ static int mv_init_one(struct pci_dev *p
 
 	probe_ent->sht = mv_port_info[board_idx].sht;
 	probe_ent->host_flags = mv_port_info[board_idx].host_flags;
-	probe_ent->pio_mask = mv_port_info[board_idx].pio_mask;
-	probe_ent->udma_mask = mv_port_info[board_idx].udma_mask;
+	probe_ent->xfer_mask = mv_port_info[board_idx].xfer_mask;
 	probe_ent->port_ops = mv_port_info[board_idx].port_ops;
 
 	probe_ent->irq = pdev->irq;
diff --git a/drivers/scsi/sata_nv.c b/drivers/scsi/sata_nv.c
index caffadc..ac824c2 100644
--- a/drivers/scsi/sata_nv.c
+++ b/drivers/scsi/sata_nv.c
@@ -75,9 +75,7 @@
 #define DRV_VERSION			"0.8"
 
 #define NV_PORTS			2
-#define NV_PIO_MASK			0x1f
-#define NV_MWDMA_MASK			0x07
-#define NV_UDMA_MASK			0x7f
+#define NV_XFER_MASK			ata_pack_xfermask(0x1f, 0x07, 0x7f)
 #define NV_PORT0_SCR_REG_OFFSET		0x00
 #define NV_PORT1_SCR_REG_OFFSET		0x40
 
@@ -281,9 +279,7 @@ static struct ata_port_info nv_port_info
 			  /* ATA_FLAG_SATA_RESET | */
 			  ATA_FLAG_SRST |
 			  ATA_FLAG_NO_LEGACY,
-	.pio_mask	= NV_PIO_MASK,
-	.mwdma_mask	= NV_MWDMA_MASK,
-	.udma_mask	= NV_UDMA_MASK,
+	.xfer_mask	= NV_XFER_MASK,
 	.port_ops	= &nv_ops,
 };
 
diff --git a/drivers/scsi/sata_promise.c b/drivers/scsi/sata_promise.c
index ab3f39a..1c637a0 100644
--- a/drivers/scsi/sata_promise.c
+++ b/drivers/scsi/sata_promise.c
@@ -77,6 +77,9 @@ enum {
 
 	PDC_COMMON_FLAGS	= ATA_FLAG_NO_LEGACY | ATA_FLAG_SRST |
 				  ATA_FLAG_MMIO | ATA_FLAG_NO_ATAPI,
+
+	/* xfer_mask: pio0-4, mwdma0-2, udma0-6 ; FIXME */
+	PDC_XFER_MASK		= ata_pack_xfermask(0x1f, 0x07, 0x7f),
 };
 
 
@@ -174,9 +177,7 @@ static const struct ata_port_info pdc_po
 	{
 		.sht		= &pdc_ata_sht,
 		.host_flags	= PDC_COMMON_FLAGS | ATA_FLAG_SATA,
-		.pio_mask	= 0x1f, /* pio0-4 */
-		.mwdma_mask	= 0x07, /* mwdma0-2 */
-		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
+		.xfer_mask	= PDC_XFER_MASK,
 		.port_ops	= &pdc_sata_ops,
 	},
 
@@ -184,9 +185,7 @@ static const struct ata_port_info pdc_po
 	{
 		.sht		= &pdc_ata_sht,
 		.host_flags	= PDC_COMMON_FLAGS | ATA_FLAG_SATA,
-		.pio_mask	= 0x1f, /* pio0-4 */
-		.mwdma_mask	= 0x07, /* mwdma0-2 */
-		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
+		.xfer_mask	= PDC_XFER_MASK,
 		.port_ops	= &pdc_sata_ops,
 	},
 
@@ -194,9 +193,7 @@ static const struct ata_port_info pdc_po
 	{
 		.sht		= &pdc_ata_sht,
 		.host_flags	= PDC_COMMON_FLAGS | ATA_FLAG_SLAVE_POSS,
-		.pio_mask	= 0x1f, /* pio0-4 */
-		.mwdma_mask	= 0x07, /* mwdma0-2 */
-		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
+		.xfer_mask	= PDC_XFER_MASK,
 		.port_ops	= &pdc_pata_ops,
 	},
 
@@ -204,9 +201,7 @@ static const struct ata_port_info pdc_po
 	{
 		.sht		= &pdc_ata_sht,
 		.host_flags	= PDC_COMMON_FLAGS | ATA_FLAG_SATA,
-		.pio_mask	= 0x1f, /* pio0-4 */
-		.mwdma_mask	= 0x07, /* mwdma0-2 */
-		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
+		.xfer_mask	= PDC_XFER_MASK,
 		.port_ops	= &pdc_sata_ops,
 	},
 
@@ -214,9 +209,7 @@ static const struct ata_port_info pdc_po
 	{
 		.sht		= &pdc_ata_sht,
 		.host_flags	= PDC_COMMON_FLAGS | ATA_FLAG_SATA,
-		.pio_mask	= 0x1f, /* pio0-4 */
-		.mwdma_mask	= 0x07, /* mwdma0-2 */
-		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
+		.xfer_mask	= PDC_XFER_MASK,
 		.port_ops	= &pdc_sata_ops,
 	},
 
@@ -224,9 +217,7 @@ static const struct ata_port_info pdc_po
 	{
 		.sht		= &pdc_ata_sht,
 		.host_flags	= PDC_COMMON_FLAGS | ATA_FLAG_SATA,
-		.pio_mask	= 0x1f, /* pio0-4 */
-		.mwdma_mask	= 0x07, /* mwdma0-2 */
-		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
+		.xfer_mask	= PDC_XFER_MASK,
 		.port_ops	= &pdc_sata_ops,
 	},
 };
@@ -725,9 +716,7 @@ static int pdc_ata_init_one (struct pci_
 
 	probe_ent->sht		= pdc_port_info[board_idx].sht;
 	probe_ent->host_flags	= pdc_port_info[board_idx].host_flags;
-	probe_ent->pio_mask	= pdc_port_info[board_idx].pio_mask;
-	probe_ent->mwdma_mask	= pdc_port_info[board_idx].mwdma_mask;
-	probe_ent->udma_mask	= pdc_port_info[board_idx].udma_mask;
+	probe_ent->xfer_mask	= pdc_port_info[board_idx].xfer_mask;
 	probe_ent->port_ops	= pdc_port_info[board_idx].port_ops;
 
        	probe_ent->irq = pdev->irq;
diff --git a/drivers/scsi/sata_qstor.c b/drivers/scsi/sata_qstor.c
index 2864826..f7b3f4e 100644
--- a/drivers/scsi/sata_qstor.c
+++ b/drivers/scsi/sata_qstor.c
@@ -178,8 +178,8 @@ static const struct ata_port_info qs_por
 				  ATA_FLAG_SATA_RESET |
 				  //FIXME ATA_FLAG_SRST |
 				  ATA_FLAG_MMIO,
-		.pio_mask	= 0x10, /* pio4 */
-		.udma_mask	= 0x7f, /* udma0-6 */
+		.xfer_mask	= ata_pack_xfermask(0x10, 0, 0x7f),
+				  /* pio4, udma0-6 */
 		.port_ops	= &qs_ata_ops,
 	},
 };
@@ -675,9 +675,7 @@ static int qs_ata_init_one(struct pci_de
 
 	probe_ent->sht		= qs_port_info[board_idx].sht;
 	probe_ent->host_flags	= qs_port_info[board_idx].host_flags;
-	probe_ent->pio_mask	= qs_port_info[board_idx].pio_mask;
-	probe_ent->mwdma_mask	= qs_port_info[board_idx].mwdma_mask;
-	probe_ent->udma_mask	= qs_port_info[board_idx].udma_mask;
+	probe_ent->xfer_mask	= qs_port_info[board_idx].xfer_mask;
 	probe_ent->port_ops	= qs_port_info[board_idx].port_ops;
 
 	probe_ent->irq		= pdev->irq;
diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
index 4aeb2c3..ad702cb 100644
--- a/drivers/scsi/sata_sil.c
+++ b/drivers/scsi/sata_sil.c
@@ -78,6 +78,9 @@ enum {
 	SIL_INTR_STEERING	= (1 << 1),
 	SIL_QUIRK_MOD15WRITE	= (1 << 0),
 	SIL_QUIRK_UDMA5MAX	= (1 << 1),
+
+	SIL_XFER_MASK		= ata_pack_xfermask(0x1f, 0x07, 0x3f),
+				  /* pio0-4, mwdma0-2, udma0-5 */
 };
 
 static int sil_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
@@ -181,27 +184,21 @@ static const struct ata_port_info sil_po
 		.sht		= &sil_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_MMIO,
-		.pio_mask	= 0x1f,			/* pio0-4 */
-		.mwdma_mask	= 0x07,			/* mwdma0-2 */
-		.udma_mask	= 0x3f,			/* udma0-5 */
+		.xfer_mask	= SIL_XFER_MASK,
 		.port_ops	= &sil_ops,
 	}, /* sil_3112_15w - keep it sync'd w/ sil_3112 */
 	{
 		.sht		= &sil_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_MMIO | SIL_FLAG_MOD15WRITE,
-		.pio_mask	= 0x1f,			/* pio0-4 */
-		.mwdma_mask	= 0x07,			/* mwdma0-2 */
-		.udma_mask	= 0x3f,			/* udma0-5 */
+		.xfer_mask	= SIL_XFER_MASK,
 		.port_ops	= &sil_ops,
 	}, /* sil_3114 */
 	{
 		.sht		= &sil_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_MMIO,
-		.pio_mask	= 0x1f,			/* pio0-4 */
-		.mwdma_mask	= 0x07,			/* mwdma0-2 */
-		.udma_mask	= 0x3f,			/* udma0-5 */
+		.xfer_mask	= SIL_XFER_MASK,
 		.port_ops	= &sil_ops,
 	},
 };
@@ -413,9 +410,7 @@ static int sil_init_one (struct pci_dev 
 	probe_ent->port_ops = sil_port_info[ent->driver_data].port_ops;
 	probe_ent->sht = sil_port_info[ent->driver_data].sht;
 	probe_ent->n_ports = (ent->driver_data == sil_3114) ? 4 : 2;
-	probe_ent->pio_mask = sil_port_info[ent->driver_data].pio_mask;
-	probe_ent->mwdma_mask = sil_port_info[ent->driver_data].mwdma_mask;
-	probe_ent->udma_mask = sil_port_info[ent->driver_data].udma_mask;
+	probe_ent->xfer_mask = sil_port_info[ent->driver_data].xfer_mask;
        	probe_ent->irq = pdev->irq;
        	probe_ent->irq_flags = SA_SHIRQ;
 	probe_ent->host_flags = sil_port_info[ent->driver_data].host_flags;
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index 1c54736..36b18b6 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -208,6 +208,9 @@ enum {
 	BID_SIL3131		= 2,
 
 	IRQ_STAT_4PORTS		= 0xf,
+
+	/* xfer_mask: pio0-4, mwdma0-2, udma0-5 */
+	XFER_MASK		= ata_pack_xfermask(0x1f, 0x07, 0x3f),
 };
 
 struct sil24_ata_block {
@@ -337,9 +340,7 @@ static struct ata_port_info sil24_port_i
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
 				  SIL24_NPORTS2FLAG(4),
-		.pio_mask	= 0x1f,			/* pio0-4 */
-		.mwdma_mask	= 0x07,			/* mwdma0-2 */
-		.udma_mask	= 0x3f,			/* udma0-5 */
+		.xfer_mask	= XFER_MASK,
 		.port_ops	= &sil24_ops,
 	},
 	/* sil_3132 */ 
@@ -348,9 +349,7 @@ static struct ata_port_info sil24_port_i
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
 				  SIL24_NPORTS2FLAG(2),
-		.pio_mask	= 0x1f,			/* pio0-4 */
-		.mwdma_mask	= 0x07,			/* mwdma0-2 */
-		.udma_mask	= 0x3f,			/* udma0-5 */
+		.xfer_mask	= XFER_MASK,
 		.port_ops	= &sil24_ops,
 	},
 	/* sil_3131/sil_3531 */
@@ -359,9 +358,7 @@ static struct ata_port_info sil24_port_i
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
 				  SIL24_NPORTS2FLAG(1),
-		.pio_mask	= 0x1f,			/* pio0-4 */
-		.mwdma_mask	= 0x07,			/* mwdma0-2 */
-		.udma_mask	= 0x3f,			/* udma0-5 */
+		.xfer_mask	= XFER_MASK,
 		.port_ops	= &sil24_ops,
 	},
 };
@@ -890,9 +887,7 @@ static int sil24_init_one(struct pci_dev
 
 	probe_ent->sht		= pinfo->sht;
 	probe_ent->host_flags	= pinfo->host_flags;
-	probe_ent->pio_mask	= pinfo->pio_mask;
-	probe_ent->mwdma_mask	= pinfo->mwdma_mask;
-	probe_ent->udma_mask	= pinfo->udma_mask;
+	probe_ent->xfer_mask	= pinfo->xfer_mask;
 	probe_ent->port_ops	= pinfo->port_ops;
 	probe_ent->n_ports	= SIL24_FLAG2NPORTS(pinfo->host_flags);
 
diff --git a/drivers/scsi/sata_sis.c b/drivers/scsi/sata_sis.c
index 7fd45f8..f572bd7 100644
--- a/drivers/scsi/sata_sis.c
+++ b/drivers/scsi/sata_sis.c
@@ -129,9 +129,7 @@ static struct ata_port_info sis_port_inf
 	.sht		= &sis_sht,
 	.host_flags	= ATA_FLAG_SATA | ATA_FLAG_SATA_RESET |
 			  ATA_FLAG_NO_LEGACY,
-	.pio_mask	= 0x1f,
-	.mwdma_mask	= 0x7,
-	.udma_mask	= 0x7f,
+	.xfer_mask	= ata_pack_xfermask(0x1f, 0x7, 0x7f),
 	.port_ops	= &sis_ops,
 };
 
diff --git a/drivers/scsi/sata_svw.c b/drivers/scsi/sata_svw.c
index 4aaccd5..9474f5e 100644
--- a/drivers/scsi/sata_svw.c
+++ b/drivers/scsi/sata_svw.c
@@ -431,9 +431,7 @@ static int k2_sata_init_one (struct pci_
 	/* We don't care much about the PIO/UDMA masks, but the core won't like us
 	 * if we don't fill these
 	 */
-	probe_ent->pio_mask = 0x1f;
-	probe_ent->mwdma_mask = 0x7;
-	probe_ent->udma_mask = 0x7f;
+	probe_ent->xfer_mask = ata_pack_xfermask(0x1f, 0x7, 0x7f);
 
 	/* different controllers have different number of ports - currently 4 or 8 */
 	/* All ports are on the same function. Multi-function device is no
diff --git a/drivers/scsi/sata_sx4.c b/drivers/scsi/sata_sx4.c
index 9f8a768..f715192 100644
--- a/drivers/scsi/sata_sx4.c
+++ b/drivers/scsi/sata_sx4.c
@@ -221,9 +221,8 @@ static const struct ata_port_info pdc_po
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_SRST | ATA_FLAG_MMIO |
 				  ATA_FLAG_NO_ATAPI,
-		.pio_mask	= 0x1f, /* pio0-4 */
-		.mwdma_mask	= 0x07, /* mwdma0-2 */
-		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
+		.xfer_mask	= ata_pack_xfermask(0x1f, 0x07, 0x7f),
+				  /* pio0-4, mwdma0-2, udma0-6 ; FIXME */
 		.port_ops	= &pdc_20621_ops,
 	},
 
@@ -1433,9 +1432,7 @@ static int pdc_sata_init_one (struct pci
 
 	probe_ent->sht		= pdc_port_info[board_idx].sht;
 	probe_ent->host_flags	= pdc_port_info[board_idx].host_flags;
-	probe_ent->pio_mask	= pdc_port_info[board_idx].pio_mask;
-	probe_ent->mwdma_mask	= pdc_port_info[board_idx].mwdma_mask;
-	probe_ent->udma_mask	= pdc_port_info[board_idx].udma_mask;
+	probe_ent->xfer_mask	= pdc_port_info[board_idx].xfer_mask;
 	probe_ent->port_ops	= pdc_port_info[board_idx].port_ops;
 
        	probe_ent->irq = pdev->irq;
diff --git a/drivers/scsi/sata_uli.c b/drivers/scsi/sata_uli.c
index 37a487b..e8286e3 100644
--- a/drivers/scsi/sata_uli.c
+++ b/drivers/scsi/sata_uli.c
@@ -124,8 +124,7 @@ static struct ata_port_info uli_port_inf
 	.sht            = &uli_sht,
 	.host_flags     = ATA_FLAG_SATA | ATA_FLAG_SATA_RESET |
 			  ATA_FLAG_NO_LEGACY,
-	.pio_mask       = 0x1f,		/* pio0-4 */
-	.udma_mask      = 0x7f,		/* udma0-6 */
+	.xfer_mask	= ata_pack_xfermask(0x1f, 0, 0x7f), /* pio0-4, udma0-6 */
 	.port_ops       = &uli_ops,
 };
 
diff --git a/drivers/scsi/sata_via.c b/drivers/scsi/sata_via.c
index ff65a0b..707a9d9 100644
--- a/drivers/scsi/sata_via.c
+++ b/drivers/scsi/sata_via.c
@@ -143,9 +143,7 @@ static const struct ata_port_operations 
 static struct ata_port_info svia_port_info = {
 	.sht		= &svia_sht,
 	.host_flags	= ATA_FLAG_SATA | ATA_FLAG_SRST | ATA_FLAG_NO_LEGACY,
-	.pio_mask	= 0x1f,
-	.mwdma_mask	= 0x07,
-	.udma_mask	= 0x7f,
+	.xfer_mask	= ata_pack_xfermask(0x1f, 0x07, 0x7f),
 	.port_ops	= &svia_sata_ops,
 };
 
@@ -243,9 +241,7 @@ static struct ata_probe_ent *vt6421_init
 	probe_ent->n_ports	= N_PORTS;
 	probe_ent->irq		= pdev->irq;
 	probe_ent->irq_flags	= SA_SHIRQ;
-	probe_ent->pio_mask	= 0x1f;
-	probe_ent->mwdma_mask	= 0x07;
-	probe_ent->udma_mask	= 0x7f;
+	probe_ent->xfer_mask	= ata_pack_xfermask(0x1f, 0x07, 0x7f);
 
 	for (i = 0; i < N_PORTS; i++)
 		vt6421_init_addrs(probe_ent, pdev, i);
diff --git a/drivers/scsi/sata_vsc.c b/drivers/scsi/sata_vsc.c
index e124db8..55865fd 100644
--- a/drivers/scsi/sata_vsc.c
+++ b/drivers/scsi/sata_vsc.c
@@ -357,9 +357,7 @@ static int __devinit vsc_sata_init_one (
 	/* We don't care much about the PIO/UDMA masks, but the core won't like us
 	 * if we don't fill these
 	 */
-	probe_ent->pio_mask = 0x1f;
-	probe_ent->mwdma_mask = 0x07;
-	probe_ent->udma_mask = 0x7f;
+	probe_ent->xfer_mask = ata_pack_xfermask(0x1f, 0x07, 0x7f);
 
 	/* We have 4 ports per PCI function */
 	vsc_sata_setup_port(&probe_ent->port[0], base + 1 * VSC_SATA_PORT_OFFSET);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index e0ffca4..d2c04ab 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -272,9 +272,7 @@ struct ata_probe_ent {
 	struct ata_ioports	port[ATA_MAX_PORTS];
 	unsigned int		n_ports;
 	unsigned int		hard_port_no;
-	unsigned int		pio_mask;
-	unsigned int		mwdma_mask;
-	unsigned int		udma_mask;
+	unsigned int		xfer_mask;
 	unsigned int		legacy_mode;
 	unsigned long		irq;
 	unsigned int		irq_flags;
@@ -453,9 +451,7 @@ struct ata_port_operations {
 struct ata_port_info {
 	struct scsi_host_template	*sht;
 	unsigned long		host_flags;
-	unsigned long		pio_mask;
-	unsigned long		mwdma_mask;
-	unsigned long		udma_mask;
+	unsigned int		xfer_mask;
 	const struct ata_port_operations *port_ops;
 	void 			*private_data;
 };
-- 
1.1.5

[PATCHSET] libata: use single unsigned int xfer_mask

[Tejun Heo]

Hello, all.

Here's the patchset to unify trasnfer mode/mask representation into
single unsigned int xfer_mask.  Transfer mode/mask are always stored
in single xfer_mask and a few transformation helpers are supplied to
help manipulating it.

This patchset is against...

upstream (f1b318793dcd2d9ff6b5ac06e7762098fa079cee)
+ ahci-softreset patch [1]
+ configure-reorganize patchset [2]
+ revalidation patchset [3]

This patchset is composed of three patches.

#01 : minor fix to sata_sil24 regarding mwdma_mask setting

#02 : make ata_device and ata_port use xfer_mask.  This patch changes
      most of transfer mode handling in libata core layer but doesn't
      affect low level drivers much.

#03 : make ata_port_info and ata_probe_ent use xfer_mask.  This patch
      doesn't change libata core layer any significant way.  It just
      changes the way low level drivers report supported transfer
      modes.

Decoding device transfer mode from IDENTIFY data is now done in
ata_id_xfermask() proper.  Determining device transfer mode is done
soley in ata_dev_xfermask().  base_from_shift() and offset calculation
are substitued by ata_xfer_modeval().

Thanks.

--
tejun

[PATCH 2/3] libata: make ata_device and ata_port use unsigned int xfer_mask

[Tejun Heo]

ata_device used pio_mode, dma_mode, xfer_mode and xfer_shift to
describe the currently configured transfer mode and ata_port used
pio_mask, mwdma_mask and udma_mask to describe supported trasnfer
modes.  This patch makes both ata_device and ata_port use single
unsigned int xfer_mask instead.

This change removes simplifies code in libata-core and makes
integration of later EH speed-down and per-device trasnfer mode
configuration easier.  This patch does not change any behavior.

Note that xfer_mode_str array is now only referenced in
ata_mode_string() and thus moved into the function.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/ata_piix.c     |    9 -
 drivers/scsi/libata-core.c  |  456 +++++++++++++++++--------------------------
 drivers/scsi/pdc_adma.c     |    2 
 drivers/scsi/sata_promise.c |    2 
 drivers/scsi/sata_sil.c     |    2 
 include/linux/libata.h      |  131 +++++++++++-
 6 files changed, 305 insertions(+), 297 deletions(-)

d6bbac7263b10a4fa5c52d8c3b1447e6ad762ebc
diff --git a/drivers/scsi/ata_piix.c b/drivers/scsi/ata_piix.c
index 4cc1108..f806772 100644
--- a/drivers/scsi/ata_piix.c
+++ b/drivers/scsi/ata_piix.c
@@ -348,7 +348,7 @@ static void piix_pata_cbl_detect(struct 
 	u8 tmp, mask;
 
 	/* no 80c support in host controller? */
-	if ((ap->udma_mask & ~ATA_UDMA_MASK_40C) == 0)
+	if (ata_xfer_mask2mode(ap->xfer_mask, ATA_SHIFT_UDMA) <= 2)
 		goto cbl40;
 
 	/* check BIOS cable detect results */
@@ -362,7 +362,7 @@ static void piix_pata_cbl_detect(struct 
 
 cbl40:
 	ap->cbl = ATA_CBL_PATA40;
-	ap->udma_mask &= ATA_UDMA_MASK_40C;
+	ap->xfer_mask &= ata_xfer_limitmask(ATA_SHIFT_UDMA, 2);
 }
 
 /**
@@ -474,7 +474,7 @@ static void piix_sata_phy_reset(struct a
 
 static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev)
 {
-	unsigned int pio	= adev->pio_mode - XFER_PIO_0;
+	unsigned int pio	= ata_xfer_mask2mode(ap->xfer_mask, ATA_SHIFT_PIO);
 	struct pci_dev *dev	= to_pci_dev(ap->host_set->dev);
 	unsigned int is_slave	= (adev->devno != 0);
 	unsigned int master_port= ap->hard_port_no ? 0x42 : 0x40;
@@ -526,10 +526,9 @@ static void piix_set_piomode (struct ata
 
 static void piix_set_dmamode (struct ata_port *ap, struct ata_device *adev)
 {
-	unsigned int udma	= adev->dma_mode; /* FIXME: MWDMA too */
 	struct pci_dev *dev	= to_pci_dev(ap->host_set->dev);
 	u8 maslave		= ap->hard_port_no ? 0x42 : 0x40;
-	u8 speed		= udma;
+	u8 speed		= ata_xfer_modeval(ap->xfer_mask);
 	unsigned int drive_dn	= (ap->hard_port_no ? 2 : 0) + adev->devno;
 	int a_speed		= 3 << (drive_dn * 4);
 	int u_flag		= 1 << drive_dn;
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index c971c15..3286df1 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -65,11 +65,8 @@ 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_get_mode_mask(const struct ata_port *ap, int shift);
-static int fgb(u32 bitmap);
-static int ata_choose_xfer_mode(const struct ata_port *ap,
-				u8 *xfer_mode_out,
-				unsigned int *xfer_shift_out);
+static unsigned int ata_dev_xfermask(struct ata_port *ap,
+				     struct ata_device *dev);
 
 static unsigned int ata_unique_id = 1;
 static struct workqueue_struct *ata_wq;
@@ -227,58 +224,47 @@ int ata_rwcmd_protocol(struct ata_queued
 	return -1;
 }
 
-static const char * const xfer_mode_str[] = {
-	"UDMA/16",
-	"UDMA/25",
-	"UDMA/33",
-	"UDMA/44",
-	"UDMA/66",
-	"UDMA/100",
-	"UDMA/133",
-	"UDMA7",
-	"MWDMA0",
-	"MWDMA1",
-	"MWDMA2",
-	"PIO0",
-	"PIO1",
-	"PIO2",
-	"PIO3",
-	"PIO4",
-};
-
 /**
- *	ata_udma_string - convert UDMA bit offset to string
- *	@mask: mask of bits supported; only highest bit counts.
+ *	ata_mode_string - convert xfer_mask to string
+ *	@xfer_mask: mask of bits supported; only highest bit counts.
  *
  *	Determine string which represents the highest speed
- *	(highest bit in @udma_mask).
+ *	(highest bit in @modemask).
  *
  *	LOCKING:
  *	None.
  *
  *	RETURNS:
  *	Constant C string representing highest speed listed in
- *	@udma_mask, or the constant C string "<n/a>".
+ *	@modemask, or the constant C string "<n/a>".
  */
 
-static const char *ata_mode_string(unsigned int mask)
+static const char *ata_mode_string(unsigned int xfer_mask)
 {
-	int i;
-
-	for (i = 7; i >= 0; i--)
-		if (mask & (1 << i))
-			goto out;
-	for (i = ATA_SHIFT_MWDMA + 2; i >= ATA_SHIFT_MWDMA; i--)
-		if (mask & (1 << i))
-			goto out;
-	for (i = ATA_SHIFT_PIO + 4; i >= ATA_SHIFT_PIO; i--)
-		if (mask & (1 << i))
-			goto out;
-
+	static const char * const xfer_mode_str[] = {
+		"PIO0",
+		"PIO1",
+		"PIO2",
+		"PIO3",
+		"PIO4",
+		"MWDMA0",
+		"MWDMA1",
+		"MWDMA2",
+		"UDMA/16",
+		"UDMA/25",
+		"UDMA/33",
+		"UDMA/44",
+		"UDMA/66",
+		"UDMA/100",
+		"UDMA/133",
+		"UDMA7",
+	};
+	int highbit;
+
+	highbit = fls(xfer_mask) - 1;
+	if (highbit >= 0 && highbit < ARRAY_SIZE(xfer_mode_str))
+		return xfer_mode_str[highbit];
 	return "<n/a>";
-
-out:
-	return xfer_mode_str[i];
 }
 
 /**
@@ -688,33 +674,49 @@ static inline void ata_dump_id(const u16
 		id[93]);
 }
 
-/*
- *	Compute the PIO modes available for this device. This is not as
- *	trivial as it seems if we must consider early devices correctly.
+/**
+ *	ata_id_xfermask - Compute xfermask from the given IDENTIFY data
+ *	@id: IDENTIFY data to compute xfer mask from
+ *
+ *	Compute the xfermask for this device. This is not as trivial
+ *	as it seems if we must consider early devices correctly.
  *
- *	FIXME: pre IDE drive timing (do we care ?). 
+ *	FIXME: pre IDE drive timing (do we care ?).
+ *
+ *	LOCKING:
+ *	None.
+ *
+ *	RETURNS:
+ *	Computed xfermask
  */
-
-static unsigned int ata_pio_modes(const struct ata_device *adev)
+static unsigned int ata_id_xfermask(const u16 *id)
 {
-	u16 modes;
+	unsigned int pio_mask, mwdma_mask, udma_mask;
 
 	/* Usual case. Word 53 indicates word 64 is valid */
-	if (adev->id[ATA_ID_FIELD_VALID] & (1 << 1)) {
-		modes = adev->id[ATA_ID_PIO_MODES] & 0x03;
-		modes <<= 3;
-		modes |= 0x7;
-		return modes;
+	if (id[ATA_ID_FIELD_VALID] & (1 << 1)) {
+		pio_mask = id[ATA_ID_PIO_MODES] & 0x03;
+		pio_mask <<= 3;
+		pio_mask |= 0x7;
+	} else {
+		/* If word 64 isn't valid then Word 51 high byte holds
+		 * the PIO timing number for the maximum. Turn it into
+		 * a mask.
+		 */
+		pio_mask = (2 << (id[ATA_ID_OLD_PIO_MODES] & 0xFF)) - 1 ;
+
+		/* But wait.. there's more. Design your standards by
+		 * committee and you too can get a free iordy field to
+		 * process. However its the speeds not the modes that
+		 * are supported... Note drivers using the timing API
+		 * will get this right anyway
+		 */
 	}
 
-	/* If word 64 isn't valid then Word 51 high byte holds the PIO timing
-	   number for the maximum. Turn it into a mask and return it */
-	modes = (2 << ((adev->id[ATA_ID_OLD_PIO_MODES] >> 8) & 0xFF)) - 1 ;
-	return modes;
-	/* But wait.. there's more. Design your standards by committee and
-	   you too can get a free iordy field to process. However its the 
-	   speeds not the modes that are supported... Note drivers using the
-	   timing API will get this right anyway */
+	mwdma_mask = id[ATA_ID_MWDMA_MODES] & 0x07;
+	udma_mask = id[ATA_ID_UDMA_MODES] & 0xff;
+
+	return ata_pack_xfermask(pio_mask, mwdma_mask, udma_mask);
 }
 
 static inline void
@@ -879,13 +881,14 @@ ata_exec_internal(struct ata_port *ap, s
 unsigned int ata_pio_need_iordy(const struct ata_device *adev)
 {
 	int pio;
-	int speed = adev->pio_mode - XFER_PIO_0;
+	unsigned int speed;
 
+	speed = ata_xfer_mask2mode(adev->xfer_mask, ATA_SHIFT_PIO);
 	if (speed < 2)
 		return 0;
 	if (speed > 2)
 		return 1;
-		
+
 	/* If we have no drive specific rule, then PIO 2 is non IORDY */
 
 	if (adev->id[ATA_ID_FIELD_VALID] & 2) {	/* EIDE */
@@ -1073,7 +1076,7 @@ static inline u8 ata_dev_knobble(const s
 static int ata_dev_configure(struct ata_port *ap, struct ata_device *dev,
 			     int verbose)
 {
-	unsigned long xfer_modes;
+	unsigned int xfer_mask;
 	int i, rc;
 
 	if (!ata_dev_present(dev)) {
@@ -1104,12 +1107,8 @@ static int ata_dev_configure(struct ata_
 		goto err_out_nosup;
 	}
 
-	/* quick-n-dirty find max transfer mode; for printk only */
-	xfer_modes = dev->id[ATA_ID_UDMA_MODES];
-	if (!xfer_modes)
-		xfer_modes = (dev->id[ATA_ID_MWDMA_MODES]) << ATA_SHIFT_MWDMA;
-	if (!xfer_modes)
-		xfer_modes = ata_pio_modes(dev);
+	/* find max transfer mode; for printk only */
+	xfer_mask = ata_id_xfermask(dev->id);
 
 	ata_dump_id(dev->id);
 
@@ -1133,7 +1132,7 @@ static int ata_dev_configure(struct ata_
 				       "max %s, %Lu sectors: %s\n",
 				       ap->id, dev->devno,
 				       ata_id_major_version(dev->id),
-				       ata_mode_string(xfer_modes),
+				       ata_mode_string(xfer_mask),
 				       (unsigned long long)dev->n_sectors,
 				       lba_desc);
 		} else {
@@ -1157,7 +1156,7 @@ static int ata_dev_configure(struct ata_
 				       "max %s, %Lu sectors: CHS %u/%u/%u\n",
 				       ap->id, dev->devno,
 				       ata_id_major_version(dev->id),
-				       ata_mode_string(xfer_modes),
+				       ata_mode_string(xfer_mask),
 				       (unsigned long long)dev->n_sectors,
 				       dev->cylinders, dev->heads, dev->sectors);
 		}
@@ -1178,7 +1177,7 @@ static int ata_dev_configure(struct ata_
 		/* print device info to dmesg */
 		if (verbose)
 			printk(KERN_INFO "ata%u: dev %u ATAPI, max %s\n",
-			       ap->id, dev->devno, ata_mode_string(xfer_modes));
+			       ap->id, dev->devno, ata_mode_string(xfer_mask));
 	}
 
 	ap->host->max_cmd_len = 0;
@@ -1192,7 +1191,7 @@ static int ata_dev_configure(struct ata_
 		if (verbose)
 			printk(KERN_INFO "ata%u(%u): applying bridge limits\n",
 			       ap->id, dev->devno);
-		ap->udma_mask &= ATA_UDMA5;
+		ap->xfer_mask &= ata_xfer_limitmask(ATA_SHIFT_UDMA, 5);
 		dev->max_sectors = ATA_MAX_SECTORS;
 	}
 
@@ -1561,7 +1560,9 @@ int ata_timing_compute(struct ata_device
 	 */
 
 	if (speed > XFER_PIO_4) {
-		ata_timing_compute(adev, adev->pio_mode, &p, T, UT);
+		unsigned int pio_mode;
+		pio_mode = ata_xfer_modeval(adev->xfer_mask & ATA_MASK_PIO);
+		ata_timing_compute(adev, pio_mode, &p, T, UT);
 		ata_timing_merge(&p, t, t, ATA_TIMING_ALL);
 	}
 
@@ -1582,104 +1583,100 @@ int ata_timing_compute(struct ata_device
 	return 0;
 }
 
-static const struct {
-	unsigned int shift;
-	u8 base;
-} xfer_mode_classes[] = {
-	{ ATA_SHIFT_UDMA,	XFER_UDMA_0 },
-	{ ATA_SHIFT_MWDMA,	XFER_MW_DMA_0 },
-	{ ATA_SHIFT_PIO,	XFER_PIO_0 },
-};
-
-static u8 base_from_shift(unsigned int shift)
+/**
+ *	ata_xfer_modeval - Find matching XFER_* for the given xfer_mask
+ *	@xfer_mask: xfer_mask of interest
+ *
+ *	Return matching XFER_* value for @xfer_mask.  Only the highest
+ *	bit of @xfer_mask is considered.
+ *
+ *	LOCKING:
+ *	None.
+ *
+ *	RETURNS:
+ *	Matching XFER_* value
+ */
+unsigned int ata_xfer_modeval(unsigned int xfer_mask)
 {
-	int i;
+	int idx = fls(xfer_mask) - 1;
+
+	BUG_ON(idx < 0);
+
+	if (idx < ATA_BITS_PIO)
+		return XFER_PIO_0 + idx;
+	idx -= ATA_BITS_PIO;
+
+	if (idx < ATA_BITS_MWDMA)
+		return XFER_MW_DMA_0 + idx;
+	idx -= ATA_BITS_MWDMA;
 
-	for (i = 0; i < ARRAY_SIZE(xfer_mode_classes); i++)
-		if (xfer_mode_classes[i].shift == shift)
-			return xfer_mode_classes[i].base;
+	if (idx < ATA_BITS_UDMA)
+		return XFER_UDMA_0 + idx;
 
-	return 0xff;
+	BUG();
+	return 0;
 }
 
 static void ata_dev_set_mode(struct ata_port *ap, struct ata_device *dev)
 {
-	int ofs, idx;
-	u8 base;
-
 	if (!ata_dev_present(dev) || (ap->flags & ATA_FLAG_PORT_DISABLED))
 		return;
 
-	if (dev->xfer_shift == ATA_SHIFT_PIO)
+	if (!(dev->xfer_mask & (ATA_MASK_MWDMA | ATA_MASK_UDMA)))
 		dev->flags |= ATA_DFLAG_PIO;
 
 	ata_dev_set_xfermode(ap, dev);
 
-	base = base_from_shift(dev->xfer_shift);
-	ofs = dev->xfer_mode - base;
-	idx = ofs + dev->xfer_shift;
-	WARN_ON(idx >= ARRAY_SIZE(xfer_mode_str));
-
 	if (ata_dev_revalidate(ap, dev, 0)) {
 		printk(KERN_ERR "ata%u: failed to revalidate after set "
 		       "xfermode, disabled\n", ap->id);
 		ata_port_disable(ap);
 	}
 
-	DPRINTK("idx=%d xfer_shift=%u, xfer_mode=0x%x, base=0x%x, offset=%d\n",
-		idx, dev->xfer_shift, (int)dev->xfer_mode, (int)base, ofs);
+	DPRINTK("ap->xfer_mask=0x%x dev->xfer_mask=0x%x,\n",
+		ap->xfer_mask, dev->xfer_mask);
 
 	printk(KERN_INFO "ata%u: dev %u configured for %s\n",
-		ap->id, dev->devno, xfer_mode_str[idx]);
+	       ap->id, dev->devno, ata_mode_string(dev->xfer_mask));
 }
 
 static int ata_host_set_pio(struct ata_port *ap)
 {
-	unsigned int mask;
-	int x, i;
-	u8 base, xfer_mode;
-
-	mask = ata_get_mode_mask(ap, ATA_SHIFT_PIO);
-	x = fgb(mask);
-	if (x < 0) {
-		printk(KERN_WARNING "ata%u: no PIO support\n", ap->id);
-		return -1;
-	}
-
-	base = base_from_shift(ATA_SHIFT_PIO);
-	xfer_mode = base + x;
-
-	DPRINTK("base 0x%x xfer_mode 0x%x mask 0x%x x %d\n",
-		(int)base, (int)xfer_mode, mask, x);
+	int i;
 
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {
 		struct ata_device *dev = &ap->device[i];
-		if (ata_dev_present(dev)) {
-			dev->pio_mode = xfer_mode;
-			dev->xfer_mode = xfer_mode;
-			dev->xfer_shift = ATA_SHIFT_PIO;
-			if (ap->ops->set_piomode)
-				ap->ops->set_piomode(ap, dev);
+
+		if (!ata_dev_present(dev))
+			continue;
+
+		if (!(dev->xfer_mask & ATA_MASK_PIO)) {
+			printk(KERN_WARNING "ata%u: no PIO support\n", ap->id);
+			return -1;
 		}
+
+		if (ap->ops->set_piomode)
+			ap->ops->set_piomode(ap, dev);
 	}
 
 	return 0;
 }
 
-static void ata_host_set_dma(struct ata_port *ap, u8 xfer_mode,
-			    unsigned int xfer_shift)
+static void ata_host_set_dma(struct ata_port *ap)
 {
 	int i;
 
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {
 		struct ata_device *dev = &ap->device[i];
-		if (ata_dev_present(dev)) {
-			dev->dma_mode = xfer_mode;
-			dev->xfer_mode = xfer_mode;
-			dev->xfer_shift = xfer_shift;
-			if (ap->ops->set_dmamode)
-				ap->ops->set_dmamode(ap, dev);
-		}
+
+		if (!ata_dev_present(dev))
+			continue;
+
+		if (!(dev->xfer_mask & (ATA_MASK_MWDMA | ATA_MASK_UDMA)))
+			continue;
+
+		if (ap->ops->set_dmamode)
+			ap->ops->set_dmamode(ap, dev);
 	}
 }
 
@@ -1694,24 +1691,22 @@ static void ata_host_set_dma(struct ata_
  */
 static void ata_set_mode(struct ata_port *ap)
 {
-	unsigned int xfer_shift;
-	u8 xfer_mode;
-	int rc;
+	int i, rc;
 
-	/* step 1: always set host PIO timings */
-	rc = ata_host_set_pio(ap);
-	if (rc)
-		goto err_out;
+	/* step 1: calculate xfer_mask */
+	for (i = 0; i < ATA_MAX_DEVICES; i++) {
+		struct ata_device *dev = &ap->device[i];
+		if (ata_dev_present(dev))
+			dev->xfer_mask = ata_dev_xfermask(ap, dev);
+	}
 
-	/* step 2: choose the best data xfer mode */
-	xfer_mode = xfer_shift = 0;
-	rc = ata_choose_xfer_mode(ap, &xfer_mode, &xfer_shift);
+	/* step 2: always set host PIO timings */
+	rc = ata_host_set_pio(ap);
 	if (rc)
 		goto err_out;
 
-	/* step 3: if that xfer mode isn't PIO, set host DMA timings */
-	if (xfer_shift != ATA_SHIFT_PIO)
-		ata_host_set_dma(ap, xfer_mode, xfer_shift);
+	/* step 3: set host DMA timings */
+	ata_host_set_dma(ap);
 
 	/* step 4: update devices' xfer mode */
 	ata_dev_set_mode(ap, &ap->device[0]);
@@ -2461,13 +2456,6 @@ int ata_dev_revalidate(struct ata_port *
 	return rc;
 }
 
-static void ata_pr_blacklisted(const struct ata_port *ap,
-			       const struct ata_device *dev)
-{
-	printk(KERN_WARNING "ata%u: dev %u is on DMA blacklist, disabling DMA\n",
-		ap->id, dev->devno);
-}
-
 static const char * const ata_dma_blacklist [] = {
 	"WDC AC11000H",
 	"WDC AC22100H",
@@ -2514,128 +2502,43 @@ static int ata_dma_blacklisted(const str
 	return 0;
 }
 
-static unsigned int ata_get_mode_mask(const struct ata_port *ap, int shift)
-{
-	const struct ata_device *master, *slave;
-	unsigned int mask;
-
-	master = &ap->device[0];
-	slave = &ap->device[1];
-
-	WARN_ON(!ata_dev_present(master) && !ata_dev_present(slave));
-
-	if (shift == ATA_SHIFT_UDMA) {
-		mask = ap->udma_mask;
-		if (ata_dev_present(master)) {
-			mask &= (master->id[ATA_ID_UDMA_MODES] & 0xff);
-			if (ata_dma_blacklisted(master)) {
-				mask = 0;
-				ata_pr_blacklisted(ap, master);
-			}
-		}
-		if (ata_dev_present(slave)) {
-			mask &= (slave->id[ATA_ID_UDMA_MODES] & 0xff);
-			if (ata_dma_blacklisted(slave)) {
-				mask = 0;
-				ata_pr_blacklisted(ap, slave);
-			}
-		}
-	}
-	else if (shift == ATA_SHIFT_MWDMA) {
-		mask = ap->mwdma_mask;
-		if (ata_dev_present(master)) {
-			mask &= (master->id[ATA_ID_MWDMA_MODES] & 0x07);
-			if (ata_dma_blacklisted(master)) {
-				mask = 0;
-				ata_pr_blacklisted(ap, master);
-			}
-		}
-		if (ata_dev_present(slave)) {
-			mask &= (slave->id[ATA_ID_MWDMA_MODES] & 0x07);
-			if (ata_dma_blacklisted(slave)) {
-				mask = 0;
-				ata_pr_blacklisted(ap, slave);
-			}
-		}
-	}
-	else if (shift == ATA_SHIFT_PIO) {
-		mask = ap->pio_mask;
-		if (ata_dev_present(master)) {
-			/* spec doesn't return explicit support for
-			 * PIO0-2, so we fake it
-			 */
-			u16 tmp_mode = master->id[ATA_ID_PIO_MODES] & 0x03;
-			tmp_mode <<= 3;
-			tmp_mode |= 0x7;
-			mask &= tmp_mode;
-		}
-		if (ata_dev_present(slave)) {
-			/* spec doesn't return explicit support for
-			 * PIO0-2, so we fake it
-			 */
-			u16 tmp_mode = slave->id[ATA_ID_PIO_MODES] & 0x03;
-			tmp_mode <<= 3;
-			tmp_mode |= 0x7;
-			mask &= tmp_mode;
-		}
-	}
-	else {
-		mask = 0xffffffff; /* shut up compiler warning */
-		BUG();
-	}
-
-	return mask;
-}
-
-/* find greatest bit */
-static int fgb(u32 bitmap)
-{
-	unsigned int i;
-	int x = -1;
-
-	for (i = 0; i < 32; i++)
-		if (bitmap & (1 << i))
-			x = i;
-
-	return x;
-}
-
 /**
- *	ata_choose_xfer_mode - attempt to find best transfer mode
- *	@ap: Port for which an xfer mode will be selected
- *	@xfer_mode_out: (output) SET FEATURES - XFER MODE code
- *	@xfer_shift_out: (output) bit shift that selects this mode
- *
- *	Based on host and device capabilities, determine the
- *	maximum transfer mode that is amenable to all.
+ *	ata_dev_xfermask - Compute supported xfermask of the given device
+ *	@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...
  *
  *	LOCKING:
- *	PCI/etc. bus probe sem.
+ *	None.
  *
  *	RETURNS:
- *	Zero on success, negative on error.
+ *	Computed xfermask.
  */
+static unsigned int ata_dev_xfermask(struct ata_port *ap,
+				     struct ata_device *dev)
+{
+	unsigned long xfer_mask;
+	int i;
 
-static int ata_choose_xfer_mode(const struct ata_port *ap,
-				u8 *xfer_mode_out,
-				unsigned int *xfer_shift_out)
-{
-	unsigned int mask, shift;
-	int x, i;
-
-	for (i = 0; i < ARRAY_SIZE(xfer_mode_classes); i++) {
-		shift = xfer_mode_classes[i].shift;
-		mask = ata_get_mode_mask(ap, shift);
-
-		x = fgb(mask);
-		if (x >= 0) {
-			*xfer_mode_out = xfer_mode_classes[i].base + x;
-			*xfer_shift_out = shift;
-			return 0;
-		}
+	xfer_mask = ap->xfer_mask;
+
+	for (i = 0; i < ATA_MAX_DEVICES; i++) {
+		struct ata_device *d = &ap->device[i];
+		if (!ata_dev_present(d))
+			continue;
+		xfer_mask &= ata_id_xfermask(d->id);
+		if (ata_dma_blacklisted(d))
+			xfer_mask &= ~(ATA_MASK_MWDMA | ATA_MASK_UDMA);
 	}
 
-	return -1;
+	if (ata_dma_blacklisted(dev))
+		printk(KERN_WARNING "ata%u: dev %u is on DMA blacklist, "
+		       "disabling DMA\n", ap->id, dev->devno);
+
+	return xfer_mask;
 }
 
 /**
@@ -2662,7 +2565,7 @@ static void ata_dev_set_xfermode(struct 
 	tf.feature = SETFEATURES_XFER;
 	tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
 	tf.protocol = ATA_PROT_NODATA;
-	tf.nsect = dev->xfer_mode;
+	tf.nsect = ata_xfer_modeval(dev->xfer_mask);
 
 	if (ata_exec_internal(ap, dev, &tf, DMA_NONE, NULL, 0)) {
 		printk(KERN_ERR "ata%u: failed to set xfermode, disabled\n",
@@ -4600,9 +4503,9 @@ static void ata_host_init(struct ata_por
 	ap->port_no = port_no;
 	ap->hard_port_no =
 		ent->legacy_mode ? ent->hard_port_no : port_no;
-	ap->pio_mask = ent->pio_mask;
-	ap->mwdma_mask = ent->mwdma_mask;
-	ap->udma_mask = ent->udma_mask;
+	ap->xfer_mask = ata_pack_xfermask(ent->pio_mask,
+					  ent->mwdma_mask,
+					  ent->udma_mask);
 	ap->flags |= ent->host_flags;
 	ap->ops = ent->port_ops;
 	ap->cbl = ATA_CBL_NONE;
@@ -4710,23 +4613,19 @@ int ata_device_add(const struct ata_prob
 	/* register each port bound to this device */
 	for (i = 0; i < ent->n_ports; i++) {
 		struct ata_port *ap;
-		unsigned long xfer_mode_mask;
 
 		ap = ata_host_add(ent, host_set, i);
 		if (!ap)
 			goto err_out;
 
 		host_set->ports[i] = ap;
-		xfer_mode_mask =(ap->udma_mask << ATA_SHIFT_UDMA) |
-				(ap->mwdma_mask << ATA_SHIFT_MWDMA) |
-				(ap->pio_mask << ATA_SHIFT_PIO);
 
 		/* print per-port info to dmesg */
 		printk(KERN_INFO "ata%u: %cATA max %s cmd 0x%lX ctl 0x%lX "
 				 "bmdma 0x%lX irq %lu\n",
 			ap->id,
 			ap->flags & ATA_FLAG_SATA ? 'S' : 'P',
-			ata_mode_string(xfer_mode_mask),
+			ata_mode_string(ap->xfer_mask),
 	       		ap->ioaddr.cmd_addr,
 	       		ap->ioaddr.ctl_addr,
 	       		ap->ioaddr.bmdma_addr,
@@ -5079,6 +4978,7 @@ EXPORT_SYMBOL_GPL(ata_drive_probe_reset)
 EXPORT_SYMBOL_GPL(ata_dev_revalidate);
 EXPORT_SYMBOL_GPL(ata_port_disable);
 EXPORT_SYMBOL_GPL(ata_ratelimit);
+EXPORT_SYMBOL_GPL(ata_xfer_modeval);
 EXPORT_SYMBOL_GPL(ata_busy_sleep);
 EXPORT_SYMBOL_GPL(ata_scsi_ioctl);
 EXPORT_SYMBOL_GPL(ata_scsi_queuecmd);
diff --git a/drivers/scsi/pdc_adma.c b/drivers/scsi/pdc_adma.c
index 5f33cc9..9f9b4d5 100644
--- a/drivers/scsi/pdc_adma.c
+++ b/drivers/scsi/pdc_adma.c
@@ -314,7 +314,7 @@ static int adma_fill_sg(struct ata_queue
 		if (ata_sg_is_last(sg, qc))
 			pFLAGS |= pEND;
 		buf[i++] = pFLAGS;
-		buf[i++] = qc->dev->dma_mode & 0xf;
+		buf[i++] = ata_xfer_mask2mode(qc->dev->xfer_mask, ATA_SHIFT_UDMA);
 		buf[i++] = 0;	/* pPKLW */
 		buf[i++] = 0;	/* reserved */
 
diff --git a/drivers/scsi/sata_promise.c b/drivers/scsi/sata_promise.c
index ba2b7a0..ab3f39a 100644
--- a/drivers/scsi/sata_promise.c
+++ b/drivers/scsi/sata_promise.c
@@ -367,7 +367,7 @@ static void pdc_pata_phy_reset(struct at
 {
 	/* FIXME: add cable detect.  Don't assume 40-pin cable */
 	ap->cbl = ATA_CBL_PATA40;
-	ap->udma_mask &= ATA_UDMA_MASK_40C;
+	ap->xfer_mask &= ata_xfer_limitmask(ATA_SHIFT_UDMA, 2);
 
 	pdc_reset_port(ap);
 	ata_port_probe(ap);
diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
index e14ed4e..4aeb2c3 100644
--- a/drivers/scsi/sata_sil.c
+++ b/drivers/scsi/sata_sil.c
@@ -360,7 +360,7 @@ static void sil_dev_config(struct ata_po
 	if (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;
+		ap->xfer_mask &= ata_xfer_limitmask(ATA_SHIFT_UDMA, 5);
 		return;
 	}
 }
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 81014be..e0ffca4 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -188,11 +188,19 @@ enum {
 	PORT_DISABLED		= 2,
 
 	/* encoding various smaller bitmaps into a single
-	 * unsigned long bitmap
+	 * unsigned int bitmap
 	 */
-	ATA_SHIFT_UDMA		= 0,
-	ATA_SHIFT_MWDMA		= 8,
-	ATA_SHIFT_PIO		= 11,
+	ATA_BITS_PIO		= 5,
+	ATA_BITS_MWDMA		= 3,
+	ATA_BITS_UDMA		= 8,
+
+	ATA_SHIFT_PIO		= 0,
+	ATA_SHIFT_MWDMA		= ATA_SHIFT_PIO + ATA_BITS_PIO,
+	ATA_SHIFT_UDMA		= ATA_SHIFT_MWDMA + ATA_BITS_MWDMA,
+
+	ATA_MASK_PIO		= ((1 << ATA_BITS_PIO) - 1) << ATA_SHIFT_PIO,
+	ATA_MASK_MWDMA		= ((1 << ATA_BITS_MWDMA) - 1) << ATA_SHIFT_MWDMA,
+	ATA_MASK_UDMA		= ((1 << ATA_BITS_UDMA) - 1) << ATA_SHIFT_UDMA,
 
 	/* size of buffer to pad xfers ending on unaligned boundaries */
 	ATA_DMA_PAD_SZ		= 4,
@@ -340,10 +348,7 @@ struct ata_device {
 	unsigned int		class;		/* ATA_DEV_xxx */
 	unsigned int		devno;		/* 0 or 1 */
 	u16			*id;		/* IDENTIFY xxx DEVICE data */
-	u8			pio_mode;
-	u8			dma_mode;
-	u8			xfer_mode;
-	unsigned int		xfer_shift;	/* ATA_SHIFT_xxx */
+	unsigned int		xfer_mask;
 
 	unsigned int		multi_count;	/* sectors count for
 						   READ/WRITE MULTIPLE */
@@ -374,9 +379,7 @@ struct ata_port {
 
 	u8			ctl;	/* cache of ATA control register */
 	u8			last_ctl;	/* Cache last written value */
-	unsigned int		pio_mask;
-	unsigned int		mwdma_mask;
-	unsigned int		udma_mask;
+	unsigned int		xfer_mask;
 	unsigned int		cbl;	/* cable type; ATA_CBL_xxx */
 
 	struct ata_device	device[ATA_MAX_DEVICES];
@@ -512,6 +515,7 @@ extern int ata_scsi_device_suspend(struc
 extern int ata_device_resume(struct ata_port *, struct ata_device *);
 extern int ata_device_suspend(struct ata_port *, struct ata_device *);
 extern int ata_ratelimit(void);
+extern unsigned int ata_xfer_modeval(unsigned int xfer_mask);
 extern unsigned int ata_busy_sleep(struct ata_port *ap,
 				   unsigned long timeout_pat,
 				   unsigned long timeout);
@@ -604,6 +608,111 @@ extern int pci_test_config_bits(struct p
 #endif /* CONFIG_PCI */
 
 
+/**
+ *	ata_pack_xfermask - Pack pio, mwdma and udma masks into xfer_mask
+ *	@pio_mask: pio_mask
+ *	@mwdma_mask: mwdma_mask
+ *	@udma_mask: udma_mask
+ *
+ *	Pack @pio_mask, @mwdma_mask and @udma_mask into a single
+ *	unsigned int xfer_mask.  This is a macro such that it can be
+ *	used as an initializer.
+ *
+ *	LOCKING:
+ *	None.
+ *
+ *	RETURNS:
+ *	Packed xfer_mask.
+ */
+#define ata_pack_xfermask(pio_mask, mwdma_mask, udma_mask)		\
+	 ((((pio_mask) << ATA_SHIFT_PIO) & ATA_MASK_PIO) |		\
+	  (((mwdma_mask) << ATA_SHIFT_MWDMA) & ATA_MASK_MWDMA) |	\
+	  (((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 destination masks will be ignored.
+ *
+ *	LOCKING:
+ *	None.
+ */
+static inline 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 inline unsigned int __ata_xfer_shift2mask(int shift)
+{
+	switch (shift) {
+	case ATA_SHIFT_PIO:
+		return ATA_MASK_PIO;
+	case ATA_SHIFT_MWDMA:
+		return ATA_MASK_MWDMA;
+	case ATA_SHIFT_UDMA:
+		return ATA_MASK_UDMA;
+	}
+	BUG();
+	return 0;
+}
+
+/**
+ *	ata_xfer_mask2mode - Extract transfer mode of given type from xfer_mask
+ *	@xfer_mask: xfer_mask to extract transfer mode from
+ *	@shift: Target transfer type
+ *
+ *	Extract transfer mode of type @shift from @xfer_mask.  e.g. If
+ *	a xfer_mask has PIO 0-3 set, the following returns 3.
+ *
+ *	ata_xfer_mask2mode(xfer_mask, ATA_SHIFT_PIO)
+ *
+ *	LOCKING:
+ *	None.
+ *
+ *	RETURNS:
+ *	Extracted transfer mode
+ */
+static inline int ata_xfer_mask2mode(unsigned int xfer_mask, int shift)
+{
+	return fls(xfer_mask & __ata_xfer_shift2mask(shift)) - 1 - shift;
+}
+
+/**
+ *	ata_xfer_limitmask - Compute mask to limit transfer mode of given type
+ *	@shift: Transfer type to limit
+ *	@mode: Maximum transfer mode
+ *
+ *	Compute a bit mask which can be used to limit transfer mode of
+ *	the type @shift to @mode.  e.g. The following will return a
+ *	mask which turns off any PIO mode above PIO 3 but doesn't
+ *	affect any other transfer modes.
+ *
+ *	ata_xfer_limitmask(ATA_SHIFT_PIO, 3)
+ *
+ *	LOCKING:
+ *	None.
+ *
+ *	RETURNS:
+ *	Computed limit mask
+ */
+static inline unsigned int ata_xfer_limitmask(int shift, int mode)
+{
+	return ((1 << (shift + mode + 1)) - 1) | ~__ata_xfer_shift2mask(shift);
+}
+
 static inline int
 ata_sg_is_last(struct scatterlist *sg, struct ata_queued_cmd *qc)
 {
-- 
1.1.5

Re: [PATCHSET] libata: use single unsigned int xfer_mask

[Tejun Heo]

Oops, I forgot to write about the following notes.

* In the second patch, some of ATA_UDMA_MASK_40C usages are converted to 
ata_xfer_*() helper usages with ATA_SHIFT_UDMA and explicit maximum mode 
number 2 as arguments.  Also, ATA_UDMAn and ATA_UDMA_MASK_40C macros are 
very lightly used.  Something can be done about these constants, I think.

* In the second patch, ata_timing_comput() is left to use XFER_* values 
as arguments and for calculation purposes.  As this function has no 
in-tree user yet, I couldn't determine how to transform it.  It's also 
quite possible that simply continuing to use XFER_* values are okay.

Thanks.

-- 
tejun

Re: [PATCH 2/3] libata: make ata_device and ata_port use unsigned int xfer_mask

[Alan Cox]

On Iau, 2006-02-16 at 23:09 +0900, Tejun Heo wrote:
> This change removes simplifies code in libata-core and makes
> integration of later EH speed-down and per-device trasnfer mode
> configuration easier.  This patch does not change any behavior.

It also makes all the drive code extremely ugly and non-obvious.

We replace clear code like

	speed = dev->pio_mode;

with

       speed = ata_xfer_mask2mode(adev->xfer_mask, ATA_SHIFT_PIO);

I don't think that is exactly an improvement. The replacement of clear
easy to read values in the tables with macros that are convoluted and so
long winded you've even used defines everywhere to avoid repeatign them
is not an improvement either

I'm definitely opposed to this gross uglification of the libata API.
Keep libata clean for driver writers because we have a lot of drivers
which will be written by non-experts and a single core which is
maintained entirely by smart people.

Alan

Re: [PATCHSET] libata: use single unsigned int xfer_mask

[Alan Cox]

On Iau, 2006-02-16 at 23:27 +0900, Tejun Heo wrote:
> * In the second patch, ata_timing_comput() is left to use XFER_* values 
> as arguments and for calculation purposes.  As this function has no 
> in-tree user yet, I couldn't determine how to transform it.  It's also 
> quite possible that simply continuing to use XFER_* values are okay.

Grab my pata patch and you'll find a collection of users. The function
turns a given mode into timing data and is basically a port of the handy
function ide_compute_timing from the original IDE code. It needs to be
passed a mode so XFER_ is the logical thing to pass it anyway.

Alan

Re: [PATCH 2/3] libata: make ata_device and ata_port use unsigned int xfer_mask

[Tejun Heo]

Alan Cox wrote:
> On Iau, 2006-02-16 at 23:09 +0900, Tejun Heo wrote:
> 
>>This change removes simplifies code in libata-core and makes
>>integration of later EH speed-down and per-device trasnfer mode
>>configuration easier.  This patch does not change any behavior.
> 
> 
> It also makes all the drive code extremely ugly and non-obvious.
> 
> We replace clear code like
> 
> 	speed = dev->pio_mode;
> 
> with
> 
>        speed = ata_xfer_mask2mode(adev->xfer_mask, ATA_SHIFT_PIO);
> 
> I don't think that is exactly an improvement. The replacement of clear
> easy to read values in the tables with macros that are convoluted and so
> long winded you've even used defines everywhere to avoid repeatign them
> is not an improvement either

I think it's better to use such XFER_MASK macros than repeating numeric
values if it's used multiple times with or without this change.

> I'm definitely opposed to this gross uglification of the libata API.
> Keep libata clean for driver writers because we have a lot of drivers
> which will be written by non-experts and a single core which is
> maintained entirely by smart people.

Yeap, point taken.  It does make code uglier for lower level drivers
although it helps cleaning up libata core layer.  I'll try to clean up
core layer without changing low level driver API.

Thanks for the comment.

-- 
tejun