Re: [PATCH 1/5] i2c: Add SMBus alert support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, 14 Feb 2010 10:05:53 -0800, David Brownell wrote:
> On Sunday 14 February 2010, Jean Delvare wrote:
> > > to use that infrastructure instead of a work struct?  There are
> > > several mechanisms to kick in here.  It'd be fair to have a way for
> > > IRQ threads to issue i2c_handle_smbus_alert() -- or maybe some
> > > sibling method -- especially if i2c_setup_smbus_alert() causes
> > > this code to provide the hardirq handler.
> > 
> > Honestly, this is all beyond me. I suggest that anyone needing this
> > work on it on his/her own and submits an incremental patch when done. I
> > have already spent more time than I wanted on this, I can't afford
> > spending more, especially for a feature I don't need myself.
> 
> Then I'd suggest sticking in a REVISIT comment to that effect,
> to help armor this patch against repeats of that feedback which
> don't accompany such an incremental patch.  :)

I would wholeheartedly accept a patch adding such a comment ;)
Seriously, I wouldn't know what to write.

> > > By the way ... you probably know that the I2C/SMBus controller
> > > in most of Intel's Southbridge chips (like ICH8) supports
> > > the SMBALERT# mechanism.  Testing may be awkward, but it'd be
> > > good to verify this incarnation of SMBALERT# support can work
> > > with those controllers.  (Where "alert" is just another cause
> > > for an IRQ from i2c-i801.c or whatever, not a dedicated IRQ
> > > from a GPIO or from the parport.)
> > 
> > I have only one system with an ICH and an SMBus device with support for
> > alerts. It is not currently available for testing... when it becomes
> > available, I may take a look. That being said, the main obstacle I see
> > is that the i2c-i801 driver doesn't make use of interrupts at all at
> > the moment. I don't know if it is possible to enable them to get the
> > alerts but to not make use of them during regular transactions. The
> > various attempts to make the i2c-i801 driver use interrupts never made
> > it to mainline, there was always an least one issue remaining
> > preventing it. I would love if we finally managed to switch to full
> > interrupt support.
> 
> Ah, I recall some of that mess, now that you mention it.  Ouch!
> 
> (By the way ... glad to see you did the nice thing with parport I2C
> and SMALERT#.  I've not seen much Linux code using the parport IRQ,
> and if nothing else it's nice to have some verification that it has
> not bit-rotted to death!)

I'm not so sure. You'll notice I had to call parport_disable_irq()
before registering my device. Without it, my IRQ handler would be
called before being ready. This is odd given that the comments in the
parport subsystem code say that you have to call parport_enable_irq()
if you want to receive interrupts (which seems logical). So there's
certainly something slightly broken down there. I don't really have the
time to investigate though :(

> > That being said, my code is based on yours, and I see to remember you
> > said yours was compatible with the ICH expectations, so I'd expect mine
> > to work too.
> 
> The compatibility was because the driver could say "I got an SMBALERT IRQ"
> to the infrastructure from its hardirq handler.  I think the comments in
> your version touched on that point.

-- 
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux