Patchwork [U-Boot] net: ftmac100: remove unncessary volatiles

login
register
mail settings
Submitter Po-Yu Chuang
Date Jan. 20, 2011, 3:23 p.m.
Message ID <1295537003-1789-1-git-send-email-ratbert.chuang@gmail.com>
Download mbox | patch
Permalink /patch/79702/
State Superseded
Headers show

Comments

Po-Yu Chuang - Jan. 20, 2011, 3:23 p.m.
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(-)
Sergei Shtylyov - Jan. 20, 2011, 3:38 p.m.
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
Po-Yu Chuang - Jan. 21, 2011, 6:26 a.m.
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

Patch

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;
 		}