Re: Yet another dirct config proposal for i2c busses
On Wed, 3 Feb 2010 21:33:28 +0100 Martin Husemann wrote: > 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. [...] > The previous proposals: > - use the OpenBSD way: This was my proposal... > Quick overview how it works: [...] I really like this approach. I'd say this is the way to go. -- tschüß, Jochen Homepage: http://www.unixag-kl.fh-kl.de/~jkunz/
Re: Yet another dirct config proposal for i2c busses
On Thu, Feb 04, 2010 at 06:27:47PM +1100, matthew green wrote: > have you tested it on anything besides sparc64 yet? No - my non-sparc64 machines seem to be mostly i2c-less (have to double check). Martin
re: Yet another dirct config proposal for i2c busses
i like the look of this. have you tested it on anything besides sparc64 yet? .mrg.
Re: Yet another dirct config proposal for i2c busses
I like this approach. I'll try to find some time in the next few days
to build and run my amd64 boxes with your patches; at the very least,
that should verify that indirect config still works.
BTW, there are several other i2c drivers that do the address-and-mask
hack. :) Eventually, they should all be modified in the same way as
your spdmem.c changes...
On Wed, 3 Feb 2010, Martin Husemann wrote:
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?
Yet another dirct config proposal for i2c busses
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 - 1.27
+++ dev/ofw/openfirm.h 3 Feb 2010 19:46:14 -
@@ -115,4 +115,6 @@ boolean_t of_to_dataprop(prop_dicti
