Date: Sun, 6 Oct 2013 14:05:00 -0700 From: Adrian Chadd <adrian.chadd@gmail.com> To: Alexander Motin <mav@freebsd.org> Cc: Ilya Bakulin <ilya@bakulin.de>, "freebsd-embedded@freebsd.org" <freebsd-embedded@freebsd.org> Subject: Re: SDIO for FreeBSD Message-ID: <CAJ-Vmon0xr597We=sF5Uhja2qb=viuiqCbQXZgUvp7VX-FsPfA@mail.gmail.com> In-Reply-To: <5251C911.6020003@FreeBSD.org> References: <5251A9D3.1080803@bakulin.de> <5251C911.6020003@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi,
(Sorry, top-posting from a mobile device. Sorry.)
Yes, i agree with mav. If we need so many threads, I think it's a good sign
that the design needs some more thought.
What's USB do?
I think here we can get away with doing it via taskqueues. Ie, we post
interrupts in the same context(s) that we post SDIO command completions.
I'd really prefer this to be slightly more thought out. I've no idea how
you're currently implementing it and as I'm knee deep in the AR9344/MIPS74K
stuff, I'd like to focus on that until it's up to a point that others can
jump in and help with the testing and bringup. But I think modelling it
after USB would be good - have driver methods that get called to drive the
command transmit/receive and then have the interrupt handling be called
from the same taskqueue. That way your interrupt code would just be
something like:
driver_intr()
{
set_flag_that_states_an_interrupt_occured();
submit_some_register_read_command_to_the_transmit_queue();
sdio_start_xfer();
}
.. then sdio_start_xfer() would tell your stack to start a transfer if the
queue wasn't already running. It would then call the driver method to queue
another command(s) to the SDIO stack. The SDIO stack can then post them to
the hardware when it has time.
Thanks,
-adrian
On Sunday, October 6, 2013, Alexander Motin wrote:
> 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<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?CAJ-Vmon0xr597We=sF5Uhja2qb=viuiqCbQXZgUvp7VX-FsPfA>