Re: [PATCH v4 4/8] mtd: spi-nor: add support of SPI protocols like SPI 1-2-2 and SPI 1-4-4

From: Cyrille Pitchen
Date: Mon Dec 19 2016 - 12:00:29 EST


Le 16/12/2016 à 14:47, Krzeminski, Marcin (Nokia - PL/Wroclaw) a écrit :
>> -----Original Message-----
>> From: Krzeminski, Marcin (Nokia - PL/Wroclaw)
>> Sent: Tuesday, December 13, 2016 10:46 AM
>> To: Cyrille Pitchen <cyrille.pitchen@xxxxxxxxx>;
>> computersforpeace@xxxxxxxxx; marek.vasut@xxxxxxxxx;
>> boris.brezillon@xxxxxxxxxxxxxxxxxx; richard@xxxxxx; linux-
>> mtd@xxxxxxxxxxxxxxxxxxx
>> Cc: nicolas.ferre@xxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
>> Subject: RE: [PATCH v4 4/8] mtd: spi-nor: add support of SPI protocols like SPI
>> 1-2-2 and SPI 1-4-4
>>
>> Cyrille,
>>
>>> -----Original Message-----
>>> From: linux-mtd [mailto:linux-mtd-bounces@xxxxxxxxxxxxxxxxxxx] On
>>> Behalf Of Cyrille Pitchen
>>> Sent: Monday, November 21, 2016 3:16 PM
>>> To: computersforpeace@xxxxxxxxx; marek.vasut@xxxxxxxxx;
>>> boris.brezillon@xxxxxxxxxxxxxxxxxx; richard@xxxxxx; linux-
>>> mtd@xxxxxxxxxxxxxxxxxxx
>>> Cc: Cyrille Pitchen <cyrille.pitchen@xxxxxxxxx>;
>>> nicolas.ferre@xxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
>>> Subject: [PATCH v4 4/8] mtd: spi-nor: add support of SPI protocols
>>> like SPI 1-
>>> 2-2 and SPI 1-4-4
>>>
>>> This patch changes the prototype of spi_nor_scan(): its 3rd parameter
>>> is replaced by a const struct spi_nor_modes pointer, which tells the
>>> spi-nor framework about which SPI protocols are supported by the SPI
>> controller.
>>>
>>> Besides, this patch also introduces a new
>>> spi_nor_basic_flash_parameter structure telling the spi-nor framework
>>> about the SPI protocols supported by the SPI memory and the needed op
>> codes to use these SPI protocols.
>>>
>>> Currently the struct spi_nor_basic_flash_parameter is filled with
>>> legacy values but a later patch will allow to fill it dynamically by
>>> reading the
>>> JESD216 Serial Flash Discoverable Parameter (SFDP) tables from the SPI
>>> memory.
>>>
>>> With both structures, the spi-nor framework can now compute the best
>>> match between SPI protocols supported by both the (Q)SPI memory and
>>> controller hence selecting the relevant op codes for (Fast) Read, Page
>>> Program and Sector Erase operations.
>>>
>>> The spi_nor_basic_flash_parameter structure also provides the spi-nor
>>> framework with the number of dummy cycles to be used with each Fast
>>> Read commands and the erase block size associated to the erase block
>>> op codes.
>>>
>>> Finally the spi_nor_basic_flash_parameter structure, through the
>>> optional
>>> .enable_quad_io() hook, tells the spi-nor framework how to set the
>>> Quad Enable (QE) bit of the QSPI memory to enable its Quad SPI features.
>>>
>>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@xxxxxxxxx>
>>> ---
>>> drivers/mtd/devices/m25p80.c | 17 ++-
>>> drivers/mtd/spi-nor/atmel-quadspi.c | 83 ++++++----
>>> drivers/mtd/spi-nor/cadence-quadspi.c | 18 ++-
>>> drivers/mtd/spi-nor/fsl-quadspi.c | 8 +-
>>> drivers/mtd/spi-nor/hisi-sfc.c | 32 +++-
>>> drivers/mtd/spi-nor/mtk-quadspi.c | 16 +-
>>> drivers/mtd/spi-nor/nxp-spifi.c | 21 +--
>>> drivers/mtd/spi-nor/spi-nor.c | 280
>> +++++++++++++++++++++++++++-
>>> ------
[...]
>>> +static int spi_nor_setup(struct spi_nor *nor, const struct flash_info *info,
>>> + const struct spi_nor_basic_flash_parameter
>>> *params,
>>> + const struct spi_nor_modes *modes)
>>> {
>>> + bool enable_quad_io;
>>> + u32 rd_modes, wr_modes, mask;
>>> + const struct spi_nor_erase_type *erase_type = NULL;
>>> + const struct spi_nor_read *read;
>>> + int rd_pindex, wr_pindex, i, err = 0;
>>> + u8 erase_size = SNOR_ERASE_64K;
>>
>> Erase size could be configurable, then user can chose best sector size that
>> match his use case on multi-sized flash.
>>
>>> +
>>> + /* 2-2-2 or 4-4-4 modes are not supported yet. */
>>> + mask = (SNOR_MODE_2_2_2 | SNOR_MODE_4_4_4);
> IMHO could be nice to put a warning here :)
>

Then maybe only a dev_dbg() because many developers complain that there are
already too many debug messages in the boot logs.

> Thanks,
> Marcin
>