tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Overhaul of I2C and SPI device autoconfiguration



For a few months now, I've been working on improving the auto configuration of I2C and SPI devices.  These busses, of course, are not self-enumerating, and besides indirect configuration of devices listed in a kernel config file, enumerating based on information in platform config data (e.g. Devicetree, OpenFirmware, ACPI) was supported by having controller drivers explicitly call OpenFirmrware, FDT, or ACPI-specific code to populate a dictionary.  Yuck.

My new mechanism instead uses the platform config data to enumerate the devices directly, without having to get a dictionary involved at all.  It also provides a tidier mechanism to pass device handles to each driver via the normal mechanism, rather than using the i2c_attch_args::ia_cookie / i2c_attch_args::ia_cookietype hack.

The basic idea involves the device call mechanism I introduced a while ago.  The various back-ends (OpenFirmware, ACPI, etc.) that have I2C and SPI bus bindings define "i2c-enumerate-devices" and "spi-enumerate-devices" device calls.  Those calls in turn invoke a callback to the I2C or SPI code to actually attach the device specified in the platform config data.  There is no duplication of information in a dictionary, and the device handles for the child devices naturally flow from the enumeration.

Systems that lack a platform device tree (e.g. sandpoint) or that have missing device tree nodes on some models (e.g. sparc64, macppc) have the option of using static tables, and there are helper functions in the core I2C code to facilitate this.  Such platforms merely need to subclass the controller's devhandle and override that handle's "i2c-enumerate-devices" call.  This also covers the case where a platform that uses OpenFirmware has a controller whose child nodes, for whatever weird reasons, don't conform to the standard bindings (such is the case with the "ki2c" controller on macppc).

While the changes are very straightforward, the diffs are large, so I've broken them up into chunks that are easier to digest and describe.  Sometimes the diffs might not make a lot of sense because of a larger restructuring, so in those cases, I encourage you to check out the thorpej-i2c-spi-conf2 branch and simply read the file directly.

I'll start with the core I2C changes:

	https://www.netbsd.org/~thorpej/thorpej-i2c-spi-conf2/i2c-core-diffs.txt

The main changes here iic_attach().  The loop enumerating a prop_array_t of child device descriptions is gone, and has been replaced by the following small code block:

	struct i2c_enumerate_devices_args enumargs = {
		.ia = &ia,
		.callback = iic_enumerate_devices_callback,
	};
	rv = device_call(self, "i2c-enumerate-devices", &enumargs);

The iic_enumerate_devices_callback() is quite small -- it essentially just calls config_found() and performs some housekeeping.

You'll also see some other helpers there, like i2c_enumerate_deventries(), which helps to support the "static tables of devices" scenario I mentioned above.

In addition to the enumeration change, I've changed the code to not use a flat array of device_t pointers indexed by i2c address.  That array was consuming 4K or 8K of memory (depending on pointer size) for what is typically very sparsely populated -- what a waste!  It now uses a linked list of device entries, sorted by i2c address, and the manipulation of that list is MP-safe.

Also included in these diffs is a helper function in kern/subr_device.c that does the work of subclassing a device handle in the common case (used by macppc and sparc64; there is one additional case in macppc that's more complicated, so that location is open-coded -- more on that later).

Next, let's take a look at the changes to the platform config data back-ends:

	https://www.netbsd.org/~thorpej/thorpej-i2c-spi-conf2/i2c-acpi-diffs.txt
	https://www.netbsd.org/~thorpej/thorpej-i2c-spi-conf2/i2c-ofw-diffs.txt

Now, rather than allocating a populating a dictionary with platform config data, that data is used to directly enumerate and attach child devices.

In the OpenFirmware case, the old code that did this has been tossed completely, and the differences between OpenFirmware variants (including the standard Devicetree bindings) is handled in a much tidier way.  Also, because there is no more of_enter_i2c_devs() function, there is no more need for a fdtbus_attach_i2cbus() (FDT-based attachments just attach the "iicbus" instance like any other attachment would); note there is still a need for fdtbus_register_i2c_controller(), but that is not anything that's I2C or SPI specific in the FDT universe, and I'm not intending to address that issue now.

All of these changes made it necessary to adjust some (but certainly not all) I2C device drivers, mainly things that needed to make use of i2c_attch_args::ia_cookie / i2c_attch_args::ia_cookietype.  Those diffs are here:

	https://www.netbsd.org/~thorpej/thorpej-i2c-spi-conf2/i2c-mux-diffs.txt
	https://www.netbsd.org/~thorpej/thorpej-i2c-spi-conf2/i2c-device-diffs.txt

The MUX changes are a little larger, and spread across multiple files, so I carved them out separately.

Now let's take a look at the simple controller diffs:

	https://www.netbsd.org/~thorpej/thorpej-i2c-spi-conf2/i2c-controller-diffs.txt

As you can see, the main changes here are to eliminate FDT- and ACPI-specific code, making the two cases identical.  I did not do the more invasive re-factoring of these to fully de-duplicate the code that's now the same, but that is a fairly straight-forward exercise.  Additionally, a bunch of code that existed to populate the child device dictionary has been garbage-collected from a couple of machine-dependent I2C controller drivers (those instances can use the standard OpenFirmware bindings).

Controllers that have built-in MUXes (the kind that the Devicetree I2C bindings call an "i2c-mux-reg"), including "ki2c" and "smu" on macppc and "pfciic" on sparc64, now independently attach the two channels.  I may at some point in the future revisit how these controllers work.  Unfortunately, for the uses in NetBSD, the device trees on these platforms fall squarely into the "legacy" category and they don't really represent the topology in standardized ways (sparc64, for example, encodes the MUX bus number in the upper 32-bits of a 64-bit address cell, even on i2c controllers that don't have a MUX, and the macppc case is of course weirder than that).  In any case, general support for "i2c-mux-reg" MUXes is not included here, but the core I2C code can now at least express the MUX bus number in these cases along with the more standardized I2C MUX cases.

Another change I made was to remove the needless indirection of "smuiic" on macppc; the "iicbus" instances now attach directly to "smu".

And now for the more complicated cases ... the ones that need to override the standard "i2c-enumerate-devices" call.

	https://www.netbsd.org/~thorpej/thorpej-i2c-spi-conf2/i2c-override-diffs.txt

There are 4 instances of this: 2 controllers on macppc ("cuda" and "ki2c"), the sandpoint platform, and sparc64 (specific machine models, not specific to any specific controller type).

First, "cuda".  This one is pretty basic.  There are no child nodes in the device tree.  We populate a table and call i2c_enumerate_deventries() from the "i2c-enumerate-devices" call.

Second, "ki2c".  This one is a bit wackier.  In this case, all of the child nodes are there, but the device tree doesn't match the OpenFirmware bindings like *at all*.  Complicating this one is the fact that there are two schemes found on "ki2c", depending on the OpenFirmware version.  The comment I added explains it best:

+	/*
+	 * Different systems have different I2C device tree topologies.
+	 *
+	 * Some systems use a scheme like this:
+	 *
+	 *   /u3@0,f8000000/i2c@f8001000/temp-monitor@98
+	 *   /u3@0,f8000000/i2c@f8001000/fan@15e
+	 *
+	 * Here, we see the channel encoded in bit #8 of the address.
+	 *
+	 * Other systems use a scheme like this:
+	 *
+	 *   /ht@0,f2000000/pci@4000,0,0/mac-io@7/i2c@18000/i2c-bus@0
+	 *   /ht@0,f2000000/pci@4000,0,0/mac-io@7/i2c@18000/i2c-bus@0/codec@8c
+	 *
+	 *   /u4@0,f8000000/i2c@f8001000/i2c-bus@1
+	 *   /u4@0,f8000000/i2c@f8001000/i2c-bus@1/temp-monitor@94
+	 *
+	 * Here, a separate device tree node represents the channel.
+	 * Note that in BOTH cases, the I2C address of the devices are
+	 * shifted left by 1 (as it would be on the wire to leave room
+	 * for the read/write bit).
+	 *
+	 * So, what we're going to do here is look for i2c-bus nodes.  If
+	 * we find them, we remember those phandles, and will use them for
+	 * device enumeration.  If we don't, then we will use the controller
+	 * phandle for device enumeration and filter based on the channel
+	 * bit in the "reg" property.
+	 */

Note the diff for "ki2c" is misleadingly large, because I cleaned up the code a little while I was there, plus there's the MUX changes I mentioned earlier.

Third is the sandpoint platform.  We need to select the right static list of devices depending on the specific NAS model we're running on.  This is an example use of the DEVHANDLE_TYPE_PRIVATE devhandles; the sandpoint platform does not use FDT or any other type of platform config data scheme, and so we're not subclassing here, but rather creating it from whole cloth.  Anyway, once the table is selected, i2c_enumerate_deventries() actually performs the enumeration.

The final case is sparc64.  This is a case where the OpenFirmware device tree is simply missing a bunch of child nodes.  This is also tangled up with some other "add properties for GPIO pins" stuff that jdc@ added to the port a while ago.  I kept his GPIO property fixup stuff, but completely replaced the previous I2C fixup code he had with the new scheme.  And rather than being handled inline in device_register(), we call out to a table-driven mechanism for matching the platforms and the specific OpenFirmware device tree nodes that need to be fixed up.  We don't rely on fixing up any specific controller driver the way we do on macppc.  Also note that not ALL of the device tree nodes are missing!  For example, on the V210, the I2C child nodes for the GPIO controllers are present, but the nodes for the environmental sensors are not.  So, the in this case the subclass's "i2c-enumerate-devices" call first calls the superclass's "i2c-enumerate-devices" (to enumerate the nodes present in OpenFirmware) and then calls i2c_enumerate_deventries() to enumerate the model-specific table of additions.

OK!  That's I2C!  Now onto SPI.

The general gist of the SPI changes is basically the same, and it suffered from the same basic ugly bits as the I2C code did.  Here are the core SPI changes:

	https://www.netbsd.org/~thorpej/thorpej-i2c-spi-conf2/spi-core-diffs.txt

Because we don't support wildcarding of the chip select locator on SPI, direct and indirect config using this scheme are nearly identical... in the direct config case, we're enumerating the platform config data (only Devicetree bindings for now) using the "spi-enumerate-devices" device call or enumerating kernel configuration directives using config_search().  In any case, I took the opportunity to tidy up some of the bookkeeping on how child devices are tracked.

I only provided an OpenFirmware / Devicetree "spi-enumerate-devices" implementation.  As with I2C, there is no longer a need for fdtbus_attach_spibus().  I also tweaked fdtbus_register_spi_controller() to remove a pointless layer of indirection, as was done for I2C a while ago.

	https://www.netbsd.org/~thorpej/thorpej-i2c-spi-conf2/spi-ofw-diffs.txt

The controller diffs are pretty straightforward. I standardized the style of the config_found() call sites and made sure to get the controller's devhandle passed down to the "spi" bus instance.

	https://www.netbsd.org/~thorpej/thorpej-i2c-spi-conf2/spi-controller-diffs.txt

Now, the device diffs:

	https://www.netbsd.org/~thorpej/thorpej-i2c-spi-conf2/spi-device-diffs.txt

There's kind of a lot going on here, but before I dive into each one, there are a couple of changes I made to basically all of these drivers:

- Standardize the form of the "match" function.
- Ensure "match" does not have side-effects (e.g. setting the mode/speed of the that "slot").
- Fleshed out the compat_data[], and ensured that all of them support direct configuration using compat data.

And now some specific additional items:

mcp23s17.c -- Added direct configuration support, which required adding additional code to parse the device tree properties so as to be able to de-mux the chip select (TL;DR - go read the chip's data sheet and the Devicetree bindings for this device).

mcp3k.c -- Added direct configuration support, including selection of parameters based on the "compatible" property from the device tree.

mcp48x1.c -- Same.

ssdfb_spi.c -- Tidied up the FDT integration a bit.

tmp121.c -- Added place-holders for other versions of the chip that we should support eventually.


Anyway, if you've read this far, congratulations!  Now, I need your help.  I don't have all of these machines.  I've done a fair bit of testing on evbarm (FDT) and macppc, but not every model.  I need help specifically with:

-- evbarm (ACPI), specifically a system with an I2C MUX (I think jmcneill@ has such a beast).

-- macppc (variety of models) -- macallan@?

-- sandpoint (any will do; I bought a Synology, but the Ethernet interface failed after a couple of days, even PPCBOOT complains, boo).  rin@?

-- sparc64 -- I don't think Qemu can emulate any of the I2C controllers that system has.  jdc@?

-- SPI devices -- tnn@?

I have some additional changes teed up for "ihidev", but I first need to finish writing some Arduino code to simulate a I2C HID mouse so I can test it, so I'm not including any of that bigger change here.

Anyway, comments please!

-- thorpej



Home | Main Index | Thread Index | Old Index