libata: Cavium OCTEON SOC Compact Flash driver (v3)

PATCH NAK David Daney 2 participants 10 messages Dec 5, 2008 – Dec 8, 2008

Subsystems (2)

MIPS libata

lore

Share & tools

lore

David Daney

author

2008-12-05 23:06 UTC #0 lore raw

cover libata: Cavium OCTEON SOC Compact Flash driver (v3)

As part of our efforts to get the Cavium OCTEON processor support merged (see: http://marc.info/?l=linux-mips&m=122834773330212), we have this CF driver for your consideration.

This version is split into two separate patches. The first adds some entries to the ata_timing table in libata-core. The second is the driver proper.

I will reply with the two patches.

Thanks,

David Daney (2):
libata: Add two more columns to the ata_timing table.
libata: New driver for OCTEON SOC Compact Flash interface (v3).

 drivers/ata/Kconfig          |    9 +
 drivers/ata/Makefile         |    1 +
 drivers/ata/libata-core.c    |   74 ++--
 drivers/ata/pata_octeon_cf.c |  956 
++++++++++++++++++++++++++++++++++++++++++
 include/linux/libata.h       |   12 +-
 5 files changed, 1014 insertions(+), 38 deletions(-)
 create mode 100644 drivers/ata/pata_octeon_cf.c

David Daney

author

2008-12-05 23:10 UTC #1 lore raw

PATCH 1/2 libata: Add two more columns to the ata_timing table.

The forthcoming OCTEON SOC Compact Flash driver needs a few more timing values than were available in the ata_timing table. I add new columns for write_hold and read_holdz times. The values were obtained from the Compact Flash specification Rev 4.1.

Signed-off-by David Daney <[email protected]>

---
 drivers/ata/libata-core.c |   74 +++++++++++++++++++++++---------------------
 include/linux/libata.h    |   12 +++++--
 2 files changed, 48 insertions(+), 38 deletions(-)

drivers/ata/libata-core.c +39 ~~-35~~

index 5e2eb74..df6403a 100644
@@ -2953,33 +2953,33 @@ int sata_set_spd(struct ata_link *link)
  */
 
 static const struct ata_timing ata_timing[] = {
-/*	{ XFER_PIO_SLOW, 120, 290, 240, 960, 290, 240, 960,   0 }, */
-	{ XFER_PIO_0,     70, 290, 240, 600, 165, 150, 600,   0 },
-	{ XFER_PIO_1,     50, 290,  93, 383, 125, 100, 383,   0 },
-	{ XFER_PIO_2,     30, 290,  40, 330, 100,  90, 240,   0 },
-	{ XFER_PIO_3,     30,  80,  70, 180,  80,  70, 180,   0 },
-	{ XFER_PIO_4,     25,  70,  25, 120,  70,  25, 120,   0 },
-	{ XFER_PIO_5,     15,  65,  25, 100,  65,  25, 100,   0 },
-	{ XFER_PIO_6,     10,  55,  20,  80,  55,  20,  80,   0 },
-
-	{ XFER_SW_DMA_0, 120,   0,   0,   0, 480, 480, 960,   0 },
-	{ XFER_SW_DMA_1,  90,   0,   0,   0, 240, 240, 480,   0 },
-	{ XFER_SW_DMA_2,  60,   0,   0,   0, 120, 120, 240,   0 },
-
-	{ XFER_MW_DMA_0,  60,   0,   0,   0, 215, 215, 480,   0 },
-	{ XFER_MW_DMA_1,  45,   0,   0,   0,  80,  50, 150,   0 },
-	{ XFER_MW_DMA_2,  25,   0,   0,   0,  70,  25, 120,   0 },
-	{ XFER_MW_DMA_3,  25,   0,   0,   0,  65,  25, 100,   0 },
-	{ XFER_MW_DMA_4,  25,   0,   0,   0,  55,  20,  80,   0 },
-
-/*	{ XFER_UDMA_SLOW,  0,   0,   0,   0,   0,   0,   0, 150 }, */
-	{ XFER_UDMA_0,     0,   0,   0,   0,   0,   0,   0, 120 },
-	{ XFER_UDMA_1,     0,   0,   0,   0,   0,   0,   0,  80 },
-	{ XFER_UDMA_2,     0,   0,   0,   0,   0,   0,   0,  60 },
-	{ XFER_UDMA_3,     0,   0,   0,   0,   0,   0,   0,  45 },
-	{ XFER_UDMA_4,     0,   0,   0,   0,   0,   0,   0,  30 },
-	{ XFER_UDMA_5,     0,   0,   0,   0,   0,   0,   0,  20 },
-	{ XFER_UDMA_6,     0,   0,   0,   0,   0,   0,   0,  15 },
+/*	{ XFER_PIO_SLOW, 120, 290, 240, 960, 290, 240, 30, 30, 960,   0 }, */
+	{ XFER_PIO_0,     70, 290, 240, 600, 165, 150, 30, 30, 600,   0 },
+	{ XFER_PIO_1,     50, 290,  93, 383, 125, 100, 20, 30, 383,   0 },
+	{ XFER_PIO_2,     30, 290,  40, 330, 100,  90, 15, 30, 240,   0 },
+	{ XFER_PIO_3,     30,  80,  70, 180,  80,  70, 10, 30, 180,   0 },
+	{ XFER_PIO_4,     25,  70,  25, 120,  70,  25, 10, 30, 120,   0 },
+	{ XFER_PIO_5,     15,  65,  25, 100,  65,  25,  5, 20, 100,   0 },
+	{ XFER_PIO_6,     10,  55,  20,  80,  55,  20,  5, 20,  80,   0 },
+
+	{ XFER_SW_DMA_0, 120,   0,   0,   0, 480, 480, 30, 0,  960,   0 },
+	{ XFER_SW_DMA_1,  90,   0,   0,   0, 240, 240, 30, 0,  480,   0 },
+	{ XFER_SW_DMA_2,  60,   0,   0,   0, 120, 120, 15, 0,  240,   0 },
+
+	{ XFER_MW_DMA_0,  60,   0,   0,   0, 215, 215, 20, 0,  480,   0 },
+	{ XFER_MW_DMA_1,  45,   0,   0,   0,  80,  50, 15, 0,  150,   0 },
+	{ XFER_MW_DMA_2,  25,   0,   0,   0,  70,  25, 10, 0,  120,   0 },
+	{ XFER_MW_DMA_3,  25,   0,   0,   0,  65,  25,  5, 0,  100,   0 },
+	{ XFER_MW_DMA_4,  25,   0,   0,   0,  55,  20,  5, 0,   80,   0 },
+
+/*	{ XFER_UDMA_SLOW,  0,   0,   0,   0,   0,   0,  0, 0,    0, 150 }, */
+	{ XFER_UDMA_0,     0,   0,   0,   0,   0,   0,  0, 0,    0, 120 },
+	{ XFER_UDMA_1,     0,   0,   0,   0,   0,   0,  0, 0,    0,  80 },
+	{ XFER_UDMA_2,     0,   0,   0,   0,   0,   0,  0, 0,    0,  60 },
+	{ XFER_UDMA_3,     0,   0,   0,   0,   0,   0,  0, 0,    0,  45 },
+	{ XFER_UDMA_4,     0,   0,   0,   0,   0,   0,  0, 0,    0,  30 },
+	{ XFER_UDMA_5,     0,   0,   0,   0,   0,   0,  0, 0,    0,  20 },
+	{ XFER_UDMA_6,     0,   0,   0,   0,   0,   0,  0, 0,    0,  15 },
 
 	{ 0xFF }
 };

@@ -2989,14 +2989,16 @@ static const struct ata_timing ata_timing[] = {
 
 static void ata_timing_quantize(const struct ata_timing *t, struct ata_timing *q, int T, int UT)
 {
-	q->setup   = EZ(t->setup   * 1000,  T);
-	q->act8b   = EZ(t->act8b   * 1000,  T);
-	q->rec8b   = EZ(t->rec8b   * 1000,  T);
-	q->cyc8b   = EZ(t->cyc8b   * 1000,  T);
-	q->active  = EZ(t->active  * 1000,  T);
-	q->recover = EZ(t->recover * 1000,  T);
-	q->cycle   = EZ(t->cycle   * 1000,  T);
-	q->udma    = EZ(t->udma    * 1000, UT);
+	q->setup	= EZ(t->setup      * 1000,  T);
+	q->act8b	= EZ(t->act8b      * 1000,  T);
+	q->rec8b	= EZ(t->rec8b      * 1000,  T);
+	q->cyc8b	= EZ(t->cyc8b      * 1000,  T);
+	q->active	= EZ(t->active     * 1000,  T);
+	q->recover	= EZ(t->recover    * 1000,  T);
+	q->write_hold	= EZ(t->write_hold * 1000,  T);
+	q->read_holdz	= EZ(t->read_holdz * 1000,  T);
+	q->cycle	= EZ(t->cycle      * 1000,  T);
+	q->udma		= EZ(t->udma       * 1000, UT);
 }
 
 void ata_timing_merge(const struct ata_timing *a, const struct ata_timing *b,

@@ -3008,6 +3010,8 @@ void ata_timing_merge(const struct ata_timing *a, const struct ata_timing *b,
 	if (what & ATA_TIMING_CYC8B  ) m->cyc8b   = max(a->cyc8b,   b->cyc8b);
 	if (what & ATA_TIMING_ACTIVE ) m->active  = max(a->active,  b->active);
 	if (what & ATA_TIMING_RECOVER) m->recover = max(a->recover, b->recover);
+	if (what & ATA_TIMING_WRITE_HOLD) m->write_hold = max(a->write_hold, b->write_hold);
+	if (what & ATA_TIMING_READ_HOLDZ) m->read_holdz = max(a->read_holdz, b->read_holdz);
 	if (what & ATA_TIMING_CYCLE  ) m->cycle   = max(a->cycle,   b->cycle);
 	if (what & ATA_TIMING_UDMA   ) m->udma    = max(a->udma,    b->udma);
 }

include/linux/libata.h +9 -3

index ed3f26e..95fa9f6 100644
@@ -399,12 +399,16 @@ enum {
 				  ATA_TIMING_CYC8B,
 	ATA_TIMING_ACTIVE	= (1 << 4),
 	ATA_TIMING_RECOVER	= (1 << 5),
-	ATA_TIMING_CYCLE	= (1 << 6),
-	ATA_TIMING_UDMA		= (1 << 7),
+	ATA_TIMING_WRITE_HOLD	= (1 << 6),
+	ATA_TIMING_READ_HOLDZ	= (1 << 7),
+	ATA_TIMING_CYCLE	= (1 << 8),
+	ATA_TIMING_UDMA		= (1 << 9),
 	ATA_TIMING_ALL		= ATA_TIMING_SETUP | ATA_TIMING_ACT8B |
 				  ATA_TIMING_REC8B | ATA_TIMING_CYC8B |
 				  ATA_TIMING_ACTIVE | ATA_TIMING_RECOVER |
-				  ATA_TIMING_CYCLE | ATA_TIMING_UDMA,
+				  ATA_TIMING_WRITE_HOLD |
+				  ATA_TIMING_READ_HOLDZ | ATA_TIMING_CYCLE |
+				  ATA_TIMING_UDMA,
 };
 
 enum ata_xfer_mask {

@@ -864,6 +868,8 @@ struct ata_timing {
 	unsigned short cyc8b;		/* t0 for 8-bit I/O */
 	unsigned short active;		/* t2 or tD */
 	unsigned short recover;		/* t2i or tK */
+	unsigned short write_hold;	/* t4 */
+	unsigned short read_holdz;	/* t6z */
 	unsigned short cycle;		/* t0 */
 	unsigned short udma;		/* t2CYCTYP/2 */
 };

Signature · 1 line

1.5.6.5

Sergei Shtylyov

NAK

2008-12-06 20:19 UTC #2 lore raw

Re: [PATCH 1/2] libata: Add two more columns to the ata_timing table.

Hello.

Quoted 6 lines · David Daney

The forthcoming OCTEON SOC Compact Flash driver needs a few more timing values than were available in the ata_timing table. I add new columns for write_hold and read_holdz times. The values were obtained from the Compact Flash specification Rev 4.1.
Signed-off-by David Daney <[email protected]>

NAK.

Quoted 4 lines

include/linux/libata.h +0 -0
index ed3f26e..95fa9f6 100644

[...]

include/linux/libata.h struct ata_timing { · line 872

Quoted 7 lines

enum ata_xfer_mask {

include/linux/libata.h +2 -0

@@ -864,6 +868,8 @@ struct ata_timing {
 	unsigned short cyc8b;		/* t0 for 8-bit I/O */
 	unsigned short active;		/* t2 or tD */
 	unsigned short recover;		/* t2i or tK */
+	unsigned short write_hold;	/* t4 */
+	unsigned short read_holdz;	/* t6z */

Sorry for failing to notice this before but t6z is again the timing that
the host can't control. Therefore I'm seeig no sense in its addition. I don't know how your driver is going to use it -- but most probably incorrectly...

MBR, Sergei

David Daney

author

2008-12-08 18:29 UTC #3 lore raw

Re: [PATCH 1/2] libata: Add two more columns to the ata_timing table.

Quoted 18 lines · Sergei Shtylyov

Hello.
Quoted 6 lines · David Daney
The forthcoming OCTEON SOC Compact Flash driver needs a few more timing values than were available in the ata_timing table. I add new columns for write_hold and read_holdz times. The values were obtained from the Compact Flash specification Rev 4.1.
Signed-off-by David Daney <[email protected]>
NAK.
Quoted 4 lines
include/linux/libata.h +0 -0
index ed3f26e..95fa9f6 100644
[...]

Quoted 10 lines

include/linux/libata.h struct ata_timing { · line 872
Quoted 7 lines
enum ata_xfer_mask {
include/linux/libata.h +2 -0
@@ -864,6 +868,8 @@ struct ata_timing {
     unsigned short cyc8b;        /* t0 for 8-bit I/O */
     unsigned short active;        /* t2 or tD */
     unsigned short recover;        /* t2i or tK */
+    unsigned short write_hold;    /* t4 */
+    unsigned short read_holdz;    /* t6z */
Sorry for failing to notice this before but t6z is again the timing
that the host can't control. Therefore I'm seeig no sense in its addition.

The host cannot control t6z, but in some cases (like ours) it needs to be aware of its value. We cannot start driving the bus for access to other devices until the CF stops driving it. We need to know how long to wait until it is safe to use the bus for other things.

Quoted 1 line

I don't know how your driver is going to use it -- but most probably incorrectly...

Thanks for that vote of confidence.

David Daney

Sergei Shtylyov

2008-12-08 18:48 UTC #4 lore raw

Re: [PATCH 1/2] libata: Add two more columns to the ata_timing table.

Quoted 33 lines · David Daney

Quoted 20 lines
Hello.
Quoted 6 lines · David Daney
The forthcoming OCTEON SOC Compact Flash driver needs a few more timing values than were available in the ata_timing table. I add new columns for write_hold and read_holdz times. The values were obtained from the Compact Flash specification Rev 4.1.
Signed-off-by David Daney <[email protected]>
NAK.
Quoted 4 lines
include/linux/libata.h +0 -0
index ed3f26e..95fa9f6 100644
[...]
Quoted 11 lines
include/linux/libata.h struct ata_timing { · line 872
Quoted 7 lines
enum ata_xfer_mask {
include/linux/libata.h +2 -0
@@ -864,6 +868,8 @@ struct ata_timing {
     unsigned short cyc8b;        /* t0 for 8-bit I/O */
     unsigned short active;        /* t2 or tD */
     unsigned short recover;        /* t2i or tK */
+    unsigned short write_hold;    /* t4 */
+    unsigned short read_holdz;    /* t6z */
Sorry for failing to notice this before but t6z is again the timing
that the host can't control. Therefore I'm seeig no sense in its addition.

Quoted 1 line

The host cannot control t6z, but in some cases (like ours) it needs to be aware of its value. We cannot start driving the bus for access to other devices until the CF stops driving it. We need to know how long to wait until it is safe to use the bus for other things.

Re-read the spec please, the timing you need is t9 and it's much longer
than t6 that your driver using instead.

Quoted 3 lines

Quoted 1 line
I don't know how your driver is going to use it -- but most probably incorrectly...
Thanks for that vote of confidence.

You're welcome. :-)

Quoted 1 line

David Daney

MBR, Sergei

Sergei Shtylyov

2008-12-08 18:55 UTC #5 lore raw

Re: [PATCH 1/2] libata: Add two more columns to the ata_timing table.

Quoted 18 lines · Hello, I wrote:

Quoted 17 lines
Quoted 17 lines
Quoted 4 lines
include/linux/libata.h +0 -0
index ed3f26e..95fa9f6 100644
[...]
include/linux/libata.h struct ata_timing { · line 872
Quoted 7 lines
enum ata_xfer_mask {
include/linux/libata.h +2 -0
@@ -864,6 +868,8 @@ struct ata_timing {
     unsigned short cyc8b;        /* t0 for 8-bit I/O */
     unsigned short active;        /* t2 or tD */
     unsigned short recover;        /* t2i or tK */
+    unsigned short write_hold;    /* t4 */
+    unsigned short read_holdz;    /* t6z */
Sorry for failing to notice this before but t6z is again the
timing that the host can't control. Therefore I'm seeig no sense in its addition.

Quoted 4 lines

Quoted 1 line
The host cannot control t6z, but in some cases (like ours) it needs to be aware of its value. We cannot start driving the bus for access to other devices until the CF stops driving it. We need to know how long to wait until it is safe to use the bus for other things.
Re-read the spec please, the timing you need is t9 and it's much
longer than t6 that your driver using instead.

OK, t6z is yet longer than t9 but putting it into the table seems
pointless anyway as it's fixed at 30 ns, at least for the standard 5 PIO modes (and for other two modes 30 ns would be good anyway).
Though frankly speaking I don't quite understand your care for this
timing, if the ATA standard permits -CE deasserted and data bus being driven to overlap. Remember that the minimum address setup time adds to that equation too.

MBR, Sergei

David Daney

author

2008-12-08 20:45 UTC #6 lore raw

Re: [PATCH 1/2] libata: Add two more columns to the ata_timing table.

Quoted 4 lines · Sergei Shtylyov

OK, t6z is yet longer than t9 but putting it into the table seems
pointless anyway as it's fixed at 30 ns, at least for the standard 5 PIO modes (and for other two modes 30 ns would be good anyway).
Though frankly speaking I don't quite understand your care for this
timing, if the ATA standard permits -CE deasserted and data bus being driven to overlap.

It doesn't matter what the ATA standard permits in this case. We need to assure that the OCTEON Boot Bus standard is respected. There are several timing parameters that we have to set based on the documented properties of the device. Ideally if we try to run bus cycles for non-ATA/CF devices that share the signal lines of the Boot Bus, they should not interfere with the CF interface.

David Daney

Sergei Shtylyov

2008-12-08 23:19 UTC #7 lore raw

Re: [PATCH 1/2] libata: Add two more columns to the ata_timing table.

Hello.

Quoted 6 lines · David Daney

Quoted 4 lines
OK, t6z is yet longer than t9 but putting it into the table seems
pointless anyway as it's fixed at 30 ns, at least for the standard 5 PIO modes (and for other two modes 30 ns would be good anyway).
Though frankly speaking I don't quite understand your care for
this timing, if the ATA standard permits -CE deasserted and data bus being driven to overlap.
It doesn't matter what the ATA standard permits in this case. We need to assure that the OCTEON Boot Bus standard is respected. There are several timing parameters that we have to set based on the documented properties of the device. Ideally if we try to run bus cycles for non-ATA/CF devices that share the signal lines of the Boot Bus, they should not interfere with the CF interface.

Well, your driver is free to handle it on its own, and even respect a
lesser minimum for PIO5/6. It just doesn't seem practical to add this to the table. In fact, about all PATA hardware on market only permits to control the active/recovery and the address setup times -- that's why the table looks like it looks now.

Quoted 1 line

David Daney

MBR, Sergei

David Daney

author

2008-12-05 23:10 UTC #8 lore raw

PATCH 2/2 libata: New driver for OCTEON SOC Compact Flash interface (v3).

In reply to David Daney

As part of our efforts to get the Cavium OCTEON processor support merged (see: http://marc.info/?l=linux-mips&m=122834773330212), we have this CF driver for your consideration.

Most OCTEON variants have *no* DMA or interrupt support on the CF interface so for these, only PIO is supported. Although if DMA is available, we do take advantage of it.

The register definitions are part of the chip support patch set mentioned above, and are not included here.

In this third version, At the suggestion of Jeff Garzik, I ditched the attempt to emulate sff bmdma. For DMA transfers, we just hook the .qc_issue port op and handle everything ourselves.

Also gone is the mwdmamodes module parameter that as was pointed out is redundant.

Thanks,

Signed-off-by David Daney <[email protected]>

---
 drivers/ata/Kconfig          |    9 +
 drivers/ata/Makefile         |    1 +
 drivers/ata/pata_octeon_cf.c |  956 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 966 insertions(+), 0 deletions(-)
 create mode 100644 drivers/ata/pata_octeon_cf.c

drivers/ata/Kconfig +9 -0

index 78fbec8..b59904b 100644
@@ -697,6 +697,15 @@ config PATA_IXP4XX_CF
 
 	  If unsure, say N.
 
+config PATA_OCTEON_CF
+	tristate "OCTEON Boot Bus Compact Flash support"
+	depends on CPU_CAVIUM_OCTEON
+	help
+	  This option enables a polled compact flash driver for use with
+	  compact flash cards attached to the OCTEON boot bus.
+
+	  If unsure, say N.
+
 config PATA_SCC
 	tristate "Toshiba's Cell Reference Set IDE support"
 	depends on PCI && PPC_CELLEB

drivers/ata/Makefile +1 -0

index 674965f..7f1ecf9 100644
@@ -69,6 +69,7 @@ obj-$(CONFIG_PATA_IXP4XX_CF)	+= pata_ixp4xx_cf.o
 obj-$(CONFIG_PATA_SCC)		+= pata_scc.o
 obj-$(CONFIG_PATA_SCH)		+= pata_sch.o
 obj-$(CONFIG_PATA_BF54X)	+= pata_bf54x.o
+obj-$(CONFIG_PATA_OCTEON_CF)	+= pata_octeon_cf.o
 obj-$(CONFIG_PATA_PLATFORM)	+= pata_platform.o
 obj-$(CONFIG_PATA_OF_PLATFORM)	+= pata_of_platform.o
 obj-$(CONFIG_PATA_ICSIDE)	+= pata_icside.o

drivers/ata/pata_octeon_cf.c new file +956 -0

956 lines not on this page · raw message

Signature · 1 line

1.5.6.5

Sergei Shtylyov

2008-12-06 22:46 UTC #9 lore raw

Re: [PATCH 2/2] libata: New driver for OCTEON SOC Compact Flash interface (v3).

Hello.

Quoted 11 lines · David Daney

Most OCTEON variants have *no* DMA or interrupt support on the CF interface so for these, only PIO is supported. Although if DMA is available, we do take advantage of it.
The register definitions are part of the chip support patch set mentioned above, and are not included here.
In this third version, At the suggestion of Jeff Garzik, I ditched the attempt to emulate sff bmdma. For DMA transfers, we just hook the .qc_issue port op and handle everything ourselves.

Sounds good. :-)

Quoted 1 line

Signed-off-by David Daney <[email protected]>

NAK. I will comment only on the timing setup code now. The rest when
the time permits (frankly speaking, it doesn't permit me to do even this :-)...

Quoted 6 lines

drivers/ata/pata_octeon_cf.c new file +0 -0
new file mode 100644
index 0000000..3b565a5

[...]

Quoted 11 lines

drivers/ata/pata_octeon_cf.c new file +10 -0

+/**
+ * Called after libata determines the needed PIO mode. This
+ * function programs the Octeon bootbus regions to support the
+ * timing requirements of the PIO mode.
+ *
+ * @ap:     ATA port information
+ * @dev:    ATA device
+ */
+static void octeon_cf_set_piomode(struct ata_port *ap, struct ata_device *dev)
+{

[...]

drivers/ata/pata_octeon_cf.c line 123

Quoted 2 lines

drivers/ata/pata_octeon_cf.c new file +1 -0
+	t6 = ns_to_tim_reg(2, 5);

I've already told you that the host cannot control t6.

drivers/ata/pata_octeon_cf.c line 126

Quoted 1 line

drivers/ata/pata_octeon_cf.c new file +1 -0
+	t6z = timing.read_holdz;

And neither t6z.

drivers/ata/pata_octeon_cf.c line 139

Quoted 5 lines

drivers/ata/pata_octeon_cf.c new file +4 -0

+	use_iordy = ata_pio_need_iordy(dev);
+	/* CF spec. r4.1 Table 22 says no iordy on PIO5 and PIO6.  */
+	if (dev->pio_mode == XFER_PIO_5 || dev->pio_mode == XFER_PIO_6)
+		use_iordy = 0;

This should be handled in the generic way instead, right in
ata_pio_need_iordy(). There's nothing Cavium specific in this check, just CF specific.

drivers/ata/pata_octeon_cf.c line 155

Quoted 4 lines

drivers/ata/pata_octeon_cf.c new file +3 -0
+
+	/* Time after CE that signals stay valid */
+	reg_tim.s.pause = t6z - t6;

The t6/t6z timings have nothing to do with -CE at all.

drivers/ata/pata_octeon_cf.c line 159

Quoted 5 lines

drivers/ata/pata_octeon_cf.c new file +4 -0

+	/* How long to hold after a write */
+	reg_tim.s.wr_hld = t4;
+	/* How long to wait after a read to de-assert CE. */
+	reg_tim.s.rd_hld = t6;

-DIOx de-assert to -CE de-assert time is t9, not t6, and it ranges
from 20 to 10 ns, hence you're programming it to 5 ns incorrectly.

drivers/ata/pata_octeon_cf.c line 165

Quoted 3 lines

drivers/ata/pata_octeon_cf.c new file +2 -0
+	/* Time after CE that read/write starts */
+	reg_tim.s.ce = 0;

This is t1, address valid to -DIOx setup anmd ranges from 70 to 25 ns
-- I don't know why you're programming it to a fixed value...

Quoted 3 lines

drivers/ata/pata_octeon_cf.c new file +2 -0

+static void octeon_cf_set_dmamode(struct ata_port *ap, struct ata_device *dev)
+{

[...]

drivers/ata/pata_octeon_cf.c line 192

Quoted 3 lines

drivers/ata/pata_octeon_cf.c new file +2 -0
+	timing = ata_timing_find_mode(dev->dma_mode);
+	To		= timing->cycle;

It's actually called t0 for both PIO and DMA.

drivers/ata/pata_octeon_cf.c line 197

Quoted 6 lines

drivers/ata/pata_octeon_cf.c new file +5 -0

+	Td		= timing->active;
+	Tkr		= timing->recover;
+
+	if (dev->dma_mode == XFER_MW_DMA_0) {
+		Tkr = 50; /* for MWDMA0, it differes from the table. */

It's safe to use tkw ISO tkr -- these are minimum timings, not maximum.

drivers/ata/pata_octeon_cf.c line 201

Quoted 5 lines

drivers/ata/pata_octeon_cf.c new file +4 -0
+		dma_ackh = 20;
+	} else {
+		dma_ackh = 5;
+	}

We can add this timing to the table -- it's called tJ if I don't
mistake...

drivers/ata/pata_octeon_cf.c line 212

Quoted 5 lines

drivers/ata/pata_octeon_cf.c new file +4 -0

+	/* not spec'ed, value in eclocks, not affected by tim_mult */
+	dma_arq = 8;
+	pause = 25 - dma_arq * 1000 /
+		(octeon_get_clock_rate() / 1000000); /* Tz */

Host cannot control tZ timing either.

drivers/ata/pata_octeon_cf.c line 215

Quoted 3 lines

drivers/ata/pata_octeon_cf.c new file +2 -0
+	/* Td (Seem to need more margin here.... */
+	oe_a = Td + 20;

It's interesting why...

drivers/ata/pata_octeon_cf.c line 217

Quoted 3 lines

drivers/ata/pata_octeon_cf.c new file +2 -0
+	/* Tkr from cf spec, lengthened to meet To */
+	oe_n = max(To - oe_a, Tkr);

This is not a good way to calculate the recovery time. You need the
cycle/active time "quantized" first, or you'll probably get somewhat lengthened resulting cycle...

drivers/ata/pata_octeon_cf.c line 224

Quoted 2 lines

drivers/ata/pata_octeon_cf.c new file +1 -0
+	dma_tim.s.dmack_s = ns_to_tim_reg(tim_mult, dma_acks);

Your dma_acks is always 0, so quantizing is uselss.

drivers/ata/pata_octeon_cf.c line 243

Quoted 9 lines

drivers/ata/pata_octeon_cf.c new file +8 -0

+#if 0
+	pr_info("ns to ticks (mult %d) of %d is: %d\n", tim_mult, 60,
+		     ns_to_tim_reg(tim_mult, 60));
+	pr_info("oe_n: %d, oe_a: %d, dmack_s: %d, dmack_h: "
+		"%d, dmarq: %d, pause: %d\n",
+		dma_tim.s.oe_n, dma_tim.s.oe_a, dma_tim.s.dmack_s,
+		dma_tim.s.dmack_h, dma_tim.s.dmarq, dma_tim.s.pause);
+#endif

Don't leave #if'ed out code please. E.g. convert it to pr_debug()
instead...

MBR, Sergei