Date: Wed, 21 Jan 2009 16:29:50 -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.162950.1552132922.imp@bsdimp.com> In-Reply-To: <200901211542.08461.jhb@freebsd.org> References: <200901211110.33961.jhb@freebsd.org> <20090121.132805.1108816677.imp@bsdimp.com> <200901211542.08461.jhb@freebsd.org>
index | next in thread | previous in thread | raw e-mail
In message: <200901211542.08461.jhb@freebsd.org>
John Baldwin <jhb@FreeBSD.org> writes:
: On Wednesday 21 January 2009 3:28:05 pm M. Warner Losh wrote:
: > 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...
:
: That is the bug though. :) I've looked at it and I think I know how to fix
: it, I just haven't gotten around to it. I think device_probe_and_attach()
: needs to actually walk up the inheritance list of the current 'bus' driver,
: but only if all of the drivers for the current 'bus' failed to probe (or
: there are no drivers).
But why then does it work with the normal probe and only fail on the
load?
Warner
help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090121.162950.1552132922.imp>