On 4 April 2018 at 18:45, Wolfram Sang <wsa@xxxxxxxxxxxxx> wrote: > From: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx> > > Early revisions of certain SoCs cannot do multiple DMA RX streams in > parallel. To avoid data corruption, only allow one DMA RX channel and > fall back to PIO, if needed. > > Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx> > --- > > Okay, here is what I came up with to solve this problem. Compared to > Shimoda-san's sketch, I replaced the semaphore with atomic bit operations. I > think this should do because we really need only a flag telling us if RX is in > use already. And bitops should be more lightweight and less complex. > > I used this test on the target with SD cards in both slots: > > === > #! /bin/sh -e > > IF="/dev/mmcblk$1p1" > PTH="/mnt/mmcblk$1" > CHK='/tmp/check' > > while :; do > mkdir -p $PTH > mount $IF $PTH &> /dev/null > cd $PTH > time md5sum -c $CHK >> /tmp/mmc$1 > cd > umount $PTH > done > === > > CHK was manually created beforehand. Before this patch, I got checksum failures > within seconds. Now, it runs for hours without checksum problems. Of course, > there is a performance regression because of falling back to PIO now, but we > can't help that. > > I also ran the test on a H3 ES2.0 which doesn't suffer from the problem. No > regressions, it works out of the box there. > > Shimoda-san: what do you think of this approach? Please note that I didn't > remove the 'revision' from the whitelist yet. This will be done incrementally. > I thought to first fix the data corruption issue. I assume this is material for stable as well, right? > > Kind regards, > > Wolfram > > drivers/mmc/host/renesas_sdhi_internal_dmac.c | 35 ++++++++++++++++++++++++--- > 1 file changed, 32 insertions(+), 3 deletions(-) > > diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c b/drivers/mmc/host/renesas_sdhi_internal_dmac.c > index 8e0acd197c43..380570a26a09 100644 > --- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c > +++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c > @@ -9,6 +9,7 @@ > * published by the Free Software Foundation. > */ > > +#include <linux/bitops.h> > #include <linux/device.h> > #include <linux/dma-mapping.h> > #include <linux/io-64-nonatomic-hi-lo.h> > @@ -62,6 +63,17 @@ > * need a custom accessor. > */ > > +static unsigned long global_flags = 0; There's no need to initialize static variables to zero. [...] Kind regards Uffe
