Re: [PATCH v4 0/3] Support multiplexed main SMBus interface on SB800

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Sun, 2015-11-15 at 12:33 +0100, Christian Fetzer wrote:
> This is an attempt to upstream the patches created by Thomas Brandon
> and
> Eddi De Pieri to support the multiplexed main SMBus interface on the
> SB800
> chipset. (https://www.mail-archive.com/linux-i2c@vger.kernel.org/msg0
> 6757.html)
> 
> I have mainly rebased the latest patch version and tested the driver
> on a
> HP ProLiant MicroServer G7 N54L (where this patch allows to access
> sensor data
> from a w83795adg).
> 
> The patched driver is running stable on the machine, given that
> ic2_piix4 is
> loaded before jc42 and w83795. If jc42 is loaded before i2c_piix4
> calling
> sensors triggers some errors:
>     ERROR: Can't get value of subfeature temp1_min_alarm: Can't read
> 
> While the kernel log shows:
>     i2c i2c-1: Transaction (pre): CNT=0c, CMD=05, ADD=31, DAT0=03,
> DAT1=c0
>     i2c i2c-1: Error: no response!
>     i2c i2c-1: Transaction (post): CNT=0c, CMD=05, ADD=31, DAT0=ff,
> DAT1=ff
> Unfortunately I don't know how to tackle this specific issue.
> 
> Please review and let me know required changes in order to get this
> upstream
> finally.

One nitpick in one patch, though it looks okay for me:

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> 
> Eddi, Thomas, it would be great if you could verify the changes on
> your
> machines.
> 
> Regards,
> Christian
> 
> v4:
> - Incorporated changes requested by Andy
>     - added mutex to struct i2c_piix4_adapdata
>     - added flag for releasing SMBus index region to struct
> i2c_piix4_adapdata
>         - this flag now indicates that the adapter is a sb800 main
> adapter
>         - together with the port number it simplifies the adapter
>           releasing and the first patch in v3 is no more needed
>         - unfortunately patch 3 and 4 in v3 had to be combined as
> only
>           the patch introducing multiplexing adds a separate
> add_adapters_sb800
>           method that can be used to set the flag.
>     - fixed releasing the SMBus index region in case setting up the
>       adapter fails
> 
> v3:
> - Incorporated changes requested by Mika and Andy
>     - main adapter name set to 'main'
>     - defined constant idx address
>     - block comment style, joined string literals, reworked for loops
>       into while loops
> 
> v2:
> - Incorporated changes requested by Mika
>     - remove adapter in reverse order
>     - ERROR label
>     - request base address index region only once
> 
> Christian Fetzer (3):
>   i2c-piix4: Convert piix4_main_adapter to array
>   i2c-piix4: Add support for multiplexed main adapter in SB800
>   i2c-piix4: Add adapter port name support for SB800 chipset
> 
>  drivers/i2c/busses/i2c-piix4.c | 194
> +++++++++++++++++++++++++++++++++++------
>  1 file changed, 165 insertions(+), 29 deletions(-)
> 

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy