Hello,

# Tobias Nygren 2007-04-01:
> I've done some cleanup on my envctrl(sparc64) driver and would like
> someone to review it. One thing I'm not happy with right now is how
> pcfiic attaches to envctrl.
> 
> ( Sources at http://netilium.org/~tnn/netbsd-envctrl/ )
> 
> 1) Should pcf8584.c be moved from dev/i2c to dev/ic or it is fine
>   the way I implemented it?

PCF8584 is an I2C controller (not device), so it would belong to dev/ic
or a sparc64-specific place -- depends if it's a commonly used chip or
not.

The PCF8574 and TDA8444 are fairly generic parts (AD/DA converters), so
I'd say they should live under dev/i2c and provide a kernel-only API to
set/get values on the AD/DA channels.

> 2) In case of (1), should pcfiic attach through autoconf?
> 3) In case of (2), how should it attach?
> "pcfiic at ebus" doesn't make sense to me as the i2c bus should be
> internal to the envctrl driver. Another option would be
> "pcfiic at envctrl" but I'm not sure how to accomplish this.

Hmm, hook envctrl (this name seems too generic, BTW) at ebus and use
dev/ic/pc8584.c to give you i2c handle in exchange for bus_space
tag & handle. Then, use the i2c handle to access the AD/DA parts
with dev/i2c/pcf8574.c and dev/i2c/tda8444.c -- still from envctrl
because noone else can make actual sense of the AD/DA channels and
voltages? Does it make sense?

Oh, it seems this is more or less how you're doing things (sorry, I'm
in hurry so I only had a very brief look at the code).

> Please comment on any bad code and KNF deviations as well.

You can nuke "envctrl_initialized" and use "cold" (true == can't sleep)
to decide whether to use I2C_F_POLL. I'd even say the I2C controllers
should do that for you, but in that case this should be made clear by
the i2c framework -- not sure that's the case.

Hope this helps (not quite sure about that ;-).

	-- Jachym