[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <AANLkTi=E0hkTUSd-rXWh5tkct0wwYX_75E-oaSWaa2bn@mail.gmail.com>
Date: Wed, 5 Jan 2011 18:51:44 -0500
From: Arnaud Lacombe <lacombar@...il.com>
To: Angelo Dureghello <angelo70@...il.com>
Cc: linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
Randy Dunlap <randy.dunlap@...cle.com>
Subject: Re: [RFC v2 PATCH] m68knommu: added dm9000 support
Hi,
[disclamer: I have no power whatsoever on the dm9k driver, and the
only dm9000 chip I've access to belongs to hardware whose parent
company is now defunct.]
Btw, Linux 68k folks is missing from the CC list.
On Wed, Jan 5, 2011 at 1:03 PM, Angelo Dureghello <angelo70@...il.com> wrote:
> This patch allows to use the dm9000 network chip with a m68knommu
> big-endian cpu. From the HW point of view,
what hardware ?
> the cpu data bus connected to
> the dm9000 chip should be hardware-byte-swapped, crossing the bytes
> wires (D0:7 to D24:31, etc.). In anyway, has been also added an option
> to swap the bytes in the driver, if some cpu has been wired straight
> D0:D31 to dm9000.
>
> Signed-off-by: Angelo Dureghello <angelo70@...il.com>
> ---
>
> --- linux/drivers/net/Kconfig.orig 2011-01-05 17:11:37.992376124 +0100
> +++ linux/drivers/net/Kconfig 2011-01-04 22:33:14.132301872 +0100
> @@ -960,7 +960,7 @@ config TI_DAVINCI_EMAC
>
> config DM9000
> tristate "DM9000 support"
> - depends on ARM || BLACKFIN || MIPS
> + depends on COLDFIRE || ARM || BLACKFIN || MIPS
> select CRC32
> select MII
> ---help---
> @@ -986,6 +986,14 @@ config DM9000_FORCE_SIMPLE_PHY_POLL
> costly MII PHY reads. Note, this will not work if the chip is
> operating with an external PHY.
>
> +config DM9000_32BIT_SW_SWAP
> + bool "Software byte swap for 32 bit data bus"
> + depends on DM9000 && COLDFIRE
> + ---help---
> + This configuration allows to swap data bytes from the dm9000
> + driver itself, when the big endian cpu is wired straight to
> + the dm9000 32 bit data bus.
> +
I'm not sure COLDFIRE is the best dependency. AFAICS, it should
depends on endianness, and potentially board specific settings.
> config ENC28J60
> tristate "ENC28J60 support"
> depends on EXPERIMENTAL && SPI && NET_ETHERNET
> @@ -3347,4 +3355,3 @@ config VMXNET3
> module will be called vmxnet3.
>
> endif # NETDEVICES
> -
>
useless whitespace change ...
> --- linux/drivers/net/dm9000.c.orig 2010-12-30 23:19:39.747836070 +0100
> +++ linux/drivers/net/dm9000.c 2011-01-05 16:30:48.636116500 +0100
> [...]
> @@ -981,7 +1032,11 @@ dm9000_rx(struct net_device *dev)
> ior(db, DM9000_MRCMDX); /* Dummy read */
>
> /* Get most updated data */
> - rxbyte = readb(db->io_data);
> +#ifdef CONFIG_DM9000_32BIT_SW_SWAP
> + rxbyte = (u8)readl(db->io_data);
> +#else
> + rxbyte = readb(db->io_data);
> +#endif
>
> /* Status check: this byte must be 0 or 1 */
> if (rxbyte & DM9000_PKT_ERR) {
> @@ -996,8 +1051,13 @@ dm9000_rx(struct net_device *dev)
>
> /* A packet ready now & Get status/length */
> GoodPacket = true;
> - writeb(DM9000_MRCMD, db->io_addr);
>
> +#ifdef CONFIG_DM9000_32BIT_SW_SWAP
> + writel(DM9000_MRCMD, db->io_addr);
> +#else
> + writeb(DM9000_MRCMD, db->io_addr);
> +#endif
> +
All these #ifdef make the driver quite horrible to read.
> (db->inblk)(db->io_data, &rxhdr, sizeof(rxhdr));
>
> RxLen = le16_to_cpu(rxhdr.RxLen);
> @@ -1077,7 +1137,7 @@ static irqreturn_t dm9000_interrupt(int
> unsigned long flags;
> u8 reg_save;
>
> - dm9000_dbg(db, 3, "entering %s\n", __func__);
> + //dm9000_dbg(db, 3, "entering %s\n", __func__);
>
C++ comment ... Why do you comment this ?
> /* Disable all interrupts */
> iow(db, DM9000_IMR, IMR_PAR);
> @@ -1100,7 +1164,7 @@ static irqreturn_t dm9000_interrupt(int
> /* Received the coming packet */
> if (int_status & ISR_PRS)
> dm9000_rx(dev);
> -
> +
whitespace ...
> @@ -1233,11 +1301,15 @@ dm9000_phy_read(struct net_device *dev,
> int ret;
>
> mutex_lock(&db->addr_lock);
> -
> +
whitespace ...
> @@ -1713,4 +1811,3 @@ MODULE_AUTHOR("Sascha Hauer, Ben Dooks")
> MODULE_DESCRIPTION("Davicom DM9000 network driver");
> MODULE_LICENSE("GPL");
> MODULE_ALIAS("platform:dm9000");
> -
whitespace ...
> --- linux/arch/m68k/include/asm/io_no.h.orig 2011-01-05 16:53:55.904905038 +0100
> +++ linux/arch/m68k/include/asm/io_no.h 2011-01-04 23:45:08.893049554 +0100
> @@ -47,6 +47,91 @@ static inline unsigned int _swapl(volati
> #define writew(b,addr) (void)((*(volatile unsigned short *) (addr)) = (b))
> #define writel(b,addr) (void)((*(volatile unsigned int *) (addr)) = (b))
>
> +static inline void writesb (void __iomem *reg, void *data, int count)
> +{
> + unsigned char *p = (unsigned char*) data;
> +
> + while (count--) writeb(*p++, reg);
> +}
> +
> +static inline void writesbsw (void __iomem *reg, void *data, int count)
> +{
> + unsigned char *p = (unsigned char *) data;
> +
> + while (count--) writel((int)(*p++), reg);
> +}
> +
> +static inline void writesw (void __iomem *reg, void *data, int count)
> +{
> + unsigned short *p = (unsigned short*) data;
> +
> + while (count--) writew(*p++, reg);
> +}
> +
> +static inline void writeswsw (void __iomem *reg, void *data, int count)
> +{
> + unsigned short *p = (unsigned short *) data;
> +
> + while (count--) writel((int)(_swapw(*p++)), reg);
> +}
> +
> +static inline void writesl (void __iomem *reg, void *data, int count)
> +{
> + unsigned long *p = (unsigned long*) data;
> +
> + while (count--) writel(*p++, reg);
> +}
> +
> +static inline void writeslsw (void __iomem *reg, void *data, int count)
> +{
> + unsigned long *p = (unsigned long *) data;
> +
> + while (count--) writel((int)(_swapl(*p++)), reg);
> +}
> +
> +static inline void readsb (void __iomem *reg, void *data, int count)
> +{
> + unsigned char *p = (unsigned char *) data;
> +
> + while (count--) *p++ = readb(reg);
> +}
> +
> +static inline void readsbsw (void __iomem *reg, void *data, int count)
> +{
> + unsigned char *p = (unsigned char *) data;
> +
> + while (count--) *p++ = (unsigned char)readl(reg);
> +}
> +
> +static inline void readsw (void __iomem *reg, void *data, int count)
> +{
> + unsigned short *p = (unsigned short *) data;
> +
> + while (count--) *p++ = readb(reg);
> +}
> +
> +static inline void readswsw (void __iomem *reg, void *data, int count)
> +{
> + unsigned short *p = (unsigned short *) data;
> +
> + while (count--) *p++ = _swapw((unsigned short)readw(reg));
> +}
> +
> +static inline void readsl (void __iomem *reg, void *data, int count)
> +{
> + unsigned long *p = (unsigned long *) data;
> +
> + while (count--) *p++ = readb(reg);
> +}
> +
> +static inline void readslsw (void __iomem *reg, void *data, int count)
> +{
> + unsigned long *p = (unsigned long *) data;
> +
> + while (count--) *p++ = _swapl(readl(reg));
> +}
> +
> +
> #define __raw_readb readb
> #define __raw_readw readw
> #define __raw_readl readl
> @@ -180,4 +265,3 @@ extern void iounmap(void *addr);
> #endif /* __KERNEL__ */
>
> #endif /* _M68KNOMMU_IO_H */
Could not this be moved to a separate patch ? That way arch specific
changes are separated from the network arch-indep changes.
- Arnaud
> -
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists