Date: Sun, 06 Oct 2013 23:33:21 +0300 From: Alexander Motin <mav@FreeBSD.org> To: Ilya Bakulin <ilya@bakulin.de> Cc: "freebsd-embedded@freebsd.org" <freebsd-embedded@freebsd.org> Subject: Re: SDIO for FreeBSD Message-ID: <5251C911.6020003@FreeBSD.org> In-Reply-To: <5251A9D3.1080803@bakulin.de> References: <5251A9D3.1080803@bakulin.de>
next in thread | previous in thread | raw e-mail | index | archive | help
On 06.10.2013 21:20, Ilya Bakulin wrote: > Just a quick status update. > I have managed to make interrupts work. Was rather silly copy&paste mistake. > > So now I'm able to issue a command to the WiFi card and actually get an > interrupt from > the controller. > > I have added several bus methods for setting up interrupt handlers and > allocating resources, > both to the MMC stack and to my WiFi driver, as well as mv_sdio SDIO > controller driver for DreamPlug. > The code is kept under [1], as usual. > > I have the fully working stack now and I'd like to discuss the locking > model for SDIO. > > So, the SDIO controller detects an interrupt. > SDIO controller driver handles it its own interrupt thread (I assume > that we use ITHREAD ISRs). > All the controller can do at this stage is: > 1. read device registers to get the interrupt cause; > 2. clear/set some bits in device registers; > 3. If there was a command- or data-related interrupt, call req->done() > method of struct mmc_request, > which sets MMC_REQ_DONE flag in the request structure and then calls > wakeup() on mmc_request. > This is what actually happens now. MMC stack which is doing msleep() > waiting for MMC_REQ_DONE > flag detects this flag and continues with request processing, returning > a result to the upper layer. > See mmc_wakeup() and mmc_wait_for_req() in sys/dev/mmc/mmc.c > > Everything is syncronous at this point, no complications. > > Now the SDIO case. The controller can detect an interrupt at any time, > then it notifies the MMC stack > about it. This happens in the ITHREAD of SDIO controller's ISR. > In MMC stack, we now want to read the "Penging interrupts" register to > figure out which function has > raised the interrupt. We cannot, however, issue another request to SDIO > controller since that inhibits > sleeping and we cannot sleep in ISR. We will also need to call ISRs of > our child devices that can possibly > do another weird stuff. > > So I propose adding a dedicated thread in MMC layer that will just > process the interrupts from the card. > It can be started on MMC stack attach, grab MMC lock and then just sleep > on condition variable > (which will be per-bus, ie for each SDIO card since there can only be > one SD[IO] card on the bus), giving the lock back. > When the MMC stack's ISR is called by SDIO controller, it will just > cv_signal() that variable and do nothing more than that. > The MMC interrupt servicing thread will then wakeup (and grab the MMC > lock!), > it will be able to do another MMC request to read "pending interrupts" > register > and call the ISRs of respective child drivers. > We expect them to be quick and probably do the similar stuff -- just > signaling CVs > and/or taskqueue_enqueue()-ing interrupt processing tasks. I am sorry, can't hold from rephrasing the joke: how many threads does it take to handle an interrupt? ;) IMHO 3 threads used to handle single hardware interrupt is a good point to invent something better. That would be easy if controller and MMC layers used the same lock and MMC could send interrupt status request asynchronously just from the ISR, but they don't. May be things could be much more simple if if would be controller driver obligation to read interrupt status from the card instead of jumping up and down over the stack and having to use additional threads to decouple the locks? That is like in CAM for both ATA and SCSI now it is controller driver duty to fetch errors details from the device, even sending additional commands if needed. > Sorry if I totally misunderstand the concept here, please tell your > opinions. > > [1] https://github.com/kibab/freebsd/compare/master...kibab-dplug > Note that I have made a rebase and pushed the rebased branch back to GitHub. > This is an epic fuckup and it requires everyone to delete the local > branch and pull it once again, > as the IDs have changed. Sorry for that, I will do merges next time. -- Alexander Motin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5251C911.6020003>