Kmod driver at iicbus. attach() and config_intrhook(9)

[Alexander Mishin]

Ian Lepore писал 2020-08-21 02:51:
> On Thu, 2020-08-20 at 15:39 -0700, John-Mark Gurney wrote:
>> 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 at 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...
>> 
> 
> Hmm, yeah, it looks like more has been done along these lines than I
> remembered.  In fact, the work may be done.
> 
> Some i2c controllers have to work properly before interrupts are
> available, to control things like PMIC chips that are required very
> early in device configuration.  Typically they have some sort of
> polling mechanism that's used early, and revert to using interrupts
> once they're available.  The allwinner and rockchip drivers work that
> way.
> 
> -- Ian
Just to dot the i's, my SoC is allwinner (Orange PI PC). And the issue 
at boot time really showed itself up until config_intrhook was used.
FreeBSD myhost.mh.net.ru 13.0-CURRENT FreeBSD 13.0-CURRENT #3 r364004