[RFC] bcm47xxsflash: just use memcpy for reading

Submitted by Rafał Miłecki on Sept. 19, 2012, 12:08 p.m.

Details

Message ID 1348056494-30019-1-git-send-email-zajec5@gmail.com
State RFC
Headers show

Commit Message

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(-)

Comments

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 hide | download patch | download mbox

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,