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

[Marek Vasut]

On 01/17/2017 11:50 AM, Rafał Miłecki wrote:
> On 17 January 2017 at 11:49, Marek Vasut <marek.vasut at gmail.com> wrote:
>> On 01/17/2017 11:39 AM, Rafał Miłecki wrote:
>>> On 17 January 2017 at 10:49, Marek Vasut <marek.vasut at gmail.com> wrote:
>>>> On 01/17/2017 07:58 AM, Rafał Miłecki wrote:
>>>>> On 17 January 2017 at 04:18, Marek Vasut <marek.vasut at gmail.com> wrote:
>>>>>> On 01/16/2017 11:09 PM, 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>
>>>>>>> ---
>>>>>>>  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..5d57119 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);
>>>>>>> +     } else {
>>>>>>> +             size_t i;
>>>>>>> +
>>>>>>> +             for (i = 0; i < len; i++) {
>>>>>>> +                     b47s->cc_write(b47s, BCMA_CC_FLASHADDR, from++);
>>>>>>> +                     bcm47xxsflash_cmd(b47s, OPCODE_ST_READ4B);
>>>>>>> +                     *(buf++) = b47s->cc_read(b47s, BCMA_CC_FLASHDATA) & 0xff;
>>>>>>
>>>>>> Do you really need to send command on every write into the FLASHADDR
>>>>>> register ? Doesn't this hardware support some sort of seqeuntial reading?
>>>>>
>>>>> Yes, I need to. If there is some other way it's undocumented and not
>>>>> used by Broadcom. As a quick check I tried not writing to FLASHADDR
>>>>> every time, but it didn't work.
>>>>
>>>> Oh well, that's a bit sad. Thanks for checking.
>>>>
>>>>>> Are you sure this has no side-effects when you mix this with reading
>>>>>> from the flash window ?
>>>>>
>>>>> No side effects. I also tried reading whole flash content using this
>>>>> indirect access and it worked as well.
>>>>
>>>> Well, if you read the whole flash, you will trigger the first windowed
>>>> read and then the new code, in sequence. Try doing some read pattern
>>>> where you read from the beginning and mix it with reads from the end.
>>>> That will alternate between these two bits of code.
>>>
>>> I really can't follow your possible-fail scenario. With my patch first
>>> 16 MiB are read using windowed access, another 16 MiB using indirect
>>> access. I scanned whole flash this way, it worked. I got a working
>>> system. It just works...
>>>
>> Well if you scanned the whole flash, you triggered the memcpy_fromio()
>> first and then the b47s->cc_write(..) stuff , in that order. So does it
>> work if you read the first 16 MiB after you read the region past 16 MiB?
> 
> Yes, of course it does.

Ah cool, thanks for checking.

-- 
Best regards,
Marek Vasut