Re: [PATCH v2 04/10] spi: Extend the core to ease integration of SPI memory controllers

[Vignesh R <vigneshr@ti.com>]

Hi,

- Sourav Poddar <sourav.poddar@ti.com> mail ID no longer valid.

On Wednesday 11 April 2018 04:14 AM, Boris Brezillon wrote:
> Some controllers are exposing high-level interfaces to access various
> kind of SPI memories. Unfortunately they do not fit in the current
> spi_controller model and usually have drivers placed in
> drivers/mtd/spi-nor which are only supporting SPI NORs and not SPI
> memories in general.
> 
> This is an attempt at defining a SPI memory interface which works for
> all kinds of SPI memories (NORs, NANDs, SRAMs).
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---

[...]

> +/**
> + * struct spi_mem_op - describes a SPI memory operation
> + * @cmd.buswidth: number of IO lines used to transmit the command
> + * @cmd.opcode: operation opcode
> + * @addr.nbytes: number of address bytes to send. Can be zero if the operation
> + *		 does not need to send an address
> + * @addr.buswidth: number of IO lines used to transmit the address cycles
> + * @addr.val: address value. This value is always sent MSB first on the bus.
> + *	      Note that only @addr.nbytes are taken into account in this
> + *	      address value, so users should make sure the value fits in the
> + *	      assigned number of bytes.
> + * @dummy.nbytes: number of dummy bytes to send after an opcode or address. Can
> + *		  be zero if the operation does not require dummy bytes
> + * @dummy.buswidth: number of IO lanes used to transmit the dummy bytes
> + * @data.buswidth: number of IO lanes used to send/receive the data
> + * @data.dir: direction of the transfer
> + * @data.buf.in: input buffer
> + * @data.buf.out: output buffer
> + */
> +struct spi_mem_op {
> +	struct {
> +		u8 buswidth;
> +		u8 opcode;
> +	} cmd;
> +
> +	struct {
> +		u8 nbytes;
> +		u8 buswidth;
> +		u64 val;

You could consider using loff_t here and be consistent with
spi_nor_read/write() API as well as mtd->_read().

> +	} addr;
> +
> +	struct {
> +		u8 nbytes;
> +		u8 buswidth;
> +	} dummy;
> +
> +	struct {
> +		u8 buswidth;
> +		enum spi_mem_data_dir dir;
> +		unsigned int nbytes;
> +		/* buf.{in,out} must be DMA-able. */
> +		union {
> +			void *in;
> +			const void *out;
> +		} buf;
> +	} data;
> +};
> +

Some flash devices support Dual/Quad DDR (Double Data Rate) mode and the
SPI controller driver would need to know this information. We will need
to add a field for that.

Currently, there are drivers under mtd/spi-nor/ that need to know
page/sector/total size of flash memory(info available in
-`struct spi_nor). We would need a way to provide this info to spi_mem
drivers, if we ever plan to move drivers under mtd/spi-nor to spi/


[...]
-- 
Regards
Vignesh