Patchwork [RFC] bcm47xxsflash: just use memcpy for reading

login
register
mail settings
Submitter Rafał Miłecki
Date Sept. 19, 2012, 12:08 p.m.
Message ID <1348056494-30019-1-git-send-email-zajec5@gmail.com>
Download mbox | patch
Permalink /patch/185009/
State New
Headers show

Comments

Rafał Miłecki - Sept. 19, 2012, 12:08 p.m.
---
Still have to check it with aiaiai
---
 drivers/mtd/devices/bcm47xxsflash.c |   31 ++-----------------------------
 1 files changed, 2 insertions(+), 29 deletions(-)
Marc Kleine-Budde - Sept. 19, 2012, 1:45 p.m.
On 09/19/2012 02:08 PM, Rafał Miłecki wrote:
> ---
> Still have to check it with aiaiai
> ---
>  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..2f8dfd4 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(buf, (void *)KSEG0ADDR(sflash->window + from), len);

You should not access iomem with memcpy, there is a memcpy_fromio(), but
it's an unoptimized readb loop on arm.

Marc
>  
> -	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,
>
Rafał Miłecki - Sept. 19, 2012, 2:07 p.m.
2012/9/19 Marc Kleine-Budde <mkl@pengutronix.de>:
> On 09/19/2012 02:08 PM, Rafał Miłecki wrote:
>> ---
>> Still have to check it with aiaiai
>> ---
>>  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..2f8dfd4 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(buf, (void *)KSEG0ADDR(sflash->window + from), len);
>
> You should not access iomem with memcpy, there is a memcpy_fromio(), but
> it's an unoptimized readb loop on arm.

Sure, I can replace that with _fromio, which AFAIK is the same for mips.

I'm afraid access to the serial flash can't be really optimal. Saving
is actually even worse, it requires some operations on BCMA core,
polling for ready, etc.

Patch

diff --git a/drivers/mtd/devices/bcm47xxsflash.c b/drivers/mtd/devices/bcm47xxsflash.c
index a328872..2f8dfd4 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(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,