Folks,
we still have to solve the problem of "scanning" i2c busses, especially
on machines where no scan is needed since the firmware happily tells
us everything we might want to know.
In the past (as far as I remember) two proposal where presented and
both shot down. Now I have a machine where I realy needed the i2c
based fan controll to make the noise bearble - so I put on my asbestos
suit and wrote another implementation, which I'd like to present here.
The previous proposals:
- use the OpenBSD way: an optional "scan" callback provided by the
i2c controller driver. Downsides: needs changes ~every i2c controller
driver in-tree
- the macppc way: see macppc/dev/kiic* - basically a slightly different
bus, needs frontend/backend split of i2c device drivers and a lot
of additional frontends to be written
I may misrember details and critics raised against one of those.
Goals I tried to achieve:
- Allow both direct and indirect config at the i2c bus layer,
depending on availability of firmware provided locators
- Allow unmodified i2c device drivers to continue working
- Keep MD changes as simple and small as possible
- No changes to MI i2c bus controllers
- Allow MD i2c bus controllers to easily override the generic
behaviour (i.e.: provide additional locators or modify firmware
provided ones)
Seems like it worked out, and the changes are pretty small in the end.
Quick overview how it works:
- If we are doing direct config, MD code (via generic support routines,
or by overriding those) adds a prop_array to the device properties
of the i2c bus controller (the parent(!) of the i2c bus). This array
contains a dictionary for each i2c device on the bus. Entries in
this dictionary are:
"name" -> string, device name
"address" -> uint32, i2c address
"size" -> uint32 (optional)
"compatibility" -> a list of names, i.e. the chip used, used for
matching a hardware driver (think: alternative "name" props)
- When the i2c bus attaches, it queries the device properties of it's
parent device and checks the "i2c-child-devices" property (the array
described above), and if it is present, iterates over the array
creating i2c_attach_arg from it. To allow direct config matches,
the i2c_attach_args structure has been extended.
If the device property is not available, indirect config is done.
- An i2c device driver for a proper device will need no changes, but
for i.e. write-only devices matching based on strings can be added.
A generic helper function to match the "compatible" string list against
a driver specific list is provided.
A few more details:
Let's start with setting up the device property. In the attached sparc64 MD
code the setup is done inside device_register() whenever a "iic" device
attaches and there is not yet a "i2c-child-devices" property at the parent.
This check is needed to allow MD i2c controller drivers to override
the generic behaviour. For OpenFirmware based machines, a convenience
funtction "of_enter_i2c_devs()" to do the device property setup is provided.
Next step: the i2c bus attaches and checks for the device property. This is
all done in the i2c bus code, no i2c controller driver needs modifications.
Last part of the puzzle: the i2c device drivers can check for the (new)
ia_name pointer in the i2c_attach_args structure to find out if direct
config is available. For example the spdmem driver does a (nasty/stupid?)
check for certain address values - which does not make any sense in the
direct config case:
@@ -164,8 +165,17 @@ spdmem_match(device_t parent, cfdata_t m
int spd_len, spd_crc_cover;
uint16_t crc_calc, crc_spd;
- if ((ia->ia_addr & SPDMEM_ADDRMASK) != SPDMEM_ADDR)
- return 0;
+ if (ia->ia_name) {
+ /* add other names as we find more firmware variations */
+ if (strcmp(ia->ia_name, "dimm-spd"))
+ return 0;
+ }
+
+ /* only do this lame test when not using direct config */
+ if (ia->ia_name == NULL) {
+ if ((ia->ia_addr & SPDMEM_ADDRMASK) != SPDMEM_ADDR)
+ return 0;
+ }
sc.sc_tag = ia->ia_tag;
sc.sc_addr = ia->ia_addr;
If the firmware name is not a good indicator of the driver to use, the
"compatible" list can be used, via the generic iic_compat_match()
function.
I have tested this on a few sparc64 machines, it works for me.
I won't mind if we decide to not used this but go with one of the older
proposals instead - but we need to move on.
Comments?
Martin
Index: dev/ofw/openfirm.h
===================================================================
RCS file: /cvsroot/src/sys/dev/ofw/openfirm.h,v
retrieving revision 1.27
diff -c -u -p -r1.27 openfirm.h
--- dev/ofw/openfirm.h 11 Nov 2009 16:56:52 -0000 1.27
+++ dev/ofw/openfirm.h 3 Feb 2010 19:46:14 -0000
@@ -115,4 +115,6 @@ boolean_t of_to_dataprop(prop_dictionary
int *of_network_decode_media(int, int *, int *);
char *of_get_mode_string(char *, int);
+void of_enter_i2c_devs(prop_dictionary_t, int);
+
#endif /*_OPENFIRM_H_*/
Index: dev/ofw/ofw_subr.c
===================================================================
RCS file: /cvsroot/src/sys/dev/ofw/ofw_subr.c,v
retrieving revision 1.16
diff -c -u -p -r1.16 ofw_subr.c
--- dev/ofw/ofw_subr.c 21 Jan 2010 15:56:08 -0000 1.16
+++ dev/ofw/ofw_subr.c 3 Feb 2010 19:46:14 -0000
@@ -325,3 +325,59 @@ of_get_mode_string(char *buffer, int len
strncpy(buffer, pos + 2, len);
return buffer;
}
+
+/*
+ * Iterate over the subtree of a i2c controller node.
+ * Add all sub-devices into an array as part of the controller's
+ * device properties.
+ * This is used by the i2c bus attach code to do direct configuration.
+ */
+void
+of_enter_i2c_devs(prop_dictionary_t props, int ofnode)
+{
+ int node, len;
+ char name[32];
+ uint64_t r64;
+ uint64_t r32;
+ uint8_t smr[24];
+ prop_array_t array;
+ prop_dictionary_t dev;
+
+ array = prop_array_create();
+
+ for (node = OF_child(ofnode); node; node = OF_peer(node)) {
+ if (OF_getprop(node, "name", name, sizeof(name)) <= 0)
+ continue;
+ len = OF_getproplen(node, "reg");
+ if (len == sizeof(r64)) {
+ if (OF_getprop(node, "reg", &r64, sizeof(r64))
+ != sizeof(r64))
+ continue;
+ r32 = r64;
+ } else if (len == sizeof(r32)) {
+ if (OF_getprop(node, "reg", &r32, sizeof(r32))
+ != sizeof(r32))
+ continue;
+ } else if (len == 24) {
+ if (OF_getprop(node, "reg", smr, sizeof(smr))
+ != sizeof(smr))
+ continue;
+ /* smbus reg property */
+ r32 = smr[7];
+ } else {
+ panic("unexpected \"reg\" size %d for \"%s\", "
+ "parent %x, node %x",
+ len, name, ofnode, node);
+ }
+
+ dev = prop_dictionary_create();
+ prop_dictionary_set_cstring(dev, "name", name);
+ prop_dictionary_set_uint32(dev, "addr", r32 >> 1);
+ of_to_dataprop(dev, node, "compatible", "compatible");
+ prop_array_add(array, dev);
+ prop_object_release(dev);
+ }
+
+ prop_dictionary_set(props, "i2c-child-devices", array);
+ prop_object_release(array);
+}
Index: dev/i2c/i2cvar.h
===================================================================
RCS file: /cvsroot/src/sys/dev/i2c/i2cvar.h,v
retrieving revision 1.6
diff -c -u -p -r1.6 i2cvar.h
--- dev/i2c/i2cvar.h 9 Jul 2007 21:00:33 -0000 1.6
+++ dev/i2c/i2cvar.h 3 Feb 2010 19:46:14 -0000
@@ -118,12 +118,18 @@ struct i2c_attach_args {
i2c_addr_t ia_addr; /* address of device */
int ia_size; /* size (for EEPROMs) */
int ia_type; /* bus type */
+ /* only set if using direct config */
+ const char * ia_name; /* name of the device */
+ int ia_ncompat; /* number of pointers in the
+ ia_compat array */
+ const char ** ia_compat; /* chip names */
};
/*
* API presented to i2c controllers.
*/
int iicbus_print(void *, const char *);
+int iic_compat_match(struct i2c_attach_args*, const char **);
#ifdef _I2C_PRIVATE
/*
Index: dev/i2c/i2c.c
===================================================================
RCS file: /cvsroot/src/sys/dev/i2c/i2c.c,v
retrieving revision 1.23
diff -c -u -p -r1.23 i2c.c
--- dev/i2c/i2c.c 3 Feb 2009 16:41:31 -0000 1.23
+++ dev/i2c/i2c.c 3 Feb 2010 19:46:14 -0000
@@ -59,6 +59,8 @@ struct iic_softc {
};
static void iic_smbus_intr_thread(void *);
+static void iic_fill_compat(struct i2c_attach_args*, const char*,
+ size_t, char **);
int
iicbus_print(void *aux, const char *pnp)
@@ -71,6 +73,20 @@ iicbus_print(void *aux, const char *pnp)
}
static int
+iic_print_direct(void *aux, const char *pnp)
+{
+ struct i2c_attach_args *ia = aux;
+
+ if (pnp != NULL)
+ aprint_normal("%s at %s addr 0x%02x", ia->ia_name, pnp,
+ ia->ia_addr);
+ else
+ aprint_normal(" addr 0x%02x", ia->ia_addr);
+
+ return UNCONF;
+}
+
+static int
iic_print(void *aux, const char *pnp)
{
struct i2c_attach_args *ia = aux;
@@ -110,6 +126,8 @@ iic_attach(device_t parent, device_t sel
{
struct iic_softc *sc = device_private(self);
struct i2cbus_attach_args *iba = aux;
+ prop_array_t child_devices;
+ char *buf;
i2c_tag_t ic;
int rv;
@@ -196,11 +214,61 @@ iic_attach(device_t parent, device_t sel
if (!pmf_device_register(self, NULL, NULL))
aprint_error_dev(self, "couldn't establish power handler\n");
- /*
- * Attach all i2c devices described in the kernel
- * configuration file.
- */
- config_search_ia(iic_search, self, "iic", NULL);
+ child_devices = prop_dictionary_get(device_properties(parent),
+ "i2c-child-devices");
+ if (child_devices) {
+ unsigned int i, count;
+ prop_dictionary_t dev;
+ prop_data_t cdata;
+ uint32_t addr, size;
+ const char *name;
+ struct i2c_attach_args ia;
+ int loc[2];
+
+ memset(loc, 0, sizeof loc);
+ count = prop_array_count(child_devices);
+ for (i = 0; i < count; i++) {
+ dev = prop_array_get(child_devices, i);
+ if (!dev) continue;
+ if (!prop_dictionary_get_cstring_nocopy(
+ dev, "name", &name))
+ continue;
+ if (!prop_dictionary_get_uint32(dev, "addr", &addr))
+ continue;
+ loc[0] = addr;
+ if (prop_dictionary_get_uint32(dev, "size", &size))
+ loc[1] = size;
+ else
+ loc[1] = -1;
+
+ memset(&ia, 0, sizeof ia);
+ ia.ia_addr = addr;
+ ia.ia_type = sc->sc_type;
+ ia.ia_tag = ic;
+ ia.ia_name = name;
+
+ buf = NULL;
+ cdata = prop_dictionary_get(dev, "compatible");
+ if (cdata)
+ iic_fill_compat(&ia,
+ prop_data_data_nocopy(cdata),
+ prop_data_size(cdata), &buf);
+
+ config_found_sm_loc(self, "iic", loc, &ia,
+ iic_print_direct, NULL);
+
+ if (ia.ia_compat)
+ free(ia.ia_compat, M_TEMP);
+ if (buf)
+ free(buf, M_TEMP);
+ }
+ } else {
+ /*
+ * Attach all i2c devices described in the kernel
+ * configuration file.
+ */
+ config_search_ia(iic_search, self, "iic", NULL);
+ }
}
static void
@@ -303,5 +371,55 @@ iic_smbus_intr(i2c_tag_t ic)
return 1;
}
+static void
+iic_fill_compat(struct i2c_attach_args *ia, const char *compat, size_t len,
+ char **buffer)
+{
+ int count, i;
+ const char *c, *start, **ptr;
+
+ *buffer = NULL;
+ for (i = count = 0, c = compat; i < len; i++, c++)
+ if (*c == 0)
+ count++;
+ count += 2;
+ ptr = malloc(sizeof(char*)*count, M_TEMP, M_WAITOK);
+ if (!ptr) return;
+
+ for (i = count = 0, start = c = compat; i < len; i++, c++) {
+ if (*c == 0) {
+ ptr[count++] = start;
+ start = c+1;
+ }
+ }
+ if (start < compat+len) {
+ /* last string not 0 terminated */
+ size_t l = c-start;
+ *buffer = malloc(l+1, M_TEMP, M_WAITOK);
+ memcpy(*buffer, start, l);
+ (*buffer)[l] = 0;
+ ptr[count++] = *buffer;
+ }
+ ptr[count] = NULL;
+
+ ia->ia_compat = ptr;
+ ia->ia_ncompat = count;
+}
+
+int
+iic_compat_match(struct i2c_attach_args *ia, const char ** compats)
+{
+ int i;
+
+ for (; compats && *compats; compats++) {
+ for (i = 0; i < ia->ia_ncompat; i++) {
+ if (strcmp(*compats, ia->ia_compat[i]) == 0)
+ return 1;
+ }
+ }
+ return 0;
+}
+
+
CFATTACH_DECL_NEW(iic, sizeof(struct iic_softc),
iic_match, iic_attach, NULL, NULL);
Index: arch/sparc64/sparc64/autoconf.c
===================================================================
RCS file: /cvsroot/src/sys/arch/sparc64/sparc64/autoconf.c,v
retrieving revision 1.166
diff -c -u -p -r1.166 autoconf.c
--- arch/sparc64/sparc64/autoconf.c 21 Jan 2010 15:58:32 -0000 1.166
+++ arch/sparc64/sparc64/autoconf.c 3 Feb 2010 19:43:49 -0000
@@ -94,6 +94,7 @@ __KERNEL_RCSID(0, "$NetBSD: autoconf.c,v
#include <dev/ata/atavar.h>
#include <dev/pci/pcivar.h>
+#include <dev/ebus/ebusvar.h>
#include <dev/sbus/sbusvar.h>
#ifdef DDB
@@ -867,6 +868,10 @@ device_register(struct device *dev, void
struct sbus_attach_args *sa = aux;
ofnode = sa->sa_node;
+ } else if (device_is_a(busdev, "ebus")) {
+ struct ebus_attach_args *ea = aux;
+
+ ofnode = ea->ea_node;
} else if (device_is_a(dev, "sd") || device_is_a(dev, "cd")) {
struct scsipibus_attach_args *sa = aux;
struct scsipi_periph *periph = sa->sa_periph;
@@ -974,6 +979,21 @@ noether:
}
}
+ /*
+ * Check for I2C busses and add data for their direct configuration.
+ */
+ if (device_is_a(dev, "iic")) {
+ int busnode = device_ofnode(busdev);
+
+ if (busnode) {
+ prop_dictionary_t props = device_properties(busdev);
+ prop_object_t cfg = prop_dictionary_get(props,
+ "i2c-child-devices");
+ if (!cfg)
+ of_enter_i2c_devs(props, busnode);
+ }
+ }
+
/* set properties for PCI framebuffers */
if (device_is_a(busdev, "pci")) {
/* see if this is going to be console */
Index: dev/i2c/spdmem.c
===================================================================
RCS file: /cvsroot/src/sys/dev/i2c/spdmem.c,v
retrieving revision 1.15
diff -c -u -p -r1.15 spdmem.c
--- dev/i2c/spdmem.c 9 May 2009 17:32:27 -0000 1.15
+++ dev/i2c/spdmem.c 3 Feb 2010 19:45:10 -0000
@@ -154,6 +154,7 @@ static uint16_t spdcrc16 (struct spdmem_
}
return (crc & 0xFFFF);
}
+
static int
spdmem_match(device_t parent, cfdata_t match, void *aux)
{
@@ -164,8 +165,17 @@ spdmem_match(device_t parent, cfdata_t m
int spd_len, spd_crc_cover;
uint16_t crc_calc, crc_spd;
- if ((ia->ia_addr & SPDMEM_ADDRMASK) != SPDMEM_ADDR)
- return 0;
+ if (ia->ia_name) {
+ /* add other names as we find more firmware variations */
+ if (strcmp(ia->ia_name, "dimm-spd"))
+ return 0;
+ }
+
+ /* only do this lame test when not using direct config */
+ if (ia->ia_name == NULL) {
+ if ((ia->ia_addr & SPDMEM_ADDRMASK) != SPDMEM_ADDR)
+ return 0;
+ }
sc.sc_tag = ia->ia_tag;
sc.sc_addr = ia->ia_addr;
Index: dev/i2c/adm1021.c
===================================================================
RCS file: /cvsroot/src/sys/dev/i2c/adm1021.c,v
retrieving revision 1.3
diff -c -u -p -r1.3 adm1021.c
--- dev/i2c/adm1021.c 5 Jun 2009 12:42:43 -0000 1.3
+++ dev/i2c/adm1021.c 3 Feb 2010 19:45:10 -0000
@@ -62,18 +62,35 @@ void admtemp_refresh(struct sysmon_envsy
CFATTACH_DECL_NEW(admtemp, sizeof(struct admtemp_softc),
admtemp_match, admtemp_attach, NULL, NULL);
+static const char * admtemp_compats[] = {
+ "i2c-max1617",
+ NULL
+};
int
admtemp_match(device_t parent, cfdata_t match, void *aux)
{
struct i2c_attach_args *ia = aux;
- if (((ia->ia_addr >= 0x18) && (ia->ia_addr <= 0x1a)) ||
- ((ia->ia_addr >= 0x29) && (ia->ia_addr <= 0x2b)) ||
- ((ia->ia_addr >= 0x4c) && (ia->ia_addr <= 0x4e)))
- return (1);
+ if (ia->ia_name == NULL) {
+ /*
+ * Indirect config - not much we can do!
+ * Check typical addresses.
+ */
+ if (((ia->ia_addr >= 0x18) && (ia->ia_addr <= 0x1a)) ||
+ ((ia->ia_addr >= 0x29) && (ia->ia_addr <= 0x2b)) ||
+ ((ia->ia_addr >= 0x4c) && (ia->ia_addr <= 0x4e)))
+ return (1);
+ } else {
+ /*
+ * Direct config - match via the list of compatible
+ * hardware.
+ */
+ if (iic_compat_match(ia, admtemp_compats))
+ return 1;
+ }
- return (0);
+ return 0;
}
Attachment:
pgpg5FpUlUB6D.pgp
Description: PGP signature