[PATCH 00/03] i2c-i801: Add basic interrupt support

[PATCH 00/03] i2c-i801: Add basic interrupt support

[Ivo Manca]

Hey All,

It's been a while since I last posted about this matter, but the patch 
is finally there.

This patch adds basic interrupt support to the i2c-i801 bus driver. This 
means i2c transactions can be completed without useless and pricy 
waiting loops (the polling) and speeds things up.
Currently only basic transactions are support. The HW PEC or i2c block 
transactions still use polling.

To write this patch, I've extensively used Mark M. Hoffman's i2c-i801 
rewrite of the i2c-i801 driver dating back from 28 Nov 2008. This can be 
found at http://archives.andrew.net.au/lm-sensors/msg28496.html

I've tested this patch extensively on a machine provided by the Hague 
University (Thanks Hans! ;) which is ICH5 based, but it needs a lot of 
more testing. So please be welcome to give the patch a shot.

Since I’m completely new to linux device driver development, interrupts 
and i2c, stupid mistakes can be very well possible. Please use this 
patch with caution and let me know whatever I did wrong :). Oh, and if 
your PC won’t start up anymore, removing power from the PC always worked 
for me ;p.

All patches are made against 2.6.26-rc9

Patch 1 implements the functionality, while patch 2 only adds the 
alignment changes. This is done to make it easier reviewable. Patch 3 is 
quite unrelated and only fixes some checkPatch warnings..

---
Ivo Manca

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 00/03] i2c-i801: Add basic interrupt support

[Jean Delvare]

Hi Ivo,

On Wed, 23 Jul 2008 18:56:00 +0200, Ivo Manca wrote:
> Hey All,
> 
> It's been a while since I last posted about this matter, but the patch 
> is finally there.
> 
> This patch adds basic interrupt support to the i2c-i801 bus driver. This 
> means i2c transactions can be completed without useless and pricy 
> waiting loops (the polling) and speeds things up.
> Currently only basic transactions are support. The HW PEC or i2c block 
> transactions still use polling.

Ah, really great! Thanks for working on this :)

Is there any inherent difficulty to get interrupts to work with HW PEC?
And with I2C block transactions? Or did you just not get around to
doing it?

> To write this patch, I've extensively used Mark M. Hoffman's i2c-i801 
> rewrite of the i2c-i801 driver dating back from 28 Nov 2008. This can be 
> found at http://archives.andrew.net.au/lm-sensors/msg28496.html

Yeah, Mark has been traveling a lot in time lately. Unfortunately this
also means that we can't watch this archived post yet, it'll be a few
more months before it shows up! ;)

(Seriously, archives.andrew.net.au is the old list archive and
apparently it has been discontinued, please use lists.lm-sensors.org
instead.)

> I've tested this patch extensively on a machine provided by the Hague 
> University (Thanks Hans! ;) which is ICH5 based, but it needs a lot of 
> more testing. So please be welcome to give the patch a shot.
> 
> Since I’m completely new to linux device driver development, interrupts 
> and i2c, stupid mistakes can be very well possible. Please use this 
> patch with caution and let me know whatever I did wrong :). Oh, and if 
> your PC won’t start up anymore, removing power from the PC always worked 
> for me ;p.

I've just given it a quick try on my old ICH3-M-based laptop and it
seems to work fine there. The speed improvement compared to the polling
code is very impressive.

I'm going to do more tests on 2 other machines (with ICH5 and ICH7
respectively) and will report when I'm done. A patch review will also
follow.

> All patches are made against 2.6.26-rc9

The i2c-i801 driver has changed a lot since then. Any chance you could
respin the patches against 2.6.27-rc2?

> Patch 1 implements the functionality, while patch 2 only adds the 
> alignment changes. This is done to make it easier reviewable. Patch 3 is 
> quite unrelated and only fixes some checkPatch warnings..

Thanks,
-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 00/03] i2c-i801: Add basic interrupt support

[Ivo Manca]

Hey Jean,

Jean Delvare wrote:
> Ah, really great! Thanks for working on this :)
>
> Is there any inherent difficulty to get interrupts to work with HW PEC?
> And with I2C block transactions? Or did you just not get around to
> doing it?
>   
To be honoust: I don't know a thing about HW PEC and was not able to 
test it anyway. Therefor, I decided not to touch this part yet. I did 
try to add support for interrupts with I2C block transactions. This 
however caused various nasty problems.

That made me first want to test and implement basic interrupt support 
before going there again. When I know for sure the basics work correctly 
and stable (and when I know for sure I haven't made very stupid coding 
mistakes), I'll try again to add support for block transactions :).
> Yeah, Mark has been traveling a lot in time lately. Unfortunately this
> also means that we can't watch this archived post yet, it'll be a few
> more months before it shows up! ;)
>
> (Seriously, archives.andrew.net.au is the old list archive and
> apparently it has been discontinued, please use lists.lm-sensors.org
> instead.)
>   
Ugh, you _can't_ time travel yourself? ";p

(I used archives.andrew.net.au because that was the first hit on 
google... Anyway, here is the correct link: 
http://lists.lm-sensors.org/pipermail/lm-sensors/2004-November/009588.html )
> I've just given it a quick try on my old ICH3-M-based laptop and it
> seems to work fine there. The speed improvement compared to the polling
> code is very impressive.
>
> I'm going to do more tests on 2 other machines (with ICH5 and ICH7
> respectively) and will report when I'm done. A patch review will also
> follow.
>   
Thanks! Glad to hear I didn't kill your laptop :).
> The i2c-i801 driver has changed a lot since then. Any chance you could
> respin the patches against 2.6.27-rc2?
>   
Sure, I'll try to respin it as soon as possible. Hopefully, that will be 
tomorrow.
> Thanks,
>   
Ivo

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 00/03] i2c-i801: Add basic interrupt support

[Jean Delvare]

On Sun, 10 Aug 2008 22:08:03 +0200, Ivo Manca wrote:
> Hey Jean,
> 
> Jean Delvare wrote:
> > Ah, really great! Thanks for working on this :)
> >
> > Is there any inherent difficulty to get interrupts to work with HW PEC?
> > And with I2C block transactions? Or did you just not get around to
> > doing it?
>
> To be honoust: I don't know a thing about HW PEC and was not able to 
> test it anyway. Therefor, I decided not to touch this part yet. I did 
> try to add support for interrupts with I2C block transactions. This 
> however caused various nasty problems.

PEC is an optional extra byte at the end of SMBus transactions,
containing a checksum to detect transaction errors. It can be handled
directly by the ICH4 hardware and later, thus the name "hwpec". The
ICH3 can do software PEC but we don't support that.

I don't think you need to care much about this as far as interrupts are
concerned, this should be pretty transparent.

I2C block writes shouldn't differ from other block transactions. I2C
block reads OTOH are special in many aspects so I do expect special
handling to be required for interrupt support.

> That made me first want to test and implement basic interrupt support 
> before going there again. When I know for sure the basics work correctly 
> and stable (and when I know for sure I haven't made very stupid coding 
> mistakes), I'll try again to add support for block transactions :).

This is a good strategy, no question about that.

> > I've just given it a quick try on my old ICH3-M-based laptop and it
> > seems to work fine there. The speed improvement compared to the polling
> > code is very impressive.
> >
> > I'm going to do more tests on 2 other machines (with ICH5 and ICH7
> > respectively) and will report when I'm done. A patch review will also
> > follow.
>
> Thanks! Glad to hear I didn't kill your laptop :).

Testing is done, and all 3 systems survived and actually worked :) Well
done!

> > The i2c-i801 driver has changed a lot since then. Any chance you could
> > respin the patches against 2.6.27-rc2?
>
> Sure, I'll try to respin it as soon as possible. Hopefully, that will be 
> tomorrow.

Thanks. No need to hurry though, take your time. You may want to wait
for my review so that you can address my concerns at the same time.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c