--- David Brownell <[email protected]> wrote:
> > > > I notice that there is no bus lock. Are you expecting the adapter
> > > > driver to handle the fact that its transfer routine could be called
> > > > before a previous call returns?
> > >
> > > Yes. The transfer routine is purely async, and its responsibility
> > > is to append that spi_message to the current queue. (Assuming
> > > the driver isn't a simple pure-PIO driver without a queue...)
> > >
> > > That's a simple matter of a spin_lock_irqsave/list_add_tail/unlock.
> > >
> >
> > OK. Thought so. I think that in the documentation (when it gets written ;)
> > we need to warn people that they can only do quick work (adding message
> > to a queue or waking up a kthread) in the transfer routine
>
> The documented constraint -- right by the declaration of that
> particular method!! -- is that it may not sleep. That suffices.
>
Sorry, must have had my glasses on back to front ;)
>
> > as it would
> > not be fair for a PIO driver to transfer several KB in what might be
> > interrupt context.
>
> That's a "quality of implementation" issue. There are a lot of
> different SPI drivers floating around today that are pure PIO;
> they're used for sensor access, and work in exactly that way.
> (And without any buslock.)
>
> When the driver is only reading/writing a handful of bytes, PIO
> can easily be "quick" ... and may well be quicker than going
> through a queue manager. Example: if SPI is clocked at 8 MHz,
> that's a microsecond per byte. Add a smidgeon of overhead,
> and call it 5 usecs to read a sensor that way.
>
> One point of standardizing an API is to support a broad range
> of different controller driver optimization points. They should
> all work correctly of course. A DMA driver may be the ticket for
> running from SPI flash ... but setting up DMA for just a couple
> bytes is likely not a win.
>
True. In our SPI adapter driver we check to see if the transfer is below is certain size in which
case it is quicker to do PIO, otherwise we do DMA.
>
> > So your asking the adapter to keep a 'personality' for each device on
> > that bus (clock speed, cs & clock mode etc) and then just before the
> > transfer to/from a device is started the adapter takes the 'personality'
> > of that device (i.e. sets clock speed registers if needed etc)?
>
> As you noted later, yes. Most of the SPI controllers I've looked
> at will do that in hardware, for that matter. PCI drivers don't
> need to arbitrate bus access themselves; neither should SPI drivers.
>
OK. Our hardware doesn't :(, so I'll have to emulate it. It's an interesting idea and as you say
it is more optimal for devices that have this support :).
To make it quicker for devices that don't have this support in hardware how would you feel about
having a 'void *personality' pointer in the spi_device structure which the adapter could use for
storing and accessing the register settings for clock etc for that SPI device?
>
> > > > > +EXPORT_SYMBOL_GPL(spi_new_device);
> > > >
> > > > I think we should have a bus lock (in the adapter structure) for
> > > > safety, and in the remove routine as well.
> > >
> > > Why? I don't see any need for one, at least in the "all drivers
> > > must use this one" category. Persuade me; what problems would such
> > > a lock solve?
> > >
> >
> > Problems with parallel calls to register spi device/unregister
> > spi device/transfer?
>
> Only an issue if the driver core had bugs ... bugs that would break
> many more things than just SPI. :)
>
>
> > > The parallel port adapter wouldn't use that interface. It would
> > > instead be using spi_new_device() with board_info matching the
> > > device (Ethernet, EEPROM, USB controller, etc) ...
> >
> > OK. So if I had an array of devices then I have to go though that array
> > and call spi_new_device() for each one? Where do I get spi_master
> > from? I need a function to which I can pass the name/bus number to and
> > get a spi_master pointer in return.
>
> You're the one who's defining the "parallel port adapter with device"
> thing ... so you've got the spi_master that you created. In fact you
> probably used dev_set_drvdata(dev, master) to keep it handy.
>
Ahh, but the spi_master structure is in /usr/src/linux/drivers/spi/busses/spi-parport.c and my
array of devices is in ~/spi-work/parprt_adapter_1.c
>
>
> > Sorry I didn't make myself clear. I mean check the complete element in
> > the spi_message structure when spi_transfer is called. So:
> >
> > int spi_transfer(struct spi_device *spi, struct spi_message *message)
> > {
> > if (message->complete)
> > /* We have callback so transfer is async */
> > else
> > /* We have no callback so transfer is sync */
> > }
> >
> > Although thinking about it this is probably a bad idea as it could b
> > prone to errors
>
> That's a large part of why I would never support that model. :)
>
>
> > > > Hmm, using local variables for messages, so DMA adapter drivers have
> > > > to check if this is non-kmalloc'ed space (how?)
> > >
> > > They can't check that. It turns out that most current Linuxes
> > > have no issues DMAing a few bytes from the stack.
> >
> > Will the DMA remapping calls work with data from the stack?
>
> On "most current Linuxes" yes. All I know about, in fact.
> But it's not guaranteed.
OK. Thanks
Mark
>
> - Dave
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
___________________________________________________________
Yahoo! Messenger - NEW crystal clear PC to PC calling worldwide with voicemail http://uk.messenger.yahoo.com
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
