From: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> To: Stefan Roese <sr-ynQEQJNshbs@public.gmane.org> Cc: Nadav Haklai <nadavh-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>, Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>, Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Subject: Re: [PATCH RFC] spi: orion.c: Add direct write mode Date: Thu, 14 Jan 2016 10:52:11 +0000 [thread overview] Message-ID: <20160114105211.GQ6588@sirena.org.uk> (raw) In-Reply-To: <569747E4.2080402-ynQEQJNshbs@public.gmane.org> [-- Attachment #1: Type: text/plain, Size: 2040 bytes --] On Thu, Jan 14, 2016 at 08:01:56AM +0100, Stefan Roese wrote: > On 12.01.2016 11:13, Mark Brown wrote: > >>+ /* Use SPI direct write mode if such an address is provided via DT */ > >>+ orion_spi = spi_master_get_devdata(spi->master); > >>+ direct_addr = orion_spi->slave_direct_addr[spi->chip_select]; > >>+ if (direct_addr && xfer->tx_buf) { > >>+ /* Deassert CS between the SPI transfers */ > >>+ writel(0x00010000, spi_reg(orion_spi, > >>+ SPI_DIRECT_WRITE_CONFIG_REG)); > >This is badly broken, we should be asserting /CS over the entire message > >unless the individual transfer says otherwise. I'm surprised this > >works. > It works, at least on the FPGA that I'm programming the bitstream into > right now. The reason for deasserting the CS after each SPI transfer > was, to work around potential problems with concurrent accesses to > other SPI devices connected to the same SPI controller. Like SPI NOR > flash devices using the "normal" indirect mode. Deasserting the CS > seemed like the "safe" way to me here. No, this is not remotely safe - it is going to break any device where the driver uses more than one transfer per message. Instead of one operation on the bus with /CS held over the entire operation the device will see multiple operations. > >and what if the transfer is bidirectional? It looks like we > >can only do this for transmit only transfers. > This patch only adds support for the direct write mode. As the main > purpose is to speed up the SPI TX throughput for FPGA bitstream > programming. It could be extended to support also the direct read > mode. But this would need more configuration options, like the > number of address-bytes to transfer in each read access. Not sure > how this should interact with the SPI flash driver from the MTD > subsystem. > I would prefer to not adding this direct read mode just yet. It > can be added later, if really needed. I can't see anything which will prevent trying to use direct write in combination with a read which I'd not expect to work. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: broonie@kernel.org (Mark Brown) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH RFC] spi: orion.c: Add direct write mode Date: Thu, 14 Jan 2016 10:52:11 +0000 [thread overview] Message-ID: <20160114105211.GQ6588@sirena.org.uk> (raw) In-Reply-To: <569747E4.2080402@denx.de> On Thu, Jan 14, 2016 at 08:01:56AM +0100, Stefan Roese wrote: > On 12.01.2016 11:13, Mark Brown wrote: > >>+ /* Use SPI direct write mode if such an address is provided via DT */ > >>+ orion_spi = spi_master_get_devdata(spi->master); > >>+ direct_addr = orion_spi->slave_direct_addr[spi->chip_select]; > >>+ if (direct_addr && xfer->tx_buf) { > >>+ /* Deassert CS between the SPI transfers */ > >>+ writel(0x00010000, spi_reg(orion_spi, > >>+ SPI_DIRECT_WRITE_CONFIG_REG)); > >This is badly broken, we should be asserting /CS over the entire message > >unless the individual transfer says otherwise. I'm surprised this > >works. > It works, at least on the FPGA that I'm programming the bitstream into > right now. The reason for deasserting the CS after each SPI transfer > was, to work around potential problems with concurrent accesses to > other SPI devices connected to the same SPI controller. Like SPI NOR > flash devices using the "normal" indirect mode. Deasserting the CS > seemed like the "safe" way to me here. No, this is not remotely safe - it is going to break any device where the driver uses more than one transfer per message. Instead of one operation on the bus with /CS held over the entire operation the device will see multiple operations. > >and what if the transfer is bidirectional? It looks like we > >can only do this for transmit only transfers. > This patch only adds support for the direct write mode. As the main > purpose is to speed up the SPI TX throughput for FPGA bitstream > programming. It could be extended to support also the direct read > mode. But this would need more configuration options, like the > number of address-bytes to transfer in each read access. Not sure > how this should interact with the SPI flash driver from the MTD > subsystem. > I would prefer to not adding this direct read mode just yet. It > can be added later, if really needed. I can't see anything which will prevent trying to use direct write in combination with a read which I'd not expect to work. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 473 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160114/d0fc1ebc/attachment.sig>
next prev parent reply other threads:[~2016-01-14 10:52 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-01-12 9:02 [PATCH RFC] spi: orion.c: Add direct write mode Stefan Roese 2016-01-12 9:02 ` Stefan Roese [not found] ` <1452589339-21402-1-git-send-email-sr-ynQEQJNshbs@public.gmane.org> 2016-01-12 10:13 ` Mark Brown 2016-01-12 10:13 ` Mark Brown [not found] ` <20160112101358.GY6588-GFdadSzt00ze9xe1eoZjHA@public.gmane.org> 2016-01-14 7:01 ` Stefan Roese 2016-01-14 7:01 ` Stefan Roese [not found] ` <569747E4.2080402-ynQEQJNshbs@public.gmane.org> 2016-01-14 10:52 ` Mark Brown [this message] 2016-01-14 10:52 ` Mark Brown
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20160114105211.GQ6588@sirena.org.uk \ --to=broonie-dgejt+ai2ygdnm+yrofe0a@public.gmane.org \ --cc=gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org \ --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \ --cc=linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=nadavh-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org \ --cc=sr-ynQEQJNshbs@public.gmane.org \ --cc=thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.