[Patch]8139too: remove unnecessary cast of ioread32()'s return value

[Patch]8139too: remove unnecessary cast of ioread32()'s return value

[Junchang Wang]

ioread32() returns a 32-bit integer on all platforms.
There is no need to cast its return value.

Signed-off-by: Junchang Wang <junchangwang@gmail.com>
---
 drivers/net/8139too.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/8139too.c b/drivers/net/8139too.c
index 4ba7293..cc7d462 100644
--- a/drivers/net/8139too.c
+++ b/drivers/net/8139too.c
@@ -662,7 +662,7 @@ static const struct ethtool_ops rtl8139_ethtool_ops;
 /* read MMIO register */
 #define RTL_R8(reg)		ioread8 (ioaddr + (reg))
 #define RTL_R16(reg)		ioread16 (ioaddr + (reg))
-#define RTL_R32(reg)		((unsigned long) ioread32 (ioaddr + (reg)))
+#define RTL_R32(reg)		ioread32 (ioaddr + (reg))
 
 
 static const u16 rtl8139_intr_mask =
@@ -861,7 +861,7 @@ retry:
 
 	/* if unknown chip, assume array element #0, original RTL-8139 in this case */
 	dev_dbg(&pdev->dev, "unknown chip version, assuming RTL-8139\n");
-	dev_dbg(&pdev->dev, "TxConfig = 0x%lx\n", RTL_R32 (TxConfig));
+	dev_dbg(&pdev->dev, "TxConfig = 0x%x\n", RTL_R32 (TxConfig));
 	tp->chipset = 0;
 
 match:
@@ -1642,7 +1642,7 @@ static void rtl8139_tx_timeout_task (struct work_struct *work)
 	netdev_dbg(dev, "Tx queue start entry %ld  dirty entry %ld\n",
 		   tp->cur_tx, tp->dirty_tx);
 	for (i = 0; i < NUM_TX_DESC; i++)
-		netdev_dbg(dev, "Tx descriptor %d is %08lx%s\n",
+		netdev_dbg(dev, "Tx descriptor %d is %08x%s\n",
 			   i, RTL_R32(TxStatus0 + (i * 4)),
 			   i == tp->dirty_tx % NUM_TX_DESC ?
 			   " (queue head)" : "");
@@ -2486,7 +2486,7 @@ static void __set_rx_mode (struct net_device *dev)
 	int rx_mode;
 	u32 tmp;
 
-	netdev_dbg(dev, "rtl8139_set_rx_mode(%04x) done -- Rx config %08lx\n",
+	netdev_dbg(dev, "rtl8139_set_rx_mode(%04x) done -- Rx config %08x\n",
 		   dev->flags, RTL_R32(RxConfig));
 
 	/* Note: do not reorder, GCC is clever about common statements. */
--

Re: [Patch]8139too: remove unnecessary cast of ioread32()'s return value

[Jeff Garzik]

On 05/30/2010 08:22 AM, Junchang Wang wrote:
> ioread32() returns a 32-bit integer on all platforms.
> There is no need to cast its return value.
>
> Signed-off-by: Junchang Wang<junchangwang@gmail.com>
> ---
>   drivers/net/8139too.c |    8 ++++----
>   1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/8139too.c b/drivers/net/8139too.c
> index 4ba7293..cc7d462 100644
> --- a/drivers/net/8139too.c
> +++ b/drivers/net/8139too.c
> @@ -662,7 +662,7 @@ static const struct ethtool_ops rtl8139_ethtool_ops;
>   /* read MMIO register */
>   #define RTL_R8(reg)		ioread8 (ioaddr + (reg))
>   #define RTL_R16(reg)		ioread16 (ioaddr + (reg))
> -#define RTL_R32(reg)		((unsigned long) ioread32 (ioaddr + (reg)))
> +#define RTL_R32(reg)		ioread32 (ioaddr + (reg))

Have you verified this matches all architectures definition of readl()?

	Jeff

Re: [Patch]8139too: remove unnecessary cast of ioread32()'s return value

[David Miller]

From: Jeff Garzik <jeff@garzik.org>
Date: Sun, 30 May 2010 13:35:51 -0400

> Have you verified this matches all architectures definition of
> readl()?

Jeff, when you come out of hiding after months if not years
of not reviewing network driver changes, could you provide
some useful commentary instead of some trite stuff like this?

It does match, that's why I told this person to write these patches.
And if you have been following the thread where we discussed this, you
wouldn't feel the need to ask this question about these two patches.

And if it doesn't match, that's an arch bug which should be fixed and
in any event there is only one possibility of a non-match and that is
if the routine returns "unsigned long"

Which, surprise surprise Jeff, retains current behavior!

So there is no risk whatsoever possible from this change.

Re: [Patch]8139too: remove unnecessary cast of ioread32()'s return value

[Jeff Garzik]

On 05/30/2010 06:34 PM, David Miller wrote:
> From: Jeff Garzik<jeff@garzik.org>
> Date: Sun, 30 May 2010 13:35:51 -0400
>
>> Have you verified this matches all architectures definition of
>> readl()?

> And if it doesn't match, that's an arch bug which should be fixed and
> in any event there is only one possibility of a non-match and that is
> if the routine returns "unsigned long"

That was the genesis of the question.  Some arches still use unsigned long.

	Jeff

Re: [Patch]8139too: remove unnecessary cast of ioread32()'s return value

[Junchang Wang]

On Sun, May 30, 2010 at 01:35:51PM -0400, Jeff Garzik wrote:
>
>Have you verified this matches all architectures definition of readl()?
>

Hi Jeff,

Thanks for your question. Just browsed the kernel. ioread32() returns either 
unsigned int or u32 in all arches. There is no arch that uses unsigned long 
or something else.

Secondly, There's a bug if an arch returns unsigned long. What happen when 
programmers invoke sizeof(ioread32()) on 64-bit platforms?

--Junchang

Re: [Patch]8139too: remove unnecessary cast of ioread32()'s return value

[David Miller]

From: Jeff Garzik <jeff@garzik.org>
Date: Sun, 30 May 2010 19:24:18 -0400

> That was the genesis of the question.  Some arches still use unsigned
> long.

They are 32-bit.

Re: [Patch]8139too: remove unnecessary cast of ioread32()'s return value

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Sun, 30 May 2010 18:29:48 -0700 (PDT)

> From: Jeff Garzik <jeff@garzik.org>
> Date: Sun, 30 May 2010 19:24:18 -0400
> 
>> That was the genesis of the question.  Some arches still use unsigned
>> long.
> 
> They are 32-bit.

In fact the only two offenders are h8300 and m32r, which are
both 32-bit.

This is really in the realm of "who cares."

Re: [Patch]8139too: remove unnecessary cast of ioread32()'s return value

[David Miller]

From: Junchang Wang <junchangwang@gmail.com>
Date: Sun, 30 May 2010 20:22:14 +0800

> ioread32() returns a 32-bit integer on all platforms.
> There is no need to cast its return value.
> 
> Signed-off-by: Junchang Wang <junchangwang@gmail.com>

Applied.

Re: [Patch]8139too: remove unnecessary cast of ioread32()'s return value

[Jeff Garzik]

On 05/30/2010 09:35 PM, David Miller wrote:
> From: David Miller<davem@davemloft.net>
> Date: Sun, 30 May 2010 18:29:48 -0700 (PDT)
>
>> From: Jeff Garzik<jeff@garzik.org>
>> Date: Sun, 30 May 2010 19:24:18 -0400
>>
>>> That was the genesis of the question.  Some arches still use unsigned
>>> long.
>>
>> They are 32-bit.
>
> In fact the only two offenders are h8300 and m32r, which are
> both 32-bit.

The main interesting one is an ARM sub-arch that supports PCI.


> This is really in the realm of "who cares."

Fair enough.  That answers my question.

	Jeff