* [PATCH v2 1/3] PATA host controller driver for ep93xx
@ 2012-04-03 14:45 ` Rafal Prylowski
0 siblings, 0 replies; 51+ messages in thread
From: Rafal Prylowski @ 2012-04-03 14:45 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Rafal Prylowski <prylowski@metasoft.pl>
Cc: Joao Ramos <joao.ramos@inov.pt>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
Cc: Ryan Mallon <rmallon@gmail.com>
Cc: Sergei Shtylyov <sshtylyov@mvista.com>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ata/Kconfig | 9
drivers/ata/Makefile | 1
drivers/ata/pata_ep93xx.c | 976 ++++++++++++++++++++++++++++++++++++
3 files changed, 986 insertions(+)
Index: linux-2.6/drivers/ata/pata_ep93xx.c
===================================================================
--- /dev/null
+++ linux-2.6/drivers/ata/pata_ep93xx.c
@@ -0,0 +1,976 @@
+/*
+ * EP93XX PATA controller driver.
+ *
+ * Copyright (c) 2012, Metasoft s.c.
+ * Rafal Prylowski <prylowski@metasoft.pl>
+ *
+ * Based on pata_scc.c, pata_icside.c and on earlier version of EP93XX
+ * PATA driver by Lennert Buytenhek and Alessandro Zummo.
+ * Read/Write timings, resource management and other improvements
+ * from driver by Joao Ramos and Bartlomiej Zolnierkiewicz.
+ * DMA engine support based on spi-ep93xx.c by Mika Westerberg.
+ *
+ * Original copyrights:
+ *
+ * Support for Cirrus Logic's EP93xx (EP9312, EP9315) CPUs
+ * PATA host controller driver.
+ *
+ * Copyright (c) 2009, Bartlomiej Zolnierkiewicz
+ *
+ * Heavily based on the ep93xx-ide.c driver:
+ *
+ * Copyright (c) 2009, Joao Ramos <joao.ramos@inov.pt>
+ * INESC Inovacao (INOV)
+ *
+ * EP93XX PATA controller driver.
+ * Copyright (C) 2007 Lennert Buytenhek <buytenh@wantstofly.org>
+ *
+ * An ATA driver for the Cirrus Logic EP93xx PATA controller.
+ *
+ * Based on an earlier version by Alessandro Zummo, which is:
+ * Copyright (C) 2006 Tower Technologies
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/blkdev.h>
+#include <scsi/scsi_host.h>
+#include <linux/ata.h>
+#include <linux/libata.h>
+#include <linux/platform_device.h>
+#include <linux/delay.h>
+#include <linux/dmaengine.h>
+
+#include <mach/dma.h>
+#include <mach/platform.h>
+
+#define DRV_NAME "ep93xx-ide"
+#define DRV_VERSION "1.0"
+
+enum {
+ /* IDE Control Register */
+ IDECTRL = 0x00,
+ IDECTRL_CS0N = (1 << 0),
+ IDECTRL_CS1N = (1 << 1),
+ IDECTRL_DIORN = (1 << 5),
+ IDECTRL_DIOWN = (1 << 6),
+ IDECTRL_INTRQ = (1 << 9),
+ IDECTRL_IORDY = (1 << 10),
+ /*
+ * the device IDE register to be accessed is selected through
+ * IDECTRL register's specific bitfields 'DA', 'CS1N' and 'CS0N':
+ * b4 b3 b2 b1 b0
+ * A2 A1 A0 CS1N CS0N
+ * the values filled in this structure allows the value to be directly
+ * ORed to the IDECTRL register, hence giving directly the A[2:0] and
+ * CS1N/CS0N values for each IDE register.
+ * The values correspond to the transformation:
+ * ((real IDE address) << 2) | CS1N value << 1 | CS0N value
+ */
+ IDECTRL_ADDR_CMD = 0 + 2, /* CS1 */
+ IDECTRL_ADDR_DATA = (ATA_REG_DATA << 2) + 2,
+ IDECTRL_ADDR_ERROR = (ATA_REG_ERR << 2) + 2,
+ IDECTRL_ADDR_FEATURE = (ATA_REG_FEATURE << 2) + 2,
+ IDECTRL_ADDR_NSECT = (ATA_REG_NSECT << 2) + 2,
+ IDECTRL_ADDR_LBAL = (ATA_REG_LBAL << 2) + 2,
+ IDECTRL_ADDR_LBAM = (ATA_REG_LBAM << 2) + 2,
+ IDECTRL_ADDR_LBAH = (ATA_REG_LBAH << 2) + 2,
+ IDECTRL_ADDR_DEVICE = (ATA_REG_DEVICE << 2) + 2,
+ IDECTRL_ADDR_STATUS = (ATA_REG_STATUS << 2) + 2,
+ IDECTRL_ADDR_COMMAND = (ATA_REG_CMD << 2) + 2,
+ IDECTRL_ADDR_ALTSTATUS = (0x06 << 2) + 1, /* CS0 */
+ IDECTRL_ADDR_CTL = (0x06 << 2) + 1, /* CS0 */
+
+ /* IDE Configuration Register */
+ IDECFG = 0x04,
+ IDECFG_IDEEN = (1 << 0),
+ IDECFG_PIO = (1 << 1),
+ IDECFG_MDMA = (1 << 2),
+ IDECFG_UDMA = (1 << 3),
+ IDECFG_MODE_SHIFT = 4,
+ IDECFG_MODE_MASK = (0xf << 4),
+ IDECFG_WST_SHIFT = 8,
+ IDECFG_WST_MASK = (0x3 << 8),
+
+ /* MDMA Operation Register */
+ IDEMDMAOP = 0x08,
+
+ /* UDMA Operation Register */
+ IDEUDMAOP = 0x0c,
+ IDEUDMAOP_UEN = (1 << 0),
+ IDEUDMAOP_RWOP = (1 << 1),
+
+ /* PIO/MDMA/UDMA Data Registers */
+ IDEDATAOUT = 0x10,
+ IDEDATAIN = 0x14,
+ IDEMDMADATAOUT = 0x18,
+ IDEMDMADATAIN = 0x1c,
+ IDEUDMADATAOUT = 0x20,
+ IDEUDMADATAIN = 0x24,
+
+ /* UDMA Status Register */
+ IDEUDMASTS = 0x28,
+ IDEUDMASTS_DMAIDE = (1 << 16),
+ IDEUDMASTS_INTIDE = (1 << 17),
+ IDEUDMASTS_SBUSY = (1 << 18),
+ IDEUDMASTS_NDO = (1 << 24),
+ IDEUDMASTS_NDI = (1 << 25),
+ IDEUDMASTS_N4X = (1 << 26),
+
+ /* UDMA Debug Status Register */
+ IDEUDMADEBUG = 0x2c,
+};
+
+struct ep93xx_pata_data /* private_data in ata_host */
+{
+ const struct platform_device *pdev;
+ void __iomem *ide_base;
+ const struct ata_timing *t;
+ const struct ata_timing *cmd_t;
+ bool iordy;
+
+ unsigned long udma_in_phys;
+ unsigned long udma_out_phys;
+
+ struct dma_chan *dma_rx_channel;
+ struct ep93xx_dma_data dma_rx_data;
+ struct dma_chan *dma_tx_channel;
+ struct ep93xx_dma_data dma_tx_data;
+};
+
+static void ep93xx_pata_clear_regs(void __iomem *base)
+{
+ writel(IDECTRL_CS0N | IDECTRL_CS1N | IDECTRL_DIORN |
+ IDECTRL_DIOWN, base + IDECTRL);
+
+ writel(0, base + IDECFG);
+ writel(0, base + IDEMDMAOP);
+ writel(0, base + IDEUDMAOP);
+ writel(0, base + IDEDATAOUT);
+ writel(0, base + IDEDATAIN);
+ writel(0, base + IDEMDMADATAOUT);
+ writel(0, base + IDEMDMADATAIN);
+ writel(0, base + IDEUDMADATAOUT);
+ writel(0, base + IDEUDMADATAIN);
+ writel(0, base + IDEUDMADEBUG);
+}
+
+static int ep93xx_pata_check_iordy(void __iomem *base)
+{
+ return !!(readl(base + IDECTRL) & IDECTRL_IORDY);
+}
+
+/*
+ * According to EP93xx User's Guide, WST field of IDECFG specifies number
+ * of HCLK cycles to hold the data bus after a PIO write operation.
+ * It should be programmed to guarantee following delays:
+ *
+ * PIO Mode [ns]
+ * 0 30
+ * 1 20
+ * 2 15
+ * 3 10
+ * 4 5
+ *
+ * Maximum possible value for HCLK is 100MHz.
+ */
+static int ep93xx_pata_get_wst(int pio_mode)
+{
+ int val;
+
+ if (pio_mode == 0)
+ val = 3;
+ else if (pio_mode < 3)
+ val = 2;
+ else
+ val = 1;
+
+ return val << IDECFG_WST_SHIFT;
+}
+
+static void ep93xx_pata_enable_pio(void __iomem *base, int pio_mode)
+{
+ writel(IDECFG_IDEEN | IDECFG_PIO |
+ ep93xx_pata_get_wst(pio_mode) |
+ (pio_mode << IDECFG_MODE_SHIFT), base + IDECFG);
+}
+
+static void ep93xx_pata_wait_for_iordy(void __iomem *base)
+{
+ unsigned long deadline = jiffies + msecs_to_jiffies(1000);
+ while (!ep93xx_pata_check_iordy(base) &&
+ time_is_before_jiffies(deadline))
+ cpu_relax();
+}
+
+static u16 ep93xx_pata_read(struct ep93xx_pata_data *drv_data,
+ unsigned long addr,
+ const struct ata_timing *t)
+{
+ void __iomem *base = drv_data->ide_base;
+ u16 ret;
+
+ addr &= 0x1f;
+ writel(IDECTRL_DIOWN | IDECTRL_DIORN | addr, base + IDECTRL);
+ ndelay(t->setup);
+ writel(IDECTRL_DIOWN | addr, base + IDECTRL);
+ ndelay(t->act8b);
+ if (drv_data->iordy)
+ ep93xx_pata_wait_for_iordy(base);
+ /*
+ * The IDEDATAIN register is loaded from the DD pins at the positive
+ * edge of the DIORN signal. (EP93xx UG p27-14)
+ */
+ writel(IDECTRL_DIOWN | IDECTRL_DIORN | addr, base + IDECTRL);
+ ret = readl(base + IDEDATAIN);
+ ndelay(t->cyc8b - t->act8b - t->setup);
+ return ret;
+}
+
+static void ep93xx_pata_write(struct ep93xx_pata_data *drv_data,
+ u16 value, unsigned long addr,
+ const struct ata_timing *t)
+{
+ void __iomem *base = drv_data->ide_base;
+
+ addr &= 0x1f;
+ writel(IDECTRL_DIOWN | IDECTRL_DIORN | addr, base + IDECTRL);
+ ndelay(t->setup);
+ /*
+ * Value from IDEDATAOUT register is driven onto the DD pins when
+ * DIOWN is low. (EP93xx UG p27-13)
+ */
+ writel(value, base + IDEDATAOUT);
+ writel(IDECTRL_DIORN | addr, base + IDECTRL);
+ ndelay(t->act8b);
+ if (drv_data->iordy)
+ ep93xx_pata_wait_for_iordy(base);
+ writel(IDECTRL_DIOWN | IDECTRL_DIORN | addr, base + IDECTRL);
+ ndelay(t->cyc8b - t->act8b - t->setup);
+}
+
+static void ep93xx_pata_set_piomode(struct ata_port *ap,
+ struct ata_device *adev)
+{
+ struct ep93xx_pata_data *drv_data = ap->host->private_data;
+ struct ata_device *pair = ata_dev_pair(adev);
+
+ drv_data->t = ata_timing_find_mode(adev->pio_mode);
+ drv_data->cmd_t = drv_data->t;
+ drv_data->iordy = ata_pio_need_iordy(adev);
+
+ if (pair && pair->pio_mode < adev->pio_mode)
+ drv_data->cmd_t = ata_timing_find_mode(pair->pio_mode);
+
+ ep93xx_pata_enable_pio(drv_data->ide_base,
+ adev->pio_mode - XFER_PIO_0);
+}
+
+/* Note: original code is ata_sff_check_status */
+static u8 ep93xx_pata_check_status(struct ata_port *ap)
+{
+ struct ep93xx_pata_data *drv_data = ap->host->private_data;
+
+ return ep93xx_pata_read(drv_data, IDECTRL_ADDR_STATUS,
+ drv_data->cmd_t);
+}
+
+static u8 ep93xx_pata_check_altstatus(struct ata_port *ap)
+{
+ struct ep93xx_pata_data *drv_data = ap->host->private_data;
+
+ return ep93xx_pata_read(drv_data, IDECTRL_ADDR_ALTSTATUS,
+ drv_data->cmd_t);
+}
+
+/* Note: original code is ata_sff_tf_load */
+static void ep93xx_pata_tf_load(struct ata_port *ap,
+ const struct ata_taskfile *tf)
+{
+ struct ep93xx_pata_data *drv_data = ap->host->private_data;
+ const struct ata_timing *t = drv_data->cmd_t;
+ unsigned int is_addr = tf->flags & ATA_TFLAG_ISADDR;
+
+ if (tf->ctl != ap->last_ctl) {
+ ep93xx_pata_write(drv_data, tf->ctl, IDECTRL_ADDR_CTL, t);
+ ap->last_ctl = tf->ctl;
+ ata_wait_idle(ap);
+ }
+
+ if (is_addr && (tf->flags & ATA_TFLAG_LBA48)) {
+ ep93xx_pata_write(drv_data, tf->hob_feature,
+ IDECTRL_ADDR_FEATURE, t);
+ ep93xx_pata_write(drv_data, tf->hob_nsect,
+ IDECTRL_ADDR_NSECT, t);
+ ep93xx_pata_write(drv_data, tf->hob_lbal,
+ IDECTRL_ADDR_LBAL, t);
+ ep93xx_pata_write(drv_data, tf->hob_lbam,
+ IDECTRL_ADDR_LBAM, t);
+ ep93xx_pata_write(drv_data, tf->hob_lbah,
+ IDECTRL_ADDR_LBAH, t);
+ VPRINTK("hob: feat 0x%X nsect 0x%X, lba 0x%X 0x%X 0x%X\n",
+ tf->hob_feature, tf->hob_nsect, tf->hob_lbal,
+ tf->hob_lbam, tf->hob_lbah);
+ }
+
+ if (is_addr) {
+ ep93xx_pata_write(drv_data, tf->feature,
+ IDECTRL_ADDR_FEATURE, t);
+ ep93xx_pata_write(drv_data, tf->nsect, IDECTRL_ADDR_NSECT, t);
+ ep93xx_pata_write(drv_data, tf->lbal, IDECTRL_ADDR_LBAL, t);
+ ep93xx_pata_write(drv_data, tf->lbam, IDECTRL_ADDR_LBAM, t);
+ ep93xx_pata_write(drv_data, tf->lbah, IDECTRL_ADDR_LBAH, t);
+ VPRINTK("feat 0x%X nsect 0x%X lba 0x%X 0x%X 0x%X\n",
+ tf->feature, tf->nsect, tf->lbal, tf->lbam, tf->lbah);
+ }
+
+ if (tf->flags & ATA_TFLAG_DEVICE) {
+ ep93xx_pata_write(drv_data, tf->device, IDECTRL_ADDR_DEVICE, t);
+ VPRINTK("device 0x%X\n", tf->device);
+ }
+
+ ata_wait_idle(ap);
+}
+
+/* Note: original code is ata_sff_tf_read */
+static void ep93xx_pata_tf_read(struct ata_port *ap, struct ata_taskfile *tf)
+{
+ struct ep93xx_pata_data *drv_data = ap->host->private_data;
+ const struct ata_timing *t = drv_data->cmd_t;
+
+ tf->command = ep93xx_pata_check_status(ap);
+ tf->feature = ep93xx_pata_read(drv_data, IDECTRL_ADDR_FEATURE, t);
+ tf->nsect = ep93xx_pata_read(drv_data, IDECTRL_ADDR_NSECT, t);
+ tf->lbal = ep93xx_pata_read(drv_data, IDECTRL_ADDR_LBAL, t);
+ tf->lbam = ep93xx_pata_read(drv_data, IDECTRL_ADDR_LBAM, t);
+ tf->lbah = ep93xx_pata_read(drv_data, IDECTRL_ADDR_LBAH, t);
+ tf->device = ep93xx_pata_read(drv_data, IDECTRL_ADDR_DEVICE, t);
+
+ if (tf->flags & ATA_TFLAG_LBA48) {
+ ep93xx_pata_write(drv_data, tf->ctl | ATA_HOB,
+ IDECTRL_ADDR_CTL, t);
+ tf->hob_feature = ep93xx_pata_read(drv_data,
+ IDECTRL_ADDR_FEATURE, t);
+ tf->hob_nsect = ep93xx_pata_read(drv_data,
+ IDECTRL_ADDR_NSECT, t);
+ tf->hob_lbal = ep93xx_pata_read(drv_data,
+ IDECTRL_ADDR_LBAL, t);
+ tf->hob_lbam = ep93xx_pata_read(drv_data,
+ IDECTRL_ADDR_LBAM, t);
+ tf->hob_lbah = ep93xx_pata_read(drv_data,
+ IDECTRL_ADDR_LBAH, t);
+ ep93xx_pata_write(drv_data, tf->ctl, IDECTRL_ADDR_CTL, t);
+ ap->last_ctl = tf->ctl;
+ }
+}
+
+/* Note: original code is ata_sff_exec_command */
+static void ep93xx_pata_exec_command(struct ata_port *ap,
+ const struct ata_taskfile *tf)
+{
+ struct ep93xx_pata_data *drv_data = ap->host->private_data;
+
+ DPRINTK("ata%u: cmd 0x%X\n", ap->print_id, tf->command);
+
+ ep93xx_pata_write(drv_data, tf->command,
+ IDECTRL_ADDR_COMMAND, drv_data->cmd_t);
+ ata_sff_pause(ap);
+}
+
+/* Note: original code is ata_sff_dev_select */
+static void ep93xx_pata_dev_select(struct ata_port *ap, unsigned int device)
+{
+ u8 tmp = ATA_DEVICE_OBS;
+ struct ep93xx_pata_data *drv_data = ap->host->private_data;
+
+ if (device != 0)
+ tmp |= ATA_DEV1;
+
+ ep93xx_pata_write(drv_data, tmp, IDECTRL_ADDR_DEVICE,
+ drv_data->cmd_t);
+ ata_sff_pause(ap); /* needed; also flushes, for mmio */
+}
+
+/* Note: original code is ata_sff_set_devctl */
+static void ep93xx_pata_set_devctl(struct ata_port *ap, u8 ctl)
+{
+ struct ep93xx_pata_data *drv_data = ap->host->private_data;
+
+ ep93xx_pata_write(drv_data, ctl, IDECTRL_ADDR_CTL,
+ drv_data->cmd_t);
+}
+
+/* Note: original code is ata_sff_data_xfer */
+unsigned int ep93xx_pata_data_xfer(struct ata_device *adev, unsigned char *buf,
+ unsigned int buflen, int rw)
+{
+ struct ata_port *ap = adev->link->ap;
+ struct ep93xx_pata_data *drv_data = ap->host->private_data;
+ const struct ata_timing *t = drv_data->t;
+ u16 *data = (u16 *)buf;
+ unsigned int words = buflen >> 1;
+
+ /* Transfer multiple of 2 bytes */
+ while (words--)
+ if (rw == READ)
+ *data++ = cpu_to_le16(
+ ep93xx_pata_read(
+ drv_data, IDECTRL_ADDR_DATA, t));
+ else
+ ep93xx_pata_write(drv_data, le16_to_cpu(*data++),
+ IDECTRL_ADDR_DATA, t);
+
+ /* Transfer trailing 1 byte, if any. */
+ if (unlikely(buflen & 0x01)) {
+ unsigned char pad[2] = { };
+
+ buf += buflen - 1;
+
+ if (rw == READ) {
+ *pad = cpu_to_le16(
+ ep93xx_pata_read(
+ drv_data, IDECTRL_ADDR_DATA, t));
+ *buf = pad[0];
+ } else {
+ pad[0] = *buf;
+ ep93xx_pata_write(drv_data, le16_to_cpu(*pad),
+ IDECTRL_ADDR_DATA, t);
+ }
+ words++;
+ }
+
+ return words << 1;
+}
+
+/* Note: original code is ata_devchk */
+static bool ep93xx_pata_device_is_present(struct ata_port *ap,
+ unsigned int device)
+{
+ struct ep93xx_pata_data *drv_data = ap->host->private_data;
+ const struct ata_timing *t = drv_data->cmd_t;
+ u8 nsect, lbal;
+
+ ap->ops->sff_dev_select(ap, device);
+
+ ep93xx_pata_write(drv_data, 0x55, IDECTRL_ADDR_NSECT, t);
+ ep93xx_pata_write(drv_data, 0xaa, IDECTRL_ADDR_LBAL, t);
+
+ ep93xx_pata_write(drv_data, 0xaa, IDECTRL_ADDR_NSECT, t);
+ ep93xx_pata_write(drv_data, 0x55, IDECTRL_ADDR_LBAL, t);
+
+ ep93xx_pata_write(drv_data, 0x55, IDECTRL_ADDR_NSECT, t);
+ ep93xx_pata_write(drv_data, 0xaa, IDECTRL_ADDR_LBAL, t);
+
+ nsect = ep93xx_pata_read(drv_data, IDECTRL_ADDR_NSECT, t);
+ lbal = ep93xx_pata_read(drv_data, IDECTRL_ADDR_LBAL, t);
+
+ if ((nsect == 0x55) && (lbal == 0xaa))
+ return true; /* we found a device */
+
+ return false; /* nothing found */
+}
+
+/* Note: original code is ata_sff_wait_after_reset */
+int ep93xx_pata_wait_after_reset(struct ata_link *link, unsigned int devmask,
+ unsigned long deadline)
+{
+ struct ata_port *ap = link->ap;
+ struct ep93xx_pata_data *drv_data = ap->host->private_data;
+ const struct ata_timing *t = drv_data->cmd_t;
+ unsigned int dev0 = devmask & (1 << 0);
+ unsigned int dev1 = devmask & (1 << 1);
+ int rc, ret = 0;
+
+ ata_msleep(ap, ATA_WAIT_AFTER_RESET);
+
+ /* always check readiness of the master device */
+ rc = ata_sff_wait_ready(link, deadline);
+ /* -ENODEV means the odd clown forgot the D7 pulldown resistor
+ * and TF status is 0xff, bail out on it too.
+ */
+ if (rc)
+ return rc;
+
+ /* if device 1 was found in ata_devchk, wait for register
+ * access briefly, then wait for BSY to clear.
+ */
+ if (dev1) {
+ int i;
+
+ ap->ops->sff_dev_select(ap, 1);
+
+ /* Wait for register access. Some ATAPI devices fail
+ * to set nsect/lbal after reset, so don't waste too
+ * much time on it. We're gonna wait for !BSY anyway.
+ */
+ for (i = 0; i < 2; i++) {
+ u8 nsect, lbal;
+
+ nsect = ep93xx_pata_read(drv_data,
+ IDECTRL_ADDR_NSECT, t);
+ lbal = ep93xx_pata_read(drv_data,
+ IDECTRL_ADDR_LBAL, t);
+ if (nsect == 1 && lbal == 1)
+ break;
+ msleep(50); /* give drive a breather */
+ }
+
+ rc = ata_sff_wait_ready(link, deadline);
+ if (rc) {
+ if (rc != -ENODEV)
+ return rc;
+ ret = rc;
+ }
+ }
+ /* is all this really necessary? */
+ ap->ops->sff_dev_select(ap, 0);
+ if (dev1)
+ ap->ops->sff_dev_select(ap, 1);
+ if (dev0)
+ ap->ops->sff_dev_select(ap, 0);
+
+ return ret;
+}
+
+/* Note: original code is ata_bus_softreset */
+static int ep93xx_pata_bus_softreset(struct ata_port *ap, unsigned int devmask,
+ unsigned long deadline)
+{
+ struct ep93xx_pata_data *drv_data = ap->host->private_data;
+ const struct ata_timing *t = drv_data->cmd_t;
+
+ DPRINTK("ata%u: bus reset via SRST\n", ap->print_id);
+
+ ep93xx_pata_write(drv_data, ap->ctl, IDECTRL_ADDR_CTL, t);
+ udelay(20); /* FIXME: flush */
+ ep93xx_pata_write(drv_data, ap->ctl | ATA_SRST, IDECTRL_ADDR_CTL, t);
+ udelay(20); /* FIXME: flush */
+ ep93xx_pata_write(drv_data, ap->ctl, IDECTRL_ADDR_CTL, t);
+ ap->last_ctl = ap->ctl;
+
+ return ep93xx_pata_wait_after_reset(&ap->link, devmask, deadline);
+}
+
+static void ep93xx_pata_release_dma(struct ep93xx_pata_data *drv_data)
+{
+ if (drv_data->dma_rx_channel) {
+ dma_release_channel(drv_data->dma_rx_channel);
+ drv_data->dma_rx_channel = NULL;
+ }
+ if (drv_data->dma_tx_channel) {
+ dma_release_channel(drv_data->dma_tx_channel);
+ drv_data->dma_tx_channel = NULL;
+ }
+}
+
+static bool ep93xx_pata_dma_filter(struct dma_chan *chan, void *filter_param)
+{
+ if (ep93xx_dma_chan_is_m2p(chan))
+ return false;
+
+ chan->private = filter_param;
+ return true;
+}
+
+static void ep93xx_pata_dma_init(struct ep93xx_pata_data *drv_data)
+{
+ const struct platform_device *pdev = drv_data->pdev;
+ dma_cap_mask_t mask;
+ struct dma_slave_config conf;
+
+ dma_cap_zero(mask);
+ dma_cap_set(DMA_SLAVE, mask);
+
+ /*
+ * Request two channels for IDE. Another possibility would be
+ * to request only one channel, and reprogram it's direction at
+ * start of new transfer.
+ */
+ drv_data->dma_rx_data.port = EP93XX_DMA_IDE;
+ drv_data->dma_rx_data.direction = DMA_FROM_DEVICE;
+ drv_data->dma_rx_data.name = "ep93xx-pata-rx";
+ drv_data->dma_rx_channel = dma_request_channel(mask,
+ ep93xx_pata_dma_filter, &drv_data->dma_rx_data);
+ if (!drv_data->dma_rx_channel)
+ return;
+
+ drv_data->dma_tx_data.port = EP93XX_DMA_IDE;
+ drv_data->dma_tx_data.direction = DMA_TO_DEVICE;
+ drv_data->dma_tx_data.name = "ep93xx-pata-tx";
+ drv_data->dma_tx_channel = dma_request_channel(mask,
+ ep93xx_pata_dma_filter, &drv_data->dma_tx_data);
+ if (!drv_data->dma_tx_channel) {
+ dma_release_channel(drv_data->dma_rx_channel);
+ return;
+ }
+
+ /* Configure receive channel direction and source address */
+ memset(&conf, 0, sizeof(conf));
+ conf.direction = DMA_FROM_DEVICE;
+ conf.src_addr = drv_data->udma_in_phys;
+ conf.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+ if (dmaengine_slave_config(drv_data->dma_rx_channel, &conf)) {
+ dev_err(&pdev->dev, "failed to configure rx dma channel\n");
+ ep93xx_pata_release_dma(drv_data);
+ return;
+ }
+
+ /* Configure transmit channel direction and destination address */
+ memset(&conf, 0, sizeof(conf));
+ conf.direction = DMA_TO_DEVICE;
+ conf.dst_addr = drv_data->udma_out_phys;
+ conf.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+ if (dmaengine_slave_config(drv_data->dma_tx_channel, &conf)) {
+ dev_err(&pdev->dev, "failed to configure tx dma channel\n");
+ ep93xx_pata_release_dma(drv_data);
+ }
+}
+
+static void ep93xx_pata_dma_start(struct ata_queued_cmd *qc)
+{
+ struct dma_async_tx_descriptor *txd;
+ struct ep93xx_pata_data *drv_data = qc->ap->host->private_data;
+ void __iomem *base = drv_data->ide_base;
+ struct ata_device *adev = qc->dev;
+ u32 v = qc->dma_dir == DMA_TO_DEVICE ? IDEUDMAOP_RWOP : 0;
+ struct dma_chan *channel = qc->dma_dir == DMA_TO_DEVICE
+ ? drv_data->dma_tx_channel : drv_data->dma_rx_channel;
+
+ txd = channel->device->device_prep_slave_sg(channel, qc->sg,
+ qc->n_elem, qc->dma_dir, DMA_CTRL_ACK, NULL);
+ if (!txd) {
+ dev_err(qc->ap->dev, "failed to prepare slave for sg dma\n");
+ return;
+ }
+ txd->callback = NULL;
+ txd->callback_param = NULL;
+
+ if (dmaengine_submit(txd) < 0) {
+ dev_err(qc->ap->dev, "failed to submit dma transfer\n");
+ return;
+ }
+ dma_async_issue_pending(channel);
+
+ /*
+ * When enabling UDMA operation, IDEUDMAOP register needs to be
+ * programmed in three step sequence:
+ * 1) set or clear the RWOP bit,
+ * 2) perform dummy read of the register,
+ * 3) set the UEN bit.
+ */
+ writel(v, base + IDEUDMAOP);
+ readl(base + IDEUDMAOP);
+ writel(v | IDEUDMAOP_UEN, base + IDEUDMAOP);
+
+ writel(IDECFG_IDEEN | IDECFG_UDMA |
+ ((adev->xfer_mode - XFER_UDMA_0) << IDECFG_MODE_SHIFT),
+ base + IDECFG);
+}
+
+static void ep93xx_pata_dma_stop(struct ata_queued_cmd *qc)
+{
+ struct ep93xx_pata_data *drv_data = qc->ap->host->private_data;
+ void __iomem *base = drv_data->ide_base;
+
+ /* terminate all dma transfers, if not yet finished */
+ dmaengine_terminate_all(drv_data->dma_rx_channel);
+ dmaengine_terminate_all(drv_data->dma_tx_channel);
+
+ /*
+ * To properly stop IDE-DMA, IDEUDMAOP register must to be cleared
+ * and IDECTRL register must be set to default value.
+ */
+ writel(0, base + IDEUDMAOP);
+ writel(readl(base + IDECTRL) | IDECTRL_DIOWN | IDECTRL_DIORN |
+ IDECTRL_CS0N | IDECTRL_CS1N, base + IDECTRL);
+
+ ep93xx_pata_enable_pio(drv_data->ide_base,
+ qc->dev->pio_mode - XFER_PIO_0);
+
+ ata_sff_dma_pause(qc->ap);
+}
+
+static void ep93xx_pata_dma_setup(struct ata_queued_cmd *qc)
+{
+ qc->ap->ops->sff_exec_command(qc->ap, &qc->tf);
+}
+
+static u8 ep93xx_pata_dma_status(struct ata_port *ap)
+{
+ struct ep93xx_pata_data *drv_data = ap->host->private_data;
+ u32 val = readl(drv_data->ide_base + IDEUDMASTS);
+
+ /*
+ * UDMA Status Register bits:
+ *
+ * DMAIDE - DMA request signal from UDMA state machine,
+ * INTIDE - INT line generated by UDMA because of errors in the
+ * state machine,
+ * SBUSY - UDMA state machine busy, not in idle state,
+ * NDO - error for data-out not completed,
+ * NDI - error for data-in not completed,
+ * N4X - error for data transferred not multiplies of four
+ * 32-bit words.
+ * (EP93xx UG p27-17)
+ */
+ if (val & IDEUDMASTS_NDO || val & IDEUDMASTS_NDI ||
+ val & IDEUDMASTS_N4X || val & IDEUDMASTS_INTIDE)
+ return ATA_DMA_ERR;
+
+ /* read INTRQ (INT[3]) pin input state */
+ if (readl(drv_data->ide_base + IDECTRL) & IDECTRL_INTRQ)
+ return ATA_DMA_INTR;
+
+ if (val & IDEUDMASTS_SBUSY || val & IDEUDMASTS_DMAIDE)
+ return ATA_DMA_ACTIVE;
+
+ return 0;
+}
+
+/* Note: original code is ata_sff_softreset */
+static int ep93xx_pata_softreset(struct ata_link *al, unsigned int *classes,
+ unsigned long deadline)
+{
+ struct ata_port *ap = al->ap;
+ unsigned int slave_possible = ap->flags & ATA_FLAG_SLAVE_POSS;
+ unsigned int devmask = 0;
+ int rc;
+ u8 err;
+
+ DPRINTK("ENTER\n");
+
+ /* determine if device 0/1 are present */
+ if (ep93xx_pata_device_is_present(ap, 0))
+ devmask |= (1 << 0);
+ if (slave_possible && ep93xx_pata_device_is_present(ap, 1))
+ devmask |= (1 << 1);
+
+ /* select device 0 again */
+ ap->ops->sff_dev_select(al->ap, 0);
+
+ /* issue bus reset */
+ DPRINTK("about to softreset, devmask=%x\n", devmask);
+ rc = ep93xx_pata_bus_softreset(ap, devmask, deadline);
+ /* if link is ocuppied, -ENODEV too is an error */
+ if (rc && (rc != -ENODEV || sata_scr_valid(al))) {
+ ata_link_printk(al, KERN_ERR, "SRST failed (errno=%d)\n",
+ rc);
+ return rc;
+ }
+
+ /* determine by signature whether we have ATA or ATAPI devices */
+ classes[0] = ata_sff_dev_classify(&al->device[0], devmask & (1 << 0),
+ &err);
+ if (slave_possible && err != 0x81)
+ classes[1] = ata_sff_dev_classify(&al->device[1],
+ devmask & (1 << 1), &err);
+
+ DPRINTK("EXIT, classes[0]=%u, [1]=%u\n", classes[0],
+ classes[1]);
+ return 0;
+}
+
+/* Note: original code is ata_sff_drain_fifo */
+void ep93xx_pata_drain_fifo(struct ata_queued_cmd *qc)
+{
+ int count;
+ struct ata_port *ap;
+ struct ep93xx_pata_data *drv_data;
+
+ /* We only need to flush incoming data when a command was running */
+ if (qc == NULL || qc->dma_dir == DMA_TO_DEVICE)
+ return;
+
+ ap = qc->ap;
+ drv_data = ap->host->private_data;
+ /* Drain up to 64K of data before we give up this recovery method */
+ for (count = 0; (ap->ops->sff_check_status(ap) & ATA_DRQ)
+ && count < 65536; count += 2)
+ ep93xx_pata_read(drv_data, IDECTRL_ADDR_DATA,
+ drv_data->cmd_t);
+
+ /* Can become DEBUG later */
+ if (count)
+ ata_port_printk(ap, KERN_DEBUG,
+ "drained %d bytes to clear DRQ.\n", count);
+
+}
+
+static int ep93xx_pata_port_start(struct ata_port *ap)
+{
+ struct ep93xx_pata_data *drv_data = ap->host->private_data;
+
+ drv_data->cmd_t = ata_timing_find_mode(XFER_PIO_0);
+ return 0;
+}
+
+static struct scsi_host_template ep93xx_pata_sht = {
+ ATA_BASE_SHT(DRV_NAME),
+ /* ep93xx dma implementation limit */
+ .sg_tablesize = 32,
+ /* ep93xx dma can't transfer 65536 bytes at once */
+ .dma_boundary = 0x7fff,
+};
+
+static struct ata_port_operations ep93xx_pata_port_ops = {
+ .inherits = &ata_bmdma_port_ops,
+
+ .qc_prep = ata_noop_qc_prep,
+
+ .softreset = ep93xx_pata_softreset,
+ .hardreset = ATA_OP_NULL,
+
+ .sff_dev_select = ep93xx_pata_dev_select,
+ .sff_set_devctl = ep93xx_pata_set_devctl,
+ .sff_check_status = ep93xx_pata_check_status,
+ .sff_check_altstatus = ep93xx_pata_check_altstatus,
+ .sff_tf_load = ep93xx_pata_tf_load,
+ .sff_tf_read = ep93xx_pata_tf_read,
+ .sff_exec_command = ep93xx_pata_exec_command,
+ .sff_data_xfer = ep93xx_pata_data_xfer,
+ .sff_drain_fifo = ep93xx_pata_drain_fifo,
+ .sff_irq_clear = ATA_OP_NULL,
+
+ .set_piomode = ep93xx_pata_set_piomode,
+
+ .bmdma_setup = ep93xx_pata_dma_setup,
+ .bmdma_start = ep93xx_pata_dma_start,
+ .bmdma_stop = ep93xx_pata_dma_stop,
+ .bmdma_status = ep93xx_pata_dma_status,
+
+ .cable_detect = ata_cable_unknown,
+ .port_start = ep93xx_pata_port_start,
+};
+
+static int __devinit ep93xx_pata_probe(struct platform_device *pdev)
+{
+ struct ep93xx_pata_data *drv_data;
+ struct ata_host *host;
+ struct ata_port *ap;
+ unsigned int irq;
+ struct resource *mem_res;
+ void __iomem *ide_base;
+ int err;
+
+ err = ep93xx_ide_acquire_gpio(pdev);
+ if (err)
+ return err;
+
+ /* INT[3] (IRQ_EP93XX_EXT3) line connected as pull down */
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ err = -ENXIO;
+ goto err_rel_gpio;
+ }
+
+ mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!mem_res) {
+ err = -ENXIO;
+ goto err_rel_gpio;
+ }
+
+ ide_base = devm_request_and_ioremap(&pdev->dev, mem_res);
+ if (!ide_base) {
+ err = -ENXIO;
+ goto err_rel_gpio;
+ }
+
+ drv_data = devm_kzalloc(&pdev->dev, sizeof(*drv_data), GFP_KERNEL);
+ if (!drv_data) {
+ err = -ENXIO;
+ goto err_rel_gpio;
+ }
+
+ platform_set_drvdata(pdev, drv_data);
+ drv_data->pdev = pdev;
+ drv_data->ide_base = ide_base;
+ drv_data->udma_in_phys = mem_res->start + IDEUDMADATAIN;
+ drv_data->udma_out_phys = mem_res->start + IDEUDMADATAOUT;
+ ep93xx_pata_dma_init(drv_data);
+
+ /* allocate host */
+ host = ata_host_alloc(&pdev->dev, 1);
+ if (!host) {
+ err = -ENXIO;
+ goto err_rel_dma;
+ }
+
+ ep93xx_pata_clear_regs(ide_base);
+
+ host->n_ports = 1;
+ host->private_data = drv_data;
+
+ ap = host->ports[0];
+ ap->dev = &pdev->dev;
+ ap->ops = &ep93xx_pata_port_ops;
+ ap->flags |= ATA_FLAG_SLAVE_POSS;
+ ap->pio_mask = ATA_PIO4;
+
+ /*
+ * Maximum UDMA modes:
+ * EP931x rev.E0 - UDMA2
+ * EP931x rev.E1 - UDMA3
+ * EP931x rev.E2 - UDMA4
+ *
+ * MWDMA support was removed from EP931x rev.E2,
+ * so this driver supports only UDMA modes.
+ */
+ if (drv_data->dma_rx_channel && drv_data->dma_tx_channel) {
+ int chip_rev = ep93xx_chip_revision();
+
+ if (chip_rev == EP93XX_CHIP_REV_E1)
+ ap->udma_mask = ATA_UDMA3;
+ else if (chip_rev == EP93XX_CHIP_REV_E2)
+ ap->udma_mask = ATA_UDMA4;
+ else
+ ap->udma_mask = ATA_UDMA2;
+ }
+
+ /* defaults, pio 0 */
+ ep93xx_pata_enable_pio(ide_base, 0);
+
+ dev_info(&pdev->dev, "version " DRV_VERSION "\n");
+
+ /* activate host */
+ err = ata_host_activate(host, irq, ata_bmdma_interrupt, 0,
+ &ep93xx_pata_sht);
+ if (err == 0)
+ return 0;
+
+err_rel_dma:
+ ep93xx_pata_release_dma(drv_data);
+err_rel_gpio:
+ ep93xx_ide_release_gpio(pdev);
+ return err;
+}
+
+static int __devexit ep93xx_pata_remove(struct platform_device *pdev)
+{
+ struct ata_host *host = platform_get_drvdata(pdev);
+ struct ep93xx_pata_data *drv_data = host->private_data;
+
+ ata_host_detach(host);
+ ep93xx_pata_release_dma(drv_data);
+ ep93xx_pata_clear_regs(drv_data->ide_base);
+ ep93xx_ide_release_gpio(pdev);
+ return 0;
+}
+
+static struct platform_driver ep93xx_pata_platform_driver = {
+ .driver = {
+ .name = DRV_NAME,
+ .owner = THIS_MODULE,
+ },
+ .probe = ep93xx_pata_probe,
+ .remove = __devexit_p(ep93xx_pata_remove),
+};
+
+module_platform_driver(ep93xx_pata_platform_driver);
+
+MODULE_AUTHOR("Alessandro Zummo, Lennert Buytenhek, Joao Ramos, "
+ "Bartlomiej Zolnierkiewicz, Rafal Prylowski");
+MODULE_DESCRIPTION("low-level driver for cirrus ep93xx IDE controller");
+MODULE_LICENSE("GPL");
+MODULE_VERSION(DRV_VERSION);
+MODULE_ALIAS("platform:pata_ep93xx");
Index: linux-2.6/drivers/ata/Kconfig
===================================================================
--- linux-2.6.orig/drivers/ata/Kconfig
+++ linux-2.6/drivers/ata/Kconfig
@@ -416,6 +416,15 @@ config PATA_EFAR
If unsure, say N.
+config PATA_EP93XX
+ tristate "Cirrus Logic EP93xx PATA support"
+ depends on ARCH_EP93XX
+ help
+ This option enables support for the PATA controller in
+ the Cirrus Logic EP9312 and EP9315 ARM CPU.
+
+ If unsure, say N.
+
config PATA_HPT366
tristate "HPT 366/368 PATA support"
depends on PCI
Index: linux-2.6/drivers/ata/Makefile
===================================================================
--- linux-2.6.orig/drivers/ata/Makefile
+++ linux-2.6/drivers/ata/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_PATA_CS5535) += pata_cs5535
obj-$(CONFIG_PATA_CS5536) += pata_cs5536.o
obj-$(CONFIG_PATA_CYPRESS) += pata_cypress.o
obj-$(CONFIG_PATA_EFAR) += pata_efar.o
+obj-$(CONFIG_PATA_EP93XX) += pata_ep93xx.o
obj-$(CONFIG_PATA_HPT366) += pata_hpt366.o
obj-$(CONFIG_PATA_HPT37X) += pata_hpt37x.o
obj-$(CONFIG_PATA_HPT3X2N) += pata_hpt3x2n.o
^ permalink raw reply [flat|nested] 51+ messages in thread* RE: [PATCH v2 1/3] PATA host controller driver for ep93xx
2012-04-03 14:45 ` Rafal Prylowski
@ 2012-04-03 18:25 ` H Hartley Sweeten
-1 siblings, 0 replies; 51+ messages in thread
From: H Hartley Sweeten @ 2012-04-03 18:25 UTC (permalink / raw)
To: Rafal Prylowski, linux-ide@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org, joao.ramos@inov.pt,
rmallon@gmail.com, Sergei Shtylyov, bzolnier@gmail.com
On Tuesday, April 03, 2012 7:45 AM, Rafal Prylowski wrote:
Rafal,
Just a quick review. I'll look over the driver more closely later.
>
> Signed-off-by: Rafal Prylowski <prylowski@metasoft.pl>
> Cc: Joao Ramos <joao.ramos@inov.pt>
> Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
> Cc: Ryan Mallon <rmallon@gmail.com>
> Cc: Sergei Shtylyov <sshtylyov@mvista.com>
> Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Again, there should be a commit message.
> ---
> drivers/ata/Kconfig | 9
> drivers/ata/Makefile | 1
> drivers/ata/pata_ep93xx.c | 976 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 986 insertions(+)
>
> Index: linux-2.6/drivers/ata/pata_ep93xx.c
> ===================================================================
> --- /dev/null
> +++ linux-2.6/drivers/ata/pata_ep93xx.c
> @@ -0,0 +1,976 @@
> +/*
> + * EP93XX PATA controller driver.
> + *
> + * Copyright (c) 2012, Metasoft s.c.
> + * Rafal Prylowski <prylowski@metasoft.pl>
> + *
> + * Based on pata_scc.c, pata_icside.c and on earlier version of EP93XX
> + * PATA driver by Lennert Buytenhek and Alessandro Zummo.
> + * Read/Write timings, resource management and other improvements
> + * from driver by Joao Ramos and Bartlomiej Zolnierkiewicz.
> + * DMA engine support based on spi-ep93xx.c by Mika Westerberg.
> + *
> + * Original copyrights:
> + *
> + * Support for Cirrus Logic's EP93xx (EP9312, EP9315) CPUs
> + * PATA host controller driver.
> + *
> + * Copyright (c) 2009, Bartlomiej Zolnierkiewicz
> + *
> + * Heavily based on the ep93xx-ide.c driver:
> + *
> + * Copyright (c) 2009, Joao Ramos <joao.ramos@inov.pt>
> + * INESC Inovacao (INOV)
> + *
> + * EP93XX PATA controller driver.
> + * Copyright (C) 2007 Lennert Buytenhek <buytenh@wantstofly.org>
> + *
> + * An ATA driver for the Cirrus Logic EP93xx PATA controller.
> + *
> + * Based on an earlier version by Alessandro Zummo, which is:
> + * Copyright (C) 2006 Tower Technologies
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/blkdev.h>
> +#include <scsi/scsi_host.h>
> +#include <linux/ata.h>
> +#include <linux/libata.h>
> +#include <linux/platform_device.h>
> +#include <linux/delay.h>
> +#include <linux/dmaengine.h>
> +
> +#include <mach/dma.h>
> +#include <mach/platform.h>
> +
> +#define DRV_NAME "ep93xx-ide"
> +#define DRV_VERSION "1.0"
> +
> +enum {
> + /* IDE Control Register */
> + IDECTRL = 0x00,
> + IDECTRL_CS0N = (1 << 0),
> + IDECTRL_CS1N = (1 << 1),
> + IDECTRL_DIORN = (1 << 5),
> + IDECTRL_DIOWN = (1 << 6),
> + IDECTRL_INTRQ = (1 << 9),
> + IDECTRL_IORDY = (1 << 10),
> + /*
> + * the device IDE register to be accessed is selected through
> + * IDECTRL register's specific bitfields 'DA', 'CS1N' and 'CS0N':
> + * b4 b3 b2 b1 b0
> + * A2 A1 A0 CS1N CS0N
> + * the values filled in this structure allows the value to be directly
> + * ORed to the IDECTRL register, hence giving directly the A[2:0] and
> + * CS1N/CS0N values for each IDE register.
> + * The values correspond to the transformation:
> + * ((real IDE address) << 2) | CS1N value << 1 | CS0N value
> + */
> + IDECTRL_ADDR_CMD = 0 + 2, /* CS1 */
> + IDECTRL_ADDR_DATA = (ATA_REG_DATA << 2) + 2,
> + IDECTRL_ADDR_ERROR = (ATA_REG_ERR << 2) + 2,
> + IDECTRL_ADDR_FEATURE = (ATA_REG_FEATURE << 2) + 2,
> + IDECTRL_ADDR_NSECT = (ATA_REG_NSECT << 2) + 2,
> + IDECTRL_ADDR_LBAL = (ATA_REG_LBAL << 2) + 2,
> + IDECTRL_ADDR_LBAM = (ATA_REG_LBAM << 2) + 2,
> + IDECTRL_ADDR_LBAH = (ATA_REG_LBAH << 2) + 2,
> + IDECTRL_ADDR_DEVICE = (ATA_REG_DEVICE << 2) + 2,
> + IDECTRL_ADDR_STATUS = (ATA_REG_STATUS << 2) + 2,
> + IDECTRL_ADDR_COMMAND = (ATA_REG_CMD << 2) + 2,
> + IDECTRL_ADDR_ALTSTATUS = (0x06 << 2) + 1, /* CS0 */
> + IDECTRL_ADDR_CTL = (0x06 << 2) + 1, /* CS0 */
> +
> + /* IDE Configuration Register */
> + IDECFG = 0x04,
> + IDECFG_IDEEN = (1 << 0),
> + IDECFG_PIO = (1 << 1),
> + IDECFG_MDMA = (1 << 2),
> + IDECFG_UDMA = (1 << 3),
> + IDECFG_MODE_SHIFT = 4,
> + IDECFG_MODE_MASK = (0xf << 4),
> + IDECFG_WST_SHIFT = 8,
> + IDECFG_WST_MASK = (0x3 << 8),
> +
> + /* MDMA Operation Register */
> + IDEMDMAOP = 0x08,
> +
> + /* UDMA Operation Register */
> + IDEUDMAOP = 0x0c,
> + IDEUDMAOP_UEN = (1 << 0),
> + IDEUDMAOP_RWOP = (1 << 1),
> +
> + /* PIO/MDMA/UDMA Data Registers */
> + IDEDATAOUT = 0x10,
> + IDEDATAIN = 0x14,
> + IDEMDMADATAOUT = 0x18,
> + IDEMDMADATAIN = 0x1c,
> + IDEUDMADATAOUT = 0x20,
> + IDEUDMADATAIN = 0x24,
> +
> + /* UDMA Status Register */
> + IDEUDMASTS = 0x28,
> + IDEUDMASTS_DMAIDE = (1 << 16),
> + IDEUDMASTS_INTIDE = (1 << 17),
> + IDEUDMASTS_SBUSY = (1 << 18),
> + IDEUDMASTS_NDO = (1 << 24),
> + IDEUDMASTS_NDI = (1 << 25),
> + IDEUDMASTS_N4X = (1 << 26),
> +
> + /* UDMA Debug Status Register */
> + IDEUDMADEBUG = 0x2c,
> +};
I prefer defines for these. But, the enum usage seems common in the ata drivers
so...
> +
> +struct ep93xx_pata_data /* private_data in ata_host */
> +{
Please move the brace up to the previous line.
Also, I don't think the comment is necessary.
> + const struct platform_device *pdev;
> + void __iomem *ide_base;
> + const struct ata_timing *t;
> + const struct ata_timing *cmd_t;
> + bool iordy;
> +
> + unsigned long udma_in_phys;
> + unsigned long udma_out_phys;
> +
> + struct dma_chan *dma_rx_channel;
> + struct ep93xx_dma_data dma_rx_data;
> + struct dma_chan *dma_tx_channel;
> + struct ep93xx_dma_data dma_tx_data;
> +};
> +
> +static void ep93xx_pata_clear_regs(void __iomem *base)
> +{
> + writel(IDECTRL_CS0N | IDECTRL_CS1N | IDECTRL_DIORN |
> + IDECTRL_DIOWN, base + IDECTRL);
> +
> + writel(0, base + IDECFG);
> + writel(0, base + IDEMDMAOP);
> + writel(0, base + IDEUDMAOP);
> + writel(0, base + IDEDATAOUT);
> + writel(0, base + IDEDATAIN);
> + writel(0, base + IDEMDMADATAOUT);
> + writel(0, base + IDEMDMADATAIN);
> + writel(0, base + IDEUDMADATAOUT);
> + writel(0, base + IDEUDMADATAIN);
> + writel(0, base + IDEUDMADEBUG);
> +}
> +
> +static int ep93xx_pata_check_iordy(void __iomem *base)
> +{
> + return !!(readl(base + IDECTRL) & IDECTRL_IORDY);
> +}
> +
> +/*
> + * According to EP93xx User's Guide, WST field of IDECFG specifies number
> + * of HCLK cycles to hold the data bus after a PIO write operation.
> + * It should be programmed to guarantee following delays:
> + *
> + * PIO Mode [ns]
> + * 0 30
> + * 1 20
> + * 2 15
> + * 3 10
> + * 4 5
> + *
> + * Maximum possible value for HCLK is 100MHz.
> + */
> +static int ep93xx_pata_get_wst(int pio_mode)
> +{
> + int val;
> +
> + if (pio_mode == 0)
> + val = 3;
> + else if (pio_mode < 3)
> + val = 2;
> + else
> + val = 1;
> +
> + return val << IDECFG_WST_SHIFT;
> +}
> +
> +static void ep93xx_pata_enable_pio(void __iomem *base, int pio_mode)
> +{
> + writel(IDECFG_IDEEN | IDECFG_PIO |
> + ep93xx_pata_get_wst(pio_mode) |
> + (pio_mode << IDECFG_MODE_SHIFT), base + IDECFG);
> +}
> +
> +static void ep93xx_pata_wait_for_iordy(void __iomem *base)
> +{
> + unsigned long deadline = jiffies + msecs_to_jiffies(1000);
Please add a white space here.
> + while (!ep93xx_pata_check_iordy(base) &&
> + time_is_before_jiffies(deadline))
> + cpu_relax();
> +}
> +
> +static u16 ep93xx_pata_read(struct ep93xx_pata_data *drv_data,
> + unsigned long addr,
> + const struct ata_timing *t)
> +{
> + void __iomem *base = drv_data->ide_base;
> + u16 ret;
> +
> + addr &= 0x1f;
> + writel(IDECTRL_DIOWN | IDECTRL_DIORN | addr, base + IDECTRL);
> + ndelay(t->setup);
> + writel(IDECTRL_DIOWN | addr, base + IDECTRL);
> + ndelay(t->act8b);
> + if (drv_data->iordy)
> + ep93xx_pata_wait_for_iordy(base);
> + /*
> + * The IDEDATAIN register is loaded from the DD pins at the positive
> + * edge of the DIORN signal. (EP93xx UG p27-14)
> + */
> + writel(IDECTRL_DIOWN | IDECTRL_DIORN | addr, base + IDECTRL);
> + ret = readl(base + IDEDATAIN);
> + ndelay(t->cyc8b - t->act8b - t->setup);
> + return ret;
> +}
> +
> +static void ep93xx_pata_write(struct ep93xx_pata_data *drv_data,
> + u16 value, unsigned long addr,
> + const struct ata_timing *t)
> +{
> + void __iomem *base = drv_data->ide_base;
> +
> + addr &= 0x1f;
> + writel(IDECTRL_DIOWN | IDECTRL_DIORN | addr, base + IDECTRL);
> + ndelay(t->setup);
> + /*
> + * Value from IDEDATAOUT register is driven onto the DD pins when
> + * DIOWN is low. (EP93xx UG p27-13)
> + */
> + writel(value, base + IDEDATAOUT);
> + writel(IDECTRL_DIORN | addr, base + IDECTRL);
> + ndelay(t->act8b);
> + if (drv_data->iordy)
> + ep93xx_pata_wait_for_iordy(base);
> + writel(IDECTRL_DIOWN | IDECTRL_DIORN | addr, base + IDECTRL);
> + ndelay(t->cyc8b - t->act8b - t->setup);
> +}
> +
> +static void ep93xx_pata_set_piomode(struct ata_port *ap,
> + struct ata_device *adev)
> +{
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> + struct ata_device *pair = ata_dev_pair(adev);
> +
> + drv_data->t = ata_timing_find_mode(adev->pio_mode);
> + drv_data->cmd_t = drv_data->t;
> + drv_data->iordy = ata_pio_need_iordy(adev);
> +
> + if (pair && pair->pio_mode < adev->pio_mode)
> + drv_data->cmd_t = ata_timing_find_mode(pair->pio_mode);
> +
> + ep93xx_pata_enable_pio(drv_data->ide_base,
> + adev->pio_mode - XFER_PIO_0);
> +}
> +
> +/* Note: original code is ata_sff_check_status */
> +static u8 ep93xx_pata_check_status(struct ata_port *ap)
> +{
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> +
> + return ep93xx_pata_read(drv_data, IDECTRL_ADDR_STATUS,
> + drv_data->cmd_t);
> +}
> +
> +static u8 ep93xx_pata_check_altstatus(struct ata_port *ap)
> +{
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> +
> + return ep93xx_pata_read(drv_data, IDECTRL_ADDR_ALTSTATUS,
> + drv_data->cmd_t);
> +}
> +
> +/* Note: original code is ata_sff_tf_load */
> +static void ep93xx_pata_tf_load(struct ata_port *ap,
> + const struct ata_taskfile *tf)
> +{
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> + const struct ata_timing *t = drv_data->cmd_t;
> + unsigned int is_addr = tf->flags & ATA_TFLAG_ISADDR;
> +
> + if (tf->ctl != ap->last_ctl) {
> + ep93xx_pata_write(drv_data, tf->ctl, IDECTRL_ADDR_CTL, t);
> + ap->last_ctl = tf->ctl;
> + ata_wait_idle(ap);
> + }
> +
> + if (is_addr && (tf->flags & ATA_TFLAG_LBA48)) {
> + ep93xx_pata_write(drv_data, tf->hob_feature,
> + IDECTRL_ADDR_FEATURE, t);
> + ep93xx_pata_write(drv_data, tf->hob_nsect,
> + IDECTRL_ADDR_NSECT, t);
> + ep93xx_pata_write(drv_data, tf->hob_lbal,
> + IDECTRL_ADDR_LBAL, t);
> + ep93xx_pata_write(drv_data, tf->hob_lbam,
> + IDECTRL_ADDR_LBAM, t);
> + ep93xx_pata_write(drv_data, tf->hob_lbah,
> + IDECTRL_ADDR_LBAH, t);
> + VPRINTK("hob: feat 0x%X nsect 0x%X, lba 0x%X 0x%X 0x%X\n",
> + tf->hob_feature, tf->hob_nsect, tf->hob_lbal,
> + tf->hob_lbam, tf->hob_lbah);
> + }
> +
> + if (is_addr) {
> + ep93xx_pata_write(drv_data, tf->feature,
> + IDECTRL_ADDR_FEATURE, t);
> + ep93xx_pata_write(drv_data, tf->nsect, IDECTRL_ADDR_NSECT, t);
> + ep93xx_pata_write(drv_data, tf->lbal, IDECTRL_ADDR_LBAL, t);
> + ep93xx_pata_write(drv_data, tf->lbam, IDECTRL_ADDR_LBAM, t);
> + ep93xx_pata_write(drv_data, tf->lbah, IDECTRL_ADDR_LBAH, t);
> + VPRINTK("feat 0x%X nsect 0x%X lba 0x%X 0x%X 0x%X\n",
> + tf->feature, tf->nsect, tf->lbal, tf->lbam, tf->lbah);
> + }
> +
> + if (tf->flags & ATA_TFLAG_DEVICE) {
> + ep93xx_pata_write(drv_data, tf->device, IDECTRL_ADDR_DEVICE, t);
> + VPRINTK("device 0x%X\n", tf->device);
> + }
> +
> + ata_wait_idle(ap);
> +}
> +
> +/* Note: original code is ata_sff_tf_read */
> +static void ep93xx_pata_tf_read(struct ata_port *ap, struct ata_taskfile *tf)
> +{
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> + const struct ata_timing *t = drv_data->cmd_t;
> +
> + tf->command = ep93xx_pata_check_status(ap);
> + tf->feature = ep93xx_pata_read(drv_data, IDECTRL_ADDR_FEATURE, t);
> + tf->nsect = ep93xx_pata_read(drv_data, IDECTRL_ADDR_NSECT, t);
> + tf->lbal = ep93xx_pata_read(drv_data, IDECTRL_ADDR_LBAL, t);
> + tf->lbam = ep93xx_pata_read(drv_data, IDECTRL_ADDR_LBAM, t);
> + tf->lbah = ep93xx_pata_read(drv_data, IDECTRL_ADDR_LBAH, t);
> + tf->device = ep93xx_pata_read(drv_data, IDECTRL_ADDR_DEVICE, t);
> +
> + if (tf->flags & ATA_TFLAG_LBA48) {
> + ep93xx_pata_write(drv_data, tf->ctl | ATA_HOB,
> + IDECTRL_ADDR_CTL, t);
> + tf->hob_feature = ep93xx_pata_read(drv_data,
> + IDECTRL_ADDR_FEATURE, t);
> + tf->hob_nsect = ep93xx_pata_read(drv_data,
> + IDECTRL_ADDR_NSECT, t);
> + tf->hob_lbal = ep93xx_pata_read(drv_data,
> + IDECTRL_ADDR_LBAL, t);
> + tf->hob_lbam = ep93xx_pata_read(drv_data,
> + IDECTRL_ADDR_LBAM, t);
> + tf->hob_lbah = ep93xx_pata_read(drv_data,
> + IDECTRL_ADDR_LBAH, t);
> + ep93xx_pata_write(drv_data, tf->ctl, IDECTRL_ADDR_CTL, t);
> + ap->last_ctl = tf->ctl;
> + }
> +}
> +
> +/* Note: original code is ata_sff_exec_command */
> +static void ep93xx_pata_exec_command(struct ata_port *ap,
> + const struct ata_taskfile *tf)
> +{
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> +
> + DPRINTK("ata%u: cmd 0x%X\n", ap->print_id, tf->command);
> +
> + ep93xx_pata_write(drv_data, tf->command,
> + IDECTRL_ADDR_COMMAND, drv_data->cmd_t);
> + ata_sff_pause(ap);
> +}
> +
> +/* Note: original code is ata_sff_dev_select */
> +static void ep93xx_pata_dev_select(struct ata_port *ap, unsigned int device)
> +{
> + u8 tmp = ATA_DEVICE_OBS;
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
Nitpick... Please put the struct ep93xx_pata_data line before the u8 line.
> +
> + if (device != 0)
> + tmp |= ATA_DEV1;
> +
> + ep93xx_pata_write(drv_data, tmp, IDECTRL_ADDR_DEVICE,
> + drv_data->cmd_t);
> + ata_sff_pause(ap); /* needed; also flushes, for mmio */
> +}
> +
> +/* Note: original code is ata_sff_set_devctl */
> +static void ep93xx_pata_set_devctl(struct ata_port *ap, u8 ctl)
> +{
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> +
> + ep93xx_pata_write(drv_data, ctl, IDECTRL_ADDR_CTL,
> + drv_data->cmd_t);
> +}
> +
> +/* Note: original code is ata_sff_data_xfer */
> +unsigned int ep93xx_pata_data_xfer(struct ata_device *adev, unsigned char *buf,
> + unsigned int buflen, int rw)
> +{
> + struct ata_port *ap = adev->link->ap;
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> + const struct ata_timing *t = drv_data->t;
> + u16 *data = (u16 *)buf;
> + unsigned int words = buflen >> 1;
> +
> + /* Transfer multiple of 2 bytes */
> + while (words--)
> + if (rw == READ)
> + *data++ = cpu_to_le16(
> + ep93xx_pata_read(
> + drv_data, IDECTRL_ADDR_DATA, t));
> + else
> + ep93xx_pata_write(drv_data, le16_to_cpu(*data++),
> + IDECTRL_ADDR_DATA, t);
> +
> + /* Transfer trailing 1 byte, if any. */
> + if (unlikely(buflen & 0x01)) {
> + unsigned char pad[2] = { };
> +
> + buf += buflen - 1;
> +
> + if (rw == READ) {
> + *pad = cpu_to_le16(
> + ep93xx_pata_read(
> + drv_data, IDECTRL_ADDR_DATA, t));
> + *buf = pad[0];
> + } else {
> + pad[0] = *buf;
> + ep93xx_pata_write(drv_data, le16_to_cpu(*pad),
> + IDECTRL_ADDR_DATA, t);
> + }
> + words++;
> + }
> +
> + return words << 1;
> +}
> +
> +/* Note: original code is ata_devchk */
> +static bool ep93xx_pata_device_is_present(struct ata_port *ap,
> + unsigned int device)
> +{
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> + const struct ata_timing *t = drv_data->cmd_t;
> + u8 nsect, lbal;
> +
> + ap->ops->sff_dev_select(ap, device);
> +
> + ep93xx_pata_write(drv_data, 0x55, IDECTRL_ADDR_NSECT, t);
> + ep93xx_pata_write(drv_data, 0xaa, IDECTRL_ADDR_LBAL, t);
> +
> + ep93xx_pata_write(drv_data, 0xaa, IDECTRL_ADDR_NSECT, t);
> + ep93xx_pata_write(drv_data, 0x55, IDECTRL_ADDR_LBAL, t);
> +
> + ep93xx_pata_write(drv_data, 0x55, IDECTRL_ADDR_NSECT, t);
> + ep93xx_pata_write(drv_data, 0xaa, IDECTRL_ADDR_LBAL, t);
> +
> + nsect = ep93xx_pata_read(drv_data, IDECTRL_ADDR_NSECT, t);
> + lbal = ep93xx_pata_read(drv_data, IDECTRL_ADDR_LBAL, t);
> +
> + if ((nsect == 0x55) && (lbal == 0xaa))
> + return true; /* we found a device */
> +
> + return false; /* nothing found */
Nitpick... The comments are not needed.
> +}
> +
> +/* Note: original code is ata_sff_wait_after_reset */
> +int ep93xx_pata_wait_after_reset(struct ata_link *link, unsigned int devmask,
> + unsigned long deadline)
> +{
> + struct ata_port *ap = link->ap;
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> + const struct ata_timing *t = drv_data->cmd_t;
> + unsigned int dev0 = devmask & (1 << 0);
> + unsigned int dev1 = devmask & (1 << 1);
> + int rc, ret = 0;
> +
> + ata_msleep(ap, ATA_WAIT_AFTER_RESET);
> +
> + /* always check readiness of the master device */
> + rc = ata_sff_wait_ready(link, deadline);
> + /* -ENODEV means the odd clown forgot the D7 pulldown resistor
> + * and TF status is 0xff, bail out on it too.
> + */
Multiline comments, here and below, should be:
/*
* Comment...
*/
> + if (rc)
> + return rc;
> +
> + /* if device 1 was found in ata_devchk, wait for register
> + * access briefly, then wait for BSY to clear.
> + */
> + if (dev1) {
> + int i;
> +
> + ap->ops->sff_dev_select(ap, 1);
> +
> + /* Wait for register access. Some ATAPI devices fail
> + * to set nsect/lbal after reset, so don't waste too
> + * much time on it. We're gonna wait for !BSY anyway.
> + */
> + for (i = 0; i < 2; i++) {
> + u8 nsect, lbal;
> +
> + nsect = ep93xx_pata_read(drv_data,
> + IDECTRL_ADDR_NSECT, t);
> + lbal = ep93xx_pata_read(drv_data,
> + IDECTRL_ADDR_LBAL, t);
> + if (nsect == 1 && lbal == 1)
> + break;
> + msleep(50); /* give drive a breather */
> + }
> +
> + rc = ata_sff_wait_ready(link, deadline);
> + if (rc) {
> + if (rc != -ENODEV)
> + return rc;
> + ret = rc;
> + }
> + }
> + /* is all this really necessary? */
> + ap->ops->sff_dev_select(ap, 0);
> + if (dev1)
> + ap->ops->sff_dev_select(ap, 1);
> + if (dev0)
> + ap->ops->sff_dev_select(ap, 0);
> +
> + return ret;
> +}
> +
> +/* Note: original code is ata_bus_softreset */
> +static int ep93xx_pata_bus_softreset(struct ata_port *ap, unsigned int devmask,
> + unsigned long deadline)
> +{
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> + const struct ata_timing *t = drv_data->cmd_t;
> +
> + DPRINTK("ata%u: bus reset via SRST\n", ap->print_id);
> +
> + ep93xx_pata_write(drv_data, ap->ctl, IDECTRL_ADDR_CTL, t);
> + udelay(20); /* FIXME: flush */
> + ep93xx_pata_write(drv_data, ap->ctl | ATA_SRST, IDECTRL_ADDR_CTL, t);
> + udelay(20); /* FIXME: flush */
> + ep93xx_pata_write(drv_data, ap->ctl, IDECTRL_ADDR_CTL, t);
> + ap->last_ctl = ap->ctl;
> +
> + return ep93xx_pata_wait_after_reset(&ap->link, devmask, deadline);
> +}
> +
> +static void ep93xx_pata_release_dma(struct ep93xx_pata_data *drv_data)
> +{
> + if (drv_data->dma_rx_channel) {
> + dma_release_channel(drv_data->dma_rx_channel);
> + drv_data->dma_rx_channel = NULL;
> + }
> + if (drv_data->dma_tx_channel) {
> + dma_release_channel(drv_data->dma_tx_channel);
> + drv_data->dma_tx_channel = NULL;
> + }
> +}
> +
> +static bool ep93xx_pata_dma_filter(struct dma_chan *chan, void *filter_param)
> +{
> + if (ep93xx_dma_chan_is_m2p(chan))
> + return false;
> +
> + chan->private = filter_param;
> + return true;
> +}
> +
> +static void ep93xx_pata_dma_init(struct ep93xx_pata_data *drv_data)
> +{
> + const struct platform_device *pdev = drv_data->pdev;
> + dma_cap_mask_t mask;
> + struct dma_slave_config conf;
> +
> + dma_cap_zero(mask);
> + dma_cap_set(DMA_SLAVE, mask);
> +
> + /*
> + * Request two channels for IDE. Another possibility would be
> + * to request only one channel, and reprogram it's direction at
> + * start of new transfer.
> + */
> + drv_data->dma_rx_data.port = EP93XX_DMA_IDE;
> + drv_data->dma_rx_data.direction = DMA_FROM_DEVICE;
> + drv_data->dma_rx_data.name = "ep93xx-pata-rx";
> + drv_data->dma_rx_channel = dma_request_channel(mask,
> + ep93xx_pata_dma_filter, &drv_data->dma_rx_data);
> + if (!drv_data->dma_rx_channel)
> + return;
> +
> + drv_data->dma_tx_data.port = EP93XX_DMA_IDE;
> + drv_data->dma_tx_data.direction = DMA_TO_DEVICE;
> + drv_data->dma_tx_data.name = "ep93xx-pata-tx";
> + drv_data->dma_tx_channel = dma_request_channel(mask,
> + ep93xx_pata_dma_filter, &drv_data->dma_tx_data);
> + if (!drv_data->dma_tx_channel) {
> + dma_release_channel(drv_data->dma_rx_channel);
> + return;
> + }
> +
> + /* Configure receive channel direction and source address */
> + memset(&conf, 0, sizeof(conf));
> + conf.direction = DMA_FROM_DEVICE;
> + conf.src_addr = drv_data->udma_in_phys;
> + conf.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> + if (dmaengine_slave_config(drv_data->dma_rx_channel, &conf)) {
> + dev_err(&pdev->dev, "failed to configure rx dma channel\n");
> + ep93xx_pata_release_dma(drv_data);
> + return;
> + }
> +
> + /* Configure transmit channel direction and destination address */
> + memset(&conf, 0, sizeof(conf));
> + conf.direction = DMA_TO_DEVICE;
> + conf.dst_addr = drv_data->udma_out_phys;
> + conf.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> + if (dmaengine_slave_config(drv_data->dma_tx_channel, &conf)) {
> + dev_err(&pdev->dev, "failed to configure tx dma channel\n");
> + ep93xx_pata_release_dma(drv_data);
> + }
> +}
> +
> +static void ep93xx_pata_dma_start(struct ata_queued_cmd *qc)
> +{
> + struct dma_async_tx_descriptor *txd;
> + struct ep93xx_pata_data *drv_data = qc->ap->host->private_data;
> + void __iomem *base = drv_data->ide_base;
> + struct ata_device *adev = qc->dev;
> + u32 v = qc->dma_dir == DMA_TO_DEVICE ? IDEUDMAOP_RWOP : 0;
> + struct dma_chan *channel = qc->dma_dir == DMA_TO_DEVICE
> + ? drv_data->dma_tx_channel : drv_data->dma_rx_channel;
> +
> + txd = channel->device->device_prep_slave_sg(channel, qc->sg,
> + qc->n_elem, qc->dma_dir, DMA_CTRL_ACK, NULL);
> + if (!txd) {
> + dev_err(qc->ap->dev, "failed to prepare slave for sg dma\n");
> + return;
> + }
> + txd->callback = NULL;
> + txd->callback_param = NULL;
> +
> + if (dmaengine_submit(txd) < 0) {
> + dev_err(qc->ap->dev, "failed to submit dma transfer\n");
> + return;
> + }
> + dma_async_issue_pending(channel);
> +
> + /*
> + * When enabling UDMA operation, IDEUDMAOP register needs to be
> + * programmed in three step sequence:
> + * 1) set or clear the RWOP bit,
> + * 2) perform dummy read of the register,
> + * 3) set the UEN bit.
> + */
> + writel(v, base + IDEUDMAOP);
> + readl(base + IDEUDMAOP);
> + writel(v | IDEUDMAOP_UEN, base + IDEUDMAOP);
> +
> + writel(IDECFG_IDEEN | IDECFG_UDMA |
> + ((adev->xfer_mode - XFER_UDMA_0) << IDECFG_MODE_SHIFT),
> + base + IDECFG);
> +}
> +
> +static void ep93xx_pata_dma_stop(struct ata_queued_cmd *qc)
> +{
> + struct ep93xx_pata_data *drv_data = qc->ap->host->private_data;
> + void __iomem *base = drv_data->ide_base;
> +
> + /* terminate all dma transfers, if not yet finished */
> + dmaengine_terminate_all(drv_data->dma_rx_channel);
> + dmaengine_terminate_all(drv_data->dma_tx_channel);
> +
> + /*
> + * To properly stop IDE-DMA, IDEUDMAOP register must to be cleared
> + * and IDECTRL register must be set to default value.
> + */
> + writel(0, base + IDEUDMAOP);
> + writel(readl(base + IDECTRL) | IDECTRL_DIOWN | IDECTRL_DIORN |
> + IDECTRL_CS0N | IDECTRL_CS1N, base + IDECTRL);
> +
> + ep93xx_pata_enable_pio(drv_data->ide_base,
> + qc->dev->pio_mode - XFER_PIO_0);
> +
> + ata_sff_dma_pause(qc->ap);
> +}
> +
> +static void ep93xx_pata_dma_setup(struct ata_queued_cmd *qc)
> +{
> + qc->ap->ops->sff_exec_command(qc->ap, &qc->tf);
> +}
> +
> +static u8 ep93xx_pata_dma_status(struct ata_port *ap)
> +{
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> + u32 val = readl(drv_data->ide_base + IDEUDMASTS);
> +
> + /*
> + * UDMA Status Register bits:
> + *
> + * DMAIDE - DMA request signal from UDMA state machine,
> + * INTIDE - INT line generated by UDMA because of errors in the
> + * state machine,
> + * SBUSY - UDMA state machine busy, not in idle state,
> + * NDO - error for data-out not completed,
> + * NDI - error for data-in not completed,
> + * N4X - error for data transferred not multiplies of four
> + * 32-bit words.
> + * (EP93xx UG p27-17)
> + */
> + if (val & IDEUDMASTS_NDO || val & IDEUDMASTS_NDI ||
> + val & IDEUDMASTS_N4X || val & IDEUDMASTS_INTIDE)
> + return ATA_DMA_ERR;
> +
> + /* read INTRQ (INT[3]) pin input state */
> + if (readl(drv_data->ide_base + IDECTRL) & IDECTRL_INTRQ)
> + return ATA_DMA_INTR;
> +
> + if (val & IDEUDMASTS_SBUSY || val & IDEUDMASTS_DMAIDE)
> + return ATA_DMA_ACTIVE;
> +
> + return 0;
> +}
> +
> +/* Note: original code is ata_sff_softreset */
> +static int ep93xx_pata_softreset(struct ata_link *al, unsigned int *classes,
> + unsigned long deadline)
> +{
> + struct ata_port *ap = al->ap;
> + unsigned int slave_possible = ap->flags & ATA_FLAG_SLAVE_POSS;
> + unsigned int devmask = 0;
> + int rc;
> + u8 err;
> +
> + DPRINTK("ENTER\n");
> +
> + /* determine if device 0/1 are present */
> + if (ep93xx_pata_device_is_present(ap, 0))
> + devmask |= (1 << 0);
> + if (slave_possible && ep93xx_pata_device_is_present(ap, 1))
> + devmask |= (1 << 1);
> +
> + /* select device 0 again */
> + ap->ops->sff_dev_select(al->ap, 0);
> +
> + /* issue bus reset */
> + DPRINTK("about to softreset, devmask=%x\n", devmask);
> + rc = ep93xx_pata_bus_softreset(ap, devmask, deadline);
> + /* if link is ocuppied, -ENODEV too is an error */
> + if (rc && (rc != -ENODEV || sata_scr_valid(al))) {
> + ata_link_printk(al, KERN_ERR, "SRST failed (errno=%d)\n",
> + rc);
> + return rc;
> + }
> +
> + /* determine by signature whether we have ATA or ATAPI devices */
> + classes[0] = ata_sff_dev_classify(&al->device[0], devmask & (1 << 0),
> + &err);
> + if (slave_possible && err != 0x81)
> + classes[1] = ata_sff_dev_classify(&al->device[1],
> + devmask & (1 << 1), &err);
> +
> + DPRINTK("EXIT, classes[0]=%u, [1]=%u\n", classes[0],
> + classes[1]);
> + return 0;
> +}
> +
> +/* Note: original code is ata_sff_drain_fifo */
> +void ep93xx_pata_drain_fifo(struct ata_queued_cmd *qc)
> +{
> + int count;
> + struct ata_port *ap;
> + struct ep93xx_pata_data *drv_data;
> +
> + /* We only need to flush incoming data when a command was running */
> + if (qc == NULL || qc->dma_dir == DMA_TO_DEVICE)
> + return;
> +
> + ap = qc->ap;
> + drv_data = ap->host->private_data;
> + /* Drain up to 64K of data before we give up this recovery method */
> + for (count = 0; (ap->ops->sff_check_status(ap) & ATA_DRQ)
> + && count < 65536; count += 2)
> + ep93xx_pata_read(drv_data, IDECTRL_ADDR_DATA,
> + drv_data->cmd_t);
> +
> + /* Can become DEBUG later */
> + if (count)
> + ata_port_printk(ap, KERN_DEBUG,
> + "drained %d bytes to clear DRQ.\n", count);
> +
> +}
> +
> +static int ep93xx_pata_port_start(struct ata_port *ap)
> +{
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> +
> + drv_data->cmd_t = ata_timing_find_mode(XFER_PIO_0);
> + return 0;
> +}
> +
> +static struct scsi_host_template ep93xx_pata_sht = {
> + ATA_BASE_SHT(DRV_NAME),
> + /* ep93xx dma implementation limit */
> + .sg_tablesize = 32,
> + /* ep93xx dma can't transfer 65536 bytes at once */
> + .dma_boundary = 0x7fff,
> +};
> +
> +static struct ata_port_operations ep93xx_pata_port_ops = {
> + .inherits = &ata_bmdma_port_ops,
> +
> + .qc_prep = ata_noop_qc_prep,
> +
> + .softreset = ep93xx_pata_softreset,
> + .hardreset = ATA_OP_NULL,
> +
> + .sff_dev_select = ep93xx_pata_dev_select,
> + .sff_set_devctl = ep93xx_pata_set_devctl,
> + .sff_check_status = ep93xx_pata_check_status,
> + .sff_check_altstatus = ep93xx_pata_check_altstatus,
> + .sff_tf_load = ep93xx_pata_tf_load,
> + .sff_tf_read = ep93xx_pata_tf_read,
> + .sff_exec_command = ep93xx_pata_exec_command,
> + .sff_data_xfer = ep93xx_pata_data_xfer,
> + .sff_drain_fifo = ep93xx_pata_drain_fifo,
> + .sff_irq_clear = ATA_OP_NULL,
> +
> + .set_piomode = ep93xx_pata_set_piomode,
> +
> + .bmdma_setup = ep93xx_pata_dma_setup,
> + .bmdma_start = ep93xx_pata_dma_start,
> + .bmdma_stop = ep93xx_pata_dma_stop,
> + .bmdma_status = ep93xx_pata_dma_status,
> +
> + .cable_detect = ata_cable_unknown,
> + .port_start = ep93xx_pata_port_start,
> +};
> +
> +static int __devinit ep93xx_pata_probe(struct platform_device *pdev)
> +{
> + struct ep93xx_pata_data *drv_data;
> + struct ata_host *host;
> + struct ata_port *ap;
> + unsigned int irq;
> + struct resource *mem_res;
> + void __iomem *ide_base;
> + int err;
> +
> + err = ep93xx_ide_acquire_gpio(pdev);
> + if (err)
> + return err;
> +
> + /* INT[3] (IRQ_EP93XX_EXT3) line connected as pull down */
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + err = -ENXIO;
> + goto err_rel_gpio;
> + }
> +
> + mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!mem_res) {
> + err = -ENXIO;
> + goto err_rel_gpio;
> + }
> +
> + ide_base = devm_request_and_ioremap(&pdev->dev, mem_res);
> + if (!ide_base) {
> + err = -ENXIO;
> + goto err_rel_gpio;
> + }
> +
> + drv_data = devm_kzalloc(&pdev->dev, sizeof(*drv_data), GFP_KERNEL);
> + if (!drv_data) {
> + err = -ENXIO;
> + goto err_rel_gpio;
> + }
> +
> + platform_set_drvdata(pdev, drv_data);
> + drv_data->pdev = pdev;
> + drv_data->ide_base = ide_base;
> + drv_data->udma_in_phys = mem_res->start + IDEUDMADATAIN;
> + drv_data->udma_out_phys = mem_res->start + IDEUDMADATAOUT;
> + ep93xx_pata_dma_init(drv_data);
> +
> + /* allocate host */
> + host = ata_host_alloc(&pdev->dev, 1);
> + if (!host) {
> + err = -ENXIO;
> + goto err_rel_dma;
> + }
> +
> + ep93xx_pata_clear_regs(ide_base);
> +
> + host->n_ports = 1;
Just a question. Does the single port handle both devices that the ep93xx ide
interface supports?
> + host->private_data = drv_data;
> +
> + ap = host->ports[0];
> + ap->dev = &pdev->dev;
> + ap->ops = &ep93xx_pata_port_ops;
> + ap->flags |= ATA_FLAG_SLAVE_POSS;
> + ap->pio_mask = ATA_PIO4;
> +
> + /*
> + * Maximum UDMA modes:
> + * EP931x rev.E0 - UDMA2
> + * EP931x rev.E1 - UDMA3
> + * EP931x rev.E2 - UDMA4
> + *
> + * MWDMA support was removed from EP931x rev.E2,
> + * so this driver supports only UDMA modes.
> + */
> + if (drv_data->dma_rx_channel && drv_data->dma_tx_channel) {
> + int chip_rev = ep93xx_chip_revision();
> +
> + if (chip_rev == EP93XX_CHIP_REV_E1)
> + ap->udma_mask = ATA_UDMA3;
> + else if (chip_rev == EP93XX_CHIP_REV_E2)
> + ap->udma_mask = ATA_UDMA4;
> + else
> + ap->udma_mask = ATA_UDMA2;
> + }
> +
> + /* defaults, pio 0 */
> + ep93xx_pata_enable_pio(ide_base, 0);
> +
> + dev_info(&pdev->dev, "version " DRV_VERSION "\n");
> +
> + /* activate host */
> + err = ata_host_activate(host, irq, ata_bmdma_interrupt, 0,
> + &ep93xx_pata_sht);
> + if (err == 0)
> + return 0;
> +
> +err_rel_dma:
> + ep93xx_pata_release_dma(drv_data);
> +err_rel_gpio:
> + ep93xx_ide_release_gpio(pdev);
> + return err;
> +}
> +
> +static int __devexit ep93xx_pata_remove(struct platform_device *pdev)
> +{
> + struct ata_host *host = platform_get_drvdata(pdev);
> + struct ep93xx_pata_data *drv_data = host->private_data;
> +
> + ata_host_detach(host);
> + ep93xx_pata_release_dma(drv_data);
> + ep93xx_pata_clear_regs(drv_data->ide_base);
> + ep93xx_ide_release_gpio(pdev);
> + return 0;
> +}
> +
> +static struct platform_driver ep93xx_pata_platform_driver = {
> + .driver = {
> + .name = DRV_NAME,
> + .owner = THIS_MODULE,
> + },
> + .probe = ep93xx_pata_probe,
> + .remove = __devexit_p(ep93xx_pata_remove),
> +};
> +
> +module_platform_driver(ep93xx_pata_platform_driver);
> +
> +MODULE_AUTHOR("Alessandro Zummo, Lennert Buytenhek, Joao Ramos, "
> + "Bartlomiej Zolnierkiewicz, Rafal Prylowski");
> +MODULE_DESCRIPTION("low-level driver for cirrus ep93xx IDE controller");
> +MODULE_LICENSE("GPL");
> +MODULE_VERSION(DRV_VERSION);
> +MODULE_ALIAS("platform:pata_ep93xx");
> Index: linux-2.6/drivers/ata/Kconfig
> ===================================================================
> --- linux-2.6.orig/drivers/ata/Kconfig
> +++ linux-2.6/drivers/ata/Kconfig
> @@ -416,6 +416,15 @@ config PATA_EFAR
>
> If unsure, say N.
>
> +config PATA_EP93XX
> + tristate "Cirrus Logic EP93xx PATA support"
> + depends on ARCH_EP93XX
> + help
> + This option enables support for the PATA controller in
> + the Cirrus Logic EP9312 and EP9315 ARM CPU.
> +
> + If unsure, say N.
> +
> config PATA_HPT366
> tristate "HPT 366/368 PATA support"
> depends on PCI
> Index: linux-2.6/drivers/ata/Makefile
> ===================================================================
> --- linux-2.6.orig/drivers/ata/Makefile
> +++ linux-2.6/drivers/ata/Makefile
> @@ -43,6 +43,7 @@ obj-$(CONFIG_PATA_CS5535) += pata_cs5535
> obj-$(CONFIG_PATA_CS5536) += pata_cs5536.o
> obj-$(CONFIG_PATA_CYPRESS) += pata_cypress.o
> obj-$(CONFIG_PATA_EFAR) += pata_efar.o
> +obj-$(CONFIG_PATA_EP93XX) += pata_ep93xx.o
> obj-$(CONFIG_PATA_HPT366) += pata_hpt366.o
> obj-$(CONFIG_PATA_HPT37X) += pata_hpt37x.o
> obj-$(CONFIG_PATA_HPT3X2N) += pata_hpt3x2n.o
Overall, this looks pretty good. I'll try to examine it more closely later.
Wish I had some actual hardware to test this with...
Regards,
Hartley
^ permalink raw reply [flat|nested] 51+ messages in thread* [PATCH v2 1/3] PATA host controller driver for ep93xx
@ 2012-04-03 18:25 ` H Hartley Sweeten
0 siblings, 0 replies; 51+ messages in thread
From: H Hartley Sweeten @ 2012-04-03 18:25 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday, April 03, 2012 7:45 AM, Rafal Prylowski wrote:
Rafal,
Just a quick review. I'll look over the driver more closely later.
>
> Signed-off-by: Rafal Prylowski <prylowski@metasoft.pl>
> Cc: Joao Ramos <joao.ramos@inov.pt>
> Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
> Cc: Ryan Mallon <rmallon@gmail.com>
> Cc: Sergei Shtylyov <sshtylyov@mvista.com>
> Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Again, there should be a commit message.
> ---
> drivers/ata/Kconfig | 9
> drivers/ata/Makefile | 1
> drivers/ata/pata_ep93xx.c | 976 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 986 insertions(+)
>
> Index: linux-2.6/drivers/ata/pata_ep93xx.c
> ===================================================================
> --- /dev/null
> +++ linux-2.6/drivers/ata/pata_ep93xx.c
> @@ -0,0 +1,976 @@
> +/*
> + * EP93XX PATA controller driver.
> + *
> + * Copyright (c) 2012, Metasoft s.c.
> + * Rafal Prylowski <prylowski@metasoft.pl>
> + *
> + * Based on pata_scc.c, pata_icside.c and on earlier version of EP93XX
> + * PATA driver by Lennert Buytenhek and Alessandro Zummo.
> + * Read/Write timings, resource management and other improvements
> + * from driver by Joao Ramos and Bartlomiej Zolnierkiewicz.
> + * DMA engine support based on spi-ep93xx.c by Mika Westerberg.
> + *
> + * Original copyrights:
> + *
> + * Support for Cirrus Logic's EP93xx (EP9312, EP9315) CPUs
> + * PATA host controller driver.
> + *
> + * Copyright (c) 2009, Bartlomiej Zolnierkiewicz
> + *
> + * Heavily based on the ep93xx-ide.c driver:
> + *
> + * Copyright (c) 2009, Joao Ramos <joao.ramos@inov.pt>
> + * INESC Inovacao (INOV)
> + *
> + * EP93XX PATA controller driver.
> + * Copyright (C) 2007 Lennert Buytenhek <buytenh@wantstofly.org>
> + *
> + * An ATA driver for the Cirrus Logic EP93xx PATA controller.
> + *
> + * Based on an earlier version by Alessandro Zummo, which is:
> + * Copyright (C) 2006 Tower Technologies
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/blkdev.h>
> +#include <scsi/scsi_host.h>
> +#include <linux/ata.h>
> +#include <linux/libata.h>
> +#include <linux/platform_device.h>
> +#include <linux/delay.h>
> +#include <linux/dmaengine.h>
> +
> +#include <mach/dma.h>
> +#include <mach/platform.h>
> +
> +#define DRV_NAME "ep93xx-ide"
> +#define DRV_VERSION "1.0"
> +
> +enum {
> + /* IDE Control Register */
> + IDECTRL = 0x00,
> + IDECTRL_CS0N = (1 << 0),
> + IDECTRL_CS1N = (1 << 1),
> + IDECTRL_DIORN = (1 << 5),
> + IDECTRL_DIOWN = (1 << 6),
> + IDECTRL_INTRQ = (1 << 9),
> + IDECTRL_IORDY = (1 << 10),
> + /*
> + * the device IDE register to be accessed is selected through
> + * IDECTRL register's specific bitfields 'DA', 'CS1N' and 'CS0N':
> + * b4 b3 b2 b1 b0
> + * A2 A1 A0 CS1N CS0N
> + * the values filled in this structure allows the value to be directly
> + * ORed to the IDECTRL register, hence giving directly the A[2:0] and
> + * CS1N/CS0N values for each IDE register.
> + * The values correspond to the transformation:
> + * ((real IDE address) << 2) | CS1N value << 1 | CS0N value
> + */
> + IDECTRL_ADDR_CMD = 0 + 2, /* CS1 */
> + IDECTRL_ADDR_DATA = (ATA_REG_DATA << 2) + 2,
> + IDECTRL_ADDR_ERROR = (ATA_REG_ERR << 2) + 2,
> + IDECTRL_ADDR_FEATURE = (ATA_REG_FEATURE << 2) + 2,
> + IDECTRL_ADDR_NSECT = (ATA_REG_NSECT << 2) + 2,
> + IDECTRL_ADDR_LBAL = (ATA_REG_LBAL << 2) + 2,
> + IDECTRL_ADDR_LBAM = (ATA_REG_LBAM << 2) + 2,
> + IDECTRL_ADDR_LBAH = (ATA_REG_LBAH << 2) + 2,
> + IDECTRL_ADDR_DEVICE = (ATA_REG_DEVICE << 2) + 2,
> + IDECTRL_ADDR_STATUS = (ATA_REG_STATUS << 2) + 2,
> + IDECTRL_ADDR_COMMAND = (ATA_REG_CMD << 2) + 2,
> + IDECTRL_ADDR_ALTSTATUS = (0x06 << 2) + 1, /* CS0 */
> + IDECTRL_ADDR_CTL = (0x06 << 2) + 1, /* CS0 */
> +
> + /* IDE Configuration Register */
> + IDECFG = 0x04,
> + IDECFG_IDEEN = (1 << 0),
> + IDECFG_PIO = (1 << 1),
> + IDECFG_MDMA = (1 << 2),
> + IDECFG_UDMA = (1 << 3),
> + IDECFG_MODE_SHIFT = 4,
> + IDECFG_MODE_MASK = (0xf << 4),
> + IDECFG_WST_SHIFT = 8,
> + IDECFG_WST_MASK = (0x3 << 8),
> +
> + /* MDMA Operation Register */
> + IDEMDMAOP = 0x08,
> +
> + /* UDMA Operation Register */
> + IDEUDMAOP = 0x0c,
> + IDEUDMAOP_UEN = (1 << 0),
> + IDEUDMAOP_RWOP = (1 << 1),
> +
> + /* PIO/MDMA/UDMA Data Registers */
> + IDEDATAOUT = 0x10,
> + IDEDATAIN = 0x14,
> + IDEMDMADATAOUT = 0x18,
> + IDEMDMADATAIN = 0x1c,
> + IDEUDMADATAOUT = 0x20,
> + IDEUDMADATAIN = 0x24,
> +
> + /* UDMA Status Register */
> + IDEUDMASTS = 0x28,
> + IDEUDMASTS_DMAIDE = (1 << 16),
> + IDEUDMASTS_INTIDE = (1 << 17),
> + IDEUDMASTS_SBUSY = (1 << 18),
> + IDEUDMASTS_NDO = (1 << 24),
> + IDEUDMASTS_NDI = (1 << 25),
> + IDEUDMASTS_N4X = (1 << 26),
> +
> + /* UDMA Debug Status Register */
> + IDEUDMADEBUG = 0x2c,
> +};
I prefer defines for these. But, the enum usage seems common in the ata drivers
so...
> +
> +struct ep93xx_pata_data /* private_data in ata_host */
> +{
Please move the brace up to the previous line.
Also, I don't think the comment is necessary.
> + const struct platform_device *pdev;
> + void __iomem *ide_base;
> + const struct ata_timing *t;
> + const struct ata_timing *cmd_t;
> + bool iordy;
> +
> + unsigned long udma_in_phys;
> + unsigned long udma_out_phys;
> +
> + struct dma_chan *dma_rx_channel;
> + struct ep93xx_dma_data dma_rx_data;
> + struct dma_chan *dma_tx_channel;
> + struct ep93xx_dma_data dma_tx_data;
> +};
> +
> +static void ep93xx_pata_clear_regs(void __iomem *base)
> +{
> + writel(IDECTRL_CS0N | IDECTRL_CS1N | IDECTRL_DIORN |
> + IDECTRL_DIOWN, base + IDECTRL);
> +
> + writel(0, base + IDECFG);
> + writel(0, base + IDEMDMAOP);
> + writel(0, base + IDEUDMAOP);
> + writel(0, base + IDEDATAOUT);
> + writel(0, base + IDEDATAIN);
> + writel(0, base + IDEMDMADATAOUT);
> + writel(0, base + IDEMDMADATAIN);
> + writel(0, base + IDEUDMADATAOUT);
> + writel(0, base + IDEUDMADATAIN);
> + writel(0, base + IDEUDMADEBUG);
> +}
> +
> +static int ep93xx_pata_check_iordy(void __iomem *base)
> +{
> + return !!(readl(base + IDECTRL) & IDECTRL_IORDY);
> +}
> +
> +/*
> + * According to EP93xx User's Guide, WST field of IDECFG specifies number
> + * of HCLK cycles to hold the data bus after a PIO write operation.
> + * It should be programmed to guarantee following delays:
> + *
> + * PIO Mode [ns]
> + * 0 30
> + * 1 20
> + * 2 15
> + * 3 10
> + * 4 5
> + *
> + * Maximum possible value for HCLK is 100MHz.
> + */
> +static int ep93xx_pata_get_wst(int pio_mode)
> +{
> + int val;
> +
> + if (pio_mode == 0)
> + val = 3;
> + else if (pio_mode < 3)
> + val = 2;
> + else
> + val = 1;
> +
> + return val << IDECFG_WST_SHIFT;
> +}
> +
> +static void ep93xx_pata_enable_pio(void __iomem *base, int pio_mode)
> +{
> + writel(IDECFG_IDEEN | IDECFG_PIO |
> + ep93xx_pata_get_wst(pio_mode) |
> + (pio_mode << IDECFG_MODE_SHIFT), base + IDECFG);
> +}
> +
> +static void ep93xx_pata_wait_for_iordy(void __iomem *base)
> +{
> + unsigned long deadline = jiffies + msecs_to_jiffies(1000);
Please add a white space here.
> + while (!ep93xx_pata_check_iordy(base) &&
> + time_is_before_jiffies(deadline))
> + cpu_relax();
> +}
> +
> +static u16 ep93xx_pata_read(struct ep93xx_pata_data *drv_data,
> + unsigned long addr,
> + const struct ata_timing *t)
> +{
> + void __iomem *base = drv_data->ide_base;
> + u16 ret;
> +
> + addr &= 0x1f;
> + writel(IDECTRL_DIOWN | IDECTRL_DIORN | addr, base + IDECTRL);
> + ndelay(t->setup);
> + writel(IDECTRL_DIOWN | addr, base + IDECTRL);
> + ndelay(t->act8b);
> + if (drv_data->iordy)
> + ep93xx_pata_wait_for_iordy(base);
> + /*
> + * The IDEDATAIN register is loaded from the DD pins at the positive
> + * edge of the DIORN signal. (EP93xx UG p27-14)
> + */
> + writel(IDECTRL_DIOWN | IDECTRL_DIORN | addr, base + IDECTRL);
> + ret = readl(base + IDEDATAIN);
> + ndelay(t->cyc8b - t->act8b - t->setup);
> + return ret;
> +}
> +
> +static void ep93xx_pata_write(struct ep93xx_pata_data *drv_data,
> + u16 value, unsigned long addr,
> + const struct ata_timing *t)
> +{
> + void __iomem *base = drv_data->ide_base;
> +
> + addr &= 0x1f;
> + writel(IDECTRL_DIOWN | IDECTRL_DIORN | addr, base + IDECTRL);
> + ndelay(t->setup);
> + /*
> + * Value from IDEDATAOUT register is driven onto the DD pins when
> + * DIOWN is low. (EP93xx UG p27-13)
> + */
> + writel(value, base + IDEDATAOUT);
> + writel(IDECTRL_DIORN | addr, base + IDECTRL);
> + ndelay(t->act8b);
> + if (drv_data->iordy)
> + ep93xx_pata_wait_for_iordy(base);
> + writel(IDECTRL_DIOWN | IDECTRL_DIORN | addr, base + IDECTRL);
> + ndelay(t->cyc8b - t->act8b - t->setup);
> +}
> +
> +static void ep93xx_pata_set_piomode(struct ata_port *ap,
> + struct ata_device *adev)
> +{
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> + struct ata_device *pair = ata_dev_pair(adev);
> +
> + drv_data->t = ata_timing_find_mode(adev->pio_mode);
> + drv_data->cmd_t = drv_data->t;
> + drv_data->iordy = ata_pio_need_iordy(adev);
> +
> + if (pair && pair->pio_mode < adev->pio_mode)
> + drv_data->cmd_t = ata_timing_find_mode(pair->pio_mode);
> +
> + ep93xx_pata_enable_pio(drv_data->ide_base,
> + adev->pio_mode - XFER_PIO_0);
> +}
> +
> +/* Note: original code is ata_sff_check_status */
> +static u8 ep93xx_pata_check_status(struct ata_port *ap)
> +{
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> +
> + return ep93xx_pata_read(drv_data, IDECTRL_ADDR_STATUS,
> + drv_data->cmd_t);
> +}
> +
> +static u8 ep93xx_pata_check_altstatus(struct ata_port *ap)
> +{
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> +
> + return ep93xx_pata_read(drv_data, IDECTRL_ADDR_ALTSTATUS,
> + drv_data->cmd_t);
> +}
> +
> +/* Note: original code is ata_sff_tf_load */
> +static void ep93xx_pata_tf_load(struct ata_port *ap,
> + const struct ata_taskfile *tf)
> +{
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> + const struct ata_timing *t = drv_data->cmd_t;
> + unsigned int is_addr = tf->flags & ATA_TFLAG_ISADDR;
> +
> + if (tf->ctl != ap->last_ctl) {
> + ep93xx_pata_write(drv_data, tf->ctl, IDECTRL_ADDR_CTL, t);
> + ap->last_ctl = tf->ctl;
> + ata_wait_idle(ap);
> + }
> +
> + if (is_addr && (tf->flags & ATA_TFLAG_LBA48)) {
> + ep93xx_pata_write(drv_data, tf->hob_feature,
> + IDECTRL_ADDR_FEATURE, t);
> + ep93xx_pata_write(drv_data, tf->hob_nsect,
> + IDECTRL_ADDR_NSECT, t);
> + ep93xx_pata_write(drv_data, tf->hob_lbal,
> + IDECTRL_ADDR_LBAL, t);
> + ep93xx_pata_write(drv_data, tf->hob_lbam,
> + IDECTRL_ADDR_LBAM, t);
> + ep93xx_pata_write(drv_data, tf->hob_lbah,
> + IDECTRL_ADDR_LBAH, t);
> + VPRINTK("hob: feat 0x%X nsect 0x%X, lba 0x%X 0x%X 0x%X\n",
> + tf->hob_feature, tf->hob_nsect, tf->hob_lbal,
> + tf->hob_lbam, tf->hob_lbah);
> + }
> +
> + if (is_addr) {
> + ep93xx_pata_write(drv_data, tf->feature,
> + IDECTRL_ADDR_FEATURE, t);
> + ep93xx_pata_write(drv_data, tf->nsect, IDECTRL_ADDR_NSECT, t);
> + ep93xx_pata_write(drv_data, tf->lbal, IDECTRL_ADDR_LBAL, t);
> + ep93xx_pata_write(drv_data, tf->lbam, IDECTRL_ADDR_LBAM, t);
> + ep93xx_pata_write(drv_data, tf->lbah, IDECTRL_ADDR_LBAH, t);
> + VPRINTK("feat 0x%X nsect 0x%X lba 0x%X 0x%X 0x%X\n",
> + tf->feature, tf->nsect, tf->lbal, tf->lbam, tf->lbah);
> + }
> +
> + if (tf->flags & ATA_TFLAG_DEVICE) {
> + ep93xx_pata_write(drv_data, tf->device, IDECTRL_ADDR_DEVICE, t);
> + VPRINTK("device 0x%X\n", tf->device);
> + }
> +
> + ata_wait_idle(ap);
> +}
> +
> +/* Note: original code is ata_sff_tf_read */
> +static void ep93xx_pata_tf_read(struct ata_port *ap, struct ata_taskfile *tf)
> +{
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> + const struct ata_timing *t = drv_data->cmd_t;
> +
> + tf->command = ep93xx_pata_check_status(ap);
> + tf->feature = ep93xx_pata_read(drv_data, IDECTRL_ADDR_FEATURE, t);
> + tf->nsect = ep93xx_pata_read(drv_data, IDECTRL_ADDR_NSECT, t);
> + tf->lbal = ep93xx_pata_read(drv_data, IDECTRL_ADDR_LBAL, t);
> + tf->lbam = ep93xx_pata_read(drv_data, IDECTRL_ADDR_LBAM, t);
> + tf->lbah = ep93xx_pata_read(drv_data, IDECTRL_ADDR_LBAH, t);
> + tf->device = ep93xx_pata_read(drv_data, IDECTRL_ADDR_DEVICE, t);
> +
> + if (tf->flags & ATA_TFLAG_LBA48) {
> + ep93xx_pata_write(drv_data, tf->ctl | ATA_HOB,
> + IDECTRL_ADDR_CTL, t);
> + tf->hob_feature = ep93xx_pata_read(drv_data,
> + IDECTRL_ADDR_FEATURE, t);
> + tf->hob_nsect = ep93xx_pata_read(drv_data,
> + IDECTRL_ADDR_NSECT, t);
> + tf->hob_lbal = ep93xx_pata_read(drv_data,
> + IDECTRL_ADDR_LBAL, t);
> + tf->hob_lbam = ep93xx_pata_read(drv_data,
> + IDECTRL_ADDR_LBAM, t);
> + tf->hob_lbah = ep93xx_pata_read(drv_data,
> + IDECTRL_ADDR_LBAH, t);
> + ep93xx_pata_write(drv_data, tf->ctl, IDECTRL_ADDR_CTL, t);
> + ap->last_ctl = tf->ctl;
> + }
> +}
> +
> +/* Note: original code is ata_sff_exec_command */
> +static void ep93xx_pata_exec_command(struct ata_port *ap,
> + const struct ata_taskfile *tf)
> +{
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> +
> + DPRINTK("ata%u: cmd 0x%X\n", ap->print_id, tf->command);
> +
> + ep93xx_pata_write(drv_data, tf->command,
> + IDECTRL_ADDR_COMMAND, drv_data->cmd_t);
> + ata_sff_pause(ap);
> +}
> +
> +/* Note: original code is ata_sff_dev_select */
> +static void ep93xx_pata_dev_select(struct ata_port *ap, unsigned int device)
> +{
> + u8 tmp = ATA_DEVICE_OBS;
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
Nitpick... Please put the struct ep93xx_pata_data line before the u8 line.
> +
> + if (device != 0)
> + tmp |= ATA_DEV1;
> +
> + ep93xx_pata_write(drv_data, tmp, IDECTRL_ADDR_DEVICE,
> + drv_data->cmd_t);
> + ata_sff_pause(ap); /* needed; also flushes, for mmio */
> +}
> +
> +/* Note: original code is ata_sff_set_devctl */
> +static void ep93xx_pata_set_devctl(struct ata_port *ap, u8 ctl)
> +{
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> +
> + ep93xx_pata_write(drv_data, ctl, IDECTRL_ADDR_CTL,
> + drv_data->cmd_t);
> +}
> +
> +/* Note: original code is ata_sff_data_xfer */
> +unsigned int ep93xx_pata_data_xfer(struct ata_device *adev, unsigned char *buf,
> + unsigned int buflen, int rw)
> +{
> + struct ata_port *ap = adev->link->ap;
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> + const struct ata_timing *t = drv_data->t;
> + u16 *data = (u16 *)buf;
> + unsigned int words = buflen >> 1;
> +
> + /* Transfer multiple of 2 bytes */
> + while (words--)
> + if (rw == READ)
> + *data++ = cpu_to_le16(
> + ep93xx_pata_read(
> + drv_data, IDECTRL_ADDR_DATA, t));
> + else
> + ep93xx_pata_write(drv_data, le16_to_cpu(*data++),
> + IDECTRL_ADDR_DATA, t);
> +
> + /* Transfer trailing 1 byte, if any. */
> + if (unlikely(buflen & 0x01)) {
> + unsigned char pad[2] = { };
> +
> + buf += buflen - 1;
> +
> + if (rw == READ) {
> + *pad = cpu_to_le16(
> + ep93xx_pata_read(
> + drv_data, IDECTRL_ADDR_DATA, t));
> + *buf = pad[0];
> + } else {
> + pad[0] = *buf;
> + ep93xx_pata_write(drv_data, le16_to_cpu(*pad),
> + IDECTRL_ADDR_DATA, t);
> + }
> + words++;
> + }
> +
> + return words << 1;
> +}
> +
> +/* Note: original code is ata_devchk */
> +static bool ep93xx_pata_device_is_present(struct ata_port *ap,
> + unsigned int device)
> +{
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> + const struct ata_timing *t = drv_data->cmd_t;
> + u8 nsect, lbal;
> +
> + ap->ops->sff_dev_select(ap, device);
> +
> + ep93xx_pata_write(drv_data, 0x55, IDECTRL_ADDR_NSECT, t);
> + ep93xx_pata_write(drv_data, 0xaa, IDECTRL_ADDR_LBAL, t);
> +
> + ep93xx_pata_write(drv_data, 0xaa, IDECTRL_ADDR_NSECT, t);
> + ep93xx_pata_write(drv_data, 0x55, IDECTRL_ADDR_LBAL, t);
> +
> + ep93xx_pata_write(drv_data, 0x55, IDECTRL_ADDR_NSECT, t);
> + ep93xx_pata_write(drv_data, 0xaa, IDECTRL_ADDR_LBAL, t);
> +
> + nsect = ep93xx_pata_read(drv_data, IDECTRL_ADDR_NSECT, t);
> + lbal = ep93xx_pata_read(drv_data, IDECTRL_ADDR_LBAL, t);
> +
> + if ((nsect == 0x55) && (lbal == 0xaa))
> + return true; /* we found a device */
> +
> + return false; /* nothing found */
Nitpick... The comments are not needed.
> +}
> +
> +/* Note: original code is ata_sff_wait_after_reset */
> +int ep93xx_pata_wait_after_reset(struct ata_link *link, unsigned int devmask,
> + unsigned long deadline)
> +{
> + struct ata_port *ap = link->ap;
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> + const struct ata_timing *t = drv_data->cmd_t;
> + unsigned int dev0 = devmask & (1 << 0);
> + unsigned int dev1 = devmask & (1 << 1);
> + int rc, ret = 0;
> +
> + ata_msleep(ap, ATA_WAIT_AFTER_RESET);
> +
> + /* always check readiness of the master device */
> + rc = ata_sff_wait_ready(link, deadline);
> + /* -ENODEV means the odd clown forgot the D7 pulldown resistor
> + * and TF status is 0xff, bail out on it too.
> + */
Multiline comments, here and below, should be:
/*
* Comment...
*/
> + if (rc)
> + return rc;
> +
> + /* if device 1 was found in ata_devchk, wait for register
> + * access briefly, then wait for BSY to clear.
> + */
> + if (dev1) {
> + int i;
> +
> + ap->ops->sff_dev_select(ap, 1);
> +
> + /* Wait for register access. Some ATAPI devices fail
> + * to set nsect/lbal after reset, so don't waste too
> + * much time on it. We're gonna wait for !BSY anyway.
> + */
> + for (i = 0; i < 2; i++) {
> + u8 nsect, lbal;
> +
> + nsect = ep93xx_pata_read(drv_data,
> + IDECTRL_ADDR_NSECT, t);
> + lbal = ep93xx_pata_read(drv_data,
> + IDECTRL_ADDR_LBAL, t);
> + if (nsect == 1 && lbal == 1)
> + break;
> + msleep(50); /* give drive a breather */
> + }
> +
> + rc = ata_sff_wait_ready(link, deadline);
> + if (rc) {
> + if (rc != -ENODEV)
> + return rc;
> + ret = rc;
> + }
> + }
> + /* is all this really necessary? */
> + ap->ops->sff_dev_select(ap, 0);
> + if (dev1)
> + ap->ops->sff_dev_select(ap, 1);
> + if (dev0)
> + ap->ops->sff_dev_select(ap, 0);
> +
> + return ret;
> +}
> +
> +/* Note: original code is ata_bus_softreset */
> +static int ep93xx_pata_bus_softreset(struct ata_port *ap, unsigned int devmask,
> + unsigned long deadline)
> +{
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> + const struct ata_timing *t = drv_data->cmd_t;
> +
> + DPRINTK("ata%u: bus reset via SRST\n", ap->print_id);
> +
> + ep93xx_pata_write(drv_data, ap->ctl, IDECTRL_ADDR_CTL, t);
> + udelay(20); /* FIXME: flush */
> + ep93xx_pata_write(drv_data, ap->ctl | ATA_SRST, IDECTRL_ADDR_CTL, t);
> + udelay(20); /* FIXME: flush */
> + ep93xx_pata_write(drv_data, ap->ctl, IDECTRL_ADDR_CTL, t);
> + ap->last_ctl = ap->ctl;
> +
> + return ep93xx_pata_wait_after_reset(&ap->link, devmask, deadline);
> +}
> +
> +static void ep93xx_pata_release_dma(struct ep93xx_pata_data *drv_data)
> +{
> + if (drv_data->dma_rx_channel) {
> + dma_release_channel(drv_data->dma_rx_channel);
> + drv_data->dma_rx_channel = NULL;
> + }
> + if (drv_data->dma_tx_channel) {
> + dma_release_channel(drv_data->dma_tx_channel);
> + drv_data->dma_tx_channel = NULL;
> + }
> +}
> +
> +static bool ep93xx_pata_dma_filter(struct dma_chan *chan, void *filter_param)
> +{
> + if (ep93xx_dma_chan_is_m2p(chan))
> + return false;
> +
> + chan->private = filter_param;
> + return true;
> +}
> +
> +static void ep93xx_pata_dma_init(struct ep93xx_pata_data *drv_data)
> +{
> + const struct platform_device *pdev = drv_data->pdev;
> + dma_cap_mask_t mask;
> + struct dma_slave_config conf;
> +
> + dma_cap_zero(mask);
> + dma_cap_set(DMA_SLAVE, mask);
> +
> + /*
> + * Request two channels for IDE. Another possibility would be
> + * to request only one channel, and reprogram it's direction at
> + * start of new transfer.
> + */
> + drv_data->dma_rx_data.port = EP93XX_DMA_IDE;
> + drv_data->dma_rx_data.direction = DMA_FROM_DEVICE;
> + drv_data->dma_rx_data.name = "ep93xx-pata-rx";
> + drv_data->dma_rx_channel = dma_request_channel(mask,
> + ep93xx_pata_dma_filter, &drv_data->dma_rx_data);
> + if (!drv_data->dma_rx_channel)
> + return;
> +
> + drv_data->dma_tx_data.port = EP93XX_DMA_IDE;
> + drv_data->dma_tx_data.direction = DMA_TO_DEVICE;
> + drv_data->dma_tx_data.name = "ep93xx-pata-tx";
> + drv_data->dma_tx_channel = dma_request_channel(mask,
> + ep93xx_pata_dma_filter, &drv_data->dma_tx_data);
> + if (!drv_data->dma_tx_channel) {
> + dma_release_channel(drv_data->dma_rx_channel);
> + return;
> + }
> +
> + /* Configure receive channel direction and source address */
> + memset(&conf, 0, sizeof(conf));
> + conf.direction = DMA_FROM_DEVICE;
> + conf.src_addr = drv_data->udma_in_phys;
> + conf.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> + if (dmaengine_slave_config(drv_data->dma_rx_channel, &conf)) {
> + dev_err(&pdev->dev, "failed to configure rx dma channel\n");
> + ep93xx_pata_release_dma(drv_data);
> + return;
> + }
> +
> + /* Configure transmit channel direction and destination address */
> + memset(&conf, 0, sizeof(conf));
> + conf.direction = DMA_TO_DEVICE;
> + conf.dst_addr = drv_data->udma_out_phys;
> + conf.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> + if (dmaengine_slave_config(drv_data->dma_tx_channel, &conf)) {
> + dev_err(&pdev->dev, "failed to configure tx dma channel\n");
> + ep93xx_pata_release_dma(drv_data);
> + }
> +}
> +
> +static void ep93xx_pata_dma_start(struct ata_queued_cmd *qc)
> +{
> + struct dma_async_tx_descriptor *txd;
> + struct ep93xx_pata_data *drv_data = qc->ap->host->private_data;
> + void __iomem *base = drv_data->ide_base;
> + struct ata_device *adev = qc->dev;
> + u32 v = qc->dma_dir == DMA_TO_DEVICE ? IDEUDMAOP_RWOP : 0;
> + struct dma_chan *channel = qc->dma_dir == DMA_TO_DEVICE
> + ? drv_data->dma_tx_channel : drv_data->dma_rx_channel;
> +
> + txd = channel->device->device_prep_slave_sg(channel, qc->sg,
> + qc->n_elem, qc->dma_dir, DMA_CTRL_ACK, NULL);
> + if (!txd) {
> + dev_err(qc->ap->dev, "failed to prepare slave for sg dma\n");
> + return;
> + }
> + txd->callback = NULL;
> + txd->callback_param = NULL;
> +
> + if (dmaengine_submit(txd) < 0) {
> + dev_err(qc->ap->dev, "failed to submit dma transfer\n");
> + return;
> + }
> + dma_async_issue_pending(channel);
> +
> + /*
> + * When enabling UDMA operation, IDEUDMAOP register needs to be
> + * programmed in three step sequence:
> + * 1) set or clear the RWOP bit,
> + * 2) perform dummy read of the register,
> + * 3) set the UEN bit.
> + */
> + writel(v, base + IDEUDMAOP);
> + readl(base + IDEUDMAOP);
> + writel(v | IDEUDMAOP_UEN, base + IDEUDMAOP);
> +
> + writel(IDECFG_IDEEN | IDECFG_UDMA |
> + ((adev->xfer_mode - XFER_UDMA_0) << IDECFG_MODE_SHIFT),
> + base + IDECFG);
> +}
> +
> +static void ep93xx_pata_dma_stop(struct ata_queued_cmd *qc)
> +{
> + struct ep93xx_pata_data *drv_data = qc->ap->host->private_data;
> + void __iomem *base = drv_data->ide_base;
> +
> + /* terminate all dma transfers, if not yet finished */
> + dmaengine_terminate_all(drv_data->dma_rx_channel);
> + dmaengine_terminate_all(drv_data->dma_tx_channel);
> +
> + /*
> + * To properly stop IDE-DMA, IDEUDMAOP register must to be cleared
> + * and IDECTRL register must be set to default value.
> + */
> + writel(0, base + IDEUDMAOP);
> + writel(readl(base + IDECTRL) | IDECTRL_DIOWN | IDECTRL_DIORN |
> + IDECTRL_CS0N | IDECTRL_CS1N, base + IDECTRL);
> +
> + ep93xx_pata_enable_pio(drv_data->ide_base,
> + qc->dev->pio_mode - XFER_PIO_0);
> +
> + ata_sff_dma_pause(qc->ap);
> +}
> +
> +static void ep93xx_pata_dma_setup(struct ata_queued_cmd *qc)
> +{
> + qc->ap->ops->sff_exec_command(qc->ap, &qc->tf);
> +}
> +
> +static u8 ep93xx_pata_dma_status(struct ata_port *ap)
> +{
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> + u32 val = readl(drv_data->ide_base + IDEUDMASTS);
> +
> + /*
> + * UDMA Status Register bits:
> + *
> + * DMAIDE - DMA request signal from UDMA state machine,
> + * INTIDE - INT line generated by UDMA because of errors in the
> + * state machine,
> + * SBUSY - UDMA state machine busy, not in idle state,
> + * NDO - error for data-out not completed,
> + * NDI - error for data-in not completed,
> + * N4X - error for data transferred not multiplies of four
> + * 32-bit words.
> + * (EP93xx UG p27-17)
> + */
> + if (val & IDEUDMASTS_NDO || val & IDEUDMASTS_NDI ||
> + val & IDEUDMASTS_N4X || val & IDEUDMASTS_INTIDE)
> + return ATA_DMA_ERR;
> +
> + /* read INTRQ (INT[3]) pin input state */
> + if (readl(drv_data->ide_base + IDECTRL) & IDECTRL_INTRQ)
> + return ATA_DMA_INTR;
> +
> + if (val & IDEUDMASTS_SBUSY || val & IDEUDMASTS_DMAIDE)
> + return ATA_DMA_ACTIVE;
> +
> + return 0;
> +}
> +
> +/* Note: original code is ata_sff_softreset */
> +static int ep93xx_pata_softreset(struct ata_link *al, unsigned int *classes,
> + unsigned long deadline)
> +{
> + struct ata_port *ap = al->ap;
> + unsigned int slave_possible = ap->flags & ATA_FLAG_SLAVE_POSS;
> + unsigned int devmask = 0;
> + int rc;
> + u8 err;
> +
> + DPRINTK("ENTER\n");
> +
> + /* determine if device 0/1 are present */
> + if (ep93xx_pata_device_is_present(ap, 0))
> + devmask |= (1 << 0);
> + if (slave_possible && ep93xx_pata_device_is_present(ap, 1))
> + devmask |= (1 << 1);
> +
> + /* select device 0 again */
> + ap->ops->sff_dev_select(al->ap, 0);
> +
> + /* issue bus reset */
> + DPRINTK("about to softreset, devmask=%x\n", devmask);
> + rc = ep93xx_pata_bus_softreset(ap, devmask, deadline);
> + /* if link is ocuppied, -ENODEV too is an error */
> + if (rc && (rc != -ENODEV || sata_scr_valid(al))) {
> + ata_link_printk(al, KERN_ERR, "SRST failed (errno=%d)\n",
> + rc);
> + return rc;
> + }
> +
> + /* determine by signature whether we have ATA or ATAPI devices */
> + classes[0] = ata_sff_dev_classify(&al->device[0], devmask & (1 << 0),
> + &err);
> + if (slave_possible && err != 0x81)
> + classes[1] = ata_sff_dev_classify(&al->device[1],
> + devmask & (1 << 1), &err);
> +
> + DPRINTK("EXIT, classes[0]=%u, [1]=%u\n", classes[0],
> + classes[1]);
> + return 0;
> +}
> +
> +/* Note: original code is ata_sff_drain_fifo */
> +void ep93xx_pata_drain_fifo(struct ata_queued_cmd *qc)
> +{
> + int count;
> + struct ata_port *ap;
> + struct ep93xx_pata_data *drv_data;
> +
> + /* We only need to flush incoming data when a command was running */
> + if (qc == NULL || qc->dma_dir == DMA_TO_DEVICE)
> + return;
> +
> + ap = qc->ap;
> + drv_data = ap->host->private_data;
> + /* Drain up to 64K of data before we give up this recovery method */
> + for (count = 0; (ap->ops->sff_check_status(ap) & ATA_DRQ)
> + && count < 65536; count += 2)
> + ep93xx_pata_read(drv_data, IDECTRL_ADDR_DATA,
> + drv_data->cmd_t);
> +
> + /* Can become DEBUG later */
> + if (count)
> + ata_port_printk(ap, KERN_DEBUG,
> + "drained %d bytes to clear DRQ.\n", count);
> +
> +}
> +
> +static int ep93xx_pata_port_start(struct ata_port *ap)
> +{
> + struct ep93xx_pata_data *drv_data = ap->host->private_data;
> +
> + drv_data->cmd_t = ata_timing_find_mode(XFER_PIO_0);
> + return 0;
> +}
> +
> +static struct scsi_host_template ep93xx_pata_sht = {
> + ATA_BASE_SHT(DRV_NAME),
> + /* ep93xx dma implementation limit */
> + .sg_tablesize = 32,
> + /* ep93xx dma can't transfer 65536 bytes at once */
> + .dma_boundary = 0x7fff,
> +};
> +
> +static struct ata_port_operations ep93xx_pata_port_ops = {
> + .inherits = &ata_bmdma_port_ops,
> +
> + .qc_prep = ata_noop_qc_prep,
> +
> + .softreset = ep93xx_pata_softreset,
> + .hardreset = ATA_OP_NULL,
> +
> + .sff_dev_select = ep93xx_pata_dev_select,
> + .sff_set_devctl = ep93xx_pata_set_devctl,
> + .sff_check_status = ep93xx_pata_check_status,
> + .sff_check_altstatus = ep93xx_pata_check_altstatus,
> + .sff_tf_load = ep93xx_pata_tf_load,
> + .sff_tf_read = ep93xx_pata_tf_read,
> + .sff_exec_command = ep93xx_pata_exec_command,
> + .sff_data_xfer = ep93xx_pata_data_xfer,
> + .sff_drain_fifo = ep93xx_pata_drain_fifo,
> + .sff_irq_clear = ATA_OP_NULL,
> +
> + .set_piomode = ep93xx_pata_set_piomode,
> +
> + .bmdma_setup = ep93xx_pata_dma_setup,
> + .bmdma_start = ep93xx_pata_dma_start,
> + .bmdma_stop = ep93xx_pata_dma_stop,
> + .bmdma_status = ep93xx_pata_dma_status,
> +
> + .cable_detect = ata_cable_unknown,
> + .port_start = ep93xx_pata_port_start,
> +};
> +
> +static int __devinit ep93xx_pata_probe(struct platform_device *pdev)
> +{
> + struct ep93xx_pata_data *drv_data;
> + struct ata_host *host;
> + struct ata_port *ap;
> + unsigned int irq;
> + struct resource *mem_res;
> + void __iomem *ide_base;
> + int err;
> +
> + err = ep93xx_ide_acquire_gpio(pdev);
> + if (err)
> + return err;
> +
> + /* INT[3] (IRQ_EP93XX_EXT3) line connected as pull down */
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + err = -ENXIO;
> + goto err_rel_gpio;
> + }
> +
> + mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!mem_res) {
> + err = -ENXIO;
> + goto err_rel_gpio;
> + }
> +
> + ide_base = devm_request_and_ioremap(&pdev->dev, mem_res);
> + if (!ide_base) {
> + err = -ENXIO;
> + goto err_rel_gpio;
> + }
> +
> + drv_data = devm_kzalloc(&pdev->dev, sizeof(*drv_data), GFP_KERNEL);
> + if (!drv_data) {
> + err = -ENXIO;
> + goto err_rel_gpio;
> + }
> +
> + platform_set_drvdata(pdev, drv_data);
> + drv_data->pdev = pdev;
> + drv_data->ide_base = ide_base;
> + drv_data->udma_in_phys = mem_res->start + IDEUDMADATAIN;
> + drv_data->udma_out_phys = mem_res->start + IDEUDMADATAOUT;
> + ep93xx_pata_dma_init(drv_data);
> +
> + /* allocate host */
> + host = ata_host_alloc(&pdev->dev, 1);
> + if (!host) {
> + err = -ENXIO;
> + goto err_rel_dma;
> + }
> +
> + ep93xx_pata_clear_regs(ide_base);
> +
> + host->n_ports = 1;
Just a question. Does the single port handle both devices that the ep93xx ide
interface supports?
> + host->private_data = drv_data;
> +
> + ap = host->ports[0];
> + ap->dev = &pdev->dev;
> + ap->ops = &ep93xx_pata_port_ops;
> + ap->flags |= ATA_FLAG_SLAVE_POSS;
> + ap->pio_mask = ATA_PIO4;
> +
> + /*
> + * Maximum UDMA modes:
> + * EP931x rev.E0 - UDMA2
> + * EP931x rev.E1 - UDMA3
> + * EP931x rev.E2 - UDMA4
> + *
> + * MWDMA support was removed from EP931x rev.E2,
> + * so this driver supports only UDMA modes.
> + */
> + if (drv_data->dma_rx_channel && drv_data->dma_tx_channel) {
> + int chip_rev = ep93xx_chip_revision();
> +
> + if (chip_rev == EP93XX_CHIP_REV_E1)
> + ap->udma_mask = ATA_UDMA3;
> + else if (chip_rev == EP93XX_CHIP_REV_E2)
> + ap->udma_mask = ATA_UDMA4;
> + else
> + ap->udma_mask = ATA_UDMA2;
> + }
> +
> + /* defaults, pio 0 */
> + ep93xx_pata_enable_pio(ide_base, 0);
> +
> + dev_info(&pdev->dev, "version " DRV_VERSION "\n");
> +
> + /* activate host */
> + err = ata_host_activate(host, irq, ata_bmdma_interrupt, 0,
> + &ep93xx_pata_sht);
> + if (err == 0)
> + return 0;
> +
> +err_rel_dma:
> + ep93xx_pata_release_dma(drv_data);
> +err_rel_gpio:
> + ep93xx_ide_release_gpio(pdev);
> + return err;
> +}
> +
> +static int __devexit ep93xx_pata_remove(struct platform_device *pdev)
> +{
> + struct ata_host *host = platform_get_drvdata(pdev);
> + struct ep93xx_pata_data *drv_data = host->private_data;
> +
> + ata_host_detach(host);
> + ep93xx_pata_release_dma(drv_data);
> + ep93xx_pata_clear_regs(drv_data->ide_base);
> + ep93xx_ide_release_gpio(pdev);
> + return 0;
> +}
> +
> +static struct platform_driver ep93xx_pata_platform_driver = {
> + .driver = {
> + .name = DRV_NAME,
> + .owner = THIS_MODULE,
> + },
> + .probe = ep93xx_pata_probe,
> + .remove = __devexit_p(ep93xx_pata_remove),
> +};
> +
> +module_platform_driver(ep93xx_pata_platform_driver);
> +
> +MODULE_AUTHOR("Alessandro Zummo, Lennert Buytenhek, Joao Ramos, "
> + "Bartlomiej Zolnierkiewicz, Rafal Prylowski");
> +MODULE_DESCRIPTION("low-level driver for cirrus ep93xx IDE controller");
> +MODULE_LICENSE("GPL");
> +MODULE_VERSION(DRV_VERSION);
> +MODULE_ALIAS("platform:pata_ep93xx");
> Index: linux-2.6/drivers/ata/Kconfig
> ===================================================================
> --- linux-2.6.orig/drivers/ata/Kconfig
> +++ linux-2.6/drivers/ata/Kconfig
> @@ -416,6 +416,15 @@ config PATA_EFAR
>
> If unsure, say N.
>
> +config PATA_EP93XX
> + tristate "Cirrus Logic EP93xx PATA support"
> + depends on ARCH_EP93XX
> + help
> + This option enables support for the PATA controller in
> + the Cirrus Logic EP9312 and EP9315 ARM CPU.
> +
> + If unsure, say N.
> +
> config PATA_HPT366
> tristate "HPT 366/368 PATA support"
> depends on PCI
> Index: linux-2.6/drivers/ata/Makefile
> ===================================================================
> --- linux-2.6.orig/drivers/ata/Makefile
> +++ linux-2.6/drivers/ata/Makefile
> @@ -43,6 +43,7 @@ obj-$(CONFIG_PATA_CS5535) += pata_cs5535
> obj-$(CONFIG_PATA_CS5536) += pata_cs5536.o
> obj-$(CONFIG_PATA_CYPRESS) += pata_cypress.o
> obj-$(CONFIG_PATA_EFAR) += pata_efar.o
> +obj-$(CONFIG_PATA_EP93XX) += pata_ep93xx.o
> obj-$(CONFIG_PATA_HPT366) += pata_hpt366.o
> obj-$(CONFIG_PATA_HPT37X) += pata_hpt37x.o
> obj-$(CONFIG_PATA_HPT3X2N) += pata_hpt3x2n.o
Overall, this looks pretty good. I'll try to examine it more closely later.
Wish I had some actual hardware to test this with...
Regards,
Hartley
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v2 1/3] PATA host controller driver for ep93xx
2012-04-03 18:25 ` H Hartley Sweeten
@ 2012-04-03 20:08 ` Arnd Bergmann
-1 siblings, 0 replies; 51+ messages in thread
From: Arnd Bergmann @ 2012-04-03 20:08 UTC (permalink / raw)
To: linux-arm-kernel
Cc: H Hartley Sweeten, Rafal Prylowski, linux-ide@vger.kernel.org,
Sergei Shtylyov, bzolnier@gmail.com, rmallon@gmail.com,
joao.ramos@inov.pt
On Tuesday 03 April 2012, H Hartley Sweeten wrote:
> On Tuesday, April 03, 2012 7:45 AM, Rafal Prylowski wrote:
> > +
> > + /* UDMA Debug Status Register */
> > + IDEUDMADEBUG = 0x2c,
> > +};
>
> I prefer defines for these. But, the enum usage seems common in the ata drivers
> so...
I like the enum ;-)
> > +
> > +static int ep93xx_pata_check_iordy(void __iomem *base)
> > +{
> > + return !!(readl(base + IDECTRL) & IDECTRL_IORDY);
> > +}
This one could return a bool.
> > +static void ep93xx_pata_wait_for_iordy(void __iomem *base)
> > +{
> > + unsigned long deadline = jiffies + msecs_to_jiffies(1000);
> > + while (!ep93xx_pata_check_iordy(base) &&
> > + time_is_before_jiffies(deadline))
> > + cpu_relax();
> > +}
Much better for a delay than the previous version. However, it's
still a busy wait, which is bad for realtime behavior, especially
since this can potentially take many milliseconds. If possible,
it should have an msleep() or at least cond_resched() instead of
the cpu_relax(). Obviously that will only work when no spinlocks
are held.
> > + if (is_addr) {
> > + ep93xx_pata_write(drv_data, tf->feature,
> > + IDECTRL_ADDR_FEATURE, t);
> > + ep93xx_pata_write(drv_data, tf->nsect, IDECTRL_ADDR_NSECT, t);
> > + ep93xx_pata_write(drv_data, tf->lbal, IDECTRL_ADDR_LBAL, t);
> > + ep93xx_pata_write(drv_data, tf->lbam, IDECTRL_ADDR_LBAM, t);
> > + ep93xx_pata_write(drv_data, tf->lbah, IDECTRL_ADDR_LBAH, t);
> > + VPRINTK("feat 0x%X nsect 0x%X lba 0x%X 0x%X 0x%X\n",
> > + tf->feature, tf->nsect, tf->lbal, tf->lbam, tf->lbah);
> > + }
> > +
> > + if (tf->flags & ATA_TFLAG_DEVICE) {
> > + ep93xx_pata_write(drv_data, tf->device, IDECTRL_ADDR_DEVICE, t);
> > + VPRINTK("device 0x%X\n", tf->device);
> > + }
Although VPRINTK is used in other ATA drivers, I would not recommend it
in new drivers. Instead, I suggest you use dev_err() and similar helpers
that are not specific to one subsystem.
> > +/* Note: original code is ata_sff_data_xfer */
> > +unsigned int ep93xx_pata_data_xfer(struct ata_device *adev, unsigned char *buf,
> > + unsigned int buflen, int rw)
This should be a static function. A device driver with just a single file should
not need any non-static symbols. Same thing for ep93xx_pata_drain_fifo and
possibly a few others.
> > +static int __devinit ep93xx_pata_probe(struct platform_device *pdev)
> > +{
> > + struct ep93xx_pata_data *drv_data;
> > + struct ata_host *host;
> > + struct ata_port *ap;
> > + unsigned int irq;
> > + struct resource *mem_res;
> > + void __iomem *ide_base;
> > + int err;
> > +
> > + err = ep93xx_ide_acquire_gpio(pdev);
> > + if (err)
> > + return err;
Calling into platform code like this is going to make the transition to
device tree harder. I would suggest moving the code from patch 2 here,
and subsequently putting the gpio lines into device tree properties.
when the driver gets converted.
> > @@ -43,6 +43,7 @@ obj-$(CONFIG_PATA_CS5535) += pata_cs5535
> > obj-$(CONFIG_PATA_CS5536) += pata_cs5536.o
> > obj-$(CONFIG_PATA_CYPRESS) += pata_cypress.o
> > obj-$(CONFIG_PATA_EFAR) += pata_efar.o
> > +obj-$(CONFIG_PATA_EP93XX) += pata_ep93xx.o
> > obj-$(CONFIG_PATA_HPT366) += pata_hpt366.o
> > obj-$(CONFIG_PATA_HPT37X) += pata_hpt37x.o
> > obj-$(CONFIG_PATA_HPT3X2N) += pata_hpt3x2n.o
>
> Overall, this looks pretty good. I'll try to examine it more closely later.
I agree. The driver is ok on the whole, my comments are largely nitpicking.
Arnd
^ permalink raw reply [flat|nested] 51+ messages in thread* [PATCH v2 1/3] PATA host controller driver for ep93xx
@ 2012-04-03 20:08 ` Arnd Bergmann
0 siblings, 0 replies; 51+ messages in thread
From: Arnd Bergmann @ 2012-04-03 20:08 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday 03 April 2012, H Hartley Sweeten wrote:
> On Tuesday, April 03, 2012 7:45 AM, Rafal Prylowski wrote:
> > +
> > + /* UDMA Debug Status Register */
> > + IDEUDMADEBUG = 0x2c,
> > +};
>
> I prefer defines for these. But, the enum usage seems common in the ata drivers
> so...
I like the enum ;-)
> > +
> > +static int ep93xx_pata_check_iordy(void __iomem *base)
> > +{
> > + return !!(readl(base + IDECTRL) & IDECTRL_IORDY);
> > +}
This one could return a bool.
> > +static void ep93xx_pata_wait_for_iordy(void __iomem *base)
> > +{
> > + unsigned long deadline = jiffies + msecs_to_jiffies(1000);
> > + while (!ep93xx_pata_check_iordy(base) &&
> > + time_is_before_jiffies(deadline))
> > + cpu_relax();
> > +}
Much better for a delay than the previous version. However, it's
still a busy wait, which is bad for realtime behavior, especially
since this can potentially take many milliseconds. If possible,
it should have an msleep() or at least cond_resched() instead of
the cpu_relax(). Obviously that will only work when no spinlocks
are held.
> > + if (is_addr) {
> > + ep93xx_pata_write(drv_data, tf->feature,
> > + IDECTRL_ADDR_FEATURE, t);
> > + ep93xx_pata_write(drv_data, tf->nsect, IDECTRL_ADDR_NSECT, t);
> > + ep93xx_pata_write(drv_data, tf->lbal, IDECTRL_ADDR_LBAL, t);
> > + ep93xx_pata_write(drv_data, tf->lbam, IDECTRL_ADDR_LBAM, t);
> > + ep93xx_pata_write(drv_data, tf->lbah, IDECTRL_ADDR_LBAH, t);
> > + VPRINTK("feat 0x%X nsect 0x%X lba 0x%X 0x%X 0x%X\n",
> > + tf->feature, tf->nsect, tf->lbal, tf->lbam, tf->lbah);
> > + }
> > +
> > + if (tf->flags & ATA_TFLAG_DEVICE) {
> > + ep93xx_pata_write(drv_data, tf->device, IDECTRL_ADDR_DEVICE, t);
> > + VPRINTK("device 0x%X\n", tf->device);
> > + }
Although VPRINTK is used in other ATA drivers, I would not recommend it
in new drivers. Instead, I suggest you use dev_err() and similar helpers
that are not specific to one subsystem.
> > +/* Note: original code is ata_sff_data_xfer */
> > +unsigned int ep93xx_pata_data_xfer(struct ata_device *adev, unsigned char *buf,
> > + unsigned int buflen, int rw)
This should be a static function. A device driver with just a single file should
not need any non-static symbols. Same thing for ep93xx_pata_drain_fifo and
possibly a few others.
> > +static int __devinit ep93xx_pata_probe(struct platform_device *pdev)
> > +{
> > + struct ep93xx_pata_data *drv_data;
> > + struct ata_host *host;
> > + struct ata_port *ap;
> > + unsigned int irq;
> > + struct resource *mem_res;
> > + void __iomem *ide_base;
> > + int err;
> > +
> > + err = ep93xx_ide_acquire_gpio(pdev);
> > + if (err)
> > + return err;
Calling into platform code like this is going to make the transition to
device tree harder. I would suggest moving the code from patch 2 here,
and subsequently putting the gpio lines into device tree properties.
when the driver gets converted.
> > @@ -43,6 +43,7 @@ obj-$(CONFIG_PATA_CS5535) += pata_cs5535
> > obj-$(CONFIG_PATA_CS5536) += pata_cs5536.o
> > obj-$(CONFIG_PATA_CYPRESS) += pata_cypress.o
> > obj-$(CONFIG_PATA_EFAR) += pata_efar.o
> > +obj-$(CONFIG_PATA_EP93XX) += pata_ep93xx.o
> > obj-$(CONFIG_PATA_HPT366) += pata_hpt366.o
> > obj-$(CONFIG_PATA_HPT37X) += pata_hpt37x.o
> > obj-$(CONFIG_PATA_HPT3X2N) += pata_hpt3x2n.o
>
> Overall, this looks pretty good. I'll try to examine it more closely later.
I agree. The driver is ok on the whole, my comments are largely nitpicking.
Arnd
^ permalink raw reply [flat|nested] 51+ messages in thread* RE: [PATCH v2 1/3] PATA host controller driver for ep93xx
2012-04-03 20:08 ` Arnd Bergmann
@ 2012-04-03 20:37 ` H Hartley Sweeten
-1 siblings, 0 replies; 51+ messages in thread
From: H Hartley Sweeten @ 2012-04-03 20:37 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel@lists.infradead.org
Cc: Rafal Prylowski, linux-ide@vger.kernel.org, Sergei Shtylyov,
bzolnier@gmail.com, rmallon@gmail.com, joao.ramos@inov.pt
On Tuesday, April 03, 2012 1:08 PM, Arnd Bergmann wrote:
> On Tuesday 03 April 2012, H Hartley Sweeten wrote:
>> On Tuesday, April 03, 2012 7:45 AM, Rafal Prylowski wrote:
>>> +static int __devinit ep93xx_pata_probe(struct platform_device *pdev)
>>> +{
>>> + struct ep93xx_pata_data *drv_data;
>>> + struct ata_host *host;
>>> + struct ata_port *ap;
>>> + unsigned int irq;
>>> + struct resource *mem_res;
>>> + void __iomem *ide_base;
>>> + int err;
>>> +
>>> + err = ep93xx_ide_acquire_gpio(pdev);
>>> + if (err)
>>> + return err;
>
> Calling into platform code like this is going to make the transition to
> device tree harder. I would suggest moving the code from patch 2 here,
> and subsequently putting the gpio lines into device tree properties.
> when the driver gets converted.
The SYSCON stuff is not exposed outside of the arch specific mach-ep93xx
directory. I would prefer this stay like it is for now. If/when ep93xx gets
converted to device tree we can address this.
I have been looking over the drivers/pinctrl stuff to see if it's worth creating
a pinctrl-ep93xx.c driver. If I end up doing one, the function above could
probably be replaced with something from the pin mux api.
Regards,
Hartley
^ permalink raw reply [flat|nested] 51+ messages in thread* [PATCH v2 1/3] PATA host controller driver for ep93xx
@ 2012-04-03 20:37 ` H Hartley Sweeten
0 siblings, 0 replies; 51+ messages in thread
From: H Hartley Sweeten @ 2012-04-03 20:37 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday, April 03, 2012 1:08 PM, Arnd Bergmann wrote:
> On Tuesday 03 April 2012, H Hartley Sweeten wrote:
>> On Tuesday, April 03, 2012 7:45 AM, Rafal Prylowski wrote:
>>> +static int __devinit ep93xx_pata_probe(struct platform_device *pdev)
>>> +{
>>> + struct ep93xx_pata_data *drv_data;
>>> + struct ata_host *host;
>>> + struct ata_port *ap;
>>> + unsigned int irq;
>>> + struct resource *mem_res;
>>> + void __iomem *ide_base;
>>> + int err;
>>> +
>>> + err = ep93xx_ide_acquire_gpio(pdev);
>>> + if (err)
>>> + return err;
>
> Calling into platform code like this is going to make the transition to
> device tree harder. I would suggest moving the code from patch 2 here,
> and subsequently putting the gpio lines into device tree properties.
> when the driver gets converted.
The SYSCON stuff is not exposed outside of the arch specific mach-ep93xx
directory. I would prefer this stay like it is for now. If/when ep93xx gets
converted to device tree we can address this.
I have been looking over the drivers/pinctrl stuff to see if it's worth creating
a pinctrl-ep93xx.c driver. If I end up doing one, the function above could
probably be replaced with something from the pin mux api.
Regards,
Hartley
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v2 1/3] PATA host controller driver for ep93xx
2012-04-03 20:37 ` H Hartley Sweeten
@ 2012-04-04 12:50 ` Arnd Bergmann
-1 siblings, 0 replies; 51+ messages in thread
From: Arnd Bergmann @ 2012-04-04 12:50 UTC (permalink / raw)
To: H Hartley Sweeten
Cc: linux-arm-kernel@lists.infradead.org, Rafal Prylowski,
linux-ide@vger.kernel.org, Sergei Shtylyov, bzolnier@gmail.com,
rmallon@gmail.com, joao.ramos@inov.pt
On Tuesday 03 April 2012, H Hartley Sweeten wrote:
> The SYSCON stuff is not exposed outside of the arch specific mach-ep93xx
> directory. I would prefer this stay like it is for now. If/when ep93xx gets
> converted to device tree we can address this.
Ok, fair enough.
> I have been looking over the drivers/pinctrl stuff to see if it's worth creating
> a pinctrl-ep93xx.c driver. If I end up doing one, the function above could
> probably be replaced with something from the pin mux api.
Yes, that sounds like a good long-term plan.
Arnd
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v2 1/3] PATA host controller driver for ep93xx
@ 2012-04-04 12:50 ` Arnd Bergmann
0 siblings, 0 replies; 51+ messages in thread
From: Arnd Bergmann @ 2012-04-04 12:50 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday 03 April 2012, H Hartley Sweeten wrote:
> The SYSCON stuff is not exposed outside of the arch specific mach-ep93xx
> directory. I would prefer this stay like it is for now. If/when ep93xx gets
> converted to device tree we can address this.
Ok, fair enough.
> I have been looking over the drivers/pinctrl stuff to see if it's worth creating
> a pinctrl-ep93xx.c driver. If I end up doing one, the function above could
> probably be replaced with something from the pin mux api.
Yes, that sounds like a good long-term plan.
Arnd
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v2 1/3] PATA host controller driver for ep93xx
2012-04-03 20:08 ` Arnd Bergmann
@ 2012-04-04 12:40 ` Rafal Prylowski
-1 siblings, 0 replies; 51+ messages in thread
From: Rafal Prylowski @ 2012-04-04 12:40 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, H Hartley Sweeten, linux-ide@vger.kernel.org,
Sergei Shtylyov, bzolnier@gmail.com, rmallon@gmail.com,
joao.ramos@inov.pt
On 2012-04-03 22:08, Arnd Bergmann wrote:
>>> +static void ep93xx_pata_wait_for_iordy(void __iomem *base)
>>> +{
>>> + unsigned long deadline = jiffies + msecs_to_jiffies(1000);
>>> + while (!ep93xx_pata_check_iordy(base) &&
>>> + time_is_before_jiffies(deadline))
>>> + cpu_relax();
>>> +}
>
> Much better for a delay than the previous version. However, it's
> still a busy wait, which is bad for realtime behavior, especially
> since this can potentially take many milliseconds. If possible,
> it should have an msleep() or at least cond_resched() instead of
> the cpu_relax(). Obviously that will only work when no spinlocks
> are held.
>
Unfortunately, we can't use msleep() or cond_resched() here
- ep93xx_pata_wait_for_iordy() is called from interrupt handler.
Thanks,
RP
^ permalink raw reply [flat|nested] 51+ messages in thread* [PATCH v2 1/3] PATA host controller driver for ep93xx
@ 2012-04-04 12:40 ` Rafal Prylowski
0 siblings, 0 replies; 51+ messages in thread
From: Rafal Prylowski @ 2012-04-04 12:40 UTC (permalink / raw)
To: linux-arm-kernel
On 2012-04-03 22:08, Arnd Bergmann wrote:
>>> +static void ep93xx_pata_wait_for_iordy(void __iomem *base)
>>> +{
>>> + unsigned long deadline = jiffies + msecs_to_jiffies(1000);
>>> + while (!ep93xx_pata_check_iordy(base) &&
>>> + time_is_before_jiffies(deadline))
>>> + cpu_relax();
>>> +}
>
> Much better for a delay than the previous version. However, it's
> still a busy wait, which is bad for realtime behavior, especially
> since this can potentially take many milliseconds. If possible,
> it should have an msleep() or at least cond_resched() instead of
> the cpu_relax(). Obviously that will only work when no spinlocks
> are held.
>
Unfortunately, we can't use msleep() or cond_resched() here
- ep93xx_pata_wait_for_iordy() is called from interrupt handler.
Thanks,
RP
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v2 1/3] PATA host controller driver for ep93xx
2012-04-04 12:40 ` Rafal Prylowski
@ 2012-04-04 13:23 ` Arnd Bergmann
-1 siblings, 0 replies; 51+ messages in thread
From: Arnd Bergmann @ 2012-04-04 13:23 UTC (permalink / raw)
To: Rafal Prylowski
Cc: linux-arm-kernel, H Hartley Sweeten, linux-ide@vger.kernel.org,
Sergei Shtylyov, bzolnier@gmail.com, rmallon@gmail.com,
joao.ramos@inov.pt
On Wednesday 04 April 2012, Rafal Prylowski wrote:
> On 2012-04-03 22:08, Arnd Bergmann wrote:
> >>> +static void ep93xx_pata_wait_for_iordy(void __iomem *base)
> >>> +{
> >>> + unsigned long deadline = jiffies + msecs_to_jiffies(1000);
> >>> + while (!ep93xx_pata_check_iordy(base) &&
> >>> + time_is_before_jiffies(deadline))
> >>> + cpu_relax();
> >>> +}
> >
> > Much better for a delay than the previous version. However, it's
> > still a busy wait, which is bad for realtime behavior, especially
> > since this can potentially take many milliseconds. If possible,
> > it should have an msleep() or at least cond_resched() instead of
> > the cpu_relax(). Obviously that will only work when no spinlocks
> > are held.
> >
>
> Unfortunately, we can't use msleep() or cond_resched() here
> - ep93xx_pata_wait_for_iordy() is called from interrupt handler.
Well, it also seems inappropriate to have a one second delay timeout
inside of the interrupt handler.
I suppose if you are emulating what a regular read of the status
register does, there isn't much to improve here though.
Arnd
^ permalink raw reply [flat|nested] 51+ messages in thread* [PATCH v2 1/3] PATA host controller driver for ep93xx
@ 2012-04-04 13:23 ` Arnd Bergmann
0 siblings, 0 replies; 51+ messages in thread
From: Arnd Bergmann @ 2012-04-04 13:23 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday 04 April 2012, Rafal Prylowski wrote:
> On 2012-04-03 22:08, Arnd Bergmann wrote:
> >>> +static void ep93xx_pata_wait_for_iordy(void __iomem *base)
> >>> +{
> >>> + unsigned long deadline = jiffies + msecs_to_jiffies(1000);
> >>> + while (!ep93xx_pata_check_iordy(base) &&
> >>> + time_is_before_jiffies(deadline))
> >>> + cpu_relax();
> >>> +}
> >
> > Much better for a delay than the previous version. However, it's
> > still a busy wait, which is bad for realtime behavior, especially
> > since this can potentially take many milliseconds. If possible,
> > it should have an msleep() or at least cond_resched() instead of
> > the cpu_relax(). Obviously that will only work when no spinlocks
> > are held.
> >
>
> Unfortunately, we can't use msleep() or cond_resched() here
> - ep93xx_pata_wait_for_iordy() is called from interrupt handler.
Well, it also seems inappropriate to have a one second delay timeout
inside of the interrupt handler.
I suppose if you are emulating what a regular read of the status
register does, there isn't much to improve here though.
Arnd
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v2 1/3] PATA host controller driver for ep93xx
2012-04-04 13:23 ` Arnd Bergmann
@ 2012-04-04 15:11 ` Rafal Prylowski
-1 siblings, 0 replies; 51+ messages in thread
From: Rafal Prylowski @ 2012-04-04 15:11 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, H Hartley Sweeten, linux-ide@vger.kernel.org,
Sergei Shtylyov, bzolnier@gmail.com, rmallon@gmail.com,
joao.ramos@inov.pt
On 2012-04-04 15:23, Arnd Bergmann wrote:
> On Wednesday 04 April 2012, Rafal Prylowski wrote:
>> On 2012-04-03 22:08, Arnd Bergmann wrote:
>>>>> +static void ep93xx_pata_wait_for_iordy(void __iomem *base)
>>>>> +{
>>>>> + unsigned long deadline = jiffies + msecs_to_jiffies(1000);
>>>>> + while (!ep93xx_pata_check_iordy(base) &&
>>>>> + time_is_before_jiffies(deadline))
>>>>> + cpu_relax();
>>>>> +}
>>>
>>> Much better for a delay than the previous version. However, it's
>>> still a busy wait, which is bad for realtime behavior, especially
>>> since this can potentially take many milliseconds. If possible,
>>> it should have an msleep() or at least cond_resched() instead of
>>> the cpu_relax(). Obviously that will only work when no spinlocks
>>> are held.
>>>
>>
>> Unfortunately, we can't use msleep() or cond_resched() here
>> - ep93xx_pata_wait_for_iordy() is called from interrupt handler.
>
> Well, it also seems inappropriate to have a one second delay timeout
> inside of the interrupt handler.
>
> I suppose if you are emulating what a regular read of the status
> register does, there isn't much to improve here though.
>
> Arnd
According to ATA specification, maximum IORDY pulse width is 1250ns,
so I'll set timeout to 2ms (I should check this before posting the driver..).
RP
^ permalink raw reply [flat|nested] 51+ messages in thread* [PATCH v2 1/3] PATA host controller driver for ep93xx
@ 2012-04-04 15:11 ` Rafal Prylowski
0 siblings, 0 replies; 51+ messages in thread
From: Rafal Prylowski @ 2012-04-04 15:11 UTC (permalink / raw)
To: linux-arm-kernel
On 2012-04-04 15:23, Arnd Bergmann wrote:
> On Wednesday 04 April 2012, Rafal Prylowski wrote:
>> On 2012-04-03 22:08, Arnd Bergmann wrote:
>>>>> +static void ep93xx_pata_wait_for_iordy(void __iomem *base)
>>>>> +{
>>>>> + unsigned long deadline = jiffies + msecs_to_jiffies(1000);
>>>>> + while (!ep93xx_pata_check_iordy(base) &&
>>>>> + time_is_before_jiffies(deadline))
>>>>> + cpu_relax();
>>>>> +}
>>>
>>> Much better for a delay than the previous version. However, it's
>>> still a busy wait, which is bad for realtime behavior, especially
>>> since this can potentially take many milliseconds. If possible,
>>> it should have an msleep() or at least cond_resched() instead of
>>> the cpu_relax(). Obviously that will only work when no spinlocks
>>> are held.
>>>
>>
>> Unfortunately, we can't use msleep() or cond_resched() here
>> - ep93xx_pata_wait_for_iordy() is called from interrupt handler.
>
> Well, it also seems inappropriate to have a one second delay timeout
> inside of the interrupt handler.
>
> I suppose if you are emulating what a regular read of the status
> register does, there isn't much to improve here though.
>
> Arnd
According to ATA specification, maximum IORDY pulse width is 1250ns,
so I'll set timeout to 2ms (I should check this before posting the driver..).
RP
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v2 1/3] PATA host controller driver for ep93xx
2012-04-04 15:11 ` Rafal Prylowski
@ 2012-04-04 15:16 ` Rafal Prylowski
-1 siblings, 0 replies; 51+ messages in thread
From: Rafal Prylowski @ 2012-04-04 15:16 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, H Hartley Sweeten, linux-ide@vger.kernel.org,
Sergei Shtylyov, bzolnier@gmail.com, rmallon@gmail.com,
joao.ramos@inov.pt
On 2012-04-04 17:11, Rafal Prylowski wrote:
> On 2012-04-04 15:23, Arnd Bergmann wrote:
>> On Wednesday 04 April 2012, Rafal Prylowski wrote:
>>> On 2012-04-03 22:08, Arnd Bergmann wrote:
>>>>>> +static void ep93xx_pata_wait_for_iordy(void __iomem *base)
>>>>>> +{
>>>>>> + unsigned long deadline = jiffies + msecs_to_jiffies(1000);
>>>>>> + while (!ep93xx_pata_check_iordy(base) &&
>>>>>> + time_is_before_jiffies(deadline))
>>>>>> + cpu_relax();
>>>>>> +}
>>>>
>>>> Much better for a delay than the previous version. However, it's
>>>> still a busy wait, which is bad for realtime behavior, especially
>>>> since this can potentially take many milliseconds. If possible,
>>>> it should have an msleep() or at least cond_resched() instead of
>>>> the cpu_relax(). Obviously that will only work when no spinlocks
>>>> are held.
>>>>
>>>
>>> Unfortunately, we can't use msleep() or cond_resched() here
>>> - ep93xx_pata_wait_for_iordy() is called from interrupt handler.
>>
>> Well, it also seems inappropriate to have a one second delay timeout
>> inside of the interrupt handler.
>>
>> I suppose if you are emulating what a regular read of the status
>> register does, there isn't much to improve here though.
>>
>> Arnd
>
> According to ATA specification, maximum IORDY pulse width is 1250ns,
> so I'll set timeout to 2ms (I should check this before posting the driver..).
2ms is far too big value... Should be 2us.
RP
^ permalink raw reply [flat|nested] 51+ messages in thread* [PATCH v2 1/3] PATA host controller driver for ep93xx
@ 2012-04-04 15:16 ` Rafal Prylowski
0 siblings, 0 replies; 51+ messages in thread
From: Rafal Prylowski @ 2012-04-04 15:16 UTC (permalink / raw)
To: linux-arm-kernel
On 2012-04-04 17:11, Rafal Prylowski wrote:
> On 2012-04-04 15:23, Arnd Bergmann wrote:
>> On Wednesday 04 April 2012, Rafal Prylowski wrote:
>>> On 2012-04-03 22:08, Arnd Bergmann wrote:
>>>>>> +static void ep93xx_pata_wait_for_iordy(void __iomem *base)
>>>>>> +{
>>>>>> + unsigned long deadline = jiffies + msecs_to_jiffies(1000);
>>>>>> + while (!ep93xx_pata_check_iordy(base) &&
>>>>>> + time_is_before_jiffies(deadline))
>>>>>> + cpu_relax();
>>>>>> +}
>>>>
>>>> Much better for a delay than the previous version. However, it's
>>>> still a busy wait, which is bad for realtime behavior, especially
>>>> since this can potentially take many milliseconds. If possible,
>>>> it should have an msleep() or at least cond_resched() instead of
>>>> the cpu_relax(). Obviously that will only work when no spinlocks
>>>> are held.
>>>>
>>>
>>> Unfortunately, we can't use msleep() or cond_resched() here
>>> - ep93xx_pata_wait_for_iordy() is called from interrupt handler.
>>
>> Well, it also seems inappropriate to have a one second delay timeout
>> inside of the interrupt handler.
>>
>> I suppose if you are emulating what a regular read of the status
>> register does, there isn't much to improve here though.
>>
>> Arnd
>
> According to ATA specification, maximum IORDY pulse width is 1250ns,
> so I'll set timeout to 2ms (I should check this before posting the driver..).
2ms is far too big value... Should be 2us.
RP
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v2 1/3] PATA host controller driver for ep93xx
2012-04-04 15:16 ` Rafal Prylowski
@ 2012-04-04 15:23 ` Arnd Bergmann
-1 siblings, 0 replies; 51+ messages in thread
From: Arnd Bergmann @ 2012-04-04 15:23 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Rafal Prylowski, rmallon@gmail.com, Sergei Shtylyov,
linux-ide@vger.kernel.org, joao.ramos@inov.pt, H Hartley Sweeten,
bzolnier@gmail.com
On Wednesday 04 April 2012, Rafal Prylowski wrote:
> >
> > According to ATA specification, maximum IORDY pulse width is 1250ns,
> > so I'll set timeout to 2ms (I should check this before posting the driver..).
>
> 2ms is far too big value... Should be 2us.
Ok, in this case, don't use jiffies. I guess it was already flawed because
jiffies doesn't get updated while you are in an interrupt handler.
You could use ktime_get to get a high-resolution time stamp to compare to,
but it depends on how accurate your clocksource is.
Arnd
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v2 1/3] PATA host controller driver for ep93xx
@ 2012-04-04 15:23 ` Arnd Bergmann
0 siblings, 0 replies; 51+ messages in thread
From: Arnd Bergmann @ 2012-04-04 15:23 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday 04 April 2012, Rafal Prylowski wrote:
> >
> > According to ATA specification, maximum IORDY pulse width is 1250ns,
> > so I'll set timeout to 2ms (I should check this before posting the driver..).
>
> 2ms is far too big value... Should be 2us.
Ok, in this case, don't use jiffies. I guess it was already flawed because
jiffies doesn't get updated while you are in an interrupt handler.
You could use ktime_get to get a high-resolution time stamp to compare to,
but it depends on how accurate your clocksource is.
Arnd
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v2 1/3] PATA host controller driver for ep93xx
2012-04-04 15:23 ` Arnd Bergmann
@ 2012-04-05 7:52 ` Rafal Prylowski
-1 siblings, 0 replies; 51+ messages in thread
From: Rafal Prylowski @ 2012-04-05 7:52 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, rmallon@gmail.com, Sergei Shtylyov,
linux-ide@vger.kernel.org, joao.ramos@inov.pt, H Hartley Sweeten,
bzolnier@gmail.com
On 2012-04-04 17:23, Arnd Bergmann wrote:
> On Wednesday 04 April 2012, Rafal Prylowski wrote:
>>>
>>> According to ATA specification, maximum IORDY pulse width is 1250ns,
>>> so I'll set timeout to 2ms (I should check this before posting the driver..).
>>
>> 2ms is far too big value... Should be 2us.
>
> Ok, in this case, don't use jiffies. I guess it was already flawed because
> jiffies doesn't get updated while you are in an interrupt handler.
>
> You could use ktime_get to get a high-resolution time stamp to compare to,
> but it depends on how accurate your clocksource is.
Jiffies seem to work. I was wrong with this interrupt handler. Actually
IDE register read/write (and IORDY polling) is called from softirq,
this is only triggered by interrupt. I hope this backtrace confirm this:
BUG: scheduling while atomic: swapper/0/0x40000100
[<c000cb8c>] (unwind_backtrace+0x0/0xf4) from [<c023e1c4>] (__schedule+0x2e0/0x3
30)
[<c023e1c4>] (__schedule+0x2e0/0x330) from [<c0033740>] (__cond_resched+0x24/0x3
4)
[<c0033740>] (__cond_resched+0x24/0x34) from [<c023e2a4>] (_cond_resched+0x38/0x
40)
[<c023e2a4>] (_cond_resched+0x38/0x40) from [<c016d64c>] (ep93xx_pata_wait_for_i
ordy+0x20/0x64)
[<c016d64c>] (ep93xx_pata_wait_for_iordy+0x20/0x64) from [<c016da5c>] (ep93xx_pa
ta_read+0xac/0xb4)
[<c016da5c>] (ep93xx_pata_read+0xac/0xb4) from [<c016daa8>] (ep93xx_pata_check_s
tatus+0x1c/0x28)
[<c016daa8>] (ep93xx_pata_check_status+0x1c/0x28) from [<c016cfa4>] (T.508+0x3c/
0xf0)
[<c016cfa4>] (T.508+0x3c/0xf0) from [<c016d2bc>] (ata_bmdma_qc_issue+0x50/0x19c)
[<c016d2bc>] (ata_bmdma_qc_issue+0x50/0x19c) from [<c015c2ac>] (ata_qc_issue+0x1
98/0x2f8)
[<c015c2ac>] (ata_qc_issue+0x198/0x2f8) from [<c0160c20>] (ata_scsi_translate+0x
98/0x178)
[<c0160c20>] (ata_scsi_translate+0x98/0x178) from [<c0164514>] (ata_scsi_queuecm
d+0x7c/0x230)
[<c0164514>] (ata_scsi_queuecmd+0x7c/0x230) from [<c0149750>] (scsi_dispatch_cmd
+0xd8/0x1b4)
[<c0149750>] (scsi_dispatch_cmd+0xd8/0x1b4) from [<c014ee5c>] (scsi_request_fn+0
x308/0x378)
[<c014ee5c>] (scsi_request_fn+0x308/0x378) from [<c00fb9b4>] (__blk_run_queue+0x
18/0x1c)
[<c00fb9b4>] (__blk_run_queue+0x18/0x1c) from [<c00fb9f4>] (blk_run_queue+0x14/0
x1c)
[<c00fb9f4>] (blk_run_queue+0x14/0x1c) from [<c014e574>] (scsi_run_queue+0xc0/0x
208)
[<c014e574>] (scsi_run_queue+0xc0/0x208) from [<c014f084>] (scsi_next_command+0x
2c/0x38)
[<c014f084>] (scsi_next_command+0x2c/0x38) from [<c014fa54>] (scsi_io_completion
+0x258/0x56c)
[<c014fa54>] (scsi_io_completion+0x258/0x56c) from [<c0102024>] (blk_done_softir
q+0x80/0x98)
[<c0102024>] (blk_done_softirq+0x80/0x98) from [<c0018af8>] (__do_softirq+0x94/0
x150)
[<c0018af8>] (__do_softirq+0x94/0x150) from [<c0018d00>] (irq_exit+0x48/0x50)
[<c0018d00>] (irq_exit+0x48/0x50) from [<c00097c0>] (handle_IRQ+0x34/0x84)
[<c00097c0>] (handle_IRQ+0x34/0x84) from [<c000849c>] (vic_handle_irq+0x94/0xd4)
[<c000849c>] (vic_handle_irq+0x94/0xd4) from [<c0008c14>] (__irq_svc+0x34/0x40)
Sorry for the mess. I'm doing kernel development only occasionally...
I changed msecs_to_jiffies() to usecs_to_jiffies(). Is it possible that
ktime_get() give us better resolution?
Thanks for all hints.
RP
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v2 1/3] PATA host controller driver for ep93xx
@ 2012-04-05 7:52 ` Rafal Prylowski
0 siblings, 0 replies; 51+ messages in thread
From: Rafal Prylowski @ 2012-04-05 7:52 UTC (permalink / raw)
To: linux-arm-kernel
On 2012-04-04 17:23, Arnd Bergmann wrote:
> On Wednesday 04 April 2012, Rafal Prylowski wrote:
>>>
>>> According to ATA specification, maximum IORDY pulse width is 1250ns,
>>> so I'll set timeout to 2ms (I should check this before posting the driver..).
>>
>> 2ms is far too big value... Should be 2us.
>
> Ok, in this case, don't use jiffies. I guess it was already flawed because
> jiffies doesn't get updated while you are in an interrupt handler.
>
> You could use ktime_get to get a high-resolution time stamp to compare to,
> but it depends on how accurate your clocksource is.
Jiffies seem to work. I was wrong with this interrupt handler. Actually
IDE register read/write (and IORDY polling) is called from softirq,
this is only triggered by interrupt. I hope this backtrace confirm this:
BUG: scheduling while atomic: swapper/0/0x40000100
[<c000cb8c>] (unwind_backtrace+0x0/0xf4) from [<c023e1c4>] (__schedule+0x2e0/0x3
30)
[<c023e1c4>] (__schedule+0x2e0/0x330) from [<c0033740>] (__cond_resched+0x24/0x3
4)
[<c0033740>] (__cond_resched+0x24/0x34) from [<c023e2a4>] (_cond_resched+0x38/0x
40)
[<c023e2a4>] (_cond_resched+0x38/0x40) from [<c016d64c>] (ep93xx_pata_wait_for_i
ordy+0x20/0x64)
[<c016d64c>] (ep93xx_pata_wait_for_iordy+0x20/0x64) from [<c016da5c>] (ep93xx_pa
ta_read+0xac/0xb4)
[<c016da5c>] (ep93xx_pata_read+0xac/0xb4) from [<c016daa8>] (ep93xx_pata_check_s
tatus+0x1c/0x28)
[<c016daa8>] (ep93xx_pata_check_status+0x1c/0x28) from [<c016cfa4>] (T.508+0x3c/
0xf0)
[<c016cfa4>] (T.508+0x3c/0xf0) from [<c016d2bc>] (ata_bmdma_qc_issue+0x50/0x19c)
[<c016d2bc>] (ata_bmdma_qc_issue+0x50/0x19c) from [<c015c2ac>] (ata_qc_issue+0x1
98/0x2f8)
[<c015c2ac>] (ata_qc_issue+0x198/0x2f8) from [<c0160c20>] (ata_scsi_translate+0x
98/0x178)
[<c0160c20>] (ata_scsi_translate+0x98/0x178) from [<c0164514>] (ata_scsi_queuecm
d+0x7c/0x230)
[<c0164514>] (ata_scsi_queuecmd+0x7c/0x230) from [<c0149750>] (scsi_dispatch_cmd
+0xd8/0x1b4)
[<c0149750>] (scsi_dispatch_cmd+0xd8/0x1b4) from [<c014ee5c>] (scsi_request_fn+0
x308/0x378)
[<c014ee5c>] (scsi_request_fn+0x308/0x378) from [<c00fb9b4>] (__blk_run_queue+0x
18/0x1c)
[<c00fb9b4>] (__blk_run_queue+0x18/0x1c) from [<c00fb9f4>] (blk_run_queue+0x14/0
x1c)
[<c00fb9f4>] (blk_run_queue+0x14/0x1c) from [<c014e574>] (scsi_run_queue+0xc0/0x
208)
[<c014e574>] (scsi_run_queue+0xc0/0x208) from [<c014f084>] (scsi_next_command+0x
2c/0x38)
[<c014f084>] (scsi_next_command+0x2c/0x38) from [<c014fa54>] (scsi_io_completion
+0x258/0x56c)
[<c014fa54>] (scsi_io_completion+0x258/0x56c) from [<c0102024>] (blk_done_softir
q+0x80/0x98)
[<c0102024>] (blk_done_softirq+0x80/0x98) from [<c0018af8>] (__do_softirq+0x94/0
x150)
[<c0018af8>] (__do_softirq+0x94/0x150) from [<c0018d00>] (irq_exit+0x48/0x50)
[<c0018d00>] (irq_exit+0x48/0x50) from [<c00097c0>] (handle_IRQ+0x34/0x84)
[<c00097c0>] (handle_IRQ+0x34/0x84) from [<c000849c>] (vic_handle_irq+0x94/0xd4)
[<c000849c>] (vic_handle_irq+0x94/0xd4) from [<c0008c14>] (__irq_svc+0x34/0x40)
Sorry for the mess. I'm doing kernel development only occasionally...
I changed msecs_to_jiffies() to usecs_to_jiffies(). Is it possible that
ktime_get() give us better resolution?
Thanks for all hints.
RP
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v2 1/3] PATA host controller driver for ep93xx
2012-04-05 7:52 ` Rafal Prylowski
@ 2012-04-05 9:07 ` Rafal Prylowski
-1 siblings, 0 replies; 51+ messages in thread
From: Rafal Prylowski @ 2012-04-05 9:07 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, rmallon@gmail.com, Sergei Shtylyov,
linux-ide@vger.kernel.org, joao.ramos@inov.pt, H Hartley Sweeten,
bzolnier@gmail.com
On 2012-04-05 09:52, Rafal Prylowski wrote:
> Jiffies seem to work. I was wrong with this interrupt handler. Actually
> IDE register read/write (and IORDY polling) is called from softirq,
Well, you were right... Jiffies doesn't work, I thought it work
because I wrongly selected time_is_before_jiffies instead of
time_is_after_jiffies... I will try to use ktime_get.
Thanks,
RP
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v2 1/3] PATA host controller driver for ep93xx
@ 2012-04-05 9:07 ` Rafal Prylowski
0 siblings, 0 replies; 51+ messages in thread
From: Rafal Prylowski @ 2012-04-05 9:07 UTC (permalink / raw)
To: linux-arm-kernel
On 2012-04-05 09:52, Rafal Prylowski wrote:
> Jiffies seem to work. I was wrong with this interrupt handler. Actually
> IDE register read/write (and IORDY polling) is called from softirq,
Well, you were right... Jiffies doesn't work, I thought it work
because I wrongly selected time_is_before_jiffies instead of
time_is_after_jiffies... I will try to use ktime_get.
Thanks,
RP
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v2 1/3] PATA host controller driver for ep93xx
2012-04-03 18:25 ` H Hartley Sweeten
@ 2012-04-04 7:39 ` Rafal Prylowski
-1 siblings, 0 replies; 51+ messages in thread
From: Rafal Prylowski @ 2012-04-04 7:39 UTC (permalink / raw)
To: H Hartley Sweeten
Cc: linux-ide@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
joao.ramos@inov.pt, rmallon@gmail.com, Sergei Shtylyov,
bzolnier@gmail.com
On 2012-04-03 20:25, H Hartley Sweeten wrote:
>> + /* allocate host */
>> + host = ata_host_alloc(&pdev->dev, 1);
>> + if (!host) {
>> + err = -ENXIO;
>> + goto err_rel_dma;
>> + }
>> +
>> + ep93xx_pata_clear_regs(ide_base);
>> +
>> + host->n_ports = 1;
>
> Just a question. Does the single port handle both devices that the ep93xx ide
> interface supports?
Yes, single port should handle both master and slave devices on one cable,
provided that ATA_FLAG_SLAVE_POSS is set in port flags:
>> + host->private_data = drv_data;
>> +
>> + ap = host->ports[0];
>> + ap->dev = &pdev->dev;
>> + ap->ops = &ep93xx_pata_port_ops;
>> + ap->flags |= ATA_FLAG_SLAVE_POSS;
Btw, I noticed that host->n_ports is already set in ata_host_alloc, so I'll
remove unneeded line from the driver.
I just tested two disks on one cable, and it works correctly (my previous tests
were limited to only one disk or CF card).
Thanks,
RP
^ permalink raw reply [flat|nested] 51+ messages in thread* [PATCH v2 1/3] PATA host controller driver for ep93xx
@ 2012-04-04 7:39 ` Rafal Prylowski
0 siblings, 0 replies; 51+ messages in thread
From: Rafal Prylowski @ 2012-04-04 7:39 UTC (permalink / raw)
To: linux-arm-kernel
On 2012-04-03 20:25, H Hartley Sweeten wrote:
>> + /* allocate host */
>> + host = ata_host_alloc(&pdev->dev, 1);
>> + if (!host) {
>> + err = -ENXIO;
>> + goto err_rel_dma;
>> + }
>> +
>> + ep93xx_pata_clear_regs(ide_base);
>> +
>> + host->n_ports = 1;
>
> Just a question. Does the single port handle both devices that the ep93xx ide
> interface supports?
Yes, single port should handle both master and slave devices on one cable,
provided that ATA_FLAG_SLAVE_POSS is set in port flags:
>> + host->private_data = drv_data;
>> +
>> + ap = host->ports[0];
>> + ap->dev = &pdev->dev;
>> + ap->ops = &ep93xx_pata_port_ops;
>> + ap->flags |= ATA_FLAG_SLAVE_POSS;
Btw, I noticed that host->n_ports is already set in ata_host_alloc, so I'll
remove unneeded line from the driver.
I just tested two disks on one cable, and it works correctly (my previous tests
were limited to only one disk or CF card).
Thanks,
RP
^ permalink raw reply [flat|nested] 51+ messages in thread
* RE: [PATCH v2 1/3] PATA host controller driver for ep93xx
2012-04-03 14:45 ` Rafal Prylowski
@ 2012-04-03 18:55 ` H Hartley Sweeten
-1 siblings, 0 replies; 51+ messages in thread
From: H Hartley Sweeten @ 2012-04-03 18:55 UTC (permalink / raw)
To: Rafal Prylowski, linux-ide@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org, joao.ramos@inov.pt,
rmallon@gmail.com, Sergei Shtylyov, bzolnier@gmail.com
On Tuesday, April 03, 2012 7:45 AM, Rafal Prylowski wrote:
<snip>
> +static void ep93xx_pata_dma_init(struct ep93xx_pata_data *drv_data)
> +{
> + const struct platform_device *pdev = drv_data->pdev;
> + dma_cap_mask_t mask;
> + struct dma_slave_config conf;
> +
> + dma_cap_zero(mask);
> + dma_cap_set(DMA_SLAVE, mask);
> +
> + /*
> + * Request two channels for IDE. Another possibility would be
> + * to request only one channel, and reprogram it's direction at
> + * start of new transfer.
> + */
> + drv_data->dma_rx_data.port = EP93XX_DMA_IDE;
> + drv_data->dma_rx_data.direction = DMA_FROM_DEVICE;
> + drv_data->dma_rx_data.name = "ep93xx-pata-rx";
> + drv_data->dma_rx_channel = dma_request_channel(mask,
> + ep93xx_pata_dma_filter, &drv_data->dma_rx_data);
> + if (!drv_data->dma_rx_channel)
> + return;
> +
> + drv_data->dma_tx_data.port = EP93XX_DMA_IDE;
> + drv_data->dma_tx_data.direction = DMA_TO_DEVICE;
> + drv_data->dma_tx_data.name = "ep93xx-pata-tx";
> + drv_data->dma_tx_channel = dma_request_channel(mask,
> + ep93xx_pata_dma_filter, &drv_data->dma_tx_data);
> + if (!drv_data->dma_tx_channel) {
> + dma_release_channel(drv_data->dma_rx_channel);
> + return;
> + }
> +
> + /* Configure receive channel direction and source address */
> + memset(&conf, 0, sizeof(conf));
> + conf.direction = DMA_FROM_DEVICE;
> + conf.src_addr = drv_data->udma_in_phys;
> + conf.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> + if (dmaengine_slave_config(drv_data->dma_rx_channel, &conf)) {
> + dev_err(&pdev->dev, "failed to configure rx dma channel\n");
> + ep93xx_pata_release_dma(drv_data);
> + return;
> + }
> +
> + /* Configure transmit channel direction and destination address */
> + memset(&conf, 0, sizeof(conf));
> + conf.direction = DMA_TO_DEVICE;
> + conf.dst_addr = drv_data->udma_out_phys;
> + conf.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> + if (dmaengine_slave_config(drv_data->dma_tx_channel, &conf)) {
> + dev_err(&pdev->dev, "failed to configure tx dma channel\n");
> + ep93xx_pata_release_dma(drv_data);
> + }
> +}
If the dma init fails does the driver fall back to pio mode correctly?
<snip>
> +static struct ata_port_operations ep93xx_pata_port_ops = {
> + .inherits = &ata_bmdma_port_ops,
> +
> + .qc_prep = ata_noop_qc_prep,
> +
> + .softreset = ep93xx_pata_softreset,
> + .hardreset = ATA_OP_NULL,
> +
> + .sff_dev_select = ep93xx_pata_dev_select,
> + .sff_set_devctl = ep93xx_pata_set_devctl,
> + .sff_check_status = ep93xx_pata_check_status,
> + .sff_check_altstatus = ep93xx_pata_check_altstatus,
> + .sff_tf_load = ep93xx_pata_tf_load,
> + .sff_tf_read = ep93xx_pata_tf_read,
> + .sff_exec_command = ep93xx_pata_exec_command,
> + .sff_data_xfer = ep93xx_pata_data_xfer,
> + .sff_drain_fifo = ep93xx_pata_drain_fifo,
> + .sff_irq_clear = ATA_OP_NULL,
> +
> + .set_piomode = ep93xx_pata_set_piomode,
> +
> + .bmdma_setup = ep93xx_pata_dma_setup,
> + .bmdma_start = ep93xx_pata_dma_start,
> + .bmdma_stop = ep93xx_pata_dma_stop,
> + .bmdma_status = ep93xx_pata_dma_status,
The bmdma ops pointers are still set if the dma init failed. Should they
be set to NULL?
> +
> + .cable_detect = ata_cable_unknown,
> + .port_start = ep93xx_pata_port_start,
> +};
> +
> +static int __devinit ep93xx_pata_probe(struct platform_device *pdev)
> +{
> + struct ep93xx_pata_data *drv_data;
> + struct ata_host *host;
> + struct ata_port *ap;
> + unsigned int irq;
> + struct resource *mem_res;
> + void __iomem *ide_base;
> + int err;
> +
> + err = ep93xx_ide_acquire_gpio(pdev);
> + if (err)
> + return err;
> +
> + /* INT[3] (IRQ_EP93XX_EXT3) line connected as pull down */
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + err = -ENXIO;
> + goto err_rel_gpio;
> + }
> +
> + mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!mem_res) {
> + err = -ENXIO;
> + goto err_rel_gpio;
> + }
> +
> + ide_base = devm_request_and_ioremap(&pdev->dev, mem_res);
> + if (!ide_base) {
> + err = -ENXIO;
> + goto err_rel_gpio;
> + }
> +
> + drv_data = devm_kzalloc(&pdev->dev, sizeof(*drv_data), GFP_KERNEL);
> + if (!drv_data) {
> + err = -ENXIO;
> + goto err_rel_gpio;
> + }
> +
> + platform_set_drvdata(pdev, drv_data);
> + drv_data->pdev = pdev;
> + drv_data->ide_base = ide_base;
> + drv_data->udma_in_phys = mem_res->start + IDEUDMADATAIN;
> + drv_data->udma_out_phys = mem_res->start + IDEUDMADATAOUT;
> + ep93xx_pata_dma_init(drv_data);
> +
> + /* allocate host */
> + host = ata_host_alloc(&pdev->dev, 1);
> + if (!host) {
> + err = -ENXIO;
> + goto err_rel_dma;
> + }
> +
> + ep93xx_pata_clear_regs(ide_base);
> +
> + host->n_ports = 1;
> + host->private_data = drv_data;
> +
> + ap = host->ports[0];
> + ap->dev = &pdev->dev;
> + ap->ops = &ep93xx_pata_port_ops;
> + ap->flags |= ATA_FLAG_SLAVE_POSS;
> + ap->pio_mask = ATA_PIO4;
> +
> + /*
> + * Maximum UDMA modes:
> + * EP931x rev.E0 - UDMA2
> + * EP931x rev.E1 - UDMA3
> + * EP931x rev.E2 - UDMA4
> + *
> + * MWDMA support was removed from EP931x rev.E2,
> + * so this driver supports only UDMA modes.
> + */
> + if (drv_data->dma_rx_channel && drv_data->dma_tx_channel) {
> + int chip_rev = ep93xx_chip_revision();
> +
> + if (chip_rev == EP93XX_CHIP_REV_E1)
> + ap->udma_mask = ATA_UDMA3;
> + else if (chip_rev == EP93XX_CHIP_REV_E2)
> + ap->udma_mask = ATA_UDMA4;
> + else
> + ap->udma_mask = ATA_UDMA2;
> + }
> +
> + /* defaults, pio 0 */
> + ep93xx_pata_enable_pio(ide_base, 0);
> +
> + dev_info(&pdev->dev, "version " DRV_VERSION "\n");
> +
> + /* activate host */
> + err = ata_host_activate(host, irq, ata_bmdma_interrupt, 0,
> + &ep93xx_pata_sht);
> + if (err == 0)
> + return 0;
> +
> +err_rel_dma:
> + ep93xx_pata_release_dma(drv_data);
> +err_rel_gpio:
> + ep93xx_ide_release_gpio(pdev);
> + return err;
> +}
There are only two m2m dma channels on the ep93xx. They could be already in
use by the spi driver. I just want to make sure that the ide driver will fall back to
pio mode if they are not available.
Regards,
Hartley
^ permalink raw reply [flat|nested] 51+ messages in thread* [PATCH v2 1/3] PATA host controller driver for ep93xx
@ 2012-04-03 18:55 ` H Hartley Sweeten
0 siblings, 0 replies; 51+ messages in thread
From: H Hartley Sweeten @ 2012-04-03 18:55 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday, April 03, 2012 7:45 AM, Rafal Prylowski wrote:
<snip>
> +static void ep93xx_pata_dma_init(struct ep93xx_pata_data *drv_data)
> +{
> + const struct platform_device *pdev = drv_data->pdev;
> + dma_cap_mask_t mask;
> + struct dma_slave_config conf;
> +
> + dma_cap_zero(mask);
> + dma_cap_set(DMA_SLAVE, mask);
> +
> + /*
> + * Request two channels for IDE. Another possibility would be
> + * to request only one channel, and reprogram it's direction at
> + * start of new transfer.
> + */
> + drv_data->dma_rx_data.port = EP93XX_DMA_IDE;
> + drv_data->dma_rx_data.direction = DMA_FROM_DEVICE;
> + drv_data->dma_rx_data.name = "ep93xx-pata-rx";
> + drv_data->dma_rx_channel = dma_request_channel(mask,
> + ep93xx_pata_dma_filter, &drv_data->dma_rx_data);
> + if (!drv_data->dma_rx_channel)
> + return;
> +
> + drv_data->dma_tx_data.port = EP93XX_DMA_IDE;
> + drv_data->dma_tx_data.direction = DMA_TO_DEVICE;
> + drv_data->dma_tx_data.name = "ep93xx-pata-tx";
> + drv_data->dma_tx_channel = dma_request_channel(mask,
> + ep93xx_pata_dma_filter, &drv_data->dma_tx_data);
> + if (!drv_data->dma_tx_channel) {
> + dma_release_channel(drv_data->dma_rx_channel);
> + return;
> + }
> +
> + /* Configure receive channel direction and source address */
> + memset(&conf, 0, sizeof(conf));
> + conf.direction = DMA_FROM_DEVICE;
> + conf.src_addr = drv_data->udma_in_phys;
> + conf.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> + if (dmaengine_slave_config(drv_data->dma_rx_channel, &conf)) {
> + dev_err(&pdev->dev, "failed to configure rx dma channel\n");
> + ep93xx_pata_release_dma(drv_data);
> + return;
> + }
> +
> + /* Configure transmit channel direction and destination address */
> + memset(&conf, 0, sizeof(conf));
> + conf.direction = DMA_TO_DEVICE;
> + conf.dst_addr = drv_data->udma_out_phys;
> + conf.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> + if (dmaengine_slave_config(drv_data->dma_tx_channel, &conf)) {
> + dev_err(&pdev->dev, "failed to configure tx dma channel\n");
> + ep93xx_pata_release_dma(drv_data);
> + }
> +}
If the dma init fails does the driver fall back to pio mode correctly?
<snip>
> +static struct ata_port_operations ep93xx_pata_port_ops = {
> + .inherits = &ata_bmdma_port_ops,
> +
> + .qc_prep = ata_noop_qc_prep,
> +
> + .softreset = ep93xx_pata_softreset,
> + .hardreset = ATA_OP_NULL,
> +
> + .sff_dev_select = ep93xx_pata_dev_select,
> + .sff_set_devctl = ep93xx_pata_set_devctl,
> + .sff_check_status = ep93xx_pata_check_status,
> + .sff_check_altstatus = ep93xx_pata_check_altstatus,
> + .sff_tf_load = ep93xx_pata_tf_load,
> + .sff_tf_read = ep93xx_pata_tf_read,
> + .sff_exec_command = ep93xx_pata_exec_command,
> + .sff_data_xfer = ep93xx_pata_data_xfer,
> + .sff_drain_fifo = ep93xx_pata_drain_fifo,
> + .sff_irq_clear = ATA_OP_NULL,
> +
> + .set_piomode = ep93xx_pata_set_piomode,
> +
> + .bmdma_setup = ep93xx_pata_dma_setup,
> + .bmdma_start = ep93xx_pata_dma_start,
> + .bmdma_stop = ep93xx_pata_dma_stop,
> + .bmdma_status = ep93xx_pata_dma_status,
The bmdma ops pointers are still set if the dma init failed. Should they
be set to NULL?
> +
> + .cable_detect = ata_cable_unknown,
> + .port_start = ep93xx_pata_port_start,
> +};
> +
> +static int __devinit ep93xx_pata_probe(struct platform_device *pdev)
> +{
> + struct ep93xx_pata_data *drv_data;
> + struct ata_host *host;
> + struct ata_port *ap;
> + unsigned int irq;
> + struct resource *mem_res;
> + void __iomem *ide_base;
> + int err;
> +
> + err = ep93xx_ide_acquire_gpio(pdev);
> + if (err)
> + return err;
> +
> + /* INT[3] (IRQ_EP93XX_EXT3) line connected as pull down */
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + err = -ENXIO;
> + goto err_rel_gpio;
> + }
> +
> + mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!mem_res) {
> + err = -ENXIO;
> + goto err_rel_gpio;
> + }
> +
> + ide_base = devm_request_and_ioremap(&pdev->dev, mem_res);
> + if (!ide_base) {
> + err = -ENXIO;
> + goto err_rel_gpio;
> + }
> +
> + drv_data = devm_kzalloc(&pdev->dev, sizeof(*drv_data), GFP_KERNEL);
> + if (!drv_data) {
> + err = -ENXIO;
> + goto err_rel_gpio;
> + }
> +
> + platform_set_drvdata(pdev, drv_data);
> + drv_data->pdev = pdev;
> + drv_data->ide_base = ide_base;
> + drv_data->udma_in_phys = mem_res->start + IDEUDMADATAIN;
> + drv_data->udma_out_phys = mem_res->start + IDEUDMADATAOUT;
> + ep93xx_pata_dma_init(drv_data);
> +
> + /* allocate host */
> + host = ata_host_alloc(&pdev->dev, 1);
> + if (!host) {
> + err = -ENXIO;
> + goto err_rel_dma;
> + }
> +
> + ep93xx_pata_clear_regs(ide_base);
> +
> + host->n_ports = 1;
> + host->private_data = drv_data;
> +
> + ap = host->ports[0];
> + ap->dev = &pdev->dev;
> + ap->ops = &ep93xx_pata_port_ops;
> + ap->flags |= ATA_FLAG_SLAVE_POSS;
> + ap->pio_mask = ATA_PIO4;
> +
> + /*
> + * Maximum UDMA modes:
> + * EP931x rev.E0 - UDMA2
> + * EP931x rev.E1 - UDMA3
> + * EP931x rev.E2 - UDMA4
> + *
> + * MWDMA support was removed from EP931x rev.E2,
> + * so this driver supports only UDMA modes.
> + */
> + if (drv_data->dma_rx_channel && drv_data->dma_tx_channel) {
> + int chip_rev = ep93xx_chip_revision();
> +
> + if (chip_rev == EP93XX_CHIP_REV_E1)
> + ap->udma_mask = ATA_UDMA3;
> + else if (chip_rev == EP93XX_CHIP_REV_E2)
> + ap->udma_mask = ATA_UDMA4;
> + else
> + ap->udma_mask = ATA_UDMA2;
> + }
> +
> + /* defaults, pio 0 */
> + ep93xx_pata_enable_pio(ide_base, 0);
> +
> + dev_info(&pdev->dev, "version " DRV_VERSION "\n");
> +
> + /* activate host */
> + err = ata_host_activate(host, irq, ata_bmdma_interrupt, 0,
> + &ep93xx_pata_sht);
> + if (err == 0)
> + return 0;
> +
> +err_rel_dma:
> + ep93xx_pata_release_dma(drv_data);
> +err_rel_gpio:
> + ep93xx_ide_release_gpio(pdev);
> + return err;
> +}
There are only two m2m dma channels on the ep93xx. They could be already in
use by the spi driver. I just want to make sure that the ide driver will fall back to
pio mode if they are not available.
Regards,
Hartley
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v2 1/3] PATA host controller driver for ep93xx
2012-04-03 18:55 ` H Hartley Sweeten
@ 2012-04-04 7:47 ` Rafal Prylowski
-1 siblings, 0 replies; 51+ messages in thread
From: Rafal Prylowski @ 2012-04-04 7:47 UTC (permalink / raw)
To: H Hartley Sweeten
Cc: linux-ide@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
joao.ramos@inov.pt, rmallon@gmail.com, Sergei Shtylyov,
bzolnier@gmail.com
On 2012-04-03 20:55, H Hartley Sweeten wrote:
>
> If the dma init fails does the driver fall back to pio mode correctly?
>
Yes, udma_mask of ata_port is set only if we successfully requested dma
channels. Otherwise, we are limited to PIO4.
>> + .bmdma_setup = ep93xx_pata_dma_setup,
>> + .bmdma_start = ep93xx_pata_dma_start,
>> + .bmdma_stop = ep93xx_pata_dma_stop,
>> + .bmdma_status = ep93xx_pata_dma_status,
>
> The bmdma ops pointers are still set if the dma init failed. Should they
> be set to NULL?
>
I think it's not needed. Libata will not call them if udma_mask = 0.
> There are only two m2m dma channels on the ep93xx. They could be already in
> use by the spi driver. I just want to make sure that the ide driver will fall back to
> pio mode if they are not available.
>
I tested this by enabling ep93xx spi in dma mode. In this case, pata driver
works in PIO4 mode.
Thanks,
RP
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v2 1/3] PATA host controller driver for ep93xx
@ 2012-04-04 7:47 ` Rafal Prylowski
0 siblings, 0 replies; 51+ messages in thread
From: Rafal Prylowski @ 2012-04-04 7:47 UTC (permalink / raw)
To: linux-arm-kernel
On 2012-04-03 20:55, H Hartley Sweeten wrote:
>
> If the dma init fails does the driver fall back to pio mode correctly?
>
Yes, udma_mask of ata_port is set only if we successfully requested dma
channels. Otherwise, we are limited to PIO4.
>> + .bmdma_setup = ep93xx_pata_dma_setup,
>> + .bmdma_start = ep93xx_pata_dma_start,
>> + .bmdma_stop = ep93xx_pata_dma_stop,
>> + .bmdma_status = ep93xx_pata_dma_status,
>
> The bmdma ops pointers are still set if the dma init failed. Should they
> be set to NULL?
>
I think it's not needed. Libata will not call them if udma_mask = 0.
> There are only two m2m dma channels on the ep93xx. They could be already in
> use by the spi driver. I just want to make sure that the ide driver will fall back to
> pio mode if they are not available.
>
I tested this by enabling ep93xx spi in dma mode. In this case, pata driver
works in PIO4 mode.
Thanks,
RP
^ permalink raw reply [flat|nested] 51+ messages in thread