Re: [PATCH 1/1] AVR32 PATA driver

2007-08-31 Thread Jeff Garzik

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

2007-08-15 Thread Kristoffer Nyborg Gregertsen
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

2007-08-14 Thread Kristoffer Nyborg Gregertsen
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

2007-08-08 Thread Kristoffer Nyborg Gregertsen
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

2007-08-07 Thread Alan Cox
 +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

2007-08-07 Thread Kristoffer Nyborg Gregertsen
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

2007-08-07 Thread Jeff Garzik

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

2007-08-07 Thread Kristoffer Nyborg Gregertsen
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