Re: [PATCH] pata_cmd64x: Set up MWDMA modes properly

2007-08-17 Thread Sergei Shtylyov

Hello.

Alan Cox wrote:

   That function rocks (except I didn't get what the address setup timings 
mean to SW/MW DMA)...



They don't, it merges the DMA and PIO ones and you end up with the PIO
ones.


   What I meant there was this table from libata-core.c:

/*
 * PIO 0-4, MWDMA 0-2 and UDMA 0-6 timings (in nanoseconds).
 * These were taken from ATA/ATAPI-6 standard, rev 0a, except
 * for UDMA6, which is currently supported only by Maxtor drives.
 *
 * For PIO 5/6 MWDMA 3/4 see the CFA specification 3.0.
 */

static const struct ata_timing ata_timing[] = {

{ XFER_UDMA_6, 0,   0,   0,   0,   0,   0,   0,  15 },
{ XFER_UDMA_5, 0,   0,   0,   0,   0,   0,   0,  20 },
{ XFER_UDMA_4, 0,   0,   0,   0,   0,   0,   0,  30 },
{ XFER_UDMA_3, 0,   0,   0,   0,   0,   0,   0,  45 },

{ XFER_MW_DMA_4,  25,   0,   0,   0,  55,  20,  80,   0 },
{ XFER_MW_DMA_3,  25,   0,   0,   0,  65,  25, 100,   0 },
{ XFER_UDMA_2, 0,   0,   0,   0,   0,   0,   0,  60 },
{ XFER_UDMA_1, 0,   0,   0,   0,   0,   0,   0,  80 },
{ XFER_UDMA_0, 0,   0,   0,   0,   0,   0,   0, 120 },

/*  { XFER_UDMA_SLOW,  0,   0,   0,   0,   0,   0,   0, 150 }, */



{ XFER_MW_DMA_2,  25,   0,   0,   0,  70,  25, 120,   0 },
{ XFER_MW_DMA_1,  45,   0,   0,   0,  80,  50, 150,   0 },
{ XFER_MW_DMA_0,  60,   0,   0,   0, 215, 215, 480,   0 },

{ XFER_SW_DMA_2,  60,   0,   0,   0, 120, 120, 240,   0 },
{ XFER_SW_DMA_1,  90,   0,   0,   0, 240, 240, 480,   0 },
{ XFER_SW_DMA_0, 120,   0,   0,   0, 480, 480, 960,   0 },

{ XFER_PIO_6, 10,  55,  20,  80,  55,  20,  80,   0 },
{ XFER_PIO_5, 15,  65,  25, 100,  65,  25, 100,   0 },
{ XFER_PIO_4, 25,  70,  25, 120,  70,  25, 120,   0 },
{ XFER_PIO_3, 30,  80,  70, 180,  80,  70, 180,   0 },

{ XFER_PIO_2, 30, 290,  40, 330, 100,  90, 240,   0 },
{ XFER_PIO_1, 50, 290,  93, 383, 125, 100, 383,   0 },
{ XFER_PIO_0, 70, 290, 240, 600, 165, 150, 600,   0 },

/*  { XFER_PIO_SLOW, 120, 290, 240, 960, 290, 240, 960,   0 }, */

{ 0xFF }
};

   Note non-zero values in column 2 of the initilaizers for SW/MW DMA lines 
-- that's the 'setup' field of 'struct ata_timings'. Where did those figures 
come from?
   Ah, I see -- that must be *data* setup/hold times summed up. What I don't 
see is how it connects with the address setup time...



regD |= 0x20  adev-devno;


   Hm, is setting of drive DMA capable bits really needed in the driver. 
Doesn't libata core itself do this?



For most hardware it doesn't matter. The CMD64x driver code I have all
bothers to set it. I don't know if its needed, someone with the bits
handy could find out I guess, but if we arent sure it does no harm.


   I've removed that from the IDE driver -- none yelled yet. :-)

MBR, Sergei
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] pata_cmd64x: Set up MWDMA modes properly

2007-08-11 Thread Sergei Shtylyov

Hello.

Alan Cox wrote:


Set the MWDMA timing by updating the correct registers. Split the PIO
path as this is mostly shared code. Wants testing.


   Cool! So much simpler than my fix to the old IDE driver...


Signed-off-by: Alan Cox [EMAIL PROTECTED]


Acked-by: Sergei Shtylyov [EMAIL PROTECTED]


diff -u --new-file --recursive --exclude-from /usr/src/exclude 
linux.vanilla-2.6.23rc1-mm1/drivers/ata/pata_cmd64x.c 
linux-2.6.23rc1-mm1/drivers/ata/pata_cmd64x.c
--- linux.vanilla-2.6.23rc1-mm1/drivers/ata/pata_cmd64x.c   2007-07-26 
15:01:52.0 +0100
+++ linux-2.6.23rc1-mm1/drivers/ata/pata_cmd64x.c   2007-08-08 
11:52:23.0 +0100

[...]

@@ -117,8 +118,9 @@
int arttim = arttim_port[ap-port_no][adev-devno];
int drwtim = drwtim_port[ap-port_no][adev-devno];
 
-

-   if (ata_timing_compute(adev, adev-pio_mode, t, T, 0)  0) {
+   /* ata_timing_compute is smart and will produce timings for MWDMA
+  that don't violate the drives PIO capabilities. */
+   if (ata_timing_compute(adev, mode, t, T, 0)  0) {
printk(KERN_ERR DRV_NAME : mode computation failed.\n);
return;
}


   That function rocks (except I didn't get what the address setup timings 
mean to SW/MW DMA)...



@@ -168,6 +170,20 @@
 }
 
 /**

+ * cmd64x_set_piomode  -   set initial PIO mode data
+ * @ap: ATA interface
+ * @adev: ATA device
+ *
+ * Used when configuring the devices ot set the PIO timings. All the


   What's ot?
   Hm, writing comments (and even the code) in boustrophedon manner (odd 
lines from the left to the right, and even lines from the right to to the 
left) is certainly a challenging idea. :-)



+ * actual work is done by the PIO/MWDMA setting helper
+ */
+
+static void cmd64x_set_piomode(struct ata_port *ap, struct ata_device *adev)
+{
+   cmd64x_set_timing(ap, adev, adev-pio_mode);
+}
+
+/**
  * cmd64x_set_dmamode  -   set initial DMA mode data
  * @ap: ATA interface
  * @adev: ATA device
@@ -180,9 +196,6 @@
static const u8 udma_data[] = {
0x30, 0x20, 0x10, 0x20, 0x10, 0x00
};
-   static const u8 mwdma_data[] = {
-   0x30, 0x20, 0x10
-   };
 
 	struct pci_dev *pdev = to_pci_dev(ap-host-dev);

u8 regU, regD;
@@ -208,8 +221,10 @@
regU |= 1  adev-devno; /* UDMA on */
if (adev-dma_mode  2)   /* 15nS timing */
regU |= 4  adev-devno;
-   } else
-   regD |= mwdma_data[adev-dma_mode - XFER_MW_DMA_0]  shift;
+   } else {
+   regU = ~ (1  adev-devno);  /* UDMA off */
+   cmd64x_set_timing(ap, adev, adev-dma_mode);
+   }
 
 	regD |= 0x20  adev-devno;


   Hm, is setting of drive DMA capable bits really needed in the driver. 
Doesn't libata core itself do this?


MBR, Sergei
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] pata_cmd64x: Set up MWDMA modes properly

2007-08-11 Thread Alan Cox
 That function rocks (except I didn't get what the address setup timings 
 mean to SW/MW DMA)...

They don't, it merges the DMA and PIO ones and you end up with the PIO
ones.

  + * Used when configuring the devices ot set the PIO timings. All the
 
 What's ot?

to - will fix

  regD |= 0x20  adev-devno;
 
 Hm, is setting of drive DMA capable bits really needed in the driver. 
 Doesn't libata core itself do this?

For most hardware it doesn't matter. The CMD64x driver code I have all
bothers to set it. I don't know if its needed, someone with the bits
handy could find out I guess, but if we arent sure it does no harm.
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] pata_cmd64x: Set up MWDMA modes properly

2007-08-11 Thread Bartlomiej Zolnierkiewicz
On Saturday 11 August 2007, Sergei Shtylyov wrote:
 Hello.
 
 Alan Cox wrote:
 
  Set the MWDMA timing by updating the correct registers. Split the PIO
  path as this is mostly shared code. Wants testing.
 
 Cool! So much simpler than my fix to the old IDE driver...
 
  Signed-off-by: Alan Cox [EMAIL PROTECTED]
 
 Acked-by: Sergei Shtylyov [EMAIL PROTECTED]
 
  diff -u --new-file --recursive --exclude-from /usr/src/exclude 
  linux.vanilla-2.6.23rc1-mm1/drivers/ata/pata_cmd64x.c 
  linux-2.6.23rc1-mm1/drivers/ata/pata_cmd64x.c
  --- linux.vanilla-2.6.23rc1-mm1/drivers/ata/pata_cmd64x.c   2007-07-26 
  15:01:52.0 +0100
  +++ linux-2.6.23rc1-mm1/drivers/ata/pata_cmd64x.c   2007-08-08 
  11:52:23.0 +0100
 [...]
  @@ -117,8 +118,9 @@
  int arttim = arttim_port[ap-port_no][adev-devno];
  int drwtim = drwtim_port[ap-port_no][adev-devno];
   
  -
  -   if (ata_timing_compute(adev, adev-pio_mode, t, T, 0)  0) {
  +   /* ata_timing_compute is smart and will produce timings for MWDMA
  +  that don't violate the drives PIO capabilities. */
  +   if (ata_timing_compute(adev, mode, t, T, 0)  0) {
  printk(KERN_ERR DRV_NAME : mode computation failed.\n);
  return;
  }
 
 That function rocks (except I didn't get what the address setup timings 
 mean to SW/MW DMA)...

JFYI: this function was borrowed from drivers/ide/ide-timing.h,
you can use it in IDE host drivers as well... ;)

Bart
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] pata_cmd64x: Set up MWDMA modes properly

2007-08-09 Thread Alan Cox
On Fri, 10 Aug 2007 01:09:13 +0200 (MEST)
Mikael Pettersson [EMAIL PROTECTED] wrote:

 On Wed, 8 Aug 2007 14:33:20 +0100, Alan Cox wrote:
  Set the MWDMA timing by updating the correct registers. Split the PIO
  path as this is mostly shared code. Wants testing.
 
 Works fine on my CMD646 SPARC Ultra5.

Thanks
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] pata_cmd64x: Set up MWDMA modes properly

2007-08-09 Thread Mikael Pettersson
On Wed, 8 Aug 2007 14:33:20 +0100, Alan Cox wrote:
 Set the MWDMA timing by updating the correct registers. Split the PIO
 path as this is mostly shared code. Wants testing.

Works fine on my CMD646 SPARC Ultra5.

/Mikael
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] pata_cmd64x: Set up MWDMA modes properly

2007-08-08 Thread Alan Cox
Set the MWDMA timing by updating the correct registers. Split the PIO
path as this is mostly shared code. Wants testing.

Signed-off-by: Alan Cox [EMAIL PROTECTED]

diff -u --new-file --recursive --exclude-from /usr/src/exclude 
linux.vanilla-2.6.23rc1-mm1/drivers/ata/pata_cmd64x.c 
linux-2.6.23rc1-mm1/drivers/ata/pata_cmd64x.c
--- linux.vanilla-2.6.23rc1-mm1/drivers/ata/pata_cmd64x.c   2007-07-26 
15:01:52.0 +0100
+++ linux-2.6.23rc1-mm1/drivers/ata/pata_cmd64x.c   2007-08-08 
11:52:23.0 +0100
@@ -31,7 +31,7 @@
 #include linux/libata.h
 
 #define DRV_NAME pata_cmd64x
-#define DRV_VERSION 0.2.3
+#define DRV_VERSION 0.2.4
 
 /*
  * CMD64x specific registers definition.
@@ -88,14 +88,15 @@
 }
 
 /**
- * cmd64x_set_piomode  -   set initial PIO mode data
+ * cmd64x_set_piomode  -   set PIO and MWDMA timing
  * @ap: ATA interface
  * @adev: ATA device
+ * @mode: mode
  *
- * Called to do the PIO mode setup.
+ * Called to do the PIO and MWDMA mode setup.
  */
 
-static void cmd64x_set_piomode(struct ata_port *ap, struct ata_device *adev)
+static void cmd64x_set_timing(struct ata_port *ap, struct ata_device *adev, u8 
mode)
 {
struct pci_dev *pdev = to_pci_dev(ap-host-dev);
struct ata_timing t;
@@ -117,8 +118,9 @@
int arttim = arttim_port[ap-port_no][adev-devno];
int drwtim = drwtim_port[ap-port_no][adev-devno];
 
-
-   if (ata_timing_compute(adev, adev-pio_mode, t, T, 0)  0) {
+   /* ata_timing_compute is smart and will produce timings for MWDMA
+  that don't violate the drives PIO capabilities. */
+   if (ata_timing_compute(adev, mode, t, T, 0)  0) {
printk(KERN_ERR DRV_NAME : mode computation failed.\n);
return;
}
@@ -168,6 +170,20 @@
 }
 
 /**
+ * cmd64x_set_piomode  -   set initial PIO mode data
+ * @ap: ATA interface
+ * @adev: ATA device
+ *
+ * Used when configuring the devices ot set the PIO timings. All the
+ * actual work is done by the PIO/MWDMA setting helper
+ */
+
+static void cmd64x_set_piomode(struct ata_port *ap, struct ata_device *adev)
+{
+   cmd64x_set_timing(ap, adev, adev-pio_mode);
+}
+
+/**
  * cmd64x_set_dmamode  -   set initial DMA mode data
  * @ap: ATA interface
  * @adev: ATA device
@@ -180,9 +196,6 @@
static const u8 udma_data[] = {
0x30, 0x20, 0x10, 0x20, 0x10, 0x00
};
-   static const u8 mwdma_data[] = {
-   0x30, 0x20, 0x10
-   };
 
struct pci_dev *pdev = to_pci_dev(ap-host-dev);
u8 regU, regD;
@@ -208,8 +221,10 @@
regU |= 1  adev-devno; /* UDMA on */
if (adev-dma_mode  2) /* 15nS timing */
regU |= 4  adev-devno;
-   } else
-   regD |= mwdma_data[adev-dma_mode - XFER_MW_DMA_0]  shift;
+   } else {
+   regU = ~ (1  adev-devno);   /* UDMA off */
+   cmd64x_set_timing(ap, adev, adev-dma_mode);
+   }
 
regD |= 0x20  adev-devno;
 
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html