Date: Wed, 21 Jan 2009 13:28:05 -0700 (MST) From: "M. Warner Losh" <imp@bsdimp.com> To: jhb@freebsd.org Cc: nwhitehorn@freebsd.org, freebsd-arch@freebsd.org Subject: Re: Enumerable I2C busses Message-ID: <20090121.132805.1108816677.imp@bsdimp.com> In-Reply-To: <200901211110.33961.jhb@freebsd.org> References: <200901210843.33247.jhb@freebsd.org> <49773596.2050700@freebsd.org> <200901211110.33961.jhb@freebsd.org>
index | next in thread | previous in thread | raw e-mail
In message: <200901211110.33961.jhb@freebsd.org>
John Baldwin <jhb@FreeBSD.org> writes:
: On Wednesday 21 January 2009 9:47:50 am Nathan Whitehorn wrote:
: > John Baldwin wrote:
: > > On Tuesday 06 January 2009 2:24:19 pm Nathan Whitehorn wrote:
: > >> M. Warner Losh wrote:
: > >>> In message: <492AC8DE.6050902@freebsd.org>
: > >>> Nathan Whitehorn <nwhitehorn@freebsd.org> writes:
: > >>> : M. Warner Losh wrote:
: > >>> : > In message: <4929C6D8.7090305@freebsd.org>
: > >>> : > Nathan Whitehorn <nwhitehorn@freebsd.org> writes:
: > >>> : > : Rafał Jaworowski wrote:
: > >>> : > : >
: > >>> : > : > On 2008-11-23, at 19:18, Dag-Erling Smørgrav wrote:
: > >>> : > : >
: > >>> : > : >> Nathan Whitehorn <nwhitehorn@freebsd.org> writes:
: > >>> : > : >>> The current I2C bus mechanism does not support the bus adding
: > > its own
: > >>> : > : >>> children [...]
: > >>> : > : >>
: > >>> : > : >> That's because the I2C protocol does not support device
: > > enumeration or
: > >>> : > : >> identification. You have to know in advance what kind of
: devices
: > > are
: > >>> : > : >> attached and at what address. Even worse, it is not uncommon
: for
: > >>> : > : >> similar but not entirely compatible devices to use the same I2C
: > > address
: > >>> : > : >> (for instance, every I2C-capable RTC chip uses the same
: address,
: > > even
: > >>> : > : >> though they have different feature sets)
: > >>> : > : >
: > >>> : > : > Well, hard-coded addresses and conflicting assignments between
: > > vendors
: > >>> : > : > do not technically prevent from scanning the bus; actually, our
: > > current
: > >>> : > : > iicbus code can do bus scaning when compiled with a diag define.
: > > The
: > >>> : > : > problem however is some slave devices are not well-behaved, and
: > > they
: > >>> : > : > don't like to be read/written to other than in very specific
: > > scenario:
: > >>> : > : > if polled during bus scan strange effects occur e.g. they
: > > disappear from
: > >>> : > : > the bus, or do not react to consecutive requests etc.
: > >>> : > :
: > >>> : > : All of this is true, but perhaps my question was badly worded.
: What
: > > I am
: > >>> : > : trying to figure out is how to shove information from an
: out-of-band
: > >>> : > : source (Open Firmware, in this case) into newbus without
: disrupting
: > >>> : > : existing code. In that way, my question is not I2C specific -- we
: > > run
: > >>> : > : into the same issue with the Open Firmware nexus node and
: > > pseudo-devices
: > >>> : > : like cryptosoft that attach themselves.
: > >>> : >
: > >>> : > You are looking at the problem incorrectly. In newbus, a case like
: > >>> : > this the i2c bus should be a subclass (say i2c_of) that is derived
: > >>> : > from the normal i2c bus stuff, but replaces the hints insertion of
: > >>> : > devices with OF enumeration of devices. The OF higher levels will
: > >>> : > already know to attach this kind of i2c bus to certain i2c
: > >>> : > controllers, or always on certain platforms.
: > >>> :
: > >>> : Yes, this is exactly what I wanted to do, like how ofw_pci works.
: > >>> :
: > >>> : > : What I want to do is to have the I2C bus add the children that the
: > >>> : > : firmware says it has. What the firmware cannot tell in advance,
: > > however,
: > >>> : > : is which FreeBSD driver is responsible for those devices, and so
: the
: > > I2C
: > >>> : > : bus driver can't know that without a translation table that I
: would
: > >>> : > : prefer not to hack in to the bus driver.
: > >>> : >
: > >>> : > This is the bigger problem. Today, we are stuck with a lame table
: > >>> : > that will translate the OF provided properties into FreeBSD driver
: > >>> : > names.
: > >>> :
: > >>> : At the moment, I don't believe Apple uses any of the current very
: small
: > >>> : number of I2C device drivers in tree. So I may skip the table for the
: > >>> : time being, assuming the hack below is OK. In future, this may change,
: > >>> : since G5 systems require software thermal control. But that will be
: the
: > >>> : subject of another mail to this list...
: > >>> :
: > >>> : > : It seems reasonable to allow devices to use a real probe routine
: to
: > > look
: > >>> : > : at the firmware's name and compatible properties, like we allow on
: > > other
: > >>> : > : Open Firmware busses. The trouble is that existing drivers don't
: do
: > >>> : > : this, because they expect to be attached with hints, so they will
: > > attach
: > >>> : > : to all devices. I'm trying to figure out how to avoid this.
: > >>> : > :
: > >>> : > : My basic question comes down to whether there is a good way in
: > > newbus to
: > >>> : > : handle busses that may be wholly or partially enumerated by
: firmware
: > > or
: > >>> : > : some other method, and may also have devices that can only attach
: > >>> : > : themselves if told to by hints.
: > >>> : >
: > >>> : > Yes. This is a bit of a problem. The problem is that the existing
: > >>> : > hints mechanism combines device tree and driver tree into one, and
: in
: > >>> : > such a scenario, we wind up with the problem that you have.
: > >>> : >
: > >>> : > One could make the probe routines return BUS_PROBE_GENERIC, and that
: > >>> : > would help somewhat. One could also have the probe routine check to
: > >>> : > see if a specific driver is assigned to the device or not. That
: would
: > >>> : > also help, but does mean changing all the i2c bus attached drivers
: in
: > >>> : > the tree.
: > >>> :
: > >>> : I think changing existing I2C drivers may be unavoidable. Would there
: be
: > >>> : any objection to changing the MI iicbus drivers to return
: > >>> : BUS_PROBE_NOWILDCARD in their probe routines? It seems to have been
: > >>> : introduced (by you) to solve more or less exactly this problem. By my
: > >>> : count, the relevant files are:
: > >>> : dev/iicbus/ds133x.c
: > >>> : dev/iicbus/icee.c
: > >>> : dev/iicbus/ad7418.c
: > >>> : dev/iicbus/iicsmb.c
: > >>> : dev/iicbus/ds1672.c
: > >>> : dev/iicbus/if_ic.c
: > >>> : dev/iicbus/iic.c
: > >>> :
: > >>> : I would also like to change iicbus_probe to return -1000 like
: > >>> : dev/pci/pci.c to allow it to be overridden by a subclass. Please let
: me
: > >>> : know if this is a terrible idea or if I have forgotten any I2C device
: > >>> : drivers.
: > >>>
: > >>> Short term, this is the right fix. There was an objection, I think by
: > >>> Marcel, to this approach. However, his objections were part of a
: > >>> larger set of objections and I think that we're working to solve
: > >>> those.
: > >>>
: > >>> Warner
: > >>>
: > >> This is now in the tree. Now for part 2, which I had not considered
: > >> previously: connecting the I2C bus layer to the I2C host adapters.
: > >>
: > >> Right now, we have the following:
: > >> kiic/other i2c adapters
: > >> ---iicbus
: > >> ---ofw_iicbus
: > >>
: > >> Since kiic provides an Open Firmware node, ofw_iicbus gets priority,
: > >> attaches, and everything after that is wonderful. The issue is how best
: > >> to attach the iicbus modules to kiic. Current I2C controllers contain a
: > >> line like this:
: > >> DRIVER_MODULE(iicbus, me, iicbus_driver, iicbus_devclass, 0, 0);
: > >>
: > >> This explicitly specifies that you want the standard driver, so we need
: > >> additional glue to allow the ofw_iicbus driver to attach. One solution
: > >> is that each relevant host adapter can add an additional DRIVER_MODULE
: > >> line with the ofw_iicbus driver and class, which would have to exported
: > >> in a header somewhere. This is pretty ugly. Another solution is for the
: > >> ofw_iicbus module to grow a list of the names of interesting adapters.
: > >> This is worse.
: > >>
: > >> The third option is to do what we do for pci, where all PCI adapters are
: > >> named 'pcib'. So we could make new I2C host adapters named 'iichb' or
: > >> something, and then move the DRIVER_MODULE logic for new code into the
: > >> bus modules, as is done for PCI. This decreases the amount of
: > >> information in the device names, but seems a bit cleaner. Thoughts?
: > >> -Nathan
: > >
: > > If ofw_iicbus is simply an OF-aware version of iicbus (i.e. same
: > > functionality) similar to the OF-aware PCI bus, then I would go the PCI
: route
: > > and just call it iicbus but give it a higher probe priority.
: > >
: >
: > Which it is. What I meant was the bridge devices to which iicbus
: > attaches. For pci, they all end up with the same name (pcib) so that the
: > pci layer knows to attach to them. For I2C, they are called
: > iicbb/pcf/at91_twi/etc. and each bridge device explicitly attaches the
: > standard iicbus to itself, instead of letting it and any firmware-aware
: > versions probe.
:
: I'm a bit torn on that one, especially since you have weird cases like one of
: the via parts that has both smbus and iicbus children.
:
: The other option would be to fix the attaching to subclasses thing (that
: should make all pci drivers attach to cardbus0 devices since cardbus inherits
: from pci, for example) and then you could have what would basically be an
: abstract base class "iicbridge" with no devmethods that all bridge drivers
: inherit from, and iicbus would attach to that.
Right now, the only bug that I know about with the cardbus vs pci
thing is that kldload doesn't necessarily probe/attach pci drivers on
a cardbus bus. Otherwise, it works perfectly. This is the only
reason that we have driver lines with cardbus attachments in the pci
drivers at all...
Warner
home |
help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090121.132805.1108816677.imp>