Patchwork [U-Boot,v2] BLOCK: Add freescale IMX51 PATA driver

login
register
mail settings
Submitter Marek Vasut
Date Nov. 30, 2010, 3:26 a.m.
Message ID <1291087578-16714-1-git-send-email-marek.vasut@gmail.com>
Download mbox | patch
Permalink /patch/73517/
State Changes Requested
Delegated to: Stefano Babic
Headers show

Comments

Marek Vasut - Nov. 30, 2010, 3:26 a.m.
Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
---
v2: Use structures instead of defines as Wolfgang proposed

 drivers/block/Makefile  |    1 +
 drivers/block/mxc_ata.c |  149 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 150 insertions(+), 0 deletions(-)
 create mode 100644 drivers/block/mxc_ata.c
Stefano Babic - Nov. 30, 2010, 11:15 a.m.
On 11/30/2010 04:26 AM, Marek Vasut wrote:
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> ---
> v2: Use structures instead of defines as Wolfgang proposed
> 

Hi Marek,

>  drivers/block/Makefile  |    1 +
>  drivers/block/mxc_ata.c |  149 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 150 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/block/mxc_ata.c

In which context does this driver run ? I thought to find a similar
implementation I see, for example, in pata_bfin.c, with entry points to
read/write data, but I cannot see them. I assume you set only
CONFIG_CMD_IDE for testing.

On which board do you plan to add this P-ATA driver ?

Trying to apply this driver give me a link error:
/home/stefano/Projects/imx/u-boot-imx/common/cmd_ide.c:905: undefined
reference to `outsw'
common/libcommon.o: In function `input_data':
/home/stefano/Projects/imx/u-boot-imx/common/cmd_ide.c:963: undefined
reference to `insw'
common/libcommon.o: In function `__ide_inb':
/home/stefano/Projects/imx/u-boot-imx/common/cmd_ide.c:530: undefined
reference to `inb'

This functions are not implemented for the i.MX51 SOC.

> +	/* Configure the PIO timing */
> +	set_ata_bus_timing(CONFIG_MXC_ATA_PIO_MODE);

CONFIG_MXC_ATA_PIO_MODE is a new parameter, to be set in board
configuration file. It should be documented in some ways.

Best regards,
Stefano
Marek Vasut - Dec. 1, 2010, 2:46 a.m.
On Tuesday 30 November 2010 12:15:33 Stefano Babic wrote:
> On 11/30/2010 04:26 AM, Marek Vasut wrote:
> > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> > ---
> > v2: Use structures instead of defines as Wolfgang proposed
> 
> Hi Marek,
> 
> >  drivers/block/Makefile  |    1 +
> >  drivers/block/mxc_ata.c |  149
> >  +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 150
> >  insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/block/mxc_ata.c
> 
> In which context does this driver run ? I thought to find a similar
> implementation I see, for example, in pata_bfin.c, with entry points to
> read/write data, but I cannot see them. I assume you set only
> CONFIG_CMD_IDE for testing.
> 
> On which board do you plan to add this P-ATA driver ?

On efikamx, I'll soon submit it mainline (when I clean it up enough). Here's the 
relevant part of configuration file (the #define __io is actually the important 
thing):

/*
 * ATA/IDE
 */
#ifdef  CONFIG_CMD_IDE
#define CONFIG_LBA48
#undef  CONFIG_IDE_LED
#undef  CONFIG_IDE_RESET

#define CONFIG_MX51_PATA

#define __io

#define CONFIG_SYS_IDE_MAXBUS           1
#define CONFIG_SYS_IDE_MAXDEVICE        1

#define CONFIG_SYS_ATA_BASE_ADDR        0x83fe0000
#define CONFIG_SYS_ATA_IDE0_OFFSET      0x0

#define CONFIG_SYS_ATA_DATA_OFFSET      0xa0
#define CONFIG_SYS_ATA_REG_OFFSET       0xa0
#define CONFIG_SYS_ATA_ALT_OFFSET       0xd8

#define CONFIG_SYS_ATA_STRIDE           4

#define CONFIG_IDE_PREINIT
#define CONFIG_MXC_ATA_PIO_MODE         4
#endif
> 
> Trying to apply this driver give me a link error:
> /home/stefano/Projects/imx/u-boot-imx/common/cmd_ide.c:905: undefined
> reference to `outsw'
> common/libcommon.o: In function `input_data':
> /home/stefano/Projects/imx/u-boot-imx/common/cmd_ide.c:963: undefined
> reference to `insw'
> common/libcommon.o: In function `__ide_inb':
> /home/stefano/Projects/imx/u-boot-imx/common/cmd_ide.c:530: undefined
> reference to `inb'
> 
> This functions are not implemented for the i.MX51 SOC.
> 
> > +	/* Configure the PIO timing */
> > +	set_ata_bus_timing(CONFIG_MXC_ATA_PIO_MODE);
> 
> CONFIG_MXC_ATA_PIO_MODE is a new parameter, to be set in board
> configuration file. It should be documented in some ways.

Indeed ... where if you could point me to an appropriate place ?
> 
> Best regards,
> Stefano
Wolfgang Denk - Dec. 1, 2010, 5:34 a.m.
Dear Marek Vasut,

In message <201012010346.21897.marek.vasut@gmail.com> you wrote:
>
> On efikamx, I'll soon submit it mainline (when I clean it up enough). Here's the 
> relevant part of configuration file (the #define __io is actually the important 
> thing):

Is there a way to do without this?

> #define __io

I mean, these empty defines look pretty bogus to me.

> > CONFIG_MXC_ATA_PIO_MODE is a new parameter, to be set in board
> > configuration file. It should be documented in some ways.
> 
> Indeed ... where if you could point me to an appropriate place ?

In the README, please.

Best regards,

Wolfgang Denk
Stefano Babic - Dec. 1, 2010, 8:48 a.m.
On 12/01/2010 06:34 AM, Wolfgang Denk wrote:
> Dear Marek Vasut,
> 
> In message <201012010346.21897.marek.vasut@gmail.com> you wrote:
>>
>> On efikamx, I'll soon submit it mainline (when I clean it up enough). Here's the 
>> relevant part of configuration file (the #define __io is actually the important 
>> thing):
> 
> Is there a way to do without this?
> 
>> #define __io
> 
> I mean, these empty defines look pretty bogus to me.

I have found several boards with the same behavior:

include/configs/mv-common.h:#define __io
include/configs/r2dplus.h:#define __io
include/configs/edminiv2.h:#define __io
include/configs/nhk8815.h:#define __io(a)((void __iomem *)(PCI_IO_VADDR
+ (a)))
include/configs/r7780mp.h:#define __io

__io is defined if CMD_IDE is set for most of them, meaning CMD_IDE
requires this define, even if it has no particular meaning for the
processor we are using.
It seems to me that defining __io is only required for enabling the
switch inside arch/arm/include/asm/io.h:
#ifdef __io
#define outsb(p,d,l)                    __raw_writesb(__io(p),d,l)
...

Really __io should accept a parameter to make some casting, as I can see
in the blackfin architecture. Except the nhk8815 board, no parameter is
accepted and the define enables only the #ifdef switch.

Probably there is a better way to do this ;-). Could be maybe better to
define __io inside io.h if it is not already defined by a board ?

Best regards,
Stefano Babic
Marek Vasut - Dec. 16, 2010, 1:59 a.m.
On Wednesday 01 December 2010 09:48:45 Stefano Babic wrote:
> On 12/01/2010 06:34 AM, Wolfgang Denk wrote:
> > Dear Marek Vasut,
> > 
> > In message <201012010346.21897.marek.vasut@gmail.com> you wrote:
> >> On efikamx, I'll soon submit it mainline (when I clean it up enough).
> >> Here's the relevant part of configuration file (the #define __io is
> >> actually the important
> > 
> >> thing):
> > Is there a way to do without this?
> > 
> >> #define __io
> > 
> > I mean, these empty defines look pretty bogus to me.
> 
> I have found several boards with the same behavior:
> 
> include/configs/mv-common.h:#define __io
> include/configs/r2dplus.h:#define __io
> include/configs/edminiv2.h:#define __io
> include/configs/nhk8815.h:#define __io(a)((void __iomem *)(PCI_IO_VADDR
> + (a)))
> include/configs/r7780mp.h:#define __io
> 
> __io is defined if CMD_IDE is set for most of them, meaning CMD_IDE
> requires this define, even if it has no particular meaning for the
> processor we are using.
> It seems to me that defining __io is only required for enabling the
> switch inside arch/arm/include/asm/io.h:
> #ifdef __io
> #define outsb(p,d,l)                    __raw_writesb(__io(p),d,l)
> ...
> 

Indeed

> Really __io should accept a parameter to make some casting, as I can see
> in the blackfin architecture. Except the nhk8815 board, no parameter is
> accepted and the define enables only the #ifdef switch.

Yes
> 
> Probably there is a better way to do this ;-). Could be maybe better to
> define __io inside io.h if it is not already defined by a board ?

This should go through a separate patch though. Did anything like that emerged 
while I was away or shall someone work on this ? Besides, what about this patch?

Thanks, cheers!
> 
> Best regards,
> Stefano Babic

Patch

diff --git a/drivers/block/Makefile b/drivers/block/Makefile
index e27175b..aa7dc87 100644
--- a/drivers/block/Makefile
+++ b/drivers/block/Makefile
@@ -31,6 +31,7 @@  COBJS-$(CONFIG_FSL_SATA) += fsl_sata.o
 COBJS-$(CONFIG_LIBATA) += libata.o
 COBJS-$(CONFIG_CMD_MG_DISK) += mg_disk.o
 COBJS-$(CONFIG_MVSATA_IDE) += mvsata_ide.o
+COBJS-$(CONFIG_MX51_PATA) += mxc_ata.o
 COBJS-$(CONFIG_PATA_BFIN) += pata_bfin.o
 COBJS-$(CONFIG_SATA_DWC) += sata_dwc.o
 COBJS-$(CONFIG_SATA_SIL3114) += sata_sil3114.o
diff --git a/drivers/block/mxc_ata.c b/drivers/block/mxc_ata.c
new file mode 100644
index 0000000..62c5cf4
--- /dev/null
+++ b/drivers/block/mxc_ata.c
@@ -0,0 +1,149 @@ 
+/*
+ * Freescale iMX51 ATA driver
+ *
+ * Copyright (C) 2010 Marek Vasut <marek.vasut@gmail.com>
+ *
+ * Based on code by:
+ *	Mahesh Mahadevan <mahesh.mahadevan@freescale.com>
+ *
+ * Based on code from original FSL ATA driver, which is
+ * part of eCos, the Embedded Configurable Operating System.
+ * Copyright (C) 1998, 1999, 2000, 2001, 2002 Red Hat, Inc.
+ *
+ * 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., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <command.h>
+#include <config.h>
+#include <asm/byteorder.h>
+#include <asm/io.h>
+#include <ide.h>
+
+#include <asm/arch/imx-regs.h>
+#include <asm/arch/clock.h>
+
+/* MXC ATA register offsets */
+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	fifo_data_32;	/* 0x18 */
+	u32	fifo_data_16;
+	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 */
+#define	NR_PIO_SPECS	5
+uint16_t pio_t0[NR_PIO_SPECS]		= { 600, 383, 240, 180, 120 };
+uint16_t pio_t1[NR_PIO_SPECS]		= { 70,  50,  30,  30,  25 };
+uint16_t pio_t2_8[NR_PIO_SPECS]		= { 290, 290, 290, 80,  70 };
+uint16_t pio_t2_16[NR_PIO_SPECS]	= { 165, 125, 100, 80,  70 };
+uint16_t pio_t2i[NR_PIO_SPECS]		= { 40,  0,   0,   0,   0 };
+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	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 << 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));
+	writel(val, &ata_cfg_regs->time_off);
+
+	/* Write TIME_2R/AX/RDX/4 */
+	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) << REG2OFF(&ata_cfg_regs->time_9));
+	writel(val, &ata_cfg_regs->time_9);
+}
+
+int ide_preinit(void)
+{
+	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 */
+	/* 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 */
+	/* Drive not reset, IORDY handshake */
+	writel(0x41, &ata_cfg_regs->ata_control);
+
+	return 0;
+}