[PATCH, 1/2] mtd: m25p80: Let m25p80_read() fallback to spi transfer

[Marek Vasut]

On 01/25/2017 05:28 PM, Kamal Dasu wrote:
> If the transfers are short and dest buffer or the flash address are
> unaligned.

That sounds like a DMA problem where you're trying to fall back to PIO ?

> Also in case of older version of the controller there are
> some address mapping limitations when a transfer crosses 4MB window
> (addr + len).  So in such cases  need to fallback to normal MSPI
> reads.

But the driver can also detect this mode of failure before doing the
transfer and call it's internal functions to perform the transfer as
needed, right?

> One other option is that controller divers implementation of
> bcm_qspi_spi_flash_read() can return msg.retlen = 0 and the
> m25p80_read() can fallback to normal mspi read.

I'd much rather see the driver handling such detail internally instead
of patching the core code. Moreover, if you patch the core code, the SF
read will go - in case of a failure- all the way through the SPI
framework only to land in the same driver, which doesn't make much sense.

btw please do NOT top-post:
http://www.arm.linux.org.uk/mailinglists/etiquette.php#e3

> Kamal
> 
> 
> 
> On Tue, Jan 24, 2017 at 9:08 PM, Marek Vasut <marex at denx.de> wrote:
>> On 01/24/2017 12:41 AM, Kamal Dasu wrote:
>>> "ret can never be > 0 , it is only 0 or negative "
>>>
>>> I can fix this.
>>>
>>>>>> This looks really fragile and special-casing EINVAL here doesn't scale.
>>>>>> But still, if your controller driver is buggy, fix the driver, do not
>>>>>> pollute core code with workarounds. If you do support this sort of
>>>>>> accelerated read and it fails, it means something is seriously wrong.
>>>>>> If you need to invoke regular SPI reads to complete under some obscure
>>>>>> circumstances, do it from the driver, not here.
>>>>>
>>>>> I guess the other half of m25p80_read can be factored out and used as
>>>>> fallback from either m25p80_read or the controller driver.
>>>>
>>>> I think I see what you mean, but care to show an RFC patch ?
>>>>
>>>> --
>>>
>>> Its not the controller driver, but he hardware limitation with older
>>> controller version. I have tried to see how I can do this better,
>>> however when spi_flash_read() is called  cannot handle it within my
>>> driver without returning from the function. I went over this with Mark
>>> previously and this current solution seemed reasonable. Any other
>>> solution outside of the generic driver would replicate a lot of code
>>> unnecessarily.
>>
>> Hmmm, I kinda see the problem. I was thinking splitting the m25p80_read
>> function could be the solution and invoking the second part from the
>> driver if applicable, but this cannot work because the driver does not
>> know when it's interacting with SPI NOR and when with something else .
>>
>> Can you tell me about the conditions under which the bcm controller
>> fails and should fall back to standard spi read ?
>>
>> --
>> Best regards,
>> Marek Vasut


-- 
Best regards,
Marek Vasut