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

From: Kamal Dasu <kdasu.kdev@gmail.com>
To: Marek Vasut <marex@denx.de>
Cc: Michal Suchanek <hramrach@gmail.com>,
	Cyrille Pitchen <cyrille.pitchen@atmel.com>,
	Mark Brown <broonie@kernel.org>,
	bcm-kernel-feedback-list <bcm-kernel-feedback-list@broadcom.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	MTD Maling List <linux-mtd@lists.infradead.org>
Subject: Re: [PATCH, 1/2] mtd: m25p80: Let m25p80_read() fallback to spi transfer
Date: Wed, 25 Jan 2017 11:28:29 -0500	[thread overview]
Message-ID: <CAC=U0a3mhruPtHA-1y08noS_Vq1y2KrBYiEM-GbpwLn_Eou7=A@mail.gmail.com> (raw)
In-Reply-To: <62931580-f21f-e6a1-8e8b-d3c92195a2e0@denx.de>

If the transfers are short and dest buffer or the flash address are
unaligned. 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.

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.

Kamal

On Tue, Jan 24, 2017 at 9:08 PM, Marek Vasut <marex@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