* SPI: support DUAL and QUAD[patch v1] @ 2013-07-10 8:34 ` yuhang wang 0 siblings, 0 replies; 40+ messages in thread From: yuhang wang @ 2013-07-10 8:34 UTC (permalink / raw) To: Mark Brown Cc: linux-mtd, Sourav Poddar, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f Hi, The patch below is to make spi framework support dual(2x) and quad(4x). Bus the name of members may still look strange, so I need some suggestions. Thanks. >From f33f5935776aa0b44b01a36f88e47ba0cfe8fd21 Mon Sep 17 00:00:00 2001 From: wangyuhang <wangyuhang2014-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Date: Wed, 10 Jul 2013 16:22:19 +0800 Subject: [PATCH] spi final patch v2 Signed-off-by: wangyuhang <wangyuhang2014-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Signed-off-by: wangyuhang <wangyuhang2014-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> --- drivers/spi/spi.c | 6 ++++++ include/linux/spi/spi.h | 31 +++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 004b10f..af28a62 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -452,6 +452,8 @@ struct spi_device *spi_new_device(struct spi_master *master, proxy->irq = chip->irq; strlcpy(proxy->modalias, chip->modalias, sizeof(proxy->modalias)); proxy->dev.platform_data = (void *) chip->platform_data; + proxy->tx_nbits = chip->tx_nbits ? chip->tx_nbits : SPI_NBITS_SINGLE; + proxy->rx_nbits = chip->rx_nbits ? chip->rx_nbits : SPI_NBITS_SINGLE; proxy->controller_data = chip->controller_data; proxy->controller_state = NULL; @@ -1376,6 +1378,10 @@ static int __spi_async(struct spi_device *spi, struct spi_message *message) xfer->bits_per_word = spi->bits_per_word; if (!xfer->speed_hz) xfer->speed_hz = spi->max_speed_hz; + if (!xfer->tx_nbits) + xfer->tx_nbits = SPI_NBITS_SINGLE; + if (!xfer->rx_nbits) + xfer->rx_nbits = SPI_NBITS_SINGLE; } message->spi = spi; diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 38c2b92..d9b3746 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -59,6 +59,10 @@ extern struct bus_type spi_bus_type; * for driver coldplugging, and in uevents used for hotplugging * @cs_gpio: gpio number of the chipselect line (optional, -EINVAL when * when not using a GPIO line) + * @tx_nbits: number of bits used in tx, get from @spi_board_info + * (optional, 1bit as init value if not set in @spi_board_info) + * @rx_nbits: number of bits used in rx, get from @spi_board_info + * (optional, 1bit as init value if not set in @spi_board_info) * * A @spi_device is used to interchange data between an SPI slave * (usually a discrete chip) and CPU memory. @@ -93,6 +97,8 @@ struct spi_device { void *controller_data; char modalias[SPI_NAME_SIZE]; int cs_gpio; /* chip select gpio */ + u8 tx_nbits; /* num of bits used in tx */ + u8 rx_nbits; /* num of bits used in rx */ /* * likely need more hooks for more protocol options affecting how @@ -437,6 +443,10 @@ extern struct spi_master *spi_busnum_to_master(u16 busnum); * @rx_buf: data to be read (dma-safe memory), or NULL * @tx_dma: DMA address of tx_buf, if @spi_message.is_dma_mapped * @rx_dma: DMA address of rx_buf, if @spi_message.is_dma_mapped + * @tx_nbits: number of bits used for writting, if 0 default + * (SPI_NBITS_SINGLE = 0x01) is used. + * @rx_nbits: number of bits used for reading, if 0 default + * (SPI_NBITS_SINGLE = 0x01) is used. * @len: size of rx and tx buffers (in bytes) * @speed_hz: Select a speed other than the device default for this * transfer. If 0 the default (from @spi_device) is used. @@ -491,6 +501,11 @@ extern struct spi_master *spi_busnum_to_master(u16 busnum); * by the results of previous messages and where the whole transaction * ends when the chipselect goes intactive. * + * When SPI can transfer in 1x,2x or 4x. It can get this tranfer information + * from device through @tx_nbits and @rx_nbits. In Bi-direction, these + * two should both be set. User can set transfer mode with SPI_NBITS_SINGLE(1x) + * SPI_NBITS_DUAL(2x) and SPI_NBITS_QUAD(4x) to support these three transfer. + * * The code that submits an spi_message (and its spi_transfers) * to the lower layers is responsible for managing its memory. * Zero-initialize every field you don't set up explicitly, to @@ -510,6 +525,12 @@ struct spi_transfer { dma_addr_t tx_dma; dma_addr_t rx_dma; + u8 tx_nbits; + u8 rx_nbits; +#define SPI_NBITS_SINGLE 0x01; /* 1bit transfer */ +#define SPI_NBITS_DUAL 0x02; /* 2bits transfer */ +#define SPI_NBITS_QUAD 0x03; /* 4bits transfer */ + unsigned cs_change:1; u8 bits_per_word; u16 delay_usecs; @@ -815,6 +836,10 @@ static inline ssize_t spi_w8r16(struct spi_device *spi, u8 cmd) * @mode: Initializes spi_device.mode; based on the chip datasheet, board * wiring (some devices support both 3WIRE and standard modes), and * possibly presence of an inverter in the chipselect path. + * @tx_nbits: Initializes spi_device.tx_nbits; depends on the number of bits + * the board used in tx. + * @rx_nbits: Initializes spi_device.rx_nbits; depends on the number of bits + * the board used in rx. * * When adding new SPI devices to the device tree, these structures serve * as a partial device template. They hold information which can't always @@ -859,6 +884,12 @@ struct spi_board_info { * where the default of SPI_CS_HIGH = 0 is wrong. */ u8 mode; + /* tx_nbits initialized for spi_device.tx_nbits and + * rx_nbits initialized for spi_device.rx_nbits. These members + * used to describe the how many lines used in tx and rx. + */ + u8 tx_nbits; + u8 rx_nbits; /* ... may need additional spi_device chip config data here. * avoid stuff protocol drivers can set; but include stuff -- 1.7.9.5 Best Regards. ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk ^ permalink raw reply related [flat|nested] 40+ messages in thread
* SPI: support DUAL and QUAD[patch v1] @ 2013-07-10 8:34 ` yuhang wang 0 siblings, 0 replies; 40+ messages in thread From: yuhang wang @ 2013-07-10 8:34 UTC (permalink / raw) To: Mark Brown Cc: linux-mtd, Grant Likely, Sourav Poddar, linux-mtd, spi-devel-general Hi, The patch below is to make spi framework support dual(2x) and quad(4x). Bus the name of members may still look strange, so I need some suggestions. Thanks. >From f33f5935776aa0b44b01a36f88e47ba0cfe8fd21 Mon Sep 17 00:00:00 2001 From: wangyuhang <wangyuhang2014@gmail.com> Date: Wed, 10 Jul 2013 16:22:19 +0800 Subject: [PATCH] spi final patch v2 Signed-off-by: wangyuhang <wangyuhang2014@gmail.com> Signed-off-by: wangyuhang <wangyuhang2014@gmail.com> --- drivers/spi/spi.c | 6 ++++++ include/linux/spi/spi.h | 31 +++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 004b10f..af28a62 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -452,6 +452,8 @@ struct spi_device *spi_new_device(struct spi_master *master, proxy->irq = chip->irq; strlcpy(proxy->modalias, chip->modalias, sizeof(proxy->modalias)); proxy->dev.platform_data = (void *) chip->platform_data; + proxy->tx_nbits = chip->tx_nbits ? chip->tx_nbits : SPI_NBITS_SINGLE; + proxy->rx_nbits = chip->rx_nbits ? chip->rx_nbits : SPI_NBITS_SINGLE; proxy->controller_data = chip->controller_data; proxy->controller_state = NULL; @@ -1376,6 +1378,10 @@ static int __spi_async(struct spi_device *spi, struct spi_message *message) xfer->bits_per_word = spi->bits_per_word; if (!xfer->speed_hz) xfer->speed_hz = spi->max_speed_hz; + if (!xfer->tx_nbits) + xfer->tx_nbits = SPI_NBITS_SINGLE; + if (!xfer->rx_nbits) + xfer->rx_nbits = SPI_NBITS_SINGLE; } message->spi = spi; diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 38c2b92..d9b3746 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -59,6 +59,10 @@ extern struct bus_type spi_bus_type; * for driver coldplugging, and in uevents used for hotplugging * @cs_gpio: gpio number of the chipselect line (optional, -EINVAL when * when not using a GPIO line) + * @tx_nbits: number of bits used in tx, get from @spi_board_info + * (optional, 1bit as init value if not set in @spi_board_info) + * @rx_nbits: number of bits used in rx, get from @spi_board_info + * (optional, 1bit as init value if not set in @spi_board_info) * * A @spi_device is used to interchange data between an SPI slave * (usually a discrete chip) and CPU memory. @@ -93,6 +97,8 @@ struct spi_device { void *controller_data; char modalias[SPI_NAME_SIZE]; int cs_gpio; /* chip select gpio */ + u8 tx_nbits; /* num of bits used in tx */ + u8 rx_nbits; /* num of bits used in rx */ /* * likely need more hooks for more protocol options affecting how @@ -437,6 +443,10 @@ extern struct spi_master *spi_busnum_to_master(u16 busnum); * @rx_buf: data to be read (dma-safe memory), or NULL * @tx_dma: DMA address of tx_buf, if @spi_message.is_dma_mapped * @rx_dma: DMA address of rx_buf, if @spi_message.is_dma_mapped + * @tx_nbits: number of bits used for writting, if 0 default + * (SPI_NBITS_SINGLE = 0x01) is used. + * @rx_nbits: number of bits used for reading, if 0 default + * (SPI_NBITS_SINGLE = 0x01) is used. * @len: size of rx and tx buffers (in bytes) * @speed_hz: Select a speed other than the device default for this * transfer. If 0 the default (from @spi_device) is used. @@ -491,6 +501,11 @@ extern struct spi_master *spi_busnum_to_master(u16 busnum); * by the results of previous messages and where the whole transaction * ends when the chipselect goes intactive. * + * When SPI can transfer in 1x,2x or 4x. It can get this tranfer information + * from device through @tx_nbits and @rx_nbits. In Bi-direction, these + * two should both be set. User can set transfer mode with SPI_NBITS_SINGLE(1x) + * SPI_NBITS_DUAL(2x) and SPI_NBITS_QUAD(4x) to support these three transfer. + * * The code that submits an spi_message (and its spi_transfers) * to the lower layers is responsible for managing its memory. * Zero-initialize every field you don't set up explicitly, to @@ -510,6 +525,12 @@ struct spi_transfer { dma_addr_t tx_dma; dma_addr_t rx_dma; + u8 tx_nbits; + u8 rx_nbits; +#define SPI_NBITS_SINGLE 0x01; /* 1bit transfer */ +#define SPI_NBITS_DUAL 0x02; /* 2bits transfer */ +#define SPI_NBITS_QUAD 0x03; /* 4bits transfer */ + unsigned cs_change:1; u8 bits_per_word; u16 delay_usecs; @@ -815,6 +836,10 @@ static inline ssize_t spi_w8r16(struct spi_device *spi, u8 cmd) * @mode: Initializes spi_device.mode; based on the chip datasheet, board * wiring (some devices support both 3WIRE and standard modes), and * possibly presence of an inverter in the chipselect path. + * @tx_nbits: Initializes spi_device.tx_nbits; depends on the number of bits + * the board used in tx. + * @rx_nbits: Initializes spi_device.rx_nbits; depends on the number of bits + * the board used in rx. * * When adding new SPI devices to the device tree, these structures serve * as a partial device template. They hold information which can't always @@ -859,6 +884,12 @@ struct spi_board_info { * where the default of SPI_CS_HIGH = 0 is wrong. */ u8 mode; + /* tx_nbits initialized for spi_device.tx_nbits and + * rx_nbits initialized for spi_device.rx_nbits. These members + * used to describe the how many lines used in tx and rx. + */ + u8 tx_nbits; + u8 rx_nbits; /* ... may need additional spi_device chip config data here. * avoid stuff protocol drivers can set; but include stuff -- 1.7.9.5 Best Regards. ^ permalink raw reply related [flat|nested] 40+ messages in thread
[parent not found: <CAHSAbzP0Nc1ahf_ra6yDuMvV998_+EGcUOGDv5ifKaagYf+NOg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* RE: support DUAL and QUAD[patch v1] 2013-07-10 8:34 ` yuhang wang @ 2013-07-16 8:08 ` Gupta, Pekon -1 siblings, 0 replies; 40+ messages in thread From: Gupta, Pekon @ 2013-07-16 8:08 UTC (permalink / raw) To: yuhang wang, Mark Brown Cc: linux-mtd, Poddar, Sourav, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f > Hi, > > The patch below is to make spi framework support dual(2x) and quad(4x). > Bus the name of members may still look strange, so I need some > suggestions. > Thanks. > > From f33f5935776aa0b44b01a36f88e47ba0cfe8fd21 Mon Sep 17 00:00:00 2001 > From: wangyuhang <wangyuhang2014-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > Date: Wed, 10 Jul 2013 16:22:19 +0800 > Subject: [PATCH] spi final patch v2 Signed-off-by: wangyuhang > <wangyuhang2014-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > > Signed-off-by: wangyuhang <wangyuhang2014-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > --- > drivers/spi/spi.c | 6 ++++++ > include/linux/spi/spi.h | 31 +++++++++++++++++++++++++++++++ > 2 files changed, 37 insertions(+) > > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c > index 004b10f..af28a62 100644 > --- a/drivers/spi/spi.c > +++ b/drivers/spi/spi.c > @@ -452,6 +452,8 @@ struct spi_device *spi_new_device(struct spi_master > *master, > proxy->irq = chip->irq; > strlcpy(proxy->modalias, chip->modalias, sizeof(proxy->modalias)); > proxy->dev.platform_data = (void *) chip->platform_data; > + proxy->tx_nbits = chip->tx_nbits ? chip->tx_nbits : SPI_NBITS_SINGLE; > + proxy->rx_nbits = chip->rx_nbits ? chip->rx_nbits : SPI_NBITS_SINGLE; > proxy->controller_data = chip->controller_data; > proxy->controller_state = NULL; > [Pekon]: You can use existing 'mode_bits' field in struct spi_master to pass on the controller mode. If (master->mode & QSPI_MODE) // configure for QSPI in prepare_transfer_hardware() else // continue with normal SPI > @@ -1376,6 +1378,10 @@ static int __spi_async(struct spi_device *spi, > struct spi_message *message) > xfer->bits_per_word = spi->bits_per_word; > if (!xfer->speed_hz) > xfer->speed_hz = spi->max_speed_hz; > + if (!xfer->tx_nbits) > + xfer->tx_nbits = SPI_NBITS_SINGLE; > + if (!xfer->rx_nbits) > + xfer->rx_nbits = SPI_NBITS_SINGLE; > } > > message->spi = spi; > diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h > index 38c2b92..d9b3746 100644 > --- a/include/linux/spi/spi.h > +++ b/include/linux/spi/spi.h > @@ -59,6 +59,10 @@ extern struct bus_type spi_bus_type; > * for driver coldplugging, and in uevents used for hotplugging > * @cs_gpio: gpio number of the chipselect line (optional, -EINVAL when > * when not using a GPIO line) > + * @tx_nbits: number of bits used in tx, get from @spi_board_info > + * (optional, 1bit as init value if not set in @spi_board_info) > + * @rx_nbits: number of bits used in rx, get from @spi_board_info > + * (optional, 1bit as init value if not set in @spi_board_info) > * > * A @spi_device is used to interchange data between an SPI slave > * (usually a discrete chip) and CPU memory. > @@ -93,6 +97,8 @@ struct spi_device { > void *controller_data; > char modalias[SPI_NAME_SIZE]; > int cs_gpio; /* chip select gpio */ > + u8 tx_nbits; /* num of bits used in tx */ > + u8 rx_nbits; /* num of bits used in rx */ > [Pekon]: Instead of add new fields, You can extend existing 'u8 mode' field in struct spi_device to 'u32 mode' and then use it for identifying command OPCODES. Like below.. If (device->mode & QSPI_MODE) // send QSPI specific OPCODE before transfer else // continue with normal SPI > /* > * likely need more hooks for more protocol options affecting how > @@ -437,6 +443,10 @@ extern struct spi_master > *spi_busnum_to_master(u16 busnum); > * @rx_buf: data to be read (dma-safe memory), or NULL > * @tx_dma: DMA address of tx_buf, if @spi_message.is_dma_mapped > * @rx_dma: DMA address of rx_buf, if @spi_message.is_dma_mapped > + * @tx_nbits: number of bits used for writting, if 0 default > + * (SPI_NBITS_SINGLE = 0x01) is used. > + * @rx_nbits: number of bits used for reading, if 0 default > + * (SPI_NBITS_SINGLE = 0x01) is used. > * @len: size of rx and tx buffers (in bytes) > * @speed_hz: Select a speed other than the device default for this > * transfer. If 0 the default (from @spi_device) is used. > @@ -491,6 +501,11 @@ extern struct spi_master > *spi_busnum_to_master(u16 busnum); > * by the results of previous messages and where the whole transaction > * ends when the chipselect goes intactive. > * > + * When SPI can transfer in 1x,2x or 4x. It can get this tranfer information > + * from device through @tx_nbits and @rx_nbits. In Bi-direction, these > + * two should both be set. User can set transfer mode with > SPI_NBITS_SINGLE(1x) > + * SPI_NBITS_DUAL(2x) and SPI_NBITS_QUAD(4x) to support these three > transfer. > + * > * The code that submits an spi_message (and its spi_transfers) > * to the lower layers is responsible for managing its memory. > * Zero-initialize every field you don't set up explicitly, to > @@ -510,6 +525,12 @@ struct spi_transfer { > dma_addr_t tx_dma; > dma_addr_t rx_dma; > > + u8 tx_nbits; > + u8 rx_nbits; > +#define SPI_NBITS_SINGLE 0x01; /* 1bit transfer */ > +#define SPI_NBITS_DUAL 0x02; /* 2bits transfer */ > +#define SPI_NBITS_QUAD 0x03; /* 4bits transfer */ > + > unsigned cs_change:1; > u8 bits_per_word; > u16 delay_usecs; > @@ -815,6 +836,10 @@ static inline ssize_t spi_w8r16(struct spi_device > *spi, u8 cmd) > * @mode: Initializes spi_device.mode; based on the chip datasheet, board > * wiring (some devices support both 3WIRE and standard modes), and > * possibly presence of an inverter in the chipselect path. > + * @tx_nbits: Initializes spi_device.tx_nbits; depends on the number of bits > + * the board used in tx. > + * @rx_nbits: Initializes spi_device.rx_nbits; depends on the number of bits > + * the board used in rx. > * > * When adding new SPI devices to the device tree, these structures serve > * as a partial device template. They hold information which can't always > @@ -859,6 +884,12 @@ struct spi_board_info { > * where the default of SPI_CS_HIGH = 0 is wrong. > */ > u8 mode; > + /* tx_nbits initialized for spi_device.tx_nbits and > + * rx_nbits initialized for spi_device.rx_nbits. These members > + * used to describe the how many lines used in tx and rx. > + */ > + u8 tx_nbits; > + u8 rx_nbits; > [Pekon]: Instead of adding new fields you can use existing 'mode' field to pass on the platform specific configurations. And if 'u8 mode' does not suffice you can increase it to 'u32'. #define QSPI_MODE 1 << 5; // just check which bit-fields are un-used spi_board_info->mode |= QSPI_MODE; Same way you can pass the configuration to spi_master and spi_device. > /* ... may need additional spi_device chip config data here. > * avoid stuff protocol drivers can set; but include stuff > -- > 1.7.9.5 > with regards, pekon ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk ^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: support DUAL and QUAD[patch v1] @ 2013-07-16 8:08 ` Gupta, Pekon 0 siblings, 0 replies; 40+ messages in thread From: Gupta, Pekon @ 2013-07-16 8:08 UTC (permalink / raw) To: yuhang wang, Mark Brown Cc: linux-mtd, Grant Likely, Poddar, Sourav, linux-mtd, spi-devel-general > Hi, > > The patch below is to make spi framework support dual(2x) and quad(4x). > Bus the name of members may still look strange, so I need some > suggestions. > Thanks. > > From f33f5935776aa0b44b01a36f88e47ba0cfe8fd21 Mon Sep 17 00:00:00 2001 > From: wangyuhang <wangyuhang2014@gmail.com> > Date: Wed, 10 Jul 2013 16:22:19 +0800 > Subject: [PATCH] spi final patch v2 Signed-off-by: wangyuhang > <wangyuhang2014@gmail.com> > > Signed-off-by: wangyuhang <wangyuhang2014@gmail.com> > --- > drivers/spi/spi.c | 6 ++++++ > include/linux/spi/spi.h | 31 +++++++++++++++++++++++++++++++ > 2 files changed, 37 insertions(+) > > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c > index 004b10f..af28a62 100644 > --- a/drivers/spi/spi.c > +++ b/drivers/spi/spi.c > @@ -452,6 +452,8 @@ struct spi_device *spi_new_device(struct spi_master > *master, > proxy->irq = chip->irq; > strlcpy(proxy->modalias, chip->modalias, sizeof(proxy->modalias)); > proxy->dev.platform_data = (void *) chip->platform_data; > + proxy->tx_nbits = chip->tx_nbits ? chip->tx_nbits : SPI_NBITS_SINGLE; > + proxy->rx_nbits = chip->rx_nbits ? chip->rx_nbits : SPI_NBITS_SINGLE; > proxy->controller_data = chip->controller_data; > proxy->controller_state = NULL; > [Pekon]: You can use existing 'mode_bits' field in struct spi_master to pass on the controller mode. If (master->mode & QSPI_MODE) // configure for QSPI in prepare_transfer_hardware() else // continue with normal SPI > @@ -1376,6 +1378,10 @@ static int __spi_async(struct spi_device *spi, > struct spi_message *message) > xfer->bits_per_word = spi->bits_per_word; > if (!xfer->speed_hz) > xfer->speed_hz = spi->max_speed_hz; > + if (!xfer->tx_nbits) > + xfer->tx_nbits = SPI_NBITS_SINGLE; > + if (!xfer->rx_nbits) > + xfer->rx_nbits = SPI_NBITS_SINGLE; > } > > message->spi = spi; > diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h > index 38c2b92..d9b3746 100644 > --- a/include/linux/spi/spi.h > +++ b/include/linux/spi/spi.h > @@ -59,6 +59,10 @@ extern struct bus_type spi_bus_type; > * for driver coldplugging, and in uevents used for hotplugging > * @cs_gpio: gpio number of the chipselect line (optional, -EINVAL when > * when not using a GPIO line) > + * @tx_nbits: number of bits used in tx, get from @spi_board_info > + * (optional, 1bit as init value if not set in @spi_board_info) > + * @rx_nbits: number of bits used in rx, get from @spi_board_info > + * (optional, 1bit as init value if not set in @spi_board_info) > * > * A @spi_device is used to interchange data between an SPI slave > * (usually a discrete chip) and CPU memory. > @@ -93,6 +97,8 @@ struct spi_device { > void *controller_data; > char modalias[SPI_NAME_SIZE]; > int cs_gpio; /* chip select gpio */ > + u8 tx_nbits; /* num of bits used in tx */ > + u8 rx_nbits; /* num of bits used in rx */ > [Pekon]: Instead of add new fields, You can extend existing 'u8 mode' field in struct spi_device to 'u32 mode' and then use it for identifying command OPCODES. Like below.. If (device->mode & QSPI_MODE) // send QSPI specific OPCODE before transfer else // continue with normal SPI > /* > * likely need more hooks for more protocol options affecting how > @@ -437,6 +443,10 @@ extern struct spi_master > *spi_busnum_to_master(u16 busnum); > * @rx_buf: data to be read (dma-safe memory), or NULL > * @tx_dma: DMA address of tx_buf, if @spi_message.is_dma_mapped > * @rx_dma: DMA address of rx_buf, if @spi_message.is_dma_mapped > + * @tx_nbits: number of bits used for writting, if 0 default > + * (SPI_NBITS_SINGLE = 0x01) is used. > + * @rx_nbits: number of bits used for reading, if 0 default > + * (SPI_NBITS_SINGLE = 0x01) is used. > * @len: size of rx and tx buffers (in bytes) > * @speed_hz: Select a speed other than the device default for this > * transfer. If 0 the default (from @spi_device) is used. > @@ -491,6 +501,11 @@ extern struct spi_master > *spi_busnum_to_master(u16 busnum); > * by the results of previous messages and where the whole transaction > * ends when the chipselect goes intactive. > * > + * When SPI can transfer in 1x,2x or 4x. It can get this tranfer information > + * from device through @tx_nbits and @rx_nbits. In Bi-direction, these > + * two should both be set. User can set transfer mode with > SPI_NBITS_SINGLE(1x) > + * SPI_NBITS_DUAL(2x) and SPI_NBITS_QUAD(4x) to support these three > transfer. > + * > * The code that submits an spi_message (and its spi_transfers) > * to the lower layers is responsible for managing its memory. > * Zero-initialize every field you don't set up explicitly, to > @@ -510,6 +525,12 @@ struct spi_transfer { > dma_addr_t tx_dma; > dma_addr_t rx_dma; > > + u8 tx_nbits; > + u8 rx_nbits; > +#define SPI_NBITS_SINGLE 0x01; /* 1bit transfer */ > +#define SPI_NBITS_DUAL 0x02; /* 2bits transfer */ > +#define SPI_NBITS_QUAD 0x03; /* 4bits transfer */ > + > unsigned cs_change:1; > u8 bits_per_word; > u16 delay_usecs; > @@ -815,6 +836,10 @@ static inline ssize_t spi_w8r16(struct spi_device > *spi, u8 cmd) > * @mode: Initializes spi_device.mode; based on the chip datasheet, board > * wiring (some devices support both 3WIRE and standard modes), and > * possibly presence of an inverter in the chipselect path. > + * @tx_nbits: Initializes spi_device.tx_nbits; depends on the number of bits > + * the board used in tx. > + * @rx_nbits: Initializes spi_device.rx_nbits; depends on the number of bits > + * the board used in rx. > * > * When adding new SPI devices to the device tree, these structures serve > * as a partial device template. They hold information which can't always > @@ -859,6 +884,12 @@ struct spi_board_info { > * where the default of SPI_CS_HIGH = 0 is wrong. > */ > u8 mode; > + /* tx_nbits initialized for spi_device.tx_nbits and > + * rx_nbits initialized for spi_device.rx_nbits. These members > + * used to describe the how many lines used in tx and rx. > + */ > + u8 tx_nbits; > + u8 rx_nbits; > [Pekon]: Instead of adding new fields you can use existing 'mode' field to pass on the platform specific configurations. And if 'u8 mode' does not suffice you can increase it to 'u32'. #define QSPI_MODE 1 << 5; // just check which bit-fields are un-used spi_board_info->mode |= QSPI_MODE; Same way you can pass the configuration to spi_master and spi_device. > /* ... may need additional spi_device chip config data here. > * avoid stuff protocol drivers can set; but include stuff > -- > 1.7.9.5 > with regards, pekon ^ permalink raw reply [flat|nested] 40+ messages in thread
[parent not found: <20980858CB6D3A4BAE95CA194937D5E73E9E84D2-yXqyApvAXouIQmiDNMet8wC/G2K4zDHf@public.gmane.org>]
* Re: support DUAL and QUAD[patch v1] 2013-07-16 8:08 ` Gupta, Pekon @ 2013-07-16 8:59 ` yuhang wang -1 siblings, 0 replies; 40+ messages in thread From: yuhang wang @ 2013-07-16 8:59 UTC (permalink / raw) To: Gupta, Pekon Cc: linux-mtd, Mark Brown, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Poddar, Sourav Hi, Gupta > [Pekon]: Instead of adding new fields you can use existing 'mode' field to > pass on the platform specific configurations. And if 'u8 mode' does not > suffice you can increase it to 'u32'. > #define QSPI_MODE 1 << 5; // just check which bit-fields are un-used > spi_board_info->mode |= QSPI_MODE; > well, can dual and quad be regarded as a spi mode? if so, your comment seems to be right. ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: support DUAL and QUAD[patch v1] @ 2013-07-16 8:59 ` yuhang wang 0 siblings, 0 replies; 40+ messages in thread From: yuhang wang @ 2013-07-16 8:59 UTC (permalink / raw) To: Gupta, Pekon Cc: linux-mtd, Grant Likely, Mark Brown, linux-mtd, spi-devel-general, Poddar, Sourav Hi, Gupta > [Pekon]: Instead of adding new fields you can use existing 'mode' field to > pass on the platform specific configurations. And if 'u8 mode' does not > suffice you can increase it to 'u32'. > #define QSPI_MODE 1 << 5; // just check which bit-fields are un-used > spi_board_info->mode |= QSPI_MODE; > well, can dual and quad be regarded as a spi mode? if so, your comment seems to be right. ^ permalink raw reply [flat|nested] 40+ messages in thread
[parent not found: <CAHSAbzMDOWo5oBqUHqMxybuDwUjFbczYwN+85gH+rxZ9z8u2Pw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* RE: support DUAL and QUAD[patch v1] 2013-07-16 8:59 ` yuhang wang @ 2013-07-16 9:11 ` Gupta, Pekon -1 siblings, 0 replies; 40+ messages in thread From: Gupta, Pekon @ 2013-07-16 9:11 UTC (permalink / raw) To: yuhang wang Cc: linux-mtd, Mark Brown, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Poddar, Sourav > > Hi, Gupta > > > > [Pekon]: Instead of adding new fields you can use existing 'mode' field to > > pass on the platform specific configurations. And if 'u8 mode' does not > > suffice you can increase it to 'u32'. > > #define QSPI_MODE 1 << 5; // just check which bit-fields are un-used > > spi_board_info->mode |= QSPI_MODE; > > > > well, can dual and quad be regarded as a spi mode? if so, your comment > seems > to be right. Yes, Quad and Dual modes should be regarded as extension of SPI protocol. - They follow the same basic principle of synchronous data transfer. Right ? - These modes are not adding any extra side-band | In-band signaling or controls to modify the protocol. They are just increasing the width of data-channel for better throughput. with regards, pekon ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk ^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: support DUAL and QUAD[patch v1] @ 2013-07-16 9:11 ` Gupta, Pekon 0 siblings, 0 replies; 40+ messages in thread From: Gupta, Pekon @ 2013-07-16 9:11 UTC (permalink / raw) To: yuhang wang Cc: linux-mtd, Grant Likely, Mark Brown, linux-mtd, spi-devel-general, Poddar, Sourav > > Hi, Gupta > > > > [Pekon]: Instead of adding new fields you can use existing 'mode' field to > > pass on the platform specific configurations. And if 'u8 mode' does not > > suffice you can increase it to 'u32'. > > #define QSPI_MODE 1 << 5; // just check which bit-fields are un-used > > spi_board_info->mode |= QSPI_MODE; > > > > well, can dual and quad be regarded as a spi mode? if so, your comment > seems > to be right. Yes, Quad and Dual modes should be regarded as extension of SPI protocol. - They follow the same basic principle of synchronous data transfer. Right ? - These modes are not adding any extra side-band | In-band signaling or controls to modify the protocol. They are just increasing the width of data-channel for better throughput. with regards, pekon ^ permalink raw reply [flat|nested] 40+ messages in thread
[parent not found: <20980858CB6D3A4BAE95CA194937D5E73E9E851E-yXqyApvAXouIQmiDNMet8wC/G2K4zDHf@public.gmane.org>]
* RE: support DUAL and QUAD[patch v1] 2013-07-16 9:11 ` Gupta, Pekon @ 2013-07-16 10:12 ` thomas.langer -1 siblings, 0 replies; 40+ messages in thread From: thomas.langer-th3ZBGNqt+7QT0dZR+AlfA @ 2013-07-16 10:12 UTC (permalink / raw) To: pekon-l0cyMroinI0, wangyuhang2014-Re5JQEeQqe8AvxtiuMwx3w Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, sourav.poddar-l0cyMroinI0, broonie-DgEjT+Ai2ygdnm+yROfE0A, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Hello Pekon, Gupta, Pekon wrote on 2013-07-16: > > > > Hi, Gupta > > > > > > > [Pekon]: Instead of adding new fields you can use existing 'mode' field to > > > pass on the platform specific configurations. And if 'u8 mode' does not > > > suffice you can increase it to 'u32'. > > > #define QSPI_MODE 1 << 5; // just check which bit-fields are un-used > > > spi_board_info->mode |= QSPI_MODE; > > > > > > > well, can dual and quad be regarded as a spi mode? if so, your comment > > seems > > to be right. > > Yes, Quad and Dual modes should be regarded as extension of SPI protocol. > - They follow the same basic principle of synchronous data transfer. Right ? > - These modes are not adding any extra side-band | In-band signaling or > controls to modify the protocol. They are just increasing the width of > data-channel for better throughput. > > with regards, pekon > In general, yes. But I think, for the interface we have to take care of more details. For example, what happens in the following situation: We have a spi-controller, which supports the QSPI mode, and a spi-flash, which fulfils the requirements, but the board has not connected all signals? And the interface for the slave-driver (like m25p80) should allow to specify the transfer mode for each spi_message. This will be necessary, because it depends on the flash and mode, how each phase of "cmd", "address", and "data" will be transferred. I don't know, if it is only me, but I would like to see some more abstract description for all these scenarios, before the details of an interface really could be discussed and decided. Best Regards, Thomas ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk ^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: support DUAL and QUAD[patch v1] @ 2013-07-16 10:12 ` thomas.langer 0 siblings, 0 replies; 40+ messages in thread From: thomas.langer @ 2013-07-16 10:12 UTC (permalink / raw) To: pekon, wangyuhang2014 Cc: spi-devel-general, sourav.poddar, broonie, linux-mtd Hello Pekon, Gupta, Pekon wrote on 2013-07-16: > > > > Hi, Gupta > > > > > > > [Pekon]: Instead of adding new fields you can use existing 'mode' field to > > > pass on the platform specific configurations. And if 'u8 mode' does not > > > suffice you can increase it to 'u32'. > > > #define QSPI_MODE 1 << 5; // just check which bit-fields are un-used > > > spi_board_info->mode |= QSPI_MODE; > > > > > > > well, can dual and quad be regarded as a spi mode? if so, your comment > > seems > > to be right. > > Yes, Quad and Dual modes should be regarded as extension of SPI protocol. > - They follow the same basic principle of synchronous data transfer. Right ? > - These modes are not adding any extra side-band | In-band signaling or > controls to modify the protocol. They are just increasing the width of > data-channel for better throughput. > > with regards, pekon > In general, yes. But I think, for the interface we have to take care of more details. For example, what happens in the following situation: We have a spi-controller, which supports the QSPI mode, and a spi-flash, which fulfils the requirements, but the board has not connected all signals? And the interface for the slave-driver (like m25p80) should allow to specify the transfer mode for each spi_message. This will be necessary, because it depends on the flash and mode, how each phase of "cmd", "address", and "data" will be transferred. I don't know, if it is only me, but I would like to see some more abstract description for all these scenarios, before the details of an interface really could be discussed and decided. Best Regards, Thomas ^ permalink raw reply [flat|nested] 40+ messages in thread
[parent not found: <593AEF6C47F46446852B067021A273D6D93B49E1-6yu8tajIPx7U45ihrnOXy0EOCMrvLtNR@public.gmane.org>]
* RE: support DUAL and QUAD[patch v1] 2013-07-16 10:12 ` thomas.langer @ 2013-07-16 11:20 ` Gupta, Pekon -1 siblings, 0 replies; 40+ messages in thread From: Gupta, Pekon @ 2013-07-16 11:20 UTC (permalink / raw) To: thomas.langer-th3ZBGNqt+7QT0dZR+AlfA, wangyuhang2014-Re5JQEeQqe8AvxtiuMwx3w Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Poddar, Sourav, broonie-DgEjT+Ai2ygdnm+yROfE0A, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r > > Hello Pekon, > > Gupta, Pekon wrote on 2013-07-16: > > > > > > Hi, Gupta > > > > > > > > > > [Pekon]: Instead of adding new fields you can use existing 'mode' field > to > > > > pass on the platform specific configurations. And if 'u8 mode' does not > > > > suffice you can increase it to 'u32'. > > > > #define QSPI_MODE 1 << 5; // just check which bit-fields are un- > used > > > > spi_board_info->mode |= QSPI_MODE; > > > > > > > > > > well, can dual and quad be regarded as a spi mode? if so, your comment > > > seems > > > to be right. > > > > Yes, Quad and Dual modes should be regarded as extension of SPI > protocol. > > - They follow the same basic principle of synchronous data transfer. Right ? > > - These modes are not adding any extra side-band | In-band signaling or > > controls to modify the protocol. They are just increasing the width of > > data-channel for better throughput. > > > > with regards, pekon > > > > In general, yes. But I think, for the interface we have to take care of more > details. > > For example, what happens in the following situation: > We have a spi-controller, which supports the QSPI mode, and a spi-flash, > which fulfils > the requirements, but the board has not connected all signals? > [Pekon]: So, actual implementation should set QSPI_MODE based on both (1) device capabilities (detected in probe) (2) board-DT properties (this would take care of your other constrains) > And the interface for the slave-driver (like m25p80) should allow to specify > the transfer mode > for each spi_message. > This will be necessary, because it depends on the flash and mode, how each > phase of "cmd", > "address", and "data" will be transferred. > [Pekon]: Are you saying that same controller can send interleaved Quad-SPI and Single-SPI spi_messages to same device (here flash device)? Is it due to flash-device constrain that it accepts some flash-commands (like erase, etc) only on Single-wire-SPI, whereas data transfers can be Performed on Quad-SPI mode ? In such case you need to add 'mode' field in struct spi_message, so that this information moves along with the spi_message in queue. And then when that message reaches spi_pump_message() and transfer_one_message() is called, then you can do needful configs. However, I still don't think you need to control 'QSPI_MODE' at granularity of spi_transfer level, because m25p80 flash-driver use spi_write() API to pass data to SPI driver, which internally creates a spi_message in each call. Following best explains use of spi_message in /include/linux/spi.h --------------------------------------------------- * A @spi_message is used to execute an atomic sequence of data transfers, * each represented by a struct spi_transfer. The sequence is "atomic" * in the sense that no other spi_message may use that SPI bus until that * sequence completes. On some systems, many such sequences can execute as * as single programmed DMA transfer. On all systems, these messages are * queued, and might complete after transactions to other devices. Messages * sent to a given spi_device are alway executed in FIFO order. --------------------------------------------------- > I don't know, if it is only me, but I would like to see some more abstract > description for all > these scenarios, before the details of an interface really could be discussed > and decided. > Agree.. Does above matches your requirement ? with regards, pekon > Best Regards, > Thomas ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk ^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: support DUAL and QUAD[patch v1] @ 2013-07-16 11:20 ` Gupta, Pekon 0 siblings, 0 replies; 40+ messages in thread From: Gupta, Pekon @ 2013-07-16 11:20 UTC (permalink / raw) To: thomas.langer, wangyuhang2014 Cc: spi-devel-general, Poddar, Sourav, broonie, linux-mtd > > Hello Pekon, > > Gupta, Pekon wrote on 2013-07-16: > > > > > > Hi, Gupta > > > > > > > > > > [Pekon]: Instead of adding new fields you can use existing 'mode' field > to > > > > pass on the platform specific configurations. And if 'u8 mode' does not > > > > suffice you can increase it to 'u32'. > > > > #define QSPI_MODE 1 << 5; // just check which bit-fields are un- > used > > > > spi_board_info->mode |= QSPI_MODE; > > > > > > > > > > well, can dual and quad be regarded as a spi mode? if so, your comment > > > seems > > > to be right. > > > > Yes, Quad and Dual modes should be regarded as extension of SPI > protocol. > > - They follow the same basic principle of synchronous data transfer. Right ? > > - These modes are not adding any extra side-band | In-band signaling or > > controls to modify the protocol. They are just increasing the width of > > data-channel for better throughput. > > > > with regards, pekon > > > > In general, yes. But I think, for the interface we have to take care of more > details. > > For example, what happens in the following situation: > We have a spi-controller, which supports the QSPI mode, and a spi-flash, > which fulfils > the requirements, but the board has not connected all signals? > [Pekon]: So, actual implementation should set QSPI_MODE based on both (1) device capabilities (detected in probe) (2) board-DT properties (this would take care of your other constrains) > And the interface for the slave-driver (like m25p80) should allow to specify > the transfer mode > for each spi_message. > This will be necessary, because it depends on the flash and mode, how each > phase of "cmd", > "address", and "data" will be transferred. > [Pekon]: Are you saying that same controller can send interleaved Quad-SPI and Single-SPI spi_messages to same device (here flash device)? Is it due to flash-device constrain that it accepts some flash-commands (like erase, etc) only on Single-wire-SPI, whereas data transfers can be Performed on Quad-SPI mode ? In such case you need to add 'mode' field in struct spi_message, so that this information moves along with the spi_message in queue. And then when that message reaches spi_pump_message() and transfer_one_message() is called, then you can do needful configs. However, I still don't think you need to control 'QSPI_MODE' at granularity of spi_transfer level, because m25p80 flash-driver use spi_write() API to pass data to SPI driver, which internally creates a spi_message in each call. Following best explains use of spi_message in /include/linux/spi.h --------------------------------------------------- * A @spi_message is used to execute an atomic sequence of data transfers, * each represented by a struct spi_transfer. The sequence is "atomic" * in the sense that no other spi_message may use that SPI bus until that * sequence completes. On some systems, many such sequences can execute as * as single programmed DMA transfer. On all systems, these messages are * queued, and might complete after transactions to other devices. Messages * sent to a given spi_device are alway executed in FIFO order. --------------------------------------------------- > I don't know, if it is only me, but I would like to see some more abstract > description for all > these scenarios, before the details of an interface really could be discussed > and decided. > Agree.. Does above matches your requirement ? with regards, pekon > Best Regards, > Thomas ^ permalink raw reply [flat|nested] 40+ messages in thread
[parent not found: <20980858CB6D3A4BAE95CA194937D5E73E9E85D0-yXqyApvAXouIQmiDNMet8wC/G2K4zDHf@public.gmane.org>]
* Re: support DUAL and QUAD[patch v1] 2013-07-16 11:20 ` Gupta, Pekon @ 2013-07-16 11:59 ` yuhang wang -1 siblings, 0 replies; 40+ messages in thread From: yuhang wang @ 2013-07-16 11:59 UTC (permalink / raw) To: Gupta, Pekon Cc: thomas.langer-th3ZBGNqt+7QT0dZR+AlfA, Poddar, Sourav, broonie-DgEjT+Ai2ygdnm+yROfE0A, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f Hello Pekon, 2013/7/16 Gupta, Pekon <pekon-l0cyMroinI0@public.gmane.org>: >> >> Hello Pekon, >> >> Gupta, Pekon wrote on 2013-07-16: >> > > >> > > Hi, Gupta >> > > >> > > >> > > > [Pekon]: Instead of adding new fields you can use existing 'mode' field >> to >> > > > pass on the platform specific configurations. And if 'u8 mode' does not >> > > > suffice you can increase it to 'u32'. >> > > > #define QSPI_MODE 1 << 5; // just check which bit-fields are un- >> used >> > > > spi_board_info->mode |= QSPI_MODE; >> > > > >> > > >> > > well, can dual and quad be regarded as a spi mode? if so, your comment >> > > seems >> > > to be right. >> > >> > Yes, Quad and Dual modes should be regarded as extension of SPI >> protocol. >> > - They follow the same basic principle of synchronous data transfer. Right ? >> > - These modes are not adding any extra side-band | In-band signaling or >> > controls to modify the protocol. They are just increasing the width of >> > data-channel for better throughput. >> > >> > with regards, pekon >> > >> >> In general, yes. But I think, for the interface we have to take care of more >> details. >> >> For example, what happens in the following situation: >> We have a spi-controller, which supports the QSPI mode, and a spi-flash, >> which fulfils >> the requirements, but the board has not connected all signals? >> > [Pekon]: So, actual implementation should set QSPI_MODE based on both > (1) device capabilities (detected in probe) > (2) board-DT properties (this would take care of your other constrains) > I think the name QSPI_MODE is not siutable. Not only QSPI support dual and quad. >> And the interface for the slave-driver (like m25p80) should allow to specify >> the transfer mode >> for each spi_message. >> This will be necessary, because it depends on the flash and mode, how each >> phase of "cmd", >> "address", and "data" will be transferred. >> > [Pekon]: Are you saying that same controller can send interleaved > Quad-SPI and Single-SPI spi_messages to same device (here flash device)? > > Is it due to flash-device constrain that it accepts some flash-commands > (like erase, etc) only on Single-wire-SPI, whereas data transfers can be > Performed on Quad-SPI mode ? > In such case you need to add 'mode' field in struct spi_message, so that > this information moves along with the spi_message in queue. And then > when that message reaches spi_pump_message() and > transfer_one_message() is called, then you can do needful configs. > > However, I still don't think you need to control 'QSPI_MODE' at granularity > of spi_transfer level, because m25p80 flash-driver use spi_write() API to > pass data to SPI driver, which internally creates a spi_message in each call. > > Following best explains use of spi_message in /include/linux/spi.h > --------------------------------------------------- > * A @spi_message is used to execute an atomic sequence of data transfers, > * each represented by a struct spi_transfer. The sequence is "atomic" > * in the sense that no other spi_message may use that SPI bus until that > * sequence completes. On some systems, many such sequences can execute as > * as single programmed DMA transfer. On all systems, these messages are > * queued, and might complete after transactions to other devices. Messages > * sent to a given spi_device are alway executed in FIFO order. > --------------------------------------------------- > > what thomas means that some flash device will do read or write in different mode. example: cmd with single mode; addr with quad mode; data with quad mode; So we really can not decide how to organise the spi_message just based on QSPI_MODE. But to this question, because there is no standard for all the serial-flash and different serial-flash may have different sequences. Thus what we can do is to provide the interface in spi framework. To different slave, we should adjust the driver by ourselves. >> I don't know, if it is only me, but I would like to see some more abstract >> description for all >> these scenarios, before the details of an interface really could be discussed >> and decided. >> > Agree.. Does above matches your requirement ? > Thanks for your advice and I will provide a patch later. please help me review. > > with regards, pekon > >> Best Regards, >> Thomas > ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: support DUAL and QUAD[patch v1] @ 2013-07-16 11:59 ` yuhang wang 0 siblings, 0 replies; 40+ messages in thread From: yuhang wang @ 2013-07-16 11:59 UTC (permalink / raw) To: Gupta, Pekon Cc: thomas.langer, Poddar, Sourav, broonie, linux-mtd, spi-devel-general Hello Pekon, 2013/7/16 Gupta, Pekon <pekon@ti.com>: >> >> Hello Pekon, >> >> Gupta, Pekon wrote on 2013-07-16: >> > > >> > > Hi, Gupta >> > > >> > > >> > > > [Pekon]: Instead of adding new fields you can use existing 'mode' field >> to >> > > > pass on the platform specific configurations. And if 'u8 mode' does not >> > > > suffice you can increase it to 'u32'. >> > > > #define QSPI_MODE 1 << 5; // just check which bit-fields are un- >> used >> > > > spi_board_info->mode |= QSPI_MODE; >> > > > >> > > >> > > well, can dual and quad be regarded as a spi mode? if so, your comment >> > > seems >> > > to be right. >> > >> > Yes, Quad and Dual modes should be regarded as extension of SPI >> protocol. >> > - They follow the same basic principle of synchronous data transfer. Right ? >> > - These modes are not adding any extra side-band | In-band signaling or >> > controls to modify the protocol. They are just increasing the width of >> > data-channel for better throughput. >> > >> > with regards, pekon >> > >> >> In general, yes. But I think, for the interface we have to take care of more >> details. >> >> For example, what happens in the following situation: >> We have a spi-controller, which supports the QSPI mode, and a spi-flash, >> which fulfils >> the requirements, but the board has not connected all signals? >> > [Pekon]: So, actual implementation should set QSPI_MODE based on both > (1) device capabilities (detected in probe) > (2) board-DT properties (this would take care of your other constrains) > I think the name QSPI_MODE is not siutable. Not only QSPI support dual and quad. >> And the interface for the slave-driver (like m25p80) should allow to specify >> the transfer mode >> for each spi_message. >> This will be necessary, because it depends on the flash and mode, how each >> phase of "cmd", >> "address", and "data" will be transferred. >> > [Pekon]: Are you saying that same controller can send interleaved > Quad-SPI and Single-SPI spi_messages to same device (here flash device)? > > Is it due to flash-device constrain that it accepts some flash-commands > (like erase, etc) only on Single-wire-SPI, whereas data transfers can be > Performed on Quad-SPI mode ? > In such case you need to add 'mode' field in struct spi_message, so that > this information moves along with the spi_message in queue. And then > when that message reaches spi_pump_message() and > transfer_one_message() is called, then you can do needful configs. > > However, I still don't think you need to control 'QSPI_MODE' at granularity > of spi_transfer level, because m25p80 flash-driver use spi_write() API to > pass data to SPI driver, which internally creates a spi_message in each call. > > Following best explains use of spi_message in /include/linux/spi.h > --------------------------------------------------- > * A @spi_message is used to execute an atomic sequence of data transfers, > * each represented by a struct spi_transfer. The sequence is "atomic" > * in the sense that no other spi_message may use that SPI bus until that > * sequence completes. On some systems, many such sequences can execute as > * as single programmed DMA transfer. On all systems, these messages are > * queued, and might complete after transactions to other devices. Messages > * sent to a given spi_device are alway executed in FIFO order. > --------------------------------------------------- > > what thomas means that some flash device will do read or write in different mode. example: cmd with single mode; addr with quad mode; data with quad mode; So we really can not decide how to organise the spi_message just based on QSPI_MODE. But to this question, because there is no standard for all the serial-flash and different serial-flash may have different sequences. Thus what we can do is to provide the interface in spi framework. To different slave, we should adjust the driver by ourselves. >> I don't know, if it is only me, but I would like to see some more abstract >> description for all >> these scenarios, before the details of an interface really could be discussed >> and decided. >> > Agree.. Does above matches your requirement ? > Thanks for your advice and I will provide a patch later. please help me review. > > with regards, pekon > >> Best Regards, >> Thomas > ^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: support DUAL and QUAD[patch v1] 2013-07-16 11:20 ` Gupta, Pekon @ 2013-07-16 12:18 ` thomas.langer -1 siblings, 0 replies; 40+ messages in thread From: thomas.langer-th3ZBGNqt+7QT0dZR+AlfA @ 2013-07-16 12:18 UTC (permalink / raw) To: pekon-l0cyMroinI0, wangyuhang2014-Re5JQEeQqe8AvxtiuMwx3w Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, sourav.poddar-l0cyMroinI0, broonie-DgEjT+Ai2ygdnm+yROfE0A, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Hello Pekon, Gupta, Pekon wrote on 2013-07-16: > > > > Hello Pekon, > > > > Gupta, Pekon wrote on 2013-07-16: > > > > > > > > Hi, Gupta > > > > > > > > > > > > > [Pekon]: Instead of adding new fields you can use existing 'mode' > field > > to > > > > > pass on the platform specific configurations. And if 'u8 mode' does > not > > > > > suffice you can increase it to 'u32'. > > > > > #define QSPI_MODE 1 << 5; // just check which bit-fields are un- > > used > > > > > spi_board_info->mode |= QSPI_MODE; > > > > > > > > > > > > > well, can dual and quad be regarded as a spi mode? if so, your comment > > > > seems > > > > to be right. > > > > > > Yes, Quad and Dual modes should be regarded as extension of SPI > > protocol. > > > - They follow the same basic principle of synchronous data transfer. Right > ? > > > - These modes are not adding any extra side-band | In-band signaling or > > > controls to modify the protocol. They are just increasing the width of > > > data-channel for better throughput. > > > > > > with regards, pekon > > > > > > > In general, yes. But I think, for the interface we have to take care of more > > details. > > > > For example, what happens in the following situation: > > We have a spi-controller, which supports the QSPI mode, and a spi-flash, > > which fulfils > > the requirements, but the board has not connected all signals? > > > [Pekon]: So, actual implementation should set QSPI_MODE based on both > (1) device capabilities (detected in probe) > (2) board-DT properties (this would take care of your other constrains) > yes, and in the end the slave (as of m25p80) can decide, which actions/modes are usable and should only use them. > > And the interface for the slave-driver (like m25p80) should allow to specify > > the transfer mode > > for each spi_message. > > This will be necessary, because it depends on the flash and mode, how > each > > phase of "cmd", > > "address", and "data" will be transferred. > > > [Pekon]: Are you saying that same controller can send interleaved > Quad-SPI and Single-SPI spi_messages to same device (here flash device)? Yes, for example the S25FL129P from Spansion always expects the command (first 8 bit) on a single wire and then, depending on the command, switching to dual or quad-mode for the address and/or data phase. And other flashes I have seen (don't remember name or manufacturer) support to be switched into a quad-mode, in which they expect all data (including the command) as quad transfer. > > Is it due to flash-device constrain that it accepts some flash-commands > (like erase, etc) only on Single-wire-SPI, whereas data transfers can be > Performed on Quad-SPI mode ? > In such case you need to add 'mode' field in struct spi_message, so that > this information moves along with the spi_message in queue. And then > when that message reaches spi_pump_message() and > transfer_one_message() is called, then you can do needful configs. Yes, I think this would be a good way to allow the flash driver to control these details. > > However, I still don't think you need to control 'QSPI_MODE' at granularity > of spi_transfer level, because m25p80 flash-driver use spi_write() API to > pass data to SPI driver, which internally creates a spi_message in each call. I expect some changes to this driver for adding this modes, but as discussed above, it should be enough to have the mode per message. > > Following best explains use of spi_message in /include/linux/spi.h > --------------------------------------------------- > * A @spi_message is used to execute an atomic sequence of data transfers, > * each represented by a struct spi_transfer. The sequence is "atomic" > * in the sense that no other spi_message may use that SPI bus until that > * sequence completes. On some systems, many such sequences can > execute as > * as single programmed DMA transfer. On all systems, these messages are > * queued, and might complete after transactions to other devices. > Messages > * sent to a given spi_device are alway executed in FIFO order. > --------------------------------------------------- > > > > I don't know, if it is only me, but I would like to see some more abstract > > description for all > > these scenarios, before the details of an interface really could be discussed > > and decided. > > > Agree.. Does above matches your requirement ? > Yes, looks better. Thanks! > > with regards, pekon Best Regards, Thomas ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk ^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: support DUAL and QUAD[patch v1] @ 2013-07-16 12:18 ` thomas.langer 0 siblings, 0 replies; 40+ messages in thread From: thomas.langer @ 2013-07-16 12:18 UTC (permalink / raw) To: pekon, wangyuhang2014 Cc: spi-devel-general, sourav.poddar, broonie, linux-mtd Hello Pekon, Gupta, Pekon wrote on 2013-07-16: > > > > Hello Pekon, > > > > Gupta, Pekon wrote on 2013-07-16: > > > > > > > > Hi, Gupta > > > > > > > > > > > > > [Pekon]: Instead of adding new fields you can use existing 'mode' > field > > to > > > > > pass on the platform specific configurations. And if 'u8 mode' does > not > > > > > suffice you can increase it to 'u32'. > > > > > #define QSPI_MODE 1 << 5; // just check which bit-fields are un- > > used > > > > > spi_board_info->mode |= QSPI_MODE; > > > > > > > > > > > > > well, can dual and quad be regarded as a spi mode? if so, your comment > > > > seems > > > > to be right. > > > > > > Yes, Quad and Dual modes should be regarded as extension of SPI > > protocol. > > > - They follow the same basic principle of synchronous data transfer. Right > ? > > > - These modes are not adding any extra side-band | In-band signaling or > > > controls to modify the protocol. They are just increasing the width of > > > data-channel for better throughput. > > > > > > with regards, pekon > > > > > > > In general, yes. But I think, for the interface we have to take care of more > > details. > > > > For example, what happens in the following situation: > > We have a spi-controller, which supports the QSPI mode, and a spi-flash, > > which fulfils > > the requirements, but the board has not connected all signals? > > > [Pekon]: So, actual implementation should set QSPI_MODE based on both > (1) device capabilities (detected in probe) > (2) board-DT properties (this would take care of your other constrains) > yes, and in the end the slave (as of m25p80) can decide, which actions/modes are usable and should only use them. > > And the interface for the slave-driver (like m25p80) should allow to specify > > the transfer mode > > for each spi_message. > > This will be necessary, because it depends on the flash and mode, how > each > > phase of "cmd", > > "address", and "data" will be transferred. > > > [Pekon]: Are you saying that same controller can send interleaved > Quad-SPI and Single-SPI spi_messages to same device (here flash device)? Yes, for example the S25FL129P from Spansion always expects the command (first 8 bit) on a single wire and then, depending on the command, switching to dual or quad-mode for the address and/or data phase. And other flashes I have seen (don't remember name or manufacturer) support to be switched into a quad-mode, in which they expect all data (including the command) as quad transfer. > > Is it due to flash-device constrain that it accepts some flash-commands > (like erase, etc) only on Single-wire-SPI, whereas data transfers can be > Performed on Quad-SPI mode ? > In such case you need to add 'mode' field in struct spi_message, so that > this information moves along with the spi_message in queue. And then > when that message reaches spi_pump_message() and > transfer_one_message() is called, then you can do needful configs. Yes, I think this would be a good way to allow the flash driver to control these details. > > However, I still don't think you need to control 'QSPI_MODE' at granularity > of spi_transfer level, because m25p80 flash-driver use spi_write() API to > pass data to SPI driver, which internally creates a spi_message in each call. I expect some changes to this driver for adding this modes, but as discussed above, it should be enough to have the mode per message. > > Following best explains use of spi_message in /include/linux/spi.h > --------------------------------------------------- > * A @spi_message is used to execute an atomic sequence of data transfers, > * each represented by a struct spi_transfer. The sequence is "atomic" > * in the sense that no other spi_message may use that SPI bus until that > * sequence completes. On some systems, many such sequences can > execute as > * as single programmed DMA transfer. On all systems, these messages are > * queued, and might complete after transactions to other devices. > Messages > * sent to a given spi_device are alway executed in FIFO order. > --------------------------------------------------- > > > > I don't know, if it is only me, but I would like to see some more abstract > > description for all > > these scenarios, before the details of an interface really could be discussed > > and decided. > > > Agree.. Does above matches your requirement ? > Yes, looks better. Thanks! > > with regards, pekon Best Regards, Thomas ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: support DUAL and QUAD[patch v1] 2013-07-16 11:20 ` Gupta, Pekon @ 2013-07-18 1:58 ` yuhang wang -1 siblings, 0 replies; 40+ messages in thread From: yuhang wang @ 2013-07-18 1:58 UTC (permalink / raw) To: Gupta, Pekon Cc: thomas.langer-th3ZBGNqt+7QT0dZR+AlfA, Poddar, Sourav, broonie-DgEjT+Ai2ygdnm+yROfE0A, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f Hi, 2013/7/16 Gupta, Pekon <pekon-l0cyMroinI0@public.gmane.org>: >> >> Hello Pekon, >> >> Gupta, Pekon wrote on 2013-07-16: >> > > >> > > Hi, Gupta >> > > >> > > >> > > > [Pekon]: Instead of adding new fields you can use existing 'mode' field >> to >> > > > pass on the platform specific configurations. And if 'u8 mode' does not >> > > > suffice you can increase it to 'u32'. >> > > > #define QSPI_MODE 1 << 5; // just check which bit-fields are un- >> used >> > > > spi_board_info->mode |= QSPI_MODE; >> > > > >> > > >> > > well, can dual and quad be regarded as a spi mode? if so, your comment >> > > seems >> > > to be right. >> > >> > Yes, Quad and Dual modes should be regarded as extension of SPI >> protocol. >> > - They follow the same basic principle of synchronous data transfer. Right ? >> > - These modes are not adding any extra side-band | In-band signaling or >> > controls to modify the protocol. They are just increasing the width of >> > data-channel for better throughput. >> > >> > with regards, pekon >> > >> >> In general, yes. But I think, for the interface we have to take care of more >> details. >> >> For example, what happens in the following situation: >> We have a spi-controller, which supports the QSPI mode, and a spi-flash, >> which fulfils >> the requirements, but the board has not connected all signals? >> > [Pekon]: So, actual implementation should set QSPI_MODE based on both > (1) device capabilities (detected in probe) > (2) board-DT properties (this would take care of your other constrains) > >> And the interface for the slave-driver (like m25p80) should allow to specify >> the transfer mode >> for each spi_message. >> This will be necessary, because it depends on the flash and mode, how each >> phase of "cmd", >> "address", and "data" will be transferred. >> > [Pekon]: Are you saying that same controller can send interleaved > Quad-SPI and Single-SPI spi_messages to same device (here flash device)? > > Is it due to flash-device constrain that it accepts some flash-commands > (like erase, etc) only on Single-wire-SPI, whereas data transfers can be > Performed on Quad-SPI mode ? > In such case you need to add 'mode' field in struct spi_message, so that > this information moves along with the spi_message in queue. And then > when that message reaches spi_pump_message() and > transfer_one_message() is called, then you can do needful configs. > > However, I still don't think you need to control 'QSPI_MODE' at granularity > of spi_transfer level, because m25p80 flash-driver use spi_write() API to > pass data to SPI driver, which internally creates a spi_message in each call. > > Following best explains use of spi_message in /include/linux/spi.h > --------------------------------------------------- > * A @spi_message is used to execute an atomic sequence of data transfers, > * each represented by a struct spi_transfer. The sequence is "atomic" > * in the sense that no other spi_message may use that SPI bus until that > * sequence completes. On some systems, many such sequences can execute as > * as single programmed DMA transfer. On all systems, these messages are > * queued, and might complete after transactions to other devices. Messages > * sent to a given spi_device are alway executed in FIFO order. > --------------------------------------------------- > > But in some cases, slave will use different mode(single,dual,quad) for different spi_transfer in one spi_message. So I don't think that a mode in spi_message can describe all the mode in one spi_message.I have corrected the patch as below. @spi_master->mode_bit is the mode master supports, @spi_device->mode is the slave work in, but the spi slave still need to set the mode for the spi_transfer based on the mode in @spi_device. example: if @spi_device->mode = QUAD then cmd@spi_transfer->tx_nbits = SINGLE addr@spi_transfer->tx_nbits = SINGLE data@spi_transfer->tx_nbits = QUAD our spi controller driver will deal with that in transfer_one_message(). >From 9ed7028f278ff744c301f11ea3ad6298d14b91e4 Mon Sep 17 00:00:00 2001 From: wangyuhang <wangyuhang2014-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Date: Thu, 18 Jul 2013 09:23:44 +0800 Subject: [PATCH] spi: DUAL and QUAD support Signed-off-by: wangyuhang <wangyuhang2014-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Signed-off-by: wangyuhang <wangyuhang2014-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> --- drivers/spi/spi.c | 18 ++++++++++++++++++ include/linux/spi/spi.h | 23 +++++++++++++++++++++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 004b10f..36d2451 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -868,6 +868,14 @@ static void of_register_spi_devices(struct spi_master *master) spi->mode |= SPI_CS_HIGH; if (of_find_property(nc, "spi-3wire", NULL)) spi->mode |= SPI_3WIRE; + if (of_find_property(nc, "spi-tx-dual", NULL)) + spi->mode |= SPI_TX_DUAL; + if (of_find_property(nc, "spi-tx-quad", NULL)) + spi->mode |= SPI_TX_QUAD; + if (of_find_property(nc, "spi-rx-dual", NULL)) + spi->mode |= SPI_RX_DUAL; + if (of_find_property(nc, "spi-rx-quad", NULL)) + spi->mode |= SPI_RX_QUAD; /* Device speed */ prop = of_get_property(nc, "spi-max-frequency", &len); @@ -1316,6 +1324,12 @@ int spi_setup(struct spi_device *spi) /* help drivers fail *cleanly* when they need options * that aren't supported with their current master */ + if (((spi->mode >> 8) & 0x03) == 0x03 || + ((spi->mode >> 10) & 0x03) == 0x03) { + dev_err(&spi->dev, + "setup: can not select dual and quad at the same time\n"); + return -EINVAL; + } bad_bits = spi->mode & ~spi->master->mode_bits; if (bad_bits) { dev_err(&spi->dev, "setup: unsupported mode bits %x\n", @@ -1376,6 +1390,10 @@ static int __spi_async(struct spi_device *spi, struct spi_message *message) xfer->bits_per_word = spi->bits_per_word; if (!xfer->speed_hz) xfer->speed_hz = spi->max_speed_hz; + if (!xfer->tx_nbits) + xfer->tx_nbits = SPI_NBITS_SINGLE; + if (!xfer->rx_nbits) + xfer->rx_nbits = SPI_NBITS_SINGLE; } message->spi = spi; diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 38c2b92..e9ba1cf 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -74,7 +74,7 @@ struct spi_device { struct spi_master *master; u32 max_speed_hz; u8 chip_select; - u8 mode; + u16 mode; #define SPI_CPHA 0x01 /* clock phase */ #define SPI_CPOL 0x02 /* clock polarity */ #define SPI_MODE_0 (0|0) /* (original MicroWire) */ @@ -87,6 +87,10 @@ struct spi_device { #define SPI_LOOP 0x20 /* loopback mode */ #define SPI_NO_CS 0x40 /* 1 dev/bus, no chipselect */ #define SPI_READY 0x80 /* slave pulls low to pause */ +#define SPI_TX_DUAL 0x100 /* transmit with 2 wires */ +#define SPI_TX_QUAD 0x200 /* transmit with 4 wires */ +#define SPI_RX_DUAL 0x400 /* receive with 2 wires */ +#define SPI_RX_QUAD 0x800 /* receive with 4 wires */ u8 bits_per_word; int irq; void *controller_state; @@ -437,6 +441,10 @@ extern struct spi_master *spi_busnum_to_master(u16 busnum); * @rx_buf: data to be read (dma-safe memory), or NULL * @tx_dma: DMA address of tx_buf, if @spi_message.is_dma_mapped * @rx_dma: DMA address of rx_buf, if @spi_message.is_dma_mapped + * @tx_nbits: number of bits used for writting, if 0 default + * (SPI_NBITS_SINGLE = 0x01) is used. + * @rx_nbits: number of bits used for reading, if 0 default + * (SPI_NBITS_SINGLE = 0x01) is used. * @len: size of rx and tx buffers (in bytes) * @speed_hz: Select a speed other than the device default for this * transfer. If 0 the default (from @spi_device) is used. @@ -491,6 +499,11 @@ extern struct spi_master *spi_busnum_to_master(u16 busnum); * by the results of previous messages and where the whole transaction * ends when the chipselect goes intactive. * + * When SPI can transfer in 1x,2x or 4x. It can get this tranfer information + * from device through @tx_nbits and @rx_nbits. In Bi-direction, these + * two should both be set. User can set transfer mode with SPI_NBITS_SINGLE(1x) + * SPI_NBITS_DUAL(2x) and SPI_NBITS_QUAD(4x) to support these three transfer. + * * The code that submits an spi_message (and its spi_transfers) * to the lower layers is responsible for managing its memory. * Zero-initialize every field you don't set up explicitly, to @@ -511,6 +524,12 @@ struct spi_transfer { dma_addr_t rx_dma; unsigned cs_change:1; + u8 bitwidth; + u8 tx_nbits; + u8 rx_nbits; +#define SPI_NBITS_SINGLE 0x01; /* 1bit transfer */ +#define SPI_NBITS_DUAL 0x02; /* 2bits transfer */ +#define SPI_NBITS_QUAD 0x03; /* 4bits transfer */ u8 bits_per_word; u16 delay_usecs; u32 speed_hz; @@ -858,7 +877,7 @@ struct spi_board_info { /* mode becomes spi_device.mode, and is essential for chips * where the default of SPI_CS_HIGH = 0 is wrong. */ - u8 mode; + u16 mode; /* ... may need additional spi_device chip config data here. * avoid stuff protocol drivers can set; but include stuff -- 1.7.9.5 ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: support DUAL and QUAD[patch v1] @ 2013-07-18 1:58 ` yuhang wang 0 siblings, 0 replies; 40+ messages in thread From: yuhang wang @ 2013-07-18 1:58 UTC (permalink / raw) To: Gupta, Pekon Cc: thomas.langer, Poddar, Sourav, broonie, linux-mtd, spi-devel-general Hi, 2013/7/16 Gupta, Pekon <pekon@ti.com>: >> >> Hello Pekon, >> >> Gupta, Pekon wrote on 2013-07-16: >> > > >> > > Hi, Gupta >> > > >> > > >> > > > [Pekon]: Instead of adding new fields you can use existing 'mode' field >> to >> > > > pass on the platform specific configurations. And if 'u8 mode' does not >> > > > suffice you can increase it to 'u32'. >> > > > #define QSPI_MODE 1 << 5; // just check which bit-fields are un- >> used >> > > > spi_board_info->mode |= QSPI_MODE; >> > > > >> > > >> > > well, can dual and quad be regarded as a spi mode? if so, your comment >> > > seems >> > > to be right. >> > >> > Yes, Quad and Dual modes should be regarded as extension of SPI >> protocol. >> > - They follow the same basic principle of synchronous data transfer. Right ? >> > - These modes are not adding any extra side-band | In-band signaling or >> > controls to modify the protocol. They are just increasing the width of >> > data-channel for better throughput. >> > >> > with regards, pekon >> > >> >> In general, yes. But I think, for the interface we have to take care of more >> details. >> >> For example, what happens in the following situation: >> We have a spi-controller, which supports the QSPI mode, and a spi-flash, >> which fulfils >> the requirements, but the board has not connected all signals? >> > [Pekon]: So, actual implementation should set QSPI_MODE based on both > (1) device capabilities (detected in probe) > (2) board-DT properties (this would take care of your other constrains) > >> And the interface for the slave-driver (like m25p80) should allow to specify >> the transfer mode >> for each spi_message. >> This will be necessary, because it depends on the flash and mode, how each >> phase of "cmd", >> "address", and "data" will be transferred. >> > [Pekon]: Are you saying that same controller can send interleaved > Quad-SPI and Single-SPI spi_messages to same device (here flash device)? > > Is it due to flash-device constrain that it accepts some flash-commands > (like erase, etc) only on Single-wire-SPI, whereas data transfers can be > Performed on Quad-SPI mode ? > In such case you need to add 'mode' field in struct spi_message, so that > this information moves along with the spi_message in queue. And then > when that message reaches spi_pump_message() and > transfer_one_message() is called, then you can do needful configs. > > However, I still don't think you need to control 'QSPI_MODE' at granularity > of spi_transfer level, because m25p80 flash-driver use spi_write() API to > pass data to SPI driver, which internally creates a spi_message in each call. > > Following best explains use of spi_message in /include/linux/spi.h > --------------------------------------------------- > * A @spi_message is used to execute an atomic sequence of data transfers, > * each represented by a struct spi_transfer. The sequence is "atomic" > * in the sense that no other spi_message may use that SPI bus until that > * sequence completes. On some systems, many such sequences can execute as > * as single programmed DMA transfer. On all systems, these messages are > * queued, and might complete after transactions to other devices. Messages > * sent to a given spi_device are alway executed in FIFO order. > --------------------------------------------------- > > But in some cases, slave will use different mode(single,dual,quad) for different spi_transfer in one spi_message. So I don't think that a mode in spi_message can describe all the mode in one spi_message.I have corrected the patch as below. @spi_master->mode_bit is the mode master supports, @spi_device->mode is the slave work in, but the spi slave still need to set the mode for the spi_transfer based on the mode in @spi_device. example: if @spi_device->mode = QUAD then cmd@spi_transfer->tx_nbits = SINGLE addr@spi_transfer->tx_nbits = SINGLE data@spi_transfer->tx_nbits = QUAD our spi controller driver will deal with that in transfer_one_message(). >From 9ed7028f278ff744c301f11ea3ad6298d14b91e4 Mon Sep 17 00:00:00 2001 From: wangyuhang <wangyuhang2014@gmail.com> Date: Thu, 18 Jul 2013 09:23:44 +0800 Subject: [PATCH] spi: DUAL and QUAD support Signed-off-by: wangyuhang <wangyuhang2014@gmail.com> Signed-off-by: wangyuhang <wangyuhang2014@gmail.com> --- drivers/spi/spi.c | 18 ++++++++++++++++++ include/linux/spi/spi.h | 23 +++++++++++++++++++++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 004b10f..36d2451 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -868,6 +868,14 @@ static void of_register_spi_devices(struct spi_master *master) spi->mode |= SPI_CS_HIGH; if (of_find_property(nc, "spi-3wire", NULL)) spi->mode |= SPI_3WIRE; + if (of_find_property(nc, "spi-tx-dual", NULL)) + spi->mode |= SPI_TX_DUAL; + if (of_find_property(nc, "spi-tx-quad", NULL)) + spi->mode |= SPI_TX_QUAD; + if (of_find_property(nc, "spi-rx-dual", NULL)) + spi->mode |= SPI_RX_DUAL; + if (of_find_property(nc, "spi-rx-quad", NULL)) + spi->mode |= SPI_RX_QUAD; /* Device speed */ prop = of_get_property(nc, "spi-max-frequency", &len); @@ -1316,6 +1324,12 @@ int spi_setup(struct spi_device *spi) /* help drivers fail *cleanly* when they need options * that aren't supported with their current master */ + if (((spi->mode >> 8) & 0x03) == 0x03 || + ((spi->mode >> 10) & 0x03) == 0x03) { + dev_err(&spi->dev, + "setup: can not select dual and quad at the same time\n"); + return -EINVAL; + } bad_bits = spi->mode & ~spi->master->mode_bits; if (bad_bits) { dev_err(&spi->dev, "setup: unsupported mode bits %x\n", @@ -1376,6 +1390,10 @@ static int __spi_async(struct spi_device *spi, struct spi_message *message) xfer->bits_per_word = spi->bits_per_word; if (!xfer->speed_hz) xfer->speed_hz = spi->max_speed_hz; + if (!xfer->tx_nbits) + xfer->tx_nbits = SPI_NBITS_SINGLE; + if (!xfer->rx_nbits) + xfer->rx_nbits = SPI_NBITS_SINGLE; } message->spi = spi; diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 38c2b92..e9ba1cf 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -74,7 +74,7 @@ struct spi_device { struct spi_master *master; u32 max_speed_hz; u8 chip_select; - u8 mode; + u16 mode; #define SPI_CPHA 0x01 /* clock phase */ #define SPI_CPOL 0x02 /* clock polarity */ #define SPI_MODE_0 (0|0) /* (original MicroWire) */ @@ -87,6 +87,10 @@ struct spi_device { #define SPI_LOOP 0x20 /* loopback mode */ #define SPI_NO_CS 0x40 /* 1 dev/bus, no chipselect */ #define SPI_READY 0x80 /* slave pulls low to pause */ +#define SPI_TX_DUAL 0x100 /* transmit with 2 wires */ +#define SPI_TX_QUAD 0x200 /* transmit with 4 wires */ +#define SPI_RX_DUAL 0x400 /* receive with 2 wires */ +#define SPI_RX_QUAD 0x800 /* receive with 4 wires */ u8 bits_per_word; int irq; void *controller_state; @@ -437,6 +441,10 @@ extern struct spi_master *spi_busnum_to_master(u16 busnum); * @rx_buf: data to be read (dma-safe memory), or NULL * @tx_dma: DMA address of tx_buf, if @spi_message.is_dma_mapped * @rx_dma: DMA address of rx_buf, if @spi_message.is_dma_mapped + * @tx_nbits: number of bits used for writting, if 0 default + * (SPI_NBITS_SINGLE = 0x01) is used. + * @rx_nbits: number of bits used for reading, if 0 default + * (SPI_NBITS_SINGLE = 0x01) is used. * @len: size of rx and tx buffers (in bytes) * @speed_hz: Select a speed other than the device default for this * transfer. If 0 the default (from @spi_device) is used. @@ -491,6 +499,11 @@ extern struct spi_master *spi_busnum_to_master(u16 busnum); * by the results of previous messages and where the whole transaction * ends when the chipselect goes intactive. * + * When SPI can transfer in 1x,2x or 4x. It can get this tranfer information + * from device through @tx_nbits and @rx_nbits. In Bi-direction, these + * two should both be set. User can set transfer mode with SPI_NBITS_SINGLE(1x) + * SPI_NBITS_DUAL(2x) and SPI_NBITS_QUAD(4x) to support these three transfer. + * * The code that submits an spi_message (and its spi_transfers) * to the lower layers is responsible for managing its memory. * Zero-initialize every field you don't set up explicitly, to @@ -511,6 +524,12 @@ struct spi_transfer { dma_addr_t rx_dma; unsigned cs_change:1; + u8 bitwidth; + u8 tx_nbits; + u8 rx_nbits; +#define SPI_NBITS_SINGLE 0x01; /* 1bit transfer */ +#define SPI_NBITS_DUAL 0x02; /* 2bits transfer */ +#define SPI_NBITS_QUAD 0x03; /* 4bits transfer */ u8 bits_per_word; u16 delay_usecs; u32 speed_hz; @@ -858,7 +877,7 @@ struct spi_board_info { /* mode becomes spi_device.mode, and is essential for chips * where the default of SPI_CS_HIGH = 0 is wrong. */ - u8 mode; + u16 mode; /* ... may need additional spi_device chip config data here. * avoid stuff protocol drivers can set; but include stuff -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 40+ messages in thread
[parent not found: <CAHSAbzP5jqHwCm0qhu=DiNzpP0swqWHsQ0=MnF+EKaSN22E5hg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: support DUAL and QUAD[patch v1] 2013-07-18 1:58 ` yuhang wang @ 2013-07-18 2:25 ` Trent Piepho -1 siblings, 0 replies; 40+ messages in thread From: Trent Piepho @ 2013-07-18 2:25 UTC (permalink / raw) To: yuhang wang Cc: thomas.langer-th3ZBGNqt+7QT0dZR+AlfA, broonie-DgEjT+Ai2ygdnm+yROfE0A, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Gupta, Pekon, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Poddar, Sourav On Wed, Jul 17, 2013 at 6:58 PM, yuhang wang <wangyuhang2014-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > >> > > > >> > > > [Pekon]: Instead of adding new fields you can use existing 'mode' > >> > > > field > >> to > >> > > > pass on the platform specific configurations. And if 'u8 mode' > >> > > > does not > >> > > > suffice you can increase it to 'u32'. But what about spidev? > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c > index 004b10f..36d2451 100644 > --- a/drivers/spi/spi.c > +++ b/drivers/spi/spi.c > @@ -868,6 +868,14 @@ static void of_register_spi_devices(struct > spi_master *master) > spi->mode |= SPI_CS_HIGH; > if (of_find_property(nc, "spi-3wire", NULL)) > spi->mode |= SPI_3WIRE; > + if (of_find_property(nc, "spi-tx-dual", NULL)) > + spi->mode |= SPI_TX_DUAL; > + if (of_find_property(nc, "spi-tx-quad", NULL)) > + spi->mode |= SPI_TX_QUAD; > + if (of_find_property(nc, "spi-rx-dual", NULL)) > + spi->mode |= SPI_RX_DUAL; > + if (of_find_property(nc, "spi-rx-quad", NULL)) > + spi->mode |= SPI_RX_QUAD; > > /* Device speed */ > prop = of_get_property(nc, "spi-max-frequency", &len); > @@ -1316,6 +1324,12 @@ int spi_setup(struct spi_device *spi) > /* help drivers fail *cleanly* when they need options > * that aren't supported with their current master > */ > + if (((spi->mode >> 8) & 0x03) == 0x03 || > + ((spi->mode >> 10) & 0x03) == 0x03) { > + dev_err(&spi->dev, > + "setup: can not select dual and quad at the same time\n"); > + return -EINVAL; > + } Maybe it would make more sense to have a spi-rx-width and spi-tx-width than can be set to 1, 2, 4, etc. bits instead of a bunch of properties that are mutually exclusive? Sooner or later someone is going to want 8 bits. > diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h > index 38c2b92..e9ba1cf 100644 > --- a/include/linux/spi/spi.h > +++ b/include/linux/spi/spi.h > @@ -74,7 +74,7 @@ struct spi_device { > struct spi_master *master; > u32 max_speed_hz; > u8 chip_select; > - u8 mode; > + u16 mode; What about this, from include/uapi/linux/spi/spidev.h: /* Read / Write of SPI mode (SPI_MODE_0..SPI_MODE_3) */ #define SPI_IOC_RD_MODE _IOR(SPI_IOC_MAGIC, 1, __u8) #define SPI_IOC_WR_MODE _IOW(SPI_IOC_MAGIC, 1, __u8) Note that you can't just change the type in the ioctl define, as it won't be backward compatible. ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: support DUAL and QUAD[patch v1] @ 2013-07-18 2:25 ` Trent Piepho 0 siblings, 0 replies; 40+ messages in thread From: Trent Piepho @ 2013-07-18 2:25 UTC (permalink / raw) To: yuhang wang Cc: thomas.langer, broonie, linux-mtd, Gupta, Pekon, spi-devel-general, Poddar, Sourav On Wed, Jul 17, 2013 at 6:58 PM, yuhang wang <wangyuhang2014@gmail.com> wrote: > >> > > > >> > > > [Pekon]: Instead of adding new fields you can use existing 'mode' > >> > > > field > >> to > >> > > > pass on the platform specific configurations. And if 'u8 mode' > >> > > > does not > >> > > > suffice you can increase it to 'u32'. But what about spidev? > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c > index 004b10f..36d2451 100644 > --- a/drivers/spi/spi.c > +++ b/drivers/spi/spi.c > @@ -868,6 +868,14 @@ static void of_register_spi_devices(struct > spi_master *master) > spi->mode |= SPI_CS_HIGH; > if (of_find_property(nc, "spi-3wire", NULL)) > spi->mode |= SPI_3WIRE; > + if (of_find_property(nc, "spi-tx-dual", NULL)) > + spi->mode |= SPI_TX_DUAL; > + if (of_find_property(nc, "spi-tx-quad", NULL)) > + spi->mode |= SPI_TX_QUAD; > + if (of_find_property(nc, "spi-rx-dual", NULL)) > + spi->mode |= SPI_RX_DUAL; > + if (of_find_property(nc, "spi-rx-quad", NULL)) > + spi->mode |= SPI_RX_QUAD; > > /* Device speed */ > prop = of_get_property(nc, "spi-max-frequency", &len); > @@ -1316,6 +1324,12 @@ int spi_setup(struct spi_device *spi) > /* help drivers fail *cleanly* when they need options > * that aren't supported with their current master > */ > + if (((spi->mode >> 8) & 0x03) == 0x03 || > + ((spi->mode >> 10) & 0x03) == 0x03) { > + dev_err(&spi->dev, > + "setup: can not select dual and quad at the same time\n"); > + return -EINVAL; > + } Maybe it would make more sense to have a spi-rx-width and spi-tx-width than can be set to 1, 2, 4, etc. bits instead of a bunch of properties that are mutually exclusive? Sooner or later someone is going to want 8 bits. > diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h > index 38c2b92..e9ba1cf 100644 > --- a/include/linux/spi/spi.h > +++ b/include/linux/spi/spi.h > @@ -74,7 +74,7 @@ struct spi_device { > struct spi_master *master; > u32 max_speed_hz; > u8 chip_select; > - u8 mode; > + u16 mode; What about this, from include/uapi/linux/spi/spidev.h: /* Read / Write of SPI mode (SPI_MODE_0..SPI_MODE_3) */ #define SPI_IOC_RD_MODE _IOR(SPI_IOC_MAGIC, 1, __u8) #define SPI_IOC_WR_MODE _IOW(SPI_IOC_MAGIC, 1, __u8) Note that you can't just change the type in the ioctl define, as it won't be backward compatible. ^ permalink raw reply [flat|nested] 40+ messages in thread
[parent not found: <CA+7tXihoZZhpoyvxj58c+Zi-Zy631kK5s-kDoSxPKq=ej0qyvA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: support DUAL and QUAD[patch v1] 2013-07-18 2:25 ` Trent Piepho @ 2013-07-19 2:24 ` yuhang wang -1 siblings, 0 replies; 40+ messages in thread From: yuhang wang @ 2013-07-19 2:24 UTC (permalink / raw) To: Trent Piepho Cc: thomas.langer-th3ZBGNqt+7QT0dZR+AlfA, broonie-DgEjT+Ai2ygdnm+yROfE0A, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Gupta, Pekon, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Poddar, Sourav Hi Trent, 2013/7/18 Trent Piepho <tpiepho-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>: > On Wed, Jul 17, 2013 at 6:58 PM, yuhang wang <wangyuhang2014-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: >> >> > > >> >> > > > [Pekon]: Instead of adding new fields you can use existing 'mode' >> >> > > > field >> >> to >> >> > > > pass on the platform specific configurations. And if 'u8 mode' >> >> > > > does not >> >> > > > suffice you can increase it to 'u32'. > > But what about spidev? > >> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c >> index 004b10f..36d2451 100644 >> --- a/drivers/spi/spi.c >> +++ b/drivers/spi/spi.c >> @@ -868,6 +868,14 @@ static void of_register_spi_devices(struct >> spi_master *master) >> spi->mode |= SPI_CS_HIGH; >> if (of_find_property(nc, "spi-3wire", NULL)) >> spi->mode |= SPI_3WIRE; >> + if (of_find_property(nc, "spi-tx-dual", NULL)) >> + spi->mode |= SPI_TX_DUAL; >> + if (of_find_property(nc, "spi-tx-quad", NULL)) >> + spi->mode |= SPI_TX_QUAD; >> + if (of_find_property(nc, "spi-rx-dual", NULL)) >> + spi->mode |= SPI_RX_DUAL; >> + if (of_find_property(nc, "spi-rx-quad", NULL)) >> + spi->mode |= SPI_RX_QUAD; >> >> /* Device speed */ >> prop = of_get_property(nc, "spi-max-frequency", &len); >> @@ -1316,6 +1324,12 @@ int spi_setup(struct spi_device *spi) >> /* help drivers fail *cleanly* when they need options >> * that aren't supported with their current master >> */ >> + if (((spi->mode >> 8) & 0x03) == 0x03 || >> + ((spi->mode >> 10) & 0x03) == 0x03) { >> + dev_err(&spi->dev, >> + "setup: can not select dual and quad at the same time\n"); >> + return -EINVAL; >> + } > > Maybe it would make more sense to have a spi-rx-width and spi-tx-width > than can be set to 1, 2, 4, etc. bits instead of a bunch of properties > that are mutually exclusive? Sooner or later someone is going to want > 8 bits. > >> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h >> index 38c2b92..e9ba1cf 100644 >> --- a/include/linux/spi/spi.h >> +++ b/include/linux/spi/spi.h >> @@ -74,7 +74,7 @@ struct spi_device { >> struct spi_master *master; >> u32 max_speed_hz; >> u8 chip_select; >> - u8 mode; >> + u16 mode; > > What about this, from include/uapi/linux/spi/spidev.h: > > /* Read / Write of SPI mode (SPI_MODE_0..SPI_MODE_3) */ > #define SPI_IOC_RD_MODE _IOR(SPI_IOC_MAGIC, 1, __u8) > #define SPI_IOC_WR_MODE _IOW(SPI_IOC_MAGIC, 1, __u8) > > Note that you can't just change the type in the ioctl define, as it > won't be backward compatible. well, got it.Still need to be considered. Thanks ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: support DUAL and QUAD[patch v1] @ 2013-07-19 2:24 ` yuhang wang 0 siblings, 0 replies; 40+ messages in thread From: yuhang wang @ 2013-07-19 2:24 UTC (permalink / raw) To: Trent Piepho Cc: thomas.langer, broonie, linux-mtd, Gupta, Pekon, spi-devel-general, Poddar, Sourav Hi Trent, 2013/7/18 Trent Piepho <tpiepho@gmail.com>: > On Wed, Jul 17, 2013 at 6:58 PM, yuhang wang <wangyuhang2014@gmail.com> wrote: >> >> > > >> >> > > > [Pekon]: Instead of adding new fields you can use existing 'mode' >> >> > > > field >> >> to >> >> > > > pass on the platform specific configurations. And if 'u8 mode' >> >> > > > does not >> >> > > > suffice you can increase it to 'u32'. > > But what about spidev? > >> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c >> index 004b10f..36d2451 100644 >> --- a/drivers/spi/spi.c >> +++ b/drivers/spi/spi.c >> @@ -868,6 +868,14 @@ static void of_register_spi_devices(struct >> spi_master *master) >> spi->mode |= SPI_CS_HIGH; >> if (of_find_property(nc, "spi-3wire", NULL)) >> spi->mode |= SPI_3WIRE; >> + if (of_find_property(nc, "spi-tx-dual", NULL)) >> + spi->mode |= SPI_TX_DUAL; >> + if (of_find_property(nc, "spi-tx-quad", NULL)) >> + spi->mode |= SPI_TX_QUAD; >> + if (of_find_property(nc, "spi-rx-dual", NULL)) >> + spi->mode |= SPI_RX_DUAL; >> + if (of_find_property(nc, "spi-rx-quad", NULL)) >> + spi->mode |= SPI_RX_QUAD; >> >> /* Device speed */ >> prop = of_get_property(nc, "spi-max-frequency", &len); >> @@ -1316,6 +1324,12 @@ int spi_setup(struct spi_device *spi) >> /* help drivers fail *cleanly* when they need options >> * that aren't supported with their current master >> */ >> + if (((spi->mode >> 8) & 0x03) == 0x03 || >> + ((spi->mode >> 10) & 0x03) == 0x03) { >> + dev_err(&spi->dev, >> + "setup: can not select dual and quad at the same time\n"); >> + return -EINVAL; >> + } > > Maybe it would make more sense to have a spi-rx-width and spi-tx-width > than can be set to 1, 2, 4, etc. bits instead of a bunch of properties > that are mutually exclusive? Sooner or later someone is going to want > 8 bits. > >> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h >> index 38c2b92..e9ba1cf 100644 >> --- a/include/linux/spi/spi.h >> +++ b/include/linux/spi/spi.h >> @@ -74,7 +74,7 @@ struct spi_device { >> struct spi_master *master; >> u32 max_speed_hz; >> u8 chip_select; >> - u8 mode; >> + u16 mode; > > What about this, from include/uapi/linux/spi/spidev.h: > > /* Read / Write of SPI mode (SPI_MODE_0..SPI_MODE_3) */ > #define SPI_IOC_RD_MODE _IOR(SPI_IOC_MAGIC, 1, __u8) > #define SPI_IOC_WR_MODE _IOW(SPI_IOC_MAGIC, 1, __u8) > > Note that you can't just change the type in the ioctl define, as it > won't be backward compatible. well, got it.Still need to be considered. Thanks ^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: support DUAL and QUAD[patch v1] 2013-07-18 2:25 ` Trent Piepho @ 2013-07-19 19:34 ` Gupta, Pekon -1 siblings, 0 replies; 40+ messages in thread From: Gupta, Pekon @ 2013-07-19 19:34 UTC (permalink / raw) To: Trent Piepho, yuhang wang Cc: thomas.langer-th3ZBGNqt+7QT0dZR+AlfA, Poddar, Sourav, broonie-DgEjT+Ai2ygdnm+yROfE0A, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f > > On Wed, Jul 17, 2013 at 6:58 PM, yuhang wang > <wangyuhang2014-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > > >> > > > > >> > > > [Pekon]: Instead of adding new fields you can use existing 'mode' > > >> > > > field > > >> to > > >> > > > pass on the platform specific configurations. And if 'u8 mode' > > >> > > > does not > > >> > > > suffice you can increase it to 'u32'. > > But what about spidev? > > > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c > > index 004b10f..36d2451 100644 > > --- a/drivers/spi/spi.c > > +++ b/drivers/spi/spi.c > > @@ -868,6 +868,14 @@ static void of_register_spi_devices(struct > > spi_master *master) > > spi->mode |= SPI_CS_HIGH; > > if (of_find_property(nc, "spi-3wire", NULL)) > > spi->mode |= SPI_3WIRE; > > + if (of_find_property(nc, "spi-tx-dual", NULL)) > > + spi->mode |= SPI_TX_DUAL; > > + if (of_find_property(nc, "spi-tx-quad", NULL)) > > + spi->mode |= SPI_TX_QUAD; > > + if (of_find_property(nc, "spi-rx-dual", NULL)) > > + spi->mode |= SPI_RX_DUAL; > > + if (of_find_property(nc, "spi-rx-quad", NULL)) > > + spi->mode |= SPI_RX_QUAD; > > > > /* Device speed */ > > prop = of_get_property(nc, "spi-max-frequency", &len); > > @@ -1316,6 +1324,12 @@ int spi_setup(struct spi_device *spi) > > /* help drivers fail *cleanly* when they need options > > * that aren't supported with their current master > > */ > > + if (((spi->mode >> 8) & 0x03) == 0x03 || > > + ((spi->mode >> 10) & 0x03) == 0x03) { > > + dev_err(&spi->dev, > > + "setup: can not select dual and quad at the same time\n"); > > + return -EINVAL; > > + } > > Maybe it would make more sense to have a spi-rx-width and spi-tx-width > than can be set to 1, 2, 4, etc. bits instead of a bunch of properties > that are mutually exclusive? Sooner or later someone is going to want > 8 bits. > Then it would not remain a serial interface (Serial Peripheral Interface), rather it would be more of parallel memory like interface. And you would not like to use SPI controller for it, instead you would use On-chip General Purpose External Interface controllers (like GPMC) because it gives you more options to tweak and configure. In general, If the place-holders are already there (like 'mode' field in struct spi_device), I would prefer to re-use them. But if you are adding new ones just ensure its scalability and usage. Names don't matter much there then, except for good readability. > > diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h > > index 38c2b92..e9ba1cf 100644 > > --- a/include/linux/spi/spi.h > > +++ b/include/linux/spi/spi.h > > @@ -74,7 +74,7 @@ struct spi_device { > > struct spi_master *master; > > u32 max_speed_hz; > > u8 chip_select; > > - u8 mode; > > + u16 mode; > > What about this, from include/uapi/linux/spi/spidev.h: > > /* Read / Write of SPI mode (SPI_MODE_0..SPI_MODE_3) */ > #define SPI_IOC_RD_MODE _IOR(SPI_IOC_MAGIC, 1, __u8) > #define SPI_IOC_WR_MODE _IOW(SPI_IOC_MAGIC, 1, __u8) > > Note that you can't just change the type in the ioctl define, as it > won't be backward compatible. > Thanks, for pointing this out. Yes this is a constrain, if we change the existing type. But I don't think u8 -> u32 is really required, it was just a thought, not deep-dived. with regards, pekon ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk ^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: support DUAL and QUAD[patch v1] @ 2013-07-19 19:34 ` Gupta, Pekon 0 siblings, 0 replies; 40+ messages in thread From: Gupta, Pekon @ 2013-07-19 19:34 UTC (permalink / raw) To: Trent Piepho, yuhang wang Cc: thomas.langer, Poddar, Sourav, broonie, linux-mtd, spi-devel-general > > On Wed, Jul 17, 2013 at 6:58 PM, yuhang wang > <wangyuhang2014@gmail.com> wrote: > > >> > > > > >> > > > [Pekon]: Instead of adding new fields you can use existing 'mode' > > >> > > > field > > >> to > > >> > > > pass on the platform specific configurations. And if 'u8 mode' > > >> > > > does not > > >> > > > suffice you can increase it to 'u32'. > > But what about spidev? > > > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c > > index 004b10f..36d2451 100644 > > --- a/drivers/spi/spi.c > > +++ b/drivers/spi/spi.c > > @@ -868,6 +868,14 @@ static void of_register_spi_devices(struct > > spi_master *master) > > spi->mode |= SPI_CS_HIGH; > > if (of_find_property(nc, "spi-3wire", NULL)) > > spi->mode |= SPI_3WIRE; > > + if (of_find_property(nc, "spi-tx-dual", NULL)) > > + spi->mode |= SPI_TX_DUAL; > > + if (of_find_property(nc, "spi-tx-quad", NULL)) > > + spi->mode |= SPI_TX_QUAD; > > + if (of_find_property(nc, "spi-rx-dual", NULL)) > > + spi->mode |= SPI_RX_DUAL; > > + if (of_find_property(nc, "spi-rx-quad", NULL)) > > + spi->mode |= SPI_RX_QUAD; > > > > /* Device speed */ > > prop = of_get_property(nc, "spi-max-frequency", &len); > > @@ -1316,6 +1324,12 @@ int spi_setup(struct spi_device *spi) > > /* help drivers fail *cleanly* when they need options > > * that aren't supported with their current master > > */ > > + if (((spi->mode >> 8) & 0x03) == 0x03 || > > + ((spi->mode >> 10) & 0x03) == 0x03) { > > + dev_err(&spi->dev, > > + "setup: can not select dual and quad at the same time\n"); > > + return -EINVAL; > > + } > > Maybe it would make more sense to have a spi-rx-width and spi-tx-width > than can be set to 1, 2, 4, etc. bits instead of a bunch of properties > that are mutually exclusive? Sooner or later someone is going to want > 8 bits. > Then it would not remain a serial interface (Serial Peripheral Interface), rather it would be more of parallel memory like interface. And you would not like to use SPI controller for it, instead you would use On-chip General Purpose External Interface controllers (like GPMC) because it gives you more options to tweak and configure. In general, If the place-holders are already there (like 'mode' field in struct spi_device), I would prefer to re-use them. But if you are adding new ones just ensure its scalability and usage. Names don't matter much there then, except for good readability. > > diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h > > index 38c2b92..e9ba1cf 100644 > > --- a/include/linux/spi/spi.h > > +++ b/include/linux/spi/spi.h > > @@ -74,7 +74,7 @@ struct spi_device { > > struct spi_master *master; > > u32 max_speed_hz; > > u8 chip_select; > > - u8 mode; > > + u16 mode; > > What about this, from include/uapi/linux/spi/spidev.h: > > /* Read / Write of SPI mode (SPI_MODE_0..SPI_MODE_3) */ > #define SPI_IOC_RD_MODE _IOR(SPI_IOC_MAGIC, 1, __u8) > #define SPI_IOC_WR_MODE _IOW(SPI_IOC_MAGIC, 1, __u8) > > Note that you can't just change the type in the ioctl define, as it > won't be backward compatible. > Thanks, for pointing this out. Yes this is a constrain, if we change the existing type. But I don't think u8 -> u32 is really required, it was just a thought, not deep-dived. with regards, pekon ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: support DUAL and QUAD[patch v1] 2013-07-19 19:34 ` Gupta, Pekon @ 2013-07-22 9:33 ` Mark Brown -1 siblings, 0 replies; 40+ messages in thread From: Mark Brown @ 2013-07-22 9:33 UTC (permalink / raw) To: Gupta, Pekon Cc: thomas.langer, linux-mtd, spi-devel-general, Poddar, Sourav, Trent Piepho, yuhang wang [-- Attachment #1.1: Type: text/plain, Size: 934 bytes --] On Fri, Jul 19, 2013 at 07:34:45PM +0000, Gupta, Pekon wrote: > > Maybe it would make more sense to have a spi-rx-width and spi-tx-width > > than can be set to 1, 2, 4, etc. bits instead of a bunch of properties > > that are mutually exclusive? Sooner or later someone is going to want > > 8 bits. > Then it would not remain a serial interface (Serial Peripheral Interface), > rather it would be more of parallel memory like interface. > And you would not like to use SPI controller for it, instead you would use > On-chip General Purpose External Interface controllers (like GPMC) > because it gives you more options to tweak and configure. It's still not beyoond the bounds of possibility, and of course some bright spark might decide to use 3 wires. Unless it's expensive to do so in some way it seems better to keep the interface as generic as possible even if we've no expectation that some of the flexibility will be used. [-- Attachment #1.2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] [-- Attachment #2: Type: text/plain, Size: 144 bytes --] ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: support DUAL and QUAD[patch v1] @ 2013-07-22 9:33 ` Mark Brown 0 siblings, 0 replies; 40+ messages in thread From: Mark Brown @ 2013-07-22 9:33 UTC (permalink / raw) To: Gupta, Pekon Cc: thomas.langer, linux-mtd, spi-devel-general, Poddar, Sourav, Trent Piepho, yuhang wang [-- Attachment #1: Type: text/plain, Size: 934 bytes --] On Fri, Jul 19, 2013 at 07:34:45PM +0000, Gupta, Pekon wrote: > > Maybe it would make more sense to have a spi-rx-width and spi-tx-width > > than can be set to 1, 2, 4, etc. bits instead of a bunch of properties > > that are mutually exclusive? Sooner or later someone is going to want > > 8 bits. > Then it would not remain a serial interface (Serial Peripheral Interface), > rather it would be more of parallel memory like interface. > And you would not like to use SPI controller for it, instead you would use > On-chip General Purpose External Interface controllers (like GPMC) > because it gives you more options to tweak and configure. It's still not beyoond the bounds of possibility, and of course some bright spark might decide to use 3 wires. Unless it's expensive to do so in some way it seems better to keep the interface as generic as possible even if we've no expectation that some of the flexibility will be used. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
[parent not found: <20130722093342.GJ9858-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>]
* RE: support DUAL and QUAD[patch v1] 2013-07-22 9:33 ` Mark Brown @ 2013-07-22 9:56 ` Gupta, Pekon -1 siblings, 0 replies; 40+ messages in thread From: Gupta, Pekon @ 2013-07-22 9:56 UTC (permalink / raw) To: Mark Brown Cc: thomas.langer-th3ZBGNqt+7QT0dZR+AlfA, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Poddar, Sourav, Trent Piepho, yuhang wang > > > > Maybe it would make more sense to have a spi-rx-width and spi-tx-width > > > than can be set to 1, 2, 4, etc. bits instead of a bunch of properties > > > that are mutually exclusive? Sooner or later someone is going to want > > > 8 bits. > > > Then it would not remain a serial interface (Serial Peripheral Interface), > > rather it would be more of parallel memory like interface. > > And you would not like to use SPI controller for it, instead you would use > > On-chip General Purpose External Interface controllers (like GPMC) > > because it gives you more options to tweak and configure. > > It's still not beyoond the bounds of possibility, and of course some > bright spark might decide to use 3 wires. Unless it's expensive to do > so in some way it seems better to keep the interface as generic as > possible even if we've no expectation that some of the flexibility will > be used. > Ok.. Agreed, I don't have any preference here between "rx-width , tx-width" v/s "mode". But please make sure that whichever implementation is chosen, it should allow dynamic changing of channel-width. Granularity should be based on following decisions: (a) struct spi_message: if all transfers of same message follow same channel width. Example: All spi_messages from MTD layer would use Qual-SPI only (both flash-commands and data). (b) struct spi_transfer: if each transfer of same message need to be transferred at different width. Example: Command @ Single-SPI followed by Data @ Quad-SPI with regards, pekon ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk ^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: support DUAL and QUAD[patch v1] @ 2013-07-22 9:56 ` Gupta, Pekon 0 siblings, 0 replies; 40+ messages in thread From: Gupta, Pekon @ 2013-07-22 9:56 UTC (permalink / raw) To: Mark Brown Cc: thomas.langer, linux-mtd, spi-devel-general, Poddar, Sourav, Trent Piepho, yuhang wang > > > > Maybe it would make more sense to have a spi-rx-width and spi-tx-width > > > than can be set to 1, 2, 4, etc. bits instead of a bunch of properties > > > that are mutually exclusive? Sooner or later someone is going to want > > > 8 bits. > > > Then it would not remain a serial interface (Serial Peripheral Interface), > > rather it would be more of parallel memory like interface. > > And you would not like to use SPI controller for it, instead you would use > > On-chip General Purpose External Interface controllers (like GPMC) > > because it gives you more options to tweak and configure. > > It's still not beyoond the bounds of possibility, and of course some > bright spark might decide to use 3 wires. Unless it's expensive to do > so in some way it seems better to keep the interface as generic as > possible even if we've no expectation that some of the flexibility will > be used. > Ok.. Agreed, I don't have any preference here between "rx-width , tx-width" v/s "mode". But please make sure that whichever implementation is chosen, it should allow dynamic changing of channel-width. Granularity should be based on following decisions: (a) struct spi_message: if all transfers of same message follow same channel width. Example: All spi_messages from MTD layer would use Qual-SPI only (both flash-commands and data). (b) struct spi_transfer: if each transfer of same message need to be transferred at different width. Example: Command @ Single-SPI followed by Data @ Quad-SPI with regards, pekon ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: support DUAL and QUAD[patch v1] 2013-07-22 9:56 ` Gupta, Pekon @ 2013-07-22 10:32 ` Huang Shijie -1 siblings, 0 replies; 40+ messages in thread From: Huang Shijie @ 2013-07-22 10:32 UTC (permalink / raw) To: Gupta, Pekon Cc: thomas.langer, Mark Brown, linux-mtd, spi-devel-general, Poddar, Sourav, Trent Piepho, yuhang wang 于 2013年07月22日 17:56, Gupta, Pekon 写道: > (b) struct spi_transfer: if each transfer of same message need to be > transferred at different width. Example: Command @ Single-SPI > followed by Data @ Quad-SPI > I am coding the QuadSpi driver for Freescale's Vybrid now. I think we dot need to change the spi code. the spi layer should not know the different width of the command and the data, let the qspi driver does it. In the m25p_probe(), after we knows that the qspi driver and flash support the Quad read, so we can set the m25p->read_opcode to 0xeb or 0xec. In the qspi driver, it can do the real job. thanks Huang Shijie ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: support DUAL and QUAD[patch v1] @ 2013-07-22 10:32 ` Huang Shijie 0 siblings, 0 replies; 40+ messages in thread From: Huang Shijie @ 2013-07-22 10:32 UTC (permalink / raw) To: Gupta, Pekon Cc: thomas.langer, Mark Brown, linux-mtd, spi-devel-general, Poddar, Sourav, Trent Piepho, yuhang wang 于 2013年07月22日 17:56, Gupta, Pekon 写道: > (b) struct spi_transfer: if each transfer of same message need to be > transferred at different width. Example: Command @ Single-SPI > followed by Data @ Quad-SPI > I am coding the QuadSpi driver for Freescale's Vybrid now. I think we dot need to change the spi code. the spi layer should not know the different width of the command and the data, let the qspi driver does it. In the m25p_probe(), after we knows that the qspi driver and flash support the Quad read, so we can set the m25p->read_opcode to 0xeb or 0xec. In the qspi driver, it can do the real job. thanks Huang Shijie ^ permalink raw reply [flat|nested] 40+ messages in thread
[parent not found: <51ED0A57.4060105-KZfg59tc24xl57MIdRCFDg@public.gmane.org>]
* RE: support DUAL and QUAD[patch v1] 2013-07-22 10:32 ` Huang Shijie @ 2013-07-22 11:21 ` thomas.langer -1 siblings, 0 replies; 40+ messages in thread From: thomas.langer-th3ZBGNqt+7QT0dZR+AlfA @ 2013-07-22 11:21 UTC (permalink / raw) To: b32955-KZfg59tc24xl57MIdRCFDg, pekon-l0cyMroinI0 Cc: broonie-DgEjT+Ai2ygdnm+yROfE0A, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, sourav.poddar-l0cyMroinI0, tpiepho-Re5JQEeQqe8AvxtiuMwx3w, wangyuhang2014-Re5JQEeQqe8AvxtiuMwx3w Hello Huang, Huang Shijie wrote on 2013-07-22: > 于 2013年07月22日 17:56, Gupta, Pekon 写道: >> (b) struct spi_transfer: if each transfer of same message need to be >> transferred at different width. Example: Command @ Single-SPI >> followed by Data @ Quad-SPI > I am coding the QuadSpi driver for Freescale's Vybrid now. > > I think we dot need to change the spi code. the spi layer should not > know the different width of the command and the data, let the qspi > driver does it. NO! > > In the m25p_probe(), after we knows that the qspi driver and flash > support the Quad read, so we can set the > m25p->read_opcode to 0xeb or 0xec. In the qspi driver, it can do the > real job. No, the qspi driver (as example here) should provide a transparent interface only, and the mp25p80 driver, which knows the details of the flash, has to provide the information which parts of a transfer has to be done in single-/dual- or quad-mode. Otherwise you always need to extend the qspi driver for new flashes. > > thanks > Huang Shijie > Best Regards, Thomas ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk _______________________________________________ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general ^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: support DUAL and QUAD[patch v1] @ 2013-07-22 11:21 ` thomas.langer 0 siblings, 0 replies; 40+ messages in thread From: thomas.langer @ 2013-07-22 11:21 UTC (permalink / raw) To: b32955, pekon Cc: broonie, linux-mtd, spi-devel-general, sourav.poddar, tpiepho, wangyuhang2014 Hello Huang, Huang Shijie wrote on 2013-07-22: > 于 2013年07月22日 17:56, Gupta, Pekon 写道: >> (b) struct spi_transfer: if each transfer of same message need to be >> transferred at different width. Example: Command @ Single-SPI >> followed by Data @ Quad-SPI > I am coding the QuadSpi driver for Freescale's Vybrid now. > > I think we dot need to change the spi code. the spi layer should not > know the different width of the command and the data, let the qspi > driver does it. NO! > > In the m25p_probe(), after we knows that the qspi driver and flash > support the Quad read, so we can set the > m25p->read_opcode to 0xeb or 0xec. In the qspi driver, it can do the > real job. No, the qspi driver (as example here) should provide a transparent interface only, and the mp25p80 driver, which knows the details of the flash, has to provide the information which parts of a transfer has to be done in single-/dual- or quad-mode. Otherwise you always need to extend the qspi driver for new flashes. > > thanks > Huang Shijie > Best Regards, Thomas ^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: support DUAL and QUAD[patch v1] 2013-07-22 11:21 ` thomas.langer @ 2013-07-22 12:55 ` Gupta, Pekon -1 siblings, 0 replies; 40+ messages in thread From: Gupta, Pekon @ 2013-07-22 12:55 UTC (permalink / raw) To: thomas.langer, b32955 Cc: broonie, linux-mtd, spi-devel-general, Poddar, Sourav, tpiepho, wangyuhang2014 Hi Shijie, > > Hello Huang, > > Huang Shijie wrote on 2013-07-22: > > 于 2013年07月22日 17:56, Gupta, Pekon 写道: > >> (b) struct spi_transfer: if each transfer of same message need to be > >> transferred at different width. Example: Command @ Single-SPI > >> followed by Data @ Quad-SPI > > I am coding the QuadSpi driver for Freescale's Vybrid now. > > > > I think we dot need to change the spi code. the spi layer should not > > know the different width of the command and the data, let the qspi > > driver does it. > NO! > > > > In the m25p_probe(), after we knows that the qspi driver and flash > > support the Quad read, so we can set the > > m25p->read_opcode to 0xeb or 0xec. In the qspi driver, it can do the > > real job. > No, the qspi driver (as example here) should provide a transparent interface > only, > and the mp25p80 driver, which knows the details of the flash, has to provide > the information > which parts of a transfer has to be done in single-/dual- or quad-mode. > Otherwise you always need to extend the qspi driver for new flashes. > A QSPI interface can be connected to external-Flash and to Camera also. - A camera interface may only require static config of Quad-mode. - A Flash may require dynamic switching between Single-SPI or Quad-SPI based on type of message, like Flash erase command need to be sent only Single-SPI mode, where as Read/Write Data can be done on Quad-SPI mode to increase throughput. Here only the flash driver would know, which commands to send on Single-SPI and which on Quad-SPI. Thus, its better that channel-width information is passed by upper layer driver along with spi_message, so that QSPI-controller driver can dynamically switch between the modes. This also keeps QSPI driver generic irrespective of devices connected to it. with regards, pekon ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: support DUAL and QUAD[patch v1] @ 2013-07-22 12:55 ` Gupta, Pekon 0 siblings, 0 replies; 40+ messages in thread From: Gupta, Pekon @ 2013-07-22 12:55 UTC (permalink / raw) To: thomas.langer, b32955 Cc: broonie, linux-mtd, spi-devel-general, Poddar, Sourav, tpiepho, wangyuhang2014 Hi Shijie, > > Hello Huang, > > Huang Shijie wrote on 2013-07-22: > > 于 2013年07月22日 17:56, Gupta, Pekon 写道: > >> (b) struct spi_transfer: if each transfer of same message need to be > >> transferred at different width. Example: Command @ Single-SPI > >> followed by Data @ Quad-SPI > > I am coding the QuadSpi driver for Freescale's Vybrid now. > > > > I think we dot need to change the spi code. the spi layer should not > > know the different width of the command and the data, let the qspi > > driver does it. > NO! > > > > In the m25p_probe(), after we knows that the qspi driver and flash > > support the Quad read, so we can set the > > m25p->read_opcode to 0xeb or 0xec. In the qspi driver, it can do the > > real job. > No, the qspi driver (as example here) should provide a transparent interface > only, > and the mp25p80 driver, which knows the details of the flash, has to provide > the information > which parts of a transfer has to be done in single-/dual- or quad-mode. > Otherwise you always need to extend the qspi driver for new flashes. > A QSPI interface can be connected to external-Flash and to Camera also. - A camera interface may only require static config of Quad-mode. - A Flash may require dynamic switching between Single-SPI or Quad-SPI based on type of message, like Flash erase command need to be sent only Single-SPI mode, where as Read/Write Data can be done on Quad-SPI mode to increase throughput. Here only the flash driver would know, which commands to send on Single-SPI and which on Quad-SPI. Thus, its better that channel-width information is passed by upper layer driver along with spi_message, so that QSPI-controller driver can dynamically switch between the modes. This also keeps QSPI driver generic irrespective of devices connected to it. with regards, pekon ^ permalink raw reply [flat|nested] 40+ messages in thread
[parent not found: <20980858CB6D3A4BAE95CA194937D5E73E9ECAFC-yXqyApvAXouIQmiDNMet8wC/G2K4zDHf@public.gmane.org>]
* Re: support DUAL and QUAD[patch v1] 2013-07-22 12:55 ` Gupta, Pekon @ 2013-07-23 2:24 ` Huang Shijie -1 siblings, 0 replies; 40+ messages in thread From: Huang Shijie @ 2013-07-23 2:24 UTC (permalink / raw) To: Gupta, Pekon Cc: thomas.langer-th3ZBGNqt+7QT0dZR+AlfA, broonie-DgEjT+Ai2ygdnm+yROfE0A, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Poddar, Sourav, tpiepho-Re5JQEeQqe8AvxtiuMwx3w, wangyuhang2014-Re5JQEeQqe8AvxtiuMwx3w 于 2013年07月22日 20:55, Gupta, Pekon 写道: > A QSPI interface can be connected to external-Flash and to Camera also. > > - A camera interface may only require static config of Quad-mode. I missed this case. thanks Huang Shijie ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk _______________________________________________ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: support DUAL and QUAD[patch v1] @ 2013-07-23 2:24 ` Huang Shijie 0 siblings, 0 replies; 40+ messages in thread From: Huang Shijie @ 2013-07-23 2:24 UTC (permalink / raw) To: Gupta, Pekon Cc: thomas.langer, broonie, linux-mtd, spi-devel-general, Poddar, Sourav, tpiepho, wangyuhang2014 于 2013年07月22日 20:55, Gupta, Pekon 写道: > A QSPI interface can be connected to external-Flash and to Camera also. > > - A camera interface may only require static config of Quad-mode. I missed this case. thanks Huang Shijie ^ permalink raw reply [flat|nested] 40+ messages in thread
[parent not found: <20980858CB6D3A4BAE95CA194937D5E73E9EC9B1-yXqyApvAXouIQmiDNMet8wC/G2K4zDHf@public.gmane.org>]
* Re: support DUAL and QUAD[patch v1] 2013-07-22 9:56 ` Gupta, Pekon @ 2013-07-27 11:35 ` yuhang wang -1 siblings, 0 replies; 40+ messages in thread From: yuhang wang @ 2013-07-27 11:35 UTC (permalink / raw) To: Gupta, Pekon Cc: thomas.langer-th3ZBGNqt+7QT0dZR+AlfA, Mark Brown, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Poddar, Sourav, Trent Piepho 2013/7/22 Gupta, Pekon <pekon-l0cyMroinI0@public.gmane.org>: >> >> > > Maybe it would make more sense to have a spi-rx-width and spi-tx-width >> > > than can be set to 1, 2, 4, etc. bits instead of a bunch of properties >> > > that are mutually exclusive? Sooner or later someone is going to want >> > > 8 bits. >> >> > Then it would not remain a serial interface (Serial Peripheral Interface), >> > rather it would be more of parallel memory like interface. >> > And you would not like to use SPI controller for it, instead you would use >> > On-chip General Purpose External Interface controllers (like GPMC) >> > because it gives you more options to tweak and configure. >> >> It's still not beyoond the bounds of possibility, and of course some >> bright spark might decide to use 3 wires. Unless it's expensive to do >> so in some way it seems better to keep the interface as generic as >> possible even if we've no expectation that some of the flexibility will >> be used. >> > Ok.. Agreed, I don't have any preference here between > "rx-width , tx-width" v/s "mode". > But please make sure that whichever implementation is chosen, > it should allow dynamic changing of channel-width. > > Granularity should be based on following decisions: > (a) struct spi_message: if all transfers of same message follow same > channel width. Example: All spi_messages from MTD layer would > use Qual-SPI only (both flash-commands and data). > > (b) struct spi_transfer: if each transfer of same message need to be > transferred at different width. Example: Command @ Single-SPI > followed by Data @ Quad-SPI > > > with regards, pekon Hi, I corrected the previous patch as below. 1:dual and quad is still as the attribute in mode. Just like SPI_3WIRE, dual and quad also can be regarded as a spi interface. Another point, master->mode_bits and spi_device->mode will be checked in spi_setup to prevend some mode that master dont support. It is better than add new member and do redundant check operation. 2:To spidev, in order to backward compatible, still use SPI_IOC_RD_MODE to deal with user who use <u8 mode>. Add SPI_IOC_EXTRD_MODE to fix the <u16 mode>. And SPI_IOC_RD_LSB_FIRST seems a little redundant, so del it. Signed-off-by: wangyuhang <wangyuhang2014-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> --- drivers/spi/spi.c | 14 ++++++++++++++ drivers/spi/spidev.c | 39 +++++++++++++-------------------------- include/linux/spi/spi.h | 20 ++++++++++++++++++-- include/uapi/linux/spi/spidev.h | 20 +++++++++++++++----- 4 files changed, 60 insertions(+), 33 deletions(-) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 004b10f..dc6a7ba 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -868,6 +868,14 @@ static void of_register_spi_devices(struct spi_master *master) spi->mode |= SPI_CS_HIGH; if (of_find_property(nc, "spi-3wire", NULL)) spi->mode |= SPI_3WIRE; + if (of_find_property(nc, "spi-tx-dual", NULL)) + spi->mode |= SPI_TX_DUAL; + if (of_find_property(nc, "spi-tx-quad", NULL)) + spi->mode |= SPI_TX_QUAD; + if (of_find_property(nc, "spi-rx-dual", NULL)) + spi->mode |= SPI_RX_DUAL; + if (of_find_property(nc, "spi-rx-quad", NULL)) + spi->mode |= SPI_RX_QUAD; /* Device speed */ prop = of_get_property(nc, "spi-max-frequency", &len); @@ -1316,6 +1324,12 @@ int spi_setup(struct spi_device *spi) /* help drivers fail *cleanly* when they need options * that aren't supported with their current master */ + if (((spi->mode >> 8) & 0x03) == 0x03 || + ((spi->mode >> 10) & 0x03) == 0x03) { + dev_err(&spi->dev, + "setup: can not select dual and quad at the same time\n"); + return -EINVAL; + } bad_bits = spi->mode & ~spi->master->mode_bits; if (bad_bits) { dev_err(&spi->dev, "setup: unsupported mode bits %x\n", diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c index 2e0655d..be96cb5 100644 --- a/drivers/spi/spidev.c +++ b/drivers/spi/spidev.c @@ -75,6 +75,9 @@ static DECLARE_BITMAP(minors, N_SPI_MINORS); | SPI_LSB_FIRST | SPI_3WIRE | SPI_LOOP \ | SPI_NO_CS | SPI_READY) +#define SPI_EXTMODE_MASK (SPI_MODE_MASK | SPI_TX_DUAL \ + | SPI_TX_QUAD | SPI_RX_DUAL | SPI_RX_QUAD) + struct spidev_data { dev_t devt; spinlock_t spi_lock; @@ -268,6 +271,8 @@ static int spidev_message(struct spidev_data *spidev, k_tmp->bits_per_word = u_tmp->bits_per_word; k_tmp->delay_usecs = u_tmp->delay_usecs; k_tmp->speed_hz = u_tmp->speed_hz; + k_tmp->tx_nbits = u_tmp->tx_nbits; + k_tmp->rx_nbits = u_tmp->rx_nbits; #ifdef VERBOSE dev_dbg(&spidev->spi->dev, " xfer len %zd %s%s%s%dbits %u usec %uHz\n", @@ -359,10 +364,9 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) retval = __put_user(spi->mode & SPI_MODE_MASK, (__u8 __user *)arg); break; - case SPI_IOC_RD_LSB_FIRST: - retval = __put_user((spi->mode & SPI_LSB_FIRST) ? 1 : 0, - (__u8 __user *)arg); - break; + case SPI_IOC_EXTRD_MODE: + retval = __put_user(spi->mode & SPI_EXTMODE_MASK, + (__u16 __user *)arg); case SPI_IOC_RD_BITS_PER_WORD: retval = __put_user(spi->bits_per_word, (__u8 __user *)arg); break; @@ -372,17 +376,17 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) /* write requests */ case SPI_IOC_WR_MODE: - retval = __get_user(tmp, (u8 __user *)arg); + retval = __get_user(tmp, (u16 __user *)arg); if (retval == 0) { - u8 save = spi->mode; + u16 save = spi->mode; - if (tmp & ~SPI_MODE_MASK) { + if (tmp & ~SPI_EXTMODE_MASK) { retval = -EINVAL; break; } - tmp |= spi->mode & ~SPI_MODE_MASK; - spi->mode = (u8)tmp; + tmp |= spi->mode & ~SPI_EXTMODE_MASK; + spi->mode = (u16)tmp; retval = spi_setup(spi); if (retval < 0) spi->mode = save; @@ -390,23 +394,6 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) dev_dbg(&spi->dev, "spi mode %02x\n", tmp); } break; - case SPI_IOC_WR_LSB_FIRST: - retval = __get_user(tmp, (__u8 __user *)arg); - if (retval == 0) { - u8 save = spi->mode; - - if (tmp) - spi->mode |= SPI_LSB_FIRST; - else - spi->mode &= ~SPI_LSB_FIRST; - retval = spi_setup(spi); - if (retval < 0) - spi->mode = save; - else - dev_dbg(&spi->dev, "%csb first\n", - tmp ? 'l' : 'm'); - } - break; case SPI_IOC_WR_BITS_PER_WORD: retval = __get_user(tmp, (__u8 __user *)arg); if (retval == 0) { diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 38c2b92..222e49e 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -74,7 +74,7 @@ struct spi_device { struct spi_master *master; u32 max_speed_hz; u8 chip_select; - u8 mode; + u16 mode; #define SPI_CPHA 0x01 /* clock phase */ #define SPI_CPOL 0x02 /* clock polarity */ #define SPI_MODE_0 (0|0) /* (original MicroWire) */ @@ -87,6 +87,10 @@ struct spi_device { #define SPI_LOOP 0x20 /* loopback mode */ #define SPI_NO_CS 0x40 /* 1 dev/bus, no chipselect */ #define SPI_READY 0x80 /* slave pulls low to pause */ +#define SPI_TX_DUAL 0x100 /* transmit with 2 wires */ +#define SPI_TX_QUAD 0x200 /* transmit with 4 wires */ +#define SPI_RX_DUAL 0x400 /* receive with 2 wires */ +#define SPI_RX_QUAD 0x800 /* receive with 4 wires */ u8 bits_per_word; int irq; void *controller_state; @@ -437,6 +441,8 @@ extern struct spi_master *spi_busnum_to_master(u16 busnum); * @rx_buf: data to be read (dma-safe memory), or NULL * @tx_dma: DMA address of tx_buf, if @spi_message.is_dma_mapped * @rx_dma: DMA address of rx_buf, if @spi_message.is_dma_mapped + * @tx_nbits: number of bits used for writting + * @rx_nbits: number of bits used for reading * @len: size of rx and tx buffers (in bytes) * @speed_hz: Select a speed other than the device default for this * transfer. If 0 the default (from @spi_device) is used. @@ -491,6 +497,11 @@ extern struct spi_master *spi_busnum_to_master(u16 busnum); * by the results of previous messages and where the whole transaction * ends when the chipselect goes intactive. * + * When SPI can transfer in 1x,2x or 4x. It can get this tranfer information + * from device through @tx_nbits and @rx_nbits. In Bi-direction, these + * two should both be set. User can set transfer mode with SPI_NBITS_SINGLE(1x) + * SPI_NBITS_DUAL(2x) and SPI_NBITS_QUAD(4x) to support these three transfer. + * * The code that submits an spi_message (and its spi_transfers) * to the lower layers is responsible for managing its memory. * Zero-initialize every field you don't set up explicitly, to @@ -511,6 +522,11 @@ struct spi_transfer { dma_addr_t rx_dma; unsigned cs_change:1; + u8 tx_nbits; + u8 rx_nbits; +#define SPI_NBITS_SINGLE 0x0; /* 1bit transfer */ +#define SPI_NBITS_DUAL 0x01; /* 2bits transfer */ +#define SPI_NBITS_QUAD 0x02; /* 4bits transfer */ u8 bits_per_word; u16 delay_usecs; u32 speed_hz; @@ -858,7 +874,7 @@ struct spi_board_info { /* mode becomes spi_device.mode, and is essential for chips * where the default of SPI_CS_HIGH = 0 is wrong. */ - u8 mode; + u16 mode; /* ... may need additional spi_device chip config data here. * avoid stuff protocol drivers can set; but include stuff diff --git a/include/uapi/linux/spi/spidev.h b/include/uapi/linux/spi/spidev.h index 52d9ed0..5dc1e95 100644 --- a/include/uapi/linux/spi/spidev.h +++ b/include/uapi/linux/spi/spidev.h @@ -42,7 +42,14 @@ #define SPI_LOOP 0x20 #define SPI_NO_CS 0x40 #define SPI_READY 0x80 - +#define SPI_TX_DUAL 0x100 +#define SPI_TX_QUAD 0x200 +#define SPI_RX_DUAL 0x400 +#define SPI_RX_QUAD 0x800 + +#define SPI_NBITS_SINGLE 0x0; /* 1bit transfer */ +#define SPI_NBITS_DUAL 0x01; /* 2bits transfer */ +#define SPI_NBITS_QUAD 0x02; /* 4bits transfer */ /*---------------------------------------------------------------------------*/ /* IOCTL commands */ @@ -54,6 +61,8 @@ * @tx_buf: Holds pointer to userspace buffer with transmit data, or null. * If no data is provided, zeroes are shifted out. * @rx_buf: Holds pointer to userspace buffer for receive data, or null. + * @tx_nbits: number of bits used for writting. + * @rx_nbits: number of bits used for reading. * @len: Length of tx and rx buffers, in bytes. * @speed_hz: Temporary override of the device's bitrate. * @bits_per_word: Temporary override of the device's wordsize. @@ -85,6 +94,8 @@ struct spi_ioc_transfer { __u64 tx_buf; __u64 rx_buf; + __u8 tx_nbits; + __u8 rx_nbits; __u32 len; __u32 speed_hz; @@ -112,11 +123,10 @@ struct spi_ioc_transfer { /* Read / Write of SPI mode (SPI_MODE_0..SPI_MODE_3) */ #define SPI_IOC_RD_MODE _IOR(SPI_IOC_MAGIC, 1, __u8) -#define SPI_IOC_WR_MODE _IOW(SPI_IOC_MAGIC, 1, __u8) +#define SPI_IOC_WR_MODE _IOW(SPI_IOC_MAGIC, 1, __u16) -/* Read / Write SPI bit justification */ -#define SPI_IOC_RD_LSB_FIRST _IOR(SPI_IOC_MAGIC, 2, __u8) -#define SPI_IOC_WR_LSB_FIRST _IOW(SPI_IOC_MAGIC, 2, __u8) +/* Read of SPI mode (including SPI DUAL/QUAD) */ +#define SPI_IOC_EXTRD_MODE _IOR(SPI_IOC_MAGIC, 2, __u16) /* Read / Write SPI device word length (1..N) */ #define SPI_IOC_RD_BITS_PER_WORD _IOR(SPI_IOC_MAGIC, 3, __u8) ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: support DUAL and QUAD[patch v1] @ 2013-07-27 11:35 ` yuhang wang 0 siblings, 0 replies; 40+ messages in thread From: yuhang wang @ 2013-07-27 11:35 UTC (permalink / raw) To: Gupta, Pekon Cc: thomas.langer, Mark Brown, linux-mtd, spi-devel-general, Poddar, Sourav, Trent Piepho 2013/7/22 Gupta, Pekon <pekon@ti.com>: >> >> > > Maybe it would make more sense to have a spi-rx-width and spi-tx-width >> > > than can be set to 1, 2, 4, etc. bits instead of a bunch of properties >> > > that are mutually exclusive? Sooner or later someone is going to want >> > > 8 bits. >> >> > Then it would not remain a serial interface (Serial Peripheral Interface), >> > rather it would be more of parallel memory like interface. >> > And you would not like to use SPI controller for it, instead you would use >> > On-chip General Purpose External Interface controllers (like GPMC) >> > because it gives you more options to tweak and configure. >> >> It's still not beyoond the bounds of possibility, and of course some >> bright spark might decide to use 3 wires. Unless it's expensive to do >> so in some way it seems better to keep the interface as generic as >> possible even if we've no expectation that some of the flexibility will >> be used. >> > Ok.. Agreed, I don't have any preference here between > "rx-width , tx-width" v/s "mode". > But please make sure that whichever implementation is chosen, > it should allow dynamic changing of channel-width. > > Granularity should be based on following decisions: > (a) struct spi_message: if all transfers of same message follow same > channel width. Example: All spi_messages from MTD layer would > use Qual-SPI only (both flash-commands and data). > > (b) struct spi_transfer: if each transfer of same message need to be > transferred at different width. Example: Command @ Single-SPI > followed by Data @ Quad-SPI > > > with regards, pekon Hi, I corrected the previous patch as below. 1:dual and quad is still as the attribute in mode. Just like SPI_3WIRE, dual and quad also can be regarded as a spi interface. Another point, master->mode_bits and spi_device->mode will be checked in spi_setup to prevend some mode that master dont support. It is better than add new member and do redundant check operation. 2:To spidev, in order to backward compatible, still use SPI_IOC_RD_MODE to deal with user who use <u8 mode>. Add SPI_IOC_EXTRD_MODE to fix the <u16 mode>. And SPI_IOC_RD_LSB_FIRST seems a little redundant, so del it. Signed-off-by: wangyuhang <wangyuhang2014@gmail.com> --- drivers/spi/spi.c | 14 ++++++++++++++ drivers/spi/spidev.c | 39 +++++++++++++-------------------------- include/linux/spi/spi.h | 20 ++++++++++++++++++-- include/uapi/linux/spi/spidev.h | 20 +++++++++++++++----- 4 files changed, 60 insertions(+), 33 deletions(-) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 004b10f..dc6a7ba 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -868,6 +868,14 @@ static void of_register_spi_devices(struct spi_master *master) spi->mode |= SPI_CS_HIGH; if (of_find_property(nc, "spi-3wire", NULL)) spi->mode |= SPI_3WIRE; + if (of_find_property(nc, "spi-tx-dual", NULL)) + spi->mode |= SPI_TX_DUAL; + if (of_find_property(nc, "spi-tx-quad", NULL)) + spi->mode |= SPI_TX_QUAD; + if (of_find_property(nc, "spi-rx-dual", NULL)) + spi->mode |= SPI_RX_DUAL; + if (of_find_property(nc, "spi-rx-quad", NULL)) + spi->mode |= SPI_RX_QUAD; /* Device speed */ prop = of_get_property(nc, "spi-max-frequency", &len); @@ -1316,6 +1324,12 @@ int spi_setup(struct spi_device *spi) /* help drivers fail *cleanly* when they need options * that aren't supported with their current master */ + if (((spi->mode >> 8) & 0x03) == 0x03 || + ((spi->mode >> 10) & 0x03) == 0x03) { + dev_err(&spi->dev, + "setup: can not select dual and quad at the same time\n"); + return -EINVAL; + } bad_bits = spi->mode & ~spi->master->mode_bits; if (bad_bits) { dev_err(&spi->dev, "setup: unsupported mode bits %x\n", diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c index 2e0655d..be96cb5 100644 --- a/drivers/spi/spidev.c +++ b/drivers/spi/spidev.c @@ -75,6 +75,9 @@ static DECLARE_BITMAP(minors, N_SPI_MINORS); | SPI_LSB_FIRST | SPI_3WIRE | SPI_LOOP \ | SPI_NO_CS | SPI_READY) +#define SPI_EXTMODE_MASK (SPI_MODE_MASK | SPI_TX_DUAL \ + | SPI_TX_QUAD | SPI_RX_DUAL | SPI_RX_QUAD) + struct spidev_data { dev_t devt; spinlock_t spi_lock; @@ -268,6 +271,8 @@ static int spidev_message(struct spidev_data *spidev, k_tmp->bits_per_word = u_tmp->bits_per_word; k_tmp->delay_usecs = u_tmp->delay_usecs; k_tmp->speed_hz = u_tmp->speed_hz; + k_tmp->tx_nbits = u_tmp->tx_nbits; + k_tmp->rx_nbits = u_tmp->rx_nbits; #ifdef VERBOSE dev_dbg(&spidev->spi->dev, " xfer len %zd %s%s%s%dbits %u usec %uHz\n", @@ -359,10 +364,9 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) retval = __put_user(spi->mode & SPI_MODE_MASK, (__u8 __user *)arg); break; - case SPI_IOC_RD_LSB_FIRST: - retval = __put_user((spi->mode & SPI_LSB_FIRST) ? 1 : 0, - (__u8 __user *)arg); - break; + case SPI_IOC_EXTRD_MODE: + retval = __put_user(spi->mode & SPI_EXTMODE_MASK, + (__u16 __user *)arg); case SPI_IOC_RD_BITS_PER_WORD: retval = __put_user(spi->bits_per_word, (__u8 __user *)arg); break; @@ -372,17 +376,17 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) /* write requests */ case SPI_IOC_WR_MODE: - retval = __get_user(tmp, (u8 __user *)arg); + retval = __get_user(tmp, (u16 __user *)arg); if (retval == 0) { - u8 save = spi->mode; + u16 save = spi->mode; - if (tmp & ~SPI_MODE_MASK) { + if (tmp & ~SPI_EXTMODE_MASK) { retval = -EINVAL; break; } - tmp |= spi->mode & ~SPI_MODE_MASK; - spi->mode = (u8)tmp; + tmp |= spi->mode & ~SPI_EXTMODE_MASK; + spi->mode = (u16)tmp; retval = spi_setup(spi); if (retval < 0) spi->mode = save; @@ -390,23 +394,6 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) dev_dbg(&spi->dev, "spi mode %02x\n", tmp); } break; - case SPI_IOC_WR_LSB_FIRST: - retval = __get_user(tmp, (__u8 __user *)arg); - if (retval == 0) { - u8 save = spi->mode; - - if (tmp) - spi->mode |= SPI_LSB_FIRST; - else - spi->mode &= ~SPI_LSB_FIRST; - retval = spi_setup(spi); - if (retval < 0) - spi->mode = save; - else - dev_dbg(&spi->dev, "%csb first\n", - tmp ? 'l' : 'm'); - } - break; case SPI_IOC_WR_BITS_PER_WORD: retval = __get_user(tmp, (__u8 __user *)arg); if (retval == 0) { diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 38c2b92..222e49e 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -74,7 +74,7 @@ struct spi_device { struct spi_master *master; u32 max_speed_hz; u8 chip_select; - u8 mode; + u16 mode; #define SPI_CPHA 0x01 /* clock phase */ #define SPI_CPOL 0x02 /* clock polarity */ #define SPI_MODE_0 (0|0) /* (original MicroWire) */ @@ -87,6 +87,10 @@ struct spi_device { #define SPI_LOOP 0x20 /* loopback mode */ #define SPI_NO_CS 0x40 /* 1 dev/bus, no chipselect */ #define SPI_READY 0x80 /* slave pulls low to pause */ +#define SPI_TX_DUAL 0x100 /* transmit with 2 wires */ +#define SPI_TX_QUAD 0x200 /* transmit with 4 wires */ +#define SPI_RX_DUAL 0x400 /* receive with 2 wires */ +#define SPI_RX_QUAD 0x800 /* receive with 4 wires */ u8 bits_per_word; int irq; void *controller_state; @@ -437,6 +441,8 @@ extern struct spi_master *spi_busnum_to_master(u16 busnum); * @rx_buf: data to be read (dma-safe memory), or NULL * @tx_dma: DMA address of tx_buf, if @spi_message.is_dma_mapped * @rx_dma: DMA address of rx_buf, if @spi_message.is_dma_mapped + * @tx_nbits: number of bits used for writting + * @rx_nbits: number of bits used for reading * @len: size of rx and tx buffers (in bytes) * @speed_hz: Select a speed other than the device default for this * transfer. If 0 the default (from @spi_device) is used. @@ -491,6 +497,11 @@ extern struct spi_master *spi_busnum_to_master(u16 busnum); * by the results of previous messages and where the whole transaction * ends when the chipselect goes intactive. * + * When SPI can transfer in 1x,2x or 4x. It can get this tranfer information + * from device through @tx_nbits and @rx_nbits. In Bi-direction, these + * two should both be set. User can set transfer mode with SPI_NBITS_SINGLE(1x) + * SPI_NBITS_DUAL(2x) and SPI_NBITS_QUAD(4x) to support these three transfer. + * * The code that submits an spi_message (and its spi_transfers) * to the lower layers is responsible for managing its memory. * Zero-initialize every field you don't set up explicitly, to @@ -511,6 +522,11 @@ struct spi_transfer { dma_addr_t rx_dma; unsigned cs_change:1; + u8 tx_nbits; + u8 rx_nbits; +#define SPI_NBITS_SINGLE 0x0; /* 1bit transfer */ +#define SPI_NBITS_DUAL 0x01; /* 2bits transfer */ +#define SPI_NBITS_QUAD 0x02; /* 4bits transfer */ u8 bits_per_word; u16 delay_usecs; u32 speed_hz; @@ -858,7 +874,7 @@ struct spi_board_info { /* mode becomes spi_device.mode, and is essential for chips * where the default of SPI_CS_HIGH = 0 is wrong. */ - u8 mode; + u16 mode; /* ... may need additional spi_device chip config data here. * avoid stuff protocol drivers can set; but include stuff diff --git a/include/uapi/linux/spi/spidev.h b/include/uapi/linux/spi/spidev.h index 52d9ed0..5dc1e95 100644 --- a/include/uapi/linux/spi/spidev.h +++ b/include/uapi/linux/spi/spidev.h @@ -42,7 +42,14 @@ #define SPI_LOOP 0x20 #define SPI_NO_CS 0x40 #define SPI_READY 0x80 - +#define SPI_TX_DUAL 0x100 +#define SPI_TX_QUAD 0x200 +#define SPI_RX_DUAL 0x400 +#define SPI_RX_QUAD 0x800 + +#define SPI_NBITS_SINGLE 0x0; /* 1bit transfer */ +#define SPI_NBITS_DUAL 0x01; /* 2bits transfer */ +#define SPI_NBITS_QUAD 0x02; /* 4bits transfer */ /*---------------------------------------------------------------------------*/ /* IOCTL commands */ @@ -54,6 +61,8 @@ * @tx_buf: Holds pointer to userspace buffer with transmit data, or null. * If no data is provided, zeroes are shifted out. * @rx_buf: Holds pointer to userspace buffer for receive data, or null. + * @tx_nbits: number of bits used for writting. + * @rx_nbits: number of bits used for reading. * @len: Length of tx and rx buffers, in bytes. * @speed_hz: Temporary override of the device's bitrate. * @bits_per_word: Temporary override of the device's wordsize. @@ -85,6 +94,8 @@ struct spi_ioc_transfer { __u64 tx_buf; __u64 rx_buf; + __u8 tx_nbits; + __u8 rx_nbits; __u32 len; __u32 speed_hz; @@ -112,11 +123,10 @@ struct spi_ioc_transfer { /* Read / Write of SPI mode (SPI_MODE_0..SPI_MODE_3) */ #define SPI_IOC_RD_MODE _IOR(SPI_IOC_MAGIC, 1, __u8) -#define SPI_IOC_WR_MODE _IOW(SPI_IOC_MAGIC, 1, __u8) +#define SPI_IOC_WR_MODE _IOW(SPI_IOC_MAGIC, 1, __u16) -/* Read / Write SPI bit justification */ -#define SPI_IOC_RD_LSB_FIRST _IOR(SPI_IOC_MAGIC, 2, __u8) -#define SPI_IOC_WR_LSB_FIRST _IOW(SPI_IOC_MAGIC, 2, __u8) +/* Read of SPI mode (including SPI DUAL/QUAD) */ +#define SPI_IOC_EXTRD_MODE _IOR(SPI_IOC_MAGIC, 2, __u16) /* Read / Write SPI device word length (1..N) */ #define SPI_IOC_RD_BITS_PER_WORD _IOR(SPI_IOC_MAGIC, 3, __u8) ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: support DUAL and QUAD[patch v1] 2013-07-16 8:59 ` yuhang wang @ 2013-07-16 9:29 ` Mark Brown -1 siblings, 0 replies; 40+ messages in thread From: Mark Brown @ 2013-07-16 9:29 UTC (permalink / raw) To: yuhang wang Cc: linux-mtd, Grant Likely, linux-mtd, Gupta, Pekon, spi-devel-general, Poddar, Sourav [-- Attachment #1.1: Type: text/plain, Size: 513 bytes --] On Tue, Jul 16, 2013 at 04:59:49PM +0800, yuhang wang wrote: > > [Pekon]: Instead of adding new fields you can use existing 'mode' field to > > pass on the platform specific configurations. And if 'u8 mode' does not > > suffice you can increase it to 'u32'. > > #define QSPI_MODE 1 << 5; // just check which bit-fields are un-used > > spi_board_info->mode |= QSPI_MODE; > well, can dual and quad be regarded as a spi mode? if so, your comment seems > to be right. Yes, I think this is a good suggestion. [-- Attachment #1.2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] [-- Attachment #2: Type: text/plain, Size: 144 bytes --] ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: support DUAL and QUAD[patch v1] @ 2013-07-16 9:29 ` Mark Brown 0 siblings, 0 replies; 40+ messages in thread From: Mark Brown @ 2013-07-16 9:29 UTC (permalink / raw) To: yuhang wang Cc: linux-mtd, Grant Likely, linux-mtd, Gupta, Pekon, spi-devel-general, Poddar, Sourav [-- Attachment #1: Type: text/plain, Size: 513 bytes --] On Tue, Jul 16, 2013 at 04:59:49PM +0800, yuhang wang wrote: > > [Pekon]: Instead of adding new fields you can use existing 'mode' field to > > pass on the platform specific configurations. And if 'u8 mode' does not > > suffice you can increase it to 'u32'. > > #define QSPI_MODE 1 << 5; // just check which bit-fields are un-used > > spi_board_info->mode |= QSPI_MODE; > well, can dual and quad be regarded as a spi mode? if so, your comment seems > to be right. Yes, I think this is a good suggestion. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
end of thread, other threads:[~2013-07-27 11:35 UTC | newest] Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-07-10 8:34 SPI: support DUAL and QUAD[patch v1] yuhang wang 2013-07-10 8:34 ` yuhang wang [not found] ` <CAHSAbzP0Nc1ahf_ra6yDuMvV998_+EGcUOGDv5ifKaagYf+NOg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2013-07-16 8:08 ` Gupta, Pekon 2013-07-16 8:08 ` Gupta, Pekon [not found] ` <20980858CB6D3A4BAE95CA194937D5E73E9E84D2-yXqyApvAXouIQmiDNMet8wC/G2K4zDHf@public.gmane.org> 2013-07-16 8:59 ` yuhang wang 2013-07-16 8:59 ` yuhang wang [not found] ` <CAHSAbzMDOWo5oBqUHqMxybuDwUjFbczYwN+85gH+rxZ9z8u2Pw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2013-07-16 9:11 ` Gupta, Pekon 2013-07-16 9:11 ` Gupta, Pekon [not found] ` <20980858CB6D3A4BAE95CA194937D5E73E9E851E-yXqyApvAXouIQmiDNMet8wC/G2K4zDHf@public.gmane.org> 2013-07-16 10:12 ` thomas.langer-th3ZBGNqt+7QT0dZR+AlfA 2013-07-16 10:12 ` thomas.langer [not found] ` <593AEF6C47F46446852B067021A273D6D93B49E1-6yu8tajIPx7U45ihrnOXy0EOCMrvLtNR@public.gmane.org> 2013-07-16 11:20 ` Gupta, Pekon 2013-07-16 11:20 ` Gupta, Pekon [not found] ` <20980858CB6D3A4BAE95CA194937D5E73E9E85D0-yXqyApvAXouIQmiDNMet8wC/G2K4zDHf@public.gmane.org> 2013-07-16 11:59 ` yuhang wang 2013-07-16 11:59 ` yuhang wang 2013-07-16 12:18 ` thomas.langer-th3ZBGNqt+7QT0dZR+AlfA 2013-07-16 12:18 ` thomas.langer 2013-07-18 1:58 ` yuhang wang 2013-07-18 1:58 ` yuhang wang [not found] ` <CAHSAbzP5jqHwCm0qhu=DiNzpP0swqWHsQ0=MnF+EKaSN22E5hg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2013-07-18 2:25 ` Trent Piepho 2013-07-18 2:25 ` Trent Piepho [not found] ` <CA+7tXihoZZhpoyvxj58c+Zi-Zy631kK5s-kDoSxPKq=ej0qyvA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2013-07-19 2:24 ` yuhang wang 2013-07-19 2:24 ` yuhang wang 2013-07-19 19:34 ` Gupta, Pekon 2013-07-19 19:34 ` Gupta, Pekon 2013-07-22 9:33 ` Mark Brown 2013-07-22 9:33 ` Mark Brown [not found] ` <20130722093342.GJ9858-GFdadSzt00ze9xe1eoZjHA@public.gmane.org> 2013-07-22 9:56 ` Gupta, Pekon 2013-07-22 9:56 ` Gupta, Pekon 2013-07-22 10:32 ` Huang Shijie 2013-07-22 10:32 ` Huang Shijie [not found] ` <51ED0A57.4060105-KZfg59tc24xl57MIdRCFDg@public.gmane.org> 2013-07-22 11:21 ` thomas.langer-th3ZBGNqt+7QT0dZR+AlfA 2013-07-22 11:21 ` thomas.langer 2013-07-22 12:55 ` Gupta, Pekon 2013-07-22 12:55 ` Gupta, Pekon [not found] ` <20980858CB6D3A4BAE95CA194937D5E73E9ECAFC-yXqyApvAXouIQmiDNMet8wC/G2K4zDHf@public.gmane.org> 2013-07-23 2:24 ` Huang Shijie 2013-07-23 2:24 ` Huang Shijie [not found] ` <20980858CB6D3A4BAE95CA194937D5E73E9EC9B1-yXqyApvAXouIQmiDNMet8wC/G2K4zDHf@public.gmane.org> 2013-07-27 11:35 ` yuhang wang 2013-07-27 11:35 ` yuhang wang 2013-07-16 9:29 ` Mark Brown 2013-07-16 9:29 ` Mark Brown
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.