Re: [PATCH 1/1] AVR32 PATA driver
Kristoffer Nyborg Gregertsen wrote: Updated and simplified driver. Use only register transfer timing for both data and register transfers. This gives poorer performance in PIO1 and 2, but should not be a problem in PIO3 and 4, correct me if I'm wrong :) The driver works very we'll but I still wonder about the interrupts. I have an interrupt line, that works nicely when POLLING flag is not set. The problem is the number of interrupts that eat away my CPU cycles. When using the POLLING flag there seem to be some interrupts that dosen't get cleared. Furthermore the device dosen't drive INTRQ high, it stays at 2.5 volts and generates a lot of interrupts due to ripple / noise. What to do? Signed-off-by: Kristoffer Nyborg Gregertsen [EMAIL PROTECTED] applied - 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 1/1] AVR32 PATA driver
Forget about what I said about using polling, I was just fooled by the benchmark tool. Could the last patch I've sent you be accepted? - Kristoffer Nyborg Gregertsen - 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 1/1] AVR32 PATA driver
Hi again everybody, The driver is now working very nicely. The sporadic errors I reported earlier were mostly due to the state of the EBI to ATA adaptor prototype card. I have also added some time before R/W strobe to make sure signals are stable. The only thing that bothers me is the huge number of interrupts that occurs during data transfers. There is one interrupt for every ~500 bytes or so: -sh-3.2# cat /proc/interrupts ... 65: 62015176 eic at32_ide -sh-3.2# dd if=/dev/zero of=zero.dat count=10k 10240+0 records in 10240+0 records out -sh-3.2# cat /proc/interrupts ... 65: 62025176 eic at32_ide Would I be better off using polling mode? -- Kristoffer Nyborg Gregertsen MSc. student / Summer intern Atmel Norway - 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 1/1] AVR32 PATA driver
Updated and simplified driver. Use only register transfer timing for both data and register transfers. This gives poorer performance in PIO1 and 2, but should not be a problem in PIO3 and 4, correct me if I'm wrong :) The driver works very we'll but I still wonder about the interrupts. I have an interrupt line, that works nicely when POLLING flag is not set. The problem is the number of interrupts that eat away my CPU cycles. When using the POLLING flag there seem to be some interrupts that dosen't get cleared. Furthermore the device dosen't drive INTRQ high, it stays at 2.5 volts and generates a lot of interrupts due to ripple / noise. What to do? Signed-off-by: Kristoffer Nyborg Gregertsen [EMAIL PROTECTED] --- diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig index 4ad8675..f6b8dd2 100644 --- a/drivers/ata/Kconfig +++ b/drivers/ata/Kconfig @@ -199,6 +199,15 @@ config PATA_ARTOP If unsure, say N. +config PATA_AT32 + tristate Atmel AVR32 PATA support (Experimental) + depends on AVR32 PLATFORM_AT32AP EXPERIMENTAL + help + This option enables support for the IDE devices on the + Atmel AT32AP platform. + + If unsure, say N. + config PATA_ATIIXP tristate ATI PATA support (Experimental) depends on PCI EXPERIMENTAL diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile index 8149c68..7c5e319 100644 --- a/drivers/ata/Makefile +++ b/drivers/ata/Makefile @@ -21,6 +21,7 @@ obj-$(CONFIG_PDC_ADMA)+= pdc_adma.o obj-$(CONFIG_PATA_ALI) += pata_ali.o obj-$(CONFIG_PATA_AMD) += pata_amd.o obj-$(CONFIG_PATA_ARTOP) += pata_artop.o +obj-$(CONFIG_PATA_AT32)+= pata_at32.o obj-$(CONFIG_PATA_ATIIXP) += pata_atiixp.o obj-$(CONFIG_PATA_CMD640_PCI) += pata_cmd640.o obj-$(CONFIG_PATA_CMD64X) += pata_cmd64x.o diff --git a/drivers/ata/pata_at32.c b/drivers/ata/pata_at32.c new file mode 100644 index 000..bb250a4 --- /dev/null +++ b/drivers/ata/pata_at32.c @@ -0,0 +1,441 @@ +/* + * AVR32 SMC/CFC PATA Driver + * + * Copyright (C) 2007 Atmel Norway + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License version + * 2 as published by the Free Software Foundation. + */ + +#define DEBUG + +#include linux/kernel.h +#include linux/module.h +#include linux/init.h +#include linux/device.h +#include linux/platform_device.h +#include linux/delay.h +#include linux/interrupt.h +#include linux/irq.h +#include scsi/scsi_host.h +#include linux/ata.h +#include linux/libata.h +#include linux/err.h +#include linux/io.h + +#include asm/arch/board.h +#include asm/arch/smc.h + +#define DRV_NAME pata_at32 +#define DRV_VERSION 0.0.2 + +/* + * CompactFlash controller memory layout relative to the base address: + * + * Attribute memory: - 003f + * Common memory: 0040 - 007f + * I/O memory:0080 - 00bf + * True IDE Mode: 00c0 - 00df + * Alt IDE Mode: 00e0 - 00ff + * + * Only True IDE and Alt True IDE mode are needed for this driver. + * + * True IDE mode = CS0 = 0, CS1 = 1 (cmd, error, stat, etc) + * Alt True IDE mode = CS0 = 1, CS1 = 0 (ctl, alt_stat) + */ +#define CF_IDE_OFFSET0x00c0 +#define CF_ALT_IDE_OFFSET 0x00e0 +#define CF_RES_SIZE 2048 + +/* + * Define DEBUG_BUS if you are doing debugging of your own EBI - PATA + * adaptor with a logic analyzer or similar. + */ +#undef DEBUG_BUS + +/* + * ATA PIO modes + * + * Name| Mb/s | Min cycle time | Mask + * +---++ + * Mode 0 | 3.3 | 600 ns | 0x01 + * Mode 1 | 5.2 | 383 ns | 0x03 + * Mode 2 | 8.3 | 240 ns | 0x07 + * Mode 3 | 11.1 | 180 ns | 0x0f + * Mode 4 | 16.7 | 120 ns | 0x1f + */ +#define PIO_MASK (0x1f) + +/* + * Struct containing private information about device. + */ +struct at32_ide_info { + unsigned intirq; + struct resource res_ide; + struct resource res_alt; + void __iomem*ide_addr; + void __iomem*alt_addr; + unsigned intcs; + struct smc_config smc; +}; + +/* + * Setup SMC for the given ATA timing. + */ +static int pata_at32_setup_timing(struct device *dev, + struct at32_ide_info *info, + const struct ata_timing *timing) +{ + /* These two values are found through testing */ + const int min_recover = 25; + const int ncs_hold= 15; + + struct smc_config *smc = info-smc; + + int active; + int recover; + + /* Total cycle time */ + smc-read_cycle = timing-cyc8b; + + /* DIOR = CFIOR timings */ + smc-nrd_setup = timing-setup; + smc-nrd_pulse = timing-act8b; + + /* Compute recover, extend
Re: [PATCH 1/1] AVR32 PATA driver
+static int pata_at32_get_pio_mask(void) +{ + switch (max_pio) { + case 0: + return 0x01; + case 1: + return 0x03; + case 2: + return 0x07; + case 3: + return 0x0f; + case 4: + return 0x1f; + default: + return 0x01; What is wrong with just using (1 max_pio) - 1 as the range is only 0-4 anyway. +static void pata_at32_data_xfer(struct ata_device *adev, unsigned char *buf, + unsigned int buflen, int write_data) +{ + struct at32_ide_info *info = adev-ap-host-private_data; + + /* Set SMC to data transfer speed */ + if (info-smc_pio_mode 3) + smc_restore_registers(info-cs, info-smc_16.reg); + + /* Transfer data */ + ata_data_xfer(adev, buf, buflen, write_data); + + /* Set SMC back to register transfer speed */ + if (info-smc_pio_mode 3) + smc_restore_registers(info-cs, info-smc_8.reg); Should be safe currently for IRQ driven behaviour, might be for polled but remember that without locking you could end up reading the status before you switch the clock back so I'm not 100% sure. - 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 1/1] AVR32 PATA driver
On Tuesday 07 August 2007 17:54:09 Alan Cox wrote: +static int pata_at32_get_pio_mask(void) +{ + switch (max_pio) { + case 0: + return 0x01; + case 1: + return 0x03; + case 2: + return 0x07; + case 3: + return 0x0f; + case 4: + return 0x1f; + default: + return 0x01; What is wrong with just using (1 max_pio) - 1 as the range is only 0-4 anyway. Since max_pio is a module argument it may be invalid. Perhaps: if (0 = max_pio max_pio = 4) return (1 max_pio) - 1; else return 0x01; Or is it common to trust the module arguments to be sane? +static void pata_at32_data_xfer(struct ata_device *adev, unsigned char *buf, + unsigned int buflen, int write_data) +{ + struct at32_ide_info *info = adev-ap-host-private_data; + + /* Set SMC to data transfer speed */ + if (info-smc_pio_mode 3) + smc_restore_registers(info-cs, info-smc_16.reg); + + /* Transfer data */ + ata_data_xfer(adev, buf, buflen, write_data); + + /* Set SMC back to register transfer speed */ + if (info-smc_pio_mode 3) + smc_restore_registers(info-cs, info-smc_8.reg); Should be safe currently for IRQ driven behaviour, might be for polled but remember that without locking you could end up reading the status before you switch the clock back so I'm not 100% sure. I now see that things can get messed up. I can alter the hardware so that a seperate SMC memory space for data and register transfer can be used, each with their respective timing set at all times. This will cost one extra chip select pin and an AND port on the adaptor, but it might be worth it? -- With kind regards Kristoffer Nyborg Gregertsen Student, Department of Engineering Cybernetics Norwegian University of Science and Technology Trondheim, Norway - 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 1/1] AVR32 PATA driver
Kristoffer Nyborg Gregertsen wrote: On Tuesday 07 August 2007 17:54:09 Alan Cox wrote: +static int pata_at32_get_pio_mask(void) +{ + switch (max_pio) { + case 0: + return 0x01; + case 1: + return 0x03; + case 2: + return 0x07; + case 3: + return 0x0f; + case 4: + return 0x1f; + default: + return 0x01; What is wrong with just using (1 max_pio) - 1 as the range is only 0-4 anyway. Since max_pio is a module argument it may be invalid. Perhaps: if (0 = max_pio max_pio = 4) return (1 max_pio) - 1; else return 0x01; Or is it common to trust the module arguments to be sane? Well, a higher level issue, you should not have a max_pio module parameter at all. Other drivers do not have such a thing. Jeff - 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 1/1] AVR32 PATA driver
On Tuesday 07 August 2007 20:14:27 Jeff Garzik wrote: Well, a higher level issue, you should not have a max_pio module parameter at all. Other drivers do not have such a thing. OK, I'll remove it then. It was very convenient during automated testing of all PIO modes, but I guess that's not needed by the end users. -- With kind regards Kristoffer Nyborg Gregertsen Student, Department of Engineering Cybernetics Norwegian University of Science and Technology Trondheim, Norway - 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