[PATCH V3] mtd: bcm47xxsflash: support reading flash out of mapping window

[Rafał Miłecki]

On 8 February 2017 at 20:35, Brian Norris <computersforpeace at gmail.com> wrote:
> On Mon, Jan 23, 2017 at 12:51:54PM +0100, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal at milecki.pl>
>>
>> For reading flash content we use MMIO but it's possible to read only
>> first 16 MiB this way. It's simply an arch design/limitation.
>> To support flash sizes bigger than 16 MiB implement indirect access
>> using ChipCommon registers.
>> This has been tested using MX25L25635F.
>>
>> Signed-off-by: Rafał Miłecki <rafal at milecki.pl>
>> ---
>> V2: Simplify line writing to buf
>>     Add some trivial comment for OPCODE_ST_READ4B
>>     Both requested by Marek
>> V3: Use as much as possible MMIO access for read crossing boundary (performance)
>> ---
>>  drivers/mtd/devices/bcm47xxsflash.c | 23 ++++++++++++++++++++---
>>  drivers/mtd/devices/bcm47xxsflash.h |  3 +++
>>  2 files changed, 23 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mtd/devices/bcm47xxsflash.c b/drivers/mtd/devices/bcm47xxsflash.c
>> index 4decd8c0360a..138689619ab4 100644
>> --- a/drivers/mtd/devices/bcm47xxsflash.c
>> +++ b/drivers/mtd/devices/bcm47xxsflash.c
>> @@ -105,15 +105,32 @@ static int bcm47xxsflash_read(struct mtd_info *mtd, loff_t from, size_t len,
>>                             size_t *retlen, u_char *buf)
>>  {
>>       struct bcm47xxsflash *b47s = mtd->priv;
>> +     size_t orig_len = len;
>>
>>       /* Check address range */
>>       if ((from + len) > mtd->size)
>>               return -EINVAL;
>>
>> -     memcpy_fromio(buf, b47s->window + from, len);
>> -     *retlen = len;
>> +     /* Read as much as possible using fast MMIO window */
>> +     if (from < BCM47XXSFLASH_WINDOW_SZ) {
>> +             size_t memcpy_len;
>>
>> -     return len;
>> +             memcpy_len = min(len, (size_t)(BCM47XXSFLASH_WINDOW_SZ - from));
>> +             memcpy_fromio(buf, b47s->window + from, memcpy_len);
>> +             from += memcpy_len;
>> +             len -= memcpy_len;
>
> Am I crazy, or should you either be doing 'buf += memcpy_len'?
> Otherwise, you'll be copying over the first few elements of 'buf' below,
> instead of finishing off the tail end.

Excellent catch, thank you!

-- 
Rafał