Date: Thu, 21 Feb 2013 16:16:27 +0200 From: Aleksandr Rybalko <ray@freebsd.org> To: Warner Losh <imp@bsdimp.com> Cc: Patrick Kelsey <kelsey@ieee.org>, freebsd-arm@freebsd.org, ticso@cicely.de, freebsd-mips@freebsd.org Subject: Re: SPI, _sz fields in struct spi_command Message-ID: <20130221161627.478d35aa88c95c03cafb00a7@freebsd.org> In-Reply-To: <62EDE91B-DDF3-462D-8802-7A7FBF00CE91@bsdimp.com> References: <20130220142140.f8e5a616c75d72e2519dbc69@freebsd.org> <54C08D8E-4C5F-49AF-BEE6-D78EC05D2A24@bsdimp.com> <CAD44qMXkFH9iR=ym1XBtD88HRadpGkO=WRYvz5xhAVucEuoLEA@mail.gmail.com> <20130220174449.GB6976@cicely7.cicely.de> <CAD44qMXsdrhuNRbpA1a9ikj4BGffVfhv1WY6hsqCxHwVzQAdsg@mail.gmail.com> <20130221022655.6f693eb6.ray@freebsd.org> <62EDE91B-DDF3-462D-8802-7A7FBF00CE91@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 20 Feb 2013 19:49:20 -0700 Warner Losh <imp@bsdimp.com> wrote: > > On Feb 20, 2013, at 5:26 PM, Aleksandr Rybalko wrote: > > > On Wed, 20 Feb 2013 13:57:52 -0500 > > Patrick Kelsey <kelsey@ieee.org> wrote: > > > >> On Wed, Feb 20, 2013 at 12:44 PM, Bernd Walter > >> <ticso@cicely7.cicely.de> wrote: > >>> On Wed, Feb 20, 2013 at 10:40:35AM -0500, Patrick Kelsey wrote: > >> > >>>> In (1) and (3) above, the drivers for those chips could usefully > >>>> tell the SPI bus interface that they aren't interested in received > >>>> data when transmitting to the SPI-attached devices, and certainly > >>>> it would be at least a small convenience to such SPI interface > >>>> consumers to have the lower level driver handle it. Since not all > >>>> host-side SPI interface hardware supports discarding received > >>>> data, to allow a zero-sized receive buffer in the command passed > >>>> down, at least some SPI interface hardware drivers will have to > >>>> dynamically allocate or maintain a static discard buffer and > >>>> provide error responses for allocation-failure or > >>>> transfer-too-large. > >>> > >>> If it is SPI hardware drivers decision you at least nee to take into > >>> account that in some RX-only cases you need 0xff dummy data. > >>> The slave chip driver knows if the device parses received data, but > >>> the master chip driver won't. > >>> > >>>> Example (2) is equally easy to handle on either side of the SPI bus > >>>> interface by pointing the transmit buffer pointer at the receive > >>>> buffer. > >>> > >>> This destroys the send data, which is not always welcome behavour. > >>> > >> > >> My comments are in the overall context of extending the functionality > >> of the existing SPI bus interface in ways that provide programming > >> conveniences to those who wish to use them. I am not suggesting in > >> any way that the current interface be narrowed based on any set of > >> assumptions, so in my view, there will always be a facility for fully > >> independent transmit and receive buffers specified by the caller. > >> > >> My example (2) was for a slave device that does not have a data input > >> pin and thus is oblivious to any data sent. Certainly, if you have a > >> device that can receive data sent by the master and your code design > >> calls for preserving the send data across bus transactions, you would > >> continue to use two separate buffers. > >> > >> -Patrick > > > > Thanks to all for interesting comments, some of my keys for that topic > > you all already said but I say it again anyway :) > > So why I ask about that: > > 1. Do we really need to allocate second unused buffer? > > Yes, but we could push that requirement into the spi host adapter drivr. > > > 2. Do we really need to spend clock cycles on that buffer? > > The way SPI works is that the cycles are spent, because TX cycles have to equal RX cycles. I understand how it works, I'm about allocating/zeroing/etc for that buffer. :) > > > 3. Ralink-s SPI can't do bidirectional transfer (IIRC I've tested it > > properly, only 1 8bits reg for both RX and TX and same data on read > > after TX byte) (Sorry for not commited yet, will do, hope soon :) ) > > 4. Since we have sz for both direction and both types (cmd, data) why > > controller drivers force us to make it equal? (at91_spi_transfer > > KASSERT) > > Because they must be equal. However, if we're pushing the buffer management down a level, then we can remove them. They must be equal for controller, but it's good to have way to hint controller about "we don't need to RX anything, so take care your requirements yourself" :) So at91_spi will use some own buffer for that case, but other controllers will not spend time on that buffer. > > > What I think we need to do with that: > > 1. remove KASSERT in controller drivers, make driver to accept at least > > cases when (tx_sz == rx_sz), (tx_sz && !rx_sz), (!tx_sz && rx_sz). Plus > > if controller require some buffer to drain into it (don't know how to > > correctly say in English what men do after many beers :) ) allocate it > > or make some static buffer. > > Yes, moving the responsibility from the driver to the spi controller would be possible. Yep > > > 2. teach consumers to give only correct numbers (very nice we have only > > two SPI devices in tree) > > I thought the at45 was bidirectional... Nope, all transfers fill some part of tx buffer, then read on offset=sizeof(some part) of rx buffer. > > > After that we will be able to make drivers for some (potential) devices > > which will require bidirectional communication. And controllers which > > can't do that, will just report error in that. I believe peoples thinks > > before attach such devices to controllers, so we will not have such > > incompatibility. > > If this feature becomes optional, we should have some way for the controller to communicate that it can do bidirectional transfers. If we have that, then such a device could refuse to attach on machines with controllers that can't do htat. In the normal world nobody produce systems which have incompatible parts :) > > > Hope I'm not forget something important :-D > > No, that's OK. > > Warner > > > Thanks to all! > > > > WBW > > -- > > Aleksandr Rybalko <ray@freebsd.org> > -- Aleksandr Rybalko <ray@freebsd.org>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130221161627.478d35aa88c95c03cafb00a7>