[U-Boot-Users] [PATCH] Fix dm9000 receive status and len little endian issue

[U-Boot-Users] [PATCH] Fix dm9000 receive status and len little endian issue

[Tsi-Chung Liew]

From: TsiChung Liew <Tsi-Chung.Liew@freescale.com>

The received status and len was in little endian
format and caused the ethernet unable to proceed
further. Add __le16_to_cpu() in dm9000_rx_status_8/16/32bit().

Signed-off-by: TsiChung Liew <Tsi-Chung.Liew@freescale.com>
---
 drivers/net/dm9000x.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dm9000x.c b/drivers/net/dm9000x.c
index 844fb76..e7365fe 100644
--- a/drivers/net/dm9000x.c
+++ b/drivers/net/dm9000x.c
@@ -214,24 +214,28 @@ static void dm9000_rx_status_32bit(u16 *RxStatus, u16 *RxLen)
 	DM9000_outb(DM9000_MRCMD, DM9000_IO);
 
 	tmpdata = DM9000_inl(DM9000_DATA);
-	*RxStatus = tmpdata;
-	*RxLen = tmpdata >> 16;
+	*RxStatus = __le16_to_cpu(tmpdata);
+	*RxLen = __le16_to_cpu(tmpdata >> 16);
 }
 
 static void dm9000_rx_status_16bit(u16 *RxStatus, u16 *RxLen)
 {
 	DM9000_outb(DM9000_MRCMD, DM9000_IO);
 
-	*RxStatus = DM9000_inw(DM9000_DATA);
-	*RxLen = DM9000_inw(DM9000_DATA);
+	*RxStatus = __le16_to_cpu(DM9000_inw(DM9000_DATA));
+	*RxLen = __le16_to_cpu(DM9000_inw(DM9000_DATA));
 }
 
 static void dm9000_rx_status_8bit(u16 *RxStatus, u16 *RxLen)
 {
 	DM9000_outb(DM9000_MRCMD, DM9000_IO);
 
-	*RxStatus = DM9000_inb(DM9000_DATA) + (DM9000_inb(DM9000_DATA) << 8);
-	*RxLen = DM9000_inb(DM9000_DATA) + (DM9000_inb(DM9000_DATA) << 8);
+	*RxStatus =
+	    __le16_to_cpu(DM9000_inb(DM9000_DATA) +
+			  (DM9000_inb(DM9000_DATA) << 8));
+	*RxLen =
+	    __le16_to_cpu(DM9000_inb(DM9000_DATA) +
+			  (DM9000_inb(DM9000_DATA) << 8));
 }
 
 /*
-- 
1.5.4.1

[U-Boot-Users] [PATCH] Fix dm9000 receive status and len little endian issue

[Ben Warren]

Tsi-Chung Liew wrote:
> From: TsiChung Liew <Tsi-Chung.Liew@freescale.com>
>
> The received status and len was in little endian
> format and caused the ethernet unable to proceed
> further. Add __le16_to_cpu() in dm9000_rx_status_8/16/32bit().
>
> Signed-off-by: TsiChung Liew <Tsi-Chung.Liew@freescale.com>
> ---
>  drivers/net/dm9000x.c |   16 ++++++++++------
>  1 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/dm9000x.c b/drivers/net/dm9000x.c
> index 844fb76..e7365fe 100644
> --- a/drivers/net/dm9000x.c
> +++ b/drivers/net/dm9000x.c
> @@ -214,24 +214,28 @@ static void dm9000_rx_status_32bit(u16 *RxStatus, u16 *RxLen)
>  	DM9000_outb(DM9000_MRCMD, DM9000_IO);
>  
>  	tmpdata = DM9000_inl(DM9000_DATA);
> -	*RxStatus = tmpdata;
> -	*RxLen = tmpdata >> 16;
> +	*RxStatus = __le16_to_cpu(tmpdata);
> +	*RxLen = __le16_to_cpu(tmpdata >> 16);
>  }
>  
>  static void dm9000_rx_status_16bit(u16 *RxStatus, u16 *RxLen)
>  {
>  	DM9000_outb(DM9000_MRCMD, DM9000_IO);
>  
> -	*RxStatus = DM9000_inw(DM9000_DATA);
> -	*RxLen = DM9000_inw(DM9000_DATA);
> +	*RxStatus = __le16_to_cpu(DM9000_inw(DM9000_DATA));
> +	*RxLen = __le16_to_cpu(DM9000_inw(DM9000_DATA));
>  }
>  
>  static void dm9000_rx_status_8bit(u16 *RxStatus, u16 *RxLen)
>  {
>  	DM9000_outb(DM9000_MRCMD, DM9000_IO);
>  
> -	*RxStatus = DM9000_inb(DM9000_DATA) + (DM9000_inb(DM9000_DATA) << 8);
> -	*RxLen = DM9000_inb(DM9000_DATA) + (DM9000_inb(DM9000_DATA) << 8);
> +	*RxStatus =
> +	    __le16_to_cpu(DM9000_inb(DM9000_DATA) +
> +			  (DM9000_inb(DM9000_DATA) << 8));
> +	*RxLen =
> +	    __le16_to_cpu(DM9000_inb(DM9000_DATA) +
> +			  (DM9000_inb(DM9000_DATA) << 8));
>  }
>  
>  /*
>   
Sorry for opening Pandora's box, but wouldn't it be better to redefine 
the DM9000_x macros instead of where they're called?

regards,
Ben

[U-Boot-Users] [PATCH] Fix dm9000 receive status and len little endian issue

[Liew Tsi Chung]

Ben,

	The data (packet data) and the status/len are different in
LE/BE, cannot use just the macro dm9000_x to cover both. In my case,
packet data is already in BE, but the status/len is in LE; therefore,
status/len require byteswap. This has to do with cpu endian mode.

Regards,
TsiChung

-----Original Message-----
From: Ben Warren [mailto:biggerbadderben at gmail.com] 
Sent: Wednesday, June 25, 2008 4:01 PM
To: Liew Tsi Chung
Cc: U-Boot-Users; Rigby John
Subject: Re: [PATCH] Fix dm9000 receive status and len little endian
issue

Tsi-Chung Liew wrote:
> From: TsiChung Liew <Tsi-Chung.Liew@freescale.com>
>
> The received status and len was in little endian format and caused the

> ethernet unable to proceed further. Add __le16_to_cpu() in 
> dm9000_rx_status_8/16/32bit().
>
> Signed-off-by: TsiChung Liew <Tsi-Chung.Liew@freescale.com>
> ---
>  drivers/net/dm9000x.c |   16 ++++++++++------
>  1 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/dm9000x.c b/drivers/net/dm9000x.c index 
> 844fb76..e7365fe 100644
> --- a/drivers/net/dm9000x.c
> +++ b/drivers/net/dm9000x.c
> @@ -214,24 +214,28 @@ static void dm9000_rx_status_32bit(u16
*RxStatus, u16 *RxLen)
>  	DM9000_outb(DM9000_MRCMD, DM9000_IO);
>  
>  	tmpdata = DM9000_inl(DM9000_DATA);
> -	*RxStatus = tmpdata;
> -	*RxLen = tmpdata >> 16;
> +	*RxStatus = __le16_to_cpu(tmpdata);
> +	*RxLen = __le16_to_cpu(tmpdata >> 16);
>  }
>  
>  static void dm9000_rx_status_16bit(u16 *RxStatus, u16 *RxLen)  {
>  	DM9000_outb(DM9000_MRCMD, DM9000_IO);
>  
> -	*RxStatus = DM9000_inw(DM9000_DATA);
> -	*RxLen = DM9000_inw(DM9000_DATA);
> +	*RxStatus = __le16_to_cpu(DM9000_inw(DM9000_DATA));
> +	*RxLen = __le16_to_cpu(DM9000_inw(DM9000_DATA));
>  }
>  
>  static void dm9000_rx_status_8bit(u16 *RxStatus, u16 *RxLen)  {
>  	DM9000_outb(DM9000_MRCMD, DM9000_IO);
>  
> -	*RxStatus = DM9000_inb(DM9000_DATA) + (DM9000_inb(DM9000_DATA)
<< 8);
> -	*RxLen = DM9000_inb(DM9000_DATA) + (DM9000_inb(DM9000_DATA) <<
8);
> +	*RxStatus =
> +	    __le16_to_cpu(DM9000_inb(DM9000_DATA) +
> +			  (DM9000_inb(DM9000_DATA) << 8));
> +	*RxLen =
> +	    __le16_to_cpu(DM9000_inb(DM9000_DATA) +
> +			  (DM9000_inb(DM9000_DATA) << 8));
>  }
>  
>  /*
>   
Sorry for opening Pandora's box, but wouldn't it be better to redefine
the DM9000_x macros instead of where they're called?

regards,
Ben

[U-Boot-Users] [PATCH] Fix dm9000 receive status and len little endian issue

[Liew Tsi Chung]

Ben,

May I know the status on this patch?

Regards,
TsiChung 

[U-Boot-Users] [PATCH] Fix dm9000 receive status and len little endian issue

[Ben Warren]

Liew Tsi Chung wrote:
> Ben,
>
> May I know the status on this patch?
>
> Regards,
> TsiChung 
>
>   
Sorry, I'll address it tonight.

regards,
Ben

[U-Boot-Users] [PATCH] Fix dm9000 receive status and len little endian issue

[Ben Warren]

Tsi-Chung Liew wrote:
> From: TsiChung Liew <Tsi-Chung.Liew@freescale.com>
> z
> The received status and len was in little endian
> format and caused the ethernet unable to proceed
> further. Add __le16_to_cpu() in dm9000_rx_status_8/16/32bit().
>
> Signed-off-by: TsiChung Liew <Tsi-Chung.Liew@freescale.com>
>   
Applied to 'next' branch of net repo.

thanks,
Ben

[U-Boot-Users] [PATCH] Fix dm9000 receive status and len little endian issue

[Tsi-Chung Liew]

From: TsiChung Liew <Tsi-Chung.Liew@freescale.com>

The received status and len was in little endian
format and caused the ethernet unable to proceed
further. Add __le16_to_cpu() in dm9000_rx_status_16bit().

Signed-off-by: TsiChung Liew <Tsi-Chung.Liew@freescale.com>
---
 drivers/net/dm9000x.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dm9000x.c b/drivers/net/dm9000x.c
index 844fb76..2895344 100644
--- a/drivers/net/dm9000x.c
+++ b/drivers/net/dm9000x.c
@@ -222,8 +222,8 @@ static void dm9000_rx_status_16bit(u16 *RxStatus, u16 *RxLen)
 {
 	DM9000_outb(DM9000_MRCMD, DM9000_IO);
 
-	*RxStatus = DM9000_inw(DM9000_DATA);
-	*RxLen = DM9000_inw(DM9000_DATA);
+	*RxStatus = __le16_to_cpu(DM9000_inw(DM9000_DATA));
+	*RxLen = __le16_to_cpu(DM9000_inw(DM9000_DATA));
 }
 
 static void dm9000_rx_status_8bit(u16 *RxStatus, u16 *RxLen)
-- 
1.5.4.1

[U-Boot-Users] [PATCH] Fix dm9000 receive status and len little endian issue

[Remy Bohmer]

Hello Tsi-Chung,

> The received status and len was in little endian
> format and caused the ethernet unable to proceed
> further. Add __le16_to_cpu() in dm9000_rx_status_16bit().

I can imagine that similar problems are also valid for the 32bit code
(and maybe the 8 bit code too)
It is probably better to change those other 2 routines too, to keep
them all behave the same.
Do you provide a patch for those routines too? (if required)

Kind Regards,

Remy

>
> Signed-off-by: TsiChung Liew <Tsi-Chung.Liew@freescale.com>
> ---
>  drivers/net/dm9000x.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/dm9000x.c b/drivers/net/dm9000x.c
> index 844fb76..2895344 100644
> --- a/drivers/net/dm9000x.c
> +++ b/drivers/net/dm9000x.c
> @@ -222,8 +222,8 @@ static void dm9000_rx_status_16bit(u16 *RxStatus, u16 *RxLen)
>  {
>        DM9000_outb(DM9000_MRCMD, DM9000_IO);
>
> -       *RxStatus = DM9000_inw(DM9000_DATA);
> -       *RxLen = DM9000_inw(DM9000_DATA);
> +       *RxStatus = __le16_to_cpu(DM9000_inw(DM9000_DATA));
> +       *RxLen = __le16_to_cpu(DM9000_inw(DM9000_DATA));
>  }
>
>  static void dm9000_rx_status_8bit(u16 *RxStatus, u16 *RxLen)
> --
> 1.5.4.1
>
>
> -------------------------------------------------------------------------
> Check out the new SourceForge.net Marketplace.
> It's the best place to buy or sell services for
> just about anything Open Source.
> http://sourceforge.net/services/buy/index.php
> _______________________________________________
> U-Boot-Users mailing list
> U-Boot-Users at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/u-boot-users
>

[U-Boot-Users] [PATCH] Fix dm9000 receive status and len little endian issue

[Liew Tsi Chung]

Sure.

> Do you provide a patch for those routines too? (if required)
Just the dm9000_rx_status_16bit(). Will re-submit with 8 and 32.

Regards,
TsiChung


-----Original Message-----
From: l.pinguin@gmail.com [mailto:l.pinguin at gmail.com] On Behalf Of Remy
Bohmer
Sent: Wednesday, June 25, 2008 2:50 PM
To: Liew Tsi Chung
Cc: U-Boot-Users; Rigby John; Ben Warren
Subject: Re: [U-Boot-Users] [PATCH] Fix dm9000 receive status and len
little endian issue

Hello Tsi-Chung,

> The received status and len was in little endian format and caused the

> ethernet unable to proceed further. Add __le16_to_cpu() in 
> dm9000_rx_status_16bit().

I can imagine that similar problems are also valid for the 32bit code
(and maybe the 8 bit code too) It is probably better to change those
other 2 routines too, to keep them all behave the same.
Do you provide a patch for those routines too? (if required)

Kind Regards,

Remy

>
> Signed-off-by: TsiChung Liew <Tsi-Chung.Liew@freescale.com>
> ---
>  drivers/net/dm9000x.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/dm9000x.c b/drivers/net/dm9000x.c index 
> 844fb76..2895344 100644
> --- a/drivers/net/dm9000x.c
> +++ b/drivers/net/dm9000x.c
> @@ -222,8 +222,8 @@ static void dm9000_rx_status_16bit(u16 *RxStatus, 
> u16 *RxLen)  {
>        DM9000_outb(DM9000_MRCMD, DM9000_IO);
>
> -       *RxStatus = DM9000_inw(DM9000_DATA);
> -       *RxLen = DM9000_inw(DM9000_DATA);
> +       *RxStatus = __le16_to_cpu(DM9000_inw(DM9000_DATA));
> +       *RxLen = __le16_to_cpu(DM9000_inw(DM9000_DATA));
>  }
>
>  static void dm9000_rx_status_8bit(u16 *RxStatus, u16 *RxLen)
> --
> 1.5.4.1
>
>
> ----------------------------------------------------------------------
> --- Check out the new SourceForge.net Marketplace.
> It's the best place to buy or sell services for just about anything 
> Open Source.
> http://sourceforge.net/services/buy/index.php
> _______________________________________________
> U-Boot-Users mailing list
> U-Boot-Users at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/u-boot-users
>

[U-Boot-Users] [PATCH] Fix dm9000 receive status and len little endian issue

[Tsi-Chung Liew]

From: TsiChung Liew <Tsi-Chung.Liew@freescale.com>

The received status and len was in little endian
format and caused the ethernet unable to proceed
further. Add define CFG_DM9000_BYTESWAP_STATUS_LEN in
dm9000_rx_status_16bit() to byte swap RxStatus and RxLen

Signed-off-by: TsiChung Liew <Tsi-Chung.Liew@freescale.com>
---
 drivers/net/dm9000x.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/net/dm9000x.c b/drivers/net/dm9000x.c
index 844fb76..eff676c 100644
--- a/drivers/net/dm9000x.c
+++ b/drivers/net/dm9000x.c
@@ -224,6 +224,10 @@ static void dm9000_rx_status_16bit(u16 *RxStatus, u16 *RxLen)
 
 	*RxStatus = DM9000_inw(DM9000_DATA);
 	*RxLen = DM9000_inw(DM9000_DATA);
+#ifdef CFG_DM9000_BYTESWAP_STATUS_LEN
+	*RxStatus = __sw16(*RxStatus);
+	*RxLen = __sw16(*RxLen);
+#endif
 }
 
 static void dm9000_rx_status_8bit(u16 *RxStatus, u16 *RxLen)
-- 
1.5.4.1

[U-Boot-Users] [PATCH] Fix dm9000 receive status and len little endian issue

[Ben Warren]

Hi Tsi-Chung,

Tsi-Chung Liew wrote:
> From: TsiChung Liew <Tsi-Chung.Liew@freescale.com>
>
> The received status and len was in little endian
> format and caused the ethernet unable to proceed
> further. Add define CFG_DM9000_BYTESWAP_STATUS_LEN in
> dm9000_rx_status_16bit() to byte swap RxStatus and RxLen
>
> Signed-off-by: TsiChung Liew <Tsi-Chung.Liew@freescale.com>
> ---
>  drivers/net/dm9000x.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/dm9000x.c b/drivers/net/dm9000x.c
> index 844fb76..eff676c 100644
> --- a/drivers/net/dm9000x.c
> +++ b/drivers/net/dm9000x.c
> @@ -224,6 +224,10 @@ static void dm9000_rx_status_16bit(u16 *RxStatus, u16 *RxLen)
>  
>  	*RxStatus = DM9000_inw(DM9000_DATA);
>  	*RxLen = DM9000_inw(DM9000_DATA);
> +#ifdef CFG_DM9000_BYTESWAP_STATUS_LEN
> +	*RxStatus = __sw16(*RxStatus);
> +	*RxLen = __sw16(*RxLen);
> +#endif
>  }
>  
Instead of adding yet another kludgy CONFIG option, please consider 
changing the accessors to le16_to_cpu() or whatever is appropriate.  The 
definitions of DM9000_inX() are not very portable as it is and we should 
be striving to make the code better, not worse.

regards,
Ben