2012-02-20 11:23:35

by Daniel Kurtz

[permalink] [raw]
Subject: Re: [PATCH 0/3 v2] i2c: i801: enable irq

On Fri, Jan 6, 2012 at 7:35 PM, Jean Delvare <[email protected]> wrote:
> Hi Daniel,
>
> On Fri, ?6 Jan 2012 18:58:19 +0800, Daniel Kurtz wrote:
>> This is a second version of a set of patches enables the Intel PCH SMBus
>> controller interrupt. ?It refactors the second two patches a little bit by
>> relying on DEV_ERR interrupt for timeouts, instead of using an explicit
>> wait_event_timeout.
>>
>> The first attempt received absolutely no response. Maybe this time someone
>> will be interested.
>
> I was on vacation. But I am very interested and will review and test
> your patches. There have been several attempts to add IRQ support to
> i2c-i801 in the past but each time there was a blocker issue which
> prevented it from making it into mainline. Hopefully this time we'll
> get it there!
>
>>
>> The SMBus Host Controller Interrupt can signify:
>> ?INTR - the end of a complete transaction
>> ?DEV_ERR - that a device did not ACK a transaction
>> ?BYTE_DONE - the completion of a single byte during a byte-by-byte transaction
>>
>> This patchset arrives with the following caveats:
>>
>> ?1) ?It has only been tested with a Cougar Point (Intel 6 Series PCH) SMBus
>> controller, so the irq is only enabled for that chip type.
>
> I can test on ICH10 easily, and also on ICH7 and ICH3-M if needed.
>
>> ?2) It has not been tested with any devices that do transactions that use the
>> ? ? PEC. ?In fact, I believe that an additional small patch would be required
>> ? ? to the driver working correctly in interrupt mode with PEC.
>>
>> ?3) It has not been tested in SMBus Slave mode.
>>
>> ?4) It has not been tested with SMI#-type interrupts.
>>
>> ?5) The BIOS has to configure the PCH SMBus IRQ properly.
>>
>> ?6) It has not been tested with a device that does byte-by-byte smbus (non-i2c)
>> ? ? reads.
>
> I think I can test this.
>
>> ?7) It has not been tested with smbus 'process call' transactions.
>
> But not this.
>
>> If would be very helpful if somebody could help test on other chipsets, with
>> a PEC device, or on additional BIOS that woudl be very helpful.
>
> I will do what I can with the hardware I have here.

Jean,
Ping?


>
>> In the meantime, the interrupt behavior is only enabled on the Cougar Point,
>> and even here, it can be completely disabled with the "Interrupt" feature like
>> other advanced features of the driver.
>>
>> Daniel Kurtz (3):
>> ? i2c: i801: refactor i801_block_transaction_byte_by_byte
>> ? i2c: i801: enable irq for i801 smbus transactions
>> ? i2c: i801: enable irq for byte_by_byte transactions
>>
>> ?drivers/i2c/busses/i2c-i801.c | ?199 ++++++++++++++++++++++++++++++++++++-----
>> ?1 files changed, 175 insertions(+), 24 deletions(-)
>
> --
> Jean Delvare


2012-05-09 06:36:27

by Daniel Kurtz

[permalink] [raw]
Subject: Re: [PATCH 0/3 v2] i2c: i801: enable irq

Jean? Anyone? ping?

On Mon, Feb 20, 2012 at 7:23 PM, Daniel Kurtz <[email protected]> wrote:
> On Fri, Jan 6, 2012 at 7:35 PM, Jean Delvare <[email protected]> wrote:
>> Hi Daniel,
>>
>> On Fri, ?6 Jan 2012 18:58:19 +0800, Daniel Kurtz wrote:
>>> This is a second version of a set of patches enables the Intel PCH SMBus
>>> controller interrupt. ?It refactors the second two patches a little bit by
>>> relying on DEV_ERR interrupt for timeouts, instead of using an explicit
>>> wait_event_timeout.
>>>
>>> The first attempt received absolutely no response. Maybe this time someone
>>> will be interested.
>>
>> I was on vacation. But I am very interested and will review and test
>> your patches. There have been several attempts to add IRQ support to
>> i2c-i801 in the past but each time there was a blocker issue which
>> prevented it from making it into mainline. Hopefully this time we'll
>> get it there!
>>
>>>
>>> The SMBus Host Controller Interrupt can signify:
>>> ?INTR - the end of a complete transaction
>>> ?DEV_ERR - that a device did not ACK a transaction
>>> ?BYTE_DONE - the completion of a single byte during a byte-by-byte transaction
>>>
>>> This patchset arrives with the following caveats:
>>>
>>> ?1) ?It has only been tested with a Cougar Point (Intel 6 Series PCH) SMBus
>>> controller, so the irq is only enabled for that chip type.
>>
>> I can test on ICH10 easily, and also on ICH7 and ICH3-M if needed.
>>
>>> ?2) It has not been tested with any devices that do transactions that use the
>>> ? ? PEC. ?In fact, I believe that an additional small patch would be required
>>> ? ? to the driver working correctly in interrupt mode with PEC.
>>>
>>> ?3) It has not been tested in SMBus Slave mode.
>>>
>>> ?4) It has not been tested with SMI#-type interrupts.
>>>
>>> ?5) The BIOS has to configure the PCH SMBus IRQ properly.
>>>
>>> ?6) It has not been tested with a device that does byte-by-byte smbus (non-i2c)
>>> ? ? reads.
>>
>> I think I can test this.
>>
>>> ?7) It has not been tested with smbus 'process call' transactions.
>>
>> But not this.
>>
>>> If would be very helpful if somebody could help test on other chipsets, with
>>> a PEC device, or on additional BIOS that woudl be very helpful.
>>
>> I will do what I can with the hardware I have here.
>
> Jean,
> Ping?
>
>
>>
>>> In the meantime, the interrupt behavior is only enabled on the Cougar Point,
>>> and even here, it can be completely disabled with the "Interrupt" feature like
>>> other advanced features of the driver.
>>>
>>> Daniel Kurtz (3):
>>> ? i2c: i801: refactor i801_block_transaction_byte_by_byte
>>> ? i2c: i801: enable irq for i801 smbus transactions
>>> ? i2c: i801: enable irq for byte_by_byte transactions
>>>
>>> ?drivers/i2c/busses/i2c-i801.c | ?199 ++++++++++++++++++++++++++++++++++++-----
>>> ?1 files changed, 175 insertions(+), 24 deletions(-)
>>
>> --
>> Jean Delvare