Re: [PATCH] pata_cmd64x: Set up MWDMA modes properly
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
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
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
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
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
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
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