Date: Thu, 20 Aug 2020 15:39:18 -0700 From: John-Mark Gurney <jmg@funkthat.com> To: Alexander Mishin <mishin@mh.net.ru> Cc: freebsd-arm@freebsd.org Subject: Re: Kmod driver at iicbus. attach() and config_intrhook(9) Message-ID: <20200820223918.GC4213@funkthat.com> In-Reply-To: <d603cf3b852d5e18fa5faa9066f643a3@mh.net.ru> References: <7fabb65d99aaa74775c1daa91bffb873@mh.net.ru> <3249fa7e-554a-83ef-57b2-7c38aa0b4591@FreeBSD.org> <20200819072409.GA59949@bluezbox.com> <05145b71692af74b103bb226a2e93a15e1e851cb.camel@freebsd.org> <d603cf3b852d5e18fa5faa9066f643a3@mh.net.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
Alexander Mishin wrote this message on Thu, Aug 20, 2020 at 10:07 +0400:
> Ian Lepore ?????????? 2020-08-19 19:39:
> > On Wed, 2020-08-19 at 00:24 -0700, Oleksandr Tymoshenko wrote:
> >> Andriy Gapon (avg@FreeBSD.org) wrote:
> >> > On 18/08/2020 22:05, Alexander Mishin wrote:
> >> > > Hi
> >> > > ...
> >> > > But I see that some other devices (from /usr/src/sys/dev) uses
> >> > > CONFIG_INTRHOOK(9)
> >> > > on attach() for initialize themselfs.
> >> > > I wonder if I need this too? ...
> >> >
> >> > This is usually needed when a driver needs to talk to its device
> >> > while
> >> > attaching. E.g., to set some initial configuration or to confirm
> >> > device's
> >> > identity, etc.
> >>
> >> To extend Andriy's explanation a bit: all these operations may
> >> perform
> >> I2C transfers and most I2C controllers use interrupts to get notified
> >> about tranfer status change (finished, error, etc...). There is no
> >> guarantee that when driver's attach method is called interrupts are
> >> globally enabled. What would happen in this case is: I2C controller
> >> is going to initiate I2C operation and wait for an interrupt that's
> >> never going to be delivered. CONFIG_INTRHOOK is a solution for this
> >> problem, if your attach method requires interrupts - just split it
> >> in two parts and postpone running interrupt-dependent part until
> >> after
> >> interrupts are globally enabled.
> >>
> >
> > A note about all this: It should never be necessary for an i2c slave
> > device driver to do this. The reason it's needed is because many i2c
> > controller drivers attach the iicbus from their attach() routine even
> > though they can't actually do i2c IO until interrupts are available.
> > It is these controller drivers that should have the intrhook logic to
> > not call bus_generic_attach() until interrupts are available if they
> > can't do IO until interrupts are available.
> >
> > It has long been my goal to fix all our i2c controller drivers to
> > behave correctly, so that i2c slave device drivers don't all need the
> > intrhook logic. But somehow I never get around to it.
>
> I think, it would be helpful, as it would be possible to return an error
> on early stage, from attach(), if there is no connection with the
> configured device.
Looks like there's a function bus_delayed_attach_children designed
exactly for this:
* Many buses can't run transactions on the bus which children need to probe and
* attach until after interrupts and/or timers are running. This function
* delays their attach until interrupts and timers are enabled.
and it looks like a couple controllers are already using it, imx_i2c
and ti_i2c...
It looks like maybe a simple replace of bus_generic_attach w/
bus_delayed_attach_children should be enough on those w/ interrupts...
Is there any argument for doing it for ALL controllers instead of just
some?
Poking around some, and it looks like some (one) drivers "pretend" to
use interrupts, but just busy wait instead, e.g. exynos5...
--
John-Mark Gurney Voice: +1 415 225 5579
"All that I will do, has been done, All that I have, has not."
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20200820223918.GC4213>