[U-Boot] [PATCH] BLOCK: Fixup iMX51 ATA driver

[U-Boot] [PATCH] BLOCK: Fixup iMX51 ATA driver

[Marek Vasut]

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
---
NOTE: Wolfgang, is this what you meant ? If so, I'll merge this with the old
driver and send a v2 patch. Otherwise, can you point me in a right direction?

NOTE2: I haven't tested this, I'll do so in the evening.

 drivers/block/mxc_ata.c |  131 +++++++++++++++++++++++++---------------------
 1 files changed, 71 insertions(+), 60 deletions(-)

diff --git a/drivers/block/mxc_ata.c b/drivers/block/mxc_ata.c
index 6f19e6f..e65cfa7 100644
--- a/drivers/block/mxc_ata.c
+++ b/drivers/block/mxc_ata.c
@@ -37,49 +37,53 @@
 #include <asm/arch/clock.h>
 
 /* MXC ATA register offsets */
-enum {
-	MXC_ATA_TIME_OFF		= 0x00,
-	MXC_ATA_TIME_ON			= 0x01,
-	MXC_ATA_TIME_1			= 0x02,
-	MXC_ATA_TIME_2W			= 0x03,
-	MXC_ATA_TIME_2R			= 0x04,
-	MXC_ATA_TIME_AX			= 0x05,
-	MXC_ATA_TIME_PIO_RDX		= 0x06,
-	MXC_ATA_TIME_4			= 0x07,
-	MXC_ATA_TIME_9			= 0x08,
-	MXC_ATA_TIME_M			= 0x09,
-	MXC_ATA_TIME_JN			= 0x0a,
-	MXC_ATA_TIME_D			= 0x0b,
-	MXC_ATA_TIME_K			= 0x0c,
-	MXC_ATA_TIME_ACK		= 0x0d,
-	MXC_ATA_TIME_ENV		= 0x0e,
-	MXC_ATA_TIME_UDMA_RDX		= 0x0f,
-	MXC_ATA_TIME_ZAH		= 0x10,
-	MXC_ATA_TIME_MLIX		= 0x11,
-	MXC_ATA_TIME_DVH		= 0x12,
-	MXC_ATA_TIME_DZFS		= 0x13,
-	MXC_ATA_TIME_DVS		= 0x14,
-	MXC_ATA_TIME_CVH		= 0x15,
-	MXC_ATA_TIME_SS			= 0x16,
-	MXC_ATA_TIME_CYC		= 0x17,
-	MXC_ATA_FIFO_DATA_16		= 0x1c,
-	MXC_ATA_FIFO_DATA_32		= 0x18,
-	MXC_ATA_FIFO_FILL		= 0x20,
-	MXC_ATA_ATA_CONTROL		= 0x24,
-	MXC_ATA_INTERRUPT_PENDING	= 0x28,
-	MXC_ATA_INTERRUPT_ENABLE	= 0x2c,
-	MXC_ATA_INTERRUPT_CLEAR		= 0x30,
-	MXC_ATA_FIFO_ALARM		= 0x34,
-	MXC_ATA_DRIVE_DATA		= 0xa0,
-	MXC_ATA_DRIVE_FEATURES		= 0xa4,
-	MXC_ATA_DRIVE_SECTOR_COUNT	= 0xa8,
-	MXC_ATA_DRIVE_SECTOR_NUM	= 0xac,
-	MXC_ATA_DRIVE_CYL_LOW		= 0xb0,
-	MXC_ATA_DRIVE_CYL_HIGH		= 0xb4,
-	MXC_ATA_DRIVE_DEV_HEAD		= 0xb8,
-	MXC_ATA_COMMAND			= 0xbc,
-	MXC_ATA_STATUS			= 0xbc,
-	MXC_ATA_ALT_STATUS		= 0xd8,
+struct mxc_ata_config_regs {
+	u8	time_off;	/* 0x00 */
+	u8	time_on;
+	u8	time_1;
+	u8	time_2w;
+	u8	time_2r;
+	u8	time_ax;
+	u8	time_pio_rdx;
+	u8	time_4;
+	u8	time_9;
+	u8	time_m;
+	u8	time_jn;
+	u8	time_d;
+	u8	time_k;
+	u8	time_ack;
+	u8	time_env;
+	u8	time_udma_rdx;
+	u8	time_zah;	/* 0x10 */
+	u8	time_mlix;
+	u8	time_dvh;
+	u8	time_dzfs;
+	u8	time_dvs;
+	u8	time_cvh;
+	u8	time_ss;
+	u8	time_cyc;
+	u32	hole;
+	u32	fifo_data_16;
+	u32	fifo_data_32;
+	u32	fifo_fill;
+	u32	ata_control;
+	u32	interrupt_pending;
+	u32	interrupt_enable;
+	u32	interrupt_clear;
+	u32	fifo_alarm;
+};
+
+struct mxc_data_hdd_regs {
+	u32	drive_data;	/* 0xa0 */
+	u32	drive_features;
+	u32	drive_sector_count;
+	u32	drive_sector_num;
+	u32	drive_cyl_low;
+	u32	drive_cyl_high;
+	u32	drive_dev_head;
+	u32	command;
+	u32	status;
+	u32	alt_status;
 };
 
 /* PIO timing table */
@@ -93,47 +97,54 @@ uint16_t pio_t4[NR_PIO_SPECS]		= { 30,  20,  15,  10,  10 };
 uint16_t pio_t9[NR_PIO_SPECS]		= { 20,  15,  10,  10,  10 };
 uint16_t pio_tA[NR_PIO_SPECS]		= { 50,  50,  50,  50,  50 };
 
-#define	REG_TO_OFFSET(reg)	((reg & 0x3) * 8)
+#define	REG2OFF(reg)	((((uint32_t)reg) & 0x3) * 8)
 static void set_ata_bus_timing(unsigned char mode)
 {
 	uint32_t val;
 	uint32_t T = 1000000000 / mxc_get_clock(MXC_IPG_CLK);
 
+	struct mxc_ata_config_regs *ata_cfg_regs;
+	ata_cfg_regs = (struct mxc_ata_config_regs *)CONFIG_SYS_ATA_BASE_ADDR;
+
 	if (mode >= NR_PIO_SPECS)
 		return;
 
 	/* Write TIME_OFF/ON/1/2W */
-	val =	(3 << REG_TO_OFFSET(MXC_ATA_TIME_OFF)) |
-		(3 << REG_TO_OFFSET(MXC_ATA_TIME_ON)) |
-		(((pio_t1[mode] + T) / T) << REG_TO_OFFSET(MXC_ATA_TIME_1)) |
-		(((pio_t2_8[mode] + T) / T) << REG_TO_OFFSET(MXC_ATA_TIME_2W));
-	writel(val, MXC_ATA_TIME_OFF + CONFIG_SYS_ATA_BASE_ADDR);
+	val =	(3 << REG2OFF(&ata_cfg_regs->time_off)) |
+		(3 << REG2OFF(&ata_cfg_regs->time_on)) |
+		(((pio_t1[mode] + T) / T) << REG2OFF(&ata_cfg_regs->time_1)) |
+		(((pio_t2_8[mode] + T) / T) << REG2OFF(&ata_cfg_regs->time_2w));
+	writeb(val, &ata_cfg_regs->time_off);
 
 	/* Write TIME_2R/AX/RDX/4 */
-	val =	(((pio_t2_8[mode] + T) / T) << REG_TO_OFFSET(MXC_ATA_TIME_2R)) |
-		(((pio_tA[mode] + T) / T + 2) << REG_TO_OFFSET(MXC_ATA_TIME_AX)) |
-		(1 << REG_TO_OFFSET(MXC_ATA_TIME_PIO_RDX)) |
-		(((pio_t4[mode] + T) / T) << REG_TO_OFFSET(MXC_ATA_TIME_4));
-	writel(val, MXC_ATA_TIME_2R + CONFIG_SYS_ATA_BASE_ADDR);
+	val =	(((pio_t2_8[mode] + T) / T) << REG2OFF(&ata_cfg_regs->time_2r)) |
+		(((pio_tA[mode] + T) / T + 2) << REG2OFF(&ata_cfg_regs->time_ax)) |
+		(1 << REG2OFF(&ata_cfg_regs->time_pio_rdx)) |
+		(((pio_t4[mode] + T) / T) << REG2OFF(&ata_cfg_regs->time_4));
+	writel(val, &ata_cfg_regs->time_2r);
 
 	/* Write TIME_9 ; the rest of timing registers is irrelevant for PIO */
-	val =	(((pio_t9[mode] + T) / T) << REG_TO_OFFSET(MXC_ATA_TIME_9));
-	writel(val, MXC_ATA_TIME_9 + CONFIG_SYS_ATA_BASE_ADDR);
+	val =	(((pio_t9[mode] + T) / T) << REG2OFF(&ata_cfg_regs->time_9));
+	writel(val, &ata_cfg_regs->time_9);
 }
 
 int ide_preinit(void)
 {
-	uint32_t ata_control = CONFIG_SYS_ATA_BASE_ADDR + MXC_ATA_ATA_CONTROL;
+	struct mxc_ata_config_regs *ata_cfg_regs;
+	ata_cfg_regs = (struct mxc_ata_config_regs *)CONFIG_SYS_ATA_BASE_ADDR;
 
 	/* 46.3.3.4 @ FSL iMX51 manual */
-	writel(0x80, ata_control);	/* FIFO normal op., drive reset */
-	writel(0xc0, ata_control);	/* FIFO normal op., drive not reset */
+	/* FIFO normal op., drive reset */
+	writel(0x80, ata_cfg_regs->ata_control);
+	/* FIFO normal op., drive not reset */
+	writel(0xc0, ata_cfg_regs->ata_control);
 
 	/* Configure the PIO timing */
 	set_ata_bus_timing(CONFIG_MXC_ATA_PIO_MODE);
 
 	/* 46.3.3.4 @ FSL iMX51 manual */
-	writel(0x41, ata_control);	/* Drive not reset, IORDY handshake */
+	/* Drive not reset, IORDY handshake */
+	writel(0x41, ata_cfg_regs->ata_control);
 
 	return 0;
 }
-- 
1.7.2.3

[U-Boot] [PATCH] BLOCK: Fixup iMX51 ATA driver

[Stefano Babic]

On 11/29/2010 03:18 PM, Marek Vasut wrote:
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>

Hi Marek,

in my regression tests to check if all IMX boards are compiled clean I
found the following warnings (efikamx):

mxc_ata.c:90: warning: 'pio_t0' defined but not used
mxc_ata.c:93: warning: 'pio_t2_16' defined but not used
mxc_ata.c:94: warning: 'pio_t2i' defined but not used

As I have already merged your patch into u-boot-imx (and Albert has
already merged it in u-boot-arm), I will post a patch on top of it to
remove them. Anyway, have you expected that these three variables are
not used at all ? Could I remove them ?

Regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH] BLOCK: Fixup iMX51 ATA driver

[Marek Vasut]

On Tuesday 01 February 2011 08:11:51 Stefano Babic wrote:
> On 11/29/2010 03:18 PM, Marek Vasut wrote:
> > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> 
> Hi Marek,
> 
> in my regression tests to check if all IMX boards are compiled clean I
> found the following warnings (efikamx):
> 
> mxc_ata.c:90: warning: 'pio_t0' defined but not used
> mxc_ata.c:93: warning: 'pio_t2_16' defined but not used
> mxc_ata.c:94: warning: 'pio_t2i' defined but not used
> 
> As I have already merged your patch into u-boot-imx (and Albert has
> already merged it in u-boot-arm), I will post a patch on top of it to
> remove them. Anyway, have you expected that these three variables are
> not used at all ? Could I remove them ?

Yes, you can remove them. They are unused in the original freescale driver too, 
stupid sed-mistake.

Thanks for the good catch :)

> 
> Regards,
> Stefano