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

[Boris Brezillon <boris.brezillon@bootlin.com>]

On Thu, 12 Apr 2018 20:08:28 +0530
Vignesh R <vigneshr@ti.com> wrote:

> 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().

Hm, I always have a hard time using types which does not clearly say
how large they are, but okay.

> 
> > +	} 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.

Well, let's wait until we actually need 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/

Again, we'll see when we'll try to move them, but I hope still we won't
need that. Looks like the kind of information I'd like to keep away
from spi controller drivers.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/