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

[Rafał Miłecki]

On 19 January 2017 at 22:30, Marek Vasut <marek.vasut at gmail.com> wrote:
> On 01/17/2017 01:04 PM, Rafał Miłecki wrote:
>> On 17 January 2017 at 12:49, Marek Vasut <marek.vasut at gmail.com> wrote:
>>> On 01/17/2017 12:08 PM, Rafał Miłecki wrote:
>>>> On 17 January 2017 at 12:00, Marek Vasut <marek.vasut at gmail.com> wrote:
>>>>> On 01/17/2017 11:51 AM, 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
>>>>>> ---
>>>>>>  drivers/mtd/devices/bcm47xxsflash.c | 12 +++++++++++-
>>>>>>  drivers/mtd/devices/bcm47xxsflash.h |  3 +++
>>>>>>  2 files changed, 14 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/mtd/devices/bcm47xxsflash.c b/drivers/mtd/devices/bcm47xxsflash.c
>>>>>> index 4decd8c..8d4c1db 100644
>>>>>> --- a/drivers/mtd/devices/bcm47xxsflash.c
>>>>>> +++ b/drivers/mtd/devices/bcm47xxsflash.c
>>>>>> @@ -110,7 +110,17 @@ static int bcm47xxsflash_read(struct mtd_info *mtd, loff_t from, size_t len,
>>>>>>       if ((from + len) > mtd->size)
>>>>>>               return -EINVAL;
>>>>>>
>>>>>> -     memcpy_fromio(buf, b47s->window + from, len);
>>>>>> +     if (from + len <= BCM47XXSFLASH_WINDOW_SIZE) {
>>>>>> +             memcpy_fromio(buf, b47s->window + from, len);
>>>>>
>>>>> One last nit, what if the read starts < 16 MiB and ends > 16 MiB ?
>>>>> Shouldn't that use partly the windowed mode and partly the other mode?
>>>>
>>>> You'll lost 10ns*... or not as splitting it into 2 code paths could
>>>> take longer, who knows. Most access are block aligned anyway. I don't
>>>> think such corner case with doubtful gain is much worth considering &
>>>> optimizing.
>>>
>>> So you can also read the first 16 MiB using the new method , right ?
>>
>> I could, but this could be noticeable in performance. Reading 16 MiB
>> using slower method is different from reading what... a few KiB? Are
>> you actually sure mtd does unaligned reads at all?
>
> No, but I'm quite sure the code above can be pushed to misbehave and I'm
> trying to confirm/refute that.
>
>> Please let's argue about real problems and not few % performance in
>> some corner case on some obsolete hardware with poor design.
>
> OK, real issue:
> $ dd if=/dev/mtdX of=/dev/null bs=32M
>
> How will this driver handle it ?

It results in reading using 0x400000 chunks. Now this is a good
argument to optimize bcm47xxsflash_read for reads crossing flash
window boundary.
[  401.927715] [bcm47xxsflash_read] len:0x400000
[  403.476166] [bcm47xxsflash_read] len:0x400000
(...)

-- 
Rafał