[PATCH] RFC: mtd/spi-nor: add checking for the spi_nor_read

[Michal Suchanek]

On 14 August 2015 at 05:40, Hou Zhiqiang <B48286 at freescale.com> wrote:
>
>
>> -----Original Message-----
>> From: Michal Suchanek [mailto:hramrach at gmail.com]
>> Sent: 2015年8月13日 17:42
>> To: Hou Zhiqiang-B48286
>> Cc: linux-mtd at lists.infradead.org; computersforpeace at gmail.com;
>> dwmw2 at infradead.org; jonatas.rech at datacom.ind.br; Jane.Wan at gainspeed.com;
>> shijie.huang at intel.com; valentin.longchamp at keymile.com;
>> hkallweit1 at gmail.com; beanhuo at micron.com; Hu Mingkai-B21284
>> Subject: Re: [PATCH] RFC: mtd/spi-nor: add checking for the spi_nor_read
>>
>> On 13 August 2015 at 11:03, Hou Zhiqiang <B48286 at freescale.com> wrote:
>> > Hello,
>> >
>> > I want to add the 4-Byte addressing flashes support for the Freescale
>> > eSPI controller dirver. Appreciate your suggestions.
>> >
>> > Background:
>> > In the Freescale eSPI controller dirver, if the transaction length
>> > exceed 64KiB, the contents of the transaction was touched to update
>> > the command with assumed the 3-Byte address width. It is a workaround,
>> > due to the Freescale eSPI controller have a hardware limit that the
>> > maximum one-time transaction length is 64KiB, that isn't consistent
>> with many other vendors'
>> > SPI controllers, and so far the SPI flash driver assume the SPI
>> > controller have the ability to complete the specified transaction
>> length at one-time.
>> >
>> > But this workaround is only suit for 3-Byte addressing slaves, as time
>> > goes by, there are 4-Byte address width SPI flashes, so this
>> > assumption isn't correct and this workaround discombobulate the
>> > controller driver layer and protocol driver layer. So, this workaround
>> > should be removed and the SPI client driver should ensure the
>> transaction completion.
>> >
>> > There are two solutions:
>> > 1. In spi-nor framework, compare the retlen with the transaction
>> > length specified, if the retlen less than the transaction length and
>> > with no error number returned, re-initiate the transaction with the
>> updated address.
>> > Advantage:
>> > It won't affect other SPI controllers without this limit.
>> > Disadvantage:
>> > It is not a systematic solution.
>>
>> This can probably break transactions that are performed on non-flash
>> slaves but look like flash read command.
>>
>> >
>> > 2. Add quirk support, something like i2c quirk.
>> > Advantage:
>> > It is a systematic solution.
>> > Disadvantage:
>> > The quirk mechanism is only useful for Freescale eSPI controller, but
>> > it will affect whole SPI framework.
>>
>> I added this quirk in m25p80.c since my current SPI master driver is just
>> broken so I tried to work around that. While the quirk itself is rejected
>> as non-syetematic this series that adds the required support for checking
>> the return value of the SPI master driver transfer functions.
>>
>> So you have options:
>>
>> 2a: add SPI master quirk and check it in m25p80.c and truncate the
>> transfer in m25p80.c
>>
>> 2b: truncate the transfer in SPI master driver and just return the number
>> of bytes transferred
>>
>
> I expect this way, so the spi slave driver should check the retlen with the
> transaction length and do some loop transfer, and make sure getting all data
> this transaction wanted.
>
>> This patch series should handle fragmenting the transfers in spi-nor.c
>>
>> http://lists.infradead.org/pipermail/linux-mtd/2015-July/060466.html
>>
>
> Please correct me if I'm wrong, AFAIU the mtd layer will check the retlen and
> if it isn't equal to the transaction length, the EIO will be returned.

I never had this happen because with this patch series spi-nor checks
the amount of data transferred and continues until the whole length is
transferred. Without it just sets retlen to length regardless of
amount of data transferred.

Thanks

Michal