Re: [PATCH 0/4] Support multiplexed main SMBus interface on SB800
Hi Mika, On 22.10.2015 13:43, Mika Westerberg wrote: On Thu, Oct 22, 2015 at 01:03:25PM +0200, Wolfram Sang wrote: Please review and let me know required changes in order to get this upstream finally. Eddi, Thomas, it would be great if you could verify the changes on your machines. Yes, additional tests are always good for a patch series Asking the Intel guys for help, I have not much expierence with x86 platforms... Mika, Jarkko, Andy any chance to help? Unfortunately I don't have hardware this old to test on :-/ And visual review? (That's what I need to do mostly, too) Sure. I don't have a copy of these patches but I went ahead and looked them up from archives. Christian can you Cc me on next iteration? Mostly they look good to me. Few comments though. Patch 2/4: should we remove adapter in reverse order? Patch 3/4: some stylistic issues, like: - ERROR label should not be in capital letters actually it is not needed at all if you do unlock and return -EBUSY if request_region() fails. - Block comment style In addition I'm not sure if requesting io region for each transfer is good idea. Can't we just request it once for this driver and handle the necessary serialization using the mutex? Thanks for the review. I've just sent patchset v2 where I tried to incorporate the requested changes. I'm not sure though what you mean with 'block comment style'. I've tried to apply the same style that is used throughout the file. Thanks, Christian -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] Support multiplexed main SMBus interface on SB800
On Tue, Oct 20, 2015 at 05:19:35PM +0200, Wolfram Sang wrote: > On Tue, Aug 25, 2015 at 01:05:01PM +0200, 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/msg06757.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. > > > > Eddi, Thomas, it would be great if you could verify the changes on your > > machines. > > Yes, additional tests are always good for a patch series > > Asking the Intel guys for help, I have not much expierence with x86 > platforms... Mika, Jarkko, Andy any chance to help? Unfortunately I don't have hardware this old to test on :-/ -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] Support multiplexed main SMBus interface on SB800
On Thu, Oct 22, 2015 at 01:03:25PM +0200, Wolfram Sang wrote: > > > > > Please review and let me know required changes in order to get this > > > > upstream > > > > finally. > > > > > > > > Eddi, Thomas, it would be great if you could verify the changes on your > > > > machines. > > > > > > Yes, additional tests are always good for a patch series > > > > > > Asking the Intel guys for help, I have not much expierence with x86 > > > platforms... Mika, Jarkko, Andy any chance to help? > > > > Unfortunately I don't have hardware this old to test on :-/ > > And visual review? (That's what I need to do mostly, too) Sure. I don't have a copy of these patches but I went ahead and looked them up from archives. Christian can you Cc me on next iteration? Mostly they look good to me. Few comments though. Patch 2/4: should we remove adapter in reverse order? Patch 3/4: some stylistic issues, like: - ERROR label should not be in capital letters actually it is not needed at all if you do unlock and return -EBUSY if request_region() fails. - Block comment style In addition I'm not sure if requesting io region for each transfer is good idea. Can't we just request it once for this driver and handle the necessary serialization using the mutex? -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] Support multiplexed main SMBus interface on SB800
> > > Please review and let me know required changes in order to get this > > > upstream > > > finally. > > > > > > Eddi, Thomas, it would be great if you could verify the changes on your > > > machines. > > > > Yes, additional tests are always good for a patch series > > > > Asking the Intel guys for help, I have not much expierence with x86 > > platforms... Mika, Jarkko, Andy any chance to help? > > Unfortunately I don't have hardware this old to test on :-/ And visual review? (That's what I need to do mostly, too) signature.asc Description: Digital signature
Re: [PATCH 0/4] Support multiplexed main SMBus interface on SB800
On Tue, Aug 25, 2015 at 01:05:01PM +0200, 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/msg06757.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. > > Eddi, Thomas, it would be great if you could verify the changes on your > machines. Yes, additional tests are always good for a patch series Asking the Intel guys for help, I have not much expierence with x86 platforms... Mika, Jarkko, Andy any chance to help? > > Regards, > Christian > > Christian Fetzer (4): > i2c-piix4: Optionally release smba in piix4_adap_remove > 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 | 151 > - > 1 file changed, 134 insertions(+), 17 deletions(-) > > -- > 1.9.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-i2c" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html signature.asc Description: Digital signature
Re: [PATCH 0/4] Support multiplexed main SMBus interface on SB800
On Tue, Aug 25, 2015 at 01:05:01PM +0200, 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/msg06757.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. > > Eddi, Thomas, it would be great if you could verify the changes on your > machines. > > Regards, > Christian > > Christian Fetzer (4): > i2c-piix4: Optionally release smba in piix4_adap_remove > 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 | 151 > - > 1 file changed, 134 insertions(+), 17 deletions(-) Jean, is that on your list? signature.asc Description: Digital signature
[PATCH 0/4] Support multiplexed main SMBus interface on SB800
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/msg06757.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. Eddi, Thomas, it would be great if you could verify the changes on your machines. Regards, Christian Christian Fetzer (4): i2c-piix4: Optionally release smba in piix4_adap_remove 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 | 151 - 1 file changed, 134 insertions(+), 17 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
