Re: [PATCH 2/2] mtd: m25p80: consider max_transfer_size when reading

[Heiner Kallweit <hkallweit1@xxxxxxxxx>]

Am 07.06.2016 um 08:03 schrieb Heiner Kallweit:
> Am 07.06.2016 um 01:07 schrieb Mark Brown:
>> On Mon, Jun 06, 2016 at 08:53:10PM +0200, Heiner Kallweit wrote:
>>> Am 06.06.2016 um 19:40 schrieb Mark Brown:
>>
>>>> That doesn't make any sense, the controller hardware doesn't 
>>>> magically change based on what is connected to it.
>>
>>> The issue with fsl-espi is that the controller deactivates CS after
>>> each physical transfer. And a lot of HW designs use the hardware CS,
>>> therefore the advise to use a GPIO instead doesn't really help.
>>
>> There's no pinmux to fix the broken behaviour?  The Freescale SPI
>> controllers really are terrible :(
>>
>>> In case of SPI NOR CS needs to stay active between command and data
>>
>> In the case of *all* SPI transactions this needs to happen.  This is
>> *not* optional, it's not some weird requirement of NOR, it's one of the
>> most basic requirements for a SPI driver on Linux.
>>
>>> Well, we could reduce the max_transfer_size exposed by the driver by
>>> the max length of a m25p80 read command in general.
>>
>> No!  Apart from anything else this would be a complete and total
>> abstraction failure.  This is a driver for a SPI controller, not a
>> driver for a flash controller, which means that it should not have flash
>> specific hacks in it.
>>
>>> Then we might be too strict for other SPI devices, but this may
>>> be acceptable as the current fsl-espi driver is usable with SPI NOR
>>> devices only anyway.
>>
>> What makes you say this?  I'm guessing that the driver is just broken
>> but it would be good to understand the specifics.
>>
> See fsl_espi_rw_trans():
> In case of transfers > SPCOM_TRANLEN_MAX it loops and reads in
> SPCOM_TRANLEN_MAX chunks whilst replacing bytes 2-4 of the
> message buffer with the new start address to read from.
> Having said that it implicitely assumes that the first transfer in
> the message is a m25p80 read command with a 3 byte address.
> 
> And in fsl_espi_do_one_msg() it assumes that each message includes
> max one r/o transfer (else rx_buf is overwritten).
> 
> I'm not 100% sure whether any arbitrary combination of r/o, r/w, w/o
> transfers in a SPI message is acceptable. But this controller driver
> supports just the transfer combinations generated by the m25p80
> protocol driver.
> 
I think in this discussion there was the idea already to introduce
a message size limit complementing the transfer size limit.
Could this be an acceptable way to go?
If yes I'd prepare a patch as basis for further discussion.

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html