Re: [RFC v2 PATCH] m68knommu: added dm9000 support

[Arnaud Lacombe]

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@xxxxxxxxx> 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@xxxxxxxxx>
> ---
>
> --- 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@xxxxxxxxxxxxxxx
> 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 linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/