Date: Wed, 5 Mar 2014 21:48:50 -0500 From: Patrick Kelsey <kelsey@ieee.org> To: Warner Losh <imp@bsdimp.com> Cc: "freebsd-hackers@freebsd.org" <freebsd-hackers@freebsd.org>, freebsd-arm <freebsd-arm@freebsd.org>, "freebsd-embedded@freebsd.org" <freebsd-embedded@freebsd.org>, Ian Lepore <ian@freebsd.org> Subject: Re: [PATCH] simplebus child device probe order control via FDT (motivated by BeagleBone Black) Message-ID: <9CEFA586-0319-4390-AC9A-B54EE77AD735@ieee.org> In-Reply-To: <899B9ABD-0ACC-49F2-9520-CCE837D39875@bsdimp.com> References: <CAD44qMUyqzaFtjgXdgThgHcHjPctx-oZAdhvHp4Kf0G7N4HVog@mail.gmail.com> <7C2B7036-51CC-4C97-80C4-0A439874357D@bsdimp.com> <CAD44qMXe8bh0cR60tm8%2BLZ9W3WhJDuGX6xz9FLNHrzmXNd5FDQ@mail.gmail.com> <1393939277.1149.300.camel@revolution.hippie.lan> <CAD44qMWLox0yY1-%2B2bn4dQ=z0jWKow95b=mnCJEkwmSiSipf4g@mail.gmail.com> <438620C4-7712-4B01-A46F-CB57946A30BF@bsdimp.com> <CAD44qMWNGPY70NMkTWoWcMTf1pywhSPGdbJpmbxxSxEdENSOJQ@mail.gmail.com> <16A5203B-B06D-4129-A54F-F34D5FA28D2B@bsdimp.com> <CAD44qMU46nqsw155qqKs7DrxXqawURgaUVEAaetzDExYM2LhYg@mail.gmail.com> <39ED4040-2A6A-4D85-97D5-DCDE4ECCA0EC@bsdimp.com> <CAD44qMU6ksf7Z_QmX853=bRdEQQu-oxTkx9-uAbms0F6E%2Bahgg@mail.gmail.com> <899B9ABD-0ACC-49F2-9520-CCE837D39875@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mar 5, 2014, at 8:02 PM, Warner Losh <imp@bsdimp.com> wrote:
>=20
> On Mar 5, 2014, at 5:53 PM, Patrick Kelsey <kelsey@ieee.org> wrote:
>=20
>>=20
>>=20
>>=20
>> On Wed, Mar 5, 2014 at 5:44 PM, Warner Losh <imp@bsdimp.com> wrote:
>>=20
>> On Mar 5, 2014, at 2:56 PM, Patrick Kelsey <kelsey@ieee.org> wrote:
>>=20
>>>=20
>>>=20
>>>=20
>>> On Wed, Mar 5, 2014 at 4:24 PM, Warner Losh <imp@bsdimp.com> wrote:
>>>=20
>>> On Mar 5, 2014, at 11:55 AM, Patrick Kelsey <kelsey@ieee.org> wrote:
>>>=20
>>>>=20
>>>>=20
>>>>=20
>>>> On Wed, Mar 5, 2014 at 8:31 AM, Warner Losh <imp@bsdimp.com> wrote:
>>>>=20
>>>> On Mar 4, 2014, at 11:53 PM, Patrick Kelsey <kelsey@ieee.org> wrote:
>>>>=20
>>>>>=20
>>>>>=20
>>>>>=20
>>>>> On Tue, Mar 4, 2014 at 8:21 AM, Ian Lepore <ian@freebsd.org> wrote:
>>>>>=20
>>>>> 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 (wheth=
er
>>>>> it's non-removable or a gpio pin).
>>>>>=20
>>>>> That alone doesn't solve the wider problem, though, which I think brea=
ks
>>>>> down into two pieces. Let's say you've got two slots, call them left
>>>>> and right. You end up with these two problems...
>>>>>=20
>>>>> * 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.
>>>>>=20
>>>>> And not just a boot-time issue, of course. If you were to remove thos=
e two cards and then reinsert them in the opposite time-order, their device n=
ames would swap.
>>>>>=20
>>>>> * I want the slot on the left to be named '1' and the right to be=
>>>>> '0'.
>>>>>=20
>>>>> The first problem is easily solved without impacting dts in any way. W=
e
>>>>> just wire down the relationship controllerN -> mmcN -> mmcsdN. This i=
s
>>>>> exactly equivelent to the old ATA_STATIC_ID option in its effect -- yo=
u
>>>>> 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.
>>>>>=20
>>>>> 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 ho=
w
>>>>> easy it would be.
>>>>>=20
>>>>> We should be able to assign a geographic address (controllerX:slotY) t=
o each mmcsd device in a given system (let's ignore for now the theoretical p=
ossibility of multiple cards on one bus). The 'controllerX' part of the add=
ress could be the controller's pathname from the devicetree, or an index ass=
igned by mmcbr machinery at attach time. The "slotY" part of the address is=
assigned by the specific controller device driver using some internally-det=
ermined fixed mapping between the assigned values and its physical slots. T=
his geographic address could be used to create an additional set of /dev ent=
ries with stable names. There could be a mechanism for user-configurable al=
iases for the geographical addresses.
>>>>=20
>>>> There=E2=80=99s already a chance to run a script when a device is attac=
hed that can create /dev/slot0 or /dev/slot1 that has geographical informati=
on available to it. People use ddvd for this in the USB world all the time t=
o make sure that tty devices get a symlink based on their location or serial=
number.
>>>>=20
>>>> Why is mmc so different it needs its own mechanism?
>>>>=20
>>>> I'm laying out my view of the information that would be needed and the t=
ypes of actions that would have to be taken based on that information to sol=
ve the issue. I'm not saying devd can't be the piece that is used to implem=
ent the actions (indeed, I noted devd as a potential building block for a so=
lution in my initial response). I'm also not saying mmc needs its own mecha=
nism, I'm saying it needs /a/ mechanism, and so far there still seems to be s=
omething missing (because it's not there, or I'm still ignorant of it).
>>>>=20
>>>> What seems to be missing is geographical addressing for mmc devices.
>>>>=20
>>>> I think what you might be saying is that the existing mmcsd add/remove c=
ode could be augmented to send devctl notifications, along the lines of:
>>>>=20
>>>> devctl_notify("MMC", "DEVICE", "ATTACH"|"DETACH", "... controller=3D<co=
ntroller_device_name_or_better_yet_devicetree_path> slot=3D<slot_number> rca=
=3D<rca>")
>>>>=20
>>>> and then I and the fine author of devctl and devd would be pleased.
>>>=20
>>> MMC doesn=E2=80=99t need anything special here. That=E2=80=99s already p=
resent. Looking at the device tree we see on one of the platforms that=E2=80=
=99s handy for me to access:
>>>=20
>>> at91_mci0
>>> mmc0
>>> mmcsd0 at rca=3D0xb368
>>>=20
>>> So you know that mmcsd0 is on mmc0 bus attached to at91_mci0, which is u=
ltimately the FDT node where things came from. There=E2=80=99s not a user-de=
fined slot associated with this (and we should have a SLOT A vs SLOT B as a l=
ocation 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=E2=80=99t think that yo=
u need it. We should be attaching the host controller regardless of whether t=
he or not there=E2=80=99s a card in there, so it will be fixed. While some a=
dditional information would be useful to publish, I don=E2=80=99t think your=
use case requires it=E2=80=A6
>>>=20
>>>=20
>>> The reason you need something extra here is that what is there now break=
s down whenever you don't have a one-to-one mapping between controllers and b=
uses. Any controller with more than one slot can yield something of the for=
m:
>>>=20
>>> sdhci0
>>> mmc0
>>> mmcsd0 at rca=3D0xabcd
>>> mmc1
>>> mmcsd1 at rca=3D0x1234
>>>=20
>>> and you have no idea what physical slot in the system mmcsd0 and mmcsd1 a=
re in.
>>=20
>> The driver isn=E2=80=99t 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=E2=80=99s no notion of how that map=
s to physical hardware, the driver can=E2=80=99t do anything automatically. A=
nd since mmc on down is populated by FreeSBD, there=E2=80=99s no hints in th=
e FDT tree for them.
>>=20
>>=20
>>> For my immediate use case, sure, I can rely on the one-to-one relationsh=
ip between controllers and buses. At this point, though, rather than skate b=
y 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 t=
he multiple-slots-per-controller case as well, and then build the rest of wh=
at 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=E2=
=80=99s the part I=E2=80=99m having trouble with. Where does that data come f=
rom? And how is it different than what=E2=80=99s in the device tree?
>>=20
>>=20
>> Each controller driver maintains an instance of struct mmc_host for each p=
hysical 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 cre=
ates an mmc bus instance and attaches the struct mmc_host corresponding to t=
hat slot to provide the ivar values. So let's say struct mmc_host gets a ne=
w 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 - th=
e 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-at=
tach is run on that bus, and an mmcsd device instance is created for each ca=
rd that is found. At the point where the mmcsd device instance is created, o=
ne knows the parent bus for that new mmcsd device, and thus one can get the v=
alue of MMC_IVAR_SLOT_NUMBER for that bus, as well as the name of the contro=
ller 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<controller_in=
stance_name> slot=3D<value-of-MMC_IVAR_SLOT_NUMBER> ")
>>=20
>> Because the controller attachment order is the same on every boot, as is t=
he slot number ivar for a given bus interface on each controller hardware in=
stance, an identical attach message will be generated every time a card is d=
iscovered in that physical location in the system. For a given system, ther=
e will thus be a fixed mapping between {controller_instance,slot} tuples tha=
t appear in these messages and the physical MMC/SD device locations.
>=20
> I=E2=80=99m curious how that=E2=80=99s materially different than the paren=
t=E2=80=99s mmc instance number. That too is invariant between boots. There=E2=
=80=99s a 1:1 - onto mapping between this instance number and any controller=
/slot tuple that would be created.
>=20
Controller instance (unit) numbers are the same across boots. The mmc insta=
nce (unit) number for the mmc instance created for a given bus interface on a=
given controller is not, because the instance is created dynamically in res=
ponse to card detection and thus depends on the ordering of card insertions.=
Sure, there's a one-to-one and onto mapping at any given moment between mm=
c instance numbers and the tuples I'm talking about, but the mapping is subj=
ect to change with card insertions and removals. The material difference be=
tween the two sets of labels is that a given tuple value will *always* refer=
to the same physical device location in the system, whereas a given mmc uni=
t number could refer to any physical device location in the system depending=
on the time order of insertions in the various card slots.
> Also, there doesn=E2=80=99t need to be a special MMC message for this. If w=
e do create the notion of a slot number per controller, that would be part o=
f the location information that is in the location string for the normal att=
ach message
Ah, so I can just append more variable definitions to the location string, w=
hich is already fed through to the existing generic devctl notification? Wo=
rks for me.
>> In the above, I've left out the value of rca from the discussion because u=
pon 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 th=
at bus will not be able to be referred to with stable names. This is the on=
e area where a new property in the FDT could be useful to convey multidrop-o=
r-not for each bus interface on a controller. The new property could be 'fr=
eebsd,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 interf=
ace corresponding to that cell index. The devctl notification could then in=
clude a multidrop indicator in the message.
>=20
> rca is more of a serial number than a location number. Useful for other re=
asons.
>=20
> I=E2=80=99m not sure how =E2=80=98freebsd,max-devices=E2=80=99 would solve=
the problem. The controller already knows that. If we really want to tie th=
ings to a physical location/ description, I=E2=80=99d opt for something more=
like =E2=80=98freebsd,slot-names =3D =E2=80=9CSlot 0=E2=80=9D, =E2=80=9CSlo=
t 1=E2=80=9D;=E2=80=99 type of thing, where you could just as easily have =E2=
=80=9CTop Card=E2=80=9D =E2=80=9Cbottom card=E2=80=9D or =E2=80=9Cboot card=E2=
=80=9D and =E2=80=9Ccustomer card=E2=80=9D if you wanted. Then the controlle=
r could query this property to get the names to populate somewhere in the PN=
P 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=E2=80=99t know if that woul=
d cause aliases to get created for all the geom children or not...
>=20
freebsd,max-devices is not already known by the controller. The controller k=
nows how many bus interfaces it has, but it doesn't know how many devices ca=
n be attached to that bus, as that depends on the board design. freebsd,max=
-devices informs us whether the board design is multidrop or not for each bu=
s interface on each controller. Passing through a multidrop indication in t=
he devctl notification informs the devctl consumer as to whether or not a un=
ique stable name can be assigned to the mmcsd instance based on the tuple (i=
f multidrop, then no). Not essential, but would be thorough.
I disagree with 'freebsd,slot-names' because meaningful/descriptive slot nam=
es are going to be something that are often defined at the product level, so=
I think it's better to just let them be defined via a devd action script. O=
therwise we build in an invitation to the experience of having board-level s=
lot names and product-level slot names from the same namespace, which is in m=
y experience awful. "Oh, wait, does this bug report refer to board-'top slo=
t' or front-panel-'top slot'?" In other words, I think it's handy to have u=
p until the board goes into an enclosure, then it could be trouble from then=
on. Plus it could also encourage further knee-jerk, inappropriate .dts pat=
ching of the sort I started out with here :) "Why make a devd script when I c=
an just edit the names in the .dts file?"
Agreed geom children are an open question.
-Patrick=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?9CEFA586-0319-4390-AC9A-B54EE77AD735>