[PATCH] powerpc/mpc52xx/mtd: fix mtd-ram access for 16-bit Local Plus Bus

[Grant Likely]

On Tue, Jun 9, 2009 at 1:46 PM, Albrecht Dreß<albrecht.dress at arcor.de> wrote:
> Hi all,
>
> this patch adds support for RAM chips connected to the Local Plus Bus of a
> MPC5200B in 16-bit mode.  As no single byte write accesses are allowed by
> the bus in this mode, a byte write has to be split into a word read - modify
> - write sequence (mpc52xx_memcpy2lpb16, as fix/extension for memcpy_toio;
> note that memcpy_fromio *does* work just fine).  It has been tested in
> conjunction with Wolfram Sang's mtd-ram [1] and Sascha Hauer's jffs
> unaligned access [2] patches on 2.6.29.1, with a Renesas static RAM
> connected in 16-bit "Large Flash" mode.
>
> [1] <http://lists.ozlabs.org/pipermail/linuxppc-dev/2009-June/072794.html>
> [2] <http://article.gmane.org/gmane.linux.drivers.mtd/21521>
>
> Signed-off-by: Albrecht Dreß <albrecht.dress at arcor.de>
> Cc: Grant Likely <grant.likely at secretlab.ca>
> Cc: David Woodhouse <dwmw2 at infradead.org>
> Cc: linuxppc-dev at ozlabs.org
>
> ---
>
> diff -u linux-2.6.29.1.orig/arch/powerpc/platforms/52xx/mpc52xx_common.c
> linux-2.6.29.1/arch/powerpc/platforms/52xx/mpc52xx_common.c
> --- linux-2.6.29.1.orig/arch/powerpc/platforms/52xx/mpc52xx_common.c
>  2009-04-02 22:55:27.000000000 +0200
> +++ linux-2.6.29.1/arch/powerpc/platforms/52xx/mpc52xx_common.c 2009-06-09
> 21:16:22.000000000 +0200
> @@ -225,3 +225,59 @@
>
>        while (1);
>  }
> +
> +/**
> + * mpc52xx_memcpy2lpb16: copy data to the Local Plus Bus in 16-bit mode
> which
> + * doesn't allow byte accesses
> + */
> +void
> +mpc52xx_memcpy2lpb16(volatile void __iomem *dest, const void *src,
> +                    unsigned long n)
> +{
> +       void *vdest = (void __force *) dest;
> +
> +       __asm__ __volatile__ ("sync" : : : "memory");
> +
> +       if (((unsigned long) vdest & 1) != 0) {
> +               u8 buf[2];
> +
> +               *(u16 *)buf = *((volatile u16 *)(vdest - 1));
> +               buf[1] = *((u8 *)src);
> +               *((volatile u16 *)(vdest - 1)) = *(u16 *)buf;

what is the purpose of volatile here?  If you need io barriers, then
use the in_/out_be16 macros.

> +               src++;
> +               vdest++;
> +               n--;
> +       }
> +
> +       /* looks weird, but helps the optimiser... */
> +       if (n >= 4) {
> +               unsigned long chunks = n >> 2;
> +               volatile u32 * _dst = (volatile u32 *)(vdest - 4);
> +               volatile u32 * _src = (volatile u32 *)(src - 4);
> +
> +               vdest += chunks << 2;
> +               src += chunks << 2;
> +               do {
> +                       *++_dst = *++_src;
> +               } while (--chunks);
> +               n &= 3;
> +       }
> +
> +       if (n >= 2) {
> +               *((volatile u16 *)vdest) = *((volatile u16 *)src);
> +               src += 2;
> +               vdest += 2;
> +               n -= 2;
> +       }
> +
> +       if (n > 0) {
> +               u8 buf[2];
> +
> +               *(u16 *)buf = *((volatile u16 *)vdest);
> +               buf[0] = *((u8 *)src);
> +               *((volatile u16 *)vdest) = *(u16 *)buf;

ditto.

> +       }
> +
> +       __asm__ __volatile__ ("sync" : : : "memory");
> +}
> +EXPORT_SYMBOL(mpc52xx_memcpy2lpb16);
> diff -u linux-2.6.29.1.orig/arch/powerpc/include/asm/mpc52xx.h
> linux-2.6.29.1/arch/powerpc/include/asm/mpc52xx.h
> --- linux-2.6.29.1.orig/arch/powerpc/include/asm/mpc52xx.h      2009-04-02
> 22:55:27.000000000 +0200
> +++ linux-2.6.29.1/arch/powerpc/include/asm/mpc52xx.h   2009-06-09
> 21:14:31.000000000 +0200
> @@ -274,6 +274,8 @@
>  extern void mpc52xx_map_common_devices(void);
>  extern int mpc52xx_set_psc_clkdiv(int psc_id, int clkdiv);
>  extern void mpc52xx_restart(char *cmd);
> +extern void mpc52xx_memcpy2lpb16(volatile void __iomem *dest, const void
> *src,
> +                                unsigned long n);
>
>  /* mpc52xx_pic.c */
>  extern void mpc52xx_init_irq(void);
> diff -u linux-2.6.29.1.orig/include/linux/mtd/map.h
> linux-2.6.29.1/include/linux/mtd/map.h
> --- linux-2.6.29.1.orig/include/linux/mtd/map.h 2009-04-02
> 22:55:27.000000000 +0200
> +++ linux-2.6.29.1/include/linux/mtd/map.h      2009-06-08
> 14:28:05.000000000 +0200
> @@ -13,6 +13,9 @@
>  #include <asm/unaligned.h>
>  #include <asm/system.h>
>  #include <asm/io.h>
> +#ifdef CONFIG_PPC_MPC52xx
> +#include <asm/mpc52xx.h>
> +#endif
>
>  #ifdef CONFIG_MTD_MAP_BANK_WIDTH_1
>  #define map_bankwidth(map) 1
> @@ -417,6 +420,11 @@
>
>  static inline void inline_map_copy_to(struct map_info *map, unsigned long
> to, const void *from, ssize_t len)
>  {
> +#ifdef CONFIG_PPC_MPC52xx
> +       if (map->bankwidth == 2)
> +               mpc52xx_memcpy2lpb16(map->virt + to, from, len);
> +       else
> +#endif
>        memcpy_toio(map->virt + to, from, len);
>  }

Blech.  ugly #ifdef and not really multiplatform safe (yeah, I know it
shouldn't break non-5200 platforms, but it does have an undesirable
impact).  There's got to be a better way.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.