[PATCH] Palmchip BK3710 IDE driver

[PATCH] Palmchip BK3710 IDE driver

[Anton Salnikov]

This is Palmchip BK3710 IDE controller support for kernel version 2.6.24-rc8.
The IDE controller logic supports PIO, multiword DMA and ultra-DMA modes.
Supports interface to compact Flash (CF) configured in True-IDE mode.

I had to export two functions (ide_dma_exec_cmd and __ide_dma_test_irq) from 
driver/ide/ide-dma.c to get rid of copying them.

Signed-off-by: Anton Salnikov <asalnikov@ru.mvista.com>
---

 drivers/ide/Kconfig           |    8 
 drivers/ide/arm/Makefile      |    1 
 drivers/ide/arm/palm_bk3710.c |  486 ++++++++++++++++++++++++++++++++++++++++++
 drivers/ide/ide-dma.c         |    6 
 drivers/ide/ide-proc.c        |    1 
 include/linux/ide.h           |    4 
 6 files changed, 503 insertions(+), 3 deletions(-)

Index: 2.6.24-rc7.ide/drivers/ide/Kconfig
===================================================================
--- 2.6.24-rc7.ide.orig/drivers/ide/Kconfig
+++ 2.6.24-rc7.ide/drivers/ide/Kconfig
@@ -1008,6 +1008,14 @@ config BLK_DEV_Q40IDE
 	  normally be on; disable it only if you are running a custom hard
 	  drive subsystem through an expansion card.
 
+config BLK_DEV_PALMCHIP_BK3710
+	bool "Palmchip bk3710 IDE controller support"
+	depends on ARCH_DAVINCI
+	select BLK_DEV_IDEDMA_PCI
+	help
+	  Say Y here if you want to support the onchip IDE controller on the
+	  TI DaVinci SoC
+
 config BLK_DEV_MPC8xx_IDE
 	bool "MPC8xx IDE support"
 	depends on 8xx && (LWMON || IVMS8 || IVML24 || TQM8xxL) && IDE=y && 
BLK_DEV_IDE=y && !PPC_MERGE
Index: 2.6.24-rc7.ide/drivers/ide/arm/Makefile
===================================================================
--- 2.6.24-rc7.ide.orig/drivers/ide/arm/Makefile
+++ 2.6.24-rc7.ide/drivers/ide/arm/Makefile
@@ -2,5 +2,6 @@
 obj-$(CONFIG_BLK_DEV_IDE_ICSIDE)	+= icside.o
 obj-$(CONFIG_BLK_DEV_IDE_RAPIDE)	+= rapide.o
 obj-$(CONFIG_BLK_DEV_IDE_BAST)		+= bast-ide.o
+obj-$(CONFIG_BLK_DEV_PALMCHIP_BK3710)	+= palm_bk3710.o
 
 EXTRA_CFLAGS	:= -Idrivers/ide
Index: 2.6.24-rc7.ide/drivers/ide/arm/palm_bk3710.c
===================================================================
--- /dev/null
+++ 2.6.24-rc7.ide/drivers/ide/arm/palm_bk3710.c
@@ -0,0 +1,486 @@
+/*
+ * Palmchip bk3710 IDE controller
+ *
+ * Copyright (C) 2006 Texas Instruments.
+ * Copyright (C) 2007 MontaVista Software, Inc., <source@mvista.com>
+ *
+ * ----------------------------------------------------------------------------
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ * ----------------------------------------------------------------------------
+ Modifications:
+ ver. 1.0: Oct 2005, Swaminathan S
+ -
+ *
+ */
+
+#include <linux/types.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/ioport.h>
+#include <linux/hdreg.h>
+#include <linux/ide.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/clk.h>
+#include <linux/platform_device.h>
+
+/* Offset of the primary interface registers */
+#define IDE_PALM_ATA_PRI_REG_OFFSET 0x1F0
+
+/* Primary Control Offset */
+#define IDE_PALM_ATA_PRI_CTL_OFFSET 0x3F6
+
+/*
+ * PalmChip 3710 IDE Controller UDMA timing structure Definition
+ */
+struct palm_bk3710_udmatiming {
+	unsigned int rptime;	/* Ready to pause time  */
+	unsigned int cycletime;	/* Cycle Time           */
+};
+
+/*
+ * Register Layout Structure for DmaEngine
+ */
+struct palm_bk3710_dmaengineregs {
+	unsigned short bmicp;
+	unsigned short bmisp;
+	unsigned int bmidtp;
+	unsigned short bmics;
+	unsigned short bmiss;
+	unsigned int bmidts;
+};
+
+/*
+ * Register Layout Structure for Config
+ */
+struct palm_bk3710_ideconfigregs {
+	unsigned short idetimp __attribute__((packed));
+	unsigned short idetims __attribute__((packed));
+	unsigned char sidetim;
+	unsigned short slewctl __attribute__((packed));
+	unsigned char idestatus;
+	unsigned short udmactl __attribute__((packed));
+	unsigned short udmatim __attribute__((packed));
+	unsigned char rsvd0[4];
+	unsigned int miscctl __attribute__((packed));
+	unsigned int regstb __attribute__((packed));
+	unsigned int regrcvr __attribute__((packed));
+	unsigned int datstb __attribute__((packed));
+	unsigned int datrcvr __attribute__((packed));
+	unsigned int dmastb __attribute__((packed));
+	unsigned int dmarcvr __attribute__((packed));
+	unsigned int udmastb __attribute__((packed));
+	unsigned int udmatrp __attribute__((packed));
+	unsigned int udmaenv __attribute__((packed));
+	unsigned int iordytmp __attribute__((packed));
+	unsigned int iordytms __attribute__((packed));
+};
+
+/*
+ * Register Layout Structure
+ */
+struct palm_bk3710_ideregs {
+	struct palm_bk3710_dmaengineregs dmaengine;
+	unsigned char rsvd0[48];
+	struct palm_bk3710_ideconfigregs config;
+};
+
+#include "../ide-timing.h"
+
+static ide_hwif_t *palm_bk3710_hwif;
+static struct palm_bk3710_ideregs __iomem *palm_bk3710_base;
+static long ide_palm_clk;
+
+static const struct palm_bk3710_udmatiming palm_bk3710_udmatimings[6] = {
+	{160, 240},		/* UDMA Mode 0 */
+	{125, 160},		/* UDMA Mode 1 */
+	{100, 120},		/* UDMA Mode 2 */
+	{100, 90},		/* UDMA Mode 3 */
+	{85,  60},		/* UDMA Mode 4 */
+	{85,  40}		/* UDMA Mode 5 */
+};
+
+static struct clk *ideclkp;
+
+static void palm_bk3710_setudmamode(unsigned int dev, unsigned int level)
+{
+	char ide_tenv, ide_trp, ide_t0;
+
+	/* DMA Data Setup */
+	ide_t0 = (palm_bk3710_udmatimings[level].cycletime + ide_palm_clk - 1)
+			/ ide_palm_clk - 1;
+	ide_tenv = (20 + ide_palm_clk - 1) / ide_palm_clk - 1;
+	ide_trp = (palm_bk3710_udmatimings[level].rptime + ide_palm_clk - 1)
+			/ ide_palm_clk - 1;
+
+
+	if (!dev) {
+		/* setup master device parameters */
+
+		/* udmatim Register */
+		palm_bk3710_base->config.udmatim &= 0xFFF0;
+		palm_bk3710_base->config.udmatim |= level;
+		/* udmastb Ultra DMA Access Strobe Width */
+		palm_bk3710_base->config.udmastb &= 0xFF00;
+		palm_bk3710_base->config.udmastb |= ide_t0;
+		/* udmatrp Ultra DMA Ready to Pause Time */
+		palm_bk3710_base->config.udmatrp &= 0xFF00;
+		palm_bk3710_base->config.udmatrp |= ide_trp;
+		/* udmaenv Ultra DMA envelop Time */
+		palm_bk3710_base->config.udmaenv &= 0xFF00;
+		palm_bk3710_base->config.udmaenv |= ide_tenv;
+		/* Enable UDMA for Device 0 */
+		palm_bk3710_base->config.udmactl |= 1;
+	} else {
+		/* setup slave device parameters */
+
+		/* udmatim Register */
+		palm_bk3710_base->config.udmatim &= 0xFF0F;
+		palm_bk3710_base->config.udmatim |= (level << 4);
+		/* udmastb Ultra DMA Access Strobe Width */
+		palm_bk3710_base->config.udmastb &= 0xFF;
+		palm_bk3710_base->config.udmastb |= (ide_t0 << 8);
+		/* udmatrp Ultra DMA Ready to Pause Time */
+		palm_bk3710_base->config.udmatrp &= 0xFF;
+		palm_bk3710_base->config.udmatrp |= (ide_trp << 8);
+		/* udmaenv Ultra DMA envelop Time */
+		palm_bk3710_base->config.udmaenv &= 0xFF;
+		palm_bk3710_base->config.udmaenv |= (ide_tenv << 8);
+		/* Enable UDMA for Device 1 */
+		palm_bk3710_base->config.udmactl |= (1 << 1);
+	}
+}
+
+static void palm_bk3710_setdmamode(unsigned int dev, unsigned int cycletime,
+				  unsigned int mode)
+{
+	char ide_td, ide_tkw, ide_t0;
+
+	if (cycletime < ide_timing[mode].cycle)
+		cycletime = ide_timing[mode].cycle;
+
+	/* DMA Data Setup */
+	ide_t0 = (cycletime + ide_palm_clk - 1) / ide_palm_clk;
+	ide_td = (ide_timing[mode].active + ide_palm_clk - 1) / ide_palm_clk;
+	ide_tkw = ide_t0 - ide_td - 1;
+	ide_td -= 1;
+
+	if (!dev) {
+		/* setup master device parameters */
+		palm_bk3710_base->config.dmastb &= 0xFF00;
+		palm_bk3710_base->config.dmastb |= ide_td;
+		palm_bk3710_base->config.dmarcvr &= 0xFF00;
+		palm_bk3710_base->config.dmarcvr |= ide_tkw;
+		/* Disable UDMA for Device 0 */
+		palm_bk3710_base->config.udmactl &= 0xFF02;
+	} else {
+		/* setup slave device parameters */
+		palm_bk3710_base->config.dmastb &= 0xFF;
+		palm_bk3710_base->config.dmastb |= (ide_td << 8);
+		palm_bk3710_base->config.dmarcvr &= 0xFF;
+		palm_bk3710_base->config.dmarcvr |= (ide_tkw << 8);
+		/* Disable UDMA for Device 1 */
+		palm_bk3710_base->config.udmactl &= 0xFF01;
+	}
+}
+
+static void palm_bk3710_setpiomode(ide_drive_t *mate, unsigned int dev,
+				  unsigned int cycletime, unsigned int mode)
+{
+	char ide_t2, ide_t2i, ide_t0;
+
+	/* PIO Data Setup */
+	ide_t0 = (cycletime + ide_palm_clk - 1) / ide_palm_clk;
+	ide_t2 = (ide_timing[19 - mode].active + ide_palm_clk - 1)
+			/ ide_palm_clk;
+
+	ide_t2i = ide_t0 - ide_t2 - 1;
+	ide_t2 -= 1;
+
+	if (!dev) {
+		/* setup master device parameters */
+		palm_bk3710_base->config.datstb &= 0xFF00;
+		palm_bk3710_base->config.datstb |= ide_t2;
+		palm_bk3710_base->config.datrcvr &= 0xFF00;
+		palm_bk3710_base->config.datrcvr |= ide_t2i;
+	} else {
+		/* setup slave device parameters */
+		palm_bk3710_base->config.datstb &= 0xFF;
+		palm_bk3710_base->config.datstb |= (ide_t2 << 8);
+		palm_bk3710_base->config.datrcvr &= 0xFF;
+		palm_bk3710_base->config.datrcvr |= (ide_t2i << 8);
+	}
+
+	if (mate && mate->present) {
+		u8 mode2 = ide_get_best_pio_mode(mate, 255, 4);
+
+		if (mode2 < mode)
+			mode = mode2;
+	}
+
+	/* TASKFILE Setup */
+	ide_t0 = (ide_timing[19 - mode].cyc8b + ide_palm_clk - 1)
+			/ ide_palm_clk;
+	ide_t2 = (ide_timing[19 - mode].act8b + ide_palm_clk - 1)
+			/ ide_palm_clk;
+	ide_t2i = ide_t0 - ide_t2 - 1;
+	ide_t2 -= 1;
+
+	if (!dev) {
+		/* setup master device parameters */
+		palm_bk3710_base->config.regstb &= 0xFF00;
+		palm_bk3710_base->config.regstb |= ide_t2;
+		palm_bk3710_base->config.regrcvr &= 0xFF00;
+		palm_bk3710_base->config.regrcvr |= ide_t2i;
+	} else {
+		/* setup slave device parameters */
+		palm_bk3710_base->config.regstb &= 0xFF;
+		palm_bk3710_base->config.regstb |= (ide_t2 << 8);
+		palm_bk3710_base->config.regrcvr &= 0xFF;
+		palm_bk3710_base->config.regrcvr |= (ide_t2i << 8);
+	}
+}
+
+static void palm_bk3710_set_dma_mode(ide_drive_t *drive, u8 xferspeed)
+{
+	int is_slave = drive->dn & 1;
+
+	switch (xferspeed) {
+	case XFER_UDMA_4:
+	case XFER_UDMA_3:
+	case XFER_UDMA_2:
+	case XFER_UDMA_1:
+	case XFER_UDMA_0:
+		palm_bk3710_setudmamode(is_slave, xferspeed - XFER_UDMA_0);
+		break;
+	case XFER_MW_DMA_2:
+	case XFER_MW_DMA_1:
+	case XFER_MW_DMA_0:
+		{
+			int nspeed = 10 - xferspeed + XFER_MW_DMA_0;
+			unsigned ide_cycle = max(ide_timing[nspeed].cycle,
+					     drive->id->eide_dma_min);
+
+			palm_bk3710_setdmamode(is_slave, ide_cycle, nspeed);
+		}
+		break;
+	}
+}
+
+static void palm_bk3710_set_pio_mode(ide_drive_t *drive, u8 pio)
+{
+	unsigned int cycle_time;
+	int is_slave = drive->dn & 1;
+	ide_drive_t *mate;
+
+	/*
+	 * Get the best PIO Mode supported by the drive
+	 * Obtain the drive PIO data for tuning the Palm Chip registers
+	 */
+	cycle_time = ide_pio_cycle_time(drive, pio);
+	mate = ide_get_paired_drive(drive);
+	palm_bk3710_setpiomode(mate, is_slave, cycle_time, pio);
+}
+
+static void palm_bk3710_chipinit(void)
+{
+	/*
+	 * enable the reset_en of ATA controller so that when ata signals
+	 * are brought out, by writing into device config. at that
+	 * time por_n signal should not be 'Z' and have a stable value.
+	 */
+	palm_bk3710_base->config.miscctl = 0x0300;
+
+	/* wait for some time and deassert the reset of ATA Device. */
+	mdelay(100);
+
+	/* Deassert the Reset */
+	palm_bk3710_base->config.miscctl = 0x0200;
+
+	/*
+	 * Program the IDETIMP Register Value based on the following assumptions
+	 *
+	 * (ATA_IDETIMP_IDEEN		, ENABLE ) |
+	 * (ATA_IDETIMP_SLVTIMEN	, DISABLE) |
+	 * (ATA_IDETIMP_RDYSMPL		, 70NS)    |
+	 * (ATA_IDETIMP_RDYRCVRY	, 50NS)    |
+	 * (ATA_IDETIMP_DMAFTIM1	, PIOCOMP) |
+	 * (ATA_IDETIMP_PREPOST1	, DISABLE) |
+	 * (ATA_IDETIMP_RDYSEN1		, DISABLE) |
+	 * (ATA_IDETIMP_PIOFTIM1	, DISABLE) |
+	 * (ATA_IDETIMP_DMAFTIM0	, PIOCOMP) |
+	 * (ATA_IDETIMP_PREPOST0	, DISABLE) |
+	 * (ATA_IDETIMP_RDYSEN0		, DISABLE) |
+	 * (ATA_IDETIMP_PIOFTIM0	, DISABLE)
+	 */
+
+	palm_bk3710_base->config.idetimp = 0xb388;
+
+	/*
+	 * Configure  SIDETIM  Register
+	 * (ATA_SIDETIM_RDYSMPS1	,120NS ) |
+	 * (ATA_SIDETIM_RDYRCYS1	,120NS )
+	 */
+	palm_bk3710_base->config.sidetim = 0;
+
+	/*
+	 * UDMACTL Ultra-ATA DMA Control
+	 * (ATA_UDMACTL_UDMAP1	, 0 ) |
+	 * (ATA_UDMACTL_UDMAP0	, 0 )
+	 *
+	 */
+	palm_bk3710_base->config.udmactl = 0;
+
+	/*
+	 * MISCCTL Miscellaneous Conrol Register
+	 * (ATA_MISCCTL_RSTMODEP	, 1) |
+	 * (ATA_MISCCTL_RESETP		, 0) |
+	 * (ATA_MISCCTL_TIMORIDE	, 1)
+	 */
+	palm_bk3710_base->config.miscctl = 0x201;
+
+	/*
+	 * IORDYTMP IORDY Timer for Primary Register
+	 * (ATA_IORDYTMP_IORDYTMP     , 0xffff  )
+	 */
+
+	palm_bk3710_base->config.iordytmp = 0xffff;
+
+	/*
+	 * Configure BMISP Register
+	 * (ATA_BMISP_DMAEN1	, DISABLE )	|
+	 * (ATA_BMISP_DMAEN0	, DISABLE )	|
+	 * (ATA_BMISP_IORDYINT	, CLEAR)	|
+	 * (ATA_BMISP_INTRSTAT	, CLEAR)	|
+	 * (ATA_BMISP_DMAERROR	, CLEAR)
+	 */
+
+	palm_bk3710_base->dmaengine.bmisp = 0;
+
+	palm_bk3710_setpiomode(NULL, 0, 0, 0);
+	palm_bk3710_setpiomode(NULL, 1, 0, 0);
+}
+
+int palm_bk3710_probe(struct platform_device *pdev)
+{
+	hw_regs_t ide_ctlr_info;
+	int index = 0;
+	int pribase;
+	struct clk *clkp;
+	struct resource *mem, *irq;
+
+	clkp = clk_get(NULL, "IDECLK");
+	if (IS_ERR(clkp))
+		return -ENODEV;
+
+	ideclkp = clkp;
+	clk_enable(ideclkp);
+	ide_palm_clk = clk_get_rate(ideclkp)/100000;
+	ide_palm_clk = (10000/ide_palm_clk) + 1;
+	/* Register the IDE interface with Linux ATA Interface */
+	memset(&ide_ctlr_info, 0, sizeof(ide_ctlr_info));
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (mem == NULL) {
+		printk(KERN_INFO "failed to get memory region resource\n");
+		return -ENODEV;
+	}
+	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (irq == NULL) {
+		printk(KERN_INFO "failed to get IRQ resource\n");
+		return -ENODEV;
+	}
+
+	palm_bk3710_base = (struct palm_bk3710_ideregs __iomem *)mem->start;
+	/* Configure the Palm Chip controller */
+	palm_bk3710_chipinit();
+
+	pribase = mem->start + IDE_PALM_ATA_PRI_REG_OFFSET;
+	for (index = 0; index < IDE_NR_PORTS - 2; index++)
+		ide_ctlr_info.io_ports[index] = pribase + index;
+	ide_ctlr_info.io_ports[IDE_CONTROL_OFFSET] = mem->start +
+			IDE_PALM_ATA_PRI_CTL_OFFSET;
+	ide_ctlr_info.irq = irq->start;
+	ide_ctlr_info.chipset = ide_palm3710;
+	if (ide_register_hw(&ide_ctlr_info, NULL, 0, &palm_bk3710_hwif) < 0) {
+		printk(KERN_WARNING "Palm Chip BK3710 IDE Register Fail\n");
+		return -ENODEV;
+	}
+
+	palm_bk3710_hwif->set_pio_mode = &palm_bk3710_set_pio_mode;
+	palm_bk3710_hwif->set_dma_mode = &palm_bk3710_set_dma_mode;
+
+	palm_bk3710_hwif->ultra_mask = 0x1f;	/* Ultra DMA Mode 4 Max
+						(input clk 99MHz) */
+	palm_bk3710_hwif->mwdma_mask = 0x7;
+	palm_bk3710_hwif->drives[0].autotune = 1;
+	palm_bk3710_hwif->drives[1].autotune = 1;
+
+	if (!request_region(mem->start, 8, palm_bk3710_hwif->name)) {
+		printk(KERN_ERR "Error, ports in use.\n");
+		return -EBUSY;
+	}
+
+	palm_bk3710_hwif->dmatable_cpu = dma_alloc_coherent(
+				NULL,
+				PRD_ENTRIES * PRD_BYTES,
+				&palm_bk3710_hwif->dmatable_dma,
+				GFP_ATOMIC);
+
+	if (!palm_bk3710_hwif->dmatable_cpu) {
+		printk(KERN_ERR "Error, unable to allocate DMA table.\n");
+		return -ENOMEM;
+	}
+
+	palm_bk3710_hwif->dma_base = mem->start;
+
+	palm_bk3710_hwif->dma_master = mem->start;
+
+	palm_bk3710_hwif->dma_command = mem->start;
+	palm_bk3710_hwif->dma_status = mem->start + 2;
+	palm_bk3710_hwif->dma_prdtable = mem->start + 4;
+
+	palm_bk3710_hwif->dma_off_quietly = &ide_dma_off_quietly;
+	palm_bk3710_hwif->ide_dma_on = &__ide_dma_on;
+	palm_bk3710_hwif->dma_host_off = &ide_dma_host_off;
+	palm_bk3710_hwif->dma_host_on = &ide_dma_host_on;
+	palm_bk3710_hwif->dma_setup = &ide_dma_setup;
+	palm_bk3710_hwif->dma_exec_cmd = &ide_dma_exec_cmd;
+	palm_bk3710_hwif->dma_start = &ide_dma_start;
+	palm_bk3710_hwif->ide_dma_end = &__ide_dma_end;
+	palm_bk3710_hwif->ide_dma_test_irq = &__ide_dma_test_irq;
+	palm_bk3710_hwif->dma_timeout = &ide_dma_timeout;
+	palm_bk3710_hwif->dma_lost_irq = &ide_dma_lost_irq;
+
+	return 0;
+}
+
+static struct platform_driver platform_bk_driver = {
+	.driver = {
+		.name = "palm_bk3710",
+	},
+	.probe = palm_bk3710_probe,
+	.remove = NULL,
+};
+
+int palm_bk3710_init(void)
+{
+	return platform_driver_register(&platform_bk_driver);
+}
+
+module_init(palm_bk3710_init);
+MODULE_LICENSE("GPL");
+
Index: 2.6.24-rc7.ide/drivers/ide/ide-dma.c
===================================================================
--- 2.6.24-rc7.ide.orig/drivers/ide/ide-dma.c
+++ 2.6.24-rc7.ide/drivers/ide/ide-dma.c
@@ -556,11 +556,12 @@ int ide_dma_setup(ide_drive_t *drive)
 
 EXPORT_SYMBOL_GPL(ide_dma_setup);
 
-static void ide_dma_exec_cmd(ide_drive_t *drive, u8 command)
+void ide_dma_exec_cmd(ide_drive_t *drive, u8 command)
 {
 	/* issue cmd to drive */
 	ide_execute_command(drive, command, &ide_dma_intr, 2*WAIT_CMD, 
dma_timer_expiry);
 }
+EXPORT_SYMBOL(ide_dma_exec_cmd);
 
 void ide_dma_start(ide_drive_t *drive)
 {
@@ -606,7 +607,7 @@ int __ide_dma_end (ide_drive_t *drive)
 EXPORT_SYMBOL(__ide_dma_end);
 
 /* returns 1 if dma irq issued, 0 otherwise */
-static int __ide_dma_test_irq(ide_drive_t *drive)
+int __ide_dma_test_irq(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif	= HWIF(drive);
 	u8 dma_stat		= hwif->INB(hwif->dma_status);
@@ -619,6 +620,7 @@ static int __ide_dma_test_irq(ide_drive_
 			drive->name, __FUNCTION__);
 	return 0;
 }
+EXPORT_SYMBOL(__ide_dma_test_irq);
 #else
 static inline int config_drive_for_dma(ide_drive_t *drive) { return 0; }
 #endif /* CONFIG_BLK_DEV_IDEDMA_PCI */
Index: 2.6.24-rc7.ide/drivers/ide/ide-proc.c
===================================================================
--- 2.6.24-rc7.ide.orig/drivers/ide/ide-proc.c
+++ 2.6.24-rc7.ide/drivers/ide/ide-proc.c
@@ -67,6 +67,7 @@ static int proc_ide_read_imodel
 		case ide_4drives:	name = "4drives";	break;
 		case ide_pmac:		name = "mac-io";	break;
 		case ide_au1xxx:	name = "au1xxx";	break;
+		case ide_palm3710:      name = "palm3710";      break;
 		case ide_etrax100:	name = "etrax100";	break;
 		case ide_acorn:		name = "acorn";		break;
 		default:		name = "(unknown)";	break;
Index: 2.6.24-rc7.ide/include/linux/ide.h
===================================================================
--- 2.6.24-rc7.ide.orig/include/linux/ide.h
+++ 2.6.24-rc7.ide/include/linux/ide.h
@@ -202,7 +202,7 @@ enum {		ide_unknown,	ide_generic,	ide_pc
 		ide_rz1000,	ide_trm290,
 		ide_cmd646,	ide_cy82c693,	ide_4drives,
 		ide_pmac,	ide_etrax100,	ide_acorn,
-		ide_au1xxx, ide_forced
+		ide_au1xxx,	ide_palm3710,	ide_forced
 };
 
 typedef u8 hwif_chipset_t;
@@ -1280,8 +1280,10 @@ void ide_dma_off_quietly(ide_drive_t *);
 void ide_dma_host_on(ide_drive_t *);
 extern int __ide_dma_on(ide_drive_t *);
 extern int ide_dma_setup(ide_drive_t *);
+extern void ide_dma_exec_cmd(ide_drive_t *drive, u8 command);
 extern void ide_dma_start(ide_drive_t *);
 extern int __ide_dma_end(ide_drive_t *);
+extern int __ide_dma_test_irq(ide_drive_t *drive);
 extern void ide_dma_lost_irq(ide_drive_t *);
 extern void ide_dma_timeout(ide_drive_t *);
 #endif /* CONFIG_BLK_DEV_IDEDMA_PCI */

Re: [PATCH] Palmchip BK3710 IDE driver

[Sergei Shtylyov]

Hello.

Anton Salnikov wrote:

> This is Palmchip BK3710 IDE controller support for kernel version 2.6.24-rc8.
> The IDE controller logic supports PIO, multiword DMA and ultra-DMA modes.
> Supports interface to compact Flash (CF) configured in True-IDE mode.

> I had to export two functions (ide_dma_exec_cmd and __ide_dma_test_irq) from 
> driver/ide/ide-dma.c to get rid of copying them.

> Signed-off-by: Anton Salnikov <asalnikov@ru.mvista.com>

Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

MBR, Sergei

Re: [PATCH] Palmchip BK3710 IDE driver

[Alan Cox]

On Thu, 17 Jan 2008 21:50:56 +0300
Anton Salnikov <asalnikov@ru.mvista.com> wrote:

> This is Palmchip BK3710 IDE controller support for kernel version 2.6.24-rc8.
> The IDE controller logic supports PIO, multiword DMA and ultra-DMA modes.
> Supports interface to compact Flash (CF) configured in True-IDE mode.

New drivers should really be going at least parallel into drivers/ata and
legacy IDE.

> +struct palm_bk3710_ideconfigregs {
> +	unsigned short idetimp __attribute__((packed));
> +	unsigned short idetims __attribute__((packed));
> +	unsigned char sidetim;
> +	unsigned short slewctl __attribute__((packed));
> +	unsigned char idestatus;
> +	unsigned short udmactl __attribute__((packed));
> +	unsigned short udmatim __attribute__((packed));
> +	unsigned char rsvd0[4];
> +	unsigned int miscctl __attribute__((packed));
> +	unsigned int regstb __attribute__((packed));

NAK - the size of an unsigned int in the kernel isn't defined. All the
packed stuff is also horribly platform dependant. True the driver is ARM
only currently but that doesn't make it a good idea.

Can't you use defined offsets ?


> +struct palm_bk3710_ideregs {
> +	struct palm_bk3710_dmaengineregs dmaengine;

So why are only some packed ?

> +		/* udmatim Register */
> +		palm_bk3710_base->config.udmatim &= 0xFFF0;
> +		palm_bk3710_base->config.udmatim |= level;

Direct memory access to I/O space - should be using read/write functions


> +	if (mate && mate->present) {
> +		u8 mode2 = ide_get_best_pio_mode(mate, 255, 4);

If the pair device is present but not in the best mode it can do this
will be wrong surely ?


> +	if (!request_region(mem->start, 8, palm_bk3710_hwif->name)) {
> +		printk(KERN_ERR "Error, ports in use.\n");
> +		return -EBUSY;
> +	}
> +
> +	palm_bk3710_hwif->dmatable_cpu = dma_alloc_coherent(
> +				NULL,
> +				PRD_ENTRIES * PRD_BYTES,
> +				&palm_bk3710_hwif->dmatable_dma,
> +				GFP_ATOMIC);
> +
> +	if (!palm_bk3710_hwif->dmatable_cpu) {

Leaks the reserved region

> +		printk(KERN_ERR "Error, unable to allocate DMA table.\n");
> +		return -ENOMEM;
> +	}

> -static void ide_dma_exec_cmd(ide_drive_t *drive, u8 command)
> +void ide_dma_exec_cmd(ide_drive_t *drive, u8 command)
>  {
>  	/* issue cmd to drive */
>  	ide_execute_command(drive, command, &ide_dma_intr, 2*WAIT_CMD, 
> dma_timer_expiry);
>  }
> +EXPORT_SYMBOL(ide_dma_exec_cmd);

These are not needed the NULL default request will fill them in for you

Alan

Re: [PATCH] Palmchip BK3710 IDE driver

[Sergei Shtylyov]

Hello.

Alan Cox wrote:

>>This is Palmchip BK3710 IDE controller support for kernel version 2.6.24-rc8.
>>The IDE controller logic supports PIO, multiword DMA and ultra-DMA modes.
>>Supports interface to compact Flash (CF) configured in True-IDE mode.

> New drivers should really be going at least parallel into drivers/ata and
> legacy IDE.

    Alas, it's unlikely that we can spend more time on developing libata 
equivalent which we don't need at this point.

>>+struct palm_bk3710_ideconfigregs {
>>+	unsigned short idetimp __attribute__((packed));
>>+	unsigned short idetims __attribute__((packed));
>>+	unsigned char sidetim;
>>+	unsigned short slewctl __attribute__((packed));
>>+	unsigned char idestatus;
>>+	unsigned short udmactl __attribute__((packed));
>>+	unsigned short udmatim __attribute__((packed));
>>+	unsigned char rsvd0[4];
>>+	unsigned int miscctl __attribute__((packed));
>>+	unsigned int regstb __attribute__((packed));

> NAK - the size of an unsigned int in the kernel isn't defined. All the
> packed stuff is also horribly platform dependant. True the driver is ARM
> only currently but that doesn't make it a good idea.

> Can't you use defined offsets ?

>>+struct palm_bk3710_ideregs {
>>+	struct palm_bk3710_dmaengineregs dmaengine;

> So why are only some packed ?

>>+		/* udmatim Register */
>>+		palm_bk3710_base->config.udmatim &= 0xFFF0;
>>+		palm_bk3710_base->config.udmatim |= level;

> Direct memory access to I/O space - should be using read/write functions

    Sigh, I was anticipationg that somebody would say that... :-)

>>+	if (mate && mate->present) {
>>+		u8 mode2 = ide_get_best_pio_mode(mate, 255, 4);

> If the pair device is present but not in the best mode it can do this
> will be wrong surely ?

    Why?  ATA/PI says in the note to the table "Register transfer to/from device":

5 Mode shall be selected no higher than the *highest* mode supported by the 
*slowest* device.

    Some other drivers are already using the same code (I'm not even sure it's 
needed here -- never saw the chip documentation).

>>+	if (!request_region(mem->start, 8, palm_bk3710_hwif->name)) {
>>+		printk(KERN_ERR "Error, ports in use.\n");
>>+		return -EBUSY;
>>+	}
>>+
>>+	palm_bk3710_hwif->dmatable_cpu = dma_alloc_coherent(
>>+				NULL,
>>+				PRD_ENTRIES * PRD_BYTES,
>>+				&palm_bk3710_hwif->dmatable_dma,
>>+				GFP_ATOMIC);
>>+
>>+	if (!palm_bk3710_hwif->dmatable_cpu) {

> Leaks the reserved region

    My bad -- I've managed to overlook this while reviewing.

>>+		printk(KERN_ERR "Error, unable to allocate DMA table.\n");
>>+		return -ENOMEM;
>>+	}

>>-static void ide_dma_exec_cmd(ide_drive_t *drive, u8 command)
>>+void ide_dma_exec_cmd(ide_drive_t *drive, u8 command)
>> {
>> 	/* issue cmd to drive */
>> 	ide_execute_command(drive, command, &ide_dma_intr, 2*WAIT_CMD, 
>>dma_timer_expiry);
>> }
>>+EXPORT_SYMBOL(ide_dma_exec_cmd);

> These are not needed the NULL default request will fill them in for you

    It won't -- we can *not* call ide_setup_dma() which fills them out as this 
is not a PCI chip.

MBR, Sergei

Re: [PATCH] Palmchip BK3710 IDE driver

[Alan Cox]

> >>+		/* udmatim Register */
> >>+		palm_bk3710_base->config.udmatim &= 0xFFF0;
> >>+		palm_bk3710_base->config.udmatim |= level;
> 
> > Direct memory access to I/O space - should be using read/write functions
> 
>     Sigh, I was anticipationg that somebody would say that... :-)

Well yes - its not very nice code and it isn't really upstream to scratch.

>     Why?  ATA/PI says in the note to the table "Register transfer to/from device":
> 
> 5 Mode shall be selected no higher than the *highest* mode supported by the 
> *slowest* device.

You are correct, ignore that. You don't touch the timing for the other
device.

>     It won't -- we can *not* call ide_setup_dma() which fills them out as this 
> is not a PCI chip.

Gak.

Alan

Re: [PATCH] Palmchip BK3710 IDE driver

[Sergei Shtylyov]

Alan Cox wrote:

>>    It won't -- we can *not* call ide_setup_dma() which fills them out as this 
>>is not a PCI chip.

> Gak.

    Or maybe we still can -- with hwif->mmio set?
    AFAIR (from the 2006 discussion) Bart said it's *only* for PCI devices...

> Alan

MBR, Sergei

Re: [PATCH] Palmchip BK3710 IDE driver

[Bartlomiej Zolnierkiewicz]

Hi,

On Thursday 17 January 2008, Anton Salnikov wrote:
> This is Palmchip BK3710 IDE controller support for kernel version 2.6.24-rc8.
> The IDE controller logic supports PIO, multiword DMA and ultra-DMA modes.
> Supports interface to compact Flash (CF) configured in True-IDE mode.

Thanks, overall it looks good but the way in which controller registers are
accessed needs to be reworked according to Alan's comments before this driver
can be accepted in the mainline.

> I had to export two functions (ide_dma_exec_cmd and __ide_dma_test_irq) from 
> driver/ide/ide-dma.c to get rid of copying them.

Please make the exports _GPL.

Also some minor comments below.

> Signed-off-by: Anton Salnikov <asalnikov@ru.mvista.com>
> ---
> 
>  drivers/ide/Kconfig           |    8 
>  drivers/ide/arm/Makefile      |    1 
>  drivers/ide/arm/palm_bk3710.c |  486 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/ide/ide-dma.c         |    6 
>  drivers/ide/ide-proc.c        |    1 
>  include/linux/ide.h           |    4 
>  6 files changed, 503 insertions(+), 3 deletions(-)
> 
> Index: 2.6.24-rc7.ide/drivers/ide/Kconfig
> ===================================================================
> --- 2.6.24-rc7.ide.orig/drivers/ide/Kconfig
> +++ 2.6.24-rc7.ide/drivers/ide/Kconfig
> @@ -1008,6 +1008,14 @@ config BLK_DEV_Q40IDE
>  	  normally be on; disable it only if you are running a custom hard
>  	  drive subsystem through an expansion card.
>  
> +config BLK_DEV_PALMCHIP_BK3710
> +	bool "Palmchip bk3710 IDE controller support"

tristate?

> +	depends on ARCH_DAVINCI
> +	select BLK_DEV_IDEDMA_PCI
> +	help
> +	  Say Y here if you want to support the onchip IDE controller on the
> +	  TI DaVinci SoC

[...]

> Index: 2.6.24-rc7.ide/drivers/ide/arm/palm_bk3710.c
> ===================================================================
> --- /dev/null
> +++ 2.6.24-rc7.ide/drivers/ide/arm/palm_bk3710.c
> @@ -0,0 +1,486 @@
> +/*
> + * Palmchip bk3710 IDE controller
> + *
> + * Copyright (C) 2006 Texas Instruments.
> + * Copyright (C) 2007 MontaVista Software, Inc., <source@mvista.com>

[...]

> + Modifications:
> + ver. 1.0: Oct 2005, Swaminathan S
> + -

I would prefer revision history to be removed if possible (we keep changelogs
in git tree, also the date doesn't match with Copyrights notice).

[...]

> +static ide_hwif_t *palm_bk3710_hwif;

palm_bk3710_hwif is only accessed in palm_bk3710_probe() so may be as well
moved there (+ renamed to hwif to match usual naming used by other drivers)

> +static struct palm_bk3710_ideregs __iomem *palm_bk3710_base;
> +static long ide_palm_clk;
> +
> +static const struct palm_bk3710_udmatiming palm_bk3710_udmatimings[6] = {
> +	{160, 240},		/* UDMA Mode 0 */
> +	{125, 160},		/* UDMA Mode 1 */
> +	{100, 120},		/* UDMA Mode 2 */
> +	{100, 90},		/* UDMA Mode 3 */
> +	{85,  60},		/* UDMA Mode 4 */
> +	{85,  40}		/* UDMA Mode 5 */
> +};

Hmmm, but palm_bk3710_probe() limits max UDMA to UDMA4...?

> +static struct clk *ideclkp;
> +
> +static void palm_bk3710_setudmamode(unsigned int dev, unsigned int level)

'level' is a bit confusing name for UDMA mode number

> +{
> +	char ide_tenv, ide_trp, ide_t0;

- char -> u8
- "ide_" prefix may as well be dropped

> +	/* DMA Data Setup */
> +	ide_t0 = (palm_bk3710_udmatimings[level].cycletime + ide_palm_clk - 1)
> +			/ ide_palm_clk - 1;
> +	ide_tenv = (20 + ide_palm_clk - 1) / ide_palm_clk - 1;
> +	ide_trp = (palm_bk3710_udmatimings[level].rptime + ide_palm_clk - 1)
> +			/ ide_palm_clk - 1;
> +
> +
> +	if (!dev) {

Since this code needs to be recasted anyway it would be a nice cleanup
to merge '!dev' and 'dev' cases.

> +		/* setup master device parameters */
> +
> +		/* udmatim Register */
> +		palm_bk3710_base->config.udmatim &= 0xFFF0;
> +		palm_bk3710_base->config.udmatim |= level;
> +		/* udmastb Ultra DMA Access Strobe Width */
> +		palm_bk3710_base->config.udmastb &= 0xFF00;
> +		palm_bk3710_base->config.udmastb |= ide_t0;
> +		/* udmatrp Ultra DMA Ready to Pause Time */
> +		palm_bk3710_base->config.udmatrp &= 0xFF00;
> +		palm_bk3710_base->config.udmatrp |= ide_trp;
> +		/* udmaenv Ultra DMA envelop Time */
> +		palm_bk3710_base->config.udmaenv &= 0xFF00;
> +		palm_bk3710_base->config.udmaenv |= ide_tenv;
> +		/* Enable UDMA for Device 0 */
> +		palm_bk3710_base->config.udmactl |= 1;
> +	} else {
> +		/* setup slave device parameters */
> +
> +		/* udmatim Register */
> +		palm_bk3710_base->config.udmatim &= 0xFF0F;
> +		palm_bk3710_base->config.udmatim |= (level << 4);
> +		/* udmastb Ultra DMA Access Strobe Width */
> +		palm_bk3710_base->config.udmastb &= 0xFF;
> +		palm_bk3710_base->config.udmastb |= (ide_t0 << 8);
> +		/* udmatrp Ultra DMA Ready to Pause Time */
> +		palm_bk3710_base->config.udmatrp &= 0xFF;
> +		palm_bk3710_base->config.udmatrp |= (ide_trp << 8);
> +		/* udmaenv Ultra DMA envelop Time */
> +		palm_bk3710_base->config.udmaenv &= 0xFF;
> +		palm_bk3710_base->config.udmaenv |= (ide_tenv << 8);
> +		/* Enable UDMA for Device 1 */
> +		palm_bk3710_base->config.udmactl |= (1 << 1);
> +	}
> +}
> +
> +static void palm_bk3710_setdmamode(unsigned int dev, unsigned int cycletime,
> +				  unsigned int mode)
> +{
> +	char ide_td, ide_tkw, ide_t0;

- char -> u8
- "ide_" prefix may as well be dropped

> +	if (cycletime < ide_timing[mode].cycle)
> +		cycletime = ide_timing[mode].cycle;
> +
> +	/* DMA Data Setup */
> +	ide_t0 = (cycletime + ide_palm_clk - 1) / ide_palm_clk;
> +	ide_td = (ide_timing[mode].active + ide_palm_clk - 1) / ide_palm_clk;

once ide-timing.h gets converted to ide-timing.c (or the new modes are added)
the above will break, please use ide_timing_find_mode() to access ide_timing[]

[ even better would be to use ide_timing_compute() so non-standard PIO/DMA
  timings specified by some drives are handled correctly ]

> +	ide_tkw = ide_t0 - ide_td - 1;
> +	ide_td -= 1;
> +
> +	if (!dev) {

'!dev' and 'dev' cases may be merged

> +		/* setup master device parameters */
> +		palm_bk3710_base->config.dmastb &= 0xFF00;
> +		palm_bk3710_base->config.dmastb |= ide_td;
> +		palm_bk3710_base->config.dmarcvr &= 0xFF00;
> +		palm_bk3710_base->config.dmarcvr |= ide_tkw;
> +		/* Disable UDMA for Device 0 */
> +		palm_bk3710_base->config.udmactl &= 0xFF02;
> +	} else {
> +		/* setup slave device parameters */
> +		palm_bk3710_base->config.dmastb &= 0xFF;
> +		palm_bk3710_base->config.dmastb |= (ide_td << 8);
> +		palm_bk3710_base->config.dmarcvr &= 0xFF;
> +		palm_bk3710_base->config.dmarcvr |= (ide_tkw << 8);
> +		/* Disable UDMA for Device 1 */
> +		palm_bk3710_base->config.udmactl &= 0xFF01;
> +	}
> +}
> +
> +static void palm_bk3710_setpiomode(ide_drive_t *mate, unsigned int dev,
> +				  unsigned int cycletime, unsigned int mode)
> +{
> +	char ide_t2, ide_t2i, ide_t0;

- char -> u8
- "ide_" prefix may as well be dropped

> +	/* PIO Data Setup */
> +	ide_t0 = (cycletime + ide_palm_clk - 1) / ide_palm_clk;
> +	ide_t2 = (ide_timing[19 - mode].active + ide_palm_clk - 1)

ide_timing_find_mode()

> +			/ ide_palm_clk;
> +
> +	ide_t2i = ide_t0 - ide_t2 - 1;
> +	ide_t2 -= 1;
> +
> +	if (!dev) {

'!dev' and 'dev' cases may be merged

> +		/* setup master device parameters */
> +		palm_bk3710_base->config.datstb &= 0xFF00;
> +		palm_bk3710_base->config.datstb |= ide_t2;
> +		palm_bk3710_base->config.datrcvr &= 0xFF00;
> +		palm_bk3710_base->config.datrcvr |= ide_t2i;
> +	} else {
> +		/* setup slave device parameters */
> +		palm_bk3710_base->config.datstb &= 0xFF;
> +		palm_bk3710_base->config.datstb |= (ide_t2 << 8);
> +		palm_bk3710_base->config.datrcvr &= 0xFF;
> +		palm_bk3710_base->config.datrcvr |= (ide_t2i << 8);
> +	}
> +
> +	if (mate && mate->present) {
> +		u8 mode2 = ide_get_best_pio_mode(mate, 255, 4);
> +
> +		if (mode2 < mode)
> +			mode = mode2;
> +	}
> +
> +	/* TASKFILE Setup */
> +	ide_t0 = (ide_timing[19 - mode].cyc8b + ide_palm_clk - 1)
> +			/ ide_palm_clk;
> +	ide_t2 = (ide_timing[19 - mode].act8b + ide_palm_clk - 1)
> +			/ ide_palm_clk;

ide_timing_find_mode()

> +	ide_t2i = ide_t0 - ide_t2 - 1;
> +	ide_t2 -= 1;
> +
> +	if (!dev) {

'!dev' and 'dev' cases may be merged

> +		/* setup master device parameters */
> +		palm_bk3710_base->config.regstb &= 0xFF00;
> +		palm_bk3710_base->config.regstb |= ide_t2;
> +		palm_bk3710_base->config.regrcvr &= 0xFF00;
> +		palm_bk3710_base->config.regrcvr |= ide_t2i;
> +	} else {
> +		/* setup slave device parameters */
> +		palm_bk3710_base->config.regstb &= 0xFF;
> +		palm_bk3710_base->config.regstb |= (ide_t2 << 8);
> +		palm_bk3710_base->config.regrcvr &= 0xFF;
> +		palm_bk3710_base->config.regrcvr |= (ide_t2i << 8);
> +	}
> +}
> +
> +static void palm_bk3710_set_dma_mode(ide_drive_t *drive, u8 xferspeed)
> +{
> +	int is_slave = drive->dn & 1;
> +
> +	switch (xferspeed) {

it could be simplified to:

	if (xferspeed > XFER_UDMA_0)
		palm_bk3710_setudmamode(...);
	else {
		...
	}

[ upper layers take care of mode filtering ]

> +	case XFER_UDMA_4:
> +	case XFER_UDMA_3:
> +	case XFER_UDMA_2:
> +	case XFER_UDMA_1:
> +	case XFER_UDMA_0:
> +		palm_bk3710_setudmamode(is_slave, xferspeed - XFER_UDMA_0);
> +		break;
> +	case XFER_MW_DMA_2:
> +	case XFER_MW_DMA_1:
> +	case XFER_MW_DMA_0:
> +		{
> +			int nspeed = 10 - xferspeed + XFER_MW_DMA_0;
> +			unsigned ide_cycle = max(ide_timing[nspeed].cycle,
> +					     drive->id->eide_dma_min);

The above line causes following warning (at least when compiling on x86):

drivers/ide/arm/palm_bk3710.c: In function ‘palm_bk3710_set_dma_mode’:
drivers/ide/arm/palm_bk3710.c:274: warning: comparison of distinct pointer types lacks a cast

> +			palm_bk3710_setdmamode(is_slave, ide_cycle, nspeed);
> +		}
> +		break;
> +	}
> +}
> +
> +static void palm_bk3710_set_pio_mode(ide_drive_t *drive, u8 pio)
> +{
> +	unsigned int cycle_time;
> +	int is_slave = drive->dn & 1;
> +	ide_drive_t *mate;
> +
> +	/*
> +	 * Get the best PIO Mode supported by the drive

but it doesn't

[ upper layers take care of it ]

> +	 * Obtain the drive PIO data for tuning the Palm Chip registers
> +	 */
> +	cycle_time = ide_pio_cycle_time(drive, pio);
> +	mate = ide_get_paired_drive(drive);
> +	palm_bk3710_setpiomode(mate, is_slave, cycle_time, pio);
> +}
> +
> +static void palm_bk3710_chipinit(void)

should be __devinit (or __init)

[...]

> +	palm_bk3710_setpiomode(NULL, 0, 0, 0);
> +	palm_bk3710_setpiomode(NULL, 1, 0, 0);

0 is passed as cycle_time so ide_t2i (PIO recovery timing) in
palm_bk3710_setpiomode() becomes negative, this may work well with
a bit of luck but needs fixing

> +}
> +
> +int palm_bk3710_probe(struct platform_device *pdev)

should be static and __devinit (or __init)

[...]

> +	pribase = mem->start + IDE_PALM_ATA_PRI_REG_OFFSET;
> +	for (index = 0; index < IDE_NR_PORTS - 2; index++)
> +		ide_ctlr_info.io_ports[index] = pribase + index;
> +	ide_ctlr_info.io_ports[IDE_CONTROL_OFFSET] = mem->start +
> +			IDE_PALM_ATA_PRI_CTL_OFFSET;
> +	ide_ctlr_info.irq = irq->start;
> +	ide_ctlr_info.chipset = ide_palm3710;
> +	if (ide_register_hw(&ide_ctlr_info, NULL, 0, &palm_bk3710_hwif) < 0) {
> +		printk(KERN_WARNING "Palm Chip BK3710 IDE Register Fail\n");
> +		return -ENODEV;
> +	}
> +
> +	palm_bk3710_hwif->set_pio_mode = &palm_bk3710_set_pio_mode;
> +	palm_bk3710_hwif->set_dma_mode = &palm_bk3710_set_dma_mode;
> +
> +	palm_bk3710_hwif->ultra_mask = 0x1f;	/* Ultra DMA Mode 4 Max
> +						(input clk 99MHz) */
> +	palm_bk3710_hwif->mwdma_mask = 0x7;
> +	palm_bk3710_hwif->drives[0].autotune = 1;
> +	palm_bk3710_hwif->drives[1].autotune = 1;
> +
> +	if (!request_region(mem->start, 8, palm_bk3710_hwif->name)) {
> +		printk(KERN_ERR "Error, ports in use.\n");
> +		return -EBUSY;
> +	}

resource allocation should be done _before_ ide_register_hw() call

[ while at it please use the driver name instead of palm_bk3710_hwif->name ]

> +	palm_bk3710_hwif->dmatable_cpu = dma_alloc_coherent(
> +				NULL,
> +				PRD_ENTRIES * PRD_BYTES,
> +				&palm_bk3710_hwif->dmatable_dma,
> +				GFP_ATOMIC);
> +
> +	if (!palm_bk3710_hwif->dmatable_cpu) {
> +		printk(KERN_ERR "Error, unable to allocate DMA table.\n");
> +		return -ENOMEM;

no need to fail the driver completely - we may still operate in PIO mode just
fine given that ->{ultra,mwdma}_mask are cleared and we return 0 here

> +int palm_bk3710_init(void)

static + __devinit/__init

[...]

Please recast and resubmit.

Thanks,
Bart

Re: [PATCH] Palmchip BK3710 IDE driver

[Bartlomiej Zolnierkiewicz]

On Friday 18 January 2008, Sergei Shtylyov wrote:
> Alan Cox wrote:
> 
> >>    It won't -- we can *not* call ide_setup_dma() which fills them out as this 
> >>is not a PCI chip.
> 
> > Gak.
> 
>     Or maybe we still can -- with hwif->mmio set?
>     AFAIR (from the 2006 discussion) Bart said it's *only* for PCI devices...

Well, since BLK_DEV_PALMCHIP_BK3710 selects BLK_DEV_IDEDMA_PCI it should be
possible to call ide_setup_dma() in this driver...

Bart

[PATCH] Palmchip BK3710 IDE driver

[Anton Salnikov]

This is Palmchip BK3710 IDE controller support for kernel version 2.6.24-rc8.
The IDE controller logic supports PIO, multiword DMA and ultra-DMA modes.
Supports interface to compact Flash (CF) configured in True-IDE mode.

Use ide_setup_dma() since BLK_DEV_PALMCHIP_BK3710 selects BLK_DEV_IDEDMA_PCI
So I deleted exports from ide-dma.c

Signed-off-by: Anton Salnikov <asalnikov@ru.mvista.com>
---

 drivers/ide/Kconfig           |    8 
 drivers/ide/arm/Makefile      |    1 
 drivers/ide/arm/palm_bk3710.c |  434 ++++++++++++++++++++++++++++++++++++++++++
 drivers/ide/ide-proc.c        |    1 
 include/linux/ide.h           |    2 
 5 files changed, 445 insertions(+), 1 deletion(-)

Index: 2.6.24-rc8.ide/drivers/ide/Kconfig
===================================================================
--- 2.6.24-rc8.ide.orig/drivers/ide/Kconfig
+++ 2.6.24-rc8.ide/drivers/ide/Kconfig
@@ -1008,6 +1008,14 @@ config BLK_DEV_Q40IDE
 	  normally be on; disable it only if you are running a custom hard
 	  drive subsystem through an expansion card.
 
+config BLK_DEV_PALMCHIP_BK3710
+	bool "Palmchip bk3710 IDE controller support"
+	depends on ARCH_DAVINCI
+	select BLK_DEV_IDEDMA_PCI
+	help
+	  Say Y here if you want to support the onchip IDE controller on the
+	  TI DaVinci SoC
+
 config BLK_DEV_MPC8xx_IDE
 	bool "MPC8xx IDE support"
 	depends on 8xx && (LWMON || IVMS8 || IVML24 || TQM8xxL) && IDE=y && 
BLK_DEV_IDE=y && !PPC_MERGE
Index: 2.6.24-rc8.ide/drivers/ide/arm/Makefile
===================================================================
--- 2.6.24-rc8.ide.orig/drivers/ide/arm/Makefile
+++ 2.6.24-rc8.ide/drivers/ide/arm/Makefile
@@ -2,5 +2,6 @@
 obj-$(CONFIG_BLK_DEV_IDE_ICSIDE)	+= icside.o
 obj-$(CONFIG_BLK_DEV_IDE_RAPIDE)	+= rapide.o
 obj-$(CONFIG_BLK_DEV_IDE_BAST)		+= bast-ide.o
+obj-$(CONFIG_BLK_DEV_PALMCHIP_BK3710)	+= palm_bk3710.o
 
 EXTRA_CFLAGS	:= -Idrivers/ide
Index: 2.6.24-rc8.ide/drivers/ide/arm/palm_bk3710.c
===================================================================
--- /dev/null
+++ 2.6.24-rc8.ide/drivers/ide/arm/palm_bk3710.c
@@ -0,0 +1,434 @@
+/*
+ * Palmchip bk3710 IDE controller
+ *
+ * Copyright (C) 2006 Texas Instruments.
+ * Copyright (C) 2007 MontaVista Software, Inc., <source@mvista.com>
+ *
+ * ----------------------------------------------------------------------------
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ * ----------------------------------------------------------------------------
+ *
+ */
+
+#include <linux/types.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/ioport.h>
+#include <linux/hdreg.h>
+#include <linux/ide.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/clk.h>
+#include <linux/platform_device.h>
+
+/* Offset of the primary interface registers */
+#define IDE_PALM_ATA_PRI_REG_OFFSET 0x1F0
+
+/* Primary Control Offset */
+#define IDE_PALM_ATA_PRI_CTL_OFFSET 0x3F6
+
+/*
+ * PalmChip 3710 IDE Controller UDMA timing structure Definition
+ */
+struct palm_bk3710_udmatiming {
+	unsigned int rptime;	/* Ready to pause time  */
+	unsigned int cycletime;	/* Cycle Time           */
+};
+
+#define BK3710_BMICP		0x00
+#define BK3710_BMISP		0x02
+#define BK3710_BMIDTP		0x04
+#define BK3710_BMICS		0x08
+#define BK3710_BMISS		0x0A
+#define BK3710_BMIDTS		0x0C
+#define BK3710_IDETIMP		0x40
+#define BK3710_IDETIMS		0x42
+#define BK3710_SIDETIM		0x44
+#define BK3710_SLEWCTL		0x45
+#define BK3710_IDESTATUS	0x47
+#define BK3710_UDMACTL		0x48
+#define BK3710_UDMATIM		0x4A
+#define BK3710_MISCCTL		0x50
+#define BK3710_REGSTB		0x54
+#define BK3710_REGRCVR		0x58
+#define BK3710_DATSTB		0x5C
+#define BK3710_DATRCVR		0x60
+#define BK3710_DMASTB		0x64
+#define BK3710_DMARCVR		0x68
+#define BK3710_UDMASTB		0x6C
+#define BK3710_UDMATRP		0x70
+#define BK3710_UDMAENV		0x74
+#define BK3710_IORDYTMP		0x78
+#define BK3710_IORDYTMS		0x7C
+
+#include "../ide-timing.h"
+
+static long ide_palm_clk;
+static u32 base;
+
+static const struct palm_bk3710_udmatiming palm_bk3710_udmatimings[6] = {
+	{160, 240},		/* UDMA Mode 0 */
+	{125, 160},		/* UDMA Mode 1 */
+	{100, 120},		/* UDMA Mode 2 */
+	{100, 90},		/* UDMA Mode 3 */
+	{85,  60},		/* UDMA Mode 4 */
+};
+
+static struct clk *ideclkp;
+
+static inline u32 ioread(u32 reg)
+{
+	return ioread32(base + reg);
+}
+
+static inline void iowrite(u32 val, u32 reg)
+{
+	iowrite32(val, base + reg);
+}
+
+static void palm_bk3710_setudmamode(unsigned int dev, unsigned int mode)
+{
+	u8 tenv, trp, t0;
+	u32 val;
+
+	/* DMA Data Setup */
+	t0 = (palm_bk3710_udmatimings[mode].cycletime + ide_palm_clk - 1)
+			/ ide_palm_clk - 1;
+	tenv = (20 + ide_palm_clk - 1) / ide_palm_clk - 1;
+	trp = (palm_bk3710_udmatimings[mode].rptime + ide_palm_clk - 1)
+			/ ide_palm_clk - 1;
+
+	/* udmatim Register */
+	val = ioread(BK3710_UDMATIM) & ((!dev) ? 0xFFF0 : 0xFF0F);
+	iowrite(val, BK3710_UDMATIM);
+	val = ioread(BK3710_UDMATIM) | (mode << ((!dev) ? 0 : 4));
+	iowrite(val, BK3710_UDMATIM);
+
+	/* udmastb Ultra DMA Access Strobe Width */
+	val = ioread(BK3710_UDMASTB) & (0xFF << ((!dev) << 4));
+	iowrite(val, BK3710_UDMASTB);
+	val = ioread(BK3710_UDMASTB) | (t0 << (dev << 4));
+	iowrite(val, BK3710_UDMASTB);
+
+	/* udmatrp Ultra DMA Ready to Pause Time */
+	val = ioread(BK3710_UDMATRP) & (0xFF << ((!dev) << 4));
+	iowrite(val, BK3710_UDMATRP);
+	val = ioread(BK3710_UDMATRP) | (trp << (dev << 4));
+	iowrite(val, BK3710_UDMATRP);
+
+	/* udmaenv Ultra DMA envelop Time */
+	val = ioread(BK3710_UDMAENV) & (0xFF << ((!dev) << 4));
+	iowrite(val, BK3710_UDMAENV);
+	val = ioread(BK3710_UDMAENV) | (tenv << (dev << 4));
+	iowrite(val, BK3710_UDMAENV);
+
+	/* Enable UDMA for Device */
+	val = ioread(BK3710_UDMACTL) | (1 << dev);
+	iowrite(val, BK3710_UDMACTL);
+}
+
+static void palm_bk3710_setdmamode(unsigned int dev, unsigned int cycletime,
+				  unsigned int mode)
+{
+	u8 td, tkw, t0;
+	u32 val;
+
+	if (cycletime < ide_timing[mode].cycle)
+		cycletime = ide_timing[mode].cycle;
+
+	/* DMA Data Setup */
+	t0 = (cycletime + ide_palm_clk - 1) / ide_palm_clk;
+	td = (ide_timing_find_mode(mode)->active + ide_palm_clk - 1)
+			/ ide_palm_clk;
+	tkw = t0 - td - 1;
+	td -= 1;
+
+	val = ioread(BK3710_DMASTB) & (0xFF << ((!dev) << 4));
+	iowrite(val, BK3710_DMASTB);
+	val = ioread(BK3710_DMASTB) | (td << (dev << 4));
+	iowrite(val, BK3710_DMASTB);
+
+	val = ioread(BK3710_DMARCVR) & (0xFF << ((!dev) << 4));
+	iowrite(val, BK3710_DMARCVR);
+	val = ioread(BK3710_DMARCVR) | (tkw << (dev << 4));
+	iowrite(val, BK3710_DMARCVR);
+
+	/* Disable UDMA for Device */
+	val = ioread(BK3710_UDMACTL) & (0xFF00 | (1 << (!dev)));
+	iowrite(val, BK3710_UDMACTL);
+}
+
+static void palm_bk3710_setpiomode(ide_drive_t *mate, unsigned int dev,
+				  unsigned int cycletime, unsigned int mode)
+{
+	u8 t2, t2i, t0;
+	u32 val;
+	struct ide_timing *t;
+
+	/* PIO Data Setup */
+	t0 = (cycletime + ide_palm_clk - 1) / ide_palm_clk;
+	t2 = (ide_timing_find_mode(XFER_PIO_0 + mode)->active + ide_palm_clk - 1)
+			/ ide_palm_clk;
+
+	t2i = t0 - t2 - 1;
+	t2 -= 1;
+
+	val = ioread(BK3710_DATSTB) & (0xFF << ((!dev) << 4));
+	iowrite(val, BK3710_DATSTB);
+	val = ioread(BK3710_DATSTB) | (t2 << (dev << 4));
+	iowrite(val, BK3710_DATSTB);
+
+	val = ioread(BK3710_DATRCVR) & (0xFF << ((!dev) << 4));
+	iowrite(val, BK3710_DATRCVR);
+	val = ioread(BK3710_DATRCVR) | (t2i << (dev << 4));
+	iowrite(val, BK3710_DATRCVR);
+
+	if (mate && mate->present) {
+		u8 mode2 = ide_get_best_pio_mode(mate, 255, 4);
+
+		if (mode2 < mode)
+			mode = mode2;
+	}
+
+	/* TASKFILE Setup */
+	t = ide_timing_find_mode(XFER_PIO_0 + mode);
+	t0 = (t->cyc8b + ide_palm_clk - 1)
+			/ ide_palm_clk;
+	t2 = (t->act8b + ide_palm_clk - 1)
+			/ ide_palm_clk;
+
+	t2i = t0 - t2 - 1;
+	t2 -= 1;
+
+	val = ioread(BK3710_REGSTB) & (0xFF << ((!dev) << 4));
+	iowrite(val, BK3710_REGSTB);
+	val = ioread(BK3710_REGSTB) | (t2 << (dev << 4));
+	iowrite(val, BK3710_REGSTB);
+
+	val = ioread(BK3710_REGRCVR) & (0xFF << ((!dev) << 4));
+	iowrite(val, BK3710_REGRCVR);
+	val = ioread(BK3710_REGRCVR) | (t2i << (dev << 4));
+	iowrite(val, BK3710_REGRCVR);
+}
+
+static void palm_bk3710_set_dma_mode(ide_drive_t *drive, u8 xferspeed)
+{
+	int is_slave = drive->dn & 1;
+
+	if (xferspeed >= XFER_UDMA_0) {
+		palm_bk3710_setudmamode(is_slave, xferspeed - XFER_UDMA_0);
+	} else {
+		int nspeed = 10 - xferspeed + XFER_MW_DMA_0;
+		unsigned ide_cycle = max_t(int, ide_timing[nspeed].cycle,
+					   drive->id->eide_dma_min);
+
+		palm_bk3710_setdmamode(is_slave, ide_cycle, nspeed);
+	}
+}
+
+static void palm_bk3710_set_pio_mode(ide_drive_t *drive, u8 pio)
+{
+	unsigned int cycle_time;
+	int is_slave = drive->dn & 1;
+	ide_drive_t *mate;
+
+	/*
+	 * Obtain the drive PIO data for tuning the Palm Chip registers
+	 */
+	cycle_time = ide_pio_cycle_time(drive, pio);
+	mate = ide_get_paired_drive(drive);
+	palm_bk3710_setpiomode(mate, is_slave, cycle_time, pio);
+}
+
+static void __devinit palm_bk3710_chipinit(void)
+{
+	/*
+	 * enable the reset_en of ATA controller so that when ata signals
+	 * are brought out, by writing into device config. at that
+	 * time por_n signal should not be 'Z' and have a stable value.
+	 */
+	iowrite(0x0300, BK3710_MISCCTL);
+
+	/* wait for some time and deassert the reset of ATA Device. */
+	mdelay(100);
+
+	/* Deassert the Reset */
+	iowrite(0x0200, BK3710_MISCCTL);
+
+	/*
+	 * Program the IDETIMP Register Value based on the following assumptions
+	 *
+	 * (ATA_IDETIMP_IDEEN		, ENABLE ) |
+	 * (ATA_IDETIMP_SLVTIMEN	, DISABLE) |
+	 * (ATA_IDETIMP_RDYSMPL		, 70NS)    |
+	 * (ATA_IDETIMP_RDYRCVRY	, 50NS)    |
+	 * (ATA_IDETIMP_DMAFTIM1	, PIOCOMP) |
+	 * (ATA_IDETIMP_PREPOST1	, DISABLE) |
+	 * (ATA_IDETIMP_RDYSEN1		, DISABLE) |
+	 * (ATA_IDETIMP_PIOFTIM1	, DISABLE) |
+	 * (ATA_IDETIMP_DMAFTIM0	, PIOCOMP) |
+	 * (ATA_IDETIMP_PREPOST0	, DISABLE) |
+	 * (ATA_IDETIMP_RDYSEN0		, DISABLE) |
+	 * (ATA_IDETIMP_PIOFTIM0	, DISABLE)
+	 */
+	iowrite(0xB388, BK3710_IDETIMP);
+
+	/*
+	 * Configure  SIDETIM  Register
+	 * (ATA_SIDETIM_RDYSMPS1	,120NS ) |
+	 * (ATA_SIDETIM_RDYRCYS1	,120NS )
+	 */
+	iowrite(0, BK3710_SIDETIM);
+
+	/*
+	 * UDMACTL Ultra-ATA DMA Control
+	 * (ATA_UDMACTL_UDMAP1	, 0 ) |
+	 * (ATA_UDMACTL_UDMAP0	, 0 )
+	 *
+	 */
+	iowrite(0, BK3710_UDMACTL);
+
+	/*
+	 * MISCCTL Miscellaneous Conrol Register
+	 * (ATA_MISCCTL_RSTMODEP	, 1) |
+	 * (ATA_MISCCTL_RESETP		, 0) |
+	 * (ATA_MISCCTL_TIMORIDE	, 1)
+	 */
+	iowrite(0x201, BK3710_MISCCTL);
+
+	/*
+	 * IORDYTMP IORDY Timer for Primary Register
+	 * (ATA_IORDYTMP_IORDYTMP     , 0xffff  )
+	 */
+	iowrite(0xFFFF, BK3710_IORDYTMP);
+
+	/*
+	 * Configure BMISP Register
+	 * (ATA_BMISP_DMAEN1	, DISABLE )	|
+	 * (ATA_BMISP_DMAEN0	, DISABLE )	|
+	 * (ATA_BMISP_IORDYINT	, CLEAR)	|
+	 * (ATA_BMISP_INTRSTAT	, CLEAR)	|
+	 * (ATA_BMISP_DMAERROR	, CLEAR)
+	 */
+	iowrite(0, BK3710_BMISP);
+
+	palm_bk3710_setpiomode(NULL, 0, 600, 0);
+	palm_bk3710_setpiomode(NULL, 1, 600, 0);
+}
+
+static int __devinit palm_bk3710_probe(struct platform_device *pdev)
+{
+	hw_regs_t ide_ctlr_info;
+	int index = 0;
+	int pribase;
+	struct clk *clkp;
+	struct resource *mem, *irq;
+	ide_hwif_t *hwif;
+
+	clkp = clk_get(NULL, "IDECLK");
+	if (IS_ERR(clkp))
+		return -ENODEV;
+
+	ideclkp = clkp;
+	clk_enable(ideclkp);
+	ide_palm_clk = clk_get_rate(ideclkp)/100000;
+	ide_palm_clk = (10000/ide_palm_clk) + 1;
+	/* Register the IDE interface with Linux ATA Interface */
+	memset(&ide_ctlr_info, 0, sizeof(ide_ctlr_info));
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (mem == NULL) {
+		printk(KERN_INFO "failed to get memory region resource\n");
+		return -ENODEV;
+	}
+	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (irq == NULL) {
+		printk(KERN_INFO "failed to get IRQ resource\n");
+		return -ENODEV;
+	}
+
+	base = mem->start;
+	/* Configure the Palm Chip controller */
+	palm_bk3710_chipinit();
+
+	pribase = mem->start + IDE_PALM_ATA_PRI_REG_OFFSET;
+	for (index = 0; index < IDE_NR_PORTS - 2; index++)
+		ide_ctlr_info.io_ports[index] = pribase + index;
+	ide_ctlr_info.io_ports[IDE_CONTROL_OFFSET] = mem->start +
+			IDE_PALM_ATA_PRI_CTL_OFFSET;
+	ide_ctlr_info.irq = irq->start;
+	ide_ctlr_info.chipset = ide_palm3710;
+
+	if (!request_region(mem->start, 8, "palm_bk3710")) {
+		printk(KERN_ERR "Error, ports in use.\n");
+		return -EBUSY;
+	}
+
+	if (ide_register_hw(&ide_ctlr_info, NULL, 0, &hwif) < 0) {
+		printk(KERN_WARNING "Palm Chip BK3710 IDE Register Fail\n");
+		return -ENODEV;
+	}
+
+	hwif->set_pio_mode = &palm_bk3710_set_pio_mode;
+	hwif->set_dma_mode = &palm_bk3710_set_dma_mode;
+	hwif->mmio = 1;
+	hwif->ultra_mask = 0x1f;	/* Ultra DMA Mode 4 Max
+						(input clk 99MHz) */
+	hwif->mwdma_mask = 0x7;
+	hwif->drives[0].autotune = 1;
+	hwif->drives[1].autotune = 1;
+
+	hwif->dmatable_cpu = dma_alloc_coherent(
+				NULL,
+				PRD_ENTRIES * PRD_BYTES,
+				&hwif->dmatable_dma,
+				GFP_ATOMIC);
+
+	if (!hwif->dmatable_cpu) {
+		printk(KERN_ERR "Error, unable to allocate DMA table.\n");
+		hwif->ultra_mask = 0;
+		hwif->mwdma_mask = 0;
+		return 0;
+	}
+
+	hwif->dma_base = mem->start;
+
+	hwif->dma_master = mem->start;
+
+	hwif->dma_command = mem->start;
+	hwif->dma_status = mem->start + 2;
+	hwif->dma_prdtable = mem->start + 4;
+	ide_setup_dma(hwif, mem->start, 8);
+
+	return 0;
+}
+
+static struct platform_driver platform_bk_driver = {
+	.driver = {
+		.name = "palm_bk3710",
+	},
+	.probe = palm_bk3710_probe,
+	.remove = NULL,
+};
+
+static int __init palm_bk3710_init(void)
+{
+	return platform_driver_register(&platform_bk_driver);
+}
+
+module_init(palm_bk3710_init);
+MODULE_LICENSE("GPL");
+
Index: 2.6.24-rc8.ide/drivers/ide/ide-proc.c
===================================================================
--- 2.6.24-rc8.ide.orig/drivers/ide/ide-proc.c
+++ 2.6.24-rc8.ide/drivers/ide/ide-proc.c
@@ -67,6 +67,7 @@ static int proc_ide_read_imodel
 		case ide_4drives:	name = "4drives";	break;
 		case ide_pmac:		name = "mac-io";	break;
 		case ide_au1xxx:	name = "au1xxx";	break;
+		case ide_palm3710:      name = "palm3710";      break;
 		case ide_etrax100:	name = "etrax100";	break;
 		case ide_acorn:		name = "acorn";		break;
 		default:		name = "(unknown)";	break;
Index: 2.6.24-rc8.ide/include/linux/ide.h
===================================================================
--- 2.6.24-rc8.ide.orig/include/linux/ide.h
+++ 2.6.24-rc8.ide/include/linux/ide.h
@@ -202,7 +202,7 @@ enum {		ide_unknown,	ide_generic,	ide_pc
 		ide_rz1000,	ide_trm290,
 		ide_cmd646,	ide_cy82c693,	ide_4drives,
 		ide_pmac,	ide_etrax100,	ide_acorn,
-		ide_au1xxx, ide_forced
+		ide_au1xxx,	ide_palm3710,	ide_forced
 };
 
 typedef u8 hwif_chipset_t;

Re: [PATCH] Palmchip BK3710 IDE driver

[Alan Cox]

> +static inline u32 ioread(u32 reg)
> +{
> +	return ioread32(base + reg);
> +}
> +
> +static inline void iowrite(u32 val, u32 reg)
> +{
> +	iowrite32(val, base + reg);
> +}

Why not just use ioread32/iowrite32 directly ?


Otherwise this looks way way better.

Alan

Re: [PATCH] Palmchip BK3710 IDE driver

[Anton Salnikov]

> > +static inline u32 ioread(u32 reg)
> > +{
> > +	return ioread32(base + reg);
> > +}
> > +
> > +static inline void iowrite(u32 val, u32 reg)
> > +{
> > +	iowrite32(val, base + reg);
> > +}
> 
> Why not just use ioread32/iowrite32 directly ?

Because this increases readability and does not affect functionality at all

> Otherwise this looks way way better.
> 
> Alan
> 

Anton

Re: [PATCH] Palmchip BK3710 IDE driver

[Alan Cox]

On Tue, 22 Jan 2008 15:11:37 +0300
Anton Salnikov <asalnikov@ru.mvista.com> wrote:

> > > +static inline u32 ioread(u32 reg)
> > > +{
> > > +	return ioread32(base + reg);
> > > +}
> > > +
> > > +static inline void iowrite(u32 val, u32 reg)
> > > +{
> > > +	iowrite32(val, base + reg);
> > > +}
> > 
> > Why not just use ioread32/iowrite32 directly ?
> 
> Because this increases readability and does not affect functionality at all

There I would disagree. It means that if you ever have a system with two
of these controllers in future it will break as base is a static for the
entire driver.

Easy enough to fix with a simple
	
	static inline u32 hwif_ioread32(hwif, u32 reg) { ...
hwif->dma_base ...}

version ?

Alan

Re: [PATCH] Palmchip BK3710 IDE driver

[Jeff Garzik]

Anton Salnikov wrote:
>>> +static inline u32 ioread(u32 reg)
>>> +{
>>> +	return ioread32(base + reg);
>>> +}
>>> +
>>> +static inline void iowrite(u32 val, u32 reg)
>>> +{
>>> +	iowrite32(val, base + reg);
>>> +}
>> Why not just use ioread32/iowrite32 directly ?
> 
> Because this increases readability and does not affect functionality at all

Disagree.

It confuses the namespace, /decreasing/ readability.  Anyone _but_ you 
reading this code will be confused by a non-standard API named similarly 
to an existing kernel API.

	Jeff

Re: [PATCH] Palmchip BK3710 IDE driver

[Sergei Shtylyov]

Hello.

Anton Salnikov wrote:

> This is Palmchip BK3710 IDE controller support for kernel version 2.6.24-rc8.
> The IDE controller logic supports PIO, multiword DMA and ultra-DMA modes.
> Supports interface to compact Flash (CF) configured in True-IDE mode.

> Use ide_setup_dma() since BLK_DEV_PALMCHIP_BK3710 selects BLK_DEV_IDEDMA_PCI
> So I deleted exports from ide-dma.c

> Signed-off-by: Anton Salnikov <asalnikov@ru.mvista.com>

> Index: 2.6.24-rc8.ide/drivers/ide/arm/palm_bk3710.c
> ===================================================================
> --- /dev/null
> +++ 2.6.24-rc8.ide/drivers/ide/arm/palm_bk3710.c
> @@ -0,0 +1,434 @@

> +static long ide_palm_clk;
> +static u32 base;

    This base will be in hwif->dma_master, so you can aslo use it this way if 
you make your 'hwif' pointer static instead. Although as this

> +
> +static const struct palm_bk3710_udmatiming palm_bk3710_udmatimings[6] = {
> +	{160, 240},		/* UDMA Mode 0 */
> +	{125, 160},		/* UDMA Mode 1 */
> +	{100, 120},		/* UDMA Mode 2 */
> +	{100, 90},		/* UDMA Mode 3 */
> +	{85,  60},		/* UDMA Mode 4 */
> +};
> +
> +static struct clk *ideclkp;
> +
> +static inline u32 ioread(u32 reg)
> +{
> +	return ioread32(base + reg);
> +}
> +
> +static inline void iowrite(u32 val, u32 reg)
> +{
> +	iowrite32(val, base + reg);
> +}

    Why you chose to use ioread32() and iowrite32() if your device is strictly 
memory mapped? Those functions add some overhead, and boil down to readl() and 
writel() anyway.  IIRC, they should only be used if you have a hardware 
registers accessible from both I/O and memory space and driver supports both 
types of mapping (e.g., I/O mapped for the older hardware, memory mapped for 
the newer one)...

> +static void palm_bk3710_setudmamode(unsigned int dev, unsigned int mode)
> +{
> +	u8 tenv, trp, t0;
> +	u32 val;
> +
> +	/* DMA Data Setup */
> +	t0 = (palm_bk3710_udmatimings[mode].cycletime + ide_palm_clk - 1)
> +			/ ide_palm_clk - 1;
> +	tenv = (20 + ide_palm_clk - 1) / ide_palm_clk - 1;
> +	trp = (palm_bk3710_udmatimings[mode].rptime + ide_palm_clk - 1)
> +			/ ide_palm_clk - 1;
> +
> +	/* udmatim Register */
> +	val = ioread(BK3710_UDMATIM) & ((!dev) ? 0xFFF0 : 0xFF0F);

    Hm, isn't it shorter:

	val = ioread(BK3710_UDMATIM) & (dev ? 0xFF0F : 0xFFF0);

> +	iowrite(val, BK3710_UDMATIM);

    Why not skip intermediate write and re-read?  Does hardware require 
clearing the fields before setting new values by some weird chance?

> +	val = ioread(BK3710_UDMATIM) | (mode << ((!dev) ? 0 : 4));

    You actually need to shift left by 0 or 8, so shift count should be 3, not 
4 since 1 << 4 == 16!  I'm suggesting:

	val = ioread(BK3710_UDMATIM) | (mode << (dev ? 3 : 0));

> +	iowrite(val, BK3710_UDMATIM);
> +
> +	/* udmastb Ultra DMA Access Strobe Width */
> +	val = ioread(BK3710_UDMASTB) & (0xFF << ((!dev) << 4));

    I'm suggesting:

	val = ioread(BK3710_UDMASTB) & (0xFF00 >> (dev << 3));

> +	iowrite(val, BK3710_UDMASTB);
> +	val = ioread(BK3710_UDMASTB) | (t0 << (dev << 4));
> +	iowrite(val, BK3710_UDMASTB);
> +
> +	/* udmatrp Ultra DMA Ready to Pause Time */
> +	val = ioread(BK3710_UDMATRP) & (0xFF << ((!dev) << 4));
> +	iowrite(val, BK3710_UDMATRP);
> +	val = ioread(BK3710_UDMATRP) | (trp << (dev << 4));
> +	iowrite(val, BK3710_UDMATRP);
> +
> +	/* udmaenv Ultra DMA envelop Time */
> +	val = ioread(BK3710_UDMAENV) & (0xFF << ((!dev) << 4));
> +	iowrite(val, BK3710_UDMAENV);
> +	val = ioread(BK3710_UDMAENV) | (tenv << (dev << 4));
> +	iowrite(val, BK3710_UDMAENV);

    << 3 ISO << 4 everywhere...

> +
> +	/* Enable UDMA for Device */
> +	val = ioread(BK3710_UDMACTL) | (1 << dev);
> +	iowrite(val, BK3710_UDMACTL);
> +}
> +
> +static void palm_bk3710_setdmamode(unsigned int dev, unsigned int cycletime,
> +				  unsigned int mode)
> +{
> +	u8 td, tkw, t0;
> +	u32 val;
> +
> +	if (cycletime < ide_timing[mode].cycle)
> +		cycletime = ide_timing[mode].cycle;

    I don't think this check is necessary as you're passing either 
ide_timing[mode].cycle or drive->id->eide_dma_min -- whichever is greater...

> +
> +	/* DMA Data Setup */
> +	t0 = (cycletime + ide_palm_clk - 1) / ide_palm_clk;
> +	td = (ide_timing_find_mode(mode)->active + ide_palm_clk - 1)

    You're not passing a mode suitable to ide_timing_find_mode() here, but 
index into ide_timing[] table -- almost the same thing that this call is 
supposed to yield... :-/

> +			/ ide_palm_clk;
> +	tkw = t0 - td - 1;
> +	td -= 1;
> +
> +	val = ioread(BK3710_DMASTB) & (0xFF << ((!dev) << 4));
> +	iowrite(val, BK3710_DMASTB);
> +	val = ioread(BK3710_DMASTB) | (td << (dev << 4));
> +	iowrite(val, BK3710_DMASTB);
> +
> +	val = ioread(BK3710_DMARCVR) & (0xFF << ((!dev) << 4));
> +	iowrite(val, BK3710_DMARCVR);
> +	val = ioread(BK3710_DMARCVR) | (tkw << (dev << 4));
> +	iowrite(val, BK3710_DMARCVR);
> +
> +	/* Disable UDMA for Device */
> +	val = ioread(BK3710_UDMACTL) & (0xFF00 | (1 << (!dev)));

    I think it should be just like above:

	val = ioread(BK3710_UDMACTL) & ~(1 << dev);

> +	iowrite(val, BK3710_UDMACTL);
> +}

    Same comments as above...

> +static void palm_bk3710_setpiomode(ide_drive_t *mate, unsigned int dev,
> +				  unsigned int cycletime, unsigned int mode)
> +{
> +	u8 t2, t2i, t0;
> +	u32 val;
> +	struct ide_timing *t;
> +
> +	/* PIO Data Setup */
> +	t0 = (cycletime + ide_palm_clk - 1) / ide_palm_clk;
> +	t2 = (ide_timing_find_mode(XFER_PIO_0 + mode)->active + ide_palm_clk - 1)
> +			/ ide_palm_clk;
> +
> +	t2i = t0 - t2 - 1;
> +	t2 -= 1;
> +
> +	val = ioread(BK3710_DATSTB) & (0xFF << ((!dev) << 4));
> +	iowrite(val, BK3710_DATSTB);
> +	val = ioread(BK3710_DATSTB) | (t2 << (dev << 4));
> +	iowrite(val, BK3710_DATSTB);
> +
> +	val = ioread(BK3710_DATRCVR) & (0xFF << ((!dev) << 4));
> +	iowrite(val, BK3710_DATRCVR);
> +	val = ioread(BK3710_DATRCVR) | (t2i << (dev << 4));
> +	iowrite(val, BK3710_DATRCVR);
> +
> +	if (mate && mate->present) {
> +		u8 mode2 = ide_get_best_pio_mode(mate, 255, 4);
> +
> +		if (mode2 < mode)
> +			mode = mode2;
> +	}
> +
> +	/* TASKFILE Setup */
> +	t = ide_timing_find_mode(XFER_PIO_0 + mode);
> +	t0 = (t->cyc8b + ide_palm_clk - 1)
> +			/ ide_palm_clk;
> +	t2 = (t->act8b + ide_palm_clk - 1)
> +			/ ide_palm_clk;

    Please make the two above assignments take 2 lines ISO 4...

> +
> +	t2i = t0 - t2 - 1;
> +	t2 -= 1;
> +
> +	val = ioread(BK3710_REGSTB) & (0xFF << ((!dev) << 4));
> +	iowrite(val, BK3710_REGSTB);
> +	val = ioread(BK3710_REGSTB) | (t2 << (dev << 4));
> +	iowrite(val, BK3710_REGSTB);
> +
> +	val = ioread(BK3710_REGRCVR) & (0xFF << ((!dev) << 4));
> +	iowrite(val, BK3710_REGRCVR);
> +	val = ioread(BK3710_REGRCVR) | (t2i << (dev << 4));
> +	iowrite(val, BK3710_REGRCVR);

    Same mistakes as the above functions...

> +}
> +
> +static void palm_bk3710_set_dma_mode(ide_drive_t *drive, u8 xferspeed)
> +{
> +	int is_slave = drive->dn & 1;
> +
> +	if (xferspeed >= XFER_UDMA_0) {
> +		palm_bk3710_setudmamode(is_slave, xferspeed - XFER_UDMA_0);
> +	} else {
> +		int nspeed = 10 - xferspeed + XFER_MW_DMA_0;
> +		unsigned ide_cycle = max_t(int, ide_timing[nspeed].cycle,
> +					   drive->id->eide_dma_min);

    It's probably better to move this part into palm_bk3710_setdmamode()...

> +
> +		palm_bk3710_setdmamode(is_slave, ide_cycle, nspeed);

    You should pass xferspeed here, not nspeed...

> +static int __devinit palm_bk3710_probe(struct platform_device *pdev)
> +{
> +	hw_regs_t ide_ctlr_info;
> +	int index = 0;
> +	int pribase;
> +	struct clk *clkp;
> +	struct resource *mem, *irq;
> +	ide_hwif_t *hwif;
> +
> +	clkp = clk_get(NULL, "IDECLK");
> +	if (IS_ERR(clkp))
> +		return -ENODEV;
> +
> +	ideclkp = clkp;
> +	clk_enable(ideclkp);
> +	ide_palm_clk = clk_get_rate(ideclkp)/100000;
> +	ide_palm_clk = (10000/ide_palm_clk) + 1;
> +	/* Register the IDE interface with Linux ATA Interface */
> +	memset(&ide_ctlr_info, 0, sizeof(ide_ctlr_info));
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (mem == NULL) {
> +		printk(KERN_INFO "failed to get memory region resource\n");
> +		return -ENODEV;
> +	}
> +	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +	if (irq == NULL) {
> +		printk(KERN_INFO "failed to get IRQ resource\n");
> +		return -ENODEV;
> +	}
> +
> +	base = mem->start;
> +	/* Configure the Palm Chip controller */
> +	palm_bk3710_chipinit();
> +
> +	pribase = mem->start + IDE_PALM_ATA_PRI_REG_OFFSET;
> +	for (index = 0; index < IDE_NR_PORTS - 2; index++)
> +		ide_ctlr_info.io_ports[index] = pribase + index;
> +	ide_ctlr_info.io_ports[IDE_CONTROL_OFFSET] = mem->start +
> +			IDE_PALM_ATA_PRI_CTL_OFFSET;
> +	ide_ctlr_info.irq = irq->start;
> +	ide_ctlr_info.chipset = ide_palm3710;
> +
> +	if (!request_region(mem->start, 8, "palm_bk3710")) {

    It should be request_mem_region() since the chip is memory mapped. I've 
managed to overlook this so far. :-<
    BTW, shouldn't you call request_mem_region() for the entire controller 
register region (0x400 in size if I don't mistake)?

> +		printk(KERN_ERR "Error, ports in use.\n");
> +		return -EBUSY;
> +	}
> +
> +	if (ide_register_hw(&ide_ctlr_info, NULL, 0, &hwif) < 0) {
> +		printk(KERN_WARNING "Palm Chip BK3710 IDE Register Fail\n");
> +		return -ENODEV;
> +	}
> +
> +	hwif->set_pio_mode = &palm_bk3710_set_pio_mode;
> +	hwif->set_dma_mode = &palm_bk3710_set_dma_mode;
> +	hwif->mmio = 1;
> +	hwif->ultra_mask = 0x1f;	/* Ultra DMA Mode 4 Max
> +						(input clk 99MHz) */
> +	hwif->mwdma_mask = 0x7;
> +	hwif->drives[0].autotune = 1;
> +	hwif->drives[1].autotune = 1;
> +
> +	hwif->dmatable_cpu = dma_alloc_coherent(
> +				NULL,
> +				PRD_ENTRIES * PRD_BYTES,
> +				&hwif->dmatable_dma,
> +				GFP_ATOMIC);
> +
> +	if (!hwif->dmatable_cpu) {
> +		printk(KERN_ERR "Error, unable to allocate DMA table.\n");
> +		hwif->ultra_mask = 0;
> +		hwif->mwdma_mask = 0;
> +		return 0;
> +	}
> +

    You don't need to allocate PRD table -- ide_setup_dma() will do it for you 
anyway, although using pci_alloc_consistent().

> +	hwif->dma_base = mem->start;
> +
> +	hwif->dma_master = mem->start;
> +
> +	hwif->dma_command = mem->start;
> +	hwif->dma_status = mem->start + 2;
> +	hwif->dma_prdtable = mem->start + 4;

    You don't need 5 above assignments - ide_setup_dma() will do this for you.

> +	ide_setup_dma(hwif, mem->start, 8);
> +
> +	return 0;
> +}

MBR, Sergei

Re: [PATCH] Palmchip BK3710 IDE driver

[Alan Cox]

>     Why you chose to use ioread32() and iowrite32() if your device is strictly 
> memory mapped? Those functions add some overhead, and boil down to readl() and 

There are distinct portability advantages but you shouldn't mix
ioread32/iowrite32 with ioremap as that isn't guaranteed to work.
readl/writel does fine and fixes up the driver.

Re: [PATCH] Palmchip BK3710 IDE driver

[Sergei Shtylyov]

Hello.

Alan Cox wrote:

>>    Why you chose to use ioread32() and iowrite32() if your device is strictly 
>>memory mapped? Those functions add some overhead, and boil down to readl() and 

> There are distinct portability advantages but you shouldn't mix
> ioread32/iowrite32 with ioremap as that isn't guaranteed to work.

    Hm, pci_iomap() boils down to ioremap(), so

> readl/writel does fine and fixes up the driver.

    Which brings up another issue with the latest patch: Anton, where do you 
call ioremap() or something alike to get the virtual address of the registers?

MBR, Sergei

Re: [PATCH] Palmchip BK3710 IDE driver

[Sergei Shtylyov]

Anton Salnikov wrote:

> This is Palmchip BK3710 IDE controller support for kernel version 2.6.24-rc8.
> The IDE controller logic supports PIO, multiword DMA and ultra-DMA modes.
> Supports interface to compact Flash (CF) configured in True-IDE mode.

> Use ide_setup_dma() since BLK_DEV_PALMCHIP_BK3710 selects BLK_DEV_IDEDMA_PCI
> So I deleted exports from ide-dma.c

> Signed-off-by: Anton Salnikov <asalnikov@ru.mvista.com>

    Looks like I found another issue caused by using iowrite32() for 16-bit 
registers:

> Index: 2.6.24-rc8.ide/drivers/ide/arm/palm_bk3710.c
> ===================================================================
> --- /dev/null
> +++ 2.6.24-rc8.ide/drivers/ide/arm/palm_bk3710.c
> @@ -0,0 +1,434 @@
[...]
> +#define BK3710_BMICP		0x00
> +#define BK3710_BMISP		0x02
> +#define BK3710_BMIDTP		0x04
> +#define BK3710_BMICS		0x08
> +#define BK3710_BMISS		0x0A
> +#define BK3710_BMIDTS		0x0C
> +#define BK3710_IDETIMP		0x40
> +#define BK3710_IDETIMS		0x42
> +#define BK3710_SIDETIM		0x44
> +#define BK3710_SLEWCTL		0x45
> +#define BK3710_IDESTATUS	0x47
> +#define BK3710_UDMACTL		0x48
> +#define BK3710_UDMATIM		0x4A

    Note that the above two registers are 16-bit...

> +#define BK3710_MISCCTL		0x50
> +#define BK3710_REGSTB		0x54
> +#define BK3710_REGRCVR		0x58
> +#define BK3710_DATSTB		0x5C
> +#define BK3710_DATRCVR		0x60
> +#define BK3710_DMASTB		0x64
> +#define BK3710_DMARCVR		0x68
> +#define BK3710_UDMASTB		0x6C
> +#define BK3710_UDMATRP		0x70
> +#define BK3710_UDMAENV		0x74
> +#define BK3710_IORDYTMP		0x78
> +#define BK3710_IORDYTMS		0x7C
[...]
> +static void palm_bk3710_setudmamode(unsigned int dev, unsigned int mode)
> +{
> +	u8 tenv, trp, t0;
> +	u32 val;
> +
> +	/* DMA Data Setup */
> +	t0 = (palm_bk3710_udmatimings[mode].cycletime + ide_palm_clk - 1)
> +			/ ide_palm_clk - 1;
> +	tenv = (20 + ide_palm_clk - 1) / ide_palm_clk - 1;
> +	trp = (palm_bk3710_udmatimings[mode].rptime + ide_palm_clk - 1)
> +			/ ide_palm_clk - 1;
> +
> +	/* udmatim Register */
> +	val = ioread(BK3710_UDMATIM) & ((!dev) ? 0xFFF0 : 0xFF0F);
> +	iowrite(val, BK3710_UDMATIM);
> +	val = ioread(BK3710_UDMATIM) | (mode << ((!dev) ? 0 : 4));
> +	iowrite(val, BK3710_UDMATIM);

    This register is 16-bit, so:

1) you're performing unaligned access which would end up emulated in the best
    case;
2) you're clearing MISCCTL reg. inadvertently...

[...]

> +static void palm_bk3710_setdmamode(unsigned int dev, unsigned int cycletime,
> +				  unsigned int mode)
> +{
> +	u8 td, tkw, t0;
> +	u32 val;
> +
> +	if (cycletime < ide_timing[mode].cycle)
> +		cycletime = ide_timing[mode].cycle;
> +
> +	/* DMA Data Setup */
> +	t0 = (cycletime + ide_palm_clk - 1) / ide_palm_clk;
> +	td = (ide_timing_find_mode(mode)->active + ide_palm_clk - 1)
> +			/ ide_palm_clk;
> +	tkw = t0 - td - 1;
> +	td -= 1;
> +
> +	val = ioread(BK3710_DMASTB) & (0xFF << ((!dev) << 4));
> +	iowrite(val, BK3710_DMASTB);
> +	val = ioread(BK3710_DMASTB) | (td << (dev << 4));
> +	iowrite(val, BK3710_DMASTB);
> +
> +	val = ioread(BK3710_DMARCVR) & (0xFF << ((!dev) << 4));
> +	iowrite(val, BK3710_DMARCVR);
> +	val = ioread(BK3710_DMARCVR) | (tkw << (dev << 4));
> +	iowrite(val, BK3710_DMARCVR);
> +
> +	/* Disable UDMA for Device */
> +	val = ioread(BK3710_UDMACTL) & (0xFF00 | (1 << (!dev)));
> +	iowrite(val, BK3710_UDMACTL);

    This register is also 16-bit, so:

1) you're performing unaligned access which would end up emulated in the best
    case;
2) you're clearing UDMATIM reg. inadvertently (although this seems safe as 
you're disabling UltraDMA anyway...

[...]

> +static void __devinit palm_bk3710_chipinit(void)
> +{
> +	/*
> +	 * enable the reset_en of ATA controller so that when ata signals
> +	 * are brought out, by writing into device config. at that
> +	 * time por_n signal should not be 'Z' and have a stable value.
> +	 */
> +	iowrite(0x0300, BK3710_MISCCTL);
> +
> +	/* wait for some time and deassert the reset of ATA Device. */
> +	mdelay(100);
> +
> +	/* Deassert the Reset */
> +	iowrite(0x0200, BK3710_MISCCTL);
> +
> +	/*
> +	 * Program the IDETIMP Register Value based on the following assumptions
> +	 *
> +	 * (ATA_IDETIMP_IDEEN		, ENABLE ) |
> +	 * (ATA_IDETIMP_SLVTIMEN	, DISABLE) |
> +	 * (ATA_IDETIMP_RDYSMPL		, 70NS)    |
> +	 * (ATA_IDETIMP_RDYRCVRY	, 50NS)    |
> +	 * (ATA_IDETIMP_DMAFTIM1	, PIOCOMP) |
> +	 * (ATA_IDETIMP_PREPOST1	, DISABLE) |
> +	 * (ATA_IDETIMP_RDYSEN1		, DISABLE) |
> +	 * (ATA_IDETIMP_PIOFTIM1	, DISABLE) |
> +	 * (ATA_IDETIMP_DMAFTIM0	, PIOCOMP) |
> +	 * (ATA_IDETIMP_PREPOST0	, DISABLE) |
> +	 * (ATA_IDETIMP_RDYSEN0		, DISABLE) |
> +	 * (ATA_IDETIMP_PIOFTIM0	, DISABLE)
> +	 */
> +	iowrite(0xB388, BK3710_IDETIMP);

    This register is 16-bit, so:

1) you're performing unaligned access which would end up emulated in the best
    case;
2) you're clearing IDETIMS reg. inadvertently (probably safe as your device 
doesn't seem to have a secondary channel...

[...]

> +
> +	/*
> +	 * Configure  SIDETIM  Register
> +	 * (ATA_SIDETIM_RDYSMPS1	,120NS ) |
> +	 * (ATA_SIDETIM_RDYRCYS1	,120NS )
> +	 */
> +	iowrite(0, BK3710_SIDETIM);

    This register is 8-bit, so:

1) you're performing unaligned access which would end up emulated in the best
    case;
2) you're clearing SLEWCTL/IDESTATUS regs inadvertently...

> +
> +	/*
> +	 * UDMACTL Ultra-ATA DMA Control
> +	 * (ATA_UDMACTL_UDMAP1	, 0 ) |
> +	 * (ATA_UDMACTL_UDMAP0	, 0 )
> +	 *
> +	 */
> +	iowrite(0, BK3710_UDMACTL);

    This register is 16-bit, so:

1) you're performing unaligned access which would end up emulated in the best
    case;
2) you're clearing UDMATIM regs inadvertently...

> +	/*
> +	 * Configure BMISP Register
> +	 * (ATA_BMISP_DMAEN1	, DISABLE )	|
> +	 * (ATA_BMISP_DMAEN0	, DISABLE )	|
> +	 * (ATA_BMISP_IORDYINT	, CLEAR)	|
> +	 * (ATA_BMISP_INTRSTAT	, CLEAR)	|
> +	 * (ATA_BMISP_DMAERROR	, CLEAR)
> +	 */
> +	iowrite(0, BK3710_BMISP);

    This register is 8-bit, so:

1) you're performing unaligned access which would end up emulated in the best
    case;
2) you're clearing BMIDTP regs inadvertently...

MBR, Sergei

Re: [PATCH] Palmchip BK3710 IDE driver

[Alan Cox]

On Wed, 23 Jan 2008 16:38:06 +0300
Sergei Shtylyov <sshtylyov@ru.mvista.com> wrote:

> Hello.
> 
> Alan Cox wrote:
> 
> >>    Why you chose to use ioread32() and iowrite32() if your device is strictly 
> >>memory mapped? Those functions add some overhead, and boil down to readl() and 
> 
> > There are distinct portability advantages but you shouldn't mix
> > ioread32/iowrite32 with ioremap as that isn't guaranteed to work.
> 
>     Hm, pci_iomap() boils down to ioremap(), so

For most cases on existing hardware. Do not rely on that in future.

Alan

[PATCH] Palmchip BK3710 IDE driver

[Anton Salnikov]

This is Palmchip BK3710 IDE controller support for kernel version 2.6.24-rc8.
The IDE controller logic supports PIO, multiword DMA and ultra-DMA modes.
Supports interface to compact Flash (CF) configured in True-IDE mode.

Signed-off-by: Anton Salnikov <asalnikov@ru.mvista.com>
---

 drivers/ide/Kconfig           |    8 
 drivers/ide/arm/Makefile      |    1 
 drivers/ide/arm/palm_bk3710.c |  428 ++++++++++++++++++++++++++++++++++++++++++
 drivers/ide/ide-proc.c        |    1 
 include/linux/ide.h           |    2 
 5 files changed, 439 insertions(+), 1 deletion(-)

Index: 2.6.24-rc8.ide/drivers/ide/Kconfig
===================================================================
--- 2.6.24-rc8.ide.orig/drivers/ide/Kconfig
+++ 2.6.24-rc8.ide/drivers/ide/Kconfig
@@ -1008,6 +1008,14 @@ config BLK_DEV_Q40IDE
 	  normally be on; disable it only if you are running a custom hard
 	  drive subsystem through an expansion card.
 
+config BLK_DEV_PALMCHIP_BK3710
+	bool "Palmchip bk3710 IDE controller support"
+	depends on ARCH_DAVINCI
+	select BLK_DEV_IDEDMA_PCI
+	help
+	  Say Y here if you want to support the onchip IDE controller on the
+	  TI DaVinci SoC
+
 config BLK_DEV_MPC8xx_IDE
 	bool "MPC8xx IDE support"
 	depends on 8xx && (LWMON || IVMS8 || IVML24 || TQM8xxL) && IDE=y && 
BLK_DEV_IDE=y && !PPC_MERGE
Index: 2.6.24-rc8.ide/drivers/ide/arm/Makefile
===================================================================
--- 2.6.24-rc8.ide.orig/drivers/ide/arm/Makefile
+++ 2.6.24-rc8.ide/drivers/ide/arm/Makefile
@@ -2,5 +2,6 @@
 obj-$(CONFIG_BLK_DEV_IDE_ICSIDE)	+= icside.o
 obj-$(CONFIG_BLK_DEV_IDE_RAPIDE)	+= rapide.o
 obj-$(CONFIG_BLK_DEV_IDE_BAST)		+= bast-ide.o
+obj-$(CONFIG_BLK_DEV_PALMCHIP_BK3710)	+= palm_bk3710.o
 
 EXTRA_CFLAGS	:= -Idrivers/ide
Index: 2.6.24-rc8.ide/drivers/ide/arm/palm_bk3710.c
===================================================================
--- /dev/null
+++ 2.6.24-rc8.ide/drivers/ide/arm/palm_bk3710.c
@@ -0,0 +1,428 @@
+/*
+ * Palmchip bk3710 IDE controller
+ *
+ * Copyright (C) 2006 Texas Instruments.
+ * Copyright (C) 2007 MontaVista Software, Inc., <source@mvista.com>
+ *
+ * ----------------------------------------------------------------------------
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ * ----------------------------------------------------------------------------
+ *
+ */
+
+#include <linux/types.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/ioport.h>
+#include <linux/hdreg.h>
+#include <linux/ide.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/clk.h>
+#include <linux/platform_device.h>
+
+/* Offset of the primary interface registers */
+#define IDE_PALM_ATA_PRI_REG_OFFSET 0x1F0
+
+/* Primary Control Offset */
+#define IDE_PALM_ATA_PRI_CTL_OFFSET 0x3F6
+
+/*
+ * PalmChip 3710 IDE Controller UDMA timing structure Definition
+ */
+struct palm_bk3710_udmatiming {
+	unsigned int rptime;	/* Ready to pause time  */
+	unsigned int cycletime;	/* Cycle Time           */
+};
+
+#define BK3710_BMICP		0x00
+#define BK3710_BMISP		0x02
+#define BK3710_BMIDTP		0x04
+#define BK3710_BMICS		0x08
+#define BK3710_BMISS		0x0A
+#define BK3710_BMIDTS		0x0C
+#define BK3710_IDETIMP		0x40
+#define BK3710_IDETIMS		0x42
+#define BK3710_SIDETIM		0x44
+#define BK3710_SLEWCTL		0x45
+#define BK3710_IDESTATUS	0x47
+#define BK3710_UDMACTL		0x48
+#define BK3710_UDMATIM		0x4A
+#define BK3710_MISCCTL		0x50
+#define BK3710_REGSTB		0x54
+#define BK3710_REGRCVR		0x58
+#define BK3710_DATSTB		0x5C
+#define BK3710_DATRCVR		0x60
+#define BK3710_DMASTB		0x64
+#define BK3710_DMARCVR		0x68
+#define BK3710_UDMASTB		0x6C
+#define BK3710_UDMATRP		0x70
+#define BK3710_UDMAENV		0x74
+#define BK3710_IORDYTMP		0x78
+#define BK3710_IORDYTMS		0x7C
+
+#include "../ide-timing.h"
+
+static long ide_palm_clk;
+
+static const struct palm_bk3710_udmatiming palm_bk3710_udmatimings[6] = {
+	{160, 240},		/* UDMA Mode 0 */
+	{125, 160},		/* UDMA Mode 1 */
+	{100, 120},		/* UDMA Mode 2 */
+	{100, 90},		/* UDMA Mode 3 */
+	{85,  60},		/* UDMA Mode 4 */
+};
+
+static struct clk *ideclkp;
+
+static inline u8 hwif_read8(u32 base, u32 reg)
+{
+	return readb(base + reg);
+}
+
+static inline void hwif_write8(u32 base, u8 val, u32 reg)
+{
+	writeb(val, base + reg);
+}
+
+static inline u16 hwif_read16(u32 base, u32 reg)
+{
+	return readw(base + reg);
+}
+
+static inline void hwif_write16(u32 base, u16 val, u32 reg)
+{
+	writew(val, base + reg);
+}
+
+static inline u32 hwif_read32(u32 base, u32 reg)
+{
+	return readl(base + reg);
+}
+
+static inline void hwif_write32(u32 base, u32 val, u32 reg)
+{
+	writel(val, base + reg);
+}
+
+static void palm_bk3710_setudmamode(u32 base, unsigned int dev,
+				    unsigned int mode)
+{
+	u8 tenv, trp, t0;
+	u32 val32;
+	u16 val16;
+
+	/* DMA Data Setup */
+	t0 = (palm_bk3710_udmatimings[mode].cycletime + ide_palm_clk - 1)
+			/ ide_palm_clk - 1;
+	tenv = (20 + ide_palm_clk - 1) / ide_palm_clk - 1;
+	trp = (palm_bk3710_udmatimings[mode].rptime + ide_palm_clk - 1)
+			/ ide_palm_clk - 1;
+
+	/* udmatim Register */
+	val16 = hwif_read16(base, BK3710_UDMATIM) & (dev ? 0xFF0F : 0xFFF0);
+	val16 |= (mode << ((!dev) ? 0 : 4));
+	hwif_write16(base, val16, BK3710_UDMATIM);
+
+	/* udmastb Ultra DMA Access Strobe Width */
+	val32 = hwif_read32(base, BK3710_UDMASTB) & (0xFF << (dev ? 0 : 8));
+	val32 |= (t0 << (dev ? 8 : 0));
+	hwif_write32(base, val32, BK3710_UDMASTB);
+
+	/* udmatrp Ultra DMA Ready to Pause Time */
+	val32 = hwif_read32(base, BK3710_UDMATRP) & (0xFF << (dev ? 0 : 8));
+	val32 |= (trp << (dev ? 8 : 0));
+	hwif_write32(base, val32, BK3710_UDMATRP);
+
+	/* udmaenv Ultra DMA envelop Time */
+	val32 = hwif_read32(base, BK3710_UDMAENV) & (0xFF << (dev ? 0 : 8));
+	val32 |= (tenv << (dev ? 8 : 0));
+	hwif_write32(base, val32, BK3710_UDMAENV);
+
+	/* Enable UDMA for Device */
+	val16 = hwif_read16(base, BK3710_UDMACTL) | (1 << dev);
+	hwif_write16(base, val16, BK3710_UDMACTL);
+}
+
+static void palm_bk3710_setdmamode(u32 base, unsigned int dev,
+				   unsigned short min_cycle,
+				   unsigned int mode)
+{
+	u8 td, tkw, t0;
+	u32 val32;
+	u16 val16;
+
+	unsigned cycletime = max_t(int, ide_timing_find_mode(mode)->cycle,
+				   min_cycle);
+
+	/* DMA Data Setup */
+	t0 = (cycletime + ide_palm_clk - 1) / ide_palm_clk;
+	td = (ide_timing_find_mode(mode)->active + ide_palm_clk - 1)
+			/ ide_palm_clk;
+	tkw = t0 - td - 1;
+	td -= 1;
+
+	val32 = hwif_read32(base, BK3710_DMASTB) & (0xFF << (dev ? 0 : 8));
+	val32 |= (td << (dev ? 8 : 0));
+	hwif_write32(base, val32, BK3710_DMASTB);
+
+	val32 = hwif_read32(base, BK3710_DMARCVR) & (0xFF << (dev ? 0 : 8));
+	val32 |= (tkw << (dev ? 8 : 0));
+	hwif_write32(base, val32, BK3710_DMARCVR);
+
+	/* Disable UDMA for Device */
+	val16 = hwif_read16(base, BK3710_UDMACTL) & ~(1 << dev);
+	hwif_write16(base, val16, BK3710_UDMACTL);
+}
+
+static void palm_bk3710_setpiomode(u32 base, ide_drive_t *mate,
+				   unsigned int dev, unsigned int cycletime,
+				   unsigned int mode)
+{
+	u8 t2, t2i, t0;
+	u32 val32;
+	struct ide_timing *t;
+
+	/* PIO Data Setup */
+	t0 = (cycletime + ide_palm_clk - 1) / ide_palm_clk;
+	t2 = (ide_timing_find_mode(XFER_PIO_0 + mode)->active +
+	      ide_palm_clk - 1)	/ ide_palm_clk;
+
+	t2i = t0 - t2 - 1;
+	t2 -= 1;
+
+	val32 = hwif_read32(base, BK3710_DATSTB) & (0xFF << (dev ? 0 : 8));
+	val32 |= (t2 << (dev ? 8 : 0));
+	hwif_write32(base, val32, BK3710_DATSTB);
+
+	val32 = hwif_read32(base, BK3710_DATRCVR) & (0xFF << (dev ? 0 : 8));
+	val32 |= (t2i << (dev ? 8 : 0));
+	hwif_write32(base, val32, BK3710_DATRCVR);
+
+	if (mate && mate->present) {
+		u8 mode2 = ide_get_best_pio_mode(mate, 255, 4);
+
+		if (mode2 < mode)
+			mode = mode2;
+	}
+
+	/* TASKFILE Setup */
+	t = ide_timing_find_mode(XFER_PIO_0 + mode);
+	t0 = (t->cyc8b + ide_palm_clk - 1) / ide_palm_clk;
+	t2 = (t->act8b + ide_palm_clk - 1) / ide_palm_clk;
+
+	t2i = t0 - t2 - 1;
+	t2 -= 1;
+
+	val32 = hwif_read32(base, BK3710_REGSTB) & (0xFF << (dev ? 0 : 8));
+	val32 |= (t2 << (dev ? 8 : 0));
+	hwif_write32(base, val32, BK3710_REGSTB);
+
+	val32 = hwif_read32(base, BK3710_REGRCVR) & (0xFF << (dev ? 0 : 8));
+	val32 |= (t2i << (dev ? 8 : 0));
+	hwif_write32(base, val32, BK3710_REGRCVR);
+}
+
+static void palm_bk3710_set_dma_mode(ide_drive_t *drive, u8 xferspeed)
+{
+	int is_slave = drive->dn & 1;
+	u32 base = drive->hwif->dma_master;
+
+	if (xferspeed >= XFER_UDMA_0) {
+		palm_bk3710_setudmamode(base, is_slave,
+					xferspeed - XFER_UDMA_0);
+	} else {
+		palm_bk3710_setdmamode(base, is_slave, drive->id->eide_dma_min,
+				       xferspeed);
+	}
+}
+
+static void palm_bk3710_set_pio_mode(ide_drive_t *drive, u8 pio)
+{
+	unsigned int cycle_time;
+	int is_slave = drive->dn & 1;
+	ide_drive_t *mate;
+	u32 base = drive->hwif->dma_master;
+
+	/*
+	 * Obtain the drive PIO data for tuning the Palm Chip registers
+	 */
+	cycle_time = ide_pio_cycle_time(drive, pio);
+	mate = ide_get_paired_drive(drive);
+	palm_bk3710_setpiomode(base, mate, is_slave, cycle_time, pio);
+}
+
+static void __devinit palm_bk3710_chipinit(u32 base)
+{
+	/*
+	 * enable the reset_en of ATA controller so that when ata signals
+	 * are brought out, by writing into device config. at that
+	 * time por_n signal should not be 'Z' and have a stable value.
+	 */
+	hwif_write32(base, 0x0300, BK3710_MISCCTL);
+
+	/* wait for some time and deassert the reset of ATA Device. */
+	mdelay(100);
+
+	/* Deassert the Reset */
+	hwif_write32(base, 0x0200, BK3710_MISCCTL);
+
+	/*
+	 * Program the IDETIMP Register Value based on the following assumptions
+	 *
+	 * (ATA_IDETIMP_IDEEN		, ENABLE ) |
+	 * (ATA_IDETIMP_SLVTIMEN	, DISABLE) |
+	 * (ATA_IDETIMP_RDYSMPL		, 70NS)    |
+	 * (ATA_IDETIMP_RDYRCVRY	, 50NS)    |
+	 * (ATA_IDETIMP_DMAFTIM1	, PIOCOMP) |
+	 * (ATA_IDETIMP_PREPOST1	, DISABLE) |
+	 * (ATA_IDETIMP_RDYSEN1		, DISABLE) |
+	 * (ATA_IDETIMP_PIOFTIM1	, DISABLE) |
+	 * (ATA_IDETIMP_DMAFTIM0	, PIOCOMP) |
+	 * (ATA_IDETIMP_PREPOST0	, DISABLE) |
+	 * (ATA_IDETIMP_RDYSEN0		, DISABLE) |
+	 * (ATA_IDETIMP_PIOFTIM0	, DISABLE)
+	 */
+	hwif_write16(base, 0xB388, BK3710_IDETIMP);
+
+	/*
+	 * Configure  SIDETIM  Register
+	 * (ATA_SIDETIM_RDYSMPS1	,120NS ) |
+	 * (ATA_SIDETIM_RDYRCYS1	,120NS )
+	 */
+	hwif_write8(base, 0, BK3710_SIDETIM);
+
+	/*
+	 * UDMACTL Ultra-ATA DMA Control
+	 * (ATA_UDMACTL_UDMAP1	, 0 ) |
+	 * (ATA_UDMACTL_UDMAP0	, 0 )
+	 *
+	 */
+	hwif_write16(base, 0, BK3710_UDMACTL);
+
+	/*
+	 * MISCCTL Miscellaneous Conrol Register
+	 * (ATA_MISCCTL_RSTMODEP	, 1) |
+	 * (ATA_MISCCTL_RESETP		, 0) |
+	 * (ATA_MISCCTL_TIMORIDE	, 1)
+	 */
+	hwif_write32(base, 0x201, BK3710_MISCCTL);
+
+	/*
+	 * IORDYTMP IORDY Timer for Primary Register
+	 * (ATA_IORDYTMP_IORDYTMP     , 0xffff  )
+	 */
+	hwif_write32(base, 0xFFFF, BK3710_IORDYTMP);
+
+	/*
+	 * Configure BMISP Register
+	 * (ATA_BMISP_DMAEN1	, DISABLE )	|
+	 * (ATA_BMISP_DMAEN0	, DISABLE )	|
+	 * (ATA_BMISP_IORDYINT	, CLEAR)	|
+	 * (ATA_BMISP_INTRSTAT	, CLEAR)	|
+	 * (ATA_BMISP_DMAERROR	, CLEAR)
+	 */
+	hwif_write16(base, 0, BK3710_BMISP);
+
+	palm_bk3710_setpiomode(base, NULL, 0, 600, 0);
+	palm_bk3710_setpiomode(base, NULL, 1, 600, 0);
+}
+static int __devinit palm_bk3710_probe(struct platform_device *pdev)
+{
+	hw_regs_t ide_ctlr_info;
+	int index = 0;
+	int pribase;
+	struct clk *clkp;
+	struct resource *mem, *irq;
+	ide_hwif_t *hwif;
+	u32 base;
+	int ret;
+
+	clkp = clk_get(NULL, "IDECLK");
+	if (IS_ERR(clkp))
+		return -ENODEV;
+
+	ideclkp = clkp;
+	clk_enable(ideclkp);
+	ide_palm_clk = clk_get_rate(ideclkp)/100000;
+	ide_palm_clk = (10000/ide_palm_clk) + 1;
+	/* Register the IDE interface with Linux ATA Interface */
+	memset(&ide_ctlr_info, 0, sizeof(ide_ctlr_info));
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (mem == NULL) {
+		printk(KERN_INFO "failed to get memory region resource\n");
+		return -ENODEV;
+	}
+	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (irq == NULL) {
+		printk(KERN_INFO "failed to get IRQ resource\n");
+		return -ENODEV;
+	}
+
+	base = mem->start;
+
+	/* Configure the Palm Chip controller */
+	palm_bk3710_chipinit(base);
+
+	pribase = mem->start + IDE_PALM_ATA_PRI_REG_OFFSET;
+	for (index = 0; index < IDE_NR_PORTS - 2; index++)
+		ide_ctlr_info.io_ports[index] = pribase + index;
+	ide_ctlr_info.io_ports[IDE_CONTROL_OFFSET] = mem->start +
+			IDE_PALM_ATA_PRI_CTL_OFFSET;
+	ide_ctlr_info.irq = irq->start;
+	ide_ctlr_info.chipset = ide_palm3710;
+
+	if (!request_mem_region(mem->start, 8, "palm_bk3710")) {
+		printk(KERN_ERR "Error, memory region in use.\n");
+		return -EBUSY;
+	}
+
+	if (ide_register_hw(&ide_ctlr_info, NULL, 0, &hwif) < 0) {
+		printk(KERN_WARNING "Palm Chip BK3710 IDE Register Fail\n");
+		return -ENODEV;
+	}
+
+	hwif->set_pio_mode = &palm_bk3710_set_pio_mode;
+	hwif->set_dma_mode = &palm_bk3710_set_dma_mode;
+	hwif->mmio = 1;
+	hwif->ultra_mask = 0x1f;		/* Ultra DMA Mode 4 Max
+						(input clk 99MHz) */
+	hwif->mwdma_mask = 0x7;
+	hwif->drives[0].autotune = 1;
+	hwif->drives[1].autotune = 1;
+
+	ide_setup_dma(hwif, mem->start, 8);
+
+	return 0;
+}
+
+static struct platform_driver platform_bk_driver = {
+	.driver = {
+		.name = "palm_bk3710",
+	},
+	.probe = palm_bk3710_probe,
+	.remove = NULL,
+};
+
+static int __init palm_bk3710_init(void)
+{
+	return platform_driver_register(&platform_bk_driver);
+}
+
+module_init(palm_bk3710_init);
+MODULE_LICENSE("GPL");
+
Index: 2.6.24-rc8.ide/drivers/ide/ide-proc.c
===================================================================
--- 2.6.24-rc8.ide.orig/drivers/ide/ide-proc.c
+++ 2.6.24-rc8.ide/drivers/ide/ide-proc.c
@@ -67,6 +67,7 @@ static int proc_ide_read_imodel
 		case ide_4drives:	name = "4drives";	break;
 		case ide_pmac:		name = "mac-io";	break;
 		case ide_au1xxx:	name = "au1xxx";	break;
+		case ide_palm3710:      name = "palm3710";      break;
 		case ide_etrax100:	name = "etrax100";	break;
 		case ide_acorn:		name = "acorn";		break;
 		default:		name = "(unknown)";	break;
Index: 2.6.24-rc8.ide/include/linux/ide.h
===================================================================
--- 2.6.24-rc8.ide.orig/include/linux/ide.h
+++ 2.6.24-rc8.ide/include/linux/ide.h
@@ -202,7 +202,7 @@ enum {		ide_unknown,	ide_generic,	ide_pc
 		ide_rz1000,	ide_trm290,
 		ide_cmd646,	ide_cy82c693,	ide_4drives,
 		ide_pmac,	ide_etrax100,	ide_acorn,
-		ide_au1xxx, ide_forced
+		ide_au1xxx,	ide_palm3710,	ide_forced
 };
 
 typedef u8 hwif_chipset_t;

Re: [PATCH] Palmchip BK3710 IDE driver

[Sergei Shtylyov]

Anton Salnikov wrote:

> This is Palmchip BK3710 IDE controller support for kernel version 2.6.24-rc8.
> The IDE controller logic supports PIO, multiword DMA and ultra-DMA modes.
> Supports interface to compact Flash (CF) configured in True-IDE mode.

> Signed-off-by: Anton Salnikov <asalnikov@ru.mvista.com>
> ---

> Index: 2.6.24-rc8.ide/drivers/ide/arm/palm_bk3710.c
> ===================================================================
> --- /dev/null
> +++ 2.6.24-rc8.ide/drivers/ide/arm/palm_bk3710.c
> @@ -0,0 +1,428 @@
> +static inline u8 hwif_read8(u32 base, u32 reg)
> +{
> +	return readb(base + reg);
> +}
> +
> +static inline void hwif_write8(u32 base, u8 val, u32 reg)
> +{
> +	writeb(val, base + reg);
> +}
> +
> +static inline u16 hwif_read16(u32 base, u32 reg)
> +{
> +	return readw(base + reg);
> +}
> +
> +static inline void hwif_write16(u32 base, u16 val, u32 reg)
> +{
> +	writew(val, base + reg);
> +}
> +
> +static inline u32 hwif_read32(u32 base, u32 reg)
> +{
> +	return readl(base + reg);
> +}
> +
> +static inline void hwif_write32(u32 base, u32 val, u32 reg)
> +{
> +	writel(val, base + reg);
> +}

    Hm, I don't see why you need those accessors but well...

> +static void palm_bk3710_setudmamode(u32 base, unsigned int dev,
> +				    unsigned int mode)
> +{
> +	u8 tenv, trp, t0;
> +	u32 val32;
> +	u16 val16;
> +
> +	/* DMA Data Setup */
> +	t0 = (palm_bk3710_udmatimings[mode].cycletime + ide_palm_clk - 1)
> +			/ ide_palm_clk - 1;
> +	tenv = (20 + ide_palm_clk - 1) / ide_palm_clk - 1;
> +	trp = (palm_bk3710_udmatimings[mode].rptime + ide_palm_clk - 1)
> +			/ ide_palm_clk - 1;
> +
> +	/* udmatim Register */
> +	val16 = hwif_read16(base, BK3710_UDMATIM) & (dev ? 0xFF0F : 0xFFF0);
> +	val16 |= (mode << ((!dev) ? 0 : 4));

    Why not (dev ? 4 : 0) like the rest?

> +	hwif_write16(base, val16, BK3710_UDMATIM);
> +
> +	/* udmastb Ultra DMA Access Strobe Width */
> +	val32 = hwif_read32(base, BK3710_UDMASTB) & (0xFF << (dev ? 0 : 8));
> +	val32 |= (t0 << (dev ? 8 : 0));
> +	hwif_write32(base, val32, BK3710_UDMASTB);
> +
> +	/* udmatrp Ultra DMA Ready to Pause Time */
> +	val32 = hwif_read32(base, BK3710_UDMATRP) & (0xFF << (dev ? 0 : 8));
> +	val32 |= (trp << (dev ? 8 : 0));
> +	hwif_write32(base, val32, BK3710_UDMATRP);
> +
> +	/* udmaenv Ultra DMA envelop Time */
> +	val32 = hwif_read32(base, BK3710_UDMAENV) & (0xFF << (dev ? 0 : 8));
> +	val32 |= (tenv << (dev ? 8 : 0));
> +	hwif_write32(base, val32, BK3710_UDMAENV);
> +
> +	/* Enable UDMA for Device */
> +	val16 = hwif_read16(base, BK3710_UDMACTL) | (1 << dev);
> +	hwif_write16(base, val16, BK3710_UDMACTL);
> +}
> +
> +static void palm_bk3710_setdmamode(u32 base, unsigned int dev,
> +				   unsigned short min_cycle,
> +				   unsigned int mode)
> +{
> +	u8 td, tkw, t0;
> +	u32 val32;
> +	u16 val16;
> +
> +	unsigned cycletime = max_t(int, ide_timing_find_mode(mode)->cycle,
> +				   min_cycle);

    Hm, why integer comparison and then unsigned maximum?

> +
> +	/* DMA Data Setup */
> +	t0 = (cycletime + ide_palm_clk - 1) / ide_palm_clk;
> +	td = (ide_timing_find_mode(mode)->active + ide_palm_clk - 1)
> +			/ ide_palm_clk;

    Couldn't you call ide_timing_find_mode() only once?

[...]

> +static int __devinit palm_bk3710_probe(struct platform_device *pdev)
> +{
> +	hw_regs_t ide_ctlr_info;
> +	int index = 0;
> +	int pribase;
> +	struct clk *clkp;
> +	struct resource *mem, *irq;
> +	ide_hwif_t *hwif;
> +	u32 base;
> +	int ret;
> +
> +	clkp = clk_get(NULL, "IDECLK");
> +	if (IS_ERR(clkp))
> +		return -ENODEV;
> +
> +	ideclkp = clkp;
> +	clk_enable(ideclkp);
> +	ide_palm_clk = clk_get_rate(ideclkp)/100000;
> +	ide_palm_clk = (10000/ide_palm_clk) + 1;
> +	/* Register the IDE interface with Linux ATA Interface */
> +	memset(&ide_ctlr_info, 0, sizeof(ide_ctlr_info));
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (mem == NULL) {
> +		printk(KERN_INFO "failed to get memory region resource\n");

    This deserves KERN_ERR, not KERN_INFO.

> +		return -ENODEV;
> +	}
> +	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +	if (irq == NULL) {
> +		printk(KERN_INFO "failed to get IRQ resource\n");

    This too...

> +		return -ENODEV;
> +	}
> +
> +	base = mem->start;
> +
> +	/* Configure the Palm Chip controller */
> +	palm_bk3710_chipinit(base);
> +
> +	pribase = mem->start + IDE_PALM_ATA_PRI_REG_OFFSET;
> +	for (index = 0; index < IDE_NR_PORTS - 2; index++)
> +		ide_ctlr_info.io_ports[index] = pribase + index;
> +	ide_ctlr_info.io_ports[IDE_CONTROL_OFFSET] = mem->start +
> +			IDE_PALM_ATA_PRI_CTL_OFFSET;
> +	ide_ctlr_info.irq = irq->start;
> +	ide_ctlr_info.chipset = ide_palm3710;
> +
> +	if (!request_mem_region(mem->start, 8, "palm_bk3710")) {
> +		printk(KERN_ERR "Error, memory region in use.\n");
> +		return -EBUSY;
> +	}

    Well, you either have to also register sub-resources for the IDE 
command/control register blocks since with hwif->mmiops == 1 the IDE core 
won't do this for you, or don't register any sub-resources at all, being 
content with what platform_device_add() did for us. BTW, don't forget extend 
the platform resource to account for IDE command/control register blocks.

> +	if (ide_register_hw(&ide_ctlr_info, NULL, 0, &hwif) < 0) {
> +		printk(KERN_WARNING "Palm Chip BK3710 IDE Register Fail\n");
> +		return -ENODEV;
> +	}
> +
> +	hwif->set_pio_mode = &palm_bk3710_set_pio_mode;
> +	hwif->set_dma_mode = &palm_bk3710_set_dma_mode;
> +	hwif->mmio = 1;

    Well, dealing with a memory-mapped IDE device, the driver lack 
default_hwif_mmiops() call...

MBR, Sergei

[PATCH] Palmchip BK3710 IDE driver

[Anton Salnikov]

This is Palmchip BK3710 IDE controller support for kernel version 2.6.24.
The IDE controller logic supports PIO, multiword DMA and ultra-DMA modes.
Supports interface to compact Flash (CF) configured in True-IDE mode.

Signed-off-by: Anton Salnikov <asalnikov@ru.mvista.com>
---

 drivers/ide/Kconfig           |    8 
 drivers/ide/arm/Makefile      |    1 
 drivers/ide/arm/palm_bk3710.c |  424 ++++++++++++++++++++++++++++++++++++++++++
 drivers/ide/ide-proc.c        |    1 
 include/linux/ide.h           |    2 
 5 files changed, 435 insertions(+), 1 deletion(-)

Index: 2.6.24.ide/drivers/ide/Kconfig
===================================================================
--- 2.6.24.ide.orig/drivers/ide/Kconfig
+++ 2.6.24.ide/drivers/ide/Kconfig
@@ -1008,6 +1008,14 @@ config BLK_DEV_Q40IDE
 	  normally be on; disable it only if you are running a custom hard
 	  drive subsystem through an expansion card.
 
+config BLK_DEV_PALMCHIP_BK3710
+	bool "Palmchip bk3710 IDE controller support"
+	depends on ARCH_DAVINCI
+	select BLK_DEV_IDEDMA_PCI
+	help
+	  Say Y here if you want to support the onchip IDE controller on the
+	  TI DaVinci SoC
+
 config BLK_DEV_MPC8xx_IDE
 	bool "MPC8xx IDE support"
 	depends on 8xx && (LWMON || IVMS8 || IVML24 || TQM8xxL) && IDE=y && 
BLK_DEV_IDE=y && !PPC_MERGE
Index: 2.6.24.ide/drivers/ide/arm/Makefile
===================================================================
--- 2.6.24.ide.orig/drivers/ide/arm/Makefile
+++ 2.6.24.ide/drivers/ide/arm/Makefile
@@ -2,5 +2,6 @@
 obj-$(CONFIG_BLK_DEV_IDE_ICSIDE)	+= icside.o
 obj-$(CONFIG_BLK_DEV_IDE_RAPIDE)	+= rapide.o
 obj-$(CONFIG_BLK_DEV_IDE_BAST)		+= bast-ide.o
+obj-$(CONFIG_BLK_DEV_PALMCHIP_BK3710)	+= palm_bk3710.o
 
 EXTRA_CFLAGS	:= -Idrivers/ide
Index: 2.6.24.ide/drivers/ide/arm/palm_bk3710.c
===================================================================
--- /dev/null
+++ 2.6.24.ide/drivers/ide/arm/palm_bk3710.c
@@ -0,0 +1,424 @@
+/*
+ * Palmchip bk3710 IDE controller
+ *
+ * Copyright (C) 2006 Texas Instruments.
+ * Copyright (C) 2007 MontaVista Software, Inc., <source@mvista.com>
+ *
+ * ----------------------------------------------------------------------------
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ * ----------------------------------------------------------------------------
+ *
+ */
+
+#include <linux/types.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/ioport.h>
+#include <linux/hdreg.h>
+#include <linux/ide.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/clk.h>
+#include <linux/platform_device.h>
+
+/* Offset of the primary interface registers */
+#define IDE_PALM_ATA_PRI_REG_OFFSET 0x1F0
+
+/* Primary Control Offset */
+#define IDE_PALM_ATA_PRI_CTL_OFFSET 0x3F6
+
+/*
+ * PalmChip 3710 IDE Controller UDMA timing structure Definition
+ */
+struct palm_bk3710_udmatiming {
+	unsigned int rptime;	/* Ready to pause time  */
+	unsigned int cycletime;	/* Cycle Time           */
+};
+
+#define BK3710_BMICP		0x00
+#define BK3710_BMISP		0x02
+#define BK3710_BMIDTP		0x04
+#define BK3710_BMICS		0x08
+#define BK3710_BMISS		0x0A
+#define BK3710_BMIDTS		0x0C
+#define BK3710_IDETIMP		0x40
+#define BK3710_IDETIMS		0x42
+#define BK3710_SIDETIM		0x44
+#define BK3710_SLEWCTL		0x45
+#define BK3710_IDESTATUS	0x47
+#define BK3710_UDMACTL		0x48
+#define BK3710_UDMATIM		0x4A
+#define BK3710_MISCCTL		0x50
+#define BK3710_REGSTB		0x54
+#define BK3710_REGRCVR		0x58
+#define BK3710_DATSTB		0x5C
+#define BK3710_DATRCVR		0x60
+#define BK3710_DMASTB		0x64
+#define BK3710_DMARCVR		0x68
+#define BK3710_UDMASTB		0x6C
+#define BK3710_UDMATRP		0x70
+#define BK3710_UDMAENV		0x74
+#define BK3710_IORDYTMP		0x78
+#define BK3710_IORDYTMS		0x7C
+
+#include "../ide-timing.h"
+
+static long ide_palm_clk;
+
+static const struct palm_bk3710_udmatiming palm_bk3710_udmatimings[6] = {
+	{160, 240},		/* UDMA Mode 0 */
+	{125, 160},		/* UDMA Mode 1 */
+	{100, 120},		/* UDMA Mode 2 */
+	{100, 90},		/* UDMA Mode 3 */
+	{85,  60},		/* UDMA Mode 4 */
+};
+
+static struct clk *ideclkp;
+
+static inline u8 hwif_read8(u32 base, u32 reg)
+{
+	return readb(base + reg);
+}
+
+static inline void hwif_write8(u32 base, u8 val, u32 reg)
+{
+	writeb(val, base + reg);
+}
+
+static inline u16 hwif_read16(u32 base, u32 reg)
+{
+	return readw(base + reg);
+}
+
+static inline void hwif_write16(u32 base, u16 val, u32 reg)
+{
+	writew(val, base + reg);
+}
+
+static inline u32 hwif_read32(u32 base, u32 reg)
+{
+	return readl(base + reg);
+}
+
+static inline void hwif_write32(u32 base, u32 val, u32 reg)
+{
+	writel(val, base + reg);
+}
+
+static void palm_bk3710_setudmamode(u32 base, unsigned int dev,
+				    unsigned int mode)
+{
+	u8 tenv, trp, t0;
+	u32 val32;
+	u16 val16;
+
+	/* DMA Data Setup */
+	t0 = (palm_bk3710_udmatimings[mode].cycletime + ide_palm_clk - 1)
+			/ ide_palm_clk - 1;
+	tenv = (20 + ide_palm_clk - 1) / ide_palm_clk - 1;
+	trp = (palm_bk3710_udmatimings[mode].rptime + ide_palm_clk - 1)
+			/ ide_palm_clk - 1;
+
+	/* udmatim Register */
+	val16 = hwif_read16(base, BK3710_UDMATIM) & (dev ? 0xFF0F : 0xFFF0);
+	val16 |= (mode << (dev ? 4 : 0));
+	hwif_write16(base, val16, BK3710_UDMATIM);
+
+	/* udmastb Ultra DMA Access Strobe Width */
+	val32 = hwif_read32(base, BK3710_UDMASTB) & (0xFF << (dev ? 0 : 8));
+	val32 |= (t0 << (dev ? 8 : 0));
+	hwif_write32(base, val32, BK3710_UDMASTB);
+
+	/* udmatrp Ultra DMA Ready to Pause Time */
+	val32 = hwif_read32(base, BK3710_UDMATRP) & (0xFF << (dev ? 0 : 8));
+	val32 |= (trp << (dev ? 8 : 0));
+	hwif_write32(base, val32, BK3710_UDMATRP);
+
+	/* udmaenv Ultra DMA envelop Time */
+	val32 = hwif_read32(base, BK3710_UDMAENV) & (0xFF << (dev ? 0 : 8));
+	val32 |= (tenv << (dev ? 8 : 0));
+	hwif_write32(base, val32, BK3710_UDMAENV);
+
+	/* Enable UDMA for Device */
+	val16 = hwif_read16(base, BK3710_UDMACTL) | (1 << dev);
+	hwif_write16(base, val16, BK3710_UDMACTL);
+}
+
+static void palm_bk3710_setdmamode(u32 base, unsigned int dev,
+				   unsigned short min_cycle,
+				   unsigned int mode)
+{
+	u8 td, tkw, t0;
+	u32 val32;
+	u16 val16;
+	struct ide_timing *t;
+	int cycletime;
+
+	t = ide_timing_find_mode(mode);
+	cycletime = max_t(int, t->cycle, min_cycle);
+
+	/* DMA Data Setup */
+	t0 = (cycletime + ide_palm_clk - 1) / ide_palm_clk;
+	td = (t->active + ide_palm_clk - 1) / ide_palm_clk;
+	tkw = t0 - td - 1;
+	td -= 1;
+
+	val32 = hwif_read32(base, BK3710_DMASTB) & (0xFF << (dev ? 0 : 8));
+	val32 |= (td << (dev ? 8 : 0));
+	hwif_write32(base, val32, BK3710_DMASTB);
+
+	val32 = hwif_read32(base, BK3710_DMARCVR) & (0xFF << (dev ? 0 : 8));
+	val32 |= (tkw << (dev ? 8 : 0));
+	hwif_write32(base, val32, BK3710_DMARCVR);
+
+	/* Disable UDMA for Device */
+	val16 = hwif_read16(base, BK3710_UDMACTL) & ~(1 << dev);
+	hwif_write16(base, val16, BK3710_UDMACTL);
+}
+
+static void palm_bk3710_setpiomode(u32 base, ide_drive_t *mate,
+				   unsigned int dev, unsigned int cycletime,
+				   unsigned int mode)
+{
+	u8 t2, t2i, t0;
+	u32 val32;
+	struct ide_timing *t;
+
+	/* PIO Data Setup */
+	t0 = (cycletime + ide_palm_clk - 1) / ide_palm_clk;
+	t2 = (ide_timing_find_mode(XFER_PIO_0 + mode)->active +
+	      ide_palm_clk - 1)	/ ide_palm_clk;
+
+	t2i = t0 - t2 - 1;
+	t2 -= 1;
+
+	val32 = hwif_read32(base, BK3710_DATSTB) & (0xFF << (dev ? 0 : 8));
+	val32 |= (t2 << (dev ? 8 : 0));
+	hwif_write32(base, val32, BK3710_DATSTB);
+
+	val32 = hwif_read32(base, BK3710_DATRCVR) & (0xFF << (dev ? 0 : 8));
+	val32 |= (t2i << (dev ? 8 : 0));
+	hwif_write32(base, val32, BK3710_DATRCVR);
+
+	if (mate && mate->present) {
+		u8 mode2 = ide_get_best_pio_mode(mate, 255, 4);
+
+		if (mode2 < mode)
+			mode = mode2;
+	}
+
+	/* TASKFILE Setup */
+	t = ide_timing_find_mode(XFER_PIO_0 + mode);
+	t0 = (t->cyc8b + ide_palm_clk - 1) / ide_palm_clk;
+	t2 = (t->act8b + ide_palm_clk - 1) / ide_palm_clk;
+
+	t2i = t0 - t2 - 1;
+	t2 -= 1;
+
+	val32 = hwif_read32(base, BK3710_REGSTB) & (0xFF << (dev ? 0 : 8));
+	val32 |= (t2 << (dev ? 8 : 0));
+	hwif_write32(base, val32, BK3710_REGSTB);
+
+	val32 = hwif_read32(base, BK3710_REGRCVR) & (0xFF << (dev ? 0 : 8));
+	val32 |= (t2i << (dev ? 8 : 0));
+	hwif_write32(base, val32, BK3710_REGRCVR);
+}
+
+static void palm_bk3710_set_dma_mode(ide_drive_t *drive, u8 xferspeed)
+{
+	int is_slave = drive->dn & 1;
+	u32 base = drive->hwif->dma_master;
+
+	if (xferspeed >= XFER_UDMA_0) {
+		palm_bk3710_setudmamode(base, is_slave,
+					xferspeed - XFER_UDMA_0);
+	} else {
+		palm_bk3710_setdmamode(base, is_slave, drive->id->eide_dma_min,
+				       xferspeed);
+	}
+}
+
+static void palm_bk3710_set_pio_mode(ide_drive_t *drive, u8 pio)
+{
+	unsigned int cycle_time;
+	int is_slave = drive->dn & 1;
+	ide_drive_t *mate;
+	u32 base = drive->hwif->dma_master;
+
+	/*
+	 * Obtain the drive PIO data for tuning the Palm Chip registers
+	 */
+	cycle_time = ide_pio_cycle_time(drive, pio);
+	mate = ide_get_paired_drive(drive);
+	palm_bk3710_setpiomode(base, mate, is_slave, cycle_time, pio);
+}
+
+static void __devinit palm_bk3710_chipinit(u32 base)
+{
+	/*
+	 * enable the reset_en of ATA controller so that when ata signals
+	 * are brought out, by writing into device config. at that
+	 * time por_n signal should not be 'Z' and have a stable value.
+	 */
+	hwif_write32(base, 0x0300, BK3710_MISCCTL);
+
+	/* wait for some time and deassert the reset of ATA Device. */
+	mdelay(100);
+
+	/* Deassert the Reset */
+	hwif_write32(base, 0x0200, BK3710_MISCCTL);
+
+	/*
+	 * Program the IDETIMP Register Value based on the following assumptions
+	 *
+	 * (ATA_IDETIMP_IDEEN		, ENABLE ) |
+	 * (ATA_IDETIMP_SLVTIMEN	, DISABLE) |
+	 * (ATA_IDETIMP_RDYSMPL		, 70NS)    |
+	 * (ATA_IDETIMP_RDYRCVRY	, 50NS)    |
+	 * (ATA_IDETIMP_DMAFTIM1	, PIOCOMP) |
+	 * (ATA_IDETIMP_PREPOST1	, DISABLE) |
+	 * (ATA_IDETIMP_RDYSEN1		, DISABLE) |
+	 * (ATA_IDETIMP_PIOFTIM1	, DISABLE) |
+	 * (ATA_IDETIMP_DMAFTIM0	, PIOCOMP) |
+	 * (ATA_IDETIMP_PREPOST0	, DISABLE) |
+	 * (ATA_IDETIMP_RDYSEN0		, DISABLE) |
+	 * (ATA_IDETIMP_PIOFTIM0	, DISABLE)
+	 */
+	hwif_write16(base, 0xB388, BK3710_IDETIMP);
+
+	/*
+	 * Configure  SIDETIM  Register
+	 * (ATA_SIDETIM_RDYSMPS1	,120NS ) |
+	 * (ATA_SIDETIM_RDYRCYS1	,120NS )
+	 */
+	hwif_write8(base, 0, BK3710_SIDETIM);
+
+	/*
+	 * UDMACTL Ultra-ATA DMA Control
+	 * (ATA_UDMACTL_UDMAP1	, 0 ) |
+	 * (ATA_UDMACTL_UDMAP0	, 0 )
+	 *
+	 */
+	hwif_write16(base, 0, BK3710_UDMACTL);
+
+	/*
+	 * MISCCTL Miscellaneous Conrol Register
+	 * (ATA_MISCCTL_RSTMODEP	, 1) |
+	 * (ATA_MISCCTL_RESETP		, 0) |
+	 * (ATA_MISCCTL_TIMORIDE	, 1)
+	 */
+	hwif_write32(base, 0x201, BK3710_MISCCTL);
+
+	/*
+	 * IORDYTMP IORDY Timer for Primary Register
+	 * (ATA_IORDYTMP_IORDYTMP     , 0xffff  )
+	 */
+	hwif_write32(base, 0xFFFF, BK3710_IORDYTMP);
+
+	/*
+	 * Configure BMISP Register
+	 * (ATA_BMISP_DMAEN1	, DISABLE )	|
+	 * (ATA_BMISP_DMAEN0	, DISABLE )	|
+	 * (ATA_BMISP_IORDYINT	, CLEAR)	|
+	 * (ATA_BMISP_INTRSTAT	, CLEAR)	|
+	 * (ATA_BMISP_DMAERROR	, CLEAR)
+	 */
+	hwif_write16(base, 0, BK3710_BMISP);
+
+	palm_bk3710_setpiomode(base, NULL, 0, 600, 0);
+	palm_bk3710_setpiomode(base, NULL, 1, 600, 0);
+}
+static int __devinit palm_bk3710_probe(struct platform_device *pdev)
+{
+	hw_regs_t ide_ctlr_info;
+	int index = 0;
+	int pribase;
+	struct clk *clkp;
+	struct resource *mem, *irq;
+	ide_hwif_t *hwif;
+	u32 base;
+
+	clkp = clk_get(NULL, "IDECLK");
+	if (IS_ERR(clkp))
+		return -ENODEV;
+
+	ideclkp = clkp;
+	clk_enable(ideclkp);
+	ide_palm_clk = clk_get_rate(ideclkp)/100000;
+	ide_palm_clk = (10000/ide_palm_clk) + 1;
+	/* Register the IDE interface with Linux ATA Interface */
+	memset(&ide_ctlr_info, 0, sizeof(ide_ctlr_info));
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (mem == NULL) {
+		printk(KERN_ERR "failed to get memory region resource\n");
+		return -ENODEV;
+	}
+	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (irq == NULL) {
+		printk(KERN_ERR "failed to get IRQ resource\n");
+		return -ENODEV;
+	}
+
+	base = mem->start;
+
+	/* Configure the Palm Chip controller */
+	palm_bk3710_chipinit(base);
+
+	pribase = mem->start + IDE_PALM_ATA_PRI_REG_OFFSET;
+	for (index = 0; index < IDE_NR_PORTS - 2; index++)
+		ide_ctlr_info.io_ports[index] = pribase + index;
+	ide_ctlr_info.io_ports[IDE_CONTROL_OFFSET] = mem->start +
+			IDE_PALM_ATA_PRI_CTL_OFFSET;
+	ide_ctlr_info.irq = irq->start;
+	ide_ctlr_info.chipset = ide_palm3710;
+
+	if (ide_register_hw(&ide_ctlr_info, NULL, 0, &hwif) < 0) {
+		printk(KERN_WARNING "Palm Chip BK3710 IDE Register Fail\n");
+		return -ENODEV;
+	}
+
+	hwif->set_pio_mode = &palm_bk3710_set_pio_mode;
+	hwif->set_dma_mode = &palm_bk3710_set_dma_mode;
+	hwif->mmio = 1;
+	default_hwif_mmiops(hwif);
+	hwif->ultra_mask = 0x1f;	/* Ultra DMA Mode 4 Max
+						(input clk 99MHz) */
+	hwif->mwdma_mask = 0x7;
+	hwif->drives[0].autotune = 1;
+	hwif->drives[1].autotune = 1;
+
+	ide_setup_dma(hwif, mem->start, 8);
+
+	return 0;
+}
+
+static struct platform_driver platform_bk_driver = {
+	.driver = {
+		.name = "palm_bk3710",
+	},
+	.probe = palm_bk3710_probe,
+	.remove = NULL,
+};
+
+static int __init palm_bk3710_init(void)
+{
+	return platform_driver_register(&platform_bk_driver);
+}
+
+module_init(palm_bk3710_init);
+MODULE_LICENSE("GPL");
+
Index: 2.6.24.ide/drivers/ide/ide-proc.c
===================================================================
--- 2.6.24.ide.orig/drivers/ide/ide-proc.c
+++ 2.6.24.ide/drivers/ide/ide-proc.c
@@ -67,6 +67,7 @@ static int proc_ide_read_imodel
 		case ide_4drives:	name = "4drives";	break;
 		case ide_pmac:		name = "mac-io";	break;
 		case ide_au1xxx:	name = "au1xxx";	break;
+		case ide_palm3710:      name = "palm3710";      break;
 		case ide_etrax100:	name = "etrax100";	break;
 		case ide_acorn:		name = "acorn";		break;
 		default:		name = "(unknown)";	break;
Index: 2.6.24.ide/include/linux/ide.h
===================================================================
--- 2.6.24.ide.orig/include/linux/ide.h
+++ 2.6.24.ide/include/linux/ide.h
@@ -202,7 +202,7 @@ enum {		ide_unknown,	ide_generic,	ide_pc
 		ide_rz1000,	ide_trm290,
 		ide_cmd646,	ide_cy82c693,	ide_4drives,
 		ide_pmac,	ide_etrax100,	ide_acorn,
-		ide_au1xxx, ide_forced
+		ide_au1xxx,	ide_palm3710,	ide_forced
 };
 
 typedef u8 hwif_chipset_t;

Re: [PATCH] Palmchip BK3710 IDE driver

[Sergei Shtylyov]

Hello.

Anton Salnikov wrote:

> This is Palmchip BK3710 IDE controller support for kernel version 2.6.24.
> The IDE controller logic supports PIO, multiword DMA and ultra-DMA modes.
> Supports interface to compact Flash (CF) configured in True-IDE mode.

> Signed-off-by: Anton Salnikov <asalnikov@ru.mvista.com>

Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

> Index: 2.6.24.ide/drivers/ide/arm/palm_bk3710.c
> ===================================================================
> --- /dev/null
> +++ 2.6.24.ide/drivers/ide/arm/palm_bk3710.c

> +static void palm_bk3710_setudmamode(u32 base, unsigned int dev,
> +				    unsigned int mode)
> +{
> +	u8 tenv, trp, t0;
> +	u32 val32;
> +	u16 val16;
> +
> +	/* DMA Data Setup */
> +	t0 = (palm_bk3710_udmatimings[mode].cycletime + ide_palm_clk - 1)
> +			/ ide_palm_clk - 1;
> +	tenv = (20 + ide_palm_clk - 1) / ide_palm_clk - 1;
> +	trp = (palm_bk3710_udmatimings[mode].rptime + ide_palm_clk - 1)
> +			/ ide_palm_clk - 1;
> +
> +	/* udmatim Register */
> +	val16 = hwif_read16(base, BK3710_UDMATIM) & (dev ? 0xFF0F : 0xFFF0);
> +	val16 |= (mode << (dev ? 4 : 0));

    Needless parens all over the code like this...
    Also, you could have made it either 2 (getting rid of |= operator) or 4 
lines (read/write, &=, and |=).

MBR, Sergei

Re: [PATCH] Palmchip BK3710 IDE driver

[Alan Cox]

>     Needless parens all over the code like this...
>     Also, you could have made it either 2 (getting rid of |= operator) or 4 
> lines (read/write, &=, and |=).

	mode |= dev << 4;

Re: [PATCH] Palmchip BK3710 IDE driver

[Bartlomiej Zolnierkiewicz]

Hi,

On Friday 25 January 2008, Anton Salnikov wrote:
> This is Palmchip BK3710 IDE controller support for kernel version 2.6.24.
> The IDE controller logic supports PIO, multiword DMA and ultra-DMA modes.
> Supports interface to compact Flash (CF) configured in True-IDE mode.

Unfortunately some changes merged after 2.6.24 broke the build:

drivers/ide/arm/palm_bk3710.c: In function ‘palm_bk3710_set_dma_mode’:
drivers/ide/arm/palm_bk3710.c:243: error: ‘struct hwif_s’ has no member named ‘dma_master’
drivers/ide/arm/palm_bk3710.c: In function ‘palm_bk3710_set_pio_mode’:
drivers/ide/arm/palm_bk3710.c:259: error: ‘struct hwif_s’ has no member named ‘dma_master’

[ ->dma_master is gone, ->dma_base should be used instead ]

drivers/ide/arm/palm_bk3710.c: In function ‘palm_bk3710_probe’:
drivers/ide/arm/palm_bk3710.c:389: error: too many arguments to function ‘ide_register_hw’

[ 'initializing' argument is gone, just removing it is OK ]

drivers/ide/arm/palm_bk3710.c:404: error: too many arguments to function ‘ide_setup_dma’

[ 'num_ports' argument is gone, just removing it is OK ]


Please re-sync the patch with the current Linus' git tree.


There are also few less critical issues left...

> Signed-off-by: Anton Salnikov <asalnikov@ru.mvista.com>
> ---
> 
>  drivers/ide/Kconfig           |    8 
>  drivers/ide/arm/Makefile      |    1 
>  drivers/ide/arm/palm_bk3710.c |  424 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/ide/ide-proc.c        |    1 
>  include/linux/ide.h           |    2 
>  5 files changed, 435 insertions(+), 1 deletion(-)
> 
> Index: 2.6.24.ide/drivers/ide/Kconfig
> ===================================================================
> --- 2.6.24.ide.orig/drivers/ide/Kconfig
> +++ 2.6.24.ide/drivers/ide/Kconfig
> @@ -1008,6 +1008,14 @@ config BLK_DEV_Q40IDE
>  	  normally be on; disable it only if you are running a custom hard
>  	  drive subsystem through an expansion card.
>  
> +config BLK_DEV_PALMCHIP_BK3710
> +	bool "Palmchip bk3710 IDE controller support"

'tristate'

> +	depends on ARCH_DAVINCI
> +	select BLK_DEV_IDEDMA_PCI
> +	help
> +	  Say Y here if you want to support the onchip IDE controller on the
> +	  TI DaVinci SoC
> +
>  config BLK_DEV_MPC8xx_IDE
>  	bool "MPC8xx IDE support"
>  	depends on 8xx && (LWMON || IVMS8 || IVML24 || TQM8xxL) && IDE=y && 
> BLK_DEV_IDE=y && !PPC_MERGE
> Index: 2.6.24.ide/drivers/ide/arm/Makefile
> ===================================================================
> --- 2.6.24.ide.orig/drivers/ide/arm/Makefile
> +++ 2.6.24.ide/drivers/ide/arm/Makefile

[...]

> +static inline u8 hwif_read8(u32 base, u32 reg)
> +{
> +	return readb(base + reg);
> +}
> +
> +static inline void hwif_write8(u32 base, u8 val, u32 reg)
> +{
> +	writeb(val, base + reg);
> +}
> +
> +static inline u16 hwif_read16(u32 base, u32 reg)
> +{
> +	return readw(base + reg);
> +}
> +
> +static inline void hwif_write16(u32 base, u16 val, u32 reg)
> +{
> +	writew(val, base + reg);
> +}
> +
> +static inline u32 hwif_read32(u32 base, u32 reg)
> +{
> +	return readl(base + reg);
> +}
> +
> +static inline void hwif_write32(u32 base, u32 val, u32 reg)
> +{
> +	writel(val, base + reg);
> +}

drivers/ide/arm/palm_bk3710.c: In function ‘hwif_read8’:
drivers/ide/arm/palm_bk3710.c:93: warning: passing argument 1 of ‘readb’ makes pointer from integer without a cast
drivers/ide/arm/palm_bk3710.c: In function ‘hwif_write8’:
drivers/ide/arm/palm_bk3710.c:98: warning: passing argument 2 of ‘writeb’ makes pointer from integer without a cast
drivers/ide/arm/palm_bk3710.c: In function ‘hwif_read16’:
drivers/ide/arm/palm_bk3710.c:103: warning: passing argument 1 of ‘readw’ makes pointer from integer without a cast
drivers/ide/arm/palm_bk3710.c: In function ‘hwif_write16’:
drivers/ide/arm/palm_bk3710.c:108: warning: passing argument 2 of ‘writew’ makes pointer from integer without a cast
drivers/ide/arm/palm_bk3710.c: In function ‘hwif_read32’:
drivers/ide/arm/palm_bk3710.c:113: warning: passing argument 1 of ‘readl’ makes pointer from integer without a cast
drivers/ide/arm/palm_bk3710.c: In function ‘hwif_write32’:
drivers/ide/arm/palm_bk3710.c:118: warning: passing argument 2 of ‘writel’ makes pointer from integer without a cast

'base' should be of 'void __iomem *' type for read*/write* ops

Besides: because the driver supports multiple controllers now these wrappers
doesn't provide any value and just obfuscate the code (as noticed by Alan).

Please remove them.

> +static void palm_bk3710_setudmamode(u32 base, unsigned int dev,

'base' should be of 'void __iomem *' type for read*/*write ops

[ casting from u32 can be done in the caller ]

> +				    unsigned int mode)
> +{
> +	u8 tenv, trp, t0;
> +	u32 val32;
> +	u16 val16;
> +
> +	/* DMA Data Setup */
> +	t0 = (palm_bk3710_udmatimings[mode].cycletime + ide_palm_clk - 1)
> +			/ ide_palm_clk - 1;
> +	tenv = (20 + ide_palm_clk - 1) / ide_palm_clk - 1;
> +	trp = (palm_bk3710_udmatimings[mode].rptime + ide_palm_clk - 1)
> +			/ ide_palm_clk - 1;
> +
> +	/* udmatim Register */
> +	val16 = hwif_read16(base, BK3710_UDMATIM) & (dev ? 0xFF0F : 0xFFF0);
> +	val16 |= (mode << (dev ? 4 : 0));
> +	hwif_write16(base, val16, BK3710_UDMATIM);
> +
> +	/* udmastb Ultra DMA Access Strobe Width */
> +	val32 = hwif_read32(base, BK3710_UDMASTB) & (0xFF << (dev ? 0 : 8));
> +	val32 |= (t0 << (dev ? 8 : 0));
> +	hwif_write32(base, val32, BK3710_UDMASTB);
> +
> +	/* udmatrp Ultra DMA Ready to Pause Time */
> +	val32 = hwif_read32(base, BK3710_UDMATRP) & (0xFF << (dev ? 0 : 8));
> +	val32 |= (trp << (dev ? 8 : 0));
> +	hwif_write32(base, val32, BK3710_UDMATRP);
> +
> +	/* udmaenv Ultra DMA envelop Time */
> +	val32 = hwif_read32(base, BK3710_UDMAENV) & (0xFF << (dev ? 0 : 8));
> +	val32 |= (tenv << (dev ? 8 : 0));
> +	hwif_write32(base, val32, BK3710_UDMAENV);
> +
> +	/* Enable UDMA for Device */
> +	val16 = hwif_read16(base, BK3710_UDMACTL) | (1 << dev);
> +	hwif_write16(base, val16, BK3710_UDMACTL);
> +}
> +
> +static void palm_bk3710_setdmamode(u32 base, unsigned int dev,

ditto

> +				   unsigned short min_cycle,
> +				   unsigned int mode)
> +{
> +	u8 td, tkw, t0;
> +	u32 val32;
> +	u16 val16;
> +	struct ide_timing *t;
> +	int cycletime;
> +
> +	t = ide_timing_find_mode(mode);
> +	cycletime = max_t(int, t->cycle, min_cycle);
> +
> +	/* DMA Data Setup */
> +	t0 = (cycletime + ide_palm_clk - 1) / ide_palm_clk;
> +	td = (t->active + ide_palm_clk - 1) / ide_palm_clk;
> +	tkw = t0 - td - 1;
> +	td -= 1;
> +
> +	val32 = hwif_read32(base, BK3710_DMASTB) & (0xFF << (dev ? 0 : 8));
> +	val32 |= (td << (dev ? 8 : 0));
> +	hwif_write32(base, val32, BK3710_DMASTB);
> +
> +	val32 = hwif_read32(base, BK3710_DMARCVR) & (0xFF << (dev ? 0 : 8));
> +	val32 |= (tkw << (dev ? 8 : 0));
> +	hwif_write32(base, val32, BK3710_DMARCVR);
> +
> +	/* Disable UDMA for Device */
> +	val16 = hwif_read16(base, BK3710_UDMACTL) & ~(1 << dev);
> +	hwif_write16(base, val16, BK3710_UDMACTL);
> +}
> +
> +static void palm_bk3710_setpiomode(u32 base, ide_drive_t *mate,

ditto

> +				   unsigned int dev, unsigned int cycletime,
> +				   unsigned int mode)
> +{
> +	u8 t2, t2i, t0;
> +	u32 val32;
> +	struct ide_timing *t;
> +
> +	/* PIO Data Setup */
> +	t0 = (cycletime + ide_palm_clk - 1) / ide_palm_clk;
> +	t2 = (ide_timing_find_mode(XFER_PIO_0 + mode)->active +
> +	      ide_palm_clk - 1)	/ ide_palm_clk;
> +
> +	t2i = t0 - t2 - 1;
> +	t2 -= 1;
> +
> +	val32 = hwif_read32(base, BK3710_DATSTB) & (0xFF << (dev ? 0 : 8));
> +	val32 |= (t2 << (dev ? 8 : 0));
> +	hwif_write32(base, val32, BK3710_DATSTB);
> +
> +	val32 = hwif_read32(base, BK3710_DATRCVR) & (0xFF << (dev ? 0 : 8));
> +	val32 |= (t2i << (dev ? 8 : 0));
> +	hwif_write32(base, val32, BK3710_DATRCVR);
> +
> +	if (mate && mate->present) {
> +		u8 mode2 = ide_get_best_pio_mode(mate, 255, 4);
> +
> +		if (mode2 < mode)
> +			mode = mode2;
> +	}
> +
> +	/* TASKFILE Setup */
> +	t = ide_timing_find_mode(XFER_PIO_0 + mode);
> +	t0 = (t->cyc8b + ide_palm_clk - 1) / ide_palm_clk;
> +	t2 = (t->act8b + ide_palm_clk - 1) / ide_palm_clk;
> +
> +	t2i = t0 - t2 - 1;
> +	t2 -= 1;
> +
> +	val32 = hwif_read32(base, BK3710_REGSTB) & (0xFF << (dev ? 0 : 8));
> +	val32 |= (t2 << (dev ? 8 : 0));
> +	hwif_write32(base, val32, BK3710_REGSTB);
> +
> +	val32 = hwif_read32(base, BK3710_REGRCVR) & (0xFF << (dev ? 0 : 8));
> +	val32 |= (t2i << (dev ? 8 : 0));
> +	hwif_write32(base, val32, BK3710_REGRCVR);
> +}
> +
> +static void palm_bk3710_set_dma_mode(ide_drive_t *drive, u8 xferspeed)
> +{
> +	int is_slave = drive->dn & 1;
> +	u32 base = drive->hwif->dma_master;
> +
> +	if (xferspeed >= XFER_UDMA_0) {
> +		palm_bk3710_setudmamode(base, is_slave,
> +					xferspeed - XFER_UDMA_0);
> +	} else {
> +		palm_bk3710_setdmamode(base, is_slave, drive->id->eide_dma_min,
> +				       xferspeed);
> +	}
> +}
> +
> +static void palm_bk3710_set_pio_mode(ide_drive_t *drive, u8 pio)
> +{
> +	unsigned int cycle_time;
> +	int is_slave = drive->dn & 1;
> +	ide_drive_t *mate;
> +	u32 base = drive->hwif->dma_master;
> +
> +	/*
> +	 * Obtain the drive PIO data for tuning the Palm Chip registers
> +	 */
> +	cycle_time = ide_pio_cycle_time(drive, pio);
> +	mate = ide_get_paired_drive(drive);
> +	palm_bk3710_setpiomode(base, mate, is_slave, cycle_time, pio);
> +}
> +
> +static void __devinit palm_bk3710_chipinit(u32 base)

'u32' -> 'void __iomem *'

> +{
> +	/*
> +	 * enable the reset_en of ATA controller so that when ata signals
> +	 * are brought out, by writing into device config. at that
> +	 * time por_n signal should not be 'Z' and have a stable value.
> +	 */
> +	hwif_write32(base, 0x0300, BK3710_MISCCTL);
> +
> +	/* wait for some time and deassert the reset of ATA Device. */
> +	mdelay(100);
> +
> +	/* Deassert the Reset */
> +	hwif_write32(base, 0x0200, BK3710_MISCCTL);
> +
> +	/*
> +	 * Program the IDETIMP Register Value based on the following assumptions
> +	 *
> +	 * (ATA_IDETIMP_IDEEN		, ENABLE ) |
> +	 * (ATA_IDETIMP_SLVTIMEN	, DISABLE) |
> +	 * (ATA_IDETIMP_RDYSMPL		, 70NS)    |
> +	 * (ATA_IDETIMP_RDYRCVRY	, 50NS)    |
> +	 * (ATA_IDETIMP_DMAFTIM1	, PIOCOMP) |
> +	 * (ATA_IDETIMP_PREPOST1	, DISABLE) |
> +	 * (ATA_IDETIMP_RDYSEN1		, DISABLE) |
> +	 * (ATA_IDETIMP_PIOFTIM1	, DISABLE) |
> +	 * (ATA_IDETIMP_DMAFTIM0	, PIOCOMP) |
> +	 * (ATA_IDETIMP_PREPOST0	, DISABLE) |
> +	 * (ATA_IDETIMP_RDYSEN0		, DISABLE) |
> +	 * (ATA_IDETIMP_PIOFTIM0	, DISABLE)
> +	 */
> +	hwif_write16(base, 0xB388, BK3710_IDETIMP);
> +
> +	/*
> +	 * Configure  SIDETIM  Register
> +	 * (ATA_SIDETIM_RDYSMPS1	,120NS ) |
> +	 * (ATA_SIDETIM_RDYRCYS1	,120NS )
> +	 */
> +	hwif_write8(base, 0, BK3710_SIDETIM);
> +
> +	/*
> +	 * UDMACTL Ultra-ATA DMA Control
> +	 * (ATA_UDMACTL_UDMAP1	, 0 ) |
> +	 * (ATA_UDMACTL_UDMAP0	, 0 )
> +	 *
> +	 */
> +	hwif_write16(base, 0, BK3710_UDMACTL);
> +
> +	/*
> +	 * MISCCTL Miscellaneous Conrol Register
> +	 * (ATA_MISCCTL_RSTMODEP	, 1) |
> +	 * (ATA_MISCCTL_RESETP		, 0) |
> +	 * (ATA_MISCCTL_TIMORIDE	, 1)
> +	 */
> +	hwif_write32(base, 0x201, BK3710_MISCCTL);
> +
> +	/*
> +	 * IORDYTMP IORDY Timer for Primary Register
> +	 * (ATA_IORDYTMP_IORDYTMP     , 0xffff  )
> +	 */
> +	hwif_write32(base, 0xFFFF, BK3710_IORDYTMP);
> +
> +	/*
> +	 * Configure BMISP Register
> +	 * (ATA_BMISP_DMAEN1	, DISABLE )	|
> +	 * (ATA_BMISP_DMAEN0	, DISABLE )	|
> +	 * (ATA_BMISP_IORDYINT	, CLEAR)	|
> +	 * (ATA_BMISP_INTRSTAT	, CLEAR)	|
> +	 * (ATA_BMISP_DMAERROR	, CLEAR)
> +	 */
> +	hwif_write16(base, 0, BK3710_BMISP);
> +
> +	palm_bk3710_setpiomode(base, NULL, 0, 600, 0);
> +	palm_bk3710_setpiomode(base, NULL, 1, 600, 0);
> +}
> +static int __devinit palm_bk3710_probe(struct platform_device *pdev)
> +{
> +	hw_regs_t ide_ctlr_info;
> +	int index = 0;
> +	int pribase;
> +	struct clk *clkp;
> +	struct resource *mem, *irq;
> +	ide_hwif_t *hwif;
> +	u32 base;

ditto

> +	clkp = clk_get(NULL, "IDECLK");
> +	if (IS_ERR(clkp))
> +		return -ENODEV;
> +
> +	ideclkp = clkp;
> +	clk_enable(ideclkp);
> +	ide_palm_clk = clk_get_rate(ideclkp)/100000;
> +	ide_palm_clk = (10000/ide_palm_clk) + 1;
> +	/* Register the IDE interface with Linux ATA Interface */
> +	memset(&ide_ctlr_info, 0, sizeof(ide_ctlr_info));
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (mem == NULL) {
> +		printk(KERN_ERR "failed to get memory region resource\n");
> +		return -ENODEV;
> +	}
> +	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +	if (irq == NULL) {
> +		printk(KERN_ERR "failed to get IRQ resource\n");
> +		return -ENODEV;
> +	}
> +
> +	base = mem->start;
> +
> +	/* Configure the Palm Chip controller */
> +	palm_bk3710_chipinit(base);
> +
> +	pribase = mem->start + IDE_PALM_ATA_PRI_REG_OFFSET;
> +	for (index = 0; index < IDE_NR_PORTS - 2; index++)
> +		ide_ctlr_info.io_ports[index] = pribase + index;
> +	ide_ctlr_info.io_ports[IDE_CONTROL_OFFSET] = mem->start +
> +			IDE_PALM_ATA_PRI_CTL_OFFSET;
> +	ide_ctlr_info.irq = irq->start;
> +	ide_ctlr_info.chipset = ide_palm3710;
> +
> +	if (ide_register_hw(&ide_ctlr_info, NULL, 0, &hwif) < 0) {
> +		printk(KERN_WARNING "Palm Chip BK3710 IDE Register Fail\n");
> +		return -ENODEV;
> +	}
> +
> +	hwif->set_pio_mode = &palm_bk3710_set_pio_mode;
> +	hwif->set_dma_mode = &palm_bk3710_set_dma_mode;
> +	hwif->mmio = 1;
> +	default_hwif_mmiops(hwif);
> +	hwif->ultra_mask = 0x1f;	/* Ultra DMA Mode 4 Max
> +						(input clk 99MHz) */

I've just noticed that there is no cable detection for UDMA modes > UDMA2?

> +	hwif->mwdma_mask = 0x7;
> +	hwif->drives[0].autotune = 1;
> +	hwif->drives[1].autotune = 1;
> +
> +	ide_setup_dma(hwif, mem->start, 8);
> +
> +	return 0;
> +}

[...]

Otherwise the driver looks fine and will be merged once the above issues
get addressed so please recast+resubmit it.

Thanks,
Bart

[PATCH] Palmchip BK3710 IDE driver

[Anton Salnikov]

I've tested the driver on 2.6.24.

I've made the changes you asked for. But when I tested them on arm/mach-davinci 
architecture on the current Linus' git there was link error undefined reference 
to symbol ide_hwif_setup_dma().
However on x86_64 architecture it compiles well without any warnings with 
respect to link error of undefined clk_get() clk_enable() clk_get_rate(), which 
are not present in this architecture.


> I've just noticed that there is no cable detection for UDMA modes > UDMA2?
For the present controller in documentation: Cable Select (CSEL) - unsupported 
IDE controller signal. This signal is not necessary because an 80-conductor 
cable must be used with the DM644x.

Signed-off-by: Anton Salnikov <asalnikov@ru.mvista.com>
---

 drivers/ide/Kconfig           |    9 
 drivers/ide/arm/Makefile      |    1 
 drivers/ide/arm/palm_bk3710.c |  420 ++++++++++++++++++++++++++++++++++++++++++
 drivers/ide/ide-proc.c        |    1 
 include/linux/ide.h           |    2 
 5 files changed, 432 insertions(+), 1 deletion(-)

Index: 2.6.25.ide/drivers/ide/Kconfig
===================================================================
--- 2.6.25.ide.orig/drivers/ide/Kconfig
+++ 2.6.25.ide/drivers/ide/Kconfig
@@ -1009,6 +1009,15 @@ config BLK_DEV_Q40IDE
 	  normally be on; disable it only if you are running a custom hard
 	  drive subsystem through an expansion card.
 
+config BLK_DEV_PALMCHIP_BK3710
+	tristate "Palmchip bk3710 IDE controller support"
+	depends on ARCH_DAVINCI
+	select BLK_DEV_IDEDMA_PCI
+	help
+	  Say Y here if you want to support the onchip IDE controller on the
+	  TI DaVinci SoC
+
+
 config BLK_DEV_MPC8xx_IDE
 	tristate "MPC8xx IDE support"
 	depends on 8xx && (LWMON || IVMS8 || IVML24 || TQM8xxL) && IDE=y && 
BLK_DEV_IDE=y && !PPC_MERGE
Index: 2.6.25.ide/drivers/ide/arm/Makefile
===================================================================
--- 2.6.25.ide.orig/drivers/ide/arm/Makefile
+++ 2.6.25.ide/drivers/ide/arm/Makefile
@@ -2,6 +2,7 @@
 obj-$(CONFIG_BLK_DEV_IDE_ICSIDE)	+= icside.o
 obj-$(CONFIG_BLK_DEV_IDE_RAPIDE)	+= rapide.o
 obj-$(CONFIG_BLK_DEV_IDE_BAST)		+= bast-ide.o
+obj-$(CONFIG_BLK_DEV_PALMCHIP_BK3710)	+= palm_bk3710.o
 
 ifeq ($(CONFIG_IDE_ARM), m)
 	obj-m += ide_arm.o
Index: 2.6.25.ide/drivers/ide/arm/palm_bk3710.c
===================================================================
--- /dev/null
+++ 2.6.25.ide/drivers/ide/arm/palm_bk3710.c
@@ -0,0 +1,420 @@
+/*
+ * Palmchip bk3710 IDE controller
+ *
+ * Copyright (C) 2006 Texas Instruments.
+ * Copyright (C) 2007 MontaVista Software, Inc., <source@mvista.com>
+ *
+ * ----------------------------------------------------------------------------
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ * ----------------------------------------------------------------------------
+ *
+ */
+
+#include <linux/types.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/ioport.h>
+#include <linux/hdreg.h>
+#include <linux/ide.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/clk.h>
+#include <linux/platform_device.h>
+
+/* Offset of the primary interface registers */
+#define IDE_PALM_ATA_PRI_REG_OFFSET 0x1F0
+
+/* Primary Control Offset */
+#define IDE_PALM_ATA_PRI_CTL_OFFSET 0x3F6
+
+/*
+ * PalmChip 3710 IDE Controller UDMA timing structure Definition
+ */
+struct palm_bk3710_udmatiming {
+	unsigned int rptime;	/* Ready to pause time  */
+	unsigned int cycletime;	/* Cycle Time           */
+};
+
+#define BK3710_BMICP		0x00
+#define BK3710_BMISP		0x02
+#define BK3710_BMIDTP		0x04
+#define BK3710_BMICS		0x08
+#define BK3710_BMISS		0x0A
+#define BK3710_BMIDTS		0x0C
+#define BK3710_IDETIMP		0x40
+#define BK3710_IDETIMS		0x42
+#define BK3710_SIDETIM		0x44
+#define BK3710_SLEWCTL		0x45
+#define BK3710_IDESTATUS	0x47
+#define BK3710_UDMACTL		0x48
+#define BK3710_UDMATIM		0x4A
+#define BK3710_MISCCTL		0x50
+#define BK3710_REGSTB		0x54
+#define BK3710_REGRCVR		0x58
+#define BK3710_DATSTB		0x5C
+#define BK3710_DATRCVR		0x60
+#define BK3710_DMASTB		0x64
+#define BK3710_DMARCVR		0x68
+#define BK3710_UDMASTB		0x6C
+#define BK3710_UDMATRP		0x70
+#define BK3710_UDMAENV		0x74
+#define BK3710_IORDYTMP		0x78
+#define BK3710_IORDYTMS		0x7C
+
+#include "../ide-timing.h"
+
+static long ide_palm_clk;
+
+static const struct palm_bk3710_udmatiming palm_bk3710_udmatimings[6] = {
+	{160, 240},		/* UDMA Mode 0 */
+	{125, 160},		/* UDMA Mode 1 */
+	{100, 120},		/* UDMA Mode 2 */
+	{100, 90},		/* UDMA Mode 3 */
+	{85,  60},		/* UDMA Mode 4 */
+};
+
+static struct clk *ideclkp;
+
+/*static inline u8 hwif_read8(u32 base, u32 reg)
+{
+	return readb(base + reg);
+}
+
+static inline u16 hwif_read16(u32 base, u32 reg)
+{
+	return readw(base + reg);
+}
+
+static inline void hwif_write16(u32 base, u16 val, u32 reg)
+{
+	writew(val, base + reg);
+}
+
+static inline u32 hwif_read32(u32 base, u32 reg)
+{
+	return readl(base + reg);
+}
+
+static inline void hwif_write32(u32 base, u32 val, u32 reg)
+{
+	writel(val, base + reg);
+}*/
+
+static void palm_bk3710_setudmamode(void __iomem *base, unsigned int dev,
+				    unsigned int mode)
+{
+	u8 tenv, trp, t0;
+	u32 val32;
+	u16 val16;
+
+	/* DMA Data Setup */
+	t0 = (palm_bk3710_udmatimings[mode].cycletime + ide_palm_clk - 1)
+			/ ide_palm_clk - 1;
+	tenv = (20 + ide_palm_clk - 1) / ide_palm_clk - 1;
+	trp = (palm_bk3710_udmatimings[mode].rptime + ide_palm_clk - 1)
+			/ ide_palm_clk - 1;
+
+	/* udmatim Register */
+	val16 = readw(base + BK3710_UDMATIM) & (dev ? 0xFF0F : 0xFFF0);
+	val16 |= (mode << (dev ? 4 : 0));
+	writew(val16, base + BK3710_UDMATIM);
+
+	/* udmastb Ultra DMA Access Strobe Width */
+	val32 = readl(base + BK3710_UDMASTB) & (0xFF << (dev ? 0 : 8));
+	val32 |= (t0 << (dev ? 8 : 0));
+	writel(val32, base + BK3710_UDMASTB);
+
+	/* udmatrp Ultra DMA Ready to Pause Time */
+	val32 = readl(base + BK3710_UDMATRP) & (0xFF << (dev ? 0 : 8));
+	val32 |= (trp << (dev ? 8 : 0));
+	writel(val32, base + BK3710_UDMATRP);
+
+	/* udmaenv Ultra DMA envelop Time */
+	val32 = readl(base + BK3710_UDMAENV) & (0xFF << (dev ? 0 : 8));
+	val32 |= (tenv << (dev ? 8 : 0));
+	writel(val32, base + BK3710_UDMAENV);
+
+	/* Enable UDMA for Device */
+	val16 = readw(base + BK3710_UDMACTL) | (1 << dev);
+	writew(val16, base + BK3710_UDMACTL);
+}
+
+static void palm_bk3710_setdmamode(void __iomem *base, unsigned int dev,
+				   unsigned short min_cycle,
+				   unsigned int mode)
+{
+	u8 td, tkw, t0;
+	u32 val32;
+	u16 val16;
+	struct ide_timing *t;
+	int cycletime;
+
+	t = ide_timing_find_mode(mode);
+	cycletime = max_t(int, t->cycle, min_cycle);
+
+	/* DMA Data Setup */
+	t0 = (cycletime + ide_palm_clk - 1) / ide_palm_clk;
+	td = (t->active + ide_palm_clk - 1) / ide_palm_clk;
+	tkw = t0 - td - 1;
+	td -= 1;
+
+	val32 = readl(base + BK3710_DMASTB) & (0xFF << (dev ? 0 : 8));
+	val32 |= (td << (dev ? 8 : 0));
+	writel(val32, base + BK3710_DMASTB);
+
+	val32 = readl(base + BK3710_DMARCVR) & (0xFF << (dev ? 0 : 8));
+	val32 |= (tkw << (dev ? 8 : 0));
+	writel(val32, base + BK3710_DMARCVR);
+
+	/* Disable UDMA for Device */
+	val16 = readw(base + BK3710_UDMACTL) & ~(1 << dev);
+	writew(val16, base + BK3710_UDMACTL);
+}
+
+static void palm_bk3710_setpiomode(void __iomem *base, ide_drive_t *mate,
+				   unsigned int dev, unsigned int cycletime,
+				   unsigned int mode)
+{
+	u8 t2, t2i, t0;
+	u32 val32;
+	struct ide_timing *t;
+
+	/* PIO Data Setup */
+	t0 = (cycletime + ide_palm_clk - 1) / ide_palm_clk;
+	t2 = (ide_timing_find_mode(XFER_PIO_0 + mode)->active +
+	      ide_palm_clk - 1)	/ ide_palm_clk;
+
+	t2i = t0 - t2 - 1;
+	t2 -= 1;
+
+	val32 = readl(base + BK3710_DATSTB) & (0xFF << (dev ? 0 : 8));
+	val32 |= (t2 << (dev ? 8 : 0));
+	writel(val32, base + BK3710_DATSTB);
+
+	val32 = readl(base + BK3710_DATRCVR) & (0xFF << (dev ? 0 : 8));
+	val32 |= (t2i << (dev ? 8 : 0));
+	writel(val32, base + BK3710_DATRCVR);
+
+	if (mate && mate->present) {
+		u8 mode2 = ide_get_best_pio_mode(mate, 255, 4);
+
+		if (mode2 < mode)
+			mode = mode2;
+	}
+
+	/* TASKFILE Setup */
+	t = ide_timing_find_mode(XFER_PIO_0 + mode);
+	t0 = (t->cyc8b + ide_palm_clk - 1) / ide_palm_clk;
+	t2 = (t->act8b + ide_palm_clk - 1) / ide_palm_clk;
+
+	t2i = t0 - t2 - 1;
+	t2 -= 1;
+
+	val32 = readl(base + BK3710_REGSTB) & (0xFF << (dev ? 0 : 8));
+	val32 |= (t2 << (dev ? 8 : 0));
+	writel(val32, base + BK3710_REGSTB);
+
+	val32 = readl(base + BK3710_REGRCVR) & (0xFF << (dev ? 0 : 8));
+	val32 |= (t2i << (dev ? 8 : 0));
+	writel(val32, base + BK3710_REGRCVR);
+}
+
+static void palm_bk3710_set_dma_mode(ide_drive_t *drive, u8 xferspeed)
+{
+	int is_slave = drive->dn & 1;
+	void __iomem *base = (void *)drive->hwif->dma_base;
+
+	if (xferspeed >= XFER_UDMA_0) {
+		palm_bk3710_setudmamode(base, is_slave,
+					xferspeed - XFER_UDMA_0);
+	} else {
+		palm_bk3710_setdmamode(base, is_slave, drive->id->eide_dma_min,
+				       xferspeed);
+	}
+}
+
+static void palm_bk3710_set_pio_mode(ide_drive_t *drive, u8 pio)
+{
+	unsigned int cycle_time;
+	int is_slave = drive->dn & 1;
+	ide_drive_t *mate;
+	void __iomem *base = (void *)drive->hwif->dma_base;
+
+	/*
+	 * Obtain the drive PIO data for tuning the Palm Chip registers
+	 */
+	cycle_time = ide_pio_cycle_time(drive, pio);
+	mate = ide_get_paired_drive(drive);
+	palm_bk3710_setpiomode(base, mate, is_slave, cycle_time, pio);
+}
+
+static void __devinit palm_bk3710_chipinit(void __iomem *base)
+{
+	/*
+	 * enable the reset_en of ATA controller so that when ata signals
+	 * are brought out, by writing into device config. at that
+	 * time por_n signal should not be 'Z' and have a stable value.
+	 */
+	writel(0x0300, base + BK3710_MISCCTL);
+
+	/* wait for some time and deassert the reset of ATA Device. */
+	mdelay(100);
+
+	/* Deassert the Reset */
+	writel(0x0200, base + BK3710_MISCCTL);
+
+	/*
+	 * Program the IDETIMP Register Value based on the following assumptions
+	 *
+	 * (ATA_IDETIMP_IDEEN		, ENABLE ) |
+	 * (ATA_IDETIMP_SLVTIMEN	, DISABLE) |
+	 * (ATA_IDETIMP_RDYSMPL		, 70NS)    |
+	 * (ATA_IDETIMP_RDYRCVRY	, 50NS)    |
+	 * (ATA_IDETIMP_DMAFTIM1	, PIOCOMP) |
+	 * (ATA_IDETIMP_PREPOST1	, DISABLE) |
+	 * (ATA_IDETIMP_RDYSEN1		, DISABLE) |
+	 * (ATA_IDETIMP_PIOFTIM1	, DISABLE) |
+	 * (ATA_IDETIMP_DMAFTIM0	, PIOCOMP) |
+	 * (ATA_IDETIMP_PREPOST0	, DISABLE) |
+	 * (ATA_IDETIMP_RDYSEN0		, DISABLE) |
+	 * (ATA_IDETIMP_PIOFTIM0	, DISABLE)
+	 */
+	writew(0xB388, base + BK3710_IDETIMP);
+
+	/*
+	 * Configure  SIDETIM  Register
+	 * (ATA_SIDETIM_RDYSMPS1	,120NS ) |
+	 * (ATA_SIDETIM_RDYRCYS1	,120NS )
+	 */
+	writeb(0, base + BK3710_SIDETIM);
+
+	/*
+	 * UDMACTL Ultra-ATA DMA Control
+	 * (ATA_UDMACTL_UDMAP1	, 0 ) |
+	 * (ATA_UDMACTL_UDMAP0	, 0 )
+	 *
+	 */
+	writew(0, base + BK3710_UDMACTL);
+
+	/*
+	 * MISCCTL Miscellaneous Conrol Register
+	 * (ATA_MISCCTL_RSTMODEP	, 1) |
+	 * (ATA_MISCCTL_RESETP		, 0) |
+	 * (ATA_MISCCTL_TIMORIDE	, 1)
+	 */
+	writel(0x201, base + BK3710_MISCCTL);
+
+	/*
+	 * IORDYTMP IORDY Timer for Primary Register
+	 * (ATA_IORDYTMP_IORDYTMP     , 0xffff  )
+	 */
+	writel(0xFFFF, base + BK3710_IORDYTMP);
+
+	/*
+	 * Configure BMISP Register
+	 * (ATA_BMISP_DMAEN1	, DISABLE )	|
+	 * (ATA_BMISP_DMAEN0	, DISABLE )	|
+	 * (ATA_BMISP_IORDYINT	, CLEAR)	|
+	 * (ATA_BMISP_INTRSTAT	, CLEAR)	|
+	 * (ATA_BMISP_DMAERROR	, CLEAR)
+	 */
+	writew(0, base + BK3710_BMISP);
+
+	palm_bk3710_setpiomode(base, NULL, 0, 600, 0);
+	palm_bk3710_setpiomode(base, NULL, 1, 600, 0);
+}
+static int __devinit palm_bk3710_probe(struct platform_device *pdev)
+{
+	hw_regs_t ide_ctlr_info;
+	int index = 0;
+	int pribase;
+	struct clk *clkp;
+	struct resource *mem, *irq;
+	ide_hwif_t *hwif;
+	void __iomem *base;
+
+	clkp = clk_get(NULL, "IDECLK");
+	if (IS_ERR(clkp))
+		return -ENODEV;
+
+	ideclkp = clkp;
+	clk_enable(ideclkp);
+	ide_palm_clk = clk_get_rate(ideclkp)/100000;
+	ide_palm_clk = (10000/ide_palm_clk) + 1;
+	/* Register the IDE interface with Linux ATA Interface */
+	memset(&ide_ctlr_info, 0, sizeof(ide_ctlr_info));
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (mem == NULL) {
+		printk(KERN_ERR "failed to get memory region resource\n");
+		return -ENODEV;
+	}
+	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (irq == NULL) {
+		printk(KERN_ERR "failed to get IRQ resource\n");
+		return -ENODEV;
+	}
+
+	base = (void *)mem->start;
+
+	/* Configure the Palm Chip controller */
+	palm_bk3710_chipinit(base);
+
+	pribase = mem->start + IDE_PALM_ATA_PRI_REG_OFFSET;
+	for (index = 0; index < IDE_NR_PORTS - 2; index++)
+		ide_ctlr_info.io_ports[index] = pribase + index;
+	ide_ctlr_info.io_ports[IDE_CONTROL_OFFSET] = mem->start +
+			IDE_PALM_ATA_PRI_CTL_OFFSET;
+	ide_ctlr_info.irq = irq->start;
+	ide_ctlr_info.chipset = ide_palm3710;
+
+	if (ide_register_hw(&ide_ctlr_info, NULL, &hwif) < 0) {
+		printk(KERN_WARNING "Palm Chip BK3710 IDE Register Fail\n");
+		return -ENODEV;
+	}
+
+	hwif->set_pio_mode = &palm_bk3710_set_pio_mode;
+	hwif->set_dma_mode = &palm_bk3710_set_dma_mode;
+	hwif->mmio = 1;
+	default_hwif_mmiops(hwif);
+	hwif->cbl = ATA_CBL_PATA80;
+	hwif->ultra_mask = 0x1f;	/* Ultra DMA Mode 4 Max
+						(input clk 99MHz) */
+	hwif->mwdma_mask = 0x7;
+	hwif->drives[0].autotune = 1;
+	hwif->drives[1].autotune = 1;
+
+	ide_setup_dma(hwif, mem->start);
+
+	return 0;
+}
+
+static struct platform_driver platform_bk_driver = {
+	.driver = {
+		.name = "palm_bk3710",
+	},
+	.probe = palm_bk3710_probe,
+	.remove = NULL,
+};
+
+static int __init palm_bk3710_init(void)
+{
+	return platform_driver_register(&platform_bk_driver);
+}
+
+module_init(palm_bk3710_init);
+MODULE_LICENSE("GPL");
+
Index: 2.6.25.ide/drivers/ide/ide-proc.c
===================================================================
--- 2.6.25.ide.orig/drivers/ide/ide-proc.c
+++ 2.6.25.ide/drivers/ide/ide-proc.c
@@ -65,6 +65,7 @@ static int proc_ide_read_imodel
 		case ide_4drives:	name = "4drives";	break;
 		case ide_pmac:		name = "mac-io";	break;
 		case ide_au1xxx:	name = "au1xxx";	break;
+		case ide_palm3710:      name = "palm3710";      break;
 		case ide_etrax100:	name = "etrax100";	break;
 		case ide_acorn:		name = "acorn";		break;
 		default:		name = "(unknown)";	break;
Index: 2.6.25.ide/include/linux/ide.h
===================================================================
--- 2.6.25.ide.orig/include/linux/ide.h
+++ 2.6.25.ide/include/linux/ide.h
@@ -173,7 +173,7 @@ enum {		ide_unknown,	ide_generic,	ide_pc
 		ide_rz1000,	ide_trm290,
 		ide_cmd646,	ide_cy82c693,	ide_4drives,
 		ide_pmac,	ide_etrax100,	ide_acorn,
-		ide_au1xxx, ide_forced
+		ide_au1xxx,	ide_palm3710,	ide_forced
 };
 
 typedef u8 hwif_chipset_t;

Re: [PATCH] Palmchip BK3710 IDE driver

[Bartlomiej Zolnierkiewicz]

On Tuesday 05 February 2008, Anton Salnikov wrote:
> I've tested the driver on 2.6.24.
> 
> I've made the changes you asked for. But when I tested them on arm/mach-davinci 

Thanks!

> architecture on the current Linus' git there was link error undefined reference 
> to symbol ide_hwif_setup_dma().
> However on x86_64 architecture it compiles well without any warnings with 
> respect to link error of undefined clk_get() clk_enable() clk_get_rate(), which 
> are not present in this architecture.

ide_hwif_setup_dma() comes from setup-pci.c which depends on BLK_DEV_IDEPCI.
palm_bk3710 host driver selects BLK_DEV_IDEDMA_PCI which should also select
BLK_DEV_IDEPCI but PCI doesn't even seem to be supported by this ARM platform.

I (hopefully) workarounded the issue by adding an additional #ifdef to
<linux/ide.h> but it is a unmaintanable and ugly hack:

Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1014,7 +1014,8 @@ extern int __ide_pci_register_driver(str
 void ide_pci_setup_ports(struct pci_dev *, const struct ide_port_info *, int, u8 *);
 void ide_setup_pci_noise(struct pci_dev *, const struct ide_port_info *);
 
-#ifdef CONFIG_BLK_DEV_IDEDMA_PCI
+/* FIXME: palm_bk3710 uses BLK_DEV_IDEDMA_PCI without BLK_DEV_IDEPCI! */
+#if defined(CONFIG_BLK_DEV_IDEPCI) && defined(CONFIG_BLK_DEV_IDEDMA_PCI)
 void ide_hwif_setup_dma(ide_hwif_t *, const struct ide_port_info *);
 #else
 static inline void ide_hwif_setup_dma(ide_hwif_t *hwif,


Anton/Sergei: please look into fixing it properly - we probably just need to
to have a new config option (CONFIG_IDE_SFF_DMA sounds neat) to be selected by
both BLK_DEV_{IDEDMA_PCI,PALMCHIP_BK3710} and to replace BLK_DEV_IDEDMA_PCI
#ifdefs in ide-dma.c.

> > I've just noticed that there is no cable detection for UDMA modes > UDMA2?
> For the present controller in documentation: Cable Select (CSEL) - unsupported 
> IDE controller signal. This signal is not necessary because an 80-conductor 
> cable must be used with the DM644x.
> 
> Signed-off-by: Anton Salnikov <asalnikov@ru.mvista.com>

Minor issue: the original patch description got lost somehow so I just used
the one from the previous version.

> ---
> 
>  drivers/ide/Kconfig           |    9 
>  drivers/ide/arm/Makefile      |    1 
>  drivers/ide/arm/palm_bk3710.c |  420 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/ide/ide-proc.c        |    1 
>  include/linux/ide.h           |    2 
>  5 files changed, 432 insertions(+), 1 deletion(-)
> 
> Index: 2.6.25.ide/drivers/ide/Kconfig
> ===================================================================
> --- 2.6.25.ide.orig/drivers/ide/Kconfig
> +++ 2.6.25.ide/drivers/ide/Kconfig
> @@ -1009,6 +1009,15 @@ config BLK_DEV_Q40IDE
>  	  normally be on; disable it only if you are running a custom hard
>  	  drive subsystem through an expansion card.
>  
> +config BLK_DEV_PALMCHIP_BK3710
> +	tristate "Palmchip bk3710 IDE controller support"
> +	depends on ARCH_DAVINCI
> +	select BLK_DEV_IDEDMA_PCI
> +	help
> +	  Say Y here if you want to support the onchip IDE controller on the
> +	  TI DaVinci SoC
> +
> +
>  config BLK_DEV_MPC8xx_IDE
>  	tristate "MPC8xx IDE support"
>  	depends on 8xx && (LWMON || IVMS8 || IVML24 || TQM8xxL) && IDE=y && 
> BLK_DEV_IDE=y && !PPC_MERGE
> Index: 2.6.25.ide/drivers/ide/arm/Makefile
> ===================================================================
> --- 2.6.25.ide.orig/drivers/ide/arm/Makefile
> +++ 2.6.25.ide/drivers/ide/arm/Makefile
> @@ -2,6 +2,7 @@
>  obj-$(CONFIG_BLK_DEV_IDE_ICSIDE)	+= icside.o
>  obj-$(CONFIG_BLK_DEV_IDE_RAPIDE)	+= rapide.o
>  obj-$(CONFIG_BLK_DEV_IDE_BAST)		+= bast-ide.o
> +obj-$(CONFIG_BLK_DEV_PALMCHIP_BK3710)	+= palm_bk3710.o
>  
>  ifeq ($(CONFIG_IDE_ARM), m)
>  	obj-m += ide_arm.o
> Index: 2.6.25.ide/drivers/ide/arm/palm_bk3710.c
> ===================================================================
> --- /dev/null
> +++ 2.6.25.ide/drivers/ide/arm/palm_bk3710.c

[...]

> +/*static inline u8 hwif_read8(u32 base, u32 reg)
> +{
> +	return readb(base + reg);
> +}
> +
> +static inline u16 hwif_read16(u32 base, u32 reg)
> +{
> +	return readw(base + reg);
> +}
> +
> +static inline void hwif_write16(u32 base, u16 val, u32 reg)
> +{
> +	writew(val, base + reg);
> +}
> +
> +static inline u32 hwif_read32(u32 base, u32 reg)
> +{
> +	return readl(base + reg);
> +}
> +
> +static inline void hwif_write32(u32 base, u32 val, u32 reg)
> +{
> +	writel(val, base + reg);
> +}*/

I removed this chunk while merging the patch (no need for a dead code)

also added "Reviewed-by:" tags crediting Alan & Sergei

Re: [PATCH] Palmchip BK3710 IDE driver

[Sergei Shtylyov]

Anton Salnikov wrote:

> I've tested the driver on 2.6.24.

    Unfortunately, I've never seen the platofrm device registration code until 
now...

> Signed-off-by: Anton Salnikov <asalnikov@ru.mvista.com>

> Index: 2.6.25.ide/drivers/ide/arm/palm_bk3710.c
> ===================================================================
> --- /dev/null
> +++ 2.6.25.ide/drivers/ide/arm/palm_bk3710.c
> @@ -0,0 +1,420 @@
[...]
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (mem == NULL) {
> +		printk(KERN_ERR "failed to get memory region resource\n");
> +		return -ENODEV;
> +	}
> +	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +	if (irq == NULL) {
> +		printk(KERN_ERR "failed to get IRQ resource\n");
> +		return -ENODEV;
> +	}
> +
> +	base = (void *)mem->start;

   Crrap! You shouldn't specify virtual addresses in the device resources 
whatever arch you are using. Should've been ioremap() here. And where did only 
my eyes were?

MBR, Sergei