regarding xfer mode representation in dev, ap and other places
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.
Signature · 1 line
tejun
PATCH 1/3 libata: fix sata_sil24 mwdma_mask setting
mwdma_mask was not copied from port_info to probe_ent. This bug is probably introduced by following ahci.c without thinking. Fix it.
---
drivers/scsi/sata_sil24.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
bc7de87cd24b47b5480d928755dd7c08f4d43b03
drivers/scsi/sata_sil24.c +1 -0
@@ -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);
Signature · 1 line
1.1.5
PATCH 1/1 libata: use single unsigned int xfer_mask
In reply to Tejun HeoHello, 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
In reply to Tejun Heoata_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.
---
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
drivers/scsi/ata_piix.c +4 -5
@@ -348,7 +348,7 @@ static void piix_pata_cbl_detect(structu8 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 astatic 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 atastatic 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;
drivers/scsi/libata-core.c +178 -278
661 lines not on this page · raw message
drivers/scsi/pdc_adma.c +1 -1
@@ -314,7 +314,7 @@ static int adma_fill_sg(struct ata_queueif (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 */
drivers/scsi/sata_promise.c +1 -1
@@ -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);
drivers/scsi/sata_sil.c +1 -1
@@ -360,7 +360,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 ; + ap->xfer_mask &=ata_xfer_limitmask (ATA_SHIFT_UDMA ,5 );return ; } }
include/linux/libata.h +120 -11
162 lines not on this page · raw message
Signature · 1 line
1.1.5
PATCH 3/3 libata: make ata_port_info and ata_probe_ent use xfer_mask
In reply to Tejun HeoMake ata_port_info and ata_probe_ent use single unsigned int xfer_mask instead of three masks for pio, mwdma and udma.
---
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
drivers/scsi/ahci.c +3 -4
@@ -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;
drivers/scsi/ata_piix.c +16 -21
@@ -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_inPIIX_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 ); }
drivers/scsi/libata-bmdma.c +1 -3
@@ -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;
drivers/scsi/libata-core.c +1 -3
@@ -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 ;
drivers/scsi/pdc_adma.c +3 -5
@@ -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;
drivers/scsi/sata_mv.c +11 -16
@@ -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;
drivers/scsi/sata_nv.c +2 -6
@@ -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, };
drivers/scsi/sata_promise.c +10 -21
@@ -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;
drivers/scsi/sata_qstor.c +3 -5
@@ -178,8 +178,8 @@ static const struct ata_port_info qs_porATA_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;
drivers/scsi/sata_sil.c +7 -12
@@ -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;
drivers/scsi/sata_sil24.c +7 -12
@@ -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);
drivers/scsi/sata_sis.c +1 -3
@@ -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, };
drivers/scsi/sata_svw.c +1 -3
@@ -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
drivers/scsi/sata_sx4.c +3 -6
@@ -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;
drivers/scsi/sata_uli.c +1 -2
@@ -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, };
drivers/scsi/sata_via.c +2 -6
@@ -143,9 +143,7 @@ static const struct ata_port_operationsstatic 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);
drivers/scsi/sata_vsc.c +1 -3
@@ -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 );
include/linux/libata.h +2 -6
@@ -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; };
Signature · 1 line
1.1.5
Re: [PATCHSET] libata: use single unsigned int xfer_mask
In reply to Tejun HeoOops, 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.
Signature · 1 line
tejun
Re: [PATCH 2/3] libata: make ata_device and ata_port use unsigned int xfer_mask
In reply to Tejun HeoQuoted 3 lines · Tejun Heo
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
In reply to Tejun HeoQuoted 4 lines · Tejun Heo
* 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
In reply to Alan CoxQuoted 13 lines · Alan Cox
Quoted 3 lines · Tejun Heo
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;
Quoted 8 lines
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.
Quoted 4 lines
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.
Signature · 1 line
tejun