cmd_ide: Convert to [read,write][b,w]

PATCH New Marek Vasut 3 participants 7 messages Jun 23, 2010 – Jul 21, 2010

lore

Share & tools

lore

Marek Vasut

author

2010-06-23 19:18 UTC #0 lore raw

PATCH 1/1 cmd_ide: Convert to [read,write][b,w]

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

---
 common/cmd_ide.c |   22 +++++++++++++---------
 1 files changed, 13 insertions(+), 9 deletions(-)

common/cmd_ide.c +13 -9

index 093ca9f..9c4b250 100644
@@ -161,8 +161,8 @@ static uchar ide_wait  (int dev, ulong t);
 
 #define IDE_SPIN_UP_TIME_OUT 5000 /* 5 sec spin-up timeout */
 
-static void input_data(int dev, ulong *sect_buf, int words);
-static void output_data(int dev, ulong *sect_buf, int words);
+static void input_data(int dev, uint16_t *sect_buf, int words);
+static void output_data(int dev, uint16_t *sect_buf, int words);
 static void ident_cpy (unsigned char *dest, unsigned char *src, unsigned int len);
 
 #ifndef CONFIG_SYS_ATA_PORT_ADDR

@@ -526,7 +526,7 @@ __ide_outb(int dev, int port, unsigned char val)
 {
 	debug ("ide_outb (dev= %d, port= 0x%x, val= 0x%02x) : @ 0x%08lx\n",
 		dev, port, val, (ATA_CURR_BASE(dev)+CONFIG_SYS_ATA_PORT_ADDR(port)));
-	outb(val, (ATA_CURR_BASE(dev)+CONFIG_SYS_ATA_PORT_ADDR(port)));
+	writeb(val, (ATA_CURR_BASE(dev)+CONFIG_SYS_ATA_PORT_ADDR(port)));
 }
 void ide_outb (int dev, int port, unsigned char val)
 		__attribute__((weak, alias("__ide_outb")));

@@ -535,7 +535,7 @@ unsigned char inline
 __ide_inb(int dev, int port)
 {
 	uchar val;
-	val = inb((ATA_CURR_BASE(dev)+CONFIG_SYS_ATA_PORT_ADDR(port)));
+	val = readb((ATA_CURR_BASE(dev)+CONFIG_SYS_ATA_PORT_ADDR(port)));
 	debug ("ide_inb (dev= %d, port= 0x%x) : @ 0x%08lx -> 0x%02x\n",
 		dev, port, (ATA_CURR_BASE(dev)+CONFIG_SYS_ATA_PORT_ADDR(port)), val);
 	return val;

@@ -937,9 +937,11 @@ output_data(int dev, ulong *sect_buf, int words)
 }
 #else	/* ! __PPC__ */
 static void
-output_data(int dev, ulong *sect_buf, int words)
+output_data(int dev, uint16_t *sect_buf, int words)
 {
-	outsw(ATA_CURR_BASE(dev)+ATA_DATA_REG, sect_buf, words<<1);
+	int i;
+	for (i = 0; i < (words << 1); i++)
+		writew(sect_buf[i], ATA_CURR_BASE(dev)+ATA_DATA_REG);
 }
 #endif	/* __PPC__ */

@@ -995,9 +997,11 @@ input_data(int dev, ulong *sect_buf, int words)
 }
 #else	/* ! __PPC__ */
 static void
-input_data(int dev, ulong *sect_buf, int words)
+input_data(int dev, uint16_t *sect_buf, int words)
 {
-	insw(ATA_CURR_BASE(dev)+ATA_DATA_REG, sect_buf, words << 1);
+	int i;
+	for (i = 0; i < (words << 1); i++)
+		sect_buf[i] = readw(ATA_CURR_BASE(dev)+ATA_DATA_REG);
 }
 
 #endif	/* __PPC__ */

@@ -1115,7 +1119,7 @@ static void ide_ident (block_dev_desc_t *dev_desc)
 		return;
 #endif
 
-	input_swap_data (device, iobuf, ATA_SECTORWORDS);
+	input_swap_data (device, (uint16_t *)iobuf, ATA_SECTORWORDS);
 
 	ident_cpy ((unsigned char*)dev_desc->revision, iop->fw_rev, sizeof(dev_desc->revision));
 	ident_cpy ((unsigned char*)dev_desc->vendor, iop->model, sizeof(dev_desc->vendor));

Signature · 1 line

1.7.1

Wolfgang Denk

2010-06-23 20:51 UTC #1 lore raw

PATCH 1/1 cmd_ide: Convert to [read,write][b,w]

Dear Marek Vasut,

Quoted 4 lines · In message <[email protected]> you wrote:

Signed-off-by Marek Vasut <[email protected]>
---
 common/cmd_ide.c |   22 +++++++++++++---------
 1 files changed, 13 insertions(+), 9 deletions(-)

I don't see the big advantage of this patch yet.

common/cmd_ide.c __ide_outb(int dev, int port, unsigned char val) · line 529

Quoted 7 lines

#ifndef CONFIG_SYS_ATA_PORT_ADDR

common/cmd_ide.c +1 -1

@@ -526,7 +526,7 @@ __ide_outb(int dev, int port, unsigned char val)
 {
 	debug ("ide_outb (dev= %d, port= 0x%x, val= 0x%02x) : @ 0x%08lx\n",
 		dev, port, val, (ATA_CURR_BASE(dev)+CONFIG_SYS_ATA_PORT_ADDR(port)));
-	outb(val, (ATA_CURR_BASE(dev)+CONFIG_SYS_ATA_PORT_ADDR(port)));
+	writeb(val, (ATA_CURR_BASE(dev)+CONFIG_SYS_ATA_PORT_ADDR(port)));

So we replace outb() => writeb(), but the function where this is used is still __ide_outb() ?

The __ide_outb() => outb() mapping looks more logical to me.

common/cmd_ide.c output_data(int dev, ulong *sect_buf, int words) · line 944

Quoted 7 lines

common/cmd_ide.c +4 -2

-output_data(int dev, ulong *sect_buf, int words)
+output_data(int dev, uint16_t *sect_buf, int words)
 {
-	outsw(ATA_CURR_BASE(dev)+ATA_DATA_REG, sect_buf, words<<1);
+	int i;
+	for (i = 0; i < (words << 1); i++)
+		writew(sect_buf[i], ATA_CURR_BASE(dev)+ATA_DATA_REG);

And here the code size is growing, too.

What are the exact advantages of your version?

Best regards,

Wolfgang Denk

Signature · 6 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
We are all agreed that your  theory  is  crazy.  The  question  which
divides  us  is  whether it is crazy enough to have a chance of being
correct. My own feeling is that it is not crazy enough.  - Niels Bohr

Marek Vasut

author

2010-06-24 00:36 UTC #2 lore raw

PATCH 1/1 cmd_ide: Convert to [read,write][b,w]

Dne St 23. ?ervna 2010 22:51:28 Wolfgang Denk napsal(a):

Quoted 10 lines

Dear Marek Vasut,
Quoted 5 lines · In message <[email protected]> you wrote:
Signed-off-by Marek Vasut <[email protected]>
---
 common/cmd_ide.c |   22 +++++++++++++---------
 1 files changed, 13 insertions(+), 9 deletions(-)
I don't see the big advantage of this patch yet.

It won't compile at least on ARM. Same case as with the dm9000 ethernet adapter.

Quoted 10 lines

Review __ide_outb(int dev, int port, unsigned char val) · line 531
Quoted 10 lines
#ifndef CONFIG_SYS_ATA_PORT_ADDR
(quoted diff) +0 -0
@@ -526,7 +526,7 @@ __ide_outb(int dev, int port, unsigned char val)
 
 {
 
 	debug ("ide_outb (dev= %d, port= 0x%x, val= 0x%02x) : @ 0x%08lx\n",
 
 		dev, port, val, 

(ATA_CURR_BASE(dev)+CONFIG_SYS_ATA_PORT_ADDR(port)));

Quoted 8 lines

common/cmd_ide.c __ide_outb(int dev, int port, unsigned char val) · line 529
Quoted 3 lines
common/cmd_ide.c +1 -1
-	outb(val, (ATA_CURR_BASE(dev)+CONFIG_SYS_ATA_PORT_ADDR(port)));
+	writeb(val, (ATA_CURR_BASE(dev)+CONFIG_SYS_ATA_PORT_ADDR(port)));
So we replace outb() => writeb(), but the function where this is used is still __ide_outb() ?
The __ide_outb() => outb() mapping looks more logical to me.

See above.

Quoted 4 lines

common/cmd_ide.c +1 -1

-output_data(int dev, ulong *sect_buf, int words)
+output_data(int dev, uint16_t *sect_buf, int words)

Quoted 8 lines

common/cmd_ide.c output_data(int dev, ulong *sect_buf, int words) · line 944
Quoted 6 lines
{
common/cmd_ide.c +3 -1
-	outsw(ATA_CURR_BASE(dev)+ATA_DATA_REG, sect_buf, words<<1);
+	int i;
+	for (i = 0; i < (words << 1); i++)
+		writew(sect_buf[i], ATA_CURR_BASE(dev)+ATA_DATA_REG);
And here the code size is growing, too.

Possibly, I was unsure if there is some implementation of writesw/readsw elsewhere then on PPC.

Quoted 2 lines

What are the exact advantages of your version?

That it actually compiles and works on other architectures than ppc.

Quoted 4 lines

Best regards,
Wolfgang Denk

Cheers

Marek Vasut

author

2010-07-01 00:09 UTC #3 lore raw

PATCH 1/1 cmd_ide: Convert to [read,write][b,w]

Dne ?t 24. ?ervna 2010 02:36:00 Marek Vasut napsal(a):

Quoted 4 lines

Dne St 23. ?ervna 2010 22:51:28 Wolfgang Denk napsal(a):
Quoted 3 lines
Dear Marek Vasut,
In message <[email protected]> you

Quoted 50 lines · wrote:

Quoted 7 lines
Quoted 5 lines
Signed-off-by Marek Vasut <[email protected]>
---
 common/cmd_ide.c |   22 +++++++++++++---------
 1 files changed, 13 insertions(+), 9 deletions(-)
I don't see the big advantage of this patch yet.
It won't compile at least on ARM. Same case as with the dm9000 ethernet adapter.
Quoted 9 lines
Review __ide_outb(int dev, int port, unsigned char val) · line 531
Quoted 9 lines
#ifndef CONFIG_SYS_ATA_PORT_ADDR
(quoted diff) +0 -0
@@ -526,7 +526,7 @@ __ide_outb(int dev, int port, unsigned char val)
 
 {
 
 	debug ("ide_outb (dev= %d, port= 0x%x, val= 0x%02x) : @ 0x%08lx\n",
 
 		dev, port, val,
(ATA_CURR_BASE(dev)+CONFIG_SYS_ATA_PORT_ADDR(port)));
Quoted 7 lines
common/cmd_ide.c __ide_outb(int dev, int port, unsigned char val) · line 529
Quoted 2 lines
common/cmd_ide.c +1 -1
-	outb(val, (ATA_CURR_BASE(dev)+CONFIG_SYS_ATA_PORT_ADDR(port)));
+	writeb(val, (ATA_CURR_BASE(dev)+CONFIG_SYS_ATA_PORT_ADDR(port)));
So we replace outb() => writeb(), but the function where this is used is still __ide_outb() ?
The __ide_outb() => outb() mapping looks more logical to me.
See above.
Quoted 11 lines
common/cmd_ide.c output_data(int dev, ulong *sect_buf, int words) · line 944
Quoted 9 lines
common/cmd_ide.c +4 -2
-output_data(int dev, ulong *sect_buf, int words)
+output_data(int dev, uint16_t *sect_buf, int words)
 
 {
 
-	outsw(ATA_CURR_BASE(dev)+ATA_DATA_REG, sect_buf, words<<1);
+	int i;
+	for (i = 0; i < (words << 1); i++)
+		writew(sect_buf[i], ATA_CURR_BASE(dev)+ATA_DATA_REG);
And here the code size is growing, too.
Possibly, I was unsure if there is some implementation of writesw/readsw elsewhere then on PPC.
Quoted 1 line
What are the exact advantages of your version?

Quoted 7 lines

That it actually compiles and works on other architectures than ppc.
Quoted 3 lines
Best regards,
Wolfgang Denk
Cheers

Any updates ?

Albert ARIBAUD

2010-07-01 00:44 UTC #4 lore raw

PATCH 1/1 cmd_ide: Convert to [read,write][b,w]

Le 01/07/2010 02:09, Marek Vasut a ?crit :

Quoted 5 lines

Quoted 5 lines
Quoted 4 lines
Quoted 2 lines
  common/cmd_ide.c |   22 +++++++++++++---------
  1 files changed, 13 insertions(+), 9 deletions(-)
I don't see the big advantage of this patch yet.

Quoted 8 lines

Quoted 6 lines
It won't compile at least on ARM. Same case as with the dm9000 ethernet adapter.
Quoted 1 line
What are the exact advantages of your version?
That it actually compiles and works on other architectures than ppc.
Any updates ?

Actually I've compiled cmd_ide.c for ARM -- I just sent a patchset for this. Required me to #define __io in my board's config, and also to #define CONFIG_IDE_SWAP_IO, a new configuration option which replaces the complex conditionals on __PPC__ et al.

Amicalement,

Signature · 1 line

Albert.

Marek Vasut

author

2010-07-01 03:33 UTC #5 lore raw

PATCH 1/1 cmd_ide: Convert to [read,write][b,w]

Dne ?t 1. ?ervence 2010 02:44:01 Albert ARIBAUD napsal(a):

Quoted 9 lines

Le 01/07/2010 02:09, Marek Vasut a ?crit :
Quoted 8 lines
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
  common/cmd_ide.c |   22 +++++++++++++---------
  1 files changed, 13 insertions(+), 9 deletions(-)
I don't see the big advantage of this patch yet.
It won't compile at least on ARM. Same case as with the dm9000 ethernet adapter.

Quoted 8 lines

Quoted 5 lines
Quoted 3 lines
Quoted 1 line
What are the exact advantages of your version?
That it actually compiles and works on other architectures than ppc.
Any updates ?
Actually I've compiled cmd_ide.c for ARM -- I just sent a patchset for this. Required me to #define __io in my board's config,

Are you really supposed to #define __io in your config ?! I doubt that.

Quoted 5 lines

and also to
#define CONFIG_IDE_SWAP_IO, a new configuration option which replaces the complex conditionals on __PPC__ et al.
Amicalement,

Marek Vasut

author

2010-07-21 14:42 UTC #6 lore raw

PATCH 1/1 cmd_ide: Convert to [read,write][b,w]

Dne ?t 1. ?ervence 2010 05:33:46 Marek Vasut napsal(a):

Quoted 12 lines

Dne ?t 1. ?ervence 2010 02:44:01 Albert ARIBAUD napsal(a):
Quoted 11 lines
Le 01/07/2010 02:09, Marek Vasut a ?crit :
Quoted 10 lines
Quoted 10 lines
Quoted 4 lines
Quoted 2 lines
  common/cmd_ide.c |   22 +++++++++++++---------
  1 files changed, 13 insertions(+), 9 deletions(-)
I don't see the big advantage of this patch yet.
It won't compile at least on ARM. Same case as with the dm9000 ethernet adapter.
Quoted 1 line
What are the exact advantages of your version?

Quoted 8 lines

Quoted 6 lines
Quoted 3 lines
Quoted 1 line
That it actually compiles and works on other architectures than ppc.
Any updates ?
Actually I've compiled cmd_ide.c for ARM -- I just sent a patchset for this. Required me to #define __io in my board's config,
Are you really supposed to #define __io in your config ?! I doubt that.

Bump? After some discussion with Wolfgang, we figured that'd be the easiest temporary solution actually. But after I tried this approach, I git this problem:

u-boot-pxa/arch/arm/lib/eabi_compat.o -L /usr/lib/gcc/arm-linux-gnueabi/4.4.4 - lgcc -Map u-boot.map -o u-boot
common/libcommon.a(cmd_ide.o): In function `output_data':
/home/marex/PalmLD/u-boot-pxa/common/cmd_ide.c:904: undefined reference to `__raw_writesw'
common/libcommon.a(cmd_ide.o): In function `input_data':
/home/marex/PalmLD/u-boot-pxa/common/cmd_ide.c:962: undefined reference to `__raw_readsw'
/home/marex/PalmLD/u-boot-pxa/common/cmd_ide.c:962: undefined reference to `__raw_readsw'
make: *** [u-boot] Error 1

So arm is missing the implementation of __raw_reads[bwl] etc.

Quoted 6 lines

Quoted 6 lines
and also to
#define CONFIG_IDE_SWAP_IO, a new configuration option which replaces the complex conditionals on __PPC__ et al.
Amicalement,