Skip to contentSkip to conversation

BLOCK: Add freescale IMX51 PATA driver

PATCH Superseded Marek Vasut 3 participants 6 messages Nov 30, 2010 – Dec 16, 2010

Subsystems (2)
Arm i.MX and Layerscape Block layer
lore
Share & tools
lore

Marek Vasut

author
#0 lore raw

PATCH v2 1/1 BLOCK: Add freescale IMX51 PATA driver

Signed-off-by Marek Vasut <[email protected]>

---
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
drivers/block/Makefile +1 -0
index e27175b..aa7dc87 100644
@@ -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
drivers/block/mxc_ata.c new file +149 -0

149 lines not on this page · raw message

Signature · 1 line
1.7.2.3
#1 lore raw

PATCH v2 1/1 BLOCK: Add freescale IMX51 PATA driver

Quoted 4 lines · Marek Vasut
Signed-off-by Marek Vasut <[email protected]>

---
v2: Use structures instead of defines as Wolfgang proposed

Hi Marek,

Quoted 4 lines
 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.

drivers/block/mxc_ata.c line 142

Quoted 2 lines
drivers/block/mxc_ata.c new file +2 -0
+	/* 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

Signature · 5 lines
=====================================================================
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
=====================================================================
author
#2 lore raw

PATCH v2 1/1 BLOCK: Add freescale IMX51 PATA driver

Quoted 13 lines · On Tuesday 30 November 2010 12:15:33 Stefano Babic wrote:
Quoted 3 lines · Marek Vasut
Signed-off-by Marek Vasut <[email protected]>

---
v2: Use structures instead of defines as Wolfgang proposed

Hi Marek,

Quoted 5 lines
 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
Quoted 6 lines

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

Quoted 11 lines

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'

Quoted 7 lines

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

drivers/block/mxc_ata.c line 142

Quoted 2 lines
drivers/block/mxc_ata.c new file +2 -0
+	/* 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 ?

Quoted 3 lines

Best regards,
Stefano

#3 lore raw

PATCH v2 1/1 BLOCK: Add freescale IMX51 PATA driver

Dear Marek Vasut,

Quoted 4 lines · In message <[email protected]> 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?

Quoted 1 line

#define __io

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

Quoted 4 lines
Quoted 2 lines

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

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Disobedience:  The silver lining to the cloud of servitude.
- Ambrose Bierce
#4 lore raw

PATCH v2 1/1 BLOCK: Add freescale IMX51 PATA driver

Quoted 8 lines · Wolfgang Denk

Dear Marek Vasut,

Quoted 4 lines · In message <[email protected]> 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):

Quoted 5 lines

Is there a way to do without this?

Quoted 1 line

#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

Signature · 5 lines
=====================================================================
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
=====================================================================
author
#5 lore raw

PATCH v2 1/1 BLOCK: Add freescale IMX51 PATA driver

Quoted 24 lines · On Wednesday 01 December 2010 09:48:45 Stefano Babic wrote:
Quoted 7 lines · Wolfgang Denk

Dear Marek Vasut,

Quoted 3 lines · In message <[email protected]> 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

Quoted 6 lines
Quoted 1 line

thing):

Is there a way to do without this?

Quoted 1 line

#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

Quoted 9 lines

__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

Quoted 3 lines

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

Quoted 3 lines

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!

Quoted 3 lines

Best regards,
Stefano Babic