From owner-freebsd-embedded@FreeBSD.ORG Sun Oct 6 23:19:45 2013 Return-Path: Delivered-To: freebsd-embedded@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 1F2C1A19 for ; Sun, 6 Oct 2013 23:19:45 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from mail-ie0-f170.google.com (mail-ie0-f170.google.com [209.85.223.170]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id E049B2474 for ; Sun, 6 Oct 2013 23:19:44 +0000 (UTC) Received: by mail-ie0-f170.google.com with SMTP id x13so14280660ief.1 for ; Sun, 06 Oct 2013 16:19:38 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:subject:mime-version:content-type:from :in-reply-to:date:cc:content-transfer-encoding:message-id:references :to; bh=DxjBncSGm3Zir/C3O0QdWjqCWJTfm9pIM4x49j5Iw6g=; b=TJuFpdYT0agU02T8GdLt/qY8IUJmmzETLgU+HuLHc+ZxOwQOTmlHDRlXrHnHq1VuEQ qaKXCAVI2TxH4HrHfpqeUxwa8TWrlXkfixT9XTF5ZBLmL6PNKJagR5P5IuX83fDhr83B uxeUsTuVJKQ42uFunIAt28pxpf+v3oOMxczzwmKa4eFivlvJTSrH9UJPXnte8UwIlx/z 4clAu9Zu6rQCMeEcKwu9lUqHLIxDt1y76wceYdos1+WfF+pbUBjEBMalVBL7MWDxOhqP 4tBmAswvegjz0GeiFbfDbo5R9GUY7WB+7oEc0TdsRZFyd6h/Zh3puzrMQ17VNwSWSzzk 8p2A== X-Gm-Message-State: ALoCoQmZncXm4/1wBCgM8d43gGyAjly/mLOTiyFIo+B8mXEfEfdjJHKDEGoWvOxBynvYLSqD+Lcy X-Received: by 10.43.10.198 with SMTP id pb6mr1891282icb.40.1381101578139; Sun, 06 Oct 2013 16:19:38 -0700 (PDT) Received: from 53.imp.bsdimp.com (50-78-194-198-static.hfc.comcastbusiness.net. [50.78.194.198]) by mx.google.com with ESMTPSA id ri1sm23211189igc.2.1969.12.31.16.00.00 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Sun, 06 Oct 2013 16:19:37 -0700 (PDT) Sender: Warner Losh Subject: Re: SDIO for FreeBSD Mime-Version: 1.0 (Apple Message framework v1085) Content-Type: text/plain; charset=us-ascii From: Warner Losh In-Reply-To: Date: Sun, 6 Oct 2013 17:19:36 -0600 Content-Transfer-Encoding: quoted-printable Message-Id: <4FB11076-DD88-43F2-A449-E41D2088A4DD@bsdimp.com> References: <5251A9D3.1080803@bakulin.de> <5251C911.6020003@FreeBSD.org> To: Adrian Chadd X-Mailer: Apple Mail (2.1085) Cc: Alexander Motin , Ilya Bakulin , "freebsd-embedded@freebsd.org" X-BeenThere: freebsd-embedded@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Dedicated and Embedded Systems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 06 Oct 2013 23:19:45 -0000 I've often thought that having an SDIO protocol for CAM would be where = we'd wind up because once you start adding in GPIO pin interrupt = signalling, message "interrupt" signaling, message completion, etc, it = looks a lot like many of the mechanisms we have in CAM... I'm not at = all sure that USB is the right model to use, frankly... But it does = have some superficial similarities... Warner On Oct 6, 2013, at 3:05 PM, Adrian Chadd wrote: > Hi, >=20 > (Sorry, top-posting from a mobile device. Sorry.) >=20 > 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. >=20 > What's USB do? >=20 > 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. >=20 > 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: >=20 > driver_intr() > { > set_flag_that_states_an_interrupt_occured(); > submit_some_register_read_command_to_the_transmit_queue(); > sdio_start_xfer(); > } >=20 > .. 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. >=20 > Thanks, >=20 >=20 >=20 > -adrian >=20 > 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. >=20 > So now I'm able to issue a command to the WiFi card and actually get = an > interrupt from > the controller. >=20 > 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. >=20 > I have the fully working stack now and I'd like to discuss the locking > model for SDIO. >=20 > 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 >=20 > Everything is syncronous at this point, no complications. >=20 > 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. >=20 > 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. >=20 > I am sorry, can't hold from rephrasing the joke: how many threads does = it take to handle an interrupt? ;) >=20 > 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. >=20 > Sorry if I totally misunderstand the concept here, please tell your > opinions. >=20 > [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. >=20 >=20 > --=20 > Alexander Motin