Subject: [PATCHSET] ata: rework support for PIIX-alike PATA controllers


Hi,

The following patchset reworks support for PIIX-alike PATA controllers.

The first part rewrites code for programming PIO and MWDMA timings in the
affected host drivers, while also fixing other issues discovered during
the main task. Then the second part adds support for non-Intel PIIX-alike
controllers to ata_piix host driver and removes separate host drivers.

All in all this greatly simplifies the total code needed for all PIIX-alikes
support (~1400 LOC are gone and 5 host drivers are removed) and hopefully
makes it easier to maintain it in the long-term (some drivers already got
slightly out-of-sync with ata_piix host driver, i.e. pata_rdc lacked locking
fixes and Power Management support, both issues are fixed by patchset).

All testing was done using QEMU's PIIX3 controller emulation so any testing
with real EFAR, IT8213, old PIIX, RDC and Radisys R82600 PATA controllers
would be really appreciated..

-- Bartlomiej


The following changes since commit 3c0eee3fe6a3a1c745379547c7e7c904aa64f6d5:

Linux 2.6.37 (2011-01-04 16:50:19 -0800)

are available:

Bartlomiej Zolnierkiewicz (20):
ata_piix: SITRE handling fix
ata_piix: unify code for programming PIO and MWDMA timings
pata_efar: fix register naming used in efar_set_piomode()
pata_efar: unify code for programming PIO and MWDMA timings
pata_efar: always program master_data before slave_data
pata_it8213: fix register naming used in it8213_set_piomode()
pata_it8213: unify code for programming PIO and MWDMA timings
pata_it8213: add UDMA100 and UDMA133 support
pata_oldpiix: unify code for programming PIO and MWDMA timings
pata_radisys: unify code for programming PIO and MWDMA timings
pata_rdc: unify code for programming PIO and MWDMA timings
pata_rdc: parallel scanning needs an extra locking
pata_rdc: add Power Management support
pata_oldpiix: add locking for parallel scanning
pata_oldpiix: enable parallel scan
ata_piix: add EFAR SLC90E66 support
ata_piix: add IT8213 support
ata_piix: add RDC support
ata_piix: add Intel old PIIX support
ata_piix: add Radisys R82600 support

drivers/ata/Kconfig | 5 +
drivers/ata/Makefile | 5 -
drivers/ata/ata_piix.c | 410 +++++++++++++++++++++++++++++++++-----------
drivers/ata/pata_efar.c | 319 ----------------------------------
drivers/ata/pata_it8213.c | 313 ---------------------------------
drivers/ata/pata_oldpiix.c | 289 -------------------------------
drivers/ata/pata_radisys.c | 268 -----------------------------
drivers/ata/pata_rdc.c | 401 -------------------------------------------
8 files changed, 315 insertions(+), 1695 deletions(-)
delete mode 100644 drivers/ata/pata_efar.c
delete mode 100644 drivers/ata/pata_it8213.c
delete mode 100644 drivers/ata/pata_oldpiix.c
delete mode 100644 drivers/ata/pata_radisys.c
delete mode 100644 drivers/ata/pata_rdc.c


Subject: [PATCH 01/20] ata_piix: SITRE handling fix

>From 7b6d27a9d65fef8795fdeee3733316450f840aa6 Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <[email protected]>
Date: Tue, 8 Feb 2011 12:39:24 +0100
Subject: [PATCH 01/20] ata_piix: SITRE handling fix

Set SITRE bit also in master-only configurations.

Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ata/ata_piix.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index 6cb14ca..0f4856d 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -751,8 +751,6 @@ static void piix_set_piomode(struct ata_port *ap, struct ata_device *adev)
if (is_slave) {
/* clear TIME1|IE1|PPE1|DTE1 */
master_data &= 0xff0f;
- /* Enable SITRE (separate slave timing register) */
- master_data |= 0x4000;
/* enable PPE1, IE1 and TIME1 as needed */
master_data |= (control << 4);
pci_read_config_byte(dev, slave_port, &slave_data);
@@ -770,6 +768,9 @@ static void piix_set_piomode(struct ata_port *ap, struct ata_device *adev)
(timings[pio][0] << 12) |
(timings[pio][1] << 8);
}
+
+ /* Enable SITRE (separate slave timing register) */
+ master_data |= 0x4000;
pci_write_config_word(dev, master_port, master_data);
if (is_slave)
pci_write_config_byte(dev, slave_port, slave_data);
--
1.7.1

Subject: [PATCH 02/20] ata_piix: unify code for programming PIO and MWDMA timings

>From ca6c1fbc7d853829524b87a3bcbbb1f67796281c Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <[email protected]>
Date: Tue, 8 Feb 2011 12:39:24 +0100
Subject: [PATCH 02/20] ata_piix: unify code for programming PIO and MWDMA timings

Besides making things noticably simpler it results in ~2% decrease in
the driver LOC count and also ~2% decrease in the driver binary size
(as measured on x86-32).

Fix piix_set_piomode() documentation while at it.

Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ata/ata_piix.c | 112 ++++++++++++++++--------------------------------
1 files changed, 37 insertions(+), 75 deletions(-)

diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index 0f4856d..c954f91 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -696,22 +696,11 @@ static int piix_pata_prereset(struct ata_link *link, unsigned long deadline)

static DEFINE_SPINLOCK(piix_lock);

-/**
- * piix_set_piomode - Initialize host controller PATA PIO timings
- * @ap: Port whose timings we are configuring
- * @adev: um
- *
- * Set PIO mode for device, in host controller PCI config space.
- *
- * LOCKING:
- * None (inherited from caller).
- */
-
-static void piix_set_piomode(struct ata_port *ap, struct ata_device *adev)
+static void piix_set_timings(struct ata_port *ap, struct ata_device *adev,
+ u8 pio, bool use_mwdma)
{
struct pci_dev *dev = to_pci_dev(ap->host->dev);
unsigned long flags;
- unsigned int pio = adev->pio_mode - XFER_PIO_0;
unsigned int is_slave = (adev->devno != 0);
unsigned int master_port= ap->port_no ? 0x42 : 0x40;
unsigned int slave_port = 0x44;
@@ -732,14 +721,18 @@ static void piix_set_piomode(struct ata_port *ap, struct ata_device *adev)
{ 2, 1 },
{ 2, 3 }, };

- if (pio >= 2)
+ if (pio >= 2 || use_mwdma)
control |= 1; /* TIME1 enable */
- if (ata_pio_need_iordy(adev))
+ if (ata_pio_need_iordy(adev) || use_mwdma)
control |= 2; /* IE enable */
-
/* Intel specifies that the PPE functionality is for disk only */
if (adev->class == ATA_DEV_ATA)
control |= 4; /* PPE enable */
+ /* If the drive MWDMA is faster than it can do PIO then
+ we must force PIO into PIO0 */
+ if (use_mwdma && adev->pio_mode < (XFER_PIO_0 + pio))
+ /* Enable DMA timing only */
+ control |= 8; /* PIO cycles in PIO0 */

spin_lock_irqsave(&piix_lock, flags);

@@ -788,6 +781,22 @@ static void piix_set_piomode(struct ata_port *ap, struct ata_device *adev)
}

/**
+ * piix_set_piomode - Initialize host controller PATA PIO timings
+ * @ap: Port whose timings we are configuring
+ * @adev: Drive in question
+ *
+ * Set PIO mode for device, in host controller PCI config space.
+ *
+ * LOCKING:
+ * None (inherited from caller).
+ */
+
+static void piix_set_piomode(struct ata_port *ap, struct ata_device *adev)
+{
+ piix_set_timings(ap, adev, adev->pio_mode - XFER_PIO_0, 0);
+}
+
+/**
* do_pata_set_dmamode - Initialize host controller PATA PIO timings
* @ap: Port whose timings we are configuring
* @adev: Drive in question
@@ -803,31 +812,20 @@ static void do_pata_set_dmamode(struct ata_port *ap, struct ata_device *adev, in
{
struct pci_dev *dev = to_pci_dev(ap->host->dev);
unsigned long flags;
- u8 master_port = ap->port_no ? 0x42 : 0x40;
- u16 master_data;
u8 speed = adev->dma_mode;
int devid = adev->devno + 2 * ap->port_no;
u8 udma_enable = 0;

- static const /* ISP RTC */
- u8 timings[][2] = { { 0, 0 },
- { 0, 0 },
- { 1, 0 },
- { 2, 1 },
- { 2, 3 }, };
-
- spin_lock_irqsave(&piix_lock, flags);
-
- pci_read_config_word(dev, master_port, &master_data);
- if (ap->udma_mask)
- pci_read_config_byte(dev, 0x48, &udma_enable);
-
if (speed >= XFER_UDMA_0) {
- unsigned int udma = adev->dma_mode - XFER_UDMA_0;
+ unsigned int udma = speed - XFER_UDMA_0;
u16 udma_timing;
u16 ideconf;
int u_clock, u_speed;

+ spin_lock_irqsave(&piix_lock, flags);
+
+ pci_read_config_byte(dev, 0x48, &udma_enable);
+
/*
* UDMA is handled by a combination of clock switching and
* selection of dividers
@@ -860,56 +858,20 @@ static void do_pata_set_dmamode(struct ata_port *ap, struct ata_device *adev, in
performance (WR_PingPong_En) */
pci_write_config_word(dev, 0x54, ideconf);
}
+
+ pci_write_config_byte(dev, 0x48, udma_enable);
+
+ spin_unlock_irqrestore(&piix_lock, flags);
} else {
- /*
- * MWDMA is driven by the PIO timings. We must also enable
- * IORDY unconditionally along with TIME1. PPE has already
- * been set when the PIO timing was set.
- */
- unsigned int mwdma = adev->dma_mode - XFER_MW_DMA_0;
- unsigned int control;
- u8 slave_data;
+ /* MWDMA is driven by the PIO timings. */
+ unsigned int mwdma = speed - XFER_MW_DMA_0;
const unsigned int needed_pio[3] = {
XFER_PIO_0, XFER_PIO_3, XFER_PIO_4
};
int pio = needed_pio[mwdma] - XFER_PIO_0;

- control = 3; /* IORDY|TIME1 */
-
- /* If the drive MWDMA is faster than it can do PIO then
- we must force PIO into PIO0 */
-
- if (adev->pio_mode < needed_pio[mwdma])
- /* Enable DMA timing only */
- control |= 8; /* PIO cycles in PIO0 */
-
- if (adev->devno) { /* Slave */
- master_data &= 0xFF4F; /* Mask out IORDY|TIME1|DMAONLY */
- master_data |= control << 4;
- pci_read_config_byte(dev, 0x44, &slave_data);
- slave_data &= (ap->port_no ? 0x0f : 0xf0);
- /* Load the matching timing */
- slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << (ap->port_no ? 4 : 0);
- pci_write_config_byte(dev, 0x44, slave_data);
- } else { /* Master */
- master_data &= 0xCCF4; /* Mask out IORDY|TIME1|DMAONLY
- and master timing bits */
- master_data |= control;
- master_data |=
- (timings[pio][0] << 12) |
- (timings[pio][1] << 8);
- }
-
- if (ap->udma_mask)
- udma_enable &= ~(1 << devid);
-
- pci_write_config_word(dev, master_port, master_data);
+ piix_set_timings(ap, adev, pio, 1);
}
- /* Don't scribble on 0x48 if the controller does not support UDMA */
- if (ap->udma_mask)
- pci_write_config_byte(dev, 0x48, udma_enable);
-
- spin_unlock_irqrestore(&piix_lock, flags);
}

/**
--
1.7.1

Subject: [PATCH 03/20] pata_efar: fix register naming used in efar_set_piomode()

>From efbad2a0611e47772c8696a2c5b2037c78554378 Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <[email protected]>
Date: Tue, 8 Feb 2011 12:39:25 +0100
Subject: [PATCH 03/20] pata_efar: fix register naming used in efar_set_piomode()

Rename 'idetm_port' and 'idetm_data' variables to 'master_port'
and 'master_data' respectively to match register naming used in
efar_set_dma_mode() and in ata_piix.c.

Fix efar_set_piomode() documentation and 'master_port' type
while at it.

Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ata/pata_efar.c | 22 +++++++++++-----------
1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/ata/pata_efar.c b/drivers/ata/pata_efar.c
index a088347..afe92b7 100644
--- a/drivers/ata/pata_efar.c
+++ b/drivers/ata/pata_efar.c
@@ -73,7 +73,7 @@ static DEFINE_SPINLOCK(efar_lock);
/**
* efar_set_piomode - Initialize host controller PATA PIO timings
* @ap: Port whose timings we are configuring
- * @adev: um
+ * @adev: Device to program
*
* Set PIO mode for device, in host controller PCI config space.
*
@@ -85,9 +85,9 @@ static void efar_set_piomode (struct ata_port *ap, struct ata_device *adev)
{
unsigned int pio = adev->pio_mode - XFER_PIO_0;
struct pci_dev *dev = to_pci_dev(ap->host->dev);
- unsigned int idetm_port= ap->port_no ? 0x42 : 0x40;
unsigned long flags;
- u16 idetm_data;
+ u8 master_port = ap->port_no ? 0x42 : 0x40;
+ u16 master_data;
u8 udma_enable;
int control = 0;

@@ -113,20 +113,20 @@ static void efar_set_piomode (struct ata_port *ap, struct ata_device *adev)

spin_lock_irqsave(&efar_lock, flags);

- pci_read_config_word(dev, idetm_port, &idetm_data);
+ pci_read_config_word(dev, master_port, &master_data);

/* Set PPE, IE, and TIME as appropriate */
if (adev->devno == 0) {
- idetm_data &= 0xCCF0;
- idetm_data |= control;
- idetm_data |= (timings[pio][0] << 12) |
+ master_data &= 0xCCF0;
+ master_data |= control;
+ master_data |= (timings[pio][0] << 12) |
(timings[pio][1] << 8);
} else {
int shift = 4 * ap->port_no;
u8 slave_data;

- idetm_data &= 0xFF0F;
- idetm_data |= (control << 4);
+ master_data &= 0xFF0F;
+ master_data |= (control << 4);

/* Slave timing in separate register */
pci_read_config_byte(dev, 0x44, &slave_data);
@@ -135,8 +135,8 @@ static void efar_set_piomode (struct ata_port *ap, struct ata_device *adev)
pci_write_config_byte(dev, 0x44, slave_data);
}

- idetm_data |= 0x4000; /* Ensure SITRE is set */
- pci_write_config_word(dev, idetm_port, idetm_data);
+ master_data |= 0x4000; /* Ensure SITRE is set */
+ pci_write_config_word(dev, master_port, master_data);

pci_read_config_byte(dev, 0x48, &udma_enable);
udma_enable &= ~(1 << (2 * ap->port_no + adev->devno));
--
1.7.1

Subject: [PATCH 04/20] pata_efar: unify code for programming PIO and MWDMA timings

>From 2de7db4bcc16b7e95cca9ceb5921a6f620be76b7 Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <[email protected]>
Date: Tue, 8 Feb 2011 12:39:25 +0100
Subject: [PATCH 04/20] pata_efar: unify code for programming PIO and MWDMA timings

Besides making things noticably simpler it results in ~10% decrease in
the driver LOC count and also ~8% decrease in the driver binary size
(as measured on x86-32).

Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ata/pata_efar.c | 104 +++++++++++++++++------------------------------
1 files changed, 37 insertions(+), 67 deletions(-)

diff --git a/drivers/ata/pata_efar.c b/drivers/ata/pata_efar.c
index afe92b7..1e2ff7d 100644
--- a/drivers/ata/pata_efar.c
+++ b/drivers/ata/pata_efar.c
@@ -70,20 +70,9 @@ static int efar_cable_detect(struct ata_port *ap)

static DEFINE_SPINLOCK(efar_lock);

-/**
- * efar_set_piomode - Initialize host controller PATA PIO timings
- * @ap: Port whose timings we are configuring
- * @adev: Device to program
- *
- * Set PIO mode for device, in host controller PCI config space.
- *
- * LOCKING:
- * None (inherited from caller).
- */
-
-static void efar_set_piomode (struct ata_port *ap, struct ata_device *adev)
+static void efar_set_timings(struct ata_port *ap, struct ata_device *adev,
+ u8 pio, bool use_mwdma)
{
- unsigned int pio = adev->pio_mode - XFER_PIO_0;
struct pci_dev *dev = to_pci_dev(ap->host->dev);
unsigned long flags;
u8 master_port = ap->port_no ? 0x42 : 0x40;
@@ -103,13 +92,18 @@ static void efar_set_piomode (struct ata_port *ap, struct ata_device *adev)
{ 2, 1 },
{ 2, 3 }, };

- if (pio > 1)
+ if (pio > 1 || use_mwdma)
control |= 1; /* TIME */
- if (ata_pio_need_iordy(adev)) /* PIO 3/4 require IORDY */
+ if (ata_pio_need_iordy(adev) || use_mwdma)
control |= 2; /* IE */
/* Intel specifies that the prefetch/posting is for disk only */
if (adev->class == ATA_DEV_ATA)
control |= 4; /* PPE */
+ /* If the drive MWDMA is faster than it can do PIO then
+ we must force PIO into PIO0 */
+ if (use_mwdma && adev->pio_mode < (XFER_PIO_0 + pio))
+ /* Enable DMA timing only */
+ control |= 8; /* PIO cycles in PIO0 */

spin_lock_irqsave(&efar_lock, flags);

@@ -145,6 +139,22 @@ static void efar_set_piomode (struct ata_port *ap, struct ata_device *adev)
}

/**
+ * efar_set_piomode - Initialize host controller PATA PIO timings
+ * @ap: Port whose timings we are configuring
+ * @adev: Device to program
+ *
+ * Set PIO mode for device, in host controller PCI config space.
+ *
+ * LOCKING:
+ * None (inherited from caller).
+ */
+
+static void efar_set_piomode(struct ata_port *ap, struct ata_device *adev)
+{
+ efar_set_timings(ap, adev, adev->pio_mode - XFER_PIO_0, 0);
+}
+
+/**
* efar_set_dmamode - Initialize host controller PATA DMA timings
* @ap: Port whose timings we are configuring
* @adev: Device to program
@@ -158,29 +168,19 @@ static void efar_set_piomode (struct ata_port *ap, struct ata_device *adev)
static void efar_set_dmamode (struct ata_port *ap, struct ata_device *adev)
{
struct pci_dev *dev = to_pci_dev(ap->host->dev);
- u8 master_port = ap->port_no ? 0x42 : 0x40;
- u16 master_data;
u8 speed = adev->dma_mode;
int devid = adev->devno + 2 * ap->port_no;
unsigned long flags;
u8 udma_enable;

- static const /* ISP RTC */
- u8 timings[][2] = { { 0, 0 },
- { 0, 0 },
- { 1, 0 },
- { 2, 1 },
- { 2, 3 }, };
-
- spin_lock_irqsave(&efar_lock, flags);
-
- pci_read_config_word(dev, master_port, &master_data);
- pci_read_config_byte(dev, 0x48, &udma_enable);
-
if (speed >= XFER_UDMA_0) {
- unsigned int udma = adev->dma_mode - XFER_UDMA_0;
+ unsigned int udma = speed - XFER_UDMA_0;
u16 udma_timing;

+ spin_lock_irqsave(&efar_lock, flags);
+
+ pci_read_config_byte(dev, 0x48, &udma_enable);
+
udma_enable |= (1 << devid);

/* Load the UDMA mode number */
@@ -188,50 +188,20 @@ static void efar_set_dmamode (struct ata_port *ap, struct ata_device *adev)
udma_timing &= ~(7 << (4 * devid));
udma_timing |= udma << (4 * devid);
pci_write_config_word(dev, 0x4A, udma_timing);
+
+ pci_write_config_byte(dev, 0x48, udma_enable);
+
+ spin_unlock_irqrestore(&efar_lock, flags);
} else {
- /*
- * MWDMA is driven by the PIO timings. We must also enable
- * IORDY unconditionally along with TIME1. PPE has already
- * been set when the PIO timing was set.
- */
- unsigned int mwdma = adev->dma_mode - XFER_MW_DMA_0;
- unsigned int control;
- u8 slave_data;
+ /* MWDMA is driven by the PIO timings. */
+ unsigned int mwdma = speed - XFER_MW_DMA_0;
const unsigned int needed_pio[3] = {
XFER_PIO_0, XFER_PIO_3, XFER_PIO_4
};
int pio = needed_pio[mwdma] - XFER_PIO_0;

- control = 3; /* IORDY|TIME1 */
-
- /* If the drive MWDMA is faster than it can do PIO then
- we must force PIO into PIO0 */
-
- if (adev->pio_mode < needed_pio[mwdma])
- /* Enable DMA timing only */
- control |= 8; /* PIO cycles in PIO0 */
-
- if (adev->devno) { /* Slave */
- master_data &= 0xFF4F; /* Mask out IORDY|TIME1|DMAONLY */
- master_data |= control << 4;
- pci_read_config_byte(dev, 0x44, &slave_data);
- slave_data &= ap->port_no ? 0x0F : 0xF0;
- /* Load the matching timing */
- slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << (ap->port_no ? 4 : 0);
- pci_write_config_byte(dev, 0x44, slave_data);
- } else { /* Master */
- master_data &= 0xCCF4; /* Mask out IORDY|TIME1|DMAONLY
- and master timing bits */
- master_data |= control;
- master_data |=
- (timings[pio][0] << 12) |
- (timings[pio][1] << 8);
- }
- udma_enable &= ~(1 << devid);
- pci_write_config_word(dev, master_port, master_data);
+ efar_set_timings(ap, adev, pio, 1);
}
- pci_write_config_byte(dev, 0x48, udma_enable);
- spin_unlock_irqrestore(&efar_lock, flags);
}

static struct scsi_host_template efar_sht = {
--
1.7.1

Subject: [PATCH 06/20] pata_it8213: fix register naming used in it8213_set_piomode()

>From fd377660531ca5b5d0802fa282ae7c964d0fddd7 Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <[email protected]>
Date: Tue, 8 Feb 2011 12:39:25 +0100
Subject: [PATCH 06/20] pata_it8213: fix register naming used in it8213_set_piomode()

Rename 'idetm_port' and 'idetm_data' variables to 'master_port'
and 'master_data' respectively to match register naming used in
it8213_set_dma_mode() and in ata_piix.c.

Fix 'master_port' type while at it.

Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ata/pata_it8213.c | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/ata/pata_it8213.c b/drivers/ata/pata_it8213.c
index 4d142a2..911f50b 100644
--- a/drivers/ata/pata_it8213.c
+++ b/drivers/ata/pata_it8213.c
@@ -76,8 +76,8 @@ static void it8213_set_piomode (struct ata_port *ap, struct ata_device *adev)
{
unsigned int pio = adev->pio_mode - XFER_PIO_0;
struct pci_dev *dev = to_pci_dev(ap->host->dev);
- unsigned int idetm_port= ap->port_no ? 0x42 : 0x40;
- u16 idetm_data;
+ u8 master_port = ap->port_no ? 0x42 : 0x40;
+ u16 master_data;
int control = 0;

/*
@@ -100,19 +100,19 @@ static void it8213_set_piomode (struct ata_port *ap, struct ata_device *adev)
if (adev->class != ATA_DEV_ATA)
control |= 4; /* PPE */

- pci_read_config_word(dev, idetm_port, &idetm_data);
+ pci_read_config_word(dev, master_port, &master_data);

/* Set PPE, IE, and TIME as appropriate */
if (adev->devno == 0) {
- idetm_data &= 0xCCF0;
- idetm_data |= control;
- idetm_data |= (timings[pio][0] << 12) |
+ master_data &= 0xCCF0;
+ master_data |= control;
+ master_data |= (timings[pio][0] << 12) |
(timings[pio][1] << 8);
} else {
u8 slave_data;

- idetm_data &= 0xFF0F;
- idetm_data |= (control << 4);
+ master_data &= 0xFF0F;
+ master_data |= (control << 4);

/* Slave timing in separate register */
pci_read_config_byte(dev, 0x44, &slave_data);
@@ -121,8 +121,8 @@ static void it8213_set_piomode (struct ata_port *ap, struct ata_device *adev)
pci_write_config_byte(dev, 0x44, slave_data);
}

- idetm_data |= 0x4000; /* Ensure SITRE is set */
- pci_write_config_word(dev, idetm_port, idetm_data);
+ master_data |= 0x4000; /* Ensure SITRE is set */
+ pci_write_config_word(dev, master_port, master_data);
}

/**
--
1.7.1

Subject: [PATCH 05/20] pata_efar: always program master_data before slave_data

>From 8acc9371550ce5f98da56f6888ae4b898096ed2c Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <[email protected]>
Date: Tue, 8 Feb 2011 12:39:25 +0100
Subject: [PATCH 05/20] pata_efar: always program master_data before slave_data

We may need to set SITRE before programming slave_data.

This makes pata_efar match the behavior of IDE's slc90e66 host driver
and also of libata's ata_piix one.

Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ata/pata_efar.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/pata_efar.c b/drivers/ata/pata_efar.c
index 1e2ff7d..7f564d7 100644
--- a/drivers/ata/pata_efar.c
+++ b/drivers/ata/pata_efar.c
@@ -74,10 +74,12 @@ static void efar_set_timings(struct ata_port *ap, struct ata_device *adev,
u8 pio, bool use_mwdma)
{
struct pci_dev *dev = to_pci_dev(ap->host->dev);
+ unsigned int is_slave = (adev->devno != 0);
unsigned long flags;
u8 master_port = ap->port_no ? 0x42 : 0x40;
u16 master_data;
u8 udma_enable;
+ u8 slave_data;
int control = 0;

/*
@@ -110,14 +112,13 @@ static void efar_set_timings(struct ata_port *ap, struct ata_device *adev,
pci_read_config_word(dev, master_port, &master_data);

/* Set PPE, IE, and TIME as appropriate */
- if (adev->devno == 0) {
+ if (is_slave == 0) {
master_data &= 0xCCF0;
master_data |= control;
master_data |= (timings[pio][0] << 12) |
(timings[pio][1] << 8);
} else {
int shift = 4 * ap->port_no;
- u8 slave_data;

master_data &= 0xFF0F;
master_data |= (control << 4);
@@ -126,12 +127,14 @@ static void efar_set_timings(struct ata_port *ap, struct ata_device *adev,
pci_read_config_byte(dev, 0x44, &slave_data);
slave_data &= ap->port_no ? 0x0F : 0xF0;
slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << shift;
- pci_write_config_byte(dev, 0x44, slave_data);
}

master_data |= 0x4000; /* Ensure SITRE is set */
pci_write_config_word(dev, master_port, master_data);

+ if (is_slave)
+ pci_write_config_byte(dev, 0x44, slave_data);
+
pci_read_config_byte(dev, 0x48, &udma_enable);
udma_enable &= ~(1 << (2 * ap->port_no + adev->devno));
pci_write_config_byte(dev, 0x48, udma_enable);
--
1.7.1

Subject: [PATCH 07/20] pata_it8213: unify code for programming PIO and MWDMA timings

>From 57632718f2e761e69b96e14f6900a487bad5d586 Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <[email protected]>
Date: Tue, 8 Feb 2011 12:39:26 +0100
Subject: [PATCH 07/20] pata_it8213: unify code for programming PIO and MWDMA timings

Besides making things noticably simpler it results in ~9% decrease in
the driver LOC count and also ~6% decrease in the driver binary size
(as measured on x86-32).

Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ata/pata_it8213.c | 89 +++++++++++++++------------------------------
1 files changed, 30 insertions(+), 59 deletions(-)

diff --git a/drivers/ata/pata_it8213.c b/drivers/ata/pata_it8213.c
index 911f50b..ccfd1ae 100644
--- a/drivers/ata/pata_it8213.c
+++ b/drivers/ata/pata_it8213.c
@@ -61,20 +61,9 @@ static int it8213_cable_detect(struct ata_port *ap)
return ATA_CBL_PATA80;
}

-/**
- * it8213_set_piomode - Initialize host controller PATA PIO timings
- * @ap: Port whose timings we are configuring
- * @adev: Device whose timings we are configuring
- *
- * Set PIO mode for device, in host controller PCI config space.
- *
- * LOCKING:
- * None (inherited from caller).
- */
-
-static void it8213_set_piomode (struct ata_port *ap, struct ata_device *adev)
+static void it8213_set_timings(struct ata_port *ap, struct ata_device *adev,
+ u8 pio, bool use_mwdma)
{
- unsigned int pio = adev->pio_mode - XFER_PIO_0;
struct pci_dev *dev = to_pci_dev(ap->host->dev);
u8 master_port = ap->port_no ? 0x42 : 0x40;
u16 master_data;
@@ -92,13 +81,18 @@ static void it8213_set_piomode (struct ata_port *ap, struct ata_device *adev)
{ 2, 1 },
{ 2, 3 }, };

- if (pio > 1)
+ if (pio > 1 || use_mwdma)
control |= 1; /* TIME */
- if (ata_pio_need_iordy(adev)) /* PIO 3/4 require IORDY */
+ if (ata_pio_need_iordy(adev) || use_mwdma)
control |= 2; /* IE */
/* Bit 2 is set for ATAPI on the IT8213 - reverse of ICH/PIIX */
if (adev->class != ATA_DEV_ATA)
control |= 4; /* PPE */
+ /* If the drive MWDMA is faster than it can do PIO then
+ we must force PIO into PIO0 */
+ if (use_mwdma && adev->pio_mode < (XFER_PIO_0 + pio))
+ /* Enable DMA timing only */
+ control |= 8; /* PIO cycles in PIO0 */

pci_read_config_word(dev, master_port, &master_data);

@@ -126,6 +120,22 @@ static void it8213_set_piomode (struct ata_port *ap, struct ata_device *adev)
}

/**
+ * it8213_set_piomode - Initialize host controller PATA PIO timings
+ * @ap: Port whose timings we are configuring
+ * @adev: Device whose timings we are configuring
+ *
+ * Set PIO mode for device, in host controller PCI config space.
+ *
+ * LOCKING:
+ * None (inherited from caller).
+ */
+
+static void it8213_set_piomode(struct ata_port *ap, struct ata_device *adev)
+{
+ it8213_set_timings(ap, adev, adev->pio_mode - XFER_PIO_0, 0);
+}
+
+/**
* it8213_set_dmamode - Initialize host controller PATA DMA timings
* @ap: Port whose timings we are configuring
* @adev: Device to program
@@ -140,23 +150,14 @@ static void it8213_set_piomode (struct ata_port *ap, struct ata_device *adev)
static void it8213_set_dmamode (struct ata_port *ap, struct ata_device *adev)
{
struct pci_dev *dev = to_pci_dev(ap->host->dev);
- u16 master_data;
u8 speed = adev->dma_mode;
int devid = adev->devno;
u8 udma_enable;

- static const /* ISP RTC */
- u8 timings[][2] = { { 0, 0 },
- { 0, 0 },
- { 1, 0 },
- { 2, 1 },
- { 2, 3 }, };
-
- pci_read_config_word(dev, 0x40, &master_data);
pci_read_config_byte(dev, 0x48, &udma_enable);

if (speed >= XFER_UDMA_0) {
- unsigned int udma = adev->dma_mode - XFER_UDMA_0;
+ unsigned int udma = speed - XFER_UDMA_0;
u16 udma_timing;
u16 ideconf;
int u_clock, u_speed;
@@ -184,46 +185,16 @@ static void it8213_set_dmamode (struct ata_port *ap, struct ata_device *adev)
ideconf |= u_clock << devid;
pci_write_config_word(dev, 0x54, ideconf);
} else {
- /*
- * MWDMA is driven by the PIO timings. We must also enable
- * IORDY unconditionally along with TIME1. PPE has already
- * been set when the PIO timing was set.
- */
- unsigned int mwdma = adev->dma_mode - XFER_MW_DMA_0;
- unsigned int control;
- u8 slave_data;
+ /* MWDMA is driven by the PIO timings. */
+ unsigned int mwdma = speed - XFER_MW_DMA_0;
static const unsigned int needed_pio[3] = {
XFER_PIO_0, XFER_PIO_3, XFER_PIO_4
};
int pio = needed_pio[mwdma] - XFER_PIO_0;

- control = 3; /* IORDY|TIME1 */
-
- /* If the drive MWDMA is faster than it can do PIO then
- we must force PIO into PIO0 */
-
- if (adev->pio_mode < needed_pio[mwdma])
- /* Enable DMA timing only */
- control |= 8; /* PIO cycles in PIO0 */
-
- if (devid) { /* Slave */
- master_data &= 0xFF4F; /* Mask out IORDY|TIME1|DMAONLY */
- master_data |= control << 4;
- pci_read_config_byte(dev, 0x44, &slave_data);
- slave_data &= 0xF0;
- /* Load the matching timing */
- slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << (ap->port_no ? 4 : 0);
- pci_write_config_byte(dev, 0x44, slave_data);
- } else { /* Master */
- master_data &= 0xCCF4; /* Mask out IORDY|TIME1|DMAONLY
- and master timing bits */
- master_data |= control;
- master_data |=
- (timings[pio][0] << 12) |
- (timings[pio][1] << 8);
- }
+ it8213_set_timings(ap, adev, pio, 1);
+
udma_enable &= ~(1 << devid);
- pci_write_config_word(dev, 0x40, master_data);
}
pci_write_config_byte(dev, 0x48, udma_enable);
}
--
1.7.1

Subject: [PATCH 08/20] pata_it8213: add UDMA100 and UDMA133 support

>From d7d8c9c8372230ec596814ca3281564156f94926 Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <[email protected]>
Date: Tue, 8 Feb 2011 12:39:26 +0100
Subject: [PATCH 08/20] pata_it8213: add UDMA100 and UDMA133 support

Fixes IDE -> libata regression.

IDE's it8213 host driver has been supporting those modes
(per official documentation) since the initial driver's merge
on Feb 7 2007 (commit 9c6712c0 "ide: add it8213 IDE driver").

Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ata/pata_it8213.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/pata_it8213.c b/drivers/ata/pata_it8213.c
index ccfd1ae..e045eab 100644
--- a/drivers/ata/pata_it8213.c
+++ b/drivers/ata/pata_it8213.c
@@ -164,7 +164,7 @@ static void it8213_set_dmamode (struct ata_port *ap, struct ata_device *adev)

/* Clocks follow the PIIX style */
u_speed = min(2 - (udma & 1), udma);
- if (udma == 5)
+ if (udma > 4)
u_clock = 0x1000; /* 100Mhz */
else if (udma > 2)
u_clock = 1; /* 66Mhz */
@@ -234,7 +234,7 @@ static int it8213_init_one (struct pci_dev *pdev, const struct pci_device_id *en
.flags = ATA_FLAG_SLAVE_POSS,
.pio_mask = ATA_PIO4,
.mwdma_mask = ATA_MWDMA12_ONLY,
- .udma_mask = ATA_UDMA4, /* FIXME: want UDMA 100? */
+ .udma_mask = ATA_UDMA6,
.port_ops = &it8213_ops,
};
/* Current IT8213 stuff is single port */
--
1.7.1

Subject: [PATCH 09/20] pata_oldpiix: unify code for programming PIO and MWDMA timings

>From 30f6c60ad1e2f9c640352b74279fdf8a930847bf Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <[email protected]>
Date: Tue, 8 Feb 2011 12:39:26 +0100
Subject: [PATCH 09/20] pata_oldpiix: unify code for programming PIO and MWDMA timings

Besides making things noticably simpler it results in ~12% decrease in
the driver LOC count and also ~5% decrease in the driver binary size
(as measured on x86-32).

Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ata/pata_oldpiix.c | 88 +++++++++++++------------------------------
1 files changed, 27 insertions(+), 61 deletions(-)

diff --git a/drivers/ata/pata_oldpiix.c b/drivers/ata/pata_oldpiix.c
index b811c16..433d2fc 100644
--- a/drivers/ata/pata_oldpiix.c
+++ b/drivers/ata/pata_oldpiix.c
@@ -50,20 +50,9 @@ static int oldpiix_pre_reset(struct ata_link *link, unsigned long deadline)
return ata_sff_prereset(link, deadline);
}

-/**
- * oldpiix_set_piomode - Initialize host controller PATA PIO timings
- * @ap: Port whose timings we are configuring
- * @adev: Device whose timings we are configuring
- *
- * Set PIO mode for device, in host controller PCI config space.
- *
- * LOCKING:
- * None (inherited from caller).
- */
-
-static void oldpiix_set_piomode (struct ata_port *ap, struct ata_device *adev)
+static void oldpiix_set_timings(struct ata_port *ap, struct ata_device *adev,
+ u8 pio, bool use_mwdma)
{
- unsigned int pio = adev->pio_mode - XFER_PIO_0;
struct pci_dev *dev = to_pci_dev(ap->host->dev);
unsigned int idetm_port= ap->port_no ? 0x42 : 0x40;
u16 idetm_data;
@@ -82,14 +71,18 @@ static void oldpiix_set_piomode (struct ata_port *ap, struct ata_device *adev)
{ 2, 1 },
{ 2, 3 }, };

- if (pio > 1)
+ if (pio > 1 || use_mwdma)
control |= 1; /* TIME */
- if (ata_pio_need_iordy(adev))
+ if (ata_pio_need_iordy(adev) || use_mwdma)
control |= 2; /* IE */
-
/* Intel specifies that the prefetch/posting is for disk only */
if (adev->class == ATA_DEV_ATA)
control |= 4; /* PPE */
+ /* If the drive MWDMA is faster than it can do PIO then
+ we must force PIO into PIO0 */
+ if (use_mwdma && adev->pio_mode < (XFER_PIO_0 + pio))
+ /* Enable DMA timing only */
+ control |= 8; /* PIO cycles in PIO0 */

pci_read_config_word(dev, idetm_port, &idetm_data);

@@ -113,6 +106,22 @@ static void oldpiix_set_piomode (struct ata_port *ap, struct ata_device *adev)
}

/**
+ * oldpiix_set_piomode - Initialize host controller PATA PIO timings
+ * @ap: Port whose timings we are configuring
+ * @adev: Device whose timings we are configuring
+ *
+ * Set PIO mode for device, in host controller PCI config space.
+ *
+ * LOCKING:
+ * None (inherited from caller).
+ */
+
+static void oldpiix_set_piomode(struct ata_port *ap, struct ata_device *adev)
+{
+ oldpiix_set_timings(ap, adev, adev->pio_mode - XFER_PIO_0, 0);
+}
+
+/**
* oldpiix_set_dmamode - Initialize host controller PATA DMA timings
* @ap: Port whose timings we are configuring
* @adev: Device to program
@@ -125,58 +134,15 @@ static void oldpiix_set_piomode (struct ata_port *ap, struct ata_device *adev)

static void oldpiix_set_dmamode (struct ata_port *ap, struct ata_device *adev)
{
- struct pci_dev *dev = to_pci_dev(ap->host->dev);
- u8 idetm_port = ap->port_no ? 0x42 : 0x40;
- u16 idetm_data;
-
- static const /* ISP RTC */
- u8 timings[][2] = { { 0, 0 },
- { 0, 0 },
- { 1, 0 },
- { 2, 1 },
- { 2, 3 }, };
-
- /*
- * MWDMA is driven by the PIO timings. We must also enable
- * IORDY unconditionally along with TIME1. PPE has already
- * been set when the PIO timing was set.
- */
+ /* MWDMA is driven by the PIO timings. */

unsigned int mwdma = adev->dma_mode - XFER_MW_DMA_0;
- unsigned int control;
const unsigned int needed_pio[3] = {
XFER_PIO_0, XFER_PIO_3, XFER_PIO_4
};
int pio = needed_pio[mwdma] - XFER_PIO_0;

- pci_read_config_word(dev, idetm_port, &idetm_data);
-
- control = 3; /* IORDY|TIME0 */
- /* Intel specifies that the PPE functionality is for disk only */
- if (adev->class == ATA_DEV_ATA)
- control |= 4; /* PPE enable */
-
- /* If the drive MWDMA is faster than it can do PIO then
- we must force PIO into PIO0 */
-
- if (adev->pio_mode < needed_pio[mwdma])
- /* Enable DMA timing only */
- control |= 8; /* PIO cycles in PIO0 */
-
- /* Mask out the relevant control and timing bits we will load. Also
- clear the other drive TIME register as a precaution */
- if (adev->devno == 0) {
- idetm_data &= 0xCCE0;
- idetm_data |= control;
- } else {
- idetm_data &= 0xCC0E;
- idetm_data |= (control << 4);
- }
- idetm_data |= (timings[pio][0] << 12) | (timings[pio][1] << 8);
- pci_write_config_word(dev, idetm_port, idetm_data);
-
- /* Track which port is configured */
- ap->private_data = adev;
+ oldpiix_set_timings(ap, adev, pio, 1);
}

/**
--
1.7.1

Subject: [PATCH 11/20] pata_rdc: unify code for programming PIO and MWDMA timings

>From 0bde1b0fa0d0f05659c147a204d3e4d16c05c276 Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <[email protected]>
Date: Tue, 8 Feb 2011 12:39:27 +0100
Subject: [PATCH 11/20] pata_rdc: unify code for programming PIO and MWDMA timings

Besides making things noticably simpler it results in ~8% decrease in
the driver LOC count and also ~8% decrease in the driver binary size
(as measured on x86-32).

Fix rdc_set_piomode() documentation while at it.

Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ata/pata_rdc.c | 99 ++++++++++++++++--------------------------------
1 files changed, 33 insertions(+), 66 deletions(-)

diff --git a/drivers/ata/pata_rdc.c b/drivers/ata/pata_rdc.c
index 5fbe9b1..d32b252 100644
--- a/drivers/ata/pata_rdc.c
+++ b/drivers/ata/pata_rdc.c
@@ -86,20 +86,9 @@ static int rdc_pata_prereset(struct ata_link *link, unsigned long deadline)
return ata_sff_prereset(link, deadline);
}

-/**
- * rdc_set_piomode - Initialize host controller PATA PIO timings
- * @ap: Port whose timings we are configuring
- * @adev: um
- *
- * Set PIO mode for device, in host controller PCI config space.
- *
- * LOCKING:
- * None (inherited from caller).
- */
-
-static void rdc_set_piomode(struct ata_port *ap, struct ata_device *adev)
+static void rdc_set_timings(struct ata_port *ap, struct ata_device *adev,
+ u8 pio, bool use_mwdma)
{
- unsigned int pio = adev->pio_mode - XFER_PIO_0;
struct pci_dev *dev = to_pci_dev(ap->host->dev);
unsigned int is_slave = (adev->devno != 0);
unsigned int master_port= ap->port_no ? 0x42 : 0x40;
@@ -116,13 +105,17 @@ static void rdc_set_piomode(struct ata_port *ap, struct ata_device *adev)
{ 2, 1 },
{ 2, 3 }, };

- if (pio >= 2)
+ if (pio >= 2 || use_mwdma)
control |= 1; /* TIME1 enable */
- if (ata_pio_need_iordy(adev))
+ if (ata_pio_need_iordy(adev) || use_mwdma)
control |= 2; /* IE enable */
-
if (adev->class == ATA_DEV_ATA)
control |= 4; /* PPE enable */
+ /* If the drive MWDMA is faster than it can do PIO then
+ we must force PIO into PIO0 */
+ if (use_mwdma && adev->pio_mode < (XFER_PIO_0 + pio))
+ /* Enable DMA timing only */
+ control |= 8; /* PIO cycles in PIO0 */

/* PIO configuration clears DTE unconditionally. It will be
* programmed in set_dmamode which is guaranteed to be called
@@ -164,6 +157,22 @@ static void rdc_set_piomode(struct ata_port *ap, struct ata_device *adev)
}

/**
+ * rdc_set_piomode - Initialize host controller PATA PIO timings
+ * @ap: Port whose timings we are configuring
+ * @adev: Drive in question
+ *
+ * Set PIO mode for device, in host controller PCI config space.
+ *
+ * LOCKING:
+ * None (inherited from caller).
+ */
+
+static void rdc_set_piomode(struct ata_port *ap, struct ata_device *adev)
+{
+ rdc_set_timings(ap, adev, adev->pio_mode - XFER_PIO_0, 0);
+}
+
+/**
* rdc_set_dmamode - Initialize host controller PATA PIO timings
* @ap: Port whose timings we are configuring
* @adev: Drive in question
@@ -177,28 +186,18 @@ static void rdc_set_piomode(struct ata_port *ap, struct ata_device *adev)
static void rdc_set_dmamode(struct ata_port *ap, struct ata_device *adev)
{
struct pci_dev *dev = to_pci_dev(ap->host->dev);
- u8 master_port = ap->port_no ? 0x42 : 0x40;
- u16 master_data;
u8 speed = adev->dma_mode;
int devid = adev->devno + 2 * ap->port_no;
u8 udma_enable = 0;

- static const /* ISP RTC */
- u8 timings[][2] = { { 0, 0 },
- { 0, 0 },
- { 1, 0 },
- { 2, 1 },
- { 2, 3 }, };
-
- pci_read_config_word(dev, master_port, &master_data);
- pci_read_config_byte(dev, 0x48, &udma_enable);
-
if (speed >= XFER_UDMA_0) {
- unsigned int udma = adev->dma_mode - XFER_UDMA_0;
+ unsigned int udma = speed - XFER_UDMA_0;
u16 udma_timing;
u16 ideconf;
int u_clock, u_speed;

+ pci_read_config_byte(dev, 0x48, &udma_enable);
+
/*
* UDMA is handled by a combination of clock switching and
* selection of dividers
@@ -227,50 +226,18 @@ static void rdc_set_dmamode(struct ata_port *ap, struct ata_device *adev)
ideconf &= ~(0x1001 << devid);
ideconf |= u_clock << devid;
pci_write_config_word(dev, 0x54, ideconf);
+
+ pci_write_config_byte(dev, 0x48, udma_enable);
} else {
- /*
- * MWDMA is driven by the PIO timings. We must also enable
- * IORDY unconditionally along with TIME1. PPE has already
- * been set when the PIO timing was set.
- */
- unsigned int mwdma = adev->dma_mode - XFER_MW_DMA_0;
- unsigned int control;
- u8 slave_data;
+ /* MWDMA is driven by the PIO timings. */
+ unsigned int mwdma = speed - XFER_MW_DMA_0;
const unsigned int needed_pio[3] = {
XFER_PIO_0, XFER_PIO_3, XFER_PIO_4
};
int pio = needed_pio[mwdma] - XFER_PIO_0;

- control = 3; /* IORDY|TIME1 */
-
- /* If the drive MWDMA is faster than it can do PIO then
- we must force PIO into PIO0 */
-
- if (adev->pio_mode < needed_pio[mwdma])
- /* Enable DMA timing only */
- control |= 8; /* PIO cycles in PIO0 */
-
- if (adev->devno) { /* Slave */
- master_data &= 0xFF4F; /* Mask out IORDY|TIME1|DMAONLY */
- master_data |= control << 4;
- pci_read_config_byte(dev, 0x44, &slave_data);
- slave_data &= (ap->port_no ? 0x0f : 0xf0);
- /* Load the matching timing */
- slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << (ap->port_no ? 4 : 0);
- pci_write_config_byte(dev, 0x44, slave_data);
- } else { /* Master */
- master_data &= 0xCCF4; /* Mask out IORDY|TIME1|DMAONLY
- and master timing bits */
- master_data |= control;
- master_data |=
- (timings[pio][0] << 12) |
- (timings[pio][1] << 8);
- }
-
- udma_enable &= ~(1 << devid);
- pci_write_config_word(dev, master_port, master_data);
+ rdc_set_timings(ap, adev, pio, 1);
}
- pci_write_config_byte(dev, 0x48, udma_enable);
}

static struct ata_port_operations rdc_pata_ops = {
--
1.7.1

Subject: [PATCH 12/20] pata_rdc: parallel scanning needs an extra locking

>From 540001238b8ebbc4bedd46ca4e2672fcf3787998 Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <[email protected]>
Date: Tue, 8 Feb 2011 12:39:27 +0100
Subject: [PATCH 12/20] pata_rdc: parallel scanning needs an extra locking

This is similar change as commit 60c3be3 for ata_piix host driver.

Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ata/pata_rdc.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/ata/pata_rdc.c b/drivers/ata/pata_rdc.c
index d32b252..f55ff82 100644
--- a/drivers/ata/pata_rdc.c
+++ b/drivers/ata/pata_rdc.c
@@ -86,10 +86,13 @@ static int rdc_pata_prereset(struct ata_link *link, unsigned long deadline)
return ata_sff_prereset(link, deadline);
}

+static DEFINE_SPINLOCK(rdc_lock);
+
static void rdc_set_timings(struct ata_port *ap, struct ata_device *adev,
u8 pio, bool use_mwdma)
{
struct pci_dev *dev = to_pci_dev(ap->host->dev);
+ unsigned long flags;
unsigned int is_slave = (adev->devno != 0);
unsigned int master_port= ap->port_no ? 0x42 : 0x40;
unsigned int slave_port = 0x44;
@@ -117,6 +120,8 @@ static void rdc_set_timings(struct ata_port *ap, struct ata_device *adev,
/* Enable DMA timing only */
control |= 8; /* PIO cycles in PIO0 */

+ spin_lock_irqsave(&rdc_lock, flags);
+
/* PIO configuration clears DTE unconditionally. It will be
* programmed in set_dmamode which is guaranteed to be called
* after set_piomode if any DMA mode is available.
@@ -154,6 +159,8 @@ static void rdc_set_timings(struct ata_port *ap, struct ata_device *adev,
pci_read_config_byte(dev, 0x48, &udma_enable);
udma_enable &= ~(1 << (2 * ap->port_no + adev->devno));
pci_write_config_byte(dev, 0x48, udma_enable);
+
+ spin_unlock_irqrestore(&rdc_lock, flags);
}

/**
@@ -186,6 +193,7 @@ static void rdc_set_piomode(struct ata_port *ap, struct ata_device *adev)
static void rdc_set_dmamode(struct ata_port *ap, struct ata_device *adev)
{
struct pci_dev *dev = to_pci_dev(ap->host->dev);
+ unsigned long flags;
u8 speed = adev->dma_mode;
int devid = adev->devno + 2 * ap->port_no;
u8 udma_enable = 0;
@@ -196,6 +204,8 @@ static void rdc_set_dmamode(struct ata_port *ap, struct ata_device *adev)
u16 ideconf;
int u_clock, u_speed;

+ spin_lock_irqsave(&rdc_lock, flags);
+
pci_read_config_byte(dev, 0x48, &udma_enable);

/*
@@ -228,6 +238,8 @@ static void rdc_set_dmamode(struct ata_port *ap, struct ata_device *adev)
pci_write_config_word(dev, 0x54, ideconf);

pci_write_config_byte(dev, 0x48, udma_enable);
+
+ spin_unlock_irqrestore(&rdc_lock, flags);
} else {
/* MWDMA is driven by the PIO timings. */
unsigned int mwdma = speed - XFER_MW_DMA_0;
--
1.7.1

Subject: [PATCH 13/20] pata_rdc: add Power Management support

>From fa27d376b5fa43f73305154c1ca58aa93e1d4138 Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <[email protected]>
Date: Tue, 8 Feb 2011 12:39:27 +0100
Subject: [PATCH 13/20] pata_rdc: add Power Management support

Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ata/pata_rdc.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/ata/pata_rdc.c b/drivers/ata/pata_rdc.c
index f55ff82..a4a0d88 100644
--- a/drivers/ata/pata_rdc.c
+++ b/drivers/ata/pata_rdc.c
@@ -357,6 +357,10 @@ static struct pci_driver rdc_pci_driver = {
.id_table = rdc_pci_tbl,
.probe = rdc_init_one,
.remove = rdc_remove_one,
+#ifdef CONFIG_PM
+ .suspend = ata_pci_device_suspend,
+ .resume = ata_pci_device_resume,
+#endif
};


--
1.7.1

Subject: [PATCH 14/20] pata_oldpiix: add locking for parallel scanning

>From bad44bb254a6f81776a373e9d46d30c67ef8f124 Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <[email protected]>
Date: Tue, 8 Feb 2011 12:39:27 +0100
Subject: [PATCH 14/20] pata_oldpiix: add locking for parallel scanning

Add an extra locking for parallel scanning.

This is similar change as commit 60c3be3 for ata_piix host driver
and while pata_oldpiix doesn't enable parallel scan yet the race
could probably also be triggered by requesting re-scanning of both
ports at the same time using SCSI sysfs interface.

Fix documentation while at it.

Acked-by: Alan Cox <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ata/pata_oldpiix.c | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/drivers/ata/pata_oldpiix.c b/drivers/ata/pata_oldpiix.c
index 433d2fc..845ede1 100644
--- a/drivers/ata/pata_oldpiix.c
+++ b/drivers/ata/pata_oldpiix.c
@@ -1,7 +1,8 @@
/*
- * pata_oldpiix.c - Intel PATA/SATA controllers
+ * pata_oldpiix.c - older Intel PATA controllers
*
* (C) 2005 Red Hat
+ * (C) 2010 Bartlomiej Zolnierkiewicz
*
* Some parts based on ata_piix.c by Jeff Garzik and others.
*
@@ -50,10 +51,13 @@ static int oldpiix_pre_reset(struct ata_link *link, unsigned long deadline)
return ata_sff_prereset(link, deadline);
}

+static DEFINE_SPINLOCK(piix_lock);
+
static void oldpiix_set_timings(struct ata_port *ap, struct ata_device *adev,
u8 pio, bool use_mwdma)
{
struct pci_dev *dev = to_pci_dev(ap->host->dev);
+ unsigned long flags;
unsigned int idetm_port= ap->port_no ? 0x42 : 0x40;
u16 idetm_data;
int control = 0;
@@ -84,6 +88,8 @@ static void oldpiix_set_timings(struct ata_port *ap, struct ata_device *adev,
/* Enable DMA timing only */
control |= 8; /* PIO cycles in PIO0 */

+ spin_lock_irqsave(&piix_lock, flags);
+
pci_read_config_word(dev, idetm_port, &idetm_data);

/*
@@ -101,6 +107,8 @@ static void oldpiix_set_timings(struct ata_port *ap, struct ata_device *adev,
(timings[pio][1] << 8);
pci_write_config_word(dev, idetm_port, idetm_data);

+ spin_unlock_irqrestore(&piix_lock, flags);
+
/* Track which port is configured */
ap->private_data = adev;
}
--
1.7.1

Subject: [PATCH 15/20] pata_oldpiix: enable parallel scan

>From 0343103ea4e3f8c35f938d9198f3ea265755ea6a Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <[email protected]>
Date: Tue, 8 Feb 2011 12:39:27 +0100
Subject: [PATCH 15/20] pata_oldpiix: enable parallel scan

Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ata/pata_oldpiix.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/ata/pata_oldpiix.c b/drivers/ata/pata_oldpiix.c
index 845ede1..819192d 100644
--- a/drivers/ata/pata_oldpiix.c
+++ b/drivers/ata/pata_oldpiix.c
@@ -222,7 +222,8 @@ static int oldpiix_init_one (struct pci_dev *pdev, const struct pci_device_id *e
dev_printk(KERN_DEBUG, &pdev->dev,
"version " DRV_VERSION "\n");

- return ata_pci_bmdma_init_one(pdev, ppi, &oldpiix_sht, NULL, 0);
+ return ata_pci_bmdma_init_one(pdev, ppi, &oldpiix_sht, NULL,
+ ATA_HOST_PARALLEL_SCAN);
}

static const struct pci_device_id oldpiix_pci_tbl[] = {
--
1.7.1

Subject: [PATCH 16/20] ata_piix: add EFAR SLC90E66 support

>From 11bed7feff5de752c9440ca58b232846b20e2ed6 Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <[email protected]>
Date: Tue, 8 Feb 2011 12:39:28 +0100
Subject: [PATCH 16/20] ata_piix: add EFAR SLC90E66 support

Add EFAR SLC90E66 support to ata_piix and remove no longer
needed pata_efar driver.

Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ata/Kconfig | 1 +
drivers/ata/Makefile | 1 -
drivers/ata/ata_piix.c | 74 ++++++++++--
drivers/ata/pata_efar.c | 292 -----------------------------------------------
4 files changed, 65 insertions(+), 303 deletions(-)
delete mode 100644 drivers/ata/pata_efar.c

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 36e2319..402c90c 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -375,6 +375,7 @@ config PATA_CYPRESS
config PATA_EFAR
tristate "EFAR SLC90E66 support"
depends on PCI
+ select ATA_PIIX
help
This option enables support for the EFAR SLC90E66
IDE controller found on some older machines.
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 2b67c90..b4252bc 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -40,7 +40,6 @@ obj-$(CONFIG_PATA_CS5530) += pata_cs5530.o
obj-$(CONFIG_PATA_CS5535) += pata_cs5535.o
obj-$(CONFIG_PATA_CS5536) += pata_cs5536.o
obj-$(CONFIG_PATA_CYPRESS) += pata_cypress.o
-obj-$(CONFIG_PATA_EFAR) += pata_efar.o
obj-$(CONFIG_PATA_HPT366) += pata_hpt366.o
obj-$(CONFIG_PATA_HPT37X) += pata_hpt37x.o
obj-$(CONFIG_PATA_HPT3X2N) += pata_hpt3x2n.o
diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index c954f91..3ce77d3 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -6,6 +6,7 @@
* on emails.
*
*
+ * Copyright 2009-2011 Bartlomiej Zolnierkiewicz
* Copyright 2003-2005 Red Hat Inc
* Copyright 2003-2005 Jeff Garzik
*
@@ -81,6 +82,10 @@
* ICH3 errata #15 - IDE deadlock under high load
* (BIOS must set dev 31 fn 0 bit 23)
* ICH3 errata #18 - Don't use native mode
+ *
+ * EFAR - a PIIX4 clone with UDMA66 support. Unlike the later
+ * Intel ICH controllers the EFAR widened the UDMA mode register bits
+ * and doesn't require the funky clock selection.
*/

#include <linux/kernel.h>
@@ -139,6 +144,7 @@ enum piix_controller_ids {
ich_pata_66, /* ICH up to 66 Mhz */
ich_pata_100, /* ICH up to UDMA 100 */
ich_pata_100_nomwdma1, /* ICH up to UDMA 100 but with no MWDMA1*/
+ efar_pata,
ich5_sata,
ich6_sata,
ich6m_sata,
@@ -169,6 +175,7 @@ static void piix_set_piomode(struct ata_port *ap, struct ata_device *adev);
static void piix_set_dmamode(struct ata_port *ap, struct ata_device *adev);
static void ich_set_dmamode(struct ata_port *ap, struct ata_device *adev);
static int ich_pata_cable_detect(struct ata_port *ap);
+static int efar_pata_cable_detect(struct ata_port *ap);
static u8 piix_vmw_bmdma_status(struct ata_port *ap);
static int piix_sidpr_scr_read(struct ata_link *link,
unsigned int reg, u32 *val);
@@ -229,6 +236,9 @@ static const struct pci_device_id piix_pci_tbl[] = {
/* ICH8 Mobile PATA Controller */
{ 0x8086, 0x2850, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },

+ /* EFAR */
+ { 0x1055, 0x9130, PCI_ANY_ID, PCI_ANY_ID, 0, 0, efar_pata },
+
/* SATA ports */

/* 82801EB (ICH5) */
@@ -369,6 +379,14 @@ static struct ata_port_operations piix_sidpr_sata_ops = {
.set_lpm = piix_sidpr_set_lpm,
};

+static struct ata_port_operations efar_pata_ops = {
+ .inherits = &ata_bmdma_port_ops,
+ .set_piomode = piix_set_piomode,
+ .set_dmamode = piix_set_dmamode,
+ .prereset = piix_pata_prereset,
+ .cable_detect = efar_pata_cable_detect,
+};
+
static const struct piix_map_db ich5_map_db = {
.mask = 0x7,
.port_enable = 0x3,
@@ -598,6 +616,14 @@ static struct ata_port_info piix_port_info[] = {
.port_ops = &piix_vmw_ops,
},

+ [efar_pata] =
+ {
+ .flags = PIIX_PATA_FLAGS,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA12_ONLY,
+ .udma_mask = ATA_UDMA4,
+ .port_ops = &efar_pata_ops,
+ },
};

static struct pci_bits piix_enable_bits[] = {
@@ -610,6 +636,7 @@ MODULE_DESCRIPTION("SCSI low-level driver for Intel PIIX/ICH ATA controllers");
MODULE_LICENSE("GPL");
MODULE_DEVICE_TABLE(pci, piix_pci_tbl);
MODULE_VERSION(DRV_VERSION);
+MODULE_ALIAS("pata_efar");

struct ich_laptop {
u16 device;
@@ -677,6 +704,25 @@ static int ich_pata_cable_detect(struct ata_port *ap)
}

/**
+ * efar_pata_cable_detect - check for 40/80 pin
+ * @ap: Port
+ *
+ * Perform cable detection for the EFAR ATA interface. This is
+ * different to the PIIX arrangement
+ */
+
+static int efar_pata_cable_detect(struct ata_port *ap)
+{
+ struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+ u8 tmp;
+
+ pci_read_config_byte(pdev, 0x47, &tmp);
+ if (tmp & (2 >> ap->port_no))
+ return ATA_CBL_PATA40;
+ return ATA_CBL_PATA80;
+}
+
+/**
* piix_pata_prereset - prereset for PATA host controller
* @link: Target link
* @deadline: deadline jiffies for the operation
@@ -797,12 +843,12 @@ static void piix_set_piomode(struct ata_port *ap, struct ata_device *adev)
}

/**
- * do_pata_set_dmamode - Initialize host controller PATA PIO timings
+ * do_pata_set_dmamode - Initialize host controller PATA DMA timings
* @ap: Port whose timings we are configuring
* @adev: Drive in question
* @isich: set if the chip is an ICH device
*
- * Set UDMA mode for device, in host controller PCI config space.
+ * Set UDMA/MWDMA mode for device, in host controller PCI config space.
*
* LOCKING:
* None (inherited from caller).
@@ -843,10 +889,16 @@ static void do_pata_set_dmamode(struct ata_port *ap, struct ata_device *adev, in

udma_enable |= (1 << devid);

- /* Load the CT/RP selection */
pci_read_config_word(dev, 0x4A, &udma_timing);
- udma_timing &= ~(3 << (4 * devid));
- udma_timing |= u_speed << (4 * devid);
+ if (dev->vendor == PCI_VENDOR_ID_EFAR) {
+ /* Load the UDMA mode number */
+ udma_timing &= ~(7 << (4 * devid));
+ udma_timing |= udma << (4 * devid);
+ } else {
+ /* Load the CT/RP selection */
+ udma_timing &= ~(3 << (4 * devid));
+ udma_timing |= u_speed << (4 * devid);
+ }
pci_write_config_word(dev, 0x4A, udma_timing);

if (isich) {
@@ -877,7 +929,7 @@ static void do_pata_set_dmamode(struct ata_port *ap, struct ata_device *adev, in
/**
* piix_set_dmamode - Initialize host controller PATA DMA timings
* @ap: Port whose timings we are configuring
- * @adev: um
+ * @adev: Device to program
*
* Set MW/UDMA mode for device, in host controller PCI config space.
*
@@ -893,7 +945,7 @@ static void piix_set_dmamode(struct ata_port *ap, struct ata_device *adev)
/**
* ich_set_dmamode - Initialize host controller PATA DMA timings
* @ap: Port whose timings we are configuring
- * @adev: um
+ * @adev: Device to program
*
* Set MW/UDMA mode for device, in host controller PCI config space.
*
@@ -1561,7 +1613,8 @@ static int __devinit piix_init_one(struct pci_dev *pdev,
* because some ACPI implementations mess up cable related
* bits on _STM. Reported on kernel bz#11879.
*/
- pci_read_config_dword(pdev, PIIX_IOCFG, &hpriv->saved_iocfg);
+ if (pdev->vendor == PCI_VENDOR_ID_INTEL)
+ pci_read_config_dword(pdev, PIIX_IOCFG, &hpriv->saved_iocfg);

/* ICH6R may be driven by either ata_piix or ahci driver
* regardless of BIOS configuration. Make sure AHCI mode is
@@ -1625,7 +1678,8 @@ static void piix_remove_one(struct pci_dev *pdev)
struct ata_host *host = dev_get_drvdata(&pdev->dev);
struct piix_host_priv *hpriv = host->private_data;

- pci_write_config_dword(pdev, PIIX_IOCFG, hpriv->saved_iocfg);
+ if (pdev->vendor == PCI_VENDOR_ID_INTEL)
+ pci_write_config_dword(pdev, PIIX_IOCFG, hpriv->saved_iocfg);

ata_pci_remove_one(pdev);
}
diff --git a/drivers/ata/pata_efar.c b/drivers/ata/pata_efar.c
deleted file mode 100644
index 7f564d7..0000000
--- a/drivers/ata/pata_efar.c
+++ /dev/null
@@ -1,292 +0,0 @@
-/*
- * pata_efar.c - EFAR PIIX clone controller driver
- *
- * (C) 2005 Red Hat
- * (C) 2009-2010 Bartlomiej Zolnierkiewicz
- *
- * Some parts based on ata_piix.c by Jeff Garzik and others.
- *
- * The EFAR is a PIIX4 clone with UDMA66 support. Unlike the later
- * Intel ICH controllers the EFAR widened the UDMA mode register bits
- * and doesn't require the funky clock selection.
- */
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/pci.h>
-#include <linux/init.h>
-#include <linux/blkdev.h>
-#include <linux/delay.h>
-#include <linux/device.h>
-#include <scsi/scsi_host.h>
-#include <linux/libata.h>
-#include <linux/ata.h>
-
-#define DRV_NAME "pata_efar"
-#define DRV_VERSION "0.4.5"
-
-/**
- * efar_pre_reset - Enable bits
- * @link: ATA link
- * @deadline: deadline jiffies for the operation
- *
- * Perform cable detection for the EFAR ATA interface. This is
- * different to the PIIX arrangement
- */
-
-static int efar_pre_reset(struct ata_link *link, unsigned long deadline)
-{
- static const struct pci_bits efar_enable_bits[] = {
- { 0x41U, 1U, 0x80UL, 0x80UL }, /* port 0 */
- { 0x43U, 1U, 0x80UL, 0x80UL }, /* port 1 */
- };
- struct ata_port *ap = link->ap;
- struct pci_dev *pdev = to_pci_dev(ap->host->dev);
-
- if (!pci_test_config_bits(pdev, &efar_enable_bits[ap->port_no]))
- return -ENOENT;
-
- return ata_sff_prereset(link, deadline);
-}
-
-/**
- * efar_cable_detect - check for 40/80 pin
- * @ap: Port
- *
- * Perform cable detection for the EFAR ATA interface. This is
- * different to the PIIX arrangement
- */
-
-static int efar_cable_detect(struct ata_port *ap)
-{
- struct pci_dev *pdev = to_pci_dev(ap->host->dev);
- u8 tmp;
-
- pci_read_config_byte(pdev, 0x47, &tmp);
- if (tmp & (2 >> ap->port_no))
- return ATA_CBL_PATA40;
- return ATA_CBL_PATA80;
-}
-
-static DEFINE_SPINLOCK(efar_lock);
-
-static void efar_set_timings(struct ata_port *ap, struct ata_device *adev,
- u8 pio, bool use_mwdma)
-{
- struct pci_dev *dev = to_pci_dev(ap->host->dev);
- unsigned int is_slave = (adev->devno != 0);
- unsigned long flags;
- u8 master_port = ap->port_no ? 0x42 : 0x40;
- u16 master_data;
- u8 udma_enable;
- u8 slave_data;
- int control = 0;
-
- /*
- * See Intel Document 298600-004 for the timing programing rules
- * for PIIX/ICH. The EFAR is a clone so very similar
- */
-
- static const /* ISP RTC */
- u8 timings[][2] = { { 0, 0 },
- { 0, 0 },
- { 1, 0 },
- { 2, 1 },
- { 2, 3 }, };
-
- if (pio > 1 || use_mwdma)
- control |= 1; /* TIME */
- if (ata_pio_need_iordy(adev) || use_mwdma)
- control |= 2; /* IE */
- /* Intel specifies that the prefetch/posting is for disk only */
- if (adev->class == ATA_DEV_ATA)
- control |= 4; /* PPE */
- /* If the drive MWDMA is faster than it can do PIO then
- we must force PIO into PIO0 */
- if (use_mwdma && adev->pio_mode < (XFER_PIO_0 + pio))
- /* Enable DMA timing only */
- control |= 8; /* PIO cycles in PIO0 */
-
- spin_lock_irqsave(&efar_lock, flags);
-
- pci_read_config_word(dev, master_port, &master_data);
-
- /* Set PPE, IE, and TIME as appropriate */
- if (is_slave == 0) {
- master_data &= 0xCCF0;
- master_data |= control;
- master_data |= (timings[pio][0] << 12) |
- (timings[pio][1] << 8);
- } else {
- int shift = 4 * ap->port_no;
-
- master_data &= 0xFF0F;
- master_data |= (control << 4);
-
- /* Slave timing in separate register */
- pci_read_config_byte(dev, 0x44, &slave_data);
- slave_data &= ap->port_no ? 0x0F : 0xF0;
- slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << shift;
- }
-
- master_data |= 0x4000; /* Ensure SITRE is set */
- pci_write_config_word(dev, master_port, master_data);
-
- if (is_slave)
- pci_write_config_byte(dev, 0x44, slave_data);
-
- pci_read_config_byte(dev, 0x48, &udma_enable);
- udma_enable &= ~(1 << (2 * ap->port_no + adev->devno));
- pci_write_config_byte(dev, 0x48, udma_enable);
- spin_unlock_irqrestore(&efar_lock, flags);
-}
-
-/**
- * efar_set_piomode - Initialize host controller PATA PIO timings
- * @ap: Port whose timings we are configuring
- * @adev: Device to program
- *
- * Set PIO mode for device, in host controller PCI config space.
- *
- * LOCKING:
- * None (inherited from caller).
- */
-
-static void efar_set_piomode(struct ata_port *ap, struct ata_device *adev)
-{
- efar_set_timings(ap, adev, adev->pio_mode - XFER_PIO_0, 0);
-}
-
-/**
- * efar_set_dmamode - Initialize host controller PATA DMA timings
- * @ap: Port whose timings we are configuring
- * @adev: Device to program
- *
- * Set UDMA/MWDMA mode for device, in host controller PCI config space.
- *
- * LOCKING:
- * None (inherited from caller).
- */
-
-static void efar_set_dmamode (struct ata_port *ap, struct ata_device *adev)
-{
- struct pci_dev *dev = to_pci_dev(ap->host->dev);
- u8 speed = adev->dma_mode;
- int devid = adev->devno + 2 * ap->port_no;
- unsigned long flags;
- u8 udma_enable;
-
- if (speed >= XFER_UDMA_0) {
- unsigned int udma = speed - XFER_UDMA_0;
- u16 udma_timing;
-
- spin_lock_irqsave(&efar_lock, flags);
-
- pci_read_config_byte(dev, 0x48, &udma_enable);
-
- udma_enable |= (1 << devid);
-
- /* Load the UDMA mode number */
- pci_read_config_word(dev, 0x4A, &udma_timing);
- udma_timing &= ~(7 << (4 * devid));
- udma_timing |= udma << (4 * devid);
- pci_write_config_word(dev, 0x4A, udma_timing);
-
- pci_write_config_byte(dev, 0x48, udma_enable);
-
- spin_unlock_irqrestore(&efar_lock, flags);
- } else {
- /* MWDMA is driven by the PIO timings. */
- unsigned int mwdma = speed - XFER_MW_DMA_0;
- const unsigned int needed_pio[3] = {
- XFER_PIO_0, XFER_PIO_3, XFER_PIO_4
- };
- int pio = needed_pio[mwdma] - XFER_PIO_0;
-
- efar_set_timings(ap, adev, pio, 1);
- }
-}
-
-static struct scsi_host_template efar_sht = {
- ATA_BMDMA_SHT(DRV_NAME),
-};
-
-static struct ata_port_operations efar_ops = {
- .inherits = &ata_bmdma_port_ops,
- .cable_detect = efar_cable_detect,
- .set_piomode = efar_set_piomode,
- .set_dmamode = efar_set_dmamode,
- .prereset = efar_pre_reset,
-};
-
-
-/**
- * efar_init_one - Register EFAR ATA PCI device with kernel services
- * @pdev: PCI device to register
- * @ent: Entry in efar_pci_tbl matching with @pdev
- *
- * Called from kernel PCI layer.
- *
- * LOCKING:
- * Inherited from PCI layer (may sleep).
- *
- * RETURNS:
- * Zero on success, or -ERRNO value.
- */
-
-static int efar_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
-{
- static int printed_version;
- static const struct ata_port_info info = {
- .flags = ATA_FLAG_SLAVE_POSS,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA12_ONLY,
- .udma_mask = ATA_UDMA4,
- .port_ops = &efar_ops,
- };
- const struct ata_port_info *ppi[] = { &info, &info };
-
- if (!printed_version++)
- dev_printk(KERN_DEBUG, &pdev->dev,
- "version " DRV_VERSION "\n");
-
- return ata_pci_bmdma_init_one(pdev, ppi, &efar_sht, NULL,
- ATA_HOST_PARALLEL_SCAN);
-}
-
-static const struct pci_device_id efar_pci_tbl[] = {
- { PCI_VDEVICE(EFAR, 0x9130), },
-
- { } /* terminate list */
-};
-
-static struct pci_driver efar_pci_driver = {
- .name = DRV_NAME,
- .id_table = efar_pci_tbl,
- .probe = efar_init_one,
- .remove = ata_pci_remove_one,
-#ifdef CONFIG_PM
- .suspend = ata_pci_device_suspend,
- .resume = ata_pci_device_resume,
-#endif
-};
-
-static int __init efar_init(void)
-{
- return pci_register_driver(&efar_pci_driver);
-}
-
-static void __exit efar_exit(void)
-{
- pci_unregister_driver(&efar_pci_driver);
-}
-
-module_init(efar_init);
-module_exit(efar_exit);
-
-MODULE_AUTHOR("Alan Cox");
-MODULE_DESCRIPTION("SCSI low-level driver for EFAR PIIX clones");
-MODULE_LICENSE("GPL");
-MODULE_DEVICE_TABLE(pci, efar_pci_tbl);
-MODULE_VERSION(DRV_VERSION);
-
--
1.7.1

Subject: [PATCH 17/20] ata_piix: add IT8213 support

>From ecf383059c866b4ab646a4bf1fae4abe4fade58a Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <[email protected]>
Date: Tue, 8 Feb 2011 12:39:28 +0100
Subject: [PATCH 17/20] ata_piix: add IT8213 support

Add IT8213 support to ata_piix and remove no longer
needed pata_it8213 driver.

Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ata/Kconfig | 1 +
drivers/ata/Makefile | 1 -
drivers/ata/ata_piix.c | 65 ++++++++++-
drivers/ata/pata_it8213.c | 284 ---------------------------------------------
4 files changed, 62 insertions(+), 289 deletions(-)
delete mode 100644 drivers/ata/pata_it8213.c

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 402c90c..5a92f24 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -437,6 +437,7 @@ config PATA_ICSIDE
config PATA_IT8213
tristate "IT8213 PATA support (Experimental)"
depends on PCI && EXPERIMENTAL
+ select ATA_PIIX
help
This option enables support for the ITE 821 PATA
controllers via the new ATA layer.
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index b4252bc..7e78491 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -45,7 +45,6 @@ obj-$(CONFIG_PATA_HPT37X) += pata_hpt37x.o
obj-$(CONFIG_PATA_HPT3X2N) += pata_hpt3x2n.o
obj-$(CONFIG_PATA_HPT3X3) += pata_hpt3x3.o
obj-$(CONFIG_PATA_ICSIDE) += pata_icside.o
-obj-$(CONFIG_PATA_IT8213) += pata_it8213.o
obj-$(CONFIG_PATA_IT821X) += pata_it821x.o
obj-$(CONFIG_PATA_JMICRON) += pata_jmicron.o
obj-$(CONFIG_PATA_MACIO) += pata_macio.o
diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index 3ce77d3..deb7c96 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -86,6 +86,11 @@
* EFAR - a PIIX4 clone with UDMA66 support. Unlike the later
* Intel ICH controllers the EFAR widened the UDMA mode register bits
* and doesn't require the funky clock selection.
+ *
+ * IT8213 - a very Intel ICH like device for timing purposes, having
+ * a similar register layout and the same split clock arrangement. Cable
+ * detection is different, and it does not have slave channels or all the
+ * clutter of later ICH/SATA setups.
*/

#include <linux/kernel.h>
@@ -145,6 +150,7 @@ enum piix_controller_ids {
ich_pata_100, /* ICH up to UDMA 100 */
ich_pata_100_nomwdma1, /* ICH up to UDMA 100 but with no MWDMA1*/
efar_pata,
+ it8213_pata,
ich5_sata,
ich6_sata,
ich6m_sata,
@@ -176,6 +182,7 @@ static void piix_set_dmamode(struct ata_port *ap, struct ata_device *adev);
static void ich_set_dmamode(struct ata_port *ap, struct ata_device *adev);
static int ich_pata_cable_detect(struct ata_port *ap);
static int efar_pata_cable_detect(struct ata_port *ap);
+static int it8213_pata_cable_detect(struct ata_port *ap);
static u8 piix_vmw_bmdma_status(struct ata_port *ap);
static int piix_sidpr_scr_read(struct ata_link *link,
unsigned int reg, u32 *val);
@@ -239,6 +246,9 @@ static const struct pci_device_id piix_pci_tbl[] = {
/* EFAR */
{ 0x1055, 0x9130, PCI_ANY_ID, PCI_ANY_ID, 0, 0, efar_pata },

+ /* IT8213 */
+ { 0x1283, 0x8213, PCI_ANY_ID, PCI_ANY_ID, 0, 0, it8213_pata },
+
/* SATA ports */

/* 82801EB (ICH5) */
@@ -387,6 +397,14 @@ static struct ata_port_operations efar_pata_ops = {
.cable_detect = efar_pata_cable_detect,
};

+static struct ata_port_operations it8213_pata_ops = {
+ .inherits = &ata_bmdma_port_ops,
+ .set_piomode = piix_set_piomode,
+ .set_dmamode = ich_set_dmamode,
+ .prereset = piix_pata_prereset,
+ .cable_detect = it8213_pata_cable_detect,
+};
+
static const struct piix_map_db ich5_map_db = {
.mask = 0x7,
.port_enable = 0x3,
@@ -624,6 +642,15 @@ static struct ata_port_info piix_port_info[] = {
.udma_mask = ATA_UDMA4,
.port_ops = &efar_pata_ops,
},
+
+ [it8213_pata] =
+ {
+ .flags = PIIX_PATA_FLAGS,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA12_ONLY,
+ .udma_mask = ATA_UDMA6,
+ .port_ops = &it8213_pata_ops,
+ },
};

static struct pci_bits piix_enable_bits[] = {
@@ -637,6 +664,7 @@ MODULE_LICENSE("GPL");
MODULE_DEVICE_TABLE(pci, piix_pci_tbl);
MODULE_VERSION(DRV_VERSION);
MODULE_ALIAS("pata_efar");
+MODULE_ALIAS("pata_it8213");

struct ich_laptop {
u16 device;
@@ -723,6 +751,25 @@ static int efar_pata_cable_detect(struct ata_port *ap)
}

/**
+ * it8213_pata_cable_detect - check for 40/80 pin
+ * @ap: Port
+ *
+ * Perform cable detection for the 8213 ATA interface. This is
+ * different to the PIIX arrangement
+ */
+
+static int it8213_pata_cable_detect(struct ata_port *ap)
+{
+ struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+ u8 tmp;
+
+ pci_read_config_byte(pdev, 0x42, &tmp);
+ if (tmp & 2) /* The initial docs are incorrect */
+ return ATA_CBL_PATA40;
+ return ATA_CBL_PATA80;
+}
+
+/**
* piix_pata_prereset - prereset for PATA host controller
* @link: Target link
* @deadline: deadline jiffies for the operation
@@ -771,9 +818,15 @@ static void piix_set_timings(struct ata_port *ap, struct ata_device *adev,
control |= 1; /* TIME1 enable */
if (ata_pio_need_iordy(adev) || use_mwdma)
control |= 2; /* IE enable */
- /* Intel specifies that the PPE functionality is for disk only */
- if (adev->class == ATA_DEV_ATA)
- control |= 4; /* PPE enable */
+ if (dev->vendor != PCI_VENDOR_ID_ITE) {
+ /* PPE functionality is for disk only */
+ if (adev->class == ATA_DEV_ATA)
+ control |= 4; /* PPE enable */
+ } else {
+ /* Bit 2 is set for ATAPI on the IT8213 - reverse of ICH/PIIX */
+ if (adev->class != ATA_DEV_ATA)
+ control |= 4; /* PPE enable */
+ }
/* If the drive MWDMA is faster than it can do PIO then
we must force PIO into PIO0 */
if (use_mwdma && adev->pio_mode < (XFER_PIO_0 + pio))
@@ -880,7 +933,7 @@ static void do_pata_set_dmamode(struct ata_port *ap, struct ata_device *adev, in
* except UDMA0 which is 00
*/
u_speed = min(2 - (udma & 1), udma);
- if (udma == 5)
+ if (udma > 4)
u_clock = 0x1000; /* 100Mhz */
else if (udma > 2)
u_clock = 1; /* 66Mhz */
@@ -1597,6 +1650,10 @@ static int __devinit piix_init_one(struct pci_dev *pdev,
port_info[0] = piix_port_info[ent->driver_data];
port_info[1] = piix_port_info[ent->driver_data];

+ /* IT8213 is a single port device. */
+ if (pdev->vendor == PCI_VENDOR_ID_ITE)
+ port_info[1] = ata_dummy_port_info;
+
port_flags = port_info[0].flags;

/* enable device and prepare host */
diff --git a/drivers/ata/pata_it8213.c b/drivers/ata/pata_it8213.c
deleted file mode 100644
index e045eab..0000000
--- a/drivers/ata/pata_it8213.c
+++ /dev/null
@@ -1,284 +0,0 @@
-/*
- * pata_it8213.c - iTE Tech. Inc. IT8213 PATA driver
- *
- * The IT8213 is a very Intel ICH like device for timing purposes, having
- * a similar register layout and the same split clock arrangement. Cable
- * detection is different, and it does not have slave channels or all the
- * clutter of later ICH/SATA setups.
- */
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/pci.h>
-#include <linux/init.h>
-#include <linux/blkdev.h>
-#include <linux/delay.h>
-#include <linux/device.h>
-#include <scsi/scsi_host.h>
-#include <linux/libata.h>
-#include <linux/ata.h>
-
-#define DRV_NAME "pata_it8213"
-#define DRV_VERSION "0.0.3"
-
-/**
- * it8213_pre_reset - probe begin
- * @link: link
- * @deadline: deadline jiffies for the operation
- *
- * Filter out ports by the enable bits before doing the normal reset
- * and probe.
- */
-
-static int it8213_pre_reset(struct ata_link *link, unsigned long deadline)
-{
- static const struct pci_bits it8213_enable_bits[] = {
- { 0x41U, 1U, 0x80UL, 0x80UL }, /* port 0 */
- };
- struct ata_port *ap = link->ap;
- struct pci_dev *pdev = to_pci_dev(ap->host->dev);
- if (!pci_test_config_bits(pdev, &it8213_enable_bits[ap->port_no]))
- return -ENOENT;
-
- return ata_sff_prereset(link, deadline);
-}
-
-/**
- * it8213_cable_detect - check for 40/80 pin
- * @ap: Port
- *
- * Perform cable detection for the 8213 ATA interface. This is
- * different to the PIIX arrangement
- */
-
-static int it8213_cable_detect(struct ata_port *ap)
-{
- struct pci_dev *pdev = to_pci_dev(ap->host->dev);
- u8 tmp;
- pci_read_config_byte(pdev, 0x42, &tmp);
- if (tmp & 2) /* The initial docs are incorrect */
- return ATA_CBL_PATA40;
- return ATA_CBL_PATA80;
-}
-
-static void it8213_set_timings(struct ata_port *ap, struct ata_device *adev,
- u8 pio, bool use_mwdma)
-{
- struct pci_dev *dev = to_pci_dev(ap->host->dev);
- u8 master_port = ap->port_no ? 0x42 : 0x40;
- u16 master_data;
- int control = 0;
-
- /*
- * See Intel Document 298600-004 for the timing programing rules
- * for PIIX/ICH. The 8213 is a clone so very similar
- */
-
- static const /* ISP RTC */
- u8 timings[][2] = { { 0, 0 },
- { 0, 0 },
- { 1, 0 },
- { 2, 1 },
- { 2, 3 }, };
-
- if (pio > 1 || use_mwdma)
- control |= 1; /* TIME */
- if (ata_pio_need_iordy(adev) || use_mwdma)
- control |= 2; /* IE */
- /* Bit 2 is set for ATAPI on the IT8213 - reverse of ICH/PIIX */
- if (adev->class != ATA_DEV_ATA)
- control |= 4; /* PPE */
- /* If the drive MWDMA is faster than it can do PIO then
- we must force PIO into PIO0 */
- if (use_mwdma && adev->pio_mode < (XFER_PIO_0 + pio))
- /* Enable DMA timing only */
- control |= 8; /* PIO cycles in PIO0 */
-
- pci_read_config_word(dev, master_port, &master_data);
-
- /* Set PPE, IE, and TIME as appropriate */
- if (adev->devno == 0) {
- master_data &= 0xCCF0;
- master_data |= control;
- master_data |= (timings[pio][0] << 12) |
- (timings[pio][1] << 8);
- } else {
- u8 slave_data;
-
- master_data &= 0xFF0F;
- master_data |= (control << 4);
-
- /* Slave timing in separate register */
- pci_read_config_byte(dev, 0x44, &slave_data);
- slave_data &= 0xF0;
- slave_data |= (timings[pio][0] << 2) | timings[pio][1];
- pci_write_config_byte(dev, 0x44, slave_data);
- }
-
- master_data |= 0x4000; /* Ensure SITRE is set */
- pci_write_config_word(dev, master_port, master_data);
-}
-
-/**
- * it8213_set_piomode - Initialize host controller PATA PIO timings
- * @ap: Port whose timings we are configuring
- * @adev: Device whose timings we are configuring
- *
- * Set PIO mode for device, in host controller PCI config space.
- *
- * LOCKING:
- * None (inherited from caller).
- */
-
-static void it8213_set_piomode(struct ata_port *ap, struct ata_device *adev)
-{
- it8213_set_timings(ap, adev, adev->pio_mode - XFER_PIO_0, 0);
-}
-
-/**
- * it8213_set_dmamode - Initialize host controller PATA DMA timings
- * @ap: Port whose timings we are configuring
- * @adev: Device to program
- *
- * Set UDMA/MWDMA mode for device, in host controller PCI config space.
- * This device is basically an ICH alike.
- *
- * LOCKING:
- * None (inherited from caller).
- */
-
-static void it8213_set_dmamode (struct ata_port *ap, struct ata_device *adev)
-{
- struct pci_dev *dev = to_pci_dev(ap->host->dev);
- u8 speed = adev->dma_mode;
- int devid = adev->devno;
- u8 udma_enable;
-
- pci_read_config_byte(dev, 0x48, &udma_enable);
-
- if (speed >= XFER_UDMA_0) {
- unsigned int udma = speed - XFER_UDMA_0;
- u16 udma_timing;
- u16 ideconf;
- int u_clock, u_speed;
-
- /* Clocks follow the PIIX style */
- u_speed = min(2 - (udma & 1), udma);
- if (udma > 4)
- u_clock = 0x1000; /* 100Mhz */
- else if (udma > 2)
- u_clock = 1; /* 66Mhz */
- else
- u_clock = 0; /* 33Mhz */
-
- udma_enable |= (1 << devid);
-
- /* Load the UDMA cycle time */
- pci_read_config_word(dev, 0x4A, &udma_timing);
- udma_timing &= ~(3 << (4 * devid));
- udma_timing |= u_speed << (4 * devid);
- pci_write_config_word(dev, 0x4A, udma_timing);
-
- /* Load the clock selection */
- pci_read_config_word(dev, 0x54, &ideconf);
- ideconf &= ~(0x1001 << devid);
- ideconf |= u_clock << devid;
- pci_write_config_word(dev, 0x54, ideconf);
- } else {
- /* MWDMA is driven by the PIO timings. */
- unsigned int mwdma = speed - XFER_MW_DMA_0;
- static const unsigned int needed_pio[3] = {
- XFER_PIO_0, XFER_PIO_3, XFER_PIO_4
- };
- int pio = needed_pio[mwdma] - XFER_PIO_0;
-
- it8213_set_timings(ap, adev, pio, 1);
-
- udma_enable &= ~(1 << devid);
- }
- pci_write_config_byte(dev, 0x48, udma_enable);
-}
-
-static struct scsi_host_template it8213_sht = {
- ATA_BMDMA_SHT(DRV_NAME),
-};
-
-
-static struct ata_port_operations it8213_ops = {
- .inherits = &ata_bmdma_port_ops,
- .cable_detect = it8213_cable_detect,
- .set_piomode = it8213_set_piomode,
- .set_dmamode = it8213_set_dmamode,
- .prereset = it8213_pre_reset,
-};
-
-
-/**
- * it8213_init_one - Register 8213 ATA PCI device with kernel services
- * @pdev: PCI device to register
- * @ent: Entry in it8213_pci_tbl matching with @pdev
- *
- * Called from kernel PCI layer.
- *
- * LOCKING:
- * Inherited from PCI layer (may sleep).
- *
- * RETURNS:
- * Zero on success, or -ERRNO value.
- */
-
-static int it8213_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
-{
- static int printed_version;
- static const struct ata_port_info info = {
- .flags = ATA_FLAG_SLAVE_POSS,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA12_ONLY,
- .udma_mask = ATA_UDMA6,
- .port_ops = &it8213_ops,
- };
- /* Current IT8213 stuff is single port */
- const struct ata_port_info *ppi[] = { &info, &ata_dummy_port_info };
-
- if (!printed_version++)
- dev_printk(KERN_DEBUG, &pdev->dev,
- "version " DRV_VERSION "\n");
-
- return ata_pci_bmdma_init_one(pdev, ppi, &it8213_sht, NULL, 0);
-}
-
-static const struct pci_device_id it8213_pci_tbl[] = {
- { PCI_VDEVICE(ITE, PCI_DEVICE_ID_ITE_8213), },
-
- { } /* terminate list */
-};
-
-static struct pci_driver it8213_pci_driver = {
- .name = DRV_NAME,
- .id_table = it8213_pci_tbl,
- .probe = it8213_init_one,
- .remove = ata_pci_remove_one,
-#ifdef CONFIG_PM
- .suspend = ata_pci_device_suspend,
- .resume = ata_pci_device_resume,
-#endif
-};
-
-static int __init it8213_init(void)
-{
- return pci_register_driver(&it8213_pci_driver);
-}
-
-static void __exit it8213_exit(void)
-{
- pci_unregister_driver(&it8213_pci_driver);
-}
-
-module_init(it8213_init);
-module_exit(it8213_exit);
-
-MODULE_AUTHOR("Alan Cox");
-MODULE_DESCRIPTION("SCSI low-level driver for the ITE 8213");
-MODULE_LICENSE("GPL");
-MODULE_DEVICE_TABLE(pci, it8213_pci_tbl);
-MODULE_VERSION(DRV_VERSION);
--
1.7.1

Subject: [PATCH 18/20] ata_piix: add RDC support

>From 4cc4234e2049fa6e97d5a2d3643bb5499e52f838 Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <[email protected]>
Date: Tue, 8 Feb 2011 12:39:28 +0100
Subject: [PATCH 18/20] ata_piix: add RDC support

Add RDC PATA support to ata_piix and remove no longer
needed pata_rdc driver.

Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ata/Kconfig | 1 +
drivers/ata/Makefile | 1 -
drivers/ata/ata_piix.c | 14 ++-
drivers/ata/pata_rdc.c | 384 ------------------------------------------------
4 files changed, 13 insertions(+), 387 deletions(-)
delete mode 100644 drivers/ata/pata_rdc.c

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 5a92f24..dc9038d 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -567,6 +567,7 @@ config PATA_RADISYS
config PATA_RDC
tristate "RDC PATA support"
depends on PCI
+ select ATA_PIIX
help
This option enables basic support for the later RDC PATA controllers
controllers via the new ATA layer. For the RDC 1010, you need to
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 7e78491..e627f6d 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -58,7 +58,6 @@ obj-$(CONFIG_PATA_OPTIDMA) += pata_optidma.o
obj-$(CONFIG_PATA_PDC2027X) += pata_pdc2027x.o
obj-$(CONFIG_PATA_PDC_OLD) += pata_pdc202xx_old.o
obj-$(CONFIG_PATA_RADISYS) += pata_radisys.o
-obj-$(CONFIG_PATA_RDC) += pata_rdc.o
obj-$(CONFIG_PATA_SC1200) += pata_sc1200.o
obj-$(CONFIG_PATA_SCC) += pata_scc.o
obj-$(CONFIG_PATA_SCH) += pata_sch.o
diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index deb7c96..86d12c2 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -1,6 +1,9 @@
/*
* ata_piix.c - Intel PATA/SATA controllers
*
+ * This is actually a driver for hardware meeting
+ * INCITS 370-2004 (1510D): ATA Host Adapter Standards
+ *
* Maintained by: Jeff Garzik <[email protected]>
* Please ALWAYS copy [email protected]
* on emails.
@@ -249,6 +252,10 @@ static const struct pci_device_id piix_pci_tbl[] = {
/* IT8213 */
{ 0x1283, 0x8213, PCI_ANY_ID, PCI_ANY_ID, 0, 0, it8213_pata },

+ /* RDC is ICH like */
+ { 0x17F3, 0x1011, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
+ { 0x17F3, 0x1012, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
+
/* SATA ports */

/* 82801EB (ICH5) */
@@ -665,6 +672,7 @@ MODULE_DEVICE_TABLE(pci, piix_pci_tbl);
MODULE_VERSION(DRV_VERSION);
MODULE_ALIAS("pata_efar");
MODULE_ALIAS("pata_it8213");
+MODULE_ALIAS("pata_rdc");

struct ich_laptop {
u16 device;
@@ -1670,7 +1678,8 @@ static int __devinit piix_init_one(struct pci_dev *pdev,
* because some ACPI implementations mess up cable related
* bits on _STM. Reported on kernel bz#11879.
*/
- if (pdev->vendor == PCI_VENDOR_ID_INTEL)
+ if (pdev->vendor == PCI_VENDOR_ID_INTEL ||
+ pdev->vendor == PCI_VENDOR_ID_RDC)
pci_read_config_dword(pdev, PIIX_IOCFG, &hpriv->saved_iocfg);

/* ICH6R may be driven by either ata_piix or ahci driver
@@ -1735,7 +1744,8 @@ static void piix_remove_one(struct pci_dev *pdev)
struct ata_host *host = dev_get_drvdata(&pdev->dev);
struct piix_host_priv *hpriv = host->private_data;

- if (pdev->vendor == PCI_VENDOR_ID_INTEL)
+ if (pdev->vendor == PCI_VENDOR_ID_INTEL ||
+ pdev->vendor == PCI_VENDOR_ID_RDC)
pci_write_config_dword(pdev, PIIX_IOCFG, hpriv->saved_iocfg);

ata_pci_remove_one(pdev);
diff --git a/drivers/ata/pata_rdc.c b/drivers/ata/pata_rdc.c
deleted file mode 100644
index a4a0d88..0000000
--- a/drivers/ata/pata_rdc.c
+++ /dev/null
@@ -1,384 +0,0 @@
-/*
- * pata_rdc - Driver for later RDC PATA controllers
- *
- * This is actually a driver for hardware meeting
- * INCITS 370-2004 (1510D): ATA Host Adapter Standards
- *
- * Based on ata_piix.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2, or (at your option)
- * any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; see the file COPYING. If not, write to
- * the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA.
- */
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/pci.h>
-#include <linux/init.h>
-#include <linux/blkdev.h>
-#include <linux/delay.h>
-#include <linux/device.h>
-#include <linux/gfp.h>
-#include <scsi/scsi_host.h>
-#include <linux/libata.h>
-#include <linux/dmi.h>
-
-#define DRV_NAME "pata_rdc"
-#define DRV_VERSION "0.01"
-
-struct rdc_host_priv {
- u32 saved_iocfg;
-};
-
-/**
- * rdc_pata_cable_detect - Probe host controller cable detect info
- * @ap: Port for which cable detect info is desired
- *
- * Read 80c cable indicator from ATA PCI device's PCI config
- * register. This register is normally set by firmware (BIOS).
- *
- * LOCKING:
- * None (inherited from caller).
- */
-
-static int rdc_pata_cable_detect(struct ata_port *ap)
-{
- struct rdc_host_priv *hpriv = ap->host->private_data;
- u8 mask;
-
- /* check BIOS cable detect results */
- mask = 0x30 << (2 * ap->port_no);
- if ((hpriv->saved_iocfg & mask) == 0)
- return ATA_CBL_PATA40;
- return ATA_CBL_PATA80;
-}
-
-/**
- * rdc_pata_prereset - prereset for PATA host controller
- * @link: Target link
- * @deadline: deadline jiffies for the operation
- *
- * LOCKING:
- * None (inherited from caller).
- */
-static int rdc_pata_prereset(struct ata_link *link, unsigned long deadline)
-{
- struct ata_port *ap = link->ap;
- struct pci_dev *pdev = to_pci_dev(ap->host->dev);
-
- static const struct pci_bits rdc_enable_bits[] = {
- { 0x41U, 1U, 0x80UL, 0x80UL }, /* port 0 */
- { 0x43U, 1U, 0x80UL, 0x80UL }, /* port 1 */
- };
-
- if (!pci_test_config_bits(pdev, &rdc_enable_bits[ap->port_no]))
- return -ENOENT;
- return ata_sff_prereset(link, deadline);
-}
-
-static DEFINE_SPINLOCK(rdc_lock);
-
-static void rdc_set_timings(struct ata_port *ap, struct ata_device *adev,
- u8 pio, bool use_mwdma)
-{
- struct pci_dev *dev = to_pci_dev(ap->host->dev);
- unsigned long flags;
- unsigned int is_slave = (adev->devno != 0);
- unsigned int master_port= ap->port_no ? 0x42 : 0x40;
- unsigned int slave_port = 0x44;
- u16 master_data;
- u8 slave_data;
- u8 udma_enable;
- int control = 0;
-
- static const /* ISP RTC */
- u8 timings[][2] = { { 0, 0 },
- { 0, 0 },
- { 1, 0 },
- { 2, 1 },
- { 2, 3 }, };
-
- if (pio >= 2 || use_mwdma)
- control |= 1; /* TIME1 enable */
- if (ata_pio_need_iordy(adev) || use_mwdma)
- control |= 2; /* IE enable */
- if (adev->class == ATA_DEV_ATA)
- control |= 4; /* PPE enable */
- /* If the drive MWDMA is faster than it can do PIO then
- we must force PIO into PIO0 */
- if (use_mwdma && adev->pio_mode < (XFER_PIO_0 + pio))
- /* Enable DMA timing only */
- control |= 8; /* PIO cycles in PIO0 */
-
- spin_lock_irqsave(&rdc_lock, flags);
-
- /* PIO configuration clears DTE unconditionally. It will be
- * programmed in set_dmamode which is guaranteed to be called
- * after set_piomode if any DMA mode is available.
- */
- pci_read_config_word(dev, master_port, &master_data);
- if (is_slave) {
- /* clear TIME1|IE1|PPE1|DTE1 */
- master_data &= 0xff0f;
- /* Enable SITRE (separate slave timing register) */
- master_data |= 0x4000;
- /* enable PPE1, IE1 and TIME1 as needed */
- master_data |= (control << 4);
- pci_read_config_byte(dev, slave_port, &slave_data);
- slave_data &= (ap->port_no ? 0x0f : 0xf0);
- /* Load the timing nibble for this slave */
- slave_data |= ((timings[pio][0] << 2) | timings[pio][1])
- << (ap->port_no ? 4 : 0);
- } else {
- /* clear ISP|RCT|TIME0|IE0|PPE0|DTE0 */
- master_data &= 0xccf0;
- /* Enable PPE, IE and TIME as appropriate */
- master_data |= control;
- /* load ISP and RCT */
- master_data |=
- (timings[pio][0] << 12) |
- (timings[pio][1] << 8);
- }
- pci_write_config_word(dev, master_port, master_data);
- if (is_slave)
- pci_write_config_byte(dev, slave_port, slave_data);
-
- /* Ensure the UDMA bit is off - it will be turned back on if
- UDMA is selected */
-
- pci_read_config_byte(dev, 0x48, &udma_enable);
- udma_enable &= ~(1 << (2 * ap->port_no + adev->devno));
- pci_write_config_byte(dev, 0x48, udma_enable);
-
- spin_unlock_irqrestore(&rdc_lock, flags);
-}
-
-/**
- * rdc_set_piomode - Initialize host controller PATA PIO timings
- * @ap: Port whose timings we are configuring
- * @adev: Drive in question
- *
- * Set PIO mode for device, in host controller PCI config space.
- *
- * LOCKING:
- * None (inherited from caller).
- */
-
-static void rdc_set_piomode(struct ata_port *ap, struct ata_device *adev)
-{
- rdc_set_timings(ap, adev, adev->pio_mode - XFER_PIO_0, 0);
-}
-
-/**
- * rdc_set_dmamode - Initialize host controller PATA PIO timings
- * @ap: Port whose timings we are configuring
- * @adev: Drive in question
- *
- * Set UDMA mode for device, in host controller PCI config space.
- *
- * LOCKING:
- * None (inherited from caller).
- */
-
-static void rdc_set_dmamode(struct ata_port *ap, struct ata_device *adev)
-{
- struct pci_dev *dev = to_pci_dev(ap->host->dev);
- unsigned long flags;
- u8 speed = adev->dma_mode;
- int devid = adev->devno + 2 * ap->port_no;
- u8 udma_enable = 0;
-
- if (speed >= XFER_UDMA_0) {
- unsigned int udma = speed - XFER_UDMA_0;
- u16 udma_timing;
- u16 ideconf;
- int u_clock, u_speed;
-
- spin_lock_irqsave(&rdc_lock, flags);
-
- pci_read_config_byte(dev, 0x48, &udma_enable);
-
- /*
- * UDMA is handled by a combination of clock switching and
- * selection of dividers
- *
- * Handy rule: Odd modes are UDMATIMx 01, even are 02
- * except UDMA0 which is 00
- */
- u_speed = min(2 - (udma & 1), udma);
- if (udma == 5)
- u_clock = 0x1000; /* 100Mhz */
- else if (udma > 2)
- u_clock = 1; /* 66Mhz */
- else
- u_clock = 0; /* 33Mhz */
-
- udma_enable |= (1 << devid);
-
- /* Load the CT/RP selection */
- pci_read_config_word(dev, 0x4A, &udma_timing);
- udma_timing &= ~(3 << (4 * devid));
- udma_timing |= u_speed << (4 * devid);
- pci_write_config_word(dev, 0x4A, udma_timing);
-
- /* Select a 33/66/100Mhz clock */
- pci_read_config_word(dev, 0x54, &ideconf);
- ideconf &= ~(0x1001 << devid);
- ideconf |= u_clock << devid;
- pci_write_config_word(dev, 0x54, ideconf);
-
- pci_write_config_byte(dev, 0x48, udma_enable);
-
- spin_unlock_irqrestore(&rdc_lock, flags);
- } else {
- /* MWDMA is driven by the PIO timings. */
- unsigned int mwdma = speed - XFER_MW_DMA_0;
- const unsigned int needed_pio[3] = {
- XFER_PIO_0, XFER_PIO_3, XFER_PIO_4
- };
- int pio = needed_pio[mwdma] - XFER_PIO_0;
-
- rdc_set_timings(ap, adev, pio, 1);
- }
-}
-
-static struct ata_port_operations rdc_pata_ops = {
- .inherits = &ata_bmdma32_port_ops,
- .cable_detect = rdc_pata_cable_detect,
- .set_piomode = rdc_set_piomode,
- .set_dmamode = rdc_set_dmamode,
- .prereset = rdc_pata_prereset,
-};
-
-static struct ata_port_info rdc_port_info = {
-
- .flags = ATA_FLAG_SLAVE_POSS,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA12_ONLY,
- .udma_mask = ATA_UDMA5,
- .port_ops = &rdc_pata_ops,
-};
-
-static struct scsi_host_template rdc_sht = {
- ATA_BMDMA_SHT(DRV_NAME),
-};
-
-/**
- * rdc_init_one - Register PIIX ATA PCI device with kernel services
- * @pdev: PCI device to register
- * @ent: Entry in rdc_pci_tbl matching with @pdev
- *
- * Called from kernel PCI layer. We probe for combined mode (sigh),
- * and then hand over control to libata, for it to do the rest.
- *
- * LOCKING:
- * Inherited from PCI layer (may sleep).
- *
- * RETURNS:
- * Zero on success, or -ERRNO value.
- */
-
-static int __devinit rdc_init_one(struct pci_dev *pdev,
- const struct pci_device_id *ent)
-{
- static int printed_version;
- struct device *dev = &pdev->dev;
- struct ata_port_info port_info[2];
- const struct ata_port_info *ppi[] = { &port_info[0], &port_info[1] };
- unsigned long port_flags;
- struct ata_host *host;
- struct rdc_host_priv *hpriv;
- int rc;
-
- if (!printed_version++)
- dev_printk(KERN_DEBUG, &pdev->dev,
- "version " DRV_VERSION "\n");
-
- port_info[0] = rdc_port_info;
- port_info[1] = rdc_port_info;
-
- port_flags = port_info[0].flags;
-
- /* enable device and prepare host */
- rc = pcim_enable_device(pdev);
- if (rc)
- return rc;
-
- hpriv = devm_kzalloc(dev, sizeof(*hpriv), GFP_KERNEL);
- if (!hpriv)
- return -ENOMEM;
-
- /* Save IOCFG, this will be used for cable detection, quirk
- * detection and restoration on detach.
- */
- pci_read_config_dword(pdev, 0x54, &hpriv->saved_iocfg);
-
- rc = ata_pci_bmdma_prepare_host(pdev, ppi, &host);
- if (rc)
- return rc;
- host->private_data = hpriv;
-
- pci_intx(pdev, 1);
-
- host->flags |= ATA_HOST_PARALLEL_SCAN;
-
- pci_set_master(pdev);
- return ata_pci_sff_activate_host(host, ata_bmdma_interrupt, &rdc_sht);
-}
-
-static void rdc_remove_one(struct pci_dev *pdev)
-{
- struct ata_host *host = dev_get_drvdata(&pdev->dev);
- struct rdc_host_priv *hpriv = host->private_data;
-
- pci_write_config_dword(pdev, 0x54, hpriv->saved_iocfg);
-
- ata_pci_remove_one(pdev);
-}
-
-static const struct pci_device_id rdc_pci_tbl[] = {
- { PCI_DEVICE(0x17F3, 0x1011), },
- { PCI_DEVICE(0x17F3, 0x1012), },
- { } /* terminate list */
-};
-
-static struct pci_driver rdc_pci_driver = {
- .name = DRV_NAME,
- .id_table = rdc_pci_tbl,
- .probe = rdc_init_one,
- .remove = rdc_remove_one,
-#ifdef CONFIG_PM
- .suspend = ata_pci_device_suspend,
- .resume = ata_pci_device_resume,
-#endif
-};
-
-
-static int __init rdc_init(void)
-{
- return pci_register_driver(&rdc_pci_driver);
-}
-
-static void __exit rdc_exit(void)
-{
- pci_unregister_driver(&rdc_pci_driver);
-}
-
-module_init(rdc_init);
-module_exit(rdc_exit);
-
-MODULE_AUTHOR("Alan Cox (based on ata_piix)");
-MODULE_DESCRIPTION("SCSI low-level driver for RDC PATA controllers");
-MODULE_LICENSE("GPL");
-MODULE_DEVICE_TABLE(pci, rdc_pci_tbl);
-MODULE_VERSION(DRV_VERSION);
--
1.7.1

Subject: [PATCH 19/20] ata_piix: add Intel old PIIX support

>From dc3181ebc3a8735bcd6ba334f2ad7945654e1189 Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <[email protected]>
Date: Tue, 8 Feb 2011 12:39:28 +0100
Subject: [PATCH 19/20] ata_piix: add Intel old PIIX support

Add Intel old PIIX support to ata_piix and remove no longer
needed pata_oldpiix driver.

Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ata/Kconfig | 1 +
drivers/ata/Makefile | 1 -
drivers/ata/ata_piix.c | 86 +++++++++++++--
drivers/ata/pata_oldpiix.c | 264 --------------------------------------------
4 files changed, 78 insertions(+), 274 deletions(-)
delete mode 100644 drivers/ata/pata_oldpiix.c

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index dc9038d..0e6d193 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -523,6 +523,7 @@ config PATA_NS87415
config PATA_OLDPIIX
tristate "Intel PATA old PIIX support"
depends on PCI
+ select ATA_PIIX
help
This option enables support for early PIIX PATA support.

diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index e627f6d..55272af 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -53,7 +53,6 @@ obj-$(CONFIG_PATA_MPC52xx) += pata_mpc52xx.o
obj-$(CONFIG_PATA_NETCELL) += pata_netcell.o
obj-$(CONFIG_PATA_NINJA32) += pata_ninja32.o
obj-$(CONFIG_PATA_NS87415) += pata_ns87415.o
-obj-$(CONFIG_PATA_OLDPIIX) += pata_oldpiix.o
obj-$(CONFIG_PATA_OPTIDMA) += pata_optidma.o
obj-$(CONFIG_PATA_PDC2027X) += pata_pdc2027x.o
obj-$(CONFIG_PATA_PDC_OLD) += pata_pdc202xx_old.o
diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index 86d12c2..5afa57a 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -52,8 +52,8 @@
* is fixed in Triton II. With the odd mobile exception the chips then
* change little except in gaining more modes until SATA arrives. This
* driver supports only the chips with independant timing (that is those
- * with SITRE and the 0x44 timing register). See pata_oldpiix and pata_mpiix
- * for the early chip drivers.
+ * with SITRE and the 0x44 timing register). See pata_mpiix for the very
+ * early chip support.
*
* Errata of note:
*
@@ -94,6 +94,11 @@
* a similar register layout and the same split clock arrangement. Cable
* detection is different, and it does not have slave channels or all the
* clutter of later ICH/SATA setups.
+ *
+ * Early PIIX differs significantly from the later PIIX as it lacks
+ * SITRE and the slave timing registers. This means that you have to
+ * set timing per channel, or be clever. Libata tells us whenever it
+ * does drive selection and we use this to reload the timings.
*/

#include <linux/kernel.h>
@@ -154,6 +159,7 @@ enum piix_controller_ids {
ich_pata_100_nomwdma1, /* ICH up to UDMA 100 but with no MWDMA1*/
efar_pata,
it8213_pata,
+ oldpiix_pata,
ich5_sata,
ich6_sata,
ich6m_sata,
@@ -186,6 +192,7 @@ static void ich_set_dmamode(struct ata_port *ap, struct ata_device *adev);
static int ich_pata_cable_detect(struct ata_port *ap);
static int efar_pata_cable_detect(struct ata_port *ap);
static int it8213_pata_cable_detect(struct ata_port *ap);
+static unsigned int oldpiix_qc_issue(struct ata_queued_cmd *qc);
static u8 piix_vmw_bmdma_status(struct ata_port *ap);
static int piix_sidpr_scr_read(struct ata_link *link,
unsigned int reg, u32 *val);
@@ -256,6 +263,9 @@ static const struct pci_device_id piix_pci_tbl[] = {
{ 0x17F3, 0x1011, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
{ 0x17F3, 0x1012, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },

+ /* PIIXb (a.k.a. oldpiix) */
+ { 0x8086, 0x1230, PCI_ANY_ID, PCI_ANY_ID, 0, 0, oldpiix_pata },
+
/* SATA ports */

/* 82801EB (ICH5) */
@@ -412,6 +422,15 @@ static struct ata_port_operations it8213_pata_ops = {
.cable_detect = it8213_pata_cable_detect,
};

+static struct ata_port_operations oldpiix_pata_ops = {
+ .inherits = &ata_bmdma_port_ops,
+ .qc_issue = oldpiix_qc_issue,
+ .cable_detect = ata_cable_40wire,
+ .set_piomode = piix_set_piomode,
+ .set_dmamode = piix_set_dmamode,
+ .prereset = piix_pata_prereset,
+};
+
static const struct piix_map_db ich5_map_db = {
.mask = 0x7,
.port_enable = 0x3,
@@ -658,6 +677,14 @@ static struct ata_port_info piix_port_info[] = {
.udma_mask = ATA_UDMA6,
.port_ops = &it8213_pata_ops,
},
+
+ [oldpiix_pata] =
+ {
+ .flags = PIIX_PATA_FLAGS,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA12_ONLY,
+ .port_ops = &oldpiix_pata_ops,
+ },
};

static struct pci_bits piix_enable_bits[] = {
@@ -673,6 +700,7 @@ MODULE_VERSION(DRV_VERSION);
MODULE_ALIAS("pata_efar");
MODULE_ALIAS("pata_it8213");
MODULE_ALIAS("pata_rdc");
+MODULE_ALIAS("pata_oldpiix");

struct ich_laptop {
u16 device;
@@ -803,6 +831,8 @@ static void piix_set_timings(struct ata_port *ap, struct ata_device *adev,
struct pci_dev *dev = to_pci_dev(ap->host->dev);
unsigned long flags;
unsigned int is_slave = (adev->devno != 0);
+ unsigned int has_sitre = (dev->vendor != 0x8086 ||
+ dev->device != 0x1230);
unsigned int master_port= ap->port_no ? 0x42 : 0x40;
unsigned int slave_port = 0x44;
u16 master_data;
@@ -848,7 +878,7 @@ static void piix_set_timings(struct ata_port *ap, struct ata_device *adev,
* after set_piomode if any DMA mode is available.
*/
pci_read_config_word(dev, master_port, &master_data);
- if (is_slave) {
+ if (is_slave && has_sitre) {
/* clear TIME1|IE1|PPE1|DTE1 */
master_data &= 0xff0f;
/* enable PPE1, IE1 and TIME1 as needed */
@@ -859,10 +889,20 @@ static void piix_set_timings(struct ata_port *ap, struct ata_device *adev,
slave_data |= ((timings[pio][0] << 2) | timings[pio][1])
<< (ap->port_no ? 4 : 0);
} else {
- /* clear ISP|RCT|TIME0|IE0|PPE0|DTE0 */
- master_data &= 0xccf0;
- /* Enable PPE, IE and TIME as appropriate */
- master_data |= control;
+ if (has_sitre) {
+ /* clear ISP|RCT|TIME0|IE0|PPE0|DTE0 */
+ master_data &= 0xccf0;
+ /* Enable PPE, IE and TIME as appropriate */
+ master_data |= control;
+ } else {
+ if (!is_slave) {
+ master_data &= 0xCCE0;
+ master_data |= control;
+ } else {
+ master_data &= 0xCC0E;
+ master_data |= (control << 4);
+ }
+ }
/* load ISP and RCT */
master_data |=
(timings[pio][0] << 12) |
@@ -870,9 +910,10 @@ static void piix_set_timings(struct ata_port *ap, struct ata_device *adev,
}

/* Enable SITRE (separate slave timing register) */
- master_data |= 0x4000;
+ if (has_sitre)
+ master_data |= 0x4000;
pci_write_config_word(dev, master_port, master_data);
- if (is_slave)
+ if (is_slave && has_sitre)
pci_write_config_byte(dev, slave_port, slave_data);

/* Ensure the UDMA bit is off - it will be turned back on if
@@ -885,6 +926,9 @@ static void piix_set_timings(struct ata_port *ap, struct ata_device *adev,
}

spin_unlock_irqrestore(&piix_lock, flags);
+
+ /* Track which port is configured */
+ ap->private_data = adev;
}

/**
@@ -1019,6 +1063,30 @@ static void ich_set_dmamode(struct ata_port *ap, struct ata_device *adev)
do_pata_set_dmamode(ap, adev, 1);
}

+/**
+ * oldpiix_qc_issue - command issue
+ * @qc: command pending
+ *
+ * Called when the libata layer is about to issue a command. We wrap
+ * this interface so that we can load the correct ATA timings if
+ * necessary. Our logic also clears TIME0/TIME1 for the other device so
+ * that, even if we get this wrong, cycles to the other device will
+ * be made PIO0.
+ */
+
+static unsigned int oldpiix_qc_issue(struct ata_queued_cmd *qc)
+{
+ struct ata_port *ap = qc->ap;
+ struct ata_device *adev = qc->dev;
+
+ if (adev != ap->private_data) {
+ piix_set_piomode(ap, adev);
+ if (ata_dma_enabled(adev))
+ piix_set_dmamode(ap, adev);
+ }
+ return ata_bmdma_qc_issue(qc);
+}
+
/*
* Serial ATA Index/Data Pair Superset Registers access
*
diff --git a/drivers/ata/pata_oldpiix.c b/drivers/ata/pata_oldpiix.c
deleted file mode 100644
index 819192d..0000000
--- a/drivers/ata/pata_oldpiix.c
+++ /dev/null
@@ -1,264 +0,0 @@
-/*
- * pata_oldpiix.c - older Intel PATA controllers
- *
- * (C) 2005 Red Hat
- * (C) 2010 Bartlomiej Zolnierkiewicz
- *
- * Some parts based on ata_piix.c by Jeff Garzik and others.
- *
- * Early PIIX differs significantly from the later PIIX as it lacks
- * SITRE and the slave timing registers. This means that you have to
- * set timing per channel, or be clever. Libata tells us whenever it
- * does drive selection and we use this to reload the timings.
- *
- * Because of these behaviour differences PIIX gets its own driver module.
- */
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/pci.h>
-#include <linux/init.h>
-#include <linux/blkdev.h>
-#include <linux/delay.h>
-#include <linux/device.h>
-#include <scsi/scsi_host.h>
-#include <linux/libata.h>
-#include <linux/ata.h>
-
-#define DRV_NAME "pata_oldpiix"
-#define DRV_VERSION "0.5.5"
-
-/**
- * oldpiix_pre_reset - probe begin
- * @link: ATA link
- * @deadline: deadline jiffies for the operation
- *
- * Set up cable type and use generic probe init
- */
-
-static int oldpiix_pre_reset(struct ata_link *link, unsigned long deadline)
-{
- struct ata_port *ap = link->ap;
- struct pci_dev *pdev = to_pci_dev(ap->host->dev);
- static const struct pci_bits oldpiix_enable_bits[] = {
- { 0x41U, 1U, 0x80UL, 0x80UL }, /* port 0 */
- { 0x43U, 1U, 0x80UL, 0x80UL }, /* port 1 */
- };
-
- if (!pci_test_config_bits(pdev, &oldpiix_enable_bits[ap->port_no]))
- return -ENOENT;
-
- return ata_sff_prereset(link, deadline);
-}
-
-static DEFINE_SPINLOCK(piix_lock);
-
-static void oldpiix_set_timings(struct ata_port *ap, struct ata_device *adev,
- u8 pio, bool use_mwdma)
-{
- struct pci_dev *dev = to_pci_dev(ap->host->dev);
- unsigned long flags;
- unsigned int idetm_port= ap->port_no ? 0x42 : 0x40;
- u16 idetm_data;
- int control = 0;
-
- /*
- * See Intel Document 298600-004 for the timing programing rules
- * for PIIX/ICH. Note that the early PIIX does not have the slave
- * timing port at 0x44.
- */
-
- static const /* ISP RTC */
- u8 timings[][2] = { { 0, 0 },
- { 0, 0 },
- { 1, 0 },
- { 2, 1 },
- { 2, 3 }, };
-
- if (pio > 1 || use_mwdma)
- control |= 1; /* TIME */
- if (ata_pio_need_iordy(adev) || use_mwdma)
- control |= 2; /* IE */
- /* Intel specifies that the prefetch/posting is for disk only */
- if (adev->class == ATA_DEV_ATA)
- control |= 4; /* PPE */
- /* If the drive MWDMA is faster than it can do PIO then
- we must force PIO into PIO0 */
- if (use_mwdma && adev->pio_mode < (XFER_PIO_0 + pio))
- /* Enable DMA timing only */
- control |= 8; /* PIO cycles in PIO0 */
-
- spin_lock_irqsave(&piix_lock, flags);
-
- pci_read_config_word(dev, idetm_port, &idetm_data);
-
- /*
- * Set PPE, IE and TIME as appropriate.
- * Clear the other drive's timing bits.
- */
- if (adev->devno == 0) {
- idetm_data &= 0xCCE0;
- idetm_data |= control;
- } else {
- idetm_data &= 0xCC0E;
- idetm_data |= (control << 4);
- }
- idetm_data |= (timings[pio][0] << 12) |
- (timings[pio][1] << 8);
- pci_write_config_word(dev, idetm_port, idetm_data);
-
- spin_unlock_irqrestore(&piix_lock, flags);
-
- /* Track which port is configured */
- ap->private_data = adev;
-}
-
-/**
- * oldpiix_set_piomode - Initialize host controller PATA PIO timings
- * @ap: Port whose timings we are configuring
- * @adev: Device whose timings we are configuring
- *
- * Set PIO mode for device, in host controller PCI config space.
- *
- * LOCKING:
- * None (inherited from caller).
- */
-
-static void oldpiix_set_piomode(struct ata_port *ap, struct ata_device *adev)
-{
- oldpiix_set_timings(ap, adev, adev->pio_mode - XFER_PIO_0, 0);
-}
-
-/**
- * oldpiix_set_dmamode - Initialize host controller PATA DMA timings
- * @ap: Port whose timings we are configuring
- * @adev: Device to program
- *
- * Set MWDMA mode for device, in host controller PCI config space.
- *
- * LOCKING:
- * None (inherited from caller).
- */
-
-static void oldpiix_set_dmamode (struct ata_port *ap, struct ata_device *adev)
-{
- /* MWDMA is driven by the PIO timings. */
-
- unsigned int mwdma = adev->dma_mode - XFER_MW_DMA_0;
- const unsigned int needed_pio[3] = {
- XFER_PIO_0, XFER_PIO_3, XFER_PIO_4
- };
- int pio = needed_pio[mwdma] - XFER_PIO_0;
-
- oldpiix_set_timings(ap, adev, pio, 1);
-}
-
-/**
- * oldpiix_qc_issue - command issue
- * @qc: command pending
- *
- * Called when the libata layer is about to issue a command. We wrap
- * this interface so that we can load the correct ATA timings if
- * necessary. Our logic also clears TIME0/TIME1 for the other device so
- * that, even if we get this wrong, cycles to the other device will
- * be made PIO0.
- */
-
-static unsigned int oldpiix_qc_issue(struct ata_queued_cmd *qc)
-{
- struct ata_port *ap = qc->ap;
- struct ata_device *adev = qc->dev;
-
- if (adev != ap->private_data) {
- oldpiix_set_piomode(ap, adev);
- if (ata_dma_enabled(adev))
- oldpiix_set_dmamode(ap, adev);
- }
- return ata_bmdma_qc_issue(qc);
-}
-
-
-static struct scsi_host_template oldpiix_sht = {
- ATA_BMDMA_SHT(DRV_NAME),
-};
-
-static struct ata_port_operations oldpiix_pata_ops = {
- .inherits = &ata_bmdma_port_ops,
- .qc_issue = oldpiix_qc_issue,
- .cable_detect = ata_cable_40wire,
- .set_piomode = oldpiix_set_piomode,
- .set_dmamode = oldpiix_set_dmamode,
- .prereset = oldpiix_pre_reset,
-};
-
-
-/**
- * oldpiix_init_one - Register PIIX ATA PCI device with kernel services
- * @pdev: PCI device to register
- * @ent: Entry in oldpiix_pci_tbl matching with @pdev
- *
- * Called from kernel PCI layer. We probe for combined mode (sigh),
- * and then hand over control to libata, for it to do the rest.
- *
- * LOCKING:
- * Inherited from PCI layer (may sleep).
- *
- * RETURNS:
- * Zero on success, or -ERRNO value.
- */
-
-static int oldpiix_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
-{
- static int printed_version;
- static const struct ata_port_info info = {
- .flags = ATA_FLAG_SLAVE_POSS,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA12_ONLY,
- .port_ops = &oldpiix_pata_ops,
- };
- const struct ata_port_info *ppi[] = { &info, NULL };
-
- if (!printed_version++)
- dev_printk(KERN_DEBUG, &pdev->dev,
- "version " DRV_VERSION "\n");
-
- return ata_pci_bmdma_init_one(pdev, ppi, &oldpiix_sht, NULL,
- ATA_HOST_PARALLEL_SCAN);
-}
-
-static const struct pci_device_id oldpiix_pci_tbl[] = {
- { PCI_VDEVICE(INTEL, 0x1230), },
-
- { } /* terminate list */
-};
-
-static struct pci_driver oldpiix_pci_driver = {
- .name = DRV_NAME,
- .id_table = oldpiix_pci_tbl,
- .probe = oldpiix_init_one,
- .remove = ata_pci_remove_one,
-#ifdef CONFIG_PM
- .suspend = ata_pci_device_suspend,
- .resume = ata_pci_device_resume,
-#endif
-};
-
-static int __init oldpiix_init(void)
-{
- return pci_register_driver(&oldpiix_pci_driver);
-}
-
-static void __exit oldpiix_exit(void)
-{
- pci_unregister_driver(&oldpiix_pci_driver);
-}
-
-module_init(oldpiix_init);
-module_exit(oldpiix_exit);
-
-MODULE_AUTHOR("Alan Cox");
-MODULE_DESCRIPTION("SCSI low-level driver for early PIIX series controllers");
-MODULE_LICENSE("GPL");
-MODULE_DEVICE_TABLE(pci, oldpiix_pci_tbl);
-MODULE_VERSION(DRV_VERSION);
-
--
1.7.1

Subject: [PATCH 20/20] ata_piix: add Radisys R82600 support

>From 2172ad66e1ad14599fe08d4fe898d392f10962d3 Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <[email protected]>
Date: Tue, 8 Feb 2011 12:39:28 +0100
Subject: [PATCH 20/20] ata_piix: add Radisys R82600 support

Add Radisys R82600 support to ata_piix and remove no longer
needed pata_radisys driver.

Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ata/Kconfig | 1 +
drivers/ata/Makefile | 1 -
drivers/ata/ata_piix.c | 84 ++++++++++++---
drivers/ata/pata_radisys.c | 252 --------------------------------------------
4 files changed, 72 insertions(+), 266 deletions(-)
delete mode 100644 drivers/ata/pata_radisys.c

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 0e6d193..43bbf74 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -559,6 +559,7 @@ config PATA_PDC_OLD
config PATA_RADISYS
tristate "RADISYS 82600 PATA support (Experimental)"
depends on PCI && EXPERIMENTAL
+ select ATA_PIIX
help
This option enables support for the RADISYS 82600
PATA controllers via the new ATA layer
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 55272af..cddef6e 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -56,7 +56,6 @@ obj-$(CONFIG_PATA_NS87415) += pata_ns87415.o
obj-$(CONFIG_PATA_OPTIDMA) += pata_optidma.o
obj-$(CONFIG_PATA_PDC2027X) += pata_pdc2027x.o
obj-$(CONFIG_PATA_PDC_OLD) += pata_pdc202xx_old.o
-obj-$(CONFIG_PATA_RADISYS) += pata_radisys.o
obj-$(CONFIG_PATA_SC1200) += pata_sc1200.o
obj-$(CONFIG_PATA_SCC) += pata_scc.o
obj-$(CONFIG_PATA_SCH) += pata_sch.o
diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index 5afa57a..4b00e26 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -99,6 +99,12 @@
* SITRE and the slave timing registers. This means that you have to
* set timing per channel, or be clever. Libata tells us whenever it
* does drive selection and we use this to reload the timings.
+ *
+ * Radisys R82600 - a PIIX relative, this device has a single ATA channel
+ * and no slave timings, SITRE or PPE. In that sense it is a close relative
+ * of the original PIIX. It does however support UDMA 33/66 per channel
+ * although no other modes/timings. Also lacking is 32bit I/O on the ATA
+ * port.
*/

#include <linux/kernel.h>
@@ -160,6 +166,7 @@ enum piix_controller_ids {
efar_pata,
it8213_pata,
oldpiix_pata,
+ radisys_pata,
ich5_sata,
ich6_sata,
ich6m_sata,
@@ -266,6 +273,9 @@ static const struct pci_device_id piix_pci_tbl[] = {
/* PIIXb (a.k.a. oldpiix) */
{ 0x8086, 0x1230, PCI_ANY_ID, PCI_ANY_ID, 0, 0, oldpiix_pata },

+ /* Radisys R82600 */
+ { 0x1331, 0x8201, PCI_ANY_ID, PCI_ANY_ID, 0, 0, radisys_pata },
+
/* SATA ports */

/* 82801EB (ICH5) */
@@ -431,6 +441,14 @@ static struct ata_port_operations oldpiix_pata_ops = {
.prereset = piix_pata_prereset,
};

+static struct ata_port_operations radisys_pata_ops = {
+ .inherits = &ata_bmdma_port_ops,
+ .qc_issue = oldpiix_qc_issue,
+ .cable_detect = ata_cable_unknown,
+ .set_piomode = piix_set_piomode,
+ .set_dmamode = piix_set_dmamode,
+};
+
static const struct piix_map_db ich5_map_db = {
.mask = 0x7,
.port_enable = 0x3,
@@ -685,6 +703,15 @@ static struct ata_port_info piix_port_info[] = {
.mwdma_mask = ATA_MWDMA12_ONLY,
.port_ops = &oldpiix_pata_ops,
},
+
+ [radisys_pata] =
+ {
+ .flags = PIIX_PATA_FLAGS,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA12_ONLY,
+ .udma_mask = ATA_UDMA24_ONLY,
+ .port_ops = &radisys_pata_ops,
+ },
};

static struct pci_bits piix_enable_bits[] = {
@@ -701,6 +728,7 @@ MODULE_ALIAS("pata_efar");
MODULE_ALIAS("pata_it8213");
MODULE_ALIAS("pata_rdc");
MODULE_ALIAS("pata_oldpiix");
+MODULE_ALIAS("pata_radisys");

struct ich_laptop {
u16 device;
@@ -831,8 +859,10 @@ static void piix_set_timings(struct ata_port *ap, struct ata_device *adev,
struct pci_dev *dev = to_pci_dev(ap->host->dev);
unsigned long flags;
unsigned int is_slave = (adev->devno != 0);
- unsigned int has_sitre = (dev->vendor != 0x8086 ||
- dev->device != 0x1230);
+ unsigned int is_radisys = (dev->vendor == PCI_VENDOR_ID_RADISYS &&
+ dev->device == 0x8201);
+ unsigned int has_sitre = (dev->vendor != PCI_VENDOR_ID_INTEL ||
+ dev->device != 0x1230) && !is_radisys;
unsigned int master_port= ap->port_no ? 0x42 : 0x40;
unsigned int slave_port = 0x44;
u16 master_data;
@@ -851,14 +881,21 @@ static void piix_set_timings(struct ata_port *ap, struct ata_device *adev,
{ 1, 0 },
{ 2, 1 },
{ 2, 3 }, };
+ static const u8 timings_radisys[][2] = {
+ { 0, 0 },
+ { 0, 0 },
+ { 1, 1 },
+ { 2, 2 },
+ { 3, 3 }, };

- if (pio >= 2 || use_mwdma)
+ if (pio >= 2 || (pio == 1 && is_radisys) ||
+ (use_mwdma && !is_radisys))
control |= 1; /* TIME1 enable */
- if (ata_pio_need_iordy(adev) || use_mwdma)
+ if (ata_pio_need_iordy(adev) || (use_mwdma && !is_radisys))
control |= 2; /* IE enable */
if (dev->vendor != PCI_VENDOR_ID_ITE) {
/* PPE functionality is for disk only */
- if (adev->class == ATA_DEV_ATA)
+ if (adev->class == ATA_DEV_ATA && !is_radisys)
control |= 4; /* PPE enable */
} else {
/* Bit 2 is set for ATAPI on the IT8213 - reverse of ICH/PIIX */
@@ -867,9 +904,13 @@ static void piix_set_timings(struct ata_port *ap, struct ata_device *adev,
}
/* If the drive MWDMA is faster than it can do PIO then
we must force PIO into PIO0 */
- if (use_mwdma && adev->pio_mode < (XFER_PIO_0 + pio))
- /* Enable DMA timing only */
- control |= 8; /* PIO cycles in PIO0 */
+ if (use_mwdma && adev->pio_mode < (XFER_PIO_0 + pio)) {
+ if (!is_radisys) {
+ /* Enable DMA timing only */
+ control |= 8; /* PIO cycles in PIO0 */
+ } else
+ control = 1;
+ }

spin_lock_irqsave(&piix_lock, flags);

@@ -888,7 +929,7 @@ static void piix_set_timings(struct ata_port *ap, struct ata_device *adev,
/* Load the timing nibble for this slave */
slave_data |= ((timings[pio][0] << 2) | timings[pio][1])
<< (ap->port_no ? 4 : 0);
- } else {
+ } else if (!is_radisys) {
if (has_sitre) {
/* clear ISP|RCT|TIME0|IE0|PPE0|DTE0 */
master_data &= 0xccf0;
@@ -907,6 +948,14 @@ static void piix_set_timings(struct ata_port *ap, struct ata_device *adev,
master_data |=
(timings[pio][0] << 12) |
(timings[pio][1] << 8);
+ } else { /* radisys */
+ /* Enable IE and TIME as appropriate. Clear the other
+ drive timing bits */
+ master_data &= 0xCCCC;
+ master_data |= (control << (4 * adev->devno));
+ master_data |=
+ (timings_radisys[pio][0] << 12) |
+ (timings_radisys[pio][1] << 8);
}

/* Enable SITRE (separate slave timing register) */
@@ -995,7 +1044,12 @@ static void do_pata_set_dmamode(struct ata_port *ap, struct ata_device *adev, in
udma_enable |= (1 << devid);

pci_read_config_word(dev, 0x4A, &udma_timing);
- if (dev->vendor == PCI_VENDOR_ID_EFAR) {
+ if (dev->vendor == PCI_VENDOR_ID_RADISYS) {
+ if (udma == 2)
+ udma_timing &= ~(2 << (adev->devno * 4));
+ else /* UDMA 4 */
+ udma_timing |= (2 << (adev->devno * 4));
+ } else if (dev->vendor == PCI_VENDOR_ID_EFAR) {
/* Load the UDMA mode number */
udma_timing &= ~(7 << (4 * devid));
udma_timing |= udma << (4 * devid);
@@ -1080,9 +1134,13 @@ static unsigned int oldpiix_qc_issue(struct ata_queued_cmd *qc)
struct ata_device *adev = qc->dev;

if (adev != ap->private_data) {
- piix_set_piomode(ap, adev);
- if (ata_dma_enabled(adev))
- piix_set_dmamode(ap, adev);
+ /* UDMA timing is not shared */
+ if (adev->dma_mode < XFER_UDMA_0) {
+ if (adev->dma_mode)
+ piix_set_dmamode(ap, adev);
+ else if (adev->pio_mode)
+ piix_set_piomode(ap, adev);
+ }
}
return ata_bmdma_qc_issue(qc);
}
diff --git a/drivers/ata/pata_radisys.c b/drivers/ata/pata_radisys.c
deleted file mode 100644
index e85bb38..0000000
--- a/drivers/ata/pata_radisys.c
+++ /dev/null
@@ -1,252 +0,0 @@
-/*
- * pata_radisys.c - Intel PATA/SATA controllers
- *
- * (C) 2006 Red Hat <[email protected]>
- *
- * Some parts based on ata_piix.c by Jeff Garzik and others.
- *
- * A PIIX relative, this device has a single ATA channel and no
- * slave timings, SITRE or PPE. In that sense it is a close relative
- * of the original PIIX. It does however support UDMA 33/66 per channel
- * although no other modes/timings. Also lacking is 32bit I/O on the ATA
- * port.
- */
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/pci.h>
-#include <linux/init.h>
-#include <linux/blkdev.h>
-#include <linux/delay.h>
-#include <linux/device.h>
-#include <scsi/scsi_host.h>
-#include <linux/libata.h>
-#include <linux/ata.h>
-
-#define DRV_NAME "pata_radisys"
-#define DRV_VERSION "0.4.4"
-
-static void radisys_set_timings(struct ata_port *ap, struct ata_device *adev,
- u8 pio, bool use_mwdma)
-{
- struct pci_dev *dev = to_pci_dev(ap->host->dev);
- u16 idetm_data;
- int control = 0;
-
- /*
- * See Intel Document 298600-004 for the timing programing rules
- * for PIIX/ICH. Note that the early PIIX does not have the slave
- * timing port at 0x44. The Radisys is a relative of the PIIX
- * but not the same so be careful.
- */
-
- static const /* ISP RTC */
- u8 timings[][2] = { { 0, 0 },
- { 0, 0 },
- { 1, 1 },
- { 2, 2 },
- { 3, 3 }, };
-
- if (pio > 0)
- control |= 1; /* TIME1 enable */
- if (ata_pio_need_iordy(adev))
- control |= 2; /* IE IORDY */
- /* If the drive MWDMA is faster than it can do PIO then
- we must force PIO0 for PIO cycles. */
- if (use_mwdma && adev->pio_mode < (XFER_PIO_0 + pio))
- control = 1;
-
- pci_read_config_word(dev, 0x40, &idetm_data);
-
- /* Enable IE and TIME as appropriate. Clear the other
- drive timing bits */
- idetm_data &= 0xCCCC;
- idetm_data |= (control << (4 * adev->devno));
- idetm_data |= (timings[pio][0] << 12) |
- (timings[pio][1] << 8);
- pci_write_config_word(dev, 0x40, idetm_data);
-
- /* Track which port is configured */
- ap->private_data = adev;
-}
-
-/**
- * radisys_set_piomode - Initialize host controller PATA PIO timings
- * @ap: ATA port
- * @adev: Device whose timings we are configuring
- *
- * Set PIO mode for device, in host controller PCI config space.
- *
- * LOCKING:
- * None (inherited from caller).
- */
-
-static void radisys_set_piomode(struct ata_port *ap, struct ata_device *adev)
-{
- radisys_set_timings(ap, adev, adev->pio_mode - XFER_PIO_0, 0);
-}
-
-/**
- * radisys_set_dmamode - Initialize host controller PATA DMA timings
- * @ap: Port whose timings we are configuring
- * @adev: Device to program
- *
- * Set MWDMA mode for device, in host controller PCI config space.
- *
- * LOCKING:
- * None (inherited from caller).
- */
-
-static void radisys_set_dmamode (struct ata_port *ap, struct ata_device *adev)
-{
- struct pci_dev *dev = to_pci_dev(ap->host->dev);
- u8 udma_enable;
-
- /* MWDMA is driven by the PIO timings. */
-
- pci_read_config_byte(dev, 0x48, &udma_enable);
-
- if (adev->dma_mode < XFER_UDMA_0) {
- unsigned int mwdma = adev->dma_mode - XFER_MW_DMA_0;
- const unsigned int needed_pio[3] = {
- XFER_PIO_0, XFER_PIO_3, XFER_PIO_4
- };
- int pio = needed_pio[mwdma] - XFER_PIO_0;
-
- radisys_set_timings(ap, adev, pio, 1);
-
- udma_enable &= ~(1 << adev->devno);
- } else {
- u8 udma_mode;
-
- /* UDMA66 on: UDMA 33 and 66 are switchable via register 0x4A */
-
- pci_read_config_byte(dev, 0x4A, &udma_mode);
-
- if (adev->xfer_mode == XFER_UDMA_2)
- udma_mode &= ~(2 << (adev->devno * 4));
- else /* UDMA 4 */
- udma_mode |= (2 << (adev->devno * 4));
-
- pci_write_config_byte(dev, 0x4A, udma_mode);
-
- udma_enable |= (1 << adev->devno);
- }
- pci_write_config_byte(dev, 0x48, udma_enable);
-
- /* Track which port is configured */
- ap->private_data = adev;
-}
-
-/**
- * radisys_qc_issue - command issue
- * @qc: command pending
- *
- * Called when the libata layer is about to issue a command. We wrap
- * this interface so that we can load the correct ATA timings if
- * necessary. Our logic also clears TIME0/TIME1 for the other device so
- * that, even if we get this wrong, cycles to the other device will
- * be made PIO0.
- */
-
-static unsigned int radisys_qc_issue(struct ata_queued_cmd *qc)
-{
- struct ata_port *ap = qc->ap;
- struct ata_device *adev = qc->dev;
-
- if (adev != ap->private_data) {
- /* UDMA timing is not shared */
- if (adev->dma_mode < XFER_UDMA_0) {
- if (adev->dma_mode)
- radisys_set_dmamode(ap, adev);
- else if (adev->pio_mode)
- radisys_set_piomode(ap, adev);
- }
- }
- return ata_bmdma_qc_issue(qc);
-}
-
-
-static struct scsi_host_template radisys_sht = {
- ATA_BMDMA_SHT(DRV_NAME),
-};
-
-static struct ata_port_operations radisys_pata_ops = {
- .inherits = &ata_bmdma_port_ops,
- .qc_issue = radisys_qc_issue,
- .cable_detect = ata_cable_unknown,
- .set_piomode = radisys_set_piomode,
- .set_dmamode = radisys_set_dmamode,
-};
-
-
-/**
- * radisys_init_one - Register PIIX ATA PCI device with kernel services
- * @pdev: PCI device to register
- * @ent: Entry in radisys_pci_tbl matching with @pdev
- *
- * Called from kernel PCI layer. We probe for combined mode (sigh),
- * and then hand over control to libata, for it to do the rest.
- *
- * LOCKING:
- * Inherited from PCI layer (may sleep).
- *
- * RETURNS:
- * Zero on success, or -ERRNO value.
- */
-
-static int radisys_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
-{
- static int printed_version;
- static const struct ata_port_info info = {
- .flags = ATA_FLAG_SLAVE_POSS,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA12_ONLY,
- .udma_mask = ATA_UDMA24_ONLY,
- .port_ops = &radisys_pata_ops,
- };
- const struct ata_port_info *ppi[] = { &info, NULL };
-
- if (!printed_version++)
- dev_printk(KERN_DEBUG, &pdev->dev,
- "version " DRV_VERSION "\n");
-
- return ata_pci_bmdma_init_one(pdev, ppi, &radisys_sht, NULL, 0);
-}
-
-static const struct pci_device_id radisys_pci_tbl[] = {
- { PCI_VDEVICE(RADISYS, 0x8201), },
-
- { } /* terminate list */
-};
-
-static struct pci_driver radisys_pci_driver = {
- .name = DRV_NAME,
- .id_table = radisys_pci_tbl,
- .probe = radisys_init_one,
- .remove = ata_pci_remove_one,
-#ifdef CONFIG_PM
- .suspend = ata_pci_device_suspend,
- .resume = ata_pci_device_resume,
-#endif
-};
-
-static int __init radisys_init(void)
-{
- return pci_register_driver(&radisys_pci_driver);
-}
-
-static void __exit radisys_exit(void)
-{
- pci_unregister_driver(&radisys_pci_driver);
-}
-
-module_init(radisys_init);
-module_exit(radisys_exit);
-
-MODULE_AUTHOR("Alan Cox");
-MODULE_DESCRIPTION("SCSI low-level driver for Radisys R82600 controllers");
-MODULE_LICENSE("GPL");
-MODULE_DEVICE_TABLE(pci, radisys_pci_tbl);
-MODULE_VERSION(DRV_VERSION);
-
--
1.7.1

Subject: [PATCH 10/20] pata_radisys: unify code for programming PIO and MWDMA timings

>From 656a1d1e17b035c7de7e7ea602f03724fdcbb336 Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <[email protected]>
Date: Tue, 8 Feb 2011 12:39:26 +0100
Subject: [PATCH 10/20] pata_radisys: unify code for programming PIO and MWDMA timings

Besides making things noticably simpler it results in ~6% decrease in
the driver LOC count and also ~1% decrease in the driver binary size
(as measured on x86-32).

Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ata/pata_radisys.c | 66 ++++++++++++++++---------------------------
1 files changed, 25 insertions(+), 41 deletions(-)

diff --git a/drivers/ata/pata_radisys.c b/drivers/ata/pata_radisys.c
index 8574b31..e85bb38 100644
--- a/drivers/ata/pata_radisys.c
+++ b/drivers/ata/pata_radisys.c
@@ -26,20 +26,9 @@
#define DRV_NAME "pata_radisys"
#define DRV_VERSION "0.4.4"

-/**
- * radisys_set_piomode - Initialize host controller PATA PIO timings
- * @ap: ATA port
- * @adev: Device whose timings we are configuring
- *
- * Set PIO mode for device, in host controller PCI config space.
- *
- * LOCKING:
- * None (inherited from caller).
- */
-
-static void radisys_set_piomode (struct ata_port *ap, struct ata_device *adev)
+static void radisys_set_timings(struct ata_port *ap, struct ata_device *adev,
+ u8 pio, bool use_mwdma)
{
- unsigned int pio = adev->pio_mode - XFER_PIO_0;
struct pci_dev *dev = to_pci_dev(ap->host->dev);
u16 idetm_data;
int control = 0;
@@ -52,7 +41,7 @@ static void radisys_set_piomode (struct ata_port *ap, struct ata_device *adev)
*/

static const /* ISP RTC */
- u8 timings[][2] = { { 0, 0 }, /* Check me */
+ u8 timings[][2] = { { 0, 0 },
{ 0, 0 },
{ 1, 1 },
{ 2, 2 },
@@ -62,6 +51,10 @@ static void radisys_set_piomode (struct ata_port *ap, struct ata_device *adev)
control |= 1; /* TIME1 enable */
if (ata_pio_need_iordy(adev))
control |= 2; /* IE IORDY */
+ /* If the drive MWDMA is faster than it can do PIO then
+ we must force PIO0 for PIO cycles. */
+ if (use_mwdma && adev->pio_mode < (XFER_PIO_0 + pio))
+ control = 1;

pci_read_config_word(dev, 0x40, &idetm_data);

@@ -78,6 +71,22 @@ static void radisys_set_piomode (struct ata_port *ap, struct ata_device *adev)
}

/**
+ * radisys_set_piomode - Initialize host controller PATA PIO timings
+ * @ap: ATA port
+ * @adev: Device whose timings we are configuring
+ *
+ * Set PIO mode for device, in host controller PCI config space.
+ *
+ * LOCKING:
+ * None (inherited from caller).
+ */
+
+static void radisys_set_piomode(struct ata_port *ap, struct ata_device *adev)
+{
+ radisys_set_timings(ap, adev, adev->pio_mode - XFER_PIO_0, 0);
+}
+
+/**
* radisys_set_dmamode - Initialize host controller PATA DMA timings
* @ap: Port whose timings we are configuring
* @adev: Device to program
@@ -91,22 +100,10 @@ static void radisys_set_piomode (struct ata_port *ap, struct ata_device *adev)
static void radisys_set_dmamode (struct ata_port *ap, struct ata_device *adev)
{
struct pci_dev *dev = to_pci_dev(ap->host->dev);
- u16 idetm_data;
u8 udma_enable;

- static const /* ISP RTC */
- u8 timings[][2] = { { 0, 0 },
- { 0, 0 },
- { 1, 1 },
- { 2, 2 },
- { 3, 3 }, };
-
- /*
- * MWDMA is driven by the PIO timings. We must also enable
- * IORDY unconditionally.
- */
+ /* MWDMA is driven by the PIO timings. */

- pci_read_config_word(dev, 0x40, &idetm_data);
pci_read_config_byte(dev, 0x48, &udma_enable);

if (adev->dma_mode < XFER_UDMA_0) {
@@ -115,20 +112,8 @@ static void radisys_set_dmamode (struct ata_port *ap, struct ata_device *adev)
XFER_PIO_0, XFER_PIO_3, XFER_PIO_4
};
int pio = needed_pio[mwdma] - XFER_PIO_0;
- int control = 3; /* IORDY|TIME0 */
-
- /* If the drive MWDMA is faster than it can do PIO then
- we must force PIO0 for PIO cycles. */
-
- if (adev->pio_mode < needed_pio[mwdma])
- control = 1;

- /* Mask out the relevant control and timing bits we will load. Also
- clear the other drive TIME register as a precaution */
-
- idetm_data &= 0xCCCC;
- idetm_data |= control << (4 * adev->devno);
- idetm_data |= (timings[pio][0] << 12) | (timings[pio][1] << 8);
+ radisys_set_timings(ap, adev, pio, 1);

udma_enable &= ~(1 << adev->devno);
} else {
@@ -147,7 +132,6 @@ static void radisys_set_dmamode (struct ata_port *ap, struct ata_device *adev)

udma_enable |= (1 << adev->devno);
}
- pci_write_config_word(dev, 0x40, idetm_data);
pci_write_config_byte(dev, 0x48, udma_enable);

/* Track which port is configured */
--
1.7.1

2011-02-08 13:00:43

by Alan

[permalink] [raw]
Subject: Re: [PATCH 01/20] ata_piix: SITRE handling fix

On Tue, 08 Feb 2011 13:23:37 +0100
Bartlomiej Zolnierkiewicz <[email protected]> wrote:

> >From 7b6d27a9d65fef8795fdeee3733316450f840aa6 Mon Sep 17 00:00:00 2001
> From: Bartlomiej Zolnierkiewicz <[email protected]>
> Date: Tue, 8 Feb 2011 12:39:24 +0100
> Subject: [PATCH 01/20] ata_piix: SITRE handling fix
>
> Set SITRE bit also in master-only configurations.

Why ?

Subject: Re: [PATCH 01/20] ata_piix: SITRE handling fix

On Tue, Feb 8, 2011 at 2:04 PM, Alan Cox <[email protected]> wrote:
> On Tue, 08 Feb 2011 13:23:37 +0100
> Bartlomiej Zolnierkiewicz <[email protected]> wrote:
>
>> >From 7b6d27a9d65fef8795fdeee3733316450f840aa6 Mon Sep 17 00:00:00 2001
>> From: Bartlomiej Zolnierkiewicz <[email protected]>
>> Date: Tue, 8 Feb 2011 12:39:24 +0100
>> Subject: [PATCH 01/20] ata_piix: SITRE handling fix
>>
>> Set SITRE bit also in master-only configurations.
>
> Why ?

Mainly to match other PIIX-alike drivers and make further changes easier..

2011-02-08 13:03:28

by Alan

[permalink] [raw]
Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

On Tue, 08 Feb 2011 13:24:09 +0100
Bartlomiej Zolnierkiewicz <[email protected]> wrote:

> >From 8acc9371550ce5f98da56f6888ae4b898096ed2c Mon Sep 17 00:00:00 2001
> From: Bartlomiej Zolnierkiewicz <[email protected]>
> Date: Tue, 8 Feb 2011 12:39:25 +0100
> Subject: [PATCH 05/20] pata_efar: always program master_data before slave_data
>
> We may need to set SITRE before programming slave_data.

Do you have a documentation cite for this or is it random fiddling with a
driver you can't test ?

2011-02-08 13:09:51

by Alan

[permalink] [raw]
Subject: Re: [PATCH 16/20] ata_piix: add EFAR SLC90E66 support

On Tue, 08 Feb 2011 13:25:34 +0100
Bartlomiej Zolnierkiewicz <[email protected]> wrote:

> >From 11bed7feff5de752c9440ca58b232846b20e2ed6 Mon Sep 17 00:00:00 2001
> From: Bartlomiej Zolnierkiewicz <[email protected]>
> Date: Tue, 8 Feb 2011 12:39:28 +0100
> Subject: [PATCH 16/20] ata_piix: add EFAR SLC90E66 support
>
> Add EFAR SLC90E66 support to ata_piix and remove no longer
> needed pata_efar driver.

Jeff specifically asked that these were not all folded into ata_piix
originally. This also makes memory usage higher and the system less
efficient as these are all motherboard chipsets (except an obscure dual
PIIX4 setup) so you are loading more not less code.

It also leads to hideous uglies in the main code paths like this :

+ unsigned int has_sitre = (dev->vendor != 0x8086 ||
+ dev->device != 0x1230);

which also has exactly zero comments.

and it gets better as we go on...

+ unsigned int is_radisys = (dev->vendor ==
PCI_VENDOR_ID_RADISYS &&
+ dev->device == 0x8201);
+ unsigned int has_sitre = (dev->vendor !=
PCI_VENDOR_ID_INTEL ||
+ dev->device != 0x1230) && !is_radisys;


Folding these together just makes no sense at all. The PIO/DMA folds
internally do look sensible, but really should be tested before anything
goes in. The current code works - its slower than it should be because of
core libata bugs, but it works.

Alan

Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

On Tue, Feb 8, 2011 at 2:07 PM, Alan Cox <[email protected]> wrote:
> On Tue, 08 Feb 2011 13:24:09 +0100
> Bartlomiej Zolnierkiewicz <[email protected]> wrote:
>
>> >From 8acc9371550ce5f98da56f6888ae4b898096ed2c Mon Sep 17 00:00:00 2001
>> From: Bartlomiej Zolnierkiewicz <[email protected]>
>> Date: Tue, 8 Feb 2011 12:39:25 +0100
>> Subject: [PATCH 05/20] pata_efar: always program master_data before slave_data
>>
>> We may need to set SITRE before programming slave_data.
>
> Do you have a documentation cite for this or is it random fiddling with a
> driver you can't test ?

It is good to enable SITRE register before programming it and all
Intel controllers
(from which EFAR chipset is derived) are programmed this way (per ata_piix.c).

2011-02-08 13:21:45

by Alan

[permalink] [raw]
Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

> >> We may need to set SITRE before programming slave_data.
> >
> > Do you have a documentation cite for this or is it random fiddling with a
> > driver you can't test ?
>
> It is good to enable SITRE register before programming it and all
> Intel controllers

That sounds like someone quoting religion. Documentation cite please.

Subject: Re: [PATCH 16/20] ata_piix: add EFAR SLC90E66 support

On Tue, Feb 8, 2011 at 2:13 PM, Alan Cox <[email protected]> wrote:
> On Tue, 08 Feb 2011 13:25:34 +0100
> Bartlomiej Zolnierkiewicz <[email protected]> wrote:
>
>> >From 11bed7feff5de752c9440ca58b232846b20e2ed6 Mon Sep 17 00:00:00 2001
>> From: Bartlomiej Zolnierkiewicz <[email protected]>
>> Date: Tue, 8 Feb 2011 12:39:28 +0100
>> Subject: [PATCH 16/20] ata_piix: add EFAR SLC90E66 support
>>
>> Add EFAR SLC90E66 support to ata_piix and remove no longer
>> needed pata_efar driver.
>
> Jeff specifically asked that these were not all folded into ata_piix
> originally. This also makes memory usage higher and the system less
> efficient as these are all motherboard chipsets (except an obscure dual
> PIIX4 setup) so you are loading more not less code.

A lot more from a generic SCSI code if you want to optimize things for
old or embedded systems. IMHO this is not a best place to look for
such optimizations because maintenance cost > potential savings (i.e.
making SCSI quirks optional, I have draft patch for this, itself cuts
like 10k).

> It also leads to hideous uglies in the main code paths like this :
>
> + unsigned int has_sitre = (dev->vendor != 0x8086 ||
> + dev->device != 0x1230);
>
> which also has exactly zero comments.

has_sitre variable name is documentation in itself for anyone knowing
the hardware or has read a chipset/code documentation.

Though more comments can certainly be added if needed..

Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

On Tue, Feb 8, 2011 at 2:25 PM, Alan Cox <[email protected]> wrote:
>> >> We may need to set SITRE before programming slave_data.
>> >
>> > Do you have a documentation cite for this or is it random fiddling with a
>> > driver you can't test ?
>>
>> It is good to enable SITRE register before programming it and all
>> Intel controllers
>
> That sounds like someone quoting religion. Documentation cite please.

s/religion/common sense/

2011-02-08 13:34:36

by Alan

[permalink] [raw]
Subject: Re: [PATCH 16/20] ata_piix: add EFAR SLC90E66 support

> such optimizations because maintenance cost > potential savings (i.e.
> making SCSI quirks optional, I have draft patch for this, itself cuts
> like 10k).

Interesting in itself but irrelevant because the current situation is
that a piix change cannot break oldpiix, efar, it8213, radisys etc and
vice versa. Particuarly important when the other chips are not common so
test coverage is difficult, and that far outweighs the maintenance
savings for other things, especially as this is code that just doesn't
change.

>
> > It also leads to hideous uglies in the main code paths like this :
> >
> > + unsigned int has_sitre = (dev->vendor != 0x8086 ||
> > + dev->device != 0x1230);
> >
> > which also has exactly zero comments.
>
> has_sitre variable name is documentation in itself for anyone knowing
> the hardware or has read a chipset/code documentation.

And naturally anyone randomly glancing at the code knows why it's
checking 0x8086 & 0x1230, and why the radisys check interacts with it.

Bartlomiej - those are a mess, a complete and total mess. It doesn't
necessarily argue against folding them together, but at least do a clean
job of it.

2011-02-08 13:35:50

by Alan

[permalink] [raw]
Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

> > That sounds like someone quoting religion. Documentation cite please.
>
> s/religion/common sense/

Anyone who programs ATA controllers on the basis of common sense rather
than documentation, errata sheets and actually testing rather than
speculating is na?ve. In the case of ATA more so than most hardware.

Stil waiting a documentation cite for your otherwise gratuitious and
untested change

2011-02-08 13:39:45

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

Hello.

On 08-02-2011 16:25, Alan Cox wrote:

>>>> We may need to set SITRE before programming slave_data.

>>> Do you have a documentation cite for this or is it random fiddling with a
>>> driver you can't test ?

>> It is good to enable SITRE register before programming it and all
>> Intel controllers

> That sounds like someone quoting religion. Documentation cite please.

SLC90E66 datasheet only says that SIDETIM register has no effect without
SITRE bit set.

WBR, Sergei

Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

On Tue, Feb 8, 2011 at 2:39 PM, Alan Cox <[email protected]> wrote:
>> > That sounds like someone quoting religion. Documentation cite please.
>>
>> s/religion/common sense/
>
> Anyone who programs ATA controllers on the basis of common sense rather
> than documentation, errata sheets and actually testing rather than
> speculating is na?ve. In the case of ATA more so than most hardware.

Fully agreed.

> Stil waiting a documentation cite for your otherwise gratuitious and
> untested change

Alan, I'm not doing real-time consulting with you. The code is
provided in the best effort manner and it most likely will be
underrated (like all ATA/IDE work is) anyway so I do not really care
that much if it will be merged ever. Once it will be merged it may
only mean more unpaid support time for me since some maintainers are
not doing any real maintenance work these days.. Please at least
respect some of my time spent on all this burdensome silly little
driver differences comparisons and read the whole patch set before
making comments on individual changes (which you certainly haven't
done given timing of your review mails and complexity of changes)..

Subject: Re: [PATCH 16/20] ata_piix: add EFAR SLC90E66 support

On Tue, Feb 8, 2011 at 2:38 PM, Alan Cox <[email protected]> wrote:
>> such optimizations because maintenance cost > potential savings (i.e.
>> making SCSI quirks optional, I have draft patch for this, itself cuts
>> like 10k).
>
> Interesting in itself but irrelevant because the current situation is
> that a piix change cannot break oldpiix, efar, it8213, radisys etc and
> vice versa. Particuarly important when the other chips are not common so
> test coverage is difficult, and that far outweighs the maintenance
> savings for other things, especially as this is code that just doesn't
> change.
>
>>
>> > It also leads to hideous uglies in the main code paths like this :
>> >
>> > + ? ? ? unsigned int has_sitre ?= (dev->vendor != 0x8086 ||
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?dev->device != 0x1230);
>> >
>> > which also has exactly zero comments.
>>
>> has_sitre variable name is documentation in itself for anyone knowing
>> the hardware or has read a chipset/code documentation.
>
> And naturally anyone randomly glancing at the code knows why it's
> checking 0x8086 & 0x1230, and why the radisys check interacts with it.
>
> Bartlomiej - those are a mess, a complete and total mess. It doesn't
> necessarily argue against folding them together, but at least do a clean
> job of it.

I beg to differ regardless "the mess" comment but well, you can always
take my work and "add value" to it like in 2009 (when somehow you miss
that your pata_rdc also needs locking fixes but you were more
concerned with little differences between my work and your
"dreamwork"..)

2011-02-08 14:09:08

by Alan

[permalink] [raw]
Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

> respect some of my time spent on all this burdensome silly little
> driver differences comparisons and read the whole patch set before
> making comments on individual changes (which you certainly haven't
> done given timing of your review mails and complexity of changes)..

I was hoping you'd improved but apparently not.

Any untested change is dangerous. An untested change that merges drivers
together simply means you can break lots of stuff for no gain at all.

If these were all PCI card devices it might make some sense but given
they are all motherboard chipsets putting them into one driver merely
increases memory use as well.

As far as stuff like

? unsigned int has_sitre ?= (dev->vendor != 0x8086 ||
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?dev->device != 0x1230);

and the even worse mess you generate with the added patch all the other
PIIX code does this by flags, and if you had a HAS_SITRE (or NO_SITRE)
flag in the device data it would be obvious to anyone reading stuff how
it all fitted together.

Alan

Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

On Tue, Feb 8, 2011 at 3:12 PM, Alan Cox <[email protected]> wrote:
>> respect some of my time spent on all this burdensome silly little
>> driver differences comparisons and read the whole patch set before
>> making comments on individual changes (which you certainly haven't
>> done given timing of your review mails and complexity of changes)..
>
> I was hoping you'd improved but apparently not.

I'm not the person who doesn't change..

> Any untested change is dangerous. An untested change that merges drivers
> together simply means you can break lots of stuff for no gain at all.

This is why patches were posted to mailing list with a request for a
real hardware testing:

"All testing was done using QEMU's PIIX3 controller emulation so any testing
with real EFAR, IT8213, old PIIX, RDC and Radisys R82600 PATA controllers
would be really appreciated.."

instead of request for a merge. It was all there in initial mail.

Besides decreased complexity,1400 LOC gone and 5 drivers removed
cannot be really called "no gain at all".

> If these were all PCI card devices it might make some sense but given
> they are all motherboard chipsets putting them into one driver merely
> increases memory use as well.

We can improve situation here by doing generic ATA or SCSI changes
instead, not worth keeping separate drivers when you make similar
effect but more maintainable and generic changes.

> As far as stuff like
>
> ?? unsigned int has_sitre ?= (dev->vendor != 0x8086 ||
> ?? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?dev->device != 0x1230);
>
> and the even worse mess you generate with the added patch all the other
> PIIX code does this by flags, and if you had a HAS_SITRE (or NO_SITRE)
> flag in the device data it would be obvious to anyone reading stuff how
> it all fitted together.

Now that's what is a real review work. It will be changed in the next
revision, thanks!

-- Bartlomiej

2011-02-10 14:25:00

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

Hello.

On 08-02-2011 15:24, Bartlomiej Zolnierkiewicz wrote:
[...]

> We may need to set SITRE before programming slave_data.

> This makes pata_efar match the behavior of IDE's slc90e66 host driver
> and also of libata's ata_piix one.

> Signed-off-by: Bartlomiej Zolnierkiewicz<[email protected]>
[...]

> diff --git a/drivers/ata/pata_efar.c b/drivers/ata/pata_efar.c
> index 1e2ff7d..7f564d7 100644
> --- a/drivers/ata/pata_efar.c
> +++ b/drivers/ata/pata_efar.c
> @@ -74,10 +74,12 @@ static void efar_set_timings(struct ata_port *ap, struct ata_device *adev,
> u8 pio, bool use_mwdma)
> {
> struct pci_dev *dev = to_pci_dev(ap->host->dev);
> + unsigned int is_slave = (adev->devno != 0);

What's the point of this variable? To save one pointer dereference? :-)

> unsigned long flags;
> u8 master_port = ap->port_no ? 0x42 : 0x40;
> u16 master_data;
> u8 udma_enable;
> + u8 slave_data;
> int control = 0;
>
> /*
> @@ -110,14 +112,13 @@ static void efar_set_timings(struct ata_port *ap, struct ata_device *adev,
> pci_read_config_word(dev, master_port,&master_data);
>
> /* Set PPE, IE, and TIME as appropriate */
> - if (adev->devno == 0) {
> + if (is_slave == 0) {

!is_slave appeals more to me. :-)

> @@ -126,12 +127,14 @@ static void efar_set_timings(struct ata_port *ap, struct ata_device *adev,
> pci_read_config_byte(dev, 0x44,&slave_data);
> slave_data&= ap->port_no ? 0x0F : 0xF0;
> slave_data |= ((timings[pio][0]<< 2) | timings[pio][1])<< shift;
> - pci_write_config_byte(dev, 0x44, slave_data);
> }
>
> master_data |= 0x4000; /* Ensure SITRE is set */
> pci_write_config_word(dev, master_port, master_data);
>
> + if (is_slave)
> + pci_write_config_byte(dev, 0x44, slave_data);
> +
> pci_read_config_byte(dev, 0x48,&udma_enable);
> udma_enable&= ~(1<< (2 * ap->port_no + adev->devno));
> pci_write_config_byte(dev, 0x48, udma_enable);

WBR, Sergei

2011-02-10 14:29:40

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 14/20] pata_oldpiix: add locking for parallel scanning

Hello.

On 08-02-2011 15:25, Bartlomiej Zolnierkiewicz wrote:
[...]

> Add an extra locking for parallel scanning.

> This is similar change as commit 60c3be3 for ata_piix host driver

Linus asks to also specify the commit summary in parens.

> and while pata_oldpiix doesn't enable parallel scan yet the race
> could probably also be triggered by requesting re-scanning of both
> ports at the same time using SCSI sysfs interface.

> Fix documentation while at it.

> Acked-by: Alan Cox <[email protected]>
> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
[...]

> diff --git a/drivers/ata/pata_oldpiix.c b/drivers/ata/pata_oldpiix.c
> index 433d2fc..845ede1 100644
> --- a/drivers/ata/pata_oldpiix.c
> +++ b/drivers/ata/pata_oldpiix.c
> @@ -1,7 +1,8 @@
> /*
> - * pata_oldpiix.c - Intel PATA/SATA controllers
> + * pata_oldpiix.c - older Intel PATA controllers

I'd say "controller", since there's only one. :-)

WBR, Sergei

Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

Hi,

On Thu, Feb 10, 2011 at 3:23 PM, Sergei Shtylyov <[email protected]> wrote:
> Hello.
>
> On 08-02-2011 15:24, Bartlomiej Zolnierkiewicz wrote:
> [...]
>
>> We may need to set SITRE before programming slave_data.
>
>> This makes pata_efar match the behavior of IDE's slc90e66 host driver
>> and also of libata's ata_piix one.
>
>> Signed-off-by: Bartlomiej Zolnierkiewicz<[email protected]>
>
> [...]
>
>> diff --git a/drivers/ata/pata_efar.c b/drivers/ata/pata_efar.c
>> index 1e2ff7d..7f564d7 100644
>> --- a/drivers/ata/pata_efar.c
>> +++ b/drivers/ata/pata_efar.c
>> @@ -74,10 +74,12 @@ static void efar_set_timings(struct ata_port *ap,
>> struct ata_device *adev,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? u8 pio, bool use_mwdma)
>> ?{
>> ? ? ? ?struct pci_dev *dev ? ? = to_pci_dev(ap->host->dev);
>> + ? ? ? unsigned int is_slave ? = (adev->devno != 0);
>
> ? What's the point of this variable? To save one pointer dereference? :-)

Make code more similar to ata_piix.c and thus easier for comparisons
through 'diff -ub'.

In reality it doesn't matter now that much as pata_efar (same for
pata_oldpiix) vanishes completely at the end of the patch series.. :-)

Thanks,
Bartlomiej

2011-02-10 17:57:23

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

Hello.

Bartlomiej Zolnierkiewicz wrote:

>> On 08-02-2011 15:24, Bartlomiej Zolnierkiewicz wrote:
>> [...]

>>> We may need to set SITRE before programming slave_data.
>>> This makes pata_efar match the behavior of IDE's slc90e66 host driver
>>> and also of libata's ata_piix one.

>>> Signed-off-by: Bartlomiej Zolnierkiewicz<[email protected]>
>> [...]

>>> diff --git a/drivers/ata/pata_efar.c b/drivers/ata/pata_efar.c
>>> index 1e2ff7d..7f564d7 100644
>>> --- a/drivers/ata/pata_efar.c
>>> +++ b/drivers/ata/pata_efar.c
>>> @@ -74,10 +74,12 @@ static void efar_set_timings(struct ata_port *ap,
>>> struct ata_device *adev,
>>> u8 pio, bool use_mwdma)
>>> {
>>> struct pci_dev *dev = to_pci_dev(ap->host->dev);
>>> + unsigned int is_slave = (adev->devno != 0);

>> What's the point of this variable? To save one pointer dereference? :-)

> Make code more similar to ata_piix.c and thus easier for comparisons
> through 'diff -ub'.

> In reality it doesn't matter now that much as pata_efar (same for
> pata_oldpiix) vanishes completely at the end of the patch series.. :-)

Yeah, I understood that. Just don't have time to review the full series yet.

> Thanks,
> Bartlomiej

WBR, Sergei

Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

Hi,

On Tue, Feb 8, 2011 at 2:38 PM, Sergei Shtylyov <[email protected]> wrote:
> Hello.
>
> On 08-02-2011 16:25, Alan Cox wrote:
>
>>>>> We may need to set SITRE before programming slave_data.
>
>>>> Do you have a documentation cite for this or is it random fiddling with
>>>> a
>>>> driver you can't test ?
>
>>> It is good to enable SITRE register before programming it and all
>>> Intel controllers
>
>> That sounds like someone quoting religion. Documentation cite please.
>
> ? SLC90E66 datasheet only says that SIDETIM register has no effect without
> SITRE bit set.

Alan, is this explanation sufficient to ACK this patch? (Thanks
Sergei for digging it out.)

Jeff, would it be possible to queue patches #01-15 for 2.6.39 if there
are no further concerns with them (thus leaving the merging of
PIIX-like drivers for later)? They got additional testing on ICH4 and
they look mostly safe & straight-forward compared to #16-21.

Thanks,
Bartlomiej

2011-02-19 16:47:16

by Alan

[permalink] [raw]
Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

> > ? SLC90E66 datasheet only says that SIDETIM register has no effect without
> > SITRE bit set.

Which means the current setup is fine yes ?

> Alan, is this explanation sufficient to ACK this patch? (Thanks
> Sergei for digging it out.)
>
> Jeff, would it be possible to queue patches #01-15 for 2.6.39

Have they been tested on the relevant hardware yet - I don't believe the
changes should be made for the untested stuff and you indicated you
agreed with that.

The tidyups for the easily tested cases (PIIX4 etc) look sensible

Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

On Sat, Feb 19, 2011 at 5:48 PM, Alan Cox <[email protected]> wrote:
>> > ? SLC90E66 datasheet only says that SIDETIM register has no effect without
>> > SITRE bit set.
>
> Which means the current setup is fine yes ?

That's the problem, the current setup is buggy in slave-only setups:

slave_data |= ((timings[pio][0] << 2) |
timings[pio][1]) << shift;
pci_write_config_byte(dev, 0x44, slave_data);
}

idetm_data |= 0x4000; /* Ensure SITRE is set */
pci_write_config_word(dev, idetm_port, idetm_data);

idetm_port needs to be programmed _before_ slave_data to "ensure SITRE is set".

IOW We currently depend on the BIOS here to set SITRE bit for us..

>> Alan, is this explanation sufficient to ACK this patch? ?(Thanks
>> Sergei for digging it out.)
>>
>> Jeff, would it be possible to queue patches #01-15 for 2.6.39
>
> Have they been tested on the relevant hardware yet - I don't believe the
> changes should be made for the untested stuff and you indicated you
> agreed with that.

Unless they are obvious or risk is higher than benefit (bugfixes based
on code review). I don't think that ATA code has became recently
special in this regard compared to the rest of the kernel.

> The tidyups for the easily tested cases (PIIX4 etc) look sensible

Please take look at the changes #01-15 then:

ata_piix: SITRE handling fix
ata_piix: unify code for programming PIO and MWDMA timings
pata_efar: fix register naming used in efar_set_piomode()
pata_efar: unify code for programming PIO and MWDMA timings
pata_efar: always program master_data before slave_data
pata_it8213: fix register naming used in it8213_set_piomode()
pata_it8213: unify code for programming PIO and MWDMA timings
pata_it8213: add UDMA100 and UDMA133 support
pata_oldpiix: unify code for programming PIO and MWDMA timings
pata_radisys: unify code for programming PIO and MWDMA timings
pata_rdc: unify code for programming PIO and MWDMA timings
pata_rdc: parallel scanning needs an extra locking
pata_rdc: add Power Management support
pata_oldpiix: add locking for parallel scanning
pata_oldpiix: enable parallel scan

Most patches has been posted months ago in the past as the part of
atang tree. They don't contain risky stuff (it was intentionally
pushed at the end of the patch queue). We have here obvious register
naming unification, PIO/DMA folding (it was tested with ata_piix and
only later ported to other PIIX-like drivers) and:

* para_rdc: locking bug-fix and Power Management support

* pata_it8213: IDE -> libata regression fix (UDMA100/133 support based
on vendor / old IDE driver)

* pata_oldpiix: locking bug-fix (ACK-ed by you in the past) and
parallel scanning support

I see a lot of magnitude more risky stuff going elsewhere in the
kernel, including ATA itself and if there are any concerns left those
patches (#01-15) I'll be happy to address them.

Thanks,
Bartlomiej

2011-02-20 11:35:11

by Alan

[permalink] [raw]
Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

> idetm_data |= 0x4000; /* Ensure SITRE is set */
> pci_write_config_word(dev, idetm_port, idetm_data);
>
> idetm_port needs to be programmed _before_ slave_data to "ensure SITRE is set".

That isn't what the documentation seems to say. It says it has no effect
unless SITRE is set not that you can't program the registers.

> Unless they are obvious or risk is higher than benefit (bugfixes based
> on code review). I don't think that ATA code has became recently
> special in this regard compared to the rest of the kernel.

They aren't special - random unneeded code changes that haven't been
tested shouldn't be going into the code unless there is a big gain. For
obscure ancient devices there isn't a gain.

> ata_piix: unify code for programming PIO and MWDMA timings

Which as I said makes sense

> pata_efar: fix register naming used in efar_set_piomode()
> pata_efar: unify code for programming PIO and MWDMA timings
> pata_efar: always program master_data before slave_data

All untested

> pata_it8213: fix register naming used in it8213_set_piomode()
> pata_it8213: unify code for programming PIO and MWDMA timings
> pata_it8213: add UDMA100 and UDMA133 support

All untested

> pata_oldpiix: unify code for programming PIO and MWDMA timings

Untested

> pata_radisys: unify code for programming PIO and MWDMA timings

Untested

> pata_rdc: unify code for programming PIO and MWDMA timings
> pata_rdc: parallel scanning needs an extra locking
> pata_rdc: add Power Management support

All untested but the locking is a clear bug fix and I think should go in

> pata_oldpiix: add locking for parallel scanning
> pata_oldpiix: enable parallel scan

This is an ancient device on some old pentium class boxes, it's not
worth adding this stuff really is it ? Well maybe putting the locking in
or at least comments that it will be needed ?

> Most patches has been posted months ago in the past as the part of
> atang tree.

So - where are the test reports

> * pata_it8213: IDE -> libata regression fix (UDMA100/133 support based
> on vendor / old IDE driver)

I didn't see it tested in the old IDE driver either.

> * pata_oldpiix: locking bug-fix (ACK-ed by you in the past) and
> parallel scanning support

I'm happy with the locking fix, I don't see the point in the parallel
scan - and that wants testing because I don't know how the hardware will
react. Most controllers were not designed for parallel scan and its a
path windows will never have exercised therefore may well never have been
tested out.

In the PIIX4+ cases Jeff insisted we chased this down with the hardware
folks in Intel to be sure it was ok. I'm not sure there is anyone who
remembers original PIIX however.

> I see a lot of magnitude more risky stuff going elsewhere in the
> kernel, including ATA itself

And being tested.

efar/it8213/radisys are going to be tricky to find testers for as they
are rare chips but the RDC is found in some of the embedded
CPU/ATA/Net/etc in a device embedded x86 devices.

Alan

2011-02-20 13:44:38

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 02/20] ata_piix: unify code for programming PIO and MWDMA timings

On 08.02.2011 15:23, Bartlomiej Zolnierkiewicz wrote:

> From ca6c1fbc7d853829524b87a3bcbbb1f67796281c Mon Sep 17 00:00:00 2001
> From: Bartlomiej Zolnierkiewicz<[email protected]>
> Date: Tue, 8 Feb 2011 12:39:24 +0100
> Subject: [PATCH 02/20] ata_piix: unify code for programming PIO and MWDMA timings

> Besides making things noticably simpler it results in ~2% decrease in
> the driver LOC count and also ~2% decrease in the driver binary size
> (as measured on x86-32).

> Fix piix_set_piomode() documentation while at it.

> Signed-off-by: Bartlomiej Zolnierkiewicz<[email protected]>
[...]

> diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
> index 0f4856d..c954f91 100644
> --- a/drivers/ata/ata_piix.c
> +++ b/drivers/ata/ata_piix.c
> @@ -696,22 +696,11 @@ static int piix_pata_prereset(struct ata_link *link, unsigned long deadline)
>
> static DEFINE_SPINLOCK(piix_lock);
>
> -/**
> - * piix_set_piomode - Initialize host controller PATA PIO timings
> - * @ap: Port whose timings we are configuring
> - * @adev: um
> - *
> - * Set PIO mode for device, in host controller PCI config space.
> - *
> - * LOCKING:
> - * None (inherited from caller).
> - */
> -
> -static void piix_set_piomode(struct ata_port *ap, struct ata_device *adev)
> +static void piix_set_timings(struct ata_port *ap, struct ata_device *adev,
> + u8 pio, bool use_mwdma)
> {
> struct pci_dev *dev = to_pci_dev(ap->host->dev);
> unsigned long flags;
> - unsigned int pio = adev->pio_mode - XFER_PIO_0;
> unsigned int is_slave = (adev->devno != 0);
> unsigned int master_port= ap->port_no ? 0x42 : 0x40;
> unsigned int slave_port = 0x44;
> @@ -732,14 +721,18 @@ static void piix_set_piomode(struct ata_port *ap, struct ata_device *adev)
> { 2, 1 },
> { 2, 3 }, };
>
> - if (pio >= 2)
> + if (pio >= 2 || use_mwdma)

The 'use_mwdma' check is actually superfluous...

> control |= 1; /* TIME1 enable */
> - if (ata_pio_need_iordy(adev))
> + if (ata_pio_need_iordy(adev) || use_mwdma)
> control |= 2; /* IE enable */

Why IORDY is enabled for MWDMA has always been beyond me... I understand
that the stupid Intel docs are to be blamed here.

> -
> /* Intel specifies that the PPE functionality is for disk only */
> if (adev->class == ATA_DEV_ATA)
> control |= 4; /* PPE enable */
> + /* If the drive MWDMA is faster than it can do PIO then
> + we must force PIO into PIO0 */

It's not the preferred multi-line comment style:

/*
* bla
* bla
*/

> + if (use_mwdma && adev->pio_mode < (XFER_PIO_0 + pio))

Parens not needed around +.

> @@ -803,31 +812,20 @@ static void do_pata_set_dmamode(struct ata_port *ap, struct ata_device *adev, in
> {
> struct pci_dev *dev = to_pci_dev(ap->host->dev);
> unsigned long flags;
> - u8 master_port = ap->port_no ? 0x42 : 0x40;
> - u16 master_data;
> u8 speed = adev->dma_mode;
> int devid = adev->devno + 2 * ap->port_no;
> u8 udma_enable = 0;
>
> - static const /* ISP RTC */
> - u8 timings[][2] = { { 0, 0 },
> - { 0, 0 },
> - { 1, 0 },
> - { 2, 1 },
> - { 2, 3 }, };
> -
> - spin_lock_irqsave(&piix_lock, flags);
> -
> - pci_read_config_word(dev, master_port, &master_data);
> - if (ap->udma_mask)
> - pci_read_config_byte(dev, 0x48, &udma_enable);

I don't think you should have removed that...

> @@ -860,56 +858,20 @@ static void do_pata_set_dmamode(struct ata_port *ap, struct ata_device *adev, in
> performance (WR_PingPong_En) */
> pci_write_config_word(dev, 0x54, ideconf);
> }
> +
> + pci_write_config_byte(dev, 0x48, udma_enable);
> +
> + spin_unlock_irqrestore(&piix_lock, flags);
> } else {
[...]
> - if (ap->udma_mask)
> - udma_enable &= ~(1 << devid);

This does not seem correct -- you must disable UDMA when programming MWDMA.

> -
> - pci_write_config_word(dev, master_port, master_data);
> + piix_set_timings(ap, adev, pio, 1);
> }
> - /* Don't scribble on 0x48 if the controller does not support UDMA */
> - if (ap->udma_mask)
> - pci_write_config_byte(dev, 0x48, udma_enable);
> -
> - spin_unlock_irqrestore(&piix_lock, flags);
> }
>
> /**

WBR, Sergei

2011-02-20 14:33:28

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 04/20] pata_efar: unify code for programming PIO and MWDMA timings

Hello.

On 08-02-2011 15:24, Bartlomiej Zolnierkiewicz wrote:

> From 2de7db4bcc16b7e95cca9ceb5921a6f620be76b7 Mon Sep 17 00:00:00 2001
> From: Bartlomiej Zolnierkiewicz<[email protected]>
> Date: Tue, 8 Feb 2011 12:39:25 +0100
> Subject: [PATCH 04/20] pata_efar: unify code for programming PIO and MWDMA timings

> Besides making things noticably simpler it results in ~10% decrease in
> the driver LOC count and also ~8% decrease in the driver binary size
> (as measured on x86-32).

> Signed-off-by: Bartlomiej Zolnierkiewicz<[email protected]>
[...]

> diff --git a/drivers/ata/pata_efar.c b/drivers/ata/pata_efar.c
> index afe92b7..1e2ff7d 100644
> --- a/drivers/ata/pata_efar.c
> +++ b/drivers/ata/pata_efar.c
[...]
> @@ -103,13 +92,18 @@ static void efar_set_piomode (struct ata_port *ap, struct ata_device *adev)
> { 2, 1 },
> { 2, 3 }, };
>
> - if (pio> 1)
> + if (pio> 1 || use_mwdma)
> control |= 1; /* TIME */
> - if (ata_pio_need_iordy(adev)) /* PIO 3/4 require IORDY */
> + if (ata_pio_need_iordy(adev) || use_mwdma)
> control |= 2; /* IE */

Same comments as for ata_piix for these two bits...

> /* Intel specifies that the prefetch/posting is for disk only */
> if (adev->class == ATA_DEV_ATA)
> control |= 4; /* PPE */
> + /* If the drive MWDMA is faster than it can do PIO then
> + we must force PIO into PIO0 */

Fix comment style, like in ata_piix.

> + if (use_mwdma && adev->pio_mode < (XFER_PIO_0 + pio))

Parens not needed around +.

> @@ -145,6 +139,22 @@ static void efar_set_piomode (struct ata_port *ap, struct ata_device *adev)
> }
>
> /**
> + * efar_set_piomode - Initialize host controller PATA PIO timings
> + * @ap: Port whose timings we are configuring
> + * @adev: Device to program
> + *
> + * Set PIO mode for device, in host controller PCI config space.
> + *
> + * LOCKING:
> + * None (inherited from caller).
> + */
> +
> +static void efar_set_piomode(struct ata_port *ap, struct ata_device *adev)
> +{
> + efar_set_timings(ap, adev, adev->pio_mode - XFER_PIO_0, 0);

s/0/false/? Likewise in ata_piix...

> @@ -158,29 +168,19 @@ static void efar_set_piomode (struct ata_port *ap, struct ata_device *adev)
> static void efar_set_dmamode (struct ata_port *ap, struct ata_device *adev)
> {
> struct pci_dev *dev = to_pci_dev(ap->host->dev);
> - u8 master_port = ap->port_no ? 0x42 : 0x40;
> - u16 master_data;
> u8 speed = adev->dma_mode;
> int devid = adev->devno + 2 * ap->port_no;
> unsigned long flags;
> u8 udma_enable;
>
> - static const /* ISP RTC */
> - u8 timings[][2] = { { 0, 0 },
> - { 0, 0 },
> - { 1, 0 },
> - { 2, 1 },
> - { 2, 3 }, };
> -
> - spin_lock_irqsave(&efar_lock, flags);
> -
> - pci_read_config_word(dev, master_port, &master_data);
> - pci_read_config_byte(dev, 0x48, &udma_enable);
> -
> if (speed>= XFER_UDMA_0) {
> - unsigned int udma = adev->dma_mode - XFER_UDMA_0;
> + unsigned int udma = speed - XFER_UDMA_0;
> u16 udma_timing;
>
> + spin_lock_irqsave(&efar_lock, flags);
> +
> + pci_read_config_byte(dev, 0x48, &udma_enable);

Don't think you should have moved this...

> +
> udma_enable |= (1 << devid);
>
> /* Load the UDMA mode number */
> @@ -188,50 +188,20 @@ static void efar_set_dmamode (struct ata_port *ap, struct ata_device *adev)
[...]
> - udma_enable &= ~(1 << devid);
> - pci_write_config_word(dev, master_port, master_data);

No, you should disable UDMA when programming MWDMA.

> + efar_set_timings(ap, adev, pio, 1);

s/1/true/? Likewise in ata_piix...

WBR, Sergei

2011-02-20 14:39:00

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 07/20] pata_it8213: unify code for programming PIO and MWDMA timings

Hello.

On 08-02-2011 15:24, Bartlomiej Zolnierkiewicz wrote:

> From 57632718f2e761e69b96e14f6900a487bad5d586 Mon Sep 17 00:00:00 2001
> From: Bartlomiej Zolnierkiewicz<[email protected]>
> Date: Tue, 8 Feb 2011 12:39:26 +0100
> Subject: [PATCH 07/20] pata_it8213: unify code for programming PIO and MWDMA timings

> Besides making things noticably simpler it results in ~9% decrease in
> the driver LOC count and also ~6% decrease in the driver binary size
> (as measured on x86-32).

> Signed-off-by: Bartlomiej Zolnierkiewicz<[email protected]>
[...]

> diff --git a/drivers/ata/pata_it8213.c b/drivers/ata/pata_it8213.c
> index 911f50b..ccfd1ae 100644
> --- a/drivers/ata/pata_it8213.c
> +++ b/drivers/ata/pata_it8213.c
[...]
> @@ -92,13 +81,18 @@ static void it8213_set_piomode (struct ata_port *ap, struct ata_device *adev)
> { 2, 1 },
> { 2, 3 }, };
>
> - if (pio > 1)
> + if (pio > 1 || use_mwdma)
> control |= 1; /* TIME */
> - if (ata_pio_need_iordy(adev)) /* PIO 3/4 require IORDY */
> + if (ata_pio_need_iordy(adev) || use_mwdma)
> control |= 2; /* IE */
> /* Bit 2 is set for ATAPI on the IT8213 - reverse of ICH/PIIX */
> if (adev->class != ATA_DEV_ATA)
> control |= 4; /* PPE */
> + /* If the drive MWDMA is faster than it can do PIO then
> + we must force PIO into PIO0 */

Fix the comment style, please.

> + if (use_mwdma && adev->pio_mode < (XFER_PIO_0 + pio))

Parens not needed around +.

> + /* Enable DMA timing only */
> + control |= 8; /* PIO cycles in PIO0 */
>
> pci_read_config_word(dev, master_port,&master_data);
>
> @@ -126,6 +120,22 @@ static void it8213_set_piomode (struct ata_port *ap, struct ata_device *adev)
> }
>
> /**
> + * it8213_set_piomode - Initialize host controller PATA PIO timings
> + * @ap: Port whose timings we are configuring
> + * @adev: Device whose timings we are configuring
> + *
> + * Set PIO mode for device, in host controller PCI config space.
> + *
> + * LOCKING:
> + * None (inherited from caller).
> + */
> +
> +static void it8213_set_piomode(struct ata_port *ap, struct ata_device *adev)
> +{
> + it8213_set_timings(ap, adev, adev->pio_mode - XFER_PIO_0, 0);

s/0/false/?

> @@ -140,23 +150,14 @@ static void it8213_set_piomode (struct ata_port *ap, struct ata_device *adev)
> static void it8213_set_dmamode (struct ata_port *ap, struct ata_device *adev)
> {
> struct pci_dev *dev = to_pci_dev(ap->host->dev);
> - u16 master_data;
> u8 speed = adev->dma_mode;
> int devid = adev->devno;
> u8 udma_enable;
>
> - static const /* ISP RTC */
> - u8 timings[][2] = { { 0, 0 },
> - { 0, 0 },
> - { 1, 0 },
> - { 2, 1 },
> - { 2, 3 }, };
> -
> - pci_read_config_word(dev, 0x40,&master_data);
> pci_read_config_byte(dev, 0x48,&udma_enable);

Strange, here you're not moving that read and nnot removing disabling of
UDMA -- is that because of the missing lock?

> @@ -184,46 +185,16 @@ static void it8213_set_dmamode (struct ata_port *ap, struct ata_device *adev)
[...]
> + it8213_set_timings(ap, adev, pio, 1);

s/1/true/?

> +
> udma_enable&= ~(1 << devid);
> - pci_write_config_word(dev, 0x40, master_data);
> }
> pci_write_config_byte(dev, 0x48, udma_enable);
> }

WBR, Sergei

2011-02-20 14:54:20

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 09/20] pata_oldpiix: unify code for programming PIO and MWDMA timings

Hello.

On 08-02-2011 15:24, Bartlomiej Zolnierkiewicz wrote:

> From 30f6c60ad1e2f9c640352b74279fdf8a930847bf Mon Sep 17 00:00:00 2001
> From: Bartlomiej Zolnierkiewicz<[email protected]>
> Date: Tue, 8 Feb 2011 12:39:26 +0100
> Subject: [PATCH 09/20] pata_oldpiix: unify code for programming PIO and MWDMA timings

> Besides making things noticably simpler it results in ~12% decrease in
> the driver LOC count and also ~5% decrease in the driver binary size
> (as measured on x86-32).

> Signed-off-by: Bartlomiej Zolnierkiewicz<[email protected]>
[...]

> diff --git a/drivers/ata/pata_oldpiix.c b/drivers/ata/pata_oldpiix.c
> index b811c16..433d2fc 100644
> --- a/drivers/ata/pata_oldpiix.c
> +++ b/drivers/ata/pata_oldpiix.c
[...]
> @@ -82,14 +71,18 @@ static void oldpiix_set_piomode (struct ata_port *ap, struct ata_device *adev)
> { 2, 1 },
> { 2, 3 }, };
>
> - if (pio> 1)
> + if (pio > 1 || use_mwdma)

'use_mwdma' check is superfluous...

> control |= 1; /* TIME */
> - if (ata_pio_need_iordy(adev))
> + if (ata_pio_need_iordy(adev) || use_mwdma)
> control |= 2; /* IE */
> -
> /* Intel specifies that the prefetch/posting is for disk only */
> if (adev->class == ATA_DEV_ATA)
> control |= 4; /* PPE */
> + /* If the drive MWDMA is faster than it can do PIO then
> + we must force PIO into PIO0 */

Please fix the comment style.

> + if (use_mwdma && adev->pio_mode < (XFER_PIO_0 + pio))

Parens not needed around +.

> + /* Enable DMA timing only */
> + control |= 8; /* PIO cycles in PIO0 */
>
> pci_read_config_word(dev, idetm_port,&idetm_data);
>
> @@ -113,6 +106,22 @@ static void oldpiix_set_piomode (struct ata_port *ap, struct ata_device *adev)
> }
>
> /**
> + * oldpiix_set_piomode - Initialize host controller PATA PIO timings
> + * @ap: Port whose timings we are configuring
> + * @adev: Device whose timings we are configuring
> + *
> + * Set PIO mode for device, in host controller PCI config space.
> + *
> + * LOCKING:
> + * None (inherited from caller).
> + */
> +
> +static void oldpiix_set_piomode(struct ata_port *ap, struct ata_device *adev)
> +{
> + oldpiix_set_timings(ap, adev, adev->pio_mode - XFER_PIO_0, 0);

s/0/false/?

> +}
> +
> +/**
> * oldpiix_set_dmamode - Initialize host controller PATA DMA timings
> * @ap: Port whose timings we are configuring
> * @adev: Device to program
> @@ -125,58 +134,15 @@ static void oldpiix_set_piomode (struct ata_port *ap, struct ata_device *adev)
[...]
> + oldpiix_set_timings(ap, adev, pio, 1);

s/1/true/?

WBR, Sergei

2011-02-20 15:02:56

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 10/20] pata_radisys: unify code for programming PIO and MWDMA timings

Hello.

On 08-02-2011 15:24, Bartlomiej Zolnierkiewicz wrote:

> From 656a1d1e17b035c7de7e7ea602f03724fdcbb336 Mon Sep 17 00:00:00 2001
> From: Bartlomiej Zolnierkiewicz<[email protected]>
> Date: Tue, 8 Feb 2011 12:39:26 +0100
> Subject: [PATCH 10/20] pata_radisys: unify code for programming PIO and MWDMA timings

> Besides making things noticably simpler it results in ~6% decrease in
> the driver LOC count and also ~1% decrease in the driver binary size
> (as measured on x86-32).

> Signed-off-by: Bartlomiej Zolnierkiewicz<[email protected]>
[...]

> diff --git a/drivers/ata/pata_radisys.c b/drivers/ata/pata_radisys.c
> index 8574b31..e85bb38 100644
> --- a/drivers/ata/pata_radisys.c
> +++ b/drivers/ata/pata_radisys.c
[...]
> @@ -62,6 +51,10 @@ static void radisys_set_piomode (struct ata_port *ap, struct ata_device *adev)
> control |= 1; /* TIME1 enable */
> if (ata_pio_need_iordy(adev))
> control |= 2; /* IE IORDY */
> + /* If the drive MWDMA is faster than it can do PIO then
> + we must force PIO0 for PIO cycles. */

Fix the comment style please.

> + if (use_mwdma && adev->pio_mode < (XFER_PIO_0 + pio))
> + control = 1;

Wait, this bit is enabling fast timings for both PIO and DMA, no?

> @@ -78,6 +71,22 @@ static void radisys_set_piomode (struct ata_port *ap, struct ata_device *adev)
> }
>
> /**
> + * radisys_set_piomode - Initialize host controller PATA PIO timings
> + * @ap: ATA port
> + * @adev: Device whose timings we are configuring
> + *
> + * Set PIO mode for device, in host controller PCI config space.
> + *
> + * LOCKING:
> + * None (inherited from caller).
> + */
> +
> +static void radisys_set_piomode(struct ata_port *ap, struct ata_device *adev)
> +{
> + radisys_set_timings(ap, adev, adev->pio_mode - XFER_PIO_0, 0);

s/0/false/?

> @@ -91,22 +100,10 @@ static void radisys_set_piomode (struct ata_port *ap, struct ata_device *adev)
> static void radisys_set_dmamode (struct ata_port *ap, struct ata_device *adev)
> {
> struct pci_dev *dev = to_pci_dev(ap->host->dev);
> - u16 idetm_data;
> u8 udma_enable;
>
> - static const /* ISP RTC */
> - u8 timings[][2] = { { 0, 0 },
> - { 0, 0 },
> - { 1, 1 },
> - { 2, 2 },
> - { 3, 3 }, };
> -
> - /*
> - * MWDMA is driven by the PIO timings. We must also enable
> - * IORDY unconditionally.
> - */
> + /* MWDMA is driven by the PIO timings. */
>
> - pci_read_config_word(dev, 0x40,&idetm_data);
> pci_read_config_byte(dev, 0x48,&udma_enable);

You're not touching this read here also...

> @@ -115,20 +112,8 @@ static void radisys_set_dmamode (struct ata_port *ap, struct ata_device *adev)
[...]
> + radisys_set_timings(ap, adev, pio, 1);

s/1/true/?

WBR, Sergei

2011-02-20 15:05:30

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 12/20] pata_rdc: parallel scanning needs an extra locking

Hello.

On 08-02-2011 15:25, Bartlomiej Zolnierkiewicz wrote:

> From 540001238b8ebbc4bedd46ca4e2672fcf3787998 Mon Sep 17 00:00:00 2001
> From: Bartlomiej Zolnierkiewicz<[email protected]>
> Date: Tue, 8 Feb 2011 12:39:27 +0100
> Subject: [PATCH 12/20] pata_rdc: parallel scanning needs an extra locking

> This is similar change as commit 60c3be3 for ata_piix host driver.

Linus asks to also specify the commit summary in parens.

> Signed-off-by: Bartlomiej Zolnierkiewicz<[email protected]>

WBR, Sergei

2011-02-20 18:58:39

by Alan

[permalink] [raw]
Subject: Re: [PATCH 02/20] ata_piix: unify code for programming PIO and MWDMA timings

> > + if (ata_pio_need_iordy(adev) || use_mwdma)
> > control |= 2; /* IE enable */
>
> Why IORDY is enabled for MWDMA has always been beyond me... I understand
> that the stupid Intel docs are to be blamed here.

I fail to see whats stupid about the docs ?

The same timing register set is used for MWDMA and PIO cycles in MWDMA
tuned modes (eg ATAPI). Thus the controller needs to be in an MWDMA mode
whose timings are compatible with the PIO timing and we want IORDY in use
for the PIO transfer parts.

Alan

2011-02-20 20:44:09

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 02/20] ata_piix: unify code for programming PIO and MWDMA timings

Hello.

On 20-02-2011 21:59, Alan Cox wrote:

>>> + if (ata_pio_need_iordy(adev) || use_mwdma)
>>> control |= 2; /* IE enable */

>> Why IORDY is enabled for MWDMA has always been beyond me... I understand
>> that the stupid Intel docs are to be blamed here.

> I fail to see whats stupid about the docs ?

Association of IORDY with DMA mode is wrong.

> The same timing register set is used for MWDMA and PIO cycles in MWDMA
> tuned modes (eg ATAPI). Thus the controller needs to be in an MWDMA mode
> whose timings are compatible with the PIO timing and we want IORDY in use
> for the PIO transfer parts.

Yeah, especially if we also set the bit which only enables fast timing for
DMA. ;-)
PIO mode is setup by different code, and it takes care of the IORDY
setting according to the PIO rules (and it gets called). DMA mode setup should
just ignore the IORDY setting as in all other sane drivers.

> Alan

WBR, Sergei

2011-02-20 21:06:10

by Alan

[permalink] [raw]
Subject: Re: [PATCH 02/20] ata_piix: unify code for programming PIO and MWDMA timings

> PIO mode is setup by different code, and it takes care of the IORDY
> setting according to the PIO rules (and it gets called). DMA mode setup should
> just ignore the IORDY setting as in all other sane drivers.


Well it can't ignore it - but if you mean just keep the bit as is then
that sounds sensible, have to see what the docs say happens if you ever
set MWDMA without IORDY.

2011-02-21 11:40:03

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 02/20] ata_piix: unify code for programming PIO and MWDMA timings

Hello.

On 21-02-2011 0:07, Alan Cox wrote:

>> PIO mode is setup by different code, and it takes care of the IORDY
>> setting according to the PIO rules (and it gets called). DMA mode setup should
>> just ignore the IORDY setting as in all other sane drivers.

> Well it can't ignore it - but if you mean just keep the bit as is then

Yes, I mean this.

> that sounds sensible, have to see what the docs say happens if you ever
> set MWDMA without IORDY.

Don't think they say anything on this matter but I can't imagine that
IORDY matters for DMA.

WBR, Sergei

2011-02-21 11:54:50

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 02/20] ata_piix: unify code for programming PIO and MWDMA timings

Hello.

On 21-02-2011 14:38, Sergei Shtylyov wrote:

>>> PIO mode is setup by different code, and it takes care of the IORDY
>>> setting according to the PIO rules (and it gets called). DMA mode setup should
>>> just ignore the IORDY setting as in all other sane drivers.

>> Well it can't ignore it - but if you mean just keep the bit as is then

> Yes, I mean this.

>> that sounds sensible, have to see what the docs say happens if you ever
>> set MWDMA without IORDY.

> Don't think they say anything on this matter but I can't imagine that IORDY
> matters for DMA.

What it says is that IORDY enable bit has no effect when the fast timing
bit is cleared. That's from ICH IDE PRM (29860004.pdf).

WBR, Sergei

2011-02-21 11:57:10

by Alan

[permalink] [raw]
Subject: Re: [PATCH 02/20] ata_piix: unify code for programming PIO and MWDMA timings

On Mon, 21 Feb 2011 14:38:37 +0300
Sergei Shtylyov <[email protected]> wrote:

> Hello.
>
> On 21-02-2011 0:07, Alan Cox wrote:
>
> >> PIO mode is setup by different code, and it takes care of the IORDY
> >> setting according to the PIO rules (and it gets called). DMA mode setup should
> >> just ignore the IORDY setting as in all other sane drivers.
>
> > Well it can't ignore it - but if you mean just keep the bit as is then
>
> Yes, I mean this.
>
> > that sounds sensible, have to see what the docs say happens if you ever
> > set MWDMA without IORDY.
>
> Don't think they say anything on this matter but I can't imagine that
> IORDY matters for DMA.

Sure - but it does matter for an ATAPI transaction with then has a DMA
phase because those timings will be used for the command transfer which
is PIO.

Alan

2011-02-21 11:58:44

by Alan

[permalink] [raw]
Subject: Re: [PATCH 02/20] ata_piix: unify code for programming PIO and MWDMA timings

> > Don't think they say anything on this matter but I can't imagine that IORDY
> > matters for DMA.
>
> What it says is that IORDY enable bit has no effect when the fast timing
> bit is cleared. That's from ICH IDE PRM (29860004.pdf).

Ok thats the detail I missed - cool.

2011-02-21 12:00:30

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 02/20] ata_piix: unify code for programming PIO and MWDMA timings

Hello.

On 21-02-2011 14:58, Alan Cox wrote:

>>>> PIO mode is setup by different code, and it takes care of the IORDY
>>>> setting according to the PIO rules (and it gets called). DMA mode setup should
>>>> just ignore the IORDY setting as in all other sane drivers.

>>> Well it can't ignore it - but if you mean just keep the bit as is then

>> Yes, I mean this.

>>> that sounds sensible, have to see what the docs say happens if you ever
>>> set MWDMA without IORDY.

>> Don't think they say anything on this matter but I can't imagine that
>> IORDY matters for DMA.

... unless the IE bit means something special (and undocumented) for DMA,
if Intel so insists on setting it for every DMA mode.

> Sure - but it does matter for an ATAPI transaction with then has a DMA
> phase because those timings will be used for the command transfer which
> is PIO.

So what? We've already setup PIO timings before the DMA ones. Why force
the IE bit?

> Alan

WBR, Sergei

2011-02-21 20:07:05

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

On 02/19/2011 04:25 AM, Bartlomiej Zolnierkiewicz wrote:
> Jeff, would it be possible to queue patches #01-15 for 2.6.39 if there
> are no further concerns with them (thus leaving the merging of
> PIIX-like drivers for later)? They got additional testing on ICH4 and
> they look mostly safe& straight-forward compared to #16-21.

This seems to directly contradict what you wrote earlier in the thread,

This is why patches were posted to mailing list with a request
for a real hardware testing:

"All testing was done using QEMU's PIIX3 controller emulation
so any testing with real EFAR, IT8213, old PIIX, RDC and
Radisys R82600 PATA controllers would be really appreciated.."

instead of request for a merge. It was all there in initial
mail.

and

I do not really care that much if it will be merged ever

Regardless of this self-contradictory attitude, I do want useful patches
and many of these patches seem useful.

So I will continue watching the Bart/Alan/Sergei threads play out, and
then look at merging the result. In the midst of all the arguing,
productive work / forward progress is occurring, so the end result
should be positive.

It would be nice if we could get at least an "it works" test for the
older hardware, since those are the changes /least/ likely to be tested
by queueing to linux-next.

Jeff

Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

Hi,

On Mon, Feb 21, 2011 at 9:06 PM, Jeff Garzik <[email protected]> wrote:
> On 02/19/2011 04:25 AM, Bartlomiej Zolnierkiewicz wrote:
>>
>> Jeff, would it be possible to queue patches #01-15 for 2.6.39 if there
>> are no further concerns with them (thus leaving the merging of
>> PIIX-like drivers for later)? ?They got additional testing on ICH4 and
>> they look mostly safe& ?straight-forward compared to #16-21.
>
> This seems to directly contradict what you wrote earlier in the thread,
>
> ? ? ? ?This is why patches were posted to mailing list with a request
> ? ? ? ?for a real hardware testing:
>
> ? ? ? ?"All testing was done using QEMU's PIIX3 controller emulation
> ? ? ? ?so any testing with real EFAR, IT8213, old PIIX, RDC and
> ? ? ? ?Radisys R82600 PATA controllers would be really appreciated.."
>
> ? ? ? ?instead of request for a merge. ?It was all there in initial
> ? ? ? ?mail.
>
> and
>
> ? ? ? ?I do not really care that much if it will be merged ever
>
> Regardless of this self-contradictory attitude, I do want useful patches and
> many of these patches seem useful.

Nothing self-contradictory there. :)

First quote is about patches #01-15 only, not whole patchset (#01-20)
like the second one, and I still don't care _that_ much personally if
it gets merged since it is all unpaid & voluntary work.

> So I will continue watching the Bart/Alan/Sergei threads play out, and then
> look at merging the result. ?In the midst of all the arguing, productive
> work / forward progress is occurring, so the end result should be positive.
>
> It would be nice if we could get at least an "it works" test for the older
> hardware, since those are the changes /least/ likely to be tested by
> queueing to linux-next.

I was thinking about re-doing ata_piix part in a way that we could
merge it now by adding support for older PIIX-alikes to ata_piix and
making it enabled only if "all_piixalikes" module parameter is
specified. This way older drivers would be left untouched for now
and we can easily get in-tree testing for a new code. Does it sound
as a viable alternative?

Thanks,
Bartlomiej

2011-02-22 11:13:01

by Alan

[permalink] [raw]
Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

> I was thinking about re-doing ata_piix part in a way that we could
> merge it now by adding support for older PIIX-alikes to ata_piix and
> making it enabled only if "all_piixalikes" module parameter is
> specified. This way older drivers would be left untouched for now
> and we can easily get in-tree testing for a new code. Does it sound
> as a viable alternative?

That is the bit to me that makes the least sense. Each of the devices in
question is found only in the chipset so they can never be in combination
on a board (except multiple PIIX4 which is already covered)

Merging them makes it more likely stuff breaks and increases memory usage
- its a lose/lose situation.

Alan

Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

On Tue, Feb 22, 2011 at 12:14 PM, Alan Cox <[email protected]> wrote:
>> I was thinking about re-doing ata_piix part in a way that we could
>> merge it now by adding support for older PIIX-alikes to ata_piix and
>> making it enabled only if "all_piixalikes" module parameter is
>> specified. ? This way older drivers would be left untouched for now
>> and we can easily get in-tree testing for a new code. ?Does it sound
>> as a viable alternative?
>
> That is the bit to me that makes the least sense. Each of the devices in
> question is found only in the chipset so they can never be in combination
> on a board (except multiple PIIX4 which is already covered)
>
> Merging them makes it more likely stuff breaks and increases memory usage
> - its a lose/lose situation.

Each device is very similar to other one, so keeping 6 drivers for the
(almost) same stuff makes a little sense from the long-term
maintainability POV, It is a question of different trade-offs since
on the other side of equation we have 5 drivers less, 1400 LOCs less,
no risk of losing bugfixes and features (which is true already w/
pata_rdc lacking locking fixes + Power Management and pata_oldpiix
lacking parallel scanning feature).

As for increased memory usage -- we are talking here only about 10-20k
more. If it really is a problem maybe ata_piix can be redesigned into
ata_generic-style manner so with the help of existing config options
we can keep code size / memory usage on a existing level.

Thanks,
Bartlomiej

Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

On Wed, Feb 23, 2011 at 9:45 AM, Bartlomiej Zolnierkiewicz
<[email protected]> wrote:

> As for increased memory usage -- we are talking here only about 10-20k
> more. ?If it really is a problem maybe ata_piix can be redesigned into
> ata_generic-style manner so with the help of existing config options
> we can keep code size / memory usage on a existing level.

s/ata_generic/pata_legacy/ of course -- what I have in mind is making
Intel specific code depended on ATA_PIIX and adding new config option
ATA_PIIXALIKE for generic code which would be selected by existing
PATA_[EFAR,IT8213, OLDPIIX,RADISYS,RDC] options.

Thanks,
Bartlomiej

2011-02-23 09:13:20

by Alan

[permalink] [raw]
Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

> on the other side of equation we have 5 drivers less, 1400 LOCs less,
> no risk of losing bugfixes and features (which is true already w/
> pata_rdc lacking locking fixes + Power Management and pata_oldpiix
> lacking parallel scanning feature).

The parallel scanning feature is somewhat irrelevant on an original PIIX,
if the hardware even supports it which is somewhat of an unknown. If you
have a minor boot time performance concern you'd buy a real computer
instead.

And the other stuff you actually create a problem rather than solving one
- the difficulting testing the rare chips makes it harder to fix ata_piix
bugs and do full testing.

Alan

Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

Bartlomiej Zolnierkiewicz wrote:

> On Wed, Feb 23, 2011 at 9:45 AM, Bartlomiej Zolnierkiewicz
> <[email protected]> wrote:
>
> > As for increased memory usage -- we are talking here only about 10-20k
> > more. If it really is a problem maybe ata_piix can be redesigned into
> > ata_generic-style manner so with the help of existing config options
> > we can keep code size / memory usage on a existing level.
>
> s/ata_generic/pata_legacy/ of course -- what I have in mind is making
> Intel specific code depended on ATA_PIIX and adding new config option
> ATA_PIIXALIKE for generic code which would be selected by existing
> PATA_[EFAR,IT8213, OLDPIIX,RADISYS,RDC] options.

*Draft* patch against my current "piixalike" tree to just show the idea
and implement the final conversion stage (after optional code in ata_piix
gets tested and we are ready to remove separate drivers).

CONFIG_PATA_EFAR=y only:

before:
text data bss dec hex filename
1578 1008 8 2594 a22 drivers/ata/pata_efar.o

after:
text data bss dec hex filename
2778 1152 8 3938 f62 drivers/ata/ata_piix.o

so it pretty much solves code size / memory usage issue
(I'm counting in my other patch https://lkml.org/lkml/2011/2/9/150)..

---
drivers/ata/Kconfig | 102 +++---
drivers/ata/Makefile | 7
drivers/ata/ata_piix.c | 652 ++++++++++++++++++-----------------------
drivers/ata/ata_piix_efar.h | 28 +
drivers/ata/ata_piix_it8213.h | 28 +
drivers/ata/ata_piix_oldpiix.h | 49 +++
6 files changed, 454 insertions(+), 412 deletions(-)

Index: b/drivers/ata/Kconfig
===================================================================
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -167,9 +167,19 @@ if ATA_BMDMA

comment "SATA SFF controllers with BMDMA"

+config ATA_PIIXALIKE
+ tristate "Intel PIIX-alikes PATA/SATA support"
+ depends on PCI
+ default y
+ help
+ This option enables support needed for all Intel PIIX-alike
+ Serial ATA and PATA host controllers.
+
+ If unsure, say Y.
+
config ATA_PIIX
tristate "Intel ESB, ICH, PIIX3, PIIX4 PATA/SATA support"
- depends on PCI
+ depends on ATA_PIIXALIKE
help
This option enables support for ICH5/6/7/8 Serial ATA
and support for PATA on the Intel ESB/ICH/PIIX3/PIIX4 series
@@ -177,6 +187,51 @@ config ATA_PIIX

If unsure, say N.

+config PATA_EFAR
+ bool "EFAR SLC90E66 support"
+ depends on ATA_PIIXALIKE
+ help
+ This option enables support for the EFAR SLC90E66
+ IDE controller found on some older machines.
+
+ If unsure, say N.
+
+config PATA_IT8213
+ bool "IT8213 PATA support (Experimental)"
+ depends on ATA_PIIXALIKE && EXPERIMENTAL
+ help
+ This option enables support for the ITE 821 PATA
+ controllers via the new ATA layer.
+
+ If unsure, say N.
+
+config PATA_OLDPIIX
+ bool "Intel PATA old PIIX support"
+ depends on ATA_PIIXALIKE
+ help
+ This option enables support for early PIIX PATA support.
+
+ If unsure, say N.
+
+config PATA_RADISYS
+ bool "RADISYS 82600 PATA support (Experimental)"
+ depends on ATA_PIIXALIKE && EXPERIMENTAL
+ help
+ This option enables support for the RADISYS 82600
+ PATA controllers via the new ATA layer
+
+ If unsure, say N.
+
+config PATA_RDC
+ bool "RDC PATA support"
+ depends on ATA_PIIXALIKE
+ help
+ This option enables basic support for the later RDC PATA controllers
+ controllers via the new ATA layer. For the RDC 1010, you need to
+ enable the IT821X driver instead.
+
+ If unsure, say N.
+
config SATA_DWC
tristate "DesignWare Cores SATA support"
depends on 460EX
@@ -372,15 +427,6 @@ config PATA_CYPRESS

If unsure, say N.

-config PATA_EFAR
- tristate "EFAR SLC90E66 support"
- depends on PCI
- help
- This option enables support for the EFAR SLC90E66
- IDE controller found on some older machines.
-
- If unsure, say N.
-
config PATA_HPT366
tristate "HPT 366/368 PATA support"
depends on PCI
@@ -433,15 +479,6 @@ config PATA_ICSIDE
interface card. This is not required for ICS partition support.
If you are unsure, say N to this.

-config PATA_IT8213
- tristate "IT8213 PATA support (Experimental)"
- depends on PCI && EXPERIMENTAL
- help
- This option enables support for the ITE 821 PATA
- controllers via the new ATA layer.
-
- If unsure, say N.
-
config PATA_IT821X
tristate "IT8211/2 PATA support"
depends on PCI
@@ -518,14 +555,6 @@ config PATA_NS87415

If unsure, say N.

-config PATA_OLDPIIX
- tristate "Intel PATA old PIIX support"
- depends on PCI
- help
- This option enables support for early PIIX PATA support.
-
- If unsure, say N.
-
config PATA_OPTIDMA
tristate "OPTI FireStar PATA support (Very Experimental)"
depends on PCI && EXPERIMENTAL
@@ -553,25 +582,6 @@ config PATA_PDC_OLD

If unsure, say N.

-config PATA_RADISYS
- tristate "RADISYS 82600 PATA support (Experimental)"
- depends on PCI && EXPERIMENTAL
- help
- This option enables support for the RADISYS 82600
- PATA controllers via the new ATA layer
-
- If unsure, say N.
-
-config PATA_RDC
- tristate "RDC PATA support"
- depends on PCI
- help
- This option enables basic support for the later RDC PATA controllers
- controllers via the new ATA layer. For the RDC 1010, you need to
- enable the IT821X driver instead.
-
- If unsure, say N.
-
config PATA_SC1200
tristate "SC1200 PATA support"
depends on PCI
Index: b/drivers/ata/Makefile
===================================================================
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -16,7 +16,7 @@ obj-$(CONFIG_SATA_QSTOR) += sata_qstor.o
obj-$(CONFIG_SATA_SX4) += sata_sx4.o

# SFF SATA w/ BMDMA
-obj-$(CONFIG_ATA_PIIX) += ata_piix.o
+obj-$(CONFIG_ATA_PIIXALIKE) += ata_piix.o
obj-$(CONFIG_SATA_MV) += sata_mv.o
obj-$(CONFIG_SATA_NV) += sata_nv.o
obj-$(CONFIG_SATA_PROMISE) += sata_promise.o
@@ -40,13 +40,11 @@ obj-$(CONFIG_PATA_CS5530) += pata_cs5530
obj-$(CONFIG_PATA_CS5535) += pata_cs5535.o
obj-$(CONFIG_PATA_CS5536) += pata_cs5536.o
obj-$(CONFIG_PATA_CYPRESS) += pata_cypress.o
-obj-$(CONFIG_PATA_EFAR) += pata_efar.o
obj-$(CONFIG_PATA_HPT366) += pata_hpt366.o
obj-$(CONFIG_PATA_HPT37X) += pata_hpt37x.o
obj-$(CONFIG_PATA_HPT3X2N) += pata_hpt3x2n.o
obj-$(CONFIG_PATA_HPT3X3) += pata_hpt3x3.o
obj-$(CONFIG_PATA_ICSIDE) += pata_icside.o
-obj-$(CONFIG_PATA_IT8213) += pata_it8213.o
obj-$(CONFIG_PATA_IT821X) += pata_it821x.o
obj-$(CONFIG_PATA_JMICRON) += pata_jmicron.o
obj-$(CONFIG_PATA_MACIO) += pata_macio.o
@@ -55,12 +53,9 @@ obj-$(CONFIG_PATA_MPC52xx) += pata_mpc52
obj-$(CONFIG_PATA_NETCELL) += pata_netcell.o
obj-$(CONFIG_PATA_NINJA32) += pata_ninja32.o
obj-$(CONFIG_PATA_NS87415) += pata_ns87415.o
-obj-$(CONFIG_PATA_OLDPIIX) += pata_oldpiix.o
obj-$(CONFIG_PATA_OPTIDMA) += pata_optidma.o
obj-$(CONFIG_PATA_PDC2027X) += pata_pdc2027x.o
obj-$(CONFIG_PATA_PDC_OLD) += pata_pdc202xx_old.o
-obj-$(CONFIG_PATA_RADISYS) += pata_radisys.o
-obj-$(CONFIG_PATA_RDC) += pata_rdc.o
obj-$(CONFIG_PATA_SC1200) += pata_sc1200.o
obj-$(CONFIG_PATA_SCC) += pata_scc.o
obj-$(CONFIG_PATA_SCH) += pata_sch.o
Index: b/drivers/ata/ata_piix.c
===================================================================
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -169,6 +169,7 @@ enum piix_controller_ids {
it8213_pata,
oldpiix_pata,
radisys_pata,
+ rdc_pata,
ich5_sata,
ich6_sata,
ich6m_sata,
@@ -191,33 +192,10 @@ struct piix_host_priv {
void __iomem *sidpr;
};

-static int piix_init_one(struct pci_dev *pdev,
- const struct pci_device_id *ent);
-static void piix_remove_one(struct pci_dev *pdev);
-static int piix_pata_prereset(struct ata_link *link, unsigned long deadline);
-static void piix_set_piomode(struct ata_port *ap, struct ata_device *adev);
-static void piix_set_dmamode(struct ata_port *ap, struct ata_device *adev);
-static void ich_set_dmamode(struct ata_port *ap, struct ata_device *adev);
-static int ich_pata_cable_detect(struct ata_port *ap);
-static int efar_pata_cable_detect(struct ata_port *ap);
-static int it8213_pata_cable_detect(struct ata_port *ap);
-static unsigned int oldpiix_qc_issue(struct ata_queued_cmd *qc);
-static u8 piix_vmw_bmdma_status(struct ata_port *ap);
-static int piix_sidpr_scr_read(struct ata_link *link,
- unsigned int reg, u32 *val);
-static int piix_sidpr_scr_write(struct ata_link *link,
- unsigned int reg, u32 val);
-static int piix_sidpr_set_lpm(struct ata_link *link, enum ata_lpm_policy policy,
- unsigned hints);
-static bool piix_irq_check(struct ata_port *ap);
-#ifdef CONFIG_PM
-static int piix_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg);
-static int piix_pci_device_resume(struct pci_dev *pdev);
-#endif
-
static unsigned int in_module_init = 1;

static const struct pci_device_id piix_pci_tbl[] = {
+#ifdef CONFIG_ATA_PIIX
/* Intel PIIX3 for the 430HX etc */
{ 0x8086, 0x7010, PCI_ANY_ID, PCI_ANY_ID, 0, 0, piix_pata_mwdma },
/* VMware ICH4 */
@@ -261,23 +239,29 @@ static const struct pci_device_id piix_p
{ 0x8086, 0x269E, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100_nomwdma1 },
/* ICH8 Mobile PATA Controller */
{ 0x8086, 0x2850, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
-
+#endif
+#ifdef CONFIG_PATA_EFAR
/* EFAR */
{ 0x1055, 0x9130, PCI_ANY_ID, PCI_ANY_ID, 0, 0, efar_pata },
-
+#endif
+#ifdef CONFIG_PATA_IT8213
/* IT8213 */
{ 0x1283, 0x8213, PCI_ANY_ID, PCI_ANY_ID, 0, 0, it8213_pata },
-
+#endif
+#ifdef CONFIG_PATA_RDC
/* RDC is ICH like */
- { 0x17F3, 0x1011, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
- { 0x17F3, 0x1012, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
-
+ { 0x17F3, 0x1011, PCI_ANY_ID, PCI_ANY_ID, 0, 0, rdc_pata },
+ { 0x17F3, 0x1012, PCI_ANY_ID, PCI_ANY_ID, 0, 0, rdc_pata },
+#endif
+#ifdef CONFIG_PATA_OLDPIIX
/* PIIXb (a.k.a. oldpiix) */
{ 0x8086, 0x1230, PCI_ANY_ID, PCI_ANY_ID, 0, 0, oldpiix_pata },
-
+#endif
+#ifdef CONFIG_PATA_RADISYS
/* Radisys R82600 */
{ 0x1331, 0x8201, PCI_ANY_ID, PCI_ANY_ID, 0, 0, radisys_pata },
-
+#endif
+#ifdef CONFIG_ATA_PIIX
/* SATA ports */

/* 82801EB (ICH5) */
@@ -359,98 +343,10 @@ static const struct pci_device_id piix_p
/* SATA Controller IDE (PBG) */
{ 0x8086, 0x1d08, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_2port_sata },
{ } /* terminate list */
-};
-
-static struct pci_driver piix_pci_driver = {
- .name = DRV_NAME,
- .id_table = piix_pci_tbl,
- .probe = piix_init_one,
- .remove = piix_remove_one,
-#ifdef CONFIG_PM
- .suspend = piix_pci_device_suspend,
- .resume = piix_pci_device_resume,
#endif
};

-static struct scsi_host_template piix_sht = {
- ATA_BMDMA_SHT(DRV_NAME),
-};
-
-static struct ata_port_operations piix_sata_ops = {
- .inherits = &ata_bmdma32_port_ops,
- .sff_irq_check = piix_irq_check,
-};
-
-static struct ata_port_operations piix_pata_ops = {
- .inherits = &piix_sata_ops,
- .cable_detect = ata_cable_40wire,
- .set_piomode = piix_set_piomode,
- .set_dmamode = piix_set_dmamode,
- .prereset = piix_pata_prereset,
-};
-
-static struct ata_port_operations piix_vmw_ops = {
- .inherits = &piix_pata_ops,
- .bmdma_status = piix_vmw_bmdma_status,
-};
-
-static struct ata_port_operations ich_pata_ops = {
- .inherits = &piix_pata_ops,
- .cable_detect = ich_pata_cable_detect,
- .set_dmamode = ich_set_dmamode,
-};
-
-static struct device_attribute *piix_sidpr_shost_attrs[] = {
- &dev_attr_link_power_management_policy,
- NULL
-};
-
-static struct scsi_host_template piix_sidpr_sht = {
- ATA_BMDMA_SHT(DRV_NAME),
- .shost_attrs = piix_sidpr_shost_attrs,
-};
-
-static struct ata_port_operations piix_sidpr_sata_ops = {
- .inherits = &piix_sata_ops,
- .hardreset = sata_std_hardreset,
- .scr_read = piix_sidpr_scr_read,
- .scr_write = piix_sidpr_scr_write,
- .set_lpm = piix_sidpr_set_lpm,
-};
-
-static struct ata_port_operations efar_pata_ops = {
- .inherits = &ata_bmdma_port_ops,
- .set_piomode = piix_set_piomode,
- .set_dmamode = piix_set_dmamode,
- .prereset = piix_pata_prereset,
- .cable_detect = efar_pata_cable_detect,
-};
-
-static struct ata_port_operations it8213_pata_ops = {
- .inherits = &ata_bmdma_port_ops,
- .set_piomode = piix_set_piomode,
- .set_dmamode = ich_set_dmamode,
- .prereset = piix_pata_prereset,
- .cable_detect = it8213_pata_cable_detect,
-};
-
-static struct ata_port_operations oldpiix_pata_ops = {
- .inherits = &ata_bmdma_port_ops,
- .qc_issue = oldpiix_qc_issue,
- .cable_detect = ata_cable_40wire,
- .set_piomode = piix_set_piomode,
- .set_dmamode = piix_set_dmamode,
- .prereset = piix_pata_prereset,
-};
-
-static struct ata_port_operations radisys_pata_ops = {
- .inherits = &ata_bmdma_port_ops,
- .qc_issue = oldpiix_qc_issue,
- .cable_detect = ata_cable_unknown,
- .set_piomode = piix_set_piomode,
- .set_dmamode = piix_set_dmamode,
-};
-
+#ifdef CONFIG_ATA_PIIX
static const struct piix_map_db ich5_map_db = {
.mask = 0x7,
.port_enable = 0x3,
@@ -553,169 +449,7 @@ static const struct piix_map_db *piix_ma
[ich8m_apple_sata] = &ich8m_apple_map_db,
[tolapai_sata] = &tolapai_map_db,
};
-
-static struct ata_port_info piix_port_info[] = {
- [piix_pata_mwdma] = /* PIIX3 MWDMA only */
- {
- .flags = PIIX_PATA_FLAGS,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA12_ONLY, /* mwdma1-2 ?? CHECK 0 should be ok but slow */
- .port_ops = &piix_pata_ops,
- },
-
- [piix_pata_33] = /* PIIX4 at 33MHz */
- {
- .flags = PIIX_PATA_FLAGS,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA12_ONLY, /* mwdma1-2 ?? CHECK 0 should be ok but slow */
- .udma_mask = ATA_UDMA2,
- .port_ops = &piix_pata_ops,
- },
-
- [ich_pata_33] = /* ICH0 - ICH at 33Mhz*/
- {
- .flags = PIIX_PATA_FLAGS,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA12_ONLY, /* Check: maybe MWDMA0 is ok */
- .udma_mask = ATA_UDMA2,
- .port_ops = &ich_pata_ops,
- },
-
- [ich_pata_66] = /* ICH controllers up to 66MHz */
- {
- .flags = PIIX_PATA_FLAGS,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA12_ONLY, /* MWDMA0 is broken on chip */
- .udma_mask = ATA_UDMA4,
- .port_ops = &ich_pata_ops,
- },
-
- [ich_pata_100] =
- {
- .flags = PIIX_PATA_FLAGS | PIIX_FLAG_CHECKINTR,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA12_ONLY,
- .udma_mask = ATA_UDMA5,
- .port_ops = &ich_pata_ops,
- },
-
- [ich_pata_100_nomwdma1] =
- {
- .flags = PIIX_PATA_FLAGS | PIIX_FLAG_CHECKINTR,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA2_ONLY,
- .udma_mask = ATA_UDMA5,
- .port_ops = &ich_pata_ops,
- },
-
- [ich5_sata] =
- {
- .flags = PIIX_SATA_FLAGS,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA2,
- .udma_mask = ATA_UDMA6,
- .port_ops = &piix_sata_ops,
- },
-
- [ich6_sata] =
- {
- .flags = PIIX_SATA_FLAGS,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA2,
- .udma_mask = ATA_UDMA6,
- .port_ops = &piix_sata_ops,
- },
-
- [ich6m_sata] =
- {
- .flags = PIIX_SATA_FLAGS,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA2,
- .udma_mask = ATA_UDMA6,
- .port_ops = &piix_sata_ops,
- },
-
- [ich8_sata] =
- {
- .flags = PIIX_SATA_FLAGS | PIIX_FLAG_SIDPR,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA2,
- .udma_mask = ATA_UDMA6,
- .port_ops = &piix_sata_ops,
- },
-
- [ich8_2port_sata] =
- {
- .flags = PIIX_SATA_FLAGS | PIIX_FLAG_SIDPR,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA2,
- .udma_mask = ATA_UDMA6,
- .port_ops = &piix_sata_ops,
- },
-
- [tolapai_sata] =
- {
- .flags = PIIX_SATA_FLAGS,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA2,
- .udma_mask = ATA_UDMA6,
- .port_ops = &piix_sata_ops,
- },
-
- [ich8m_apple_sata] =
- {
- .flags = PIIX_SATA_FLAGS,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA2,
- .udma_mask = ATA_UDMA6,
- .port_ops = &piix_sata_ops,
- },
-
- [piix_pata_vmw] =
- {
- .flags = PIIX_PATA_FLAGS,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA12_ONLY, /* mwdma1-2 ?? CHECK 0 should be ok but slow */
- .udma_mask = ATA_UDMA2,
- .port_ops = &piix_vmw_ops,
- },
-
- [efar_pata] =
- {
- .flags = PIIX_PATA_FLAGS,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA12_ONLY,
- .udma_mask = ATA_UDMA4,
- .port_ops = &efar_pata_ops,
- },
-
- [it8213_pata] =
- {
- .flags = PIIX_PATA_FLAGS,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA12_ONLY,
- .udma_mask = ATA_UDMA6,
- .port_ops = &it8213_pata_ops,
- },
-
- [oldpiix_pata] =
- {
- .flags = PIIX_PATA_FLAGS | PIIX_FLAG_NO_SITRE,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA12_ONLY,
- .port_ops = &oldpiix_pata_ops,
- },
-
- [radisys_pata] =
- {
- .flags = PIIX_PATA_FLAGS | PIIX_FLAG_NO_SITRE |
- PIIX_FLAG_RADISYS,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA12_ONLY,
- .udma_mask = ATA_UDMA24_ONLY,
- .port_ops = &radisys_pata_ops,
- },
-};
+#endif

static struct pci_bits piix_enable_bits[] = {
{ 0x41U, 1U, 0x80UL, 0x80UL }, /* port 0 */
@@ -728,6 +462,7 @@ MODULE_LICENSE("GPL");
MODULE_DEVICE_TABLE(pci, piix_pci_tbl);
MODULE_VERSION(DRV_VERSION);

+#if defined(CONFIG_ATA_PIIX) || defined(CONFIG_PATA_RDC)
struct ich_laptop {
u16 device;
u16 subvendor;
@@ -787,49 +522,12 @@ static int ich_pata_cable_detect(struct
}

/* check BIOS cable detect results */
- mask = ap->port_no == 0 ? PIIX_80C_PRI : PIIX_80C_SEC;
- if ((hpriv->saved_iocfg & mask) == 0)
- return ATA_CBL_PATA40;
- return ATA_CBL_PATA80;
-}
-
-/**
- * efar_pata_cable_detect - check for 40/80 pin
- * @ap: Port
- *
- * Perform cable detection for the EFAR ATA interface. This is
- * different to the PIIX arrangement
- */
-
-static int efar_pata_cable_detect(struct ata_port *ap)
-{
- struct pci_dev *pdev = to_pci_dev(ap->host->dev);
- u8 tmp;
-
- pci_read_config_byte(pdev, 0x47, &tmp);
- if (tmp & (2 >> ap->port_no))
- return ATA_CBL_PATA40;
- return ATA_CBL_PATA80;
-}
-
-/**
- * it8213_pata_cable_detect - check for 40/80 pin
- * @ap: Port
- *
- * Perform cable detection for the 8213 ATA interface. This is
- * different to the PIIX arrangement
- */
-
-static int it8213_pata_cable_detect(struct ata_port *ap)
-{
- struct pci_dev *pdev = to_pci_dev(ap->host->dev);
- u8 tmp;
-
- pci_read_config_byte(pdev, 0x42, &tmp);
- if (tmp & 2) /* The initial docs are incorrect */
+ mask = ap->port_no == 0 ? PIIX_80C_PRI : PIIX_80C_SEC;
+ if ((hpriv->saved_iocfg & mask) == 0)
return ATA_CBL_PATA40;
return ATA_CBL_PATA80;
}
+#endif

/**
* piix_pata_prereset - prereset for PATA host controller
@@ -839,7 +537,8 @@ static int it8213_pata_cable_detect(stru
* LOCKING:
* None (inherited from caller).
*/
-static int piix_pata_prereset(struct ata_link *link, unsigned long deadline)
+static int __maybe_unused piix_pata_prereset(struct ata_link *link,
+ unsigned long deadline)
{
struct ata_port *ap = link->ap;
struct pci_dev *pdev = to_pci_dev(ap->host->dev);
@@ -987,7 +686,8 @@ static void piix_set_timings(struct ata_
* None (inherited from caller).
*/

-static void piix_set_piomode(struct ata_port *ap, struct ata_device *adev)
+static void __maybe_unused piix_set_piomode(struct ata_port *ap,
+ struct ata_device *adev)
{
piix_set_timings(ap, adev, adev->pio_mode - XFER_PIO_0, 0);
}
@@ -1092,7 +792,8 @@ static void do_pata_set_dmamode(struct a
* None (inherited from caller).
*/

-static void piix_set_dmamode(struct ata_port *ap, struct ata_device *adev)
+static void __maybe_unused piix_set_dmamode(struct ata_port *ap,
+ struct ata_device *adev)
{
do_pata_set_dmamode(ap, adev, 0);
}
@@ -1108,39 +809,13 @@ static void piix_set_dmamode(struct ata_
* None (inherited from caller).
*/

-static void ich_set_dmamode(struct ata_port *ap, struct ata_device *adev)
+static void __maybe_unused ich_set_dmamode(struct ata_port *ap,
+ struct ata_device *adev)
{
do_pata_set_dmamode(ap, adev, 1);
}

-/**
- * oldpiix_qc_issue - command issue
- * @qc: command pending
- *
- * Called when the libata layer is about to issue a command. We wrap
- * this interface so that we can load the correct ATA timings if
- * necessary. Our logic also clears TIME0/TIME1 for the other device so
- * that, even if we get this wrong, cycles to the other device will
- * be made PIO0.
- */
-
-static unsigned int oldpiix_qc_issue(struct ata_queued_cmd *qc)
-{
- struct ata_port *ap = qc->ap;
- struct ata_device *adev = qc->dev;
-
- if (adev != ap->private_data) {
- /* UDMA timing is not shared */
- if (adev->dma_mode < XFER_UDMA_0) {
- if (adev->dma_mode)
- piix_set_dmamode(ap, adev);
- else if (adev->pio_mode)
- piix_set_piomode(ap, adev);
- }
- }
- return ata_bmdma_qc_issue(qc);
-}
-
+#ifdef CONFIG_ATA_PIIX
/*
* Serial ATA Index/Data Pair Superset Registers access
*
@@ -1732,8 +1407,253 @@ static bool piix_broken_system_poweroff(

return false;
}
+#else
+#define piix_pci_device_suspend ata_pci_device_suspend
+#define piix_pci_device_resume ata_pci_device_resume
+static inline bool piix_broken_system_poweroff(struct pci_dev *d) { return 0; }
+#endif
+
+static struct scsi_host_template piix_sht = {
+ ATA_BMDMA_SHT(DRV_NAME),
+};
+
+#ifdef CONFIG_ATA_PIIX
+static struct ata_port_operations piix_sata_ops = {
+ .inherits = &ata_bmdma32_port_ops,
+ .sff_irq_check = piix_irq_check,
+};
+
+static struct ata_port_operations piix_pata_ops = {
+ .inherits = &piix_sata_ops,
+ .cable_detect = ata_cable_40wire,
+ .set_piomode = piix_set_piomode,
+ .set_dmamode = piix_set_dmamode,
+ .prereset = piix_pata_prereset,
+};
+
+static struct ata_port_operations ich_pata_ops = {
+ .inherits = &piix_pata_ops,
+ .cable_detect = ich_pata_cable_detect,
+ .set_dmamode = ich_set_dmamode,
+};
+
+static struct ata_port_operations piix_vmw_ops = {
+ .inherits = &piix_pata_ops,
+ .bmdma_status = piix_vmw_bmdma_status,
+};
+
+static struct device_attribute *piix_sidpr_shost_attrs[] = {
+ &dev_attr_link_power_management_policy,
+ NULL
+};
+
+static struct scsi_host_template piix_sidpr_sht = {
+ ATA_BMDMA_SHT(DRV_NAME),
+ .shost_attrs = piix_sidpr_shost_attrs,
+};
+
+static struct ata_port_operations piix_sidpr_sata_ops = {
+ .inherits = &piix_sata_ops,
+ .hardreset = sata_std_hardreset,
+ .scr_read = piix_sidpr_scr_read,
+ .scr_write = piix_sidpr_scr_write,
+ .set_lpm = piix_sidpr_set_lpm,
+};
+#endif
+
+#include "ata_piix_efar.h"
+#include "ata_piix_it8213.h"
+#include "ata_piix_oldpiix.h" /* covers Radisys */
+#ifdef CONFIG_PATA_RDC
+static struct ata_port_operations rdc_pata_ops = {
+ .inherits = &ata_bmdma32_port_ops,
+ .cable_detect = ich_pata_cable_detect,
+ .set_piomode = piix_set_piomode,
+ .set_dmamode = ich_set_dmamode,
+ .prereset = piix_pata_prereset,
+};
+#endif
+
+static struct ata_port_info piix_port_info[] = {
+#ifdef CONFIG_ATA_PIIX
+ [piix_pata_mwdma] = /* PIIX3 MWDMA only */
+ {
+ .flags = PIIX_PATA_FLAGS,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA12_ONLY, /* mwdma1-2 ?? CHECK 0 should be ok but slow */
+ .port_ops = &piix_pata_ops,
+ },
+
+ [piix_pata_33] = /* PIIX4 at 33MHz */
+ {
+ .flags = PIIX_PATA_FLAGS,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA12_ONLY, /* mwdma1-2 ?? CHECK 0 should be ok but slow */
+ .udma_mask = ATA_UDMA2,
+ .port_ops = &piix_pata_ops,
+ },
+
+ [ich_pata_33] = /* ICH0 - ICH at 33Mhz*/
+ {
+ .flags = PIIX_PATA_FLAGS,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA12_ONLY, /* Check: maybe MWDMA0 is ok */
+ .udma_mask = ATA_UDMA2,
+ .port_ops = &ich_pata_ops,
+ },
+
+ [ich_pata_66] = /* ICH controllers up to 66MHz */
+ {
+ .flags = PIIX_PATA_FLAGS,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA12_ONLY, /* MWDMA0 is broken on chip */
+ .udma_mask = ATA_UDMA4,
+ .port_ops = &ich_pata_ops,
+ },
+
+ [ich_pata_100] =
+ {
+ .flags = PIIX_PATA_FLAGS | PIIX_FLAG_CHECKINTR,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA12_ONLY,
+ .udma_mask = ATA_UDMA5,
+ .port_ops = &ich_pata_ops,
+ },
+
+ [ich_pata_100_nomwdma1] =
+ {
+ .flags = PIIX_PATA_FLAGS | PIIX_FLAG_CHECKINTR,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA2_ONLY,
+ .udma_mask = ATA_UDMA5,
+ .port_ops = &ich_pata_ops,
+ },
+
+ [ich5_sata] =
+ {
+ .flags = PIIX_SATA_FLAGS,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA2,
+ .udma_mask = ATA_UDMA6,
+ .port_ops = &piix_sata_ops,
+ },
+
+ [ich6_sata] =
+ {
+ .flags = PIIX_SATA_FLAGS,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA2,
+ .udma_mask = ATA_UDMA6,
+ .port_ops = &piix_sata_ops,
+ },
+
+ [ich6m_sata] =
+ {
+ .flags = PIIX_SATA_FLAGS,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA2,
+ .udma_mask = ATA_UDMA6,
+ .port_ops = &piix_sata_ops,
+ },
+
+ [ich8_sata] =
+ {
+ .flags = PIIX_SATA_FLAGS | PIIX_FLAG_SIDPR,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA2,
+ .udma_mask = ATA_UDMA6,
+ .port_ops = &piix_sata_ops,
+ },
+
+ [ich8_2port_sata] =
+ {
+ .flags = PIIX_SATA_FLAGS | PIIX_FLAG_SIDPR,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA2,
+ .udma_mask = ATA_UDMA6,
+ .port_ops = &piix_sata_ops,
+ },
+
+ [tolapai_sata] =
+ {
+ .flags = PIIX_SATA_FLAGS,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA2,
+ .udma_mask = ATA_UDMA6,
+ .port_ops = &piix_sata_ops,
+ },
+
+ [ich8m_apple_sata] =
+ {
+ .flags = PIIX_SATA_FLAGS,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA2,
+ .udma_mask = ATA_UDMA6,
+ .port_ops = &piix_sata_ops,
+ },
+
+ [piix_pata_vmw] =
+ {
+ .flags = PIIX_PATA_FLAGS,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA12_ONLY, /* mwdma1-2 ?? CHECK 0 should be ok but slow */
+ .udma_mask = ATA_UDMA2,
+ .port_ops = &piix_vmw_ops,
+ },
+#endif
+#ifdef CONFIG_PATA_EFAR
+ [efar_pata] =
+ {
+ .flags = PIIX_PATA_FLAGS,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA12_ONLY,
+ .udma_mask = ATA_UDMA4,
+ .port_ops = &efar_pata_ops,
+ },
+#endif
+#ifdef CONFIG_PATA_IT8213
+ [it8213_pata] =
+ {
+ .flags = PIIX_PATA_FLAGS,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA12_ONLY,
+ .udma_mask = ATA_UDMA6,
+ .port_ops = &it8213_pata_ops,
+ },
+#endif
+#ifdef CONFIG_PATA_OLDPIIX
+ [oldpiix_pata] =
+ {
+ .flags = PIIX_PATA_FLAGS | PIIX_FLAG_NO_SITRE,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA12_ONLY,
+ .port_ops = &oldpiix_pata_ops,
+ },
+#endif
+#ifdef CONFIG_PATA_RADISYS
+ [radisys_pata] =
+ {
+ .flags = PIIX_PATA_FLAGS | PIIX_FLAG_NO_SITRE |
+ PIIX_FLAG_RADISYS,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA12_ONLY,
+ .udma_mask = ATA_UDMA24_ONLY,
+ .port_ops = &radisys_pata_ops,
+ },
+#endif
+#ifdef CONFIG_PATA_RDC
+ [rdc_pata] =
+ {
+ .flags = PIIX_PATA_FLAGS | PIIX_FLAG_CHECKINTR,
+ .pio_mask = ATA_PIO4,
+ .mwdma_mask = ATA_MWDMA12_ONLY,
+ .udma_mask = ATA_UDMA5,
+ .port_ops = &rdc_pata_ops,
+ },
+#endif
+};

-static int all_piixalikes; /* claim all PIIX-alike devices */
+static int all_piixalikes = 1; /* claim all PIIX-alike devices */

/**
* piix_init_one - Register PIIX ATA PCI device with kernel services
@@ -1810,7 +1730,7 @@ static int __devinit piix_init_one(struc
if (pdev->vendor == PCI_VENDOR_ID_INTEL ||
pdev->vendor == PCI_VENDOR_ID_RDC)
pci_read_config_dword(pdev, PIIX_IOCFG, &hpriv->saved_iocfg);
-
+#ifdef CONFIG_ATA_PIIX
/* ICH6R may be driven by either ata_piix or ahci driver
* regardless of BIOS configuration. Make sure AHCI mode is
* off.
@@ -1825,12 +1745,12 @@ static int __devinit piix_init_one(struc
if (port_flags & ATA_FLAG_SATA)
hpriv->map = piix_init_sata_map(pdev, port_info,
piix_map_db_table[idx]);
-
+#endif
rc = ata_pci_bmdma_prepare_host(pdev, ppi, &host);
if (rc)
return rc;
host->private_data = hpriv;
-
+#ifdef CONFIG_ATA_PIIX
/* initialize controller */
if (port_flags & ATA_FLAG_SATA) {
piix_init_pcs(host, piix_map_db_table[idx]);
@@ -1862,6 +1782,7 @@ static int __devinit piix_init_one(struc
host->ports[1]->mwdma_mask = 0;
host->ports[1]->udma_mask = 0;
}
+#endif
host->flags |= ATA_HOST_PARALLEL_SCAN;

pci_set_master(pdev);
@@ -1880,6 +1801,17 @@ static void piix_remove_one(struct pci_d
ata_pci_remove_one(pdev);
}

+static struct pci_driver piix_pci_driver = {
+ .name = DRV_NAME,
+ .id_table = piix_pci_tbl,
+ .probe = piix_init_one,
+ .remove = piix_remove_one,
+#ifdef CONFIG_PM
+ .suspend = piix_pci_device_suspend,
+ .resume = piix_pci_device_resume,
+#endif
+};
+
static int __init piix_init(void)
{
int rc;
Index: b/drivers/ata/ata_piix_efar.h
===================================================================
--- /dev/null
+++ b/drivers/ata/ata_piix_efar.h
@@ -0,0 +1,28 @@
+#ifdef CONFIG_PATA_EFAR
+/**
+ * efar_pata_cable_detect - check for 40/80 pin
+ * @ap: Port
+ *
+ * Perform cable detection for the EFAR ATA interface. This is
+ * different to the PIIX arrangement
+ */
+
+static int efar_pata_cable_detect(struct ata_port *ap)
+{
+ struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+ u8 tmp;
+
+ pci_read_config_byte(pdev, 0x47, &tmp);
+ if (tmp & (2 >> ap->port_no))
+ return ATA_CBL_PATA40;
+ return ATA_CBL_PATA80;
+}
+
+static struct ata_port_operations efar_pata_ops = {
+ .inherits = &ata_bmdma_port_ops,
+ .set_piomode = piix_set_piomode,
+ .set_dmamode = piix_set_dmamode,
+ .prereset = piix_pata_prereset,
+ .cable_detect = efar_pata_cable_detect,
+};
+#endif
Index: b/drivers/ata/ata_piix_it8213.h
===================================================================
--- /dev/null
+++ b/drivers/ata/ata_piix_it8213.h
@@ -0,0 +1,28 @@
+#ifdef CONFIG_PATA_IT8213
+/**
+ * it8213_pata_cable_detect - check for 40/80 pin
+ * @ap: Port
+ *
+ * Perform cable detection for the 8213 ATA interface. This is
+ * different to the PIIX arrangement
+ */
+
+static int it8213_pata_cable_detect(struct ata_port *ap)
+{
+ struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+ u8 tmp;
+
+ pci_read_config_byte(pdev, 0x42, &tmp);
+ if (tmp & 2) /* The initial docs are incorrect */
+ return ATA_CBL_PATA40;
+ return ATA_CBL_PATA80;
+}
+
+static struct ata_port_operations it8213_pata_ops = {
+ .inherits = &ata_bmdma_port_ops,
+ .set_piomode = piix_set_piomode,
+ .set_dmamode = ich_set_dmamode,
+ .prereset = piix_pata_prereset,
+ .cable_detect = it8213_pata_cable_detect,
+};
+#endif
Index: b/drivers/ata/ata_piix_oldpiix.h
===================================================================
--- /dev/null
+++ b/drivers/ata/ata_piix_oldpiix.h
@@ -0,0 +1,49 @@
+#if defined(CONFIG_PATA_OLDPIIX) || defined(CONFIG_PATA_RADISYS)
+/**
+ * oldpiix_qc_issue - command issue
+ * @qc: command pending
+ *
+ * Called when the libata layer is about to issue a command. We wrap
+ * this interface so that we can load the correct ATA timings if
+ * necessary. Our logic also clears TIME0/TIME1 for the other device so
+ * that, even if we get this wrong, cycles to the other device will
+ * be made PIO0.
+ */
+
+static unsigned int oldpiix_qc_issue(struct ata_queued_cmd *qc)
+{
+ struct ata_port *ap = qc->ap;
+ struct ata_device *adev = qc->dev;
+
+ if (adev != ap->private_data) {
+ /* UDMA timing is not shared */
+ if (adev->dma_mode < XFER_UDMA_0) {
+ if (adev->dma_mode)
+ piix_set_dmamode(ap, adev);
+ else if (adev->pio_mode)
+ piix_set_piomode(ap, adev);
+ }
+ }
+ return ata_bmdma_qc_issue(qc);
+}
+#endif
+
+#ifdef CONFIG_PATA_OLDPIIX
+static struct ata_port_operations oldpiix_pata_ops = {
+ .inherits = &ata_bmdma_port_ops,
+ .qc_issue = oldpiix_qc_issue,
+ .cable_detect = ata_cable_40wire,
+ .set_piomode = piix_set_piomode,
+ .set_dmamode = piix_set_dmamode,
+ .prereset = piix_pata_prereset,
+};
+#endif
+#ifdef CONFIG_PATA_RADISYS
+static struct ata_port_operations radisys_pata_ops = {
+ .inherits = &ata_bmdma_port_ops,
+ .qc_issue = oldpiix_qc_issue,
+ .cable_detect = ata_cable_unknown,
+ .set_piomode = piix_set_piomode,
+ .set_dmamode = piix_set_dmamode,
+};
+#endif

Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

Alan Cox wrote:

> > on the other side of equation we have 5 drivers less, 1400 LOCs less,
> > no risk of losing bugfixes and features (which is true already w/
> > pata_rdc lacking locking fixes + Power Management and pata_oldpiix
> > lacking parallel scanning feature).
>
> The parallel scanning feature is somewhat irrelevant on an original PIIX,
> if the hardware even supports it which is somewhat of an unknown. If you
> have a minor boot time performance concern you'd buy a real computer
> instead.
>
> And the other stuff you actually create a problem rather than solving one
> - the difficulting testing the rare chips makes it harder to fix ata_piix
> bugs and do full testing.

We don't have to insist on merging all PIIX-alikes (like oldPIIX and Radisys)
support to ata_piix, i.e. for RDC it looks much cleaner:

per patch from the original patch series:
4 files changed, 13 insertions(+), 387 deletions(-)

pata_rdc driver is nearly identical to ata_piix PATA support (modulo bugs in
pata_rdc and SATA stuff in ata_piix)


From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] ata_piix: add RDC support

Add RDC PATA support to ata_piix and remove no longer
needed pata_rdc driver.

Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
drivers/ata/Kconfig | 1
drivers/ata/Makefile | 1
drivers/ata/ata_piix.c | 14 +
drivers/ata/pata_rdc.c | 384 -------------------------------------------------
4 files changed, 13 insertions(+), 387 deletions(-)

Index: b/drivers/ata/Kconfig
===================================================================
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -567,6 +567,7 @@ config PATA_RADISYS
config PATA_RDC
tristate "RDC PATA support"
depends on PCI
+ select ATA_PIIX
help
This option enables basic support for the later RDC PATA controllers
controllers via the new ATA layer. For the RDC 1010, you need to
Index: b/drivers/ata/Makefile
===================================================================
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -58,7 +58,6 @@ obj-$(CONFIG_PATA_OPTIDMA) += pata_optid
obj-$(CONFIG_PATA_PDC2027X) += pata_pdc2027x.o
obj-$(CONFIG_PATA_PDC_OLD) += pata_pdc202xx_old.o
obj-$(CONFIG_PATA_RADISYS) += pata_radisys.o
-obj-$(CONFIG_PATA_RDC) += pata_rdc.o
obj-$(CONFIG_PATA_SC1200) += pata_sc1200.o
obj-$(CONFIG_PATA_SCC) += pata_scc.o
obj-$(CONFIG_PATA_SCH) += pata_sch.o
Index: b/drivers/ata/ata_piix.c
===================================================================
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -1,6 +1,9 @@
/*
* ata_piix.c - Intel PATA/SATA controllers
*
+ * This is actually a driver for hardware meeting
+ * INCITS 370-2004 (1510D): ATA Host Adapter Standards
+ *
* Maintained by: Jeff Garzik <[email protected]>
* Please ALWAYS copy [email protected]
* on emails.
@@ -255,6 +258,10 @@ static const struct pci_device_id piix_p
/* IT8213 */
{ 0x1283, 0x8213, PCI_ANY_ID, PCI_ANY_ID, 0, 0, it8213_pata },

+ /* RDC is ICH like */
+ { 0x17F3, 0x1011, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
+ { 0x17F3, 0x1012, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
+
/* SATA ports */

/* 82801EB (ICH5) */
@@ -671,6 +678,7 @@ MODULE_DEVICE_TABLE(pci, piix_pci_tbl);
MODULE_VERSION(DRV_VERSION);
MODULE_ALIAS("pata_efar");
MODULE_ALIAS("pata_it8213");
+MODULE_ALIAS("pata_rdc");

struct ich_laptop {
u16 device;
@@ -1676,7 +1684,8 @@ static int __devinit piix_init_one(struc
* because some ACPI implementations mess up cable related
* bits on _STM. Reported on kernel bz#11879.
*/
- if (pdev->vendor == PCI_VENDOR_ID_INTEL)
+ if (pdev->vendor == PCI_VENDOR_ID_INTEL ||
+ pdev->vendor == PCI_VENDOR_ID_RDC)
pci_read_config_dword(pdev, PIIX_IOCFG, &hpriv->saved_iocfg);

/* ICH6R may be driven by either ata_piix or ahci driver
@@ -1741,7 +1750,8 @@ static void piix_remove_one(struct pci_d
struct ata_host *host = dev_get_drvdata(&pdev->dev);
struct piix_host_priv *hpriv = host->private_data;

- if (pdev->vendor == PCI_VENDOR_ID_INTEL)
+ if (pdev->vendor == PCI_VENDOR_ID_INTEL ||
+ pdev->vendor == PCI_VENDOR_ID_RDC)
pci_write_config_dword(pdev, PIIX_IOCFG, hpriv->saved_iocfg);

ata_pci_remove_one(pdev);
Index: b/drivers/ata/pata_rdc.c
===================================================================
--- a/drivers/ata/pata_rdc.c
+++ /dev/null
@@ -1,384 +0,0 @@
-/*
- * pata_rdc - Driver for later RDC PATA controllers
- *
- * This is actually a driver for hardware meeting
- * INCITS 370-2004 (1510D): ATA Host Adapter Standards
- *
- * Based on ata_piix.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2, or (at your option)
- * any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; see the file COPYING. If not, write to
- * the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA.
- */
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/pci.h>
-#include <linux/init.h>
-#include <linux/blkdev.h>
-#include <linux/delay.h>
-#include <linux/device.h>
-#include <linux/gfp.h>
-#include <scsi/scsi_host.h>
-#include <linux/libata.h>
-#include <linux/dmi.h>
-
-#define DRV_NAME "pata_rdc"
-#define DRV_VERSION "0.01"
-
-struct rdc_host_priv {
- u32 saved_iocfg;
-};
-
-/**
- * rdc_pata_cable_detect - Probe host controller cable detect info
- * @ap: Port for which cable detect info is desired
- *
- * Read 80c cable indicator from ATA PCI device's PCI config
- * register. This register is normally set by firmware (BIOS).
- *
- * LOCKING:
- * None (inherited from caller).
- */
-
-static int rdc_pata_cable_detect(struct ata_port *ap)
-{
- struct rdc_host_priv *hpriv = ap->host->private_data;
- u8 mask;
-
- /* check BIOS cable detect results */
- mask = 0x30 << (2 * ap->port_no);
- if ((hpriv->saved_iocfg & mask) == 0)
- return ATA_CBL_PATA40;
- return ATA_CBL_PATA80;
-}
-
-/**
- * rdc_pata_prereset - prereset for PATA host controller
- * @link: Target link
- * @deadline: deadline jiffies for the operation
- *
- * LOCKING:
- * None (inherited from caller).
- */
-static int rdc_pata_prereset(struct ata_link *link, unsigned long deadline)
-{
- struct ata_port *ap = link->ap;
- struct pci_dev *pdev = to_pci_dev(ap->host->dev);
-
- static const struct pci_bits rdc_enable_bits[] = {
- { 0x41U, 1U, 0x80UL, 0x80UL }, /* port 0 */
- { 0x43U, 1U, 0x80UL, 0x80UL }, /* port 1 */
- };
-
- if (!pci_test_config_bits(pdev, &rdc_enable_bits[ap->port_no]))
- return -ENOENT;
- return ata_sff_prereset(link, deadline);
-}
-
-static DEFINE_SPINLOCK(rdc_lock);
-
-static void rdc_set_timings(struct ata_port *ap, struct ata_device *adev,
- u8 pio, bool use_mwdma)
-{
- struct pci_dev *dev = to_pci_dev(ap->host->dev);
- unsigned long flags;
- unsigned int is_slave = (adev->devno != 0);
- unsigned int master_port= ap->port_no ? 0x42 : 0x40;
- unsigned int slave_port = 0x44;
- u16 master_data;
- u8 slave_data;
- u8 udma_enable;
- int control = 0;
-
- static const /* ISP RTC */
- u8 timings[][2] = { { 0, 0 },
- { 0, 0 },
- { 1, 0 },
- { 2, 1 },
- { 2, 3 }, };
-
- if (pio >= 2 || use_mwdma)
- control |= 1; /* TIME1 enable */
- if (ata_pio_need_iordy(adev) || use_mwdma)
- control |= 2; /* IE enable */
- if (adev->class == ATA_DEV_ATA)
- control |= 4; /* PPE enable */
- /* If the drive MWDMA is faster than it can do PIO then
- we must force PIO into PIO0 */
- if (use_mwdma && adev->pio_mode < (XFER_PIO_0 + pio))
- /* Enable DMA timing only */
- control |= 8; /* PIO cycles in PIO0 */
-
- spin_lock_irqsave(&rdc_lock, flags);
-
- /* PIO configuration clears DTE unconditionally. It will be
- * programmed in set_dmamode which is guaranteed to be called
- * after set_piomode if any DMA mode is available.
- */
- pci_read_config_word(dev, master_port, &master_data);
- if (is_slave) {
- /* clear TIME1|IE1|PPE1|DTE1 */
- master_data &= 0xff0f;
- /* Enable SITRE (separate slave timing register) */
- master_data |= 0x4000;
- /* enable PPE1, IE1 and TIME1 as needed */
- master_data |= (control << 4);
- pci_read_config_byte(dev, slave_port, &slave_data);
- slave_data &= (ap->port_no ? 0x0f : 0xf0);
- /* Load the timing nibble for this slave */
- slave_data |= ((timings[pio][0] << 2) | timings[pio][1])
- << (ap->port_no ? 4 : 0);
- } else {
- /* clear ISP|RCT|TIME0|IE0|PPE0|DTE0 */
- master_data &= 0xccf0;
- /* Enable PPE, IE and TIME as appropriate */
- master_data |= control;
- /* load ISP and RCT */
- master_data |=
- (timings[pio][0] << 12) |
- (timings[pio][1] << 8);
- }
- pci_write_config_word(dev, master_port, master_data);
- if (is_slave)
- pci_write_config_byte(dev, slave_port, slave_data);
-
- /* Ensure the UDMA bit is off - it will be turned back on if
- UDMA is selected */
-
- pci_read_config_byte(dev, 0x48, &udma_enable);
- udma_enable &= ~(1 << (2 * ap->port_no + adev->devno));
- pci_write_config_byte(dev, 0x48, udma_enable);
-
- spin_unlock_irqrestore(&rdc_lock, flags);
-}
-
-/**
- * rdc_set_piomode - Initialize host controller PATA PIO timings
- * @ap: Port whose timings we are configuring
- * @adev: Drive in question
- *
- * Set PIO mode for device, in host controller PCI config space.
- *
- * LOCKING:
- * None (inherited from caller).
- */
-
-static void rdc_set_piomode(struct ata_port *ap, struct ata_device *adev)
-{
- rdc_set_timings(ap, adev, adev->pio_mode - XFER_PIO_0, 0);
-}
-
-/**
- * rdc_set_dmamode - Initialize host controller PATA PIO timings
- * @ap: Port whose timings we are configuring
- * @adev: Drive in question
- *
- * Set UDMA mode for device, in host controller PCI config space.
- *
- * LOCKING:
- * None (inherited from caller).
- */
-
-static void rdc_set_dmamode(struct ata_port *ap, struct ata_device *adev)
-{
- struct pci_dev *dev = to_pci_dev(ap->host->dev);
- unsigned long flags;
- u8 speed = adev->dma_mode;
- int devid = adev->devno + 2 * ap->port_no;
- u8 udma_enable = 0;
-
- if (speed >= XFER_UDMA_0) {
- unsigned int udma = speed - XFER_UDMA_0;
- u16 udma_timing;
- u16 ideconf;
- int u_clock, u_speed;
-
- spin_lock_irqsave(&rdc_lock, flags);
-
- pci_read_config_byte(dev, 0x48, &udma_enable);
-
- /*
- * UDMA is handled by a combination of clock switching and
- * selection of dividers
- *
- * Handy rule: Odd modes are UDMATIMx 01, even are 02
- * except UDMA0 which is 00
- */
- u_speed = min(2 - (udma & 1), udma);
- if (udma == 5)
- u_clock = 0x1000; /* 100Mhz */
- else if (udma > 2)
- u_clock = 1; /* 66Mhz */
- else
- u_clock = 0; /* 33Mhz */
-
- udma_enable |= (1 << devid);
-
- /* Load the CT/RP selection */
- pci_read_config_word(dev, 0x4A, &udma_timing);
- udma_timing &= ~(3 << (4 * devid));
- udma_timing |= u_speed << (4 * devid);
- pci_write_config_word(dev, 0x4A, udma_timing);
-
- /* Select a 33/66/100Mhz clock */
- pci_read_config_word(dev, 0x54, &ideconf);
- ideconf &= ~(0x1001 << devid);
- ideconf |= u_clock << devid;
- pci_write_config_word(dev, 0x54, ideconf);
-
- pci_write_config_byte(dev, 0x48, udma_enable);
-
- spin_unlock_irqrestore(&rdc_lock, flags);
- } else {
- /* MWDMA is driven by the PIO timings. */
- unsigned int mwdma = speed - XFER_MW_DMA_0;
- const unsigned int needed_pio[3] = {
- XFER_PIO_0, XFER_PIO_3, XFER_PIO_4
- };
- int pio = needed_pio[mwdma] - XFER_PIO_0;
-
- rdc_set_timings(ap, adev, pio, 1);
- }
-}
-
-static struct ata_port_operations rdc_pata_ops = {
- .inherits = &ata_bmdma32_port_ops,
- .cable_detect = rdc_pata_cable_detect,
- .set_piomode = rdc_set_piomode,
- .set_dmamode = rdc_set_dmamode,
- .prereset = rdc_pata_prereset,
-};
-
-static struct ata_port_info rdc_port_info = {
-
- .flags = ATA_FLAG_SLAVE_POSS,
- .pio_mask = ATA_PIO4,
- .mwdma_mask = ATA_MWDMA12_ONLY,
- .udma_mask = ATA_UDMA5,
- .port_ops = &rdc_pata_ops,
-};
-
-static struct scsi_host_template rdc_sht = {
- ATA_BMDMA_SHT(DRV_NAME),
-};
-
-/**
- * rdc_init_one - Register PIIX ATA PCI device with kernel services
- * @pdev: PCI device to register
- * @ent: Entry in rdc_pci_tbl matching with @pdev
- *
- * Called from kernel PCI layer. We probe for combined mode (sigh),
- * and then hand over control to libata, for it to do the rest.
- *
- * LOCKING:
- * Inherited from PCI layer (may sleep).
- *
- * RETURNS:
- * Zero on success, or -ERRNO value.
- */
-
-static int __devinit rdc_init_one(struct pci_dev *pdev,
- const struct pci_device_id *ent)
-{
- static int printed_version;
- struct device *dev = &pdev->dev;
- struct ata_port_info port_info[2];
- const struct ata_port_info *ppi[] = { &port_info[0], &port_info[1] };
- unsigned long port_flags;
- struct ata_host *host;
- struct rdc_host_priv *hpriv;
- int rc;
-
- if (!printed_version++)
- dev_printk(KERN_DEBUG, &pdev->dev,
- "version " DRV_VERSION "\n");
-
- port_info[0] = rdc_port_info;
- port_info[1] = rdc_port_info;
-
- port_flags = port_info[0].flags;
-
- /* enable device and prepare host */
- rc = pcim_enable_device(pdev);
- if (rc)
- return rc;
-
- hpriv = devm_kzalloc(dev, sizeof(*hpriv), GFP_KERNEL);
- if (!hpriv)
- return -ENOMEM;
-
- /* Save IOCFG, this will be used for cable detection, quirk
- * detection and restoration on detach.
- */
- pci_read_config_dword(pdev, 0x54, &hpriv->saved_iocfg);
-
- rc = ata_pci_bmdma_prepare_host(pdev, ppi, &host);
- if (rc)
- return rc;
- host->private_data = hpriv;
-
- pci_intx(pdev, 1);
-
- host->flags |= ATA_HOST_PARALLEL_SCAN;
-
- pci_set_master(pdev);
- return ata_pci_sff_activate_host(host, ata_bmdma_interrupt, &rdc_sht);
-}
-
-static void rdc_remove_one(struct pci_dev *pdev)
-{
- struct ata_host *host = dev_get_drvdata(&pdev->dev);
- struct rdc_host_priv *hpriv = host->private_data;
-
- pci_write_config_dword(pdev, 0x54, hpriv->saved_iocfg);
-
- ata_pci_remove_one(pdev);
-}
-
-static const struct pci_device_id rdc_pci_tbl[] = {
- { PCI_DEVICE(0x17F3, 0x1011), },
- { PCI_DEVICE(0x17F3, 0x1012), },
- { } /* terminate list */
-};
-
-static struct pci_driver rdc_pci_driver = {
- .name = DRV_NAME,
- .id_table = rdc_pci_tbl,
- .probe = rdc_init_one,
- .remove = rdc_remove_one,
-#ifdef CONFIG_PM
- .suspend = ata_pci_device_suspend,
- .resume = ata_pci_device_resume,
-#endif
-};
-
-
-static int __init rdc_init(void)
-{
- return pci_register_driver(&rdc_pci_driver);
-}
-
-static void __exit rdc_exit(void)
-{
- pci_unregister_driver(&rdc_pci_driver);
-}
-
-module_init(rdc_init);
-module_exit(rdc_exit);
-
-MODULE_AUTHOR("Alan Cox (based on ata_piix)");
-MODULE_DESCRIPTION("SCSI low-level driver for RDC PATA controllers");
-MODULE_LICENSE("GPL");
-MODULE_DEVICE_TABLE(pci, rdc_pci_tbl);
-MODULE_VERSION(DRV_VERSION);

2011-02-23 10:34:25

by Alan

[permalink] [raw]
Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

> From: Bartlomiej Zolnierkiewicz <[email protected]>
> Subject: [PATCH] ata_piix: add RDC support
>
> Add RDC PATA support to ata_piix and remove no longer
> needed pata_rdc driver.

Now that one does make sense given the similarity. I'll go over both
later today and see if I agree they match this way.

2011-02-24 17:51:20

by Alan

[permalink] [raw]
Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data

> + /* RDC is ICH like */
> + { 0x17F3, 0x1011, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
> + { 0x17F3, 0x1012, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
> +

Cable detect is now a bit iffy - probably should add the device vendor to
the table for each if you do this.

Other than that it this specific one could be pretty sensible - but it
does makes the drivers bigger especially on an embedded RDC device.