| Message ID | 20100530122213.GB1146@host-a-55.ustcsz.edu.cn |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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." -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. */
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(-) -- -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html