[PATCH] mtd: bcm47xxsflash: just use memcpy_fromio for reading

[PATCH] mtd: bcm47xxsflash: just use memcpy_fromio for reading

[Rafał Miłecki]

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
It doesn't generate any errors on normal compilation, but I wasn't able
to test it with "verify". This tool defeated me ;/

First I cloned dedekind/maintaining.git but it didn't run because or
lacking aiaiai.

So I've cloned dedekind/aiaiai.git.

Then I had to move aiaiai to ~/git/aiaiai to let "verify" find it.

So I was able to use verify:
time ./verify ../l2-mtd/ mips-bcm47xx_defconfig < ../l2-mtd/0001-mtd-bcm47xxsflash-just-use-memcpy-for-reading.patch
but it didn't work because patch was already applied. My bad, but... it
took 2m17.277s to verify to realize that. And that magic commit/message
wasn't helpful:
Failed to apply patch(es) with git am on top of:
0f1afc8 Quick fixes - applied by aiaiai

So I've removed patch from my tree, and tried again. This time it took
3m14.398s just to display:
[2012-09-20 10:39:10] aiaiai-make-kernel: Program "aiaiai-locker" is required but not found in PATH

I just wanted to add ~/git/aiaiai to my $PATH, but noticed it doesn't contain
aiaiai-locker anyway... I gave up :|

I appreciate your work on additional quality tools, but apart from good
code it also need to be easy-usable to make ppl use it :|
---
 drivers/mtd/devices/bcm47xxsflash.c |   31 ++-----------------------------
 1 files changed, 2 insertions(+), 29 deletions(-)

diff --git a/drivers/mtd/devices/bcm47xxsflash.c b/drivers/mtd/devices/bcm47xxsflash.c
index a328872..dd650db 100644
--- a/drivers/mtd/devices/bcm47xxsflash.c
+++ b/drivers/mtd/devices/bcm47xxsflash.c
@@ -14,41 +14,14 @@ static int bcm47xxsflash_read(struct mtd_info *mtd, loff_t from, size_t len,
 			      size_t *retlen, u_char *buf)
 {
 	struct bcma_sflash *sflash = mtd->priv;
-	size_t bytes_read = 0;
-	__iomem u8 *src = (__iomem u8 *)KSEG0ADDR(sflash->window + from);
-	int i;
-	size_t unaligned_before, unaligned_after;
 
 	/* Check address range */
 	if ((from + len) > mtd->size)
 		return -EINVAL;
 
-	unaligned_before = from & 0x3;
-	unaligned_after = (from + len) & 0x3;
+	memcpy_fromio(buf, (void *)KSEG0ADDR(sflash->window + from), len);
 
-	for (i = 0; i < unaligned_before; i++) {
-		*buf = readb(src);
-		buf++;
-		src++;
-		bytes_read++;
-	}
-	for (i = from - unaligned_before; i < from + len - unaligned_after;
-	     i += 4) {
-		*(u32 *)buf = readl(src);
-		buf += 4;
-		src += 4;
-		bytes_read += 4;
-	}
-	for (i = 0; i < unaligned_after; i++) {
-		*buf = readb(src);
-		buf++;
-		src++;
-		bytes_read++;
-	}
-
-	*retlen = bytes_read;
-
-	return 0;
+	return len;
 }
 
 static void bcm47xxsflash_fill_mtd(struct bcma_sflash *sflash,
-- 
1.7.7

Re: [PATCH] mtd: bcm47xxsflash: just use memcpy_fromio for reading

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 1928 bytes --]

On Thu, 2012-09-20 at 11:00 +0200, Rafał Miłecki wrote:
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
> It doesn't generate any errors on normal compilation, but I wasn't able
> to test it with "verify". This tool defeated me ;/
> 
> First I cloned dedekind/maintaining.git but it didn't run because or
> lacking aiaiai.
> 
> So I've cloned dedekind/aiaiai.git.
> 
> Then I had to move aiaiai to ~/git/aiaiai to let "verify" find it.
> 
> So I was able to use verify:
> time ./verify ../l2-mtd/ mips-bcm47xx_defconfig < ../l2-mtd/0001-mtd-bcm47xxsflash-just-use-memcpy-for-reading.patch
> but it didn't work because patch was already applied. My bad, but... it
> took 2m17.277s to verify to realize that. And that magic commit/message
> wasn't helpful:

Well, before doing anything with your tree, we clone it to a temporary
location (in the workdir, which can specify), that probably took 2
minutes in your machine.

> Failed to apply patch(es) with git am on top of:
> 0f1afc8 Quick fixes - applied by aiaiai

After this there should have been the right commit. But I agree this is
confusing, I'll look how I can improve this.

> So I've removed patch from my tree, and tried again. This time it took
> 3m14.398s just to display:
> [2012-09-20 10:39:10] aiaiai-make-kernel: Program "aiaiai-locker" is required but not found in PATH
> 
> I just wanted to add ~/git/aiaiai to my $PATH, but noticed it doesn't contain
> aiaiai-locker anyway... I gave up :|

Because you need to type "make". This is written in the docs. Dunno, how
would I make sure users do not forget to type make before running? I can
improve the error message though, and put a hint there.

> I appreciate your work on additional quality tools, but apart from good
> code it also need to be easy-usable to make ppl use it :|

Thanks for feed-back, appreciated!

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] mtd: bcm47xxsflash: just use memcpy_fromio for reading

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 686 bytes --]

On Thu, 2012-09-20 at 11:00 +0200, Rafał Miłecki wrote:
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>

Squashed this one to the previous one. Fixed up the following aiaiai
complaint:

Successfully built configuration "mips-bcm47xx_defconfig,mips,mips-linux-", results:

--- before_patching.log
+++ after_patching.log
@@ @@
+drivers/mtd/devices/bcm47xxsflash.c:22:29: warning: incorrect type in argument 2 (different address spaces) [sparse]
+drivers/mtd/devices/bcm47xxsflash.c:22:29:    expected void const volatile [noderef] <asn:2>*src [sparse]
+drivers/mtd/devices/bcm47xxsflash.c:22:29:    got void *<noident> [sparse]

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] mtd: bcm47xxsflash: just use memcpy_fromio for reading

[Rafał Miłecki]

2012/9/20 Artem Bityutskiy <dedekind1@gmail.com>:
> On Thu, 2012-09-20 at 11:00 +0200, Rafał Miłecki wrote:
>> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
>
> Squashed this one to the previous one. Fixed up the following aiaiai
> complaint:
>
> Successfully built configuration "mips-bcm47xx_defconfig,mips,mips-linux-", results:

Thanks!

I hope to learn using "verify + aiaiai" and send clean NAND driver soon :)

-- 
Rafał