[prev in list] [next in list] [prev in thread] [next in thread] 

List:       netbsd-tech-kern
Subject:    Re: resettodr() and (over)use of I2C_F_POLL
From:       Manuel Bouyer <bouyer () antioche ! eu ! org>
Date:       2019-07-25 7:50:31
Message-ID: 20190725075031.GA220 () antioche ! eu ! org
[Download RAW message or body]

On Wed, Jul 24, 2019 at 03:30:57PM -0700, Jason Thorpe wrote:
> I2C_F_POLL (poll for completion -- no sleeping) is used a lot in sys/dev/i2c.  In \
> some places, it's used conditionally based on the value of 'cold', which seems \
> fine, and I am going to centralize that logic in i2c_exec.c so it doesn't have to \
> be replicated. 
> But there's a number of places using I2C_F_POLL outside of cold auto configuration, \
> and I can't believe that it's actually necessary in all of them. 
> Each i2c device has exclusive access to the i2c bus while doing its operation, and \
> I don't believe that's allowed in any interrupt context at all.  Certainly, I don't \
> think the various i2c real-time clock drivers need to use I2C_F_POLL -- inittodr() \
> is called very late in the startup process, and resettodr() is not called in the \
> crash or shutdown process.  That said, resettodr() *is* called from \
> kern_time.c:settime1(), which wraps the entire function body in a splclock() / \
> splx() pair.  This seems ... unnecessary.  Seems like the following diff is safe / \
> appropriate, though I'm still suspicious of the whole splclock() thing.

I don't have comments on this particular patch; but in general
we are doing i2c access from interrupt contect; when a i2c device has an
interrupt line. An example of this are the i2c HID drivers.

To defer interrupt processing to a thread we'd need a way to mask/unmask
interrupts at the PIC level but our MI interrupt framework doesn't allow
this.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--


[prev in list] [next in list] [prev in thread] [next in thread]