public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] net: ftmac100: remove unncessary volatiles
@ 2011-01-20 15:23 Po-Yu Chuang
  2011-01-20 15:38 ` Sergei Shtylyov
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Po-Yu Chuang @ 2011-01-20 15:23 UTC (permalink / raw)
  To: u-boot

From: Po-Yu Chuang <ratbert@faraday-tech.com>

This patch also update get_timer() usage.

Signed-off-by: Po-Yu Chuang <ratbert@faraday-tech.com>
---
 drivers/net/ftmac100.c |   20 +++++++++-----------
 1 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ftmac100.c b/drivers/net/ftmac100.c
index 2328cb5..787d69b 100644
--- a/drivers/net/ftmac100.c
+++ b/drivers/net/ftmac100.c
@@ -30,8 +30,8 @@
 #define ETH_ZLEN	60
 
 struct ftmac100_data {
-	volatile struct ftmac100_txdes txdes[1];
-	volatile struct ftmac100_rxdes rxdes[PKTBUFSRX];
+	struct ftmac100_txdes txdes[1];
+	struct ftmac100_rxdes rxdes[PKTBUFSRX];
 	int rx_index;
 };
 
@@ -88,8 +88,8 @@ static int ftmac100_init (struct eth_device *dev, bd_t *bd)
 {
 	struct ftmac100 *ftmac100 = (struct ftmac100 *)dev->iobase;
 	struct ftmac100_data *priv = dev->priv;
-	volatile struct ftmac100_txdes *txdes = priv->txdes;
-	volatile struct ftmac100_rxdes *rxdes = priv->rxdes;
+	struct ftmac100_txdes *txdes = priv->txdes;
+	struct ftmac100_rxdes *rxdes = priv->rxdes;
 	unsigned int maccr;
 	int i;
 
@@ -153,7 +153,7 @@ static int ftmac100_init (struct eth_device *dev, bd_t *bd)
 static int ftmac100_recv (struct eth_device *dev)
 {
 	struct ftmac100_data *priv = dev->priv;
-	volatile struct ftmac100_rxdes *curr_des;
+	struct ftmac100_rxdes *curr_des;
 	unsigned short rxlen;
 
 	curr_des = &priv->rxdes[priv->rx_index];
@@ -195,8 +195,8 @@ ftmac100_send (struct eth_device *dev, volatile void *packet, int length)
 {
 	struct ftmac100 *ftmac100 = (struct ftmac100 *)dev->iobase;
 	struct ftmac100_data *priv = dev->priv;
-	volatile struct ftmac100_txdes *curr_des = priv->txdes;
-	int tmo;
+	struct ftmac100_txdes *curr_des = priv->txdes;
+	ulong start;
 
 	if (curr_des->txdes0 & FTMAC100_TXDES0_TXDMA_OWN) {
 		debug ("%s(): no TX descriptor available\n", __func__);
@@ -219,14 +219,12 @@ ftmac100_send (struct eth_device *dev, volatile void *packet, int length)
 	curr_des->txdes0 = FTMAC100_TXDES0_TXDMA_OWN;
 
 	/* start transmit */
-
 	writel (1, &ftmac100->txpd);
 
 	/* wait for transfer to succeed */
-
-	tmo = get_timer (0) + 5 * CONFIG_SYS_HZ;
+	start = get_timer (0);
 	while (curr_des->txdes0 & FTMAC100_TXDES0_TXDMA_OWN) {
-		if (get_timer (0) >= tmo) {
+		if (get_timer (start) >= 5) {
 			debug ("%s(): timed out\n", __func__);
 			return -1;
 		}
-- 
1.6.3.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH] net: ftmac100: remove unncessary volatiles
  2011-01-20 15:23 [U-Boot] [PATCH] net: ftmac100: remove unncessary volatiles Po-Yu Chuang
@ 2011-01-20 15:38 ` Sergei Shtylyov
  2011-01-21  6:26   ` Po-Yu Chuang
  2011-01-21  7:50 ` [U-Boot] [PATCH v2 1/2] " Po-Yu Chuang
  2011-01-21  7:51 ` [U-Boot] [PATCH v2 2/2] net: ftmac100: update get_timer() usages Po-Yu Chuang
  2 siblings, 1 reply; 9+ messages in thread
From: Sergei Shtylyov @ 2011-01-20 15:38 UTC (permalink / raw)
  To: u-boot

Hello.

Po-Yu Chuang wrote:

> From: Po-Yu Chuang <ratbert@faraday-tech.com>

> This patch also update get_timer() usage.

    This seems like a material for a separate patch.

> Signed-off-by: Po-Yu Chuang <ratbert@faraday-tech.com>
> ---
>  drivers/net/ftmac100.c |   20 +++++++++-----------
>  1 files changed, 9 insertions(+), 11 deletions(-)

> diff --git a/drivers/net/ftmac100.c b/drivers/net/ftmac100.c
> index 2328cb5..787d69b 100644
> --- a/drivers/net/ftmac100.c
> +++ b/drivers/net/ftmac100.c
[...]
> @@ -219,14 +219,12 @@ ftmac100_send (struct eth_device *dev, volatile void *packet, int length)
>  	curr_des->txdes0 = FTMAC100_TXDES0_TXDMA_OWN;
>  
>  	/* start transmit */
> -
>  	writel (1, &ftmac100->txpd);
>  
>  	/* wait for transfer to succeed */
> -
> -	tmo = get_timer (0) + 5 * CONFIG_SYS_HZ;
> +	start = get_timer (0);
>  	while (curr_des->txdes0 & FTMAC100_TXDES0_TXDMA_OWN) {
> -		if (get_timer (0) >= tmo) {
> +		if (get_timer (start) >= 5) {

    I'm not sure this is equivalent to the old code...

>  			debug ("%s(): timed out\n", __func__);
>  			return -1;
>  		}

    checkpatch.pl says:

WARNING: space prohibited between function name and open parenthesis '('
#66: FILE: drivers/net/ftmac100.c:225:
+	start = get_timer (0);

WARNING: space prohibited between function name and open parenthesis '('
#69: FILE: drivers/net/ftmac100.c:227:
+		if (get_timer (start) >= 5) {

    I know you're only modifying the existing code, but it's time to fix the 
style of it as well...

WBR, Sergei

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH] net: ftmac100: remove unncessary volatiles
  2011-01-20 15:38 ` Sergei Shtylyov
@ 2011-01-21  6:26   ` Po-Yu Chuang
  0 siblings, 0 replies; 9+ messages in thread
From: Po-Yu Chuang @ 2011-01-21  6:26 UTC (permalink / raw)
  To: u-boot

Hi Sergei,

On Thu, Jan 20, 2011 at 11:38 PM, Sergei Shtylyov <sshtylyov@mvista.com> wrote:
> Hello.
>
> Po-Yu Chuang wrote:
>
>> From: Po-Yu Chuang <ratbert@faraday-tech.com>
>
>> This patch also update get_timer() usage.
>
> ? This seems like a material for a separate patch.

OK, I will split this to 2 patches later.

> [...]
>>
>> @@ -219,14 +219,12 @@ ftmac100_send (struct eth_device *dev, volatile void
>> *packet, int length)
>> ? ? ? ?curr_des->txdes0 = FTMAC100_TXDES0_TXDMA_OWN;
>> ? ? ? ?/* start transmit */
>> -
>> ? ? ? ?writel (1, &ftmac100->txpd);
>> ? ? ? ?/* wait for transfer to succeed */
>> -
>> - ? ? ? tmo = get_timer (0) + 5 * CONFIG_SYS_HZ;
>> + ? ? ? start = get_timer (0);
>> ? ? ? ?while (curr_des->txdes0 & FTMAC100_TXDES0_TXDMA_OWN) {
>> - ? ? ? ? ? ? ? if (get_timer (0) >= tmo) {
>> + ? ? ? ? ? ? ? if (get_timer (start) >= 5) {
>
> ? I'm not sure this is equivalent to the old code...

The original code waits unnecessarily long. This change refers to the way
the newer driver does - drivers/net/ftgmac100.c.

>> ? ? ? ? ? ? ? ? ? ? ? ?debug ("%s(): timed out\n", __func__);
>> ? ? ? ? ? ? ? ? ? ? ? ?return -1;
>> ? ? ? ? ? ? ? ?}
>
> ? checkpatch.pl says:
>
> WARNING: space prohibited between function name and open parenthesis '('
> #66: FILE: drivers/net/ftmac100.c:225:
> + ? ? ? start = get_timer (0);
>
> WARNING: space prohibited between function name and open parenthesis '('
> #69: FILE: drivers/net/ftmac100.c:227:
> + ? ? ? ? ? ? ? if (get_timer (start) >= 5) {
>
> ? I know you're only modifying the existing code, but it's time to fix the
> style of it as well...

OK, I'll fix it.

Thanks,
Po-Yu Chuang

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH v2 1/2] net: ftmac100: remove unncessary volatiles
  2011-01-20 15:23 [U-Boot] [PATCH] net: ftmac100: remove unncessary volatiles Po-Yu Chuang
  2011-01-20 15:38 ` Sergei Shtylyov
@ 2011-01-21  7:50 ` Po-Yu Chuang
  2011-03-21 21:53   ` Wolfgang Denk
  2011-01-21  7:51 ` [U-Boot] [PATCH v2 2/2] net: ftmac100: update get_timer() usages Po-Yu Chuang
  2 siblings, 1 reply; 9+ messages in thread
From: Po-Yu Chuang @ 2011-01-21  7:50 UTC (permalink / raw)
  To: u-boot

From: Po-Yu Chuang <ratbert@faraday-tech.com>


Signed-off-by: Po-Yu Chuang <ratbert@faraday-tech.com>
---
v2:
split get_timer() changes to a seperate patch

 drivers/net/ftmac100.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ftmac100.c b/drivers/net/ftmac100.c
index 2328cb5..27381a3 100644
--- a/drivers/net/ftmac100.c
+++ b/drivers/net/ftmac100.c
@@ -30,8 +30,8 @@
 #define ETH_ZLEN	60
 
 struct ftmac100_data {
-	volatile struct ftmac100_txdes txdes[1];
-	volatile struct ftmac100_rxdes rxdes[PKTBUFSRX];
+	struct ftmac100_txdes txdes[1];
+	struct ftmac100_rxdes rxdes[PKTBUFSRX];
 	int rx_index;
 };
 
@@ -88,8 +88,8 @@ static int ftmac100_init (struct eth_device *dev, bd_t *bd)
 {
 	struct ftmac100 *ftmac100 = (struct ftmac100 *)dev->iobase;
 	struct ftmac100_data *priv = dev->priv;
-	volatile struct ftmac100_txdes *txdes = priv->txdes;
-	volatile struct ftmac100_rxdes *rxdes = priv->rxdes;
+	struct ftmac100_txdes *txdes = priv->txdes;
+	struct ftmac100_rxdes *rxdes = priv->rxdes;
 	unsigned int maccr;
 	int i;
 
@@ -153,7 +153,7 @@ static int ftmac100_init (struct eth_device *dev, bd_t *bd)
 static int ftmac100_recv (struct eth_device *dev)
 {
 	struct ftmac100_data *priv = dev->priv;
-	volatile struct ftmac100_rxdes *curr_des;
+	struct ftmac100_rxdes *curr_des;
 	unsigned short rxlen;
 
 	curr_des = &priv->rxdes[priv->rx_index];
@@ -195,8 +195,8 @@ ftmac100_send (struct eth_device *dev, volatile void *packet, int length)
 {
 	struct ftmac100 *ftmac100 = (struct ftmac100 *)dev->iobase;
 	struct ftmac100_data *priv = dev->priv;
-	volatile struct ftmac100_txdes *curr_des = priv->txdes;
 	int tmo;
+	struct ftmac100_txdes *curr_des = priv->txdes;
 
 	if (curr_des->txdes0 & FTMAC100_TXDES0_TXDMA_OWN) {
 		debug ("%s(): no TX descriptor available\n", __func__);
-- 
1.6.3.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH v2 2/2] net: ftmac100: update get_timer() usages
  2011-01-20 15:23 [U-Boot] [PATCH] net: ftmac100: remove unncessary volatiles Po-Yu Chuang
  2011-01-20 15:38 ` Sergei Shtylyov
  2011-01-21  7:50 ` [U-Boot] [PATCH v2 1/2] " Po-Yu Chuang
@ 2011-01-21  7:51 ` Po-Yu Chuang
  2011-02-25 11:16   ` Macpaul Lin
  2011-03-21 21:54   ` Wolfgang Denk
  2 siblings, 2 replies; 9+ messages in thread
From: Po-Yu Chuang @ 2011-01-21  7:51 UTC (permalink / raw)
  To: u-boot

From: Po-Yu Chuang <ratbert@faraday-tech.com>

Use get_timer() the same way as drivers/net/ftgmac100.c

Signed-off-by: Po-Yu Chuang <ratbert@faraday-tech.com>
---
v2:
this patch is splitted from "net: ftmac100: remove unncessary volatiles"

 drivers/net/ftmac100.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ftmac100.c b/drivers/net/ftmac100.c
index 27381a3..94dc6d9 100644
--- a/drivers/net/ftmac100.c
+++ b/drivers/net/ftmac100.c
@@ -195,8 +195,8 @@ ftmac100_send (struct eth_device *dev, volatile void *packet, int length)
 {
 	struct ftmac100 *ftmac100 = (struct ftmac100 *)dev->iobase;
 	struct ftmac100_data *priv = dev->priv;
-	int tmo;
 	struct ftmac100_txdes *curr_des = priv->txdes;
+	ulong start;
 
 	if (curr_des->txdes0 & FTMAC100_TXDES0_TXDMA_OWN) {
 		debug ("%s(): no TX descriptor available\n", __func__);
@@ -224,9 +224,9 @@ ftmac100_send (struct eth_device *dev, volatile void *packet, int length)
 
 	/* wait for transfer to succeed */
 
-	tmo = get_timer (0) + 5 * CONFIG_SYS_HZ;
+	start = get_timer(0);
 	while (curr_des->txdes0 & FTMAC100_TXDES0_TXDMA_OWN) {
-		if (get_timer (0) >= tmo) {
+		if (get_timer(start) >= 5) {
 			debug ("%s(): timed out\n", __func__);
 			return -1;
 		}
-- 
1.6.3.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH v2 2/2] net: ftmac100: update get_timer() usages
  2011-01-21  7:51 ` [U-Boot] [PATCH v2 2/2] net: ftmac100: update get_timer() usages Po-Yu Chuang
@ 2011-02-25 11:16   ` Macpaul Lin
  2011-02-25 11:20     ` Macpaul Lin
  2011-03-21 21:54   ` Wolfgang Denk
  1 sibling, 1 reply; 9+ messages in thread
From: Macpaul Lin @ 2011-02-25 11:16 UTC (permalink / raw)
  To: u-boot

Hi Ben,

2011/1/21 Po-Yu Chuang <ratbert.chuang@gmail.com>:
> From: Po-Yu Chuang <ratbert@faraday-tech.com>
>
> Use get_timer() the same way as drivers/net/ftgmac100.c
>
> Signed-off-by: Po-Yu Chuang <ratbert@faraday-tech.com>
> ---
> v2:
> this patch is splitted from "net: ftmac100: remove unncessary volatiles"

Just remind you please remember to review this patch
as http://patchwork.ozlabs.org/patch/79802/ in patchwork.

Reviewed-by: Macpaul Lin <macpaul@gmail.com>
Tested-by: Macpaul Lin <macpaul@gmail.com>

-- 
Best regards,
Macpaul Lin

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH v2 2/2] net: ftmac100: update get_timer() usages
  2011-02-25 11:16   ` Macpaul Lin
@ 2011-02-25 11:20     ` Macpaul Lin
  0 siblings, 0 replies; 9+ messages in thread
From: Macpaul Lin @ 2011-02-25 11:20 UTC (permalink / raw)
  To: u-boot

Hi Ben,

2011/2/25 Macpaul Lin <macpaul@gmail.com>:
> Hi Ben,
>
> 2011/1/21 Po-Yu Chuang <ratbert.chuang@gmail.com>:
>> From: Po-Yu Chuang <ratbert@faraday-tech.com>
>>
>> Use get_timer() the same way as drivers/net/ftgmac100.c
>>
>> Signed-off-by: Po-Yu Chuang <ratbert@faraday-tech.com>
>> ---
>> v2:
>> this patch is splitted from "net: ftmac100: remove unncessary volatiles"
>
> Just remind you please remember to review this patch
> as http://patchwork.ozlabs.org/patch/79802/ in patchwork.
>
> Reviewed-by: Macpaul Lin <macpaul@gmail.com>
> Tested-by: Macpaul Lin <macpaul@gmail.com>
>

Just remind you please "also" remember to review the patch
"net: ftmac100: update get_timer() usages"
as http://patchwork.ozlabs.org/patch/79803/ in patchwork.

I've lost the patch "79803" in my mailbox, however I've downloaded it
from patchwork.

Reviewed-by: Macpaul Lin <macpaul@gmail.com>
Tested-by: Macpaul Lin <macpaul@gmail.com>


-- 
Best regards,
Macpaul Lin

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH v2 1/2] net: ftmac100: remove unncessary volatiles
  2011-01-21  7:50 ` [U-Boot] [PATCH v2 1/2] " Po-Yu Chuang
@ 2011-03-21 21:53   ` Wolfgang Denk
  0 siblings, 0 replies; 9+ messages in thread
From: Wolfgang Denk @ 2011-03-21 21:53 UTC (permalink / raw)
  To: u-boot

Dear Po-Yu Chuang,

In message <1295596237-1658-1-git-send-email-ratbert.chuang@gmail.com> you wrote:
> From: Po-Yu Chuang <ratbert@faraday-tech.com>
> 
> 
> Signed-off-by: Po-Yu Chuang <ratbert@faraday-tech.com>
> ---
> v2:
> split get_timer() changes to a seperate patch
> 
>  drivers/net/ftmac100.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
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
If the car industry behaved like the computer industry over the  last
30  years, a Rolls-Royce would cost $5, get 300 miles per gallon, and
blow up once a year killing all passengers inside.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH v2 2/2] net: ftmac100: update get_timer() usages
  2011-01-21  7:51 ` [U-Boot] [PATCH v2 2/2] net: ftmac100: update get_timer() usages Po-Yu Chuang
  2011-02-25 11:16   ` Macpaul Lin
@ 2011-03-21 21:54   ` Wolfgang Denk
  1 sibling, 0 replies; 9+ messages in thread
From: Wolfgang Denk @ 2011-03-21 21:54 UTC (permalink / raw)
  To: u-boot

Dear Po-Yu Chuang,

In message <1295596285-1690-1-git-send-email-ratbert.chuang@gmail.com> you wrote:
> From: Po-Yu Chuang <ratbert@faraday-tech.com>
> 
> Use get_timer() the same way as drivers/net/ftgmac100.c
> 
> Signed-off-by: Po-Yu Chuang <ratbert@faraday-tech.com>
> ---
> v2:
> this patch is splitted from "net: ftmac100: remove unncessary volatiles"
> 
>  drivers/net/ftmac100.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
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
As usual, this being a 1.3.x release, I haven't  even  compiled  this
kernel yet. So if it works, you should be doubly impressed.
      - Linus Torvalds in <199506181536.SAA10638@keos.cs.Helsinki.FI>

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2011-03-21 21:54 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-20 15:23 [U-Boot] [PATCH] net: ftmac100: remove unncessary volatiles Po-Yu Chuang
2011-01-20 15:38 ` Sergei Shtylyov
2011-01-21  6:26   ` Po-Yu Chuang
2011-01-21  7:50 ` [U-Boot] [PATCH v2 1/2] " Po-Yu Chuang
2011-03-21 21:53   ` Wolfgang Denk
2011-01-21  7:51 ` [U-Boot] [PATCH v2 2/2] net: ftmac100: update get_timer() usages Po-Yu Chuang
2011-02-25 11:16   ` Macpaul Lin
2011-02-25 11:20     ` Macpaul Lin
2011-03-21 21:54   ` Wolfgang Denk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox