Re: [PATCH] drivers: mtd: m25p80: Add quad read support.

[Sourav Poddar <sourav.poddar@ti.com>]

Dear Marek,
On Monday 28 October 2013 12:00 AM, Marek Vasut wrote:
> Dear Sourav Poddar,
>
>> Dear Marek,
>>
>> On Sunday 27 October 2013 10:15 PM, Marek Vasut wrote:
>>> Dear Sourav Poddar,
>>>
>>> [...]
> [...]
>
>>>> @@ -774,7 +906,7 @@ static const struct spi_device_id m25p_ids[] = {
>>>>
>>>>    	{ "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) },
>>>>    	{ "mx25l25635e", INFO(0xc22019, 0, 64 * 1024, 512, 0) },
>>>>    	{ "mx25l25655e", INFO(0xc22619, 0, 64 * 1024, 512, 0) },
>>>>
>>>> -	{ "mx66l51235l", INFO(0xc2201a, 0, 64 * 1024, 1024, 0) },
>>>> +	{ "mx66l51235l", INFO(0xc2201a, 0, 64 * 1024, 1024, M25P80_QUAD_READ)
>>>> },
>>> I'm not convinced enabling 4-bit mode should be hard-coded in the MTD
>>> driver. There might be a board which uses this chip in 1-bit mode.
>>>
>>> We have a setup with Spansion chip here which uses 1-bit addressing in
>>> U-Boot, but uses 4-bit addressing in Linux. We use a DT property to
>>> configure the SPI bus width for that and I think that's a way to go.
>>> Note that there also are chips which use 2-bit wide SPI communication.
>> Yes, but if you trace down the patch below, you will see its not hard
>> coded. Enabling quad read mode depends on the following:
>>
>> 1. Whether flash chip supports it.
>>           This information comes from the flash which we are setting above.
>>
>> 2. Whether SPI controller supports it.
>>               This information is the check down below in the
>> patch(spi->mode&  SPI_RX_QUAD).
>>               spi->mode is set to "SINGLE/DUAL/QUAD" in the SPI framework
>> based on the
>>              dt property "spi-tx/rx-bus-width".
>> So, unless you set spi->mode to SPI_RX_QUAD by setting
>> "spi-rx-bus-width" to 4 in dt, default
>> will be 1 bit mode only.
> So this M25P80_QUAD_READ flag tells us "this chip supports quad read", not
> "force enable quad read" .
Yes, correct.
> All right, thanks for clearing this up. I hope it's
> documented somewhere elsewhere than in this email ;-)
Its added as a comment under struct flash_info where
this flag is defined.
> Best regards,
> Marek Vasut