| Message ID | 1440570367-22569-2-git-send-email-ranjit.waghmode@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Aug 26, 2015 at 11:56:04AM +0530, Ranjit Waghmode wrote: > To support dual parallel mode operation of ZynqMP GQSPI controller > following API's are added inside the core: As covered in SubmittingPatches please try to make each patch a single change rather than having multiple separate changes in one commit. > + /* Controller may support more than one chip. > + * This flag will enable that feature. > + */ > +#define SPI_MASTER_BOTH_CS BIT(8) /* enable both chips */ This isn't saying that the controller supports more than one chip, it's saying that the controller supports asserting more than one chip select at once which isn't the same thing. I'm also not entirely sure that this makes sense as a separate feature to the data striping one - I'm struggling to think of a way to use this sensibly separately to that.
Hi Mark, > -----Original Message----- > From: Mark Brown [mailto:broonie@kernel.org] > Sent: Thursday, September 03, 2015 5:43 PM > To: Ranjit Abhimanyu Waghmode > Cc: dwmw2@infradead.org; computersforpeace@gmail.com; Michal Simek; > Soren Brinkmann; zajec5@gmail.com; ben@decadent.org.uk; marex@denx.de; > b32955@freescale.com; knut.wohlrab@de.bosch.com; juhosg@openwrt.org; > beanhuo@micron.com; linux-mtd@lists.infradead.org; linux- > kernel@vger.kernel.org; linux-spi@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; Harini Katakam; Punnaiah Choudary Kalluri > Subject: Re: [LINUX RFC v2 1/4] spi: add support of two chip selects & data > stripe > > On Wed, Aug 26, 2015 at 11:56:04AM +0530, Ranjit Waghmode wrote: > > > To support dual parallel mode operation of ZynqMP GQSPI controller > > following API's are added inside the core: > > As covered in SubmittingPatches please try to make each patch a single change > rather than having multiple separate changes in one commit. I will split the patch. > > > + /* Controller may support more than one chip. > > + * This flag will enable that feature. > > + */ > > +#define SPI_MASTER_BOTH_CS BIT(8) /* enable both > chips */ > > This isn't saying that the controller supports more than one chip, it's saying that > the controller supports asserting more than one chip select at once which isn't > the same thing. I'm also not entirely sure that this makes sense as a separate > feature to the data striping one - I'm struggling to think of a way to use this > sensibly separately to that. If the SPI controller is having more than one chip select and the data lines are distributed equally. And also there is requirement to activate all the chip selects in one go. Now we can consider following use cases: Suppose we need to send the same data to multiple slaves of same kind: Here the application need not to do individual slave access for writing, instead it can send data to all the devices in one go. Let's take another case where application is trying to send data in such a way that first nibble of the byte will got to the one slave and the second nibble of the byte will go to the other slave: Here data in slave devices can be organized by taking advantage of above topology along with the support in hardware. Regards, Ranjit -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On 03.09.2015, at 14:12, Mark Brown <broonie@kernel.org> wrote: > > On Wed, Aug 26, 2015 at 11:56:04AM +0530, Ranjit Waghmode wrote: > >> To support dual parallel mode operation of ZynqMP GQSPI controller >> following API's are added inside the core: > > As covered in SubmittingPatches please try to make each patch a single > change rather than having multiple separate changes in one commit. > >> + /* Controller may support more than one chip. >> + * This flag will enable that feature. >> + */ >> +#define SPI_MASTER_BOTH_CS BIT(8) /* enable both chips */ > > This isn't saying that the controller supports more than one chip, it's > saying that the controller supports asserting more than one chip select > at once which isn't the same thing. I'm also not entirely sure that > this makes sense as a separate feature to the data striping one - I'm > struggling to think of a way to use this sensibly separately to that. Well - there is one use-case that I can think of: fbtft has the requirement for some devices to control a GPIO to differentiate between command and data getting transferred - sort of 9 bit. Right now it is done outside of spi in the fbtft driver itself wrapping spi_sync(). Similarly a “hold” line on an eeprom or similar could get (de)asserted without requiring holding a spi-bus-lock. But then the current patch would not allow this kind of “generic” use-case. Martin -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 04, 2015 at 02:35:52PM +0200, Martin Sperl wrote: > > On 03.09.2015, at 14:12, Mark Brown <broonie@kernel.org> wrote: > > This isn't saying that the controller supports more than one chip, it's > > saying that the controller supports asserting more than one chip select > > at once which isn't the same thing. I'm also not entirely sure that > > this makes sense as a separate feature to the data striping one - I'm > > struggling to think of a way to use this sensibly separately to that. > Well - there is one use-case that I can think of: > fbtft has the requirement for some devices to control a GPIO to > differentiate between command and data getting transferred > - sort of 9 bit. That's another thing again, isn't it? It's one device switching between two different control interfaces at runtime rather than two devices controlled in lockstep.
> On 04.09.2015, at 17:37, Mark Brown <broonie@kernel.org> wrote: > > On Fri, Sep 04, 2015 at 02:35:52PM +0200, Martin Sperl wrote: >>> On 03.09.2015, at 14:12, Mark Brown <broonie@kernel.org> wrote: > >>> This isn't saying that the controller supports more than one chip, it's >>> saying that the controller supports asserting more than one chip select >>> at once which isn't the same thing. I'm also not entirely sure that >>> this makes sense as a separate feature to the data striping one - I'm >>> struggling to think of a way to use this sensibly separately to that. > >> Well - there is one use-case that I can think of: >> fbtft has the requirement for some devices to control a GPIO to >> differentiate between command and data getting transferred >> - sort of 9 bit. > > That's another thing again, isn't it? It's one device switching between > two different control interfaces at runtime rather than two devices > controlled in lockstep. I agree, but there may be a solution that can handle both, so I wanted to mention it. -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 04, 2015 at 12:02:21PM +0000, Ranjit Abhimanyu Waghmode wrote: Please fix your mail client to word wrap within paragraphs and to quote text without reflowing it - your messages are very hard to read. > > > + /* Controller may support more than one chip. > > > + * This flag will enable that feature. > > > + */ > > > +#define SPI_MASTER_BOTH_CS BIT(8) /* enable both > > chips */ > > This isn't saying that the controller supports more than one chip, it's saying that > > the controller supports asserting more than one chip select at once which isn't > > the same thing. I'm also not entirely sure that this makes sense as a separate > > feature to the data striping one - I'm struggling to think of a way to use this > > sensibly separately to that. > If the SPI controller is having more than one chip select and the data lines are distributed equally. > And also there is requirement to activate all the chip selects in one go. I'm not sure I understand the above, sorry. At least not in so far as how it relates to my concerns, especially the fact that the comment says this enables support for more than one chip which is obviously a basic SPI feature. > Now we can consider following use cases: > Suppose we need to send the same data to multiple slaves of same kind: > Here the application need not to do individual slave access for writing, instead it can send data to all the devices in one go. That's a *very* specific application which will only work for write only devices - I'd be surprised if such systems actually had distinct chip select lines at the CPU level. > Let's take another case where application is trying to send data in such a way that first nibble of the byte will got to the one slave and the second nibble of the byte will go to the other slave: > Here data in slave devices can be organized by taking advantage of above topology along with the support in hardware. But do such devices actually exist? I can imagine systems that might be able to do that but I'd be very surprised to see anyone practically designing them, they're going to be quite hard to use.
Hi Mark, On Fri, Sep 11, 2015 at 6:06 PM, Mark Brown <broonie@kernel.org> wrote: > On Fri, Sep 04, 2015 at 12:02:21PM +0000, Ranjit Abhimanyu Waghmode wrote: > > Please fix your mail client to word wrap within paragraphs and to quote > text without reflowing it - your messages are very hard to read. > >> > > + /* Controller may support more than one chip. >> > > + * This flag will enable that feature. >> > > + */ >> > > +#define SPI_MASTER_BOTH_CS BIT(8) /* enable both >> > chips */ > >> Now we can consider following use cases: > >> Suppose we need to send the same data to multiple slaves of same kind: >> Here the application need not to do individual slave access for writing, instead it can send data to all the devices in one go. > > That's a *very* specific application which will only work for write only > devices - I'd be surprised if such systems actually had distinct chip > select lines at the CPU level. > Agreed that it is very specific but here are a few ways it is used when communicating with two flash devices in parallel configuration: - Write enable is sent to both devices using a single operation. - Writing to any configuration registers in the flash is done in one go - Some application that want to mirror important data to both devices. Even with reading, the assertion of multiple cs combined with stripe will mean: - Two status bytes, one form each will be obtained in one operation - Similarly data that was written using stripe is read back and combined. Such systems could still maintain separate chip selects to perform individual operations such as reading flash ID, debugging failures or locking specific sectors. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index cf8b91b..22e8e7f 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -1828,6 +1828,14 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message) if (list_empty(&message->transfers)) return -EINVAL; + /* + * Data stripe option is selected if and only if when + * two chips are enabled + */ + if ((master->flags & SPI_MASTER_DATA_STRIPE) + && !(master->flags & SPI_MASTER_BOTH_CS)) + return -EINVAL; + /* Half-duplex links include original MicroWire, and ones with * only one data pin like SPI_3WIRE (switches direction) or where * either MOSI or MISO is missing. They can also be caused by diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index d673072..53d3bc6 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -355,6 +355,17 @@ struct spi_master { #define SPI_MASTER_NO_TX BIT(2) /* can't do buffer write */ #define SPI_MASTER_MUST_RX BIT(3) /* requires rx */ #define SPI_MASTER_MUST_TX BIT(4) /* requires tx */ + /* Controller may support data stripe feature when more than one + * chips are present. + * Setting data stripe will send data in following manner: + * -> even bytes i.e. 0, 2, 4,... are transmitted on lower data bus + * -> odd bytes i.e. 1, 3, 5,.. are transmitted on upper data bus + */ +#define SPI_MASTER_DATA_STRIPE BIT(7) /* support data stripe */ + /* Controller may support more than one chip. + * This flag will enable that feature. + */ +#define SPI_MASTER_BOTH_CS BIT(8) /* enable both chips */ /* lock and mutex for SPI bus locking */ spinlock_t bus_lock_spinlock;
To support dual parallel mode operation of ZynqMP GQSPI controller following API's are added inside the core: - Added API to support two chip selects: Dual parallel mode supports two SPI flash memories operating in parallel i.e 8 I/O lines. Chip selects and clock are shared to both the flash devices. So newly added API will help in enabling both the chips. - Added API to support data stripe feature: with data stripe enabled, even bytes i.e. 0, 2, 4,... are transmitted on lower data bus odd bytes i.e. 1, 3, 5,.. are transmitted on upper data bus. Signed-off-by: Ranjit Waghmode <ranjit.waghmode@xilinx.com> --- V2 Changes: - Added error handling condition for newly added features --- drivers/spi/spi.c | 8 ++++++++ include/linux/spi/spi.h | 11 +++++++++++ 2 files changed, 19 insertions(+) -- 2.1.2 -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html