On Fri, Jun 10, 2022 at 09:27:53AM +0200, David Jander wrote: > Mark Brown <broonie@xxxxxxxxxx> wrote: > > On Thu, Jun 09, 2022 at 05:34:21PM +0200, David Jander wrote: > > > Apparently some drivers tend to rely on the fact that master->cur_msg is not > > > NULL and always points to the message being transferred. > > > This could be a show-stopper to this patch set, if it cannot be solved. > > That seems resolvable? If we have two things actually handling a > Ok, I first thought that this wouldn't be possible without taking the > necessary spinlock, but looking a little closer, I think I understand now. > One question to confirm I understand the code correctly: > An SPI driver that implements its own transfer_one_message() is required to > _always_ call spi_finalize_current_message() _before_ returning, right? Yes, it should. > If this is a guarantee and we take the io_mutex at the beginning of > __spi_pump_messages(), then ctlr->cur_msg is only manipulated with the > io_mutex held, and that would make it safe to be used in the sync path, which > is also behind the io_mutex. > Would appreciate if you could confirm this, just to be sure I understand the > code correctly. I think that should work. If there's something missed it should be possible to make things work that way. > The fact that spi_finalize_current_message() is a separate API function, and > not called directly from __spi_pump_messages() had me confused that it might > be called in a different context (from IRQ thread or something like that) > possibly after __spi_pump_messages() had already returned. But that doesn't > make much sense... right? It *can* be called from interrupt context, the driver can signal the caller that the message completed without having to bounce up to the message pump first which helps minimise context switching in the async case, whatever was waiting for the message can get on with things without us having to switch through the SPI thread. However something should still be in io_mutex when the completion happens at present. A brief audit of drivers suggests that this is not in fact reliably the case though and we've got drivers that were never tested with any queued transfers AFAICT. I'll look at that. Notionally the idea was that we'd be able to start the next transfer directly from interrupt context on suitable hardware, however nothing I had access to at the time I was working on this was actually capable of doing that so I never implemented anything. Actually, on that point - are you aware of any boards (ideally not requiring some non-standard DT) that are readily and affordably available and have useful CAN controllers? I'm looking to improve my test setup and they keep coming up.
Attachment:
signature.asc
Description: PGP signature
