[PATCH 00/22] Random ARM randconfig fixes in drivers

[PATCH 00/22] Random ARM randconfig fixes in drivers

[Arnd Bergmann]

These are a bunch of fixes I had to do to get all randconfig
configurations on ARM working. Most of these are really old
bugs, but there are also some new ones. I don't think any of
them require a backport to linux-stable.

I have checked that they are all still required on yesterday's
linux-next kernel. Please apply on the appropriate trees unless
there are objections.

Patch numbers are per subsystem, which I thought is less confusing
than numbering them 1-22 when they are all totall independent.

Arnd Bergmann (22):
  mdio_bus: fix devm_mdiobus_alloc_size export
  phy: kona2: use 'select GENERIC_PHY' in Kconfig
  phy: exynos: fix SATA phy license typo
  dmaengine: omap: hide filter_fn for built-in drivers
  dmaengine: sa11x0: remove broken #ifdef
  mtd/onenand: fix build warning for dma type
  mtd: orion-nand: fix build error with ARMv4
  clk/versatile: export symbols for impd1
  bus/omap_l3: avoid sync initcall for modules
  bus/arm-cci: add dependency on OF && CPU_V7
  watchdog: iop_wdt only builds for mach-iop13xx
  mpilib: use 'static inline' for mpi-inline.h
  ata: pata_at91 only works on sam9
  i2c/nuc900: fix ancient build error
  iio: always select ANON_INODES
  iio:adc: at91 requires the input subsystem
  pci: rcar host needs OF
  input: fix ps2/serio module dependency
  input: atmel-wm97xx: only build for AVR32
  misc: atmel_pwm: only build for supported platforms
  remoteproc: da8xx: don't select CMA on no-MMU
  regulator: arizona-ldo1: add missing #include

 drivers/ata/Kconfig               | 2 +-
 drivers/bus/Kconfig               | 2 +-
 drivers/bus/omap_l3_noc.c         | 4 ++++
 drivers/bus/omap_l3_smx.c         | 4 ++++
 drivers/clk/versatile/clk-icst.c  | 1 +
 drivers/clk/versatile/clk-impd1.c | 2 ++
 drivers/dma/sa11x0-dma.c          | 4 ----
 drivers/i2c/busses/i2c-nuc900.c   | 2 +-
 drivers/iio/Kconfig               | 1 +
 drivers/iio/adc/Kconfig           | 1 +
 drivers/input/keyboard/Kconfig    | 2 +-
 drivers/input/mouse/Kconfig       | 2 +-
 drivers/input/touchscreen/Kconfig | 2 +-
 drivers/misc/Kconfig              | 3 ++-
 drivers/mtd/nand/orion_nand.c     | 5 +++++
 drivers/mtd/onenand/samsung.c     | 8 ++++----
 drivers/net/phy/mdio_bus.c        | 2 +-
 drivers/pci/host/Kconfig          | 2 +-
 drivers/phy/Kconfig               | 2 +-
 drivers/phy/phy-exynos5250-sata.c | 2 +-
 drivers/regulator/arizona-ldo1.c  | 1 +
 drivers/remoteproc/Kconfig        | 2 +-
 drivers/watchdog/Kconfig          | 2 +-
 include/linux/omap-dma.h          | 2 +-
 lib/mpi/mpi-inline.h              | 2 +-
 lib/mpi/mpi-internal.h            | 8 --------
 26 files changed, 39 insertions(+), 31 deletions(-)

-- 
1.8.3.2

Cc: bhelgaas@google.com
Cc: dwmw2@infradead.org
Cc: dmitry.torokhov@gmail.com
Cc: balbi@ti.com
Cc: gregkh@linuxfoundation.org
Cc: plagnioj@jcrosoft.com
Cc: jic23@kernel.org
Cc: josh.wu@atmel.com
Cc: kishon@ti.com
Cc: linus.walleij@linaro.org
Cc: broonie@kernel.org
Cc: mturquette@linaro.org
Cc: nicolas.ferre@atmel.com
Cc: ohad@wizery.com
Cc: linux@arm.linux.org.uk
Cc: tony@atomide.com
Cc: vinod.koul@intel.com
Cc: wim@iguana.be
Cc: wsa@the-dreams.de
Cc: dmaengine@vger.kernel.org
Cc: linux-i2c@vger.kernel.org
Cc: linux-ide@vger.kernel.org
Cc: linux-iio@vger.kernel.org
Cc: linux-input@vger.kernel.org
Cc: linux-mtd@lists.infradead.org
Cc: linux-pci@vger.kernel.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: linux-watchdog@vger.kernel.org
Cc: netdev@vger.kernel.org

[PATCH 1/2] mtd/onenand: fix build warning for dma type

[Arnd Bergmann]

The samsung onenand driver passes around a dma address token
through a void pointer, which is incorrect and leads to
warnings like this one:

onenand/samsung.c:548:2: warning: passing argument 1 of '__fswab32' makes integer from pointer without a cast [enabled by default]
  writel(src, base + S5PC110_DMA_SRC_ADDR);
  ^

This patch makes it use dma_addr_t here, which is more appropriate.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Brian Norris <computersforpeace@gmail.com>
Cc: linux-mtd@lists.infradead.org
---
 drivers/mtd/onenand/samsung.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/onenand/samsung.c b/drivers/mtd/onenand/samsung.c
index b1a792f..efb819c 100644
--- a/drivers/mtd/onenand/samsung.c
+++ b/drivers/mtd/onenand/samsung.c
@@ -537,9 +537,9 @@ static int onenand_write_bufferram(struct mtd_info *mtd, int area,
 	return 0;
 }
 
-static int (*s5pc110_dma_ops)(void *dst, void *src, size_t count, int direction);
+static int (*s5pc110_dma_ops)(dma_addr_t dst, dma_addr_t src, size_t count, int direction);
 
-static int s5pc110_dma_poll(void *dst, void *src, size_t count, int direction)
+static int s5pc110_dma_poll(dma_addr_t dst, dma_addr_t src, size_t count, int direction)
 {
 	void __iomem *base = onenand->dma_addr;
 	int status;
@@ -605,7 +605,7 @@ static irqreturn_t s5pc110_onenand_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-static int s5pc110_dma_irq(void *dst, void *src, size_t count, int direction)
+static int s5pc110_dma_irq(dma_addr_t dst, dma_addr_t src, size_t count, int direction)
 {
 	void __iomem *base = onenand->dma_addr;
 	int status;
@@ -686,7 +686,7 @@ static int s5pc110_read_bufferram(struct mtd_info *mtd, int area,
 		dev_err(dev, "Couldn't map a %d byte buffer for DMA\n", count);
 		goto normal;
 	}
-	err = s5pc110_dma_ops((void *) dma_dst, (void *) dma_src,
+	err = s5pc110_dma_ops(dma_dst, dma_src,
 			count, S5PC110_DMA_DIR_READ);
 
 	if (page_dma)
-- 
1.8.3.2

Re: [PATCH 1/2] mtd/onenand: fix build warning for dma type

[Brian Norris]

On Thu, May 08, 2014 at 04:56:14PM +0200, Arnd Bergmann wrote:
> The samsung onenand driver passes around a dma address token
> through a void pointer, which is incorrect and leads to
> warnings like this one:
> 
> onenand/samsung.c:548:2: warning: passing argument 1 of '__fswab32' makes integer from pointer without a cast [enabled by default]
>   writel(src, base + S5PC110_DMA_SRC_ADDR);
>   ^
> 
> This patch makes it use dma_addr_t here, which is more appropriate.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Brian Norris <computersforpeace@gmail.com>
> Cc: linux-mtd@lists.infradead.org

Pushed this one to l2-mtd.git. Thanks!

I think the second MTD patch is still under discussion, right? I'll
follow up there if necessary.

Brian

[PATCH 2/2] mtd: orion-nand: fix build error with ARMv4

[Arnd Bergmann]

orion_nand_read_buf uses an inline assembly with the "ldrd"
instruction, which is only available from ARMv5 upwards. This
used to be fine, since all users have an ARMv5 or ARMv7 CPU,
but now we can also build a multiplatform kernel with ARMv4
support enabled in addition to the "kirkwood" (mvebu) platform.

This provides an alternative to call the readsl() function that
is supposed to have the same effect and is also optimized for
performance.

This patch is untested, and it would be worthwhile to check
if there is any performance impact, especially in case the readsl
version is actually faster.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Brian Norris <computersforpeace@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: linux-mtd@lists.infradead.org
---
 drivers/mtd/nand/orion_nand.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mtd/nand/orion_nand.c b/drivers/mtd/nand/orion_nand.c
index dd7fe81..c7b5e8a 100644
--- a/drivers/mtd/nand/orion_nand.c
+++ b/drivers/mtd/nand/orion_nand.c
@@ -56,6 +56,7 @@ static void orion_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
 		*buf++ = readb(io_base);
 		len--;
 	}
+#if __LINUX_ARM_ARCH__ >= 5
 	buf64 = (uint64_t *)buf;
 	while (i < len/8) {
 		/*
@@ -68,6 +69,10 @@ static void orion_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
 		asm volatile ("ldrd\t%0, [%1]" : "=&r" (x) : "r" (io_base));
 		buf64[i++] = x;
 	}
+#else
+	readsl(io_base, buf, len/8);
+	i = len / 8 * 8;
+#endif
 	i *= 8;
 	while (i < len)
 		buf[i++] = readb(io_base);
-- 
1.8.3.2

Re: [PATCH 2/2] mtd: orion-nand: fix build error with ARMv4

[Ezequiel Garcia]

On 08 May 04:56 PM, Arnd Bergmann wrote:
> orion_nand_read_buf uses an inline assembly with the "ldrd"
> instruction, which is only available from ARMv5 upwards. This
> used to be fine, since all users have an ARMv5 or ARMv7 CPU,
> but now we can also build a multiplatform kernel with ARMv4
> support enabled in addition to the "kirkwood" (mvebu) platform.
> 
> This provides an alternative to call the readsl() function that
> is supposed to have the same effect and is also optimized for
> performance.
> 
> This patch is untested, and it would be worthwhile to check
> if there is any performance impact, especially in case the readsl
> version is actually faster.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Brian Norris <computersforpeace@gmail.com>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: linux-mtd@lists.infradead.org
> ---
>  drivers/mtd/nand/orion_nand.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/mtd/nand/orion_nand.c b/drivers/mtd/nand/orion_nand.c
> index dd7fe81..c7b5e8a 100644
> --- a/drivers/mtd/nand/orion_nand.c
> +++ b/drivers/mtd/nand/orion_nand.c
> @@ -56,6 +56,7 @@ static void orion_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
>  		*buf++ = readb(io_base);
>  		len--;
>  	}
> +#if __LINUX_ARM_ARCH__ >= 5
>  	buf64 = (uint64_t *)buf;
>  	while (i < len/8) {
>  		/*
> @@ -68,6 +69,10 @@ static void orion_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
>  		asm volatile ("ldrd\t%0, [%1]" : "=&r" (x) : "r" (io_base));
>  		buf64[i++] = x;
>  	}
> +#else
> +	readsl(io_base, buf, len/8);

I gave this a try in order to answer Arnd's performance question. First of all,
the patch seems wrong. I guess it's because readsl reads 4-bytes pieces, instead of
8-bytes.

This patch below is tested (but not completely, see below) and works:

diff --git a/drivers/mtd/nand/orion_nand.c b/drivers/mtd/nand/orion_nand.c
index dd7fe81..7a78cc5 100644
--- a/drivers/mtd/nand/orion_nand.c
+++ b/drivers/mtd/nand/orion_nand.c
@@ -52,6 +52,7 @@ static void orion_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
 	uint64_t *buf64;
 	int i = 0;
 
+#if __LINUX_ARM_ARCH__ >= 5
 	while (len && (unsigned long)buf & 7) {
 		*buf++ = readb(io_base);
 		len--;
@@ -69,6 +70,14 @@ static void orion_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
 		buf64[i++] = x;
 	}
 	i *= 8;
+#else
+	while (len && (unsigned long)buf & 3) {
+		*buf++ = readb(io_base);
+		len--;
+	}
+	readsl(io_base, buf, len/4);
+	i = (len / 4 * 4) * 4;
+#endif
 	while (i < len)
 		buf[i++] = readb(io_base);
 }

However, all the reads are nicely aligned (in both the buffer and the
length) which means the only 'read' performed in the readsl() one.

In other words, the patch is still half-untested. Therefore, and given
this is meant only to coherce a build, maybe we'd rather just loop over
readb and stay on the safe side?

And now, answering Arnd's question:

# Using ldrd
# time nanddump /dev/mtd5 -f /dev/null -q
real	0m 5.90s
user	0m 0.22s
sys	0m 5.67s

# Using readsl
# time nanddump /dev/mtd5 -f /dev/null -q
real	0m 6.39s
user	0m 0.17s
sys	0m 6.20s

So I'd say, let's stick to the ldrd magic.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH 2/2] mtd: orion-nand: fix build error with ARMv4

[Geert Uytterhoeven]

On Fri, May 9, 2014 at 8:45 PM, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> --- a/drivers/mtd/nand/orion_nand.c
> +++ b/drivers/mtd/nand/orion_nand.c
> @@ -52,6 +52,7 @@ static void orion_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
>         uint64_t *buf64;
>         int i = 0;
>
> +#if __LINUX_ARM_ARCH__ >= 5
>         while (len && (unsigned long)buf & 7) {
>                 *buf++ = readb(io_base);
>                 len--;
> @@ -69,6 +70,14 @@ static void orion_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
>                 buf64[i++] = x;
>         }
>         i *= 8;
> +#else
> +       while (len && (unsigned long)buf & 3) {
> +               *buf++ = readb(io_base);
> +               len--;
> +       }
> +       readsl(io_base, buf, len/4);
> +       i = (len / 4 * 4) * 4;

Why multiply by 4 twice? "i" is supposed to be the number of bytes read,
right?

BTW, Arnd's version should just need s/8/4/g to make it work.

> +#endif
>         while (i < len)
>                 buf[i++] = readb(io_base);
>  }

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/2] mtd: orion-nand: fix build error with ARMv4

[Arnd Bergmann]

On Friday 09 May 2014 15:45:05 Ezequiel Garcia wrote:
> On 08 May 04:56 PM, Arnd Bergmann wrote:
> 
> I gave this a try in order to answer Arnd's performance question.

Thanks a lot for testing!

> First of all,
> the patch seems wrong. I guess it's because readsl reads 4-bytes pieces, instead of
> 8-bytes.

Oops. I guess I was thinking of a 64-bit system and didn't even notice
the difference between 4 and 8 byte accesses here. I wonder where I have
my mind sometimes.

> In other words, the patch is still half-untested. Therefore, and given
> this is meant only to coherce a build, maybe we'd rather just loop over
> readb and stay on the safe side?

I guess that would be equal to calling memcpy_fromio().

> And now, answering Arnd's question:
> 
> # Using ldrd
> # time nanddump /dev/mtd5 -f /dev/null -q
> real	0m 5.90s
> user	0m 0.22s
> sys	0m 5.67s
> 
> # Using readsl
> # time nanddump /dev/mtd5 -f /dev/null -q
> real	0m 6.39s
> user	0m 0.17s
> sys	0m 6.20s
> 
> So I'd say, let's stick to the ldrd magic.

Ok, that is a noticeable difference. For scale, what is the size of that partition?
If this is something that actually affects people, it might be worth also trying
memcpy(), which should be better at saturating the bus, but might be wrong here
(if alignment the alignment requirements on the external bus are stricter than
what memcpy does) or it might not make a difference at all if the code is already
ideal.

	Arnd

Re: [PATCH 2/2] mtd: orion-nand: fix build error with ARMv4

[Jason Gunthorpe]

On Fri, May 09, 2014 at 03:45:05PM -0300, Ezequiel Garcia wrote:

> I gave this a try in order to answer Arnd's performance
> question. First of all, the patch seems wrong. I guess it's because
> readsl reads 4-bytes pieces, instead of 8-bytes.
> 
> This patch below is tested (but not completely, see below) and works:

Compilers are better now, I think you can just ditch the weirdness:

uint64_t *from;
uint64_t *to;
void foo()
{
	for (unsigned int I = 0; I != 1000; I++)
		*to++ = *from;
}

Using even gcc 4.6.3 gives good code:

(v6)
.L2:
        ldrd    r2, [ip]
        strd    r2, [r1], #8
        cmp     r1, r0

(v4)
.L2:
        ldmia   ip, {r0-r1}
        stmia   r3!, {r0-r1}
        cmp     r3, r2

For correctness this v4 version does require that the cpu executes the
ldmia reads in increasing address order, and never in any other
order. AFAIK the periphal is just a simple fifo that basically ignores
the address.

memcpy_fromio is not as good since it will never align if the buffer
is unaligned, while this version does.

The below gives:

  c8:   ea000002        b       d8 <orion_nand_read_buf+0x84>
  cc:   e5dc0000        ldrb    r0, [ip]
  d0:   e7c30001        strb    r0, [r3, r1]
  d4:   e2811001        add     r1, r1, #1
  d8:   e1510002        cmp     r1, r2

Which looks the same as the asm version to me.

diff --git a/drivers/mtd/nand/orion_nand.c b/drivers/mtd/nand/orion_nand.c
index dc9d07f34b8a..fea1597f623e 100644
--- a/drivers/mtd/nand/orion_nand.c
+++ b/drivers/mtd/nand/orion_nand.c
@@ -95,16 +95,13 @@ static void orion_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
 	}
 	buf64 = (uint64_t *)buf;
 	while (i < len/8) {
-		/*
-		 * Since GCC has no proper constraint (PR 43518)
-		 * force x variable to r2/r3 registers as ldrd instruction
-		 * requires first register to be even.
-		 */
-		register uint64_t x asm ("r2");
-
-		asm volatile ("ldrd\t%0, [%1]" : "=&r" (x) : "r" (io_base));
-		buf64[i++] = x;
+#ifdef CONFIG_64BIT
+		buf64[i++] = readq_relaxed(io_base);
+#else
+		buf64[i++] = *(const volatile u64 __force *)io_base;
+#endif
 	}
+
 	i *= 8;
 	while (i < len)
 		buf[i++] = readb(io_base);

Re: [PATCH 2/2] mtd: orion-nand: fix build error with ARMv4

[Ezequiel Garcia]

On 09 May 03:28 PM, Jason Gunthorpe wrote:
> 
> > I gave this a try in order to answer Arnd's performance
> > question. First of all, the patch seems wrong. I guess it's because
> > readsl reads 4-bytes pieces, instead of 8-bytes.
> > 
> > This patch below is tested (but not completely, see below) and works:
> 
> Compilers are better now, I think you can just ditch the weirdness:
> 
[..]
> 
> The below gives:
> 
>   c8:   ea000002        b       d8 <orion_nand_read_buf+0x84>
>   cc:   e5dc0000        ldrb    r0, [ip]
>   d0:   e7c30001        strb    r0, [r3, r1]
>   d4:   e2811001        add     r1, r1, #1
>   d8:   e1510002        cmp     r1, r2
> 
> Which looks the same as the asm version to me.
> 

Nice! It wasn't really needed but since I have the board here:

# time nanddump /dev/mtd5 -f /dev/null -q
real	0m 5.82s
user	0m 0.20s
sys	0m 5.60s

Jason: Care to submit a proper patch?

On 08 May 04:56 PM, Arnd Bergmann wrote:
> Ok, that is a noticeable difference. For scale, what is the size of that partition?

The board is Openblocks A6, running mainline.

# cat /proc/mtd 
dev:    size   erasesize  name
mtd0: 00090000 00004000 "uboot"
mtd1: 00044000 00004000 "env"
mtd2: 00024000 00004000 "test"
mtd3: 00400000 00004000 "conf"
mtd4: 01d20000 00004000 "linux"
mtd5: 01dec000 00004000 "user"

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH 2/2] mtd: orion-nand: fix build error with ARMv4

[Arnd Bergmann]

On Friday 09 May 2014 19:09:15 Ezequiel Garcia wrote:
> # time nanddump /dev/mtd5 -f /dev/null -q
> real    0m 5.82s
> user    0m 0.20s
> sys     0m 5.60s
> 
> Jason: Care to submit a proper patch?
> 
> On 08 May 04:56 PM, Arnd Bergmann wrote:
> > Ok, that is a noticeable difference. For scale, what is the size of that partition?
> 
> The board is Openblocks A6, running mainline.
> 
> # cat /proc/mtd 
> dev:    size   erasesize  name
> mtd0: 00090000 00004000 "uboot"
> mtd1: 00044000 00004000 "env"
> mtd2: 00024000 00004000 "test"
> mtd3: 00400000 00004000 "conf"
> mtd4: 01d20000 00004000 "linux"
> mtd5: 01dec000 00004000 "user"

Ok, so it takes 5.6 seconds in kernel mode to access 31MB, which comes down
to 5.60MB/s. That isn't very fast compared to the time the CPU should take
for those instructions, so I'm surprised it actually makes any difference
at all.

There isn't a usable slave DMA engine in Armada XP by chance?

	Arnd

Re: [PATCH 2/2] mtd: orion-nand: fix build error with ARMv4

[Ezequiel Garcia]

On 10 May 12:24 AM, Arnd Bergmann wrote:
> On Friday 09 May 2014 19:09:15 Ezequiel Garcia wrote:
> > # time nanddump /dev/mtd5 -f /dev/null -q
> > real    0m 5.82s
> > user    0m 0.20s
> > sys     0m 5.60s
> > 
> > Jason: Care to submit a proper patch?
> > 
> > On 08 May 04:56 PM, Arnd Bergmann wrote:
> > > Ok, that is a noticeable difference. For scale, what is the size of that partition?
> > 
> > The board is Openblocks A6, running mainline.
> > 
> > # cat /proc/mtd 
> > dev:    size   erasesize  name
> > mtd0: 00090000 00004000 "uboot"
> > mtd1: 00044000 00004000 "env"
> > mtd2: 00024000 00004000 "test"
> > mtd3: 00400000 00004000 "conf"
> > mtd4: 01d20000 00004000 "linux"
> > mtd5: 01dec000 00004000 "user"
> 
> Ok, so it takes 5.6 seconds in kernel mode to access 31MB, which comes down
> to 5.60MB/s. That isn't very fast compared to the time the CPU should take
> for those instructions, so I'm surprised it actually makes any difference
> at all.
>
> There isn't a usable slave DMA engine in Armada XP by chance?
> 

The Openblocks A6 is a Kirkwood, not Armada XP.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH 2/2] mtd: orion-nand: fix build error with ARMv4

[Jason Gunthorpe]

On Fri, May 09, 2014 at 07:09:15PM -0300, Ezequiel Garcia wrote:
> On 09 May 03:28 PM, Jason Gunthorpe wrote:
> > 
> > > I gave this a try in order to answer Arnd's performance
> > > question. First of all, the patch seems wrong. I guess it's because
> > > readsl reads 4-bytes pieces, instead of 8-bytes.
> > > 
> > > This patch below is tested (but not completely, see below) and works:
> > 
> > Compilers are better now, I think you can just ditch the weirdness:
> > 
> [..]
> > 
> > The below gives:
> > 
> >   c8:   ea000002        b       d8 <orion_nand_read_buf+0x84>
> >   cc:   e5dc0000        ldrb    r0, [ip]
> >   d0:   e7c30001        strb    r0, [r3, r1]
> >   d4:   e2811001        add     r1, r1, #1
> >   d8:   e1510002        cmp     r1, r2
> > 
> > Which looks the same as the asm version to me.
> > 
> 
> Nice! It wasn't really needed but since I have the board here:
> 
> # time nanddump /dev/mtd5 -f /dev/null -q
> real	0m 5.82s
> user	0m 0.20s
> sys	0m 5.60s
> 
> Jason: Care to submit a proper patch?

Sure, but did anyone (Arnd?) have thoughts on a better way to do this:

+#ifdef CONFIG_64BIT
+               buf64[i++] = readq_relaxed(io_base);
+#else
+               buf64[i++] = *(const volatile u64 __force *)io_base;
+#endif

IMHO, readq should exist on any platform that can issue a 64 bit bus
transaction, and I expect many ARM's qualify.

> On 08 May 04:56 PM, Arnd Bergmann wrote:

> Ok, so it takes 5.6 seconds in kernel mode to access 31MB, which
> comes down to 5.60MB/s. That isn't very fast compared to the time
> the CPU should take for those instructions, so I'm surprised it
> actually makes any difference at all.

Likely, what is happening is that the bus interface is holding off
returning the read data until it complets the bus cycles, then the
response travels to the CPU which turns around another.

This creates a dead time where the bus isn't do anything.

The larger bus transfer the CPU can do the less percentage of time the
turnaround takes as overhead.

If the cpu could pipeline two reads then it could be highest-possible,
but I guess the memory ordering for the mapping prevents that??

Regarding DMA, who knows if the interface can handle a burst
transfer..

Jason

Re: [PATCH 2/2] mtd: orion-nand: fix build error with ARMv4

[Arnd Bergmann]

On Tuesday 13 May 2014 14:55:48 Jason Gunthorpe wrote:
> On Fri, May 09, 2014 at 07:09:15PM -0300, Ezequiel Garcia wrote:
> > On 09 May 03:28 PM, Jason Gunthorpe wrote:
> > > 
> > > > I gave this a try in order to answer Arnd's performance
> > > > question. First of all, the patch seems wrong. I guess it's because
> > > > readsl reads 4-bytes pieces, instead of 8-bytes.
> > > > 
> > > > This patch below is tested (but not completely, see below) and works:
> > > 
> > > Compilers are better now, I think you can just ditch the weirdness:
> > > 
> > [..]
> > > 
> > > The below gives:
> > > 
> > >   c8:   ea000002        b       d8 <orion_nand_read_buf+0x84>
> > >   cc:   e5dc0000        ldrb    r0, [ip]
> > >   d0:   e7c30001        strb    r0, [r3, r1]
> > >   d4:   e2811001        add     r1, r1, #1
> > >   d8:   e1510002        cmp     r1, r2
> > > 
> > > Which looks the same as the asm version to me.
> > > 
> > 
> > Nice! It wasn't really needed but since I have the board here:
> > 
> > # time nanddump /dev/mtd5 -f /dev/null -q
> > real  0m 5.82s
> > user  0m 0.20s
> > sys   0m 5.60s
> > 
> > Jason: Care to submit a proper patch?
> 
> Sure, but did anyone (Arnd?) have thoughts on a better way to do this:
> 
> +#ifdef CONFIG_64BIT
> +               buf64[i++] = readq_relaxed(io_base);
> +#else
> +               buf64[i++] = *(const volatile u64 __force *)io_base;
> +#endif
>
> IMHO, readq should exist on any platform that can issue a 64 bit bus
> transaction, and I expect many ARM's qualify.

Well, the original problem happened specifically for the case that doesn't
have a 64-bit bus transaction (building for ARMv4). If we define
readq_relaxed, it has to be an inline assembly, in order to work for
drivers that require an atomic transaction, so that would have the
same problem. We are a bit inconsistent here though: most 32-bit
architectures have no readq, parisc has one that does two 32-bit accesses,
sh relies on the compiler, and tile apparently has a native instruction.

It seems reasonable to replace the inline assembly in this driver with
a new function as a cleanup, but then how do you want to solve the case
of building a combined armv4/v5 kernel?

	Arnd

Re: [PATCH 2/2] mtd: orion-nand: fix build error with ARMv4

[Geert Uytterhoeven]

On Wed, May 14, 2014 at 1:47 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> Sure, but did anyone (Arnd?) have thoughts on a better way to do this:
>>
>> +#ifdef CONFIG_64BIT
>> +               buf64[i++] = readq_relaxed(io_base);
>> +#else
>> +               buf64[i++] = *(const volatile u64 __force *)io_base;
>> +#endif
>>
>> IMHO, readq should exist on any platform that can issue a 64 bit bus
>> transaction, and I expect many ARM's qualify.
>
> Well, the original problem happened specifically for the case that doesn't
> have a 64-bit bus transaction (building for ARMv4). If we define
> readq_relaxed, it has to be an inline assembly, in order to work for
> drivers that require an atomic transaction, so that would have the
> same problem. We are a bit inconsistent here though: most 32-bit
> architectures have no readq, parisc has one that does two 32-bit accesses,
> sh relies on the compiler, and tile apparently has a native instruction.
>
> It seems reasonable to replace the inline assembly in this driver with
> a new function as a cleanup, but then how do you want to solve the case
> of building a combined armv4/v5 kernel?

Provide two helper functions, one for v4, one for v5, and call
them through a function pointer that's set up during driver probe?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/2] mtd: orion-nand: fix build error with ARMv4

[Arnd Bergmann]

On Wednesday 14 May 2014 14:35:28 Geert Uytterhoeven wrote:
> On Wed, May 14, 2014 at 1:47 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> Sure, but did anyone (Arnd?) have thoughts on a better way to do this:
> >>
> >> +#ifdef CONFIG_64BIT
> >> +               buf64[i++] = readq_relaxed(io_base);
> >> +#else
> >> +               buf64[i++] = *(const volatile u64 __force *)io_base;
> >> +#endif
> >>
> >> IMHO, readq should exist on any platform that can issue a 64 bit bus
> >> transaction, and I expect many ARM's qualify.
> >
> > Well, the original problem happened specifically for the case that doesn't
> > have a 64-bit bus transaction (building for ARMv4). If we define
> > readq_relaxed, it has to be an inline assembly, in order to work for
> > drivers that require an atomic transaction, so that would have the
> > same problem. We are a bit inconsistent here though: most 32-bit
> > architectures have no readq, parisc has one that does two 32-bit accesses,
> > sh relies on the compiler, and tile apparently has a native instruction.
> >
> > It seems reasonable to replace the inline assembly in this driver with
> > a new function as a cleanup, but then how do you want to solve the case
> > of building a combined armv4/v5 kernel?
> 
> Provide two helper functions, one for v4, one for v5, and call
> them through a function pointer that's set up during driver probe?

No, that won't help. It's a compile-time problem only: This driver
is never actually used on ARMv4, but if we build a kernel that supports
both ARMv4 and later, gcc passes -march=armv4 to the assembler, which
barfs on an invalid instruction. It would be fine to make that error
message just go away because we know it will only be used on CPUs that
do have this instruction.

	Arnd

Re: [PATCH 00/22] Random ARM randconfig fixes in drivers

[Guenter Roeck]

On Thu, May 08, 2014 at 04:46:51PM +0200, Arnd Bergmann wrote:
> These are a bunch of fixes I had to do to get all randconfig
> configurations on ARM working. Most of these are really old
> bugs, but there are also some new ones. I don't think any of
> them require a backport to linux-stable.
> 
> I have checked that they are all still required on yesterday's
> linux-next kernel. Please apply on the appropriate trees unless
> there are objections.
> 
Is this series of patches also going to fix arm:allmodconfig ?

Thanks,
Guenter

Re: [PATCH 00/22] Random ARM randconfig fixes in drivers

[Arnd Bergmann]

On Thursday 08 May 2014, Guenter Roeck wrote:
> On Thu, May 08, 2014 at 04:46:51PM +0200, Arnd Bergmann wrote:
> > These are a bunch of fixes I had to do to get all randconfig
> > configurations on ARM working. Most of these are really old
> > bugs, but there are also some new ones. I don't think any of
> > them require a backport to linux-stable.
> > 
> > I have checked that they are all still required on yesterday's
> > linux-next kernel. Please apply on the appropriate trees unless
> > there are objections.
> > 
> Is this series of patches also going to fix arm:allmodconfig ?

Possibly, I haven't checked in a while. I'm unfortunately sitting on
about 200 other patches in the same branch, which together fix all
build errors in any configuration I encountered.

I should really do some allmodconfig/allnoconfig/allyesconfig
builds without my series again, and prioritize sending out the
ones required for that.

	Arnd