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

From: Marek Vasut <marex@denx.de>
To: Michal Suchanek <hramrach@gmail.com>
Cc: Kamal Dasu <kdasu.kdev@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:20:14 +0100	[thread overview]
Message-ID: <c7cda789-0d00-7d0a-dc2d-175986575947@denx.de> (raw)
In-Reply-To: <CAOMqctS_izeoNJyN_7N=dmGMmvA8bpiqvmhQT2OmPR0H7btcXg@mail.gmail.com>

On 01/25/2017 07:29 AM, Michal Suchanek wrote:
> On 25 January 2017 at 03:08, 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 ?
> 
> spi_flash_read is designed to perform what m25p80_read does in a
> controller-specific way. So how can you get to a point when you are
> in spi_flash_read, it fails, and you do not know you if you can call
> the bottom half of m25p80_read to finish the job?

Huh ? You can't, but that's not what my question is about. I'd like to
understand what's the problem with the bcm controller in the first place.

-- 
Best regards,
Marek Vasut