From owner-freebsd-arm@FreeBSD.ORG Thu Mar 6 01:03:08 2014 Return-Path: Delivered-To: freebsd-arm@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id C97C8A61 for ; Thu, 6 Mar 2014 01:03:08 +0000 (UTC) Received: from mail-pa0-f42.google.com (mail-pa0-f42.google.com [209.85.220.42]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 8BF9F766 for ; Thu, 6 Mar 2014 01:03:08 +0000 (UTC) Received: by mail-pa0-f42.google.com with SMTP id fb1so1877533pad.15 for ; Wed, 05 Mar 2014 17:03:02 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:content-type:mime-version:subject:from :in-reply-to:date:cc:content-transfer-encoding:message-id:references :to; bh=7sbU++ktENm+Fyc+1OZHbIBRsCavaa50VhRDyZ7/RLY=; b=I2qBmoNEho4XE3KiVSLbFAkxmQPZNEzSgR5Sgp5H2vMfQ2bymEc/JOExrjChUu9YoB Jk+Jkm/2v3hyl8sYJLSQuCfYJKSxJRXca/Uhm4iuWWHvXs9ScqufSbpdIYkifMMVG7Xx bM2G/ZOikTsIFYyelaQXgNmDYaeYCna/r2FOiDKviyDPJAf3VAvhiEi74jkIbh6EaqSC QQb0mbLLyo36SEICsdDv6bJAM8PLgLee/mSj/a5R2/pTp5Z1iFIzecJu2FyPxjTGkfX6 38Ojsi0SVlel8nSyLpK5zsZmI8njS2vV5OE+eBb7ygGC+fWBRUmDvYZeB8TdQlWGacyK J92g== X-Gm-Message-State: ALoCoQnNIJipsSwbyliUWrKLv+y3GquVpYtscHs1obXoz+CKNamQnkyU0+tAmpLGH147BxaTWhmd X-Received: by 10.68.245.162 with SMTP id xp2mr10768179pbc.69.1394067782329; Wed, 05 Mar 2014 17:03:02 -0800 (PST) Received: from lglt-rottaway.corp.netflix.com (dc1-prod.netflix.com. [69.53.236.251]) by mx.google.com with ESMTPSA id sm5sm24991394pab.19.2014.03.05.17.03.00 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 05 Mar 2014 17:03:01 -0800 (PST) Sender: Warner Losh Content-Type: text/plain; charset=windows-1252 Mime-Version: 1.0 (Mac OS X Mail 7.2 \(1874\)) Subject: Re: [PATCH] simplebus child device probe order control via FDT (motivated by BeagleBone Black) From: Warner Losh In-Reply-To: Date: Wed, 5 Mar 2014 18:02:58 -0700 Content-Transfer-Encoding: quoted-printable Message-Id: <899B9ABD-0ACC-49F2-9520-CCE837D39875@bsdimp.com> References: <7C2B7036-51CC-4C97-80C4-0A439874357D@bsdimp.com> <1393939277.1149.300.camel@revolution.hippie.lan> <438620C4-7712-4B01-A46F-CB57946A30BF@bsdimp.com> <16A5203B-B06D-4129-A54F-F34D5FA28D2B@bsdimp.com> <39ED4040-2A6A-4D85-97D5-DCDE4ECCA0EC@bsdimp.com> To: Patrick Kelsey X-Mailer: Apple Mail (2.1874) Cc: "freebsd-hackers@freebsd.org" , freebsd-arm , freebsd-embedded@freebsd.org, Ian Lepore X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "Porting FreeBSD to ARM processors." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 06 Mar 2014 01:03:08 -0000 On Mar 5, 2014, at 5:53 PM, Patrick Kelsey wrote: >=20 >=20 >=20 > On Wed, Mar 5, 2014 at 5:44 PM, Warner Losh wrote: >=20 > On Mar 5, 2014, at 2:56 PM, Patrick Kelsey wrote: >=20 > > > > > > > > On Wed, Mar 5, 2014 at 4:24 PM, Warner Losh wrote: > > > > On Mar 5, 2014, at 11:55 AM, Patrick Kelsey wrote: > > > > > > > > > > > > > > On Wed, Mar 5, 2014 at 8:31 AM, Warner Losh = wrote: > > > > > > On Mar 4, 2014, at 11:53 PM, Patrick Kelsey = wrote: > > > > > > > > > > > > > > > > > > > On Tue, Mar 4, 2014 at 8:21 AM, Ian Lepore = wrote: > > > > > > > > There's a standard property for mmc/sd devices, "non-removable" = whose > > > > presence denotes things like soldered-on eMMC parts. Only one = of our > > > > many sdhci drivers supports it right now (imx). In general the = core > > > > part of the driver (dev/sdhci) doesn't have good support for > > > > insert/remove/presence detection that's handled off to the side = (whether > > > > it's non-removable or a gpio pin). > > > > > > > > That alone doesn't solve the wider problem, though, which I = think breaks > > > > down into two pieces. Let's say you've got two slots, call them = left > > > > and right. You end up with these two problems... > > > > > > > > * Sometimes the right slot is mmcsd0, but it turns into = mmcsd1 if > > > > there's a card in the left slot when I boot; I don't = want it to > > > > change. > > > > > > > > And not just a boot-time issue, of course. If you were to = remove those two cards and then reinsert them in the opposite = time-order, their device names would swap. > > > > > > > > * I want the slot on the left to be named '1' and the = right to be > > > > '0'. > > > > > > > > The first problem is easily solved without impacting dts in any = way. We > > > > just wire down the relationship controllerN -> mmcN -> mmcsdN. = This is > > > > exactly equivelent to the old ATA_STATIC_ID option in its effect = -- you > > > > don't get to choose the names, but you know they won't change = based on > > > > which devices are present. It could be controlled with a = tunable. > > > > > > > > It's harder to envision the fix for the second part without = adding an > > > > ad-hoc property for the devices. Even with a property I'm not = sure how > > > > easy it would be. > > > > > > > > We should be able to assign a geographic address = (controllerX:slotY) to each mmcsd device in a given system (let's ignore = for now the theoretical possibility of multiple cards on one bus). The = 'controllerX' part of the address could be the controller's pathname = from the devicetree, or an index assigned by mmcbr machinery at attach = time. The "slotY" part of the address is assigned by the specific = controller device driver using some internally-determined fixed mapping = between the assigned values and its physical slots. This geographic = address could be used to create an additional set of /dev entries with = stable names. There could be a mechanism for user-configurable aliases = for the geographical addresses. > > > > > > There=92s already a chance to run a script when a device is = attached that can create /dev/slot0 or /dev/slot1 that has geographical = information available to it. People use ddvd for this in the USB world = all the time to make sure that tty devices get a symlink based on their = location or serial number. > > > > > > Why is mmc so different it needs its own mechanism? > > > > > > I'm laying out my view of the information that would be needed and = the types of actions that would have to be taken based on that = information to solve the issue. I'm not saying devd can't be the piece = that is used to implement the actions (indeed, I noted devd as a = potential building block for a solution in my initial response). I'm = also not saying mmc needs its own mechanism, I'm saying it needs /a/ = mechanism, and so far there still seems to be something missing (because = it's not there, or I'm still ignorant of it). > > > > > > What seems to be missing is geographical addressing for mmc = devices. > > > > > > I think what you might be saying is that the existing mmcsd = add/remove code could be augmented to send devctl notifications, along = the lines of: > > > > > > devctl_notify("MMC", "DEVICE", "ATTACH"|"DETACH", "... = controller=3D = slot=3D rca=3D") > > > > > > and then I and the fine author of devctl and devd would be = pleased. > > > > MMC doesn=92t need anything special here. That=92s already present. = Looking at the device tree we see on one of the platforms that=92s handy = for me to access: > > > > at91_mci0 > > mmc0 > > mmcsd0 at rca=3D0xb368 > > > > So you know that mmcsd0 is on mmc0 bus attached to at91_mci0, which = is ultimately the FDT node where things came from. There=92s not a = user-defined slot associated with this (and we should have a SLOT A vs = SLOT B as a location info for this platform, because we can have two = cards on the one bus in the MMC case), true, but for your use case I = don=92t think that you need it. We should be attaching the host = controller regardless of whether the or not there=92s a card in there, = so it will be fixed. While some additional information would be useful = to publish, I don=92t think your use case requires it=85 > > > > > > The reason you need something extra here is that what is there now = breaks down whenever you don't have a one-to-one mapping between = controllers and buses. Any controller with more than one slot can yield = something of the form: > > > > sdhci0 > > mmc0 > > mmcsd0 at rca=3D0xabcd > > mmc1 > > mmcsd1 at rca=3D0x1234 > > > > and you have no idea what physical slot in the system mmcsd0 and = mmcsd1 are in. >=20 > The driver isn=92t going to be able to help you, because it reports = mmc0 based on the data it gets from slot 0 status registers, and mmc1 = based on slot 1 status registers. Since there=92s no notion of how that = maps to physical hardware, the driver can=92t do anything automatically. = And since mmc on down is populated by FreeSBD, there=92s no hints in the = FDT tree for them. >=20 >=20 > > For my immediate use case, sure, I can rely on the one-to-one = relationship between controllers and buses. At this point, though, = rather than skate by on that happy coincidence, I'd rather invest what = now seems to be a rather small amount of effort adding mmcsd devctl = notifications that would cover the multiple-slots-per-controller case as = well, and then build the rest of what I want on top of that information = coming out of ddvd. >=20 > Trouble is, how do we know what to send with this new notification. = That=92s the part I=92m having trouble with. Where does that data come = from? And how is it different than what=92s in the device tree? >=20 >=20 > Each controller driver maintains an instance of struct mmc_host for = each physical bus interface (typically referred to as a 'slot' in the = drivers) it has. When a card is detected in a given slot by the driver, = the driver creates an mmc bus instance and attaches the struct mmc_host = corresponding to that slot to provide the ivar values. So let's say = struct mmc_host gets a new member 'slot_number', and a new ivar = MMC_IVAR_SLOT_NUMBER is defined. The slot number in each instance of = struct mmc_host a driver maintains gets set to a controller-relative = index of the corresponding physical interface - the controller will do = this the same way every time, because it is tied to the register layout = of the controller. >=20 > After the mmc bus instance is created and its ivars are set, = probe-and-attach is run on that bus, and an mmcsd device instance is = created for each card that is found. At the point where the mmcsd = device instance is created, one knows the parent bus for that new mmcsd = device, and thus one can get the value of MMC_IVAR_SLOT_NUMBER for that = bus, as well as the name of the controller device instance that is the = parent of the parent bus. It thus possible at that point to=20 >=20 > devctl_notify("MMC", "DEVICE", "ATTACH", "... = controller=3D = slot=3D ") >=20 > Because the controller attachment order is the same on every boot, as = is the slot number ivar for a given bus interface on each controller = hardware instance, an identical attach message will be generated every = time a card is discovered in that physical location in the system. For = a given system, there will thus be a fixed mapping between = {controller_instance,slot} tuples that appear in these messages and the = physical MMC/SD device locations. I=92m curious how that=92s materially different than the parent=92s mmc = instance number. That too is invariant between boots. There=92s a 1:1 - = onto mapping between this instance number and any controller/slot tuple = that would be created. Also, there doesn=92t need to be a special MMC message for this. If we = do create the notion of a slot number per controller, that would be part = of the location information that is in the location string for the = normal attach message. > In the above, I've left out the value of rca from the discussion = because upon further reflection, it cannot be stably tied to a physical = location. If there is a multidrop MMC bus in a system, the physical = card locations on that bus will not be able to be referred to with = stable names. This is the one area where a new property in the FDT = could be useful to convey multidrop-or-not for each bus interface on a = controller. The new property could be 'freebsd,max-devices' and would = be an array of cells that indicates the maximum number of MMC/SD devices = that can be connected to the controller bus interface corresponding to = that cell index. The devctl notification could then include a multidrop = indicator in the message. rca is more of a serial number than a location number. Useful for other = reasons. I=92m not sure how =91freebsd,max-devices=92 would solve the problem. = The controller already knows that. If we really want to tie things to a = physical location/ description, I=92d opt for something more like = =91freebsd,slot-names =3D =93Slot 0=94, =93Slot 1=94;=92 type of thing, = where you could just as easily have =93Top Card=94 =93bottom card=94 or = =93boot card=94 and =93customer card=94 if you wanted. Then the = controller could query this property to get the names to populate = somewhere in the PNP info for this device. The mmcsd driver would then = be free to also create a /dev/Blah alias as well for the disk, but I = don=92t know if that would cause aliases to get created for all the geom = children or not... > > On the at91 platform cited above, that allows you to connect two MMC = cards to the same bus (i.e., multidrop configuration), is there any way = to distinguish which card is in which physical slot? I'm still under = the impression that this is the one case where we aren't going to be = able to determine the physical location of every mmcsd device. >=20 > Actually, there=92s two different configurations, I believe. One that = supports two SD cards (SLOT A / SLOT B) and one that supports MMC multi = drop. The former has been tested at least once, while the latter I don=92t= think had ever been checked. >=20 > I think MMC multidrop will remain a limitation, and perhaps an = insignificant one (does *anyone* know of a current system that does = this?). I=92ve never heard of anybody trying this and having it fail in the past = 8 or so years since I wrote the SD/MMC stack. At the time I wrote it, it = was already ancient history=85 Warner=