* [PATCH] i2c: i2c-scmi: add a MS HID @ 2017-03-10 13:58 Viktor Krasnov 2017-03-23 20:47 ` Wolfram Sang 2017-03-24 9:16 ` Jean Delvare 0 siblings, 2 replies; 9+ messages in thread From: Viktor Krasnov @ 2017-03-10 13:58 UTC (permalink / raw) To: jdelvare, wsa; +Cc: linux-i2c, Viktor Krasnov, Edgar Cherkasov, Michael Brunner From: Edgar Cherkasov <echerkasov@dev.rtsoft.ru> Description of the problem: - i2c-scmi driver contains only two identifiers "SMBUS01" and "SMBUSIBM"; - the fist HID (SMBUS01) is clearly defined in "SMBus Control Method Interface Specification, version 1.0": "Each device must specify 'SMBUS01' as its _HID and use a unique _UID value"; - unfortunately, BIOS vendors (like AMI) seem to ignore this requirement and implement "SMB0001" HID instead of "SMBUS01"; - I speculate that they do this because only "SMB0001" is hard coded in Windows SMBus driver produced by Microsoft. This leads to following situation: - SMBus works out of box in Windows but not in Linux; - board vendors are forced to add correct "SMBUS01" HID to BIOS to make SMBus work in Linux. Moreover the same board vendors complain that tools (3-rd party ASL compiler) do not like the "SMBUS01" identifier and produce errors. So they need to constantly patch the compiler for each new version of BIOS. As it is very unlikely that BIOS vendors implement a correct HID in future, I would propose to consider whether it is possible to work around the problem by adding MS HID to the Linux i2c-scmi driver. Signed-off-by: Edgar Cherkasov <echerkasov@dev.rtsoft.ru> Signed-off-by: Michael Brunner <Michael.Brunner@kontron.com> Acked-by: Viktor Krasnov <vkrasnov@dev.rtsoft.ru> --- drivers/i2c/busses/i2c-scmi.c | 1 + include/acpi/acpi_drivers.h | 2 ++ 2 files changed, 3 insertions(+) diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c index dfc98df..fb9aee0 100644 --- a/drivers/i2c/busses/i2c-scmi.c +++ b/drivers/i2c/busses/i2c-scmi.c @@ -51,6 +51,7 @@ struct acpi_smbus_cmi { static const struct acpi_device_id acpi_smbus_cmi_ids[] = { {"SMBUS01", (kernel_ulong_t)&smbus_methods}, {ACPI_SMBUS_IBM_HID, (kernel_ulong_t)&ibm_smbus_methods}, + {ACPI_SMBUS_MS_HID, (kernel_ulong_t)&smbus_methods}, {"", 0} }; MODULE_DEVICE_TABLE(acpi, acpi_smbus_cmi_ids); diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h index 29c6912..d34538b 100644 --- a/include/acpi/acpi_drivers.h +++ b/include/acpi/acpi_drivers.h @@ -60,6 +60,8 @@ #define ACPI_DOCK_HID "LNXDOCK" /* Quirk for broken IBM BIOSes */ #define ACPI_SMBUS_IBM_HID "SMBUSIBM" +/* SMBUS HID definition as supported by Microsoft Windows */ +#define ACPI_SMBUS_MS_HID "SMB0001" /* * For fixed hardware buttons, we fabricate acpi_devices with HID -- 1.9.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] i2c: i2c-scmi: add a MS HID 2017-03-10 13:58 [PATCH] i2c: i2c-scmi: add a MS HID Viktor Krasnov @ 2017-03-23 20:47 ` Wolfram Sang 2017-03-30 15:52 ` Wolfram Sang 2017-03-24 9:16 ` Jean Delvare 1 sibling, 1 reply; 9+ messages in thread From: Wolfram Sang @ 2017-03-23 20:47 UTC (permalink / raw) To: Viktor Krasnov Cc: jdelvare, linux-i2c, Edgar Cherkasov, Michael Brunner, linux-acpi [-- Attachment #1: Type: text/plain, Size: 2815 bytes --] On Fri, Mar 10, 2017 at 04:58:35PM +0300, Viktor Krasnov wrote: Thanks for the patch! adding linux-acpi to CC. This is more an ACPI question than I2C. > From: Edgar Cherkasov <echerkasov@dev.rtsoft.ru> > > Description of the problem: > - i2c-scmi driver contains only two identifiers "SMBUS01" and "SMBUSIBM"; > - the fist HID (SMBUS01) is clearly defined in "SMBus Control Method > Interface Specification, version 1.0": "Each device must specify > 'SMBUS01' as its _HID and use a unique _UID value"; > - unfortunately, BIOS vendors (like AMI) seem to ignore this requirement > and implement "SMB0001" HID instead of "SMBUS01"; > - I speculate that they do this because only "SMB0001" is hard coded in > Windows SMBus driver produced by Microsoft. > > This leads to following situation: > - SMBus works out of box in Windows but not in Linux; > - board vendors are forced to add correct "SMBUS01" HID to BIOS to make > SMBus work in Linux. Moreover the same board vendors complain that > tools (3-rd party ASL compiler) do not like the "SMBUS01" identifier > and produce errors. So they need to constantly patch the compiler for > each new version of BIOS. > > As it is very unlikely that BIOS vendors implement a correct HID in > future, I would propose to consider whether it is possible to work around > the problem by adding MS HID to the Linux i2c-scmi driver. > > Signed-off-by: Edgar Cherkasov <echerkasov@dev.rtsoft.ru> > Signed-off-by: Michael Brunner <Michael.Brunner@kontron.com> > Acked-by: Viktor Krasnov <vkrasnov@dev.rtsoft.ru> > --- > drivers/i2c/busses/i2c-scmi.c | 1 + > include/acpi/acpi_drivers.h | 2 ++ > 2 files changed, 3 insertions(+) > > diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c > index dfc98df..fb9aee0 100644 > --- a/drivers/i2c/busses/i2c-scmi.c > +++ b/drivers/i2c/busses/i2c-scmi.c > @@ -51,6 +51,7 @@ struct acpi_smbus_cmi { > static const struct acpi_device_id acpi_smbus_cmi_ids[] = { > {"SMBUS01", (kernel_ulong_t)&smbus_methods}, > {ACPI_SMBUS_IBM_HID, (kernel_ulong_t)&ibm_smbus_methods}, > + {ACPI_SMBUS_MS_HID, (kernel_ulong_t)&smbus_methods}, > {"", 0} > }; > MODULE_DEVICE_TABLE(acpi, acpi_smbus_cmi_ids); > diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h > index 29c6912..d34538b 100644 > --- a/include/acpi/acpi_drivers.h > +++ b/include/acpi/acpi_drivers.h > @@ -60,6 +60,8 @@ > #define ACPI_DOCK_HID "LNXDOCK" > /* Quirk for broken IBM BIOSes */ > #define ACPI_SMBUS_IBM_HID "SMBUSIBM" > +/* SMBUS HID definition as supported by Microsoft Windows */ > +#define ACPI_SMBUS_MS_HID "SMB0001" > > /* > * For fixed hardware buttons, we fabricate acpi_devices with HID > -- > 1.9.3 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] i2c: i2c-scmi: add a MS HID 2017-03-23 20:47 ` Wolfram Sang @ 2017-03-30 15:52 ` Wolfram Sang 2017-03-31 7:20 ` Mika Westerberg 2017-03-31 10:01 ` Andy Shevchenko 0 siblings, 2 replies; 9+ messages in thread From: Wolfram Sang @ 2017-03-30 15:52 UTC (permalink / raw) To: Viktor Krasnov Cc: jdelvare, linux-i2c, Edgar Cherkasov, Michael Brunner, linux-acpi, Mika Westerberg, Andy Shevchenko, Jarkko Nikula [-- Attachment #1: Type: text/plain, Size: 3058 bytes --] On Thu, Mar 23, 2017 at 09:47:47PM +0100, Wolfram Sang wrote: > On Fri, Mar 10, 2017 at 04:58:35PM +0300, Viktor Krasnov wrote: > > Thanks for the patch! > > adding linux-acpi to CC. This is more an ACPI question than I2C. Mika? Andy? Jarkko? Any input on this? > > > From: Edgar Cherkasov <echerkasov@dev.rtsoft.ru> > > > > Description of the problem: > > - i2c-scmi driver contains only two identifiers "SMBUS01" and "SMBUSIBM"; > > - the fist HID (SMBUS01) is clearly defined in "SMBus Control Method > > Interface Specification, version 1.0": "Each device must specify > > 'SMBUS01' as its _HID and use a unique _UID value"; > > - unfortunately, BIOS vendors (like AMI) seem to ignore this requirement > > and implement "SMB0001" HID instead of "SMBUS01"; > > - I speculate that they do this because only "SMB0001" is hard coded in > > Windows SMBus driver produced by Microsoft. > > > > This leads to following situation: > > - SMBus works out of box in Windows but not in Linux; > > - board vendors are forced to add correct "SMBUS01" HID to BIOS to make > > SMBus work in Linux. Moreover the same board vendors complain that > > tools (3-rd party ASL compiler) do not like the "SMBUS01" identifier > > and produce errors. So they need to constantly patch the compiler for > > each new version of BIOS. > > > > As it is very unlikely that BIOS vendors implement a correct HID in > > future, I would propose to consider whether it is possible to work around > > the problem by adding MS HID to the Linux i2c-scmi driver. > > > > Signed-off-by: Edgar Cherkasov <echerkasov@dev.rtsoft.ru> > > Signed-off-by: Michael Brunner <Michael.Brunner@kontron.com> > > Acked-by: Viktor Krasnov <vkrasnov@dev.rtsoft.ru> > > --- > > drivers/i2c/busses/i2c-scmi.c | 1 + > > include/acpi/acpi_drivers.h | 2 ++ > > 2 files changed, 3 insertions(+) > > > > diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c > > index dfc98df..fb9aee0 100644 > > --- a/drivers/i2c/busses/i2c-scmi.c > > +++ b/drivers/i2c/busses/i2c-scmi.c > > @@ -51,6 +51,7 @@ struct acpi_smbus_cmi { > > static const struct acpi_device_id acpi_smbus_cmi_ids[] = { > > {"SMBUS01", (kernel_ulong_t)&smbus_methods}, > > {ACPI_SMBUS_IBM_HID, (kernel_ulong_t)&ibm_smbus_methods}, > > + {ACPI_SMBUS_MS_HID, (kernel_ulong_t)&smbus_methods}, > > {"", 0} > > }; > > MODULE_DEVICE_TABLE(acpi, acpi_smbus_cmi_ids); > > diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h > > index 29c6912..d34538b 100644 > > --- a/include/acpi/acpi_drivers.h > > +++ b/include/acpi/acpi_drivers.h > > @@ -60,6 +60,8 @@ > > #define ACPI_DOCK_HID "LNXDOCK" > > /* Quirk for broken IBM BIOSes */ > > #define ACPI_SMBUS_IBM_HID "SMBUSIBM" > > +/* SMBUS HID definition as supported by Microsoft Windows */ > > +#define ACPI_SMBUS_MS_HID "SMB0001" > > > > /* > > * For fixed hardware buttons, we fabricate acpi_devices with HID > > -- > > 1.9.3 > > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] i2c: i2c-scmi: add a MS HID 2017-03-30 15:52 ` Wolfram Sang @ 2017-03-31 7:20 ` Mika Westerberg 2017-03-31 7:31 ` Viktor Krasnov 2017-03-31 10:01 ` Andy Shevchenko 1 sibling, 1 reply; 9+ messages in thread From: Mika Westerberg @ 2017-03-31 7:20 UTC (permalink / raw) To: Wolfram Sang Cc: Viktor Krasnov, jdelvare, linux-i2c, Edgar Cherkasov, Michael Brunner, linux-acpi, Andy Shevchenko, Jarkko Nikula On Thu, Mar 30, 2017 at 05:52:33PM +0200, Wolfram Sang wrote: > On Thu, Mar 23, 2017 at 09:47:47PM +0100, Wolfram Sang wrote: > > On Fri, Mar 10, 2017 at 04:58:35PM +0300, Viktor Krasnov wrote: > > > > Thanks for the patch! > > > > adding linux-acpi to CC. This is more an ACPI question than I2C. > > Mika? Andy? Jarkko? Any input on this? > > > > > > From: Edgar Cherkasov <echerkasov@dev.rtsoft.ru> > > > > > > Description of the problem: > > > - i2c-scmi driver contains only two identifiers "SMBUS01" and "SMBUSIBM"; > > > - the fist HID (SMBUS01) is clearly defined in "SMBus Control Method > > > Interface Specification, version 1.0": "Each device must specify > > > 'SMBUS01' as its _HID and use a unique _UID value"; > > > - unfortunately, BIOS vendors (like AMI) seem to ignore this requirement > > > and implement "SMB0001" HID instead of "SMBUS01"; > > > - I speculate that they do this because only "SMB0001" is hard coded in > > > Windows SMBus driver produced by Microsoft. > > > > > > This leads to following situation: > > > - SMBus works out of box in Windows but not in Linux; > > > - board vendors are forced to add correct "SMBUS01" HID to BIOS to make > > > SMBus work in Linux. Moreover the same board vendors complain that > > > tools (3-rd party ASL compiler) do not like the "SMBUS01" identifier > > > and produce errors. So they need to constantly patch the compiler for > > > each new version of BIOS. > > > > > > As it is very unlikely that BIOS vendors implement a correct HID in > > > future, I would propose to consider whether it is possible to work around > > > the problem by adding MS HID to the Linux i2c-scmi driver. > > > > > > Signed-off-by: Edgar Cherkasov <echerkasov@dev.rtsoft.ru> > > > Signed-off-by: Michael Brunner <Michael.Brunner@kontron.com> > > > Acked-by: Viktor Krasnov <vkrasnov@dev.rtsoft.ru> > > > --- > > > drivers/i2c/busses/i2c-scmi.c | 1 + > > > include/acpi/acpi_drivers.h | 2 ++ > > > 2 files changed, 3 insertions(+) > > > > > > diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c > > > index dfc98df..fb9aee0 100644 > > > --- a/drivers/i2c/busses/i2c-scmi.c > > > +++ b/drivers/i2c/busses/i2c-scmi.c > > > @@ -51,6 +51,7 @@ struct acpi_smbus_cmi { > > > static const struct acpi_device_id acpi_smbus_cmi_ids[] = { > > > {"SMBUS01", (kernel_ulong_t)&smbus_methods}, > > > {ACPI_SMBUS_IBM_HID, (kernel_ulong_t)&ibm_smbus_methods}, > > > + {ACPI_SMBUS_MS_HID, (kernel_ulong_t)&smbus_methods}, > > > {"", 0} > > > }; > > > MODULE_DEVICE_TABLE(acpi, acpi_smbus_cmi_ids); > > > diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h > > > index 29c6912..d34538b 100644 > > > --- a/include/acpi/acpi_drivers.h > > > +++ b/include/acpi/acpi_drivers.h > > > @@ -60,6 +60,8 @@ > > > #define ACPI_DOCK_HID "LNXDOCK" > > > /* Quirk for broken IBM BIOSes */ > > > #define ACPI_SMBUS_IBM_HID "SMBUSIBM" > > > +/* SMBUS HID definition as supported by Microsoft Windows */ > > > +#define ACPI_SMBUS_MS_HID "SMB0001" I agree with the reasoning but why do you add the ID here also? Wouldn't it suffice if added only to the i2c-scmi.c driver? ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] i2c: i2c-scmi: add a MS HID 2017-03-31 7:20 ` Mika Westerberg @ 2017-03-31 7:31 ` Viktor Krasnov 2017-03-31 7:47 ` Mika Westerberg 0 siblings, 1 reply; 9+ messages in thread From: Viktor Krasnov @ 2017-03-31 7:31 UTC (permalink / raw) To: Mika Westerberg Cc: Wolfram Sang, jdelvare, linux-i2c, Edgar Cherkasov, Michael Brunner, linux-acpi, Andy Shevchenko, Jarkko Nikula Hello Mika, thanks for the review. > > --- a/include/acpi/acpi_drivers.h > > +++ b/include/acpi/acpi_drivers.h > > @@ -60,6 +60,8 @@ > > #define ACPI_DOCK_HID "LNXDOCK" > > /* Quirk for broken IBM BIOSes */ > > #define ACPI_SMBUS_IBM_HID "SMBUSIBM" > > +/* SMBUS HID definition as supported by Microsoft Windows */ > > +#define ACPI_SMBUS_MS_HID "SMB0001" > I agree with the reasoning but why do you add the ID here also? Wouldn't > it suffice if added only to the i2c-scmi.c driver? This has been done by analogy with existing ACPI_SMBUS_IBM_HID which is defined in acpi_drivers.h. I think that it may look confusing when some HIDs are defined in i2c-scmi.c and others - in acpi_drivers.h. Could you please clarify this moment and confirm that all HIDs must be defined only in i2c-scmi.c? I will rework the patch then. Best regards, Viktor Krasnov. On Fri, 2017-03-31 at 10:20 +0300, Mika Westerberg wrote: > On Thu, Mar 30, 2017 at 05:52:33PM +0200, Wolfram Sang wrote: > > On Thu, Mar 23, 2017 at 09:47:47PM +0100, Wolfram Sang wrote: > > > On Fri, Mar 10, 2017 at 04:58:35PM +0300, Viktor Krasnov wrote: > > > > > > Thanks for the patch! > > > > > > adding linux-acpi to CC. This is more an ACPI question than I2C. > > > > Mika? Andy? Jarkko? Any input on this? > > > > > > > > > From: Edgar Cherkasov <echerkasov@dev.rtsoft.ru> > > > > > > > > Description of the problem: > > > > - i2c-scmi driver contains only two identifiers "SMBUS01" and "SMBUSIBM"; > > > > - the fist HID (SMBUS01) is clearly defined in "SMBus Control Method > > > > Interface Specification, version 1.0": "Each device must specify > > > > 'SMBUS01' as its _HID and use a unique _UID value"; > > > > - unfortunately, BIOS vendors (like AMI) seem to ignore this requirement > > > > and implement "SMB0001" HID instead of "SMBUS01"; > > > > - I speculate that they do this because only "SMB0001" is hard coded in > > > > Windows SMBus driver produced by Microsoft. > > > > > > > > This leads to following situation: > > > > - SMBus works out of box in Windows but not in Linux; > > > > - board vendors are forced to add correct "SMBUS01" HID to BIOS to make > > > > SMBus work in Linux. Moreover the same board vendors complain that > > > > tools (3-rd party ASL compiler) do not like the "SMBUS01" identifier > > > > and produce errors. So they need to constantly patch the compiler for > > > > each new version of BIOS. > > > > > > > > As it is very unlikely that BIOS vendors implement a correct HID in > > > > future, I would propose to consider whether it is possible to work around > > > > the problem by adding MS HID to the Linux i2c-scmi driver. > > > > > > > > Signed-off-by: Edgar Cherkasov <echerkasov@dev.rtsoft.ru> > > > > Signed-off-by: Michael Brunner <Michael.Brunner@kontron.com> > > > > Acked-by: Viktor Krasnov <vkrasnov@dev.rtsoft.ru> > > > > --- > > > > drivers/i2c/busses/i2c-scmi.c | 1 + > > > > include/acpi/acpi_drivers.h | 2 ++ > > > > 2 files changed, 3 insertions(+) > > > > > > > > diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c > > > > index dfc98df..fb9aee0 100644 > > > > --- a/drivers/i2c/busses/i2c-scmi.c > > > > +++ b/drivers/i2c/busses/i2c-scmi.c > > > > @@ -51,6 +51,7 @@ struct acpi_smbus_cmi { > > > > static const struct acpi_device_id acpi_smbus_cmi_ids[] = { > > > > {"SMBUS01", (kernel_ulong_t)&smbus_methods}, > > > > {ACPI_SMBUS_IBM_HID, (kernel_ulong_t)&ibm_smbus_methods}, > > > > + {ACPI_SMBUS_MS_HID, (kernel_ulong_t)&smbus_methods}, > > > > {"", 0} > > > > }; > > > > MODULE_DEVICE_TABLE(acpi, acpi_smbus_cmi_ids); > > > > diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h > > > > index 29c6912..d34538b 100644 > > > > --- a/include/acpi/acpi_drivers.h > > > > +++ b/include/acpi/acpi_drivers.h > > > > @@ -60,6 +60,8 @@ > > > > #define ACPI_DOCK_HID "LNXDOCK" > > > > /* Quirk for broken IBM BIOSes */ > > > > #define ACPI_SMBUS_IBM_HID "SMBUSIBM" > > > > +/* SMBUS HID definition as supported by Microsoft Windows */ > > > > +#define ACPI_SMBUS_MS_HID "SMB0001" > > I agree with the reasoning but why do you add the ID here also? Wouldn't > it suffice if added only to the i2c-scmi.c driver? ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] i2c: i2c-scmi: add a MS HID 2017-03-31 7:31 ` Viktor Krasnov @ 2017-03-31 7:47 ` Mika Westerberg 2017-03-31 8:04 ` Viktor Krasnov 0 siblings, 1 reply; 9+ messages in thread From: Mika Westerberg @ 2017-03-31 7:47 UTC (permalink / raw) To: Viktor Krasnov Cc: Wolfram Sang, jdelvare, linux-i2c, Edgar Cherkasov, Michael Brunner, linux-acpi, Andy Shevchenko, Jarkko Nikula On Fri, Mar 31, 2017 at 10:31:07AM +0300, Viktor Krasnov wrote: > Hello Mika, > > thanks for the review. > > > > --- a/include/acpi/acpi_drivers.h > > > +++ b/include/acpi/acpi_drivers.h > > > @@ -60,6 +60,8 @@ > > > #define ACPI_DOCK_HID "LNXDOCK" > > > /* Quirk for broken IBM BIOSes */ > > > #define ACPI_SMBUS_IBM_HID "SMBUSIBM" > > > +/* SMBUS HID definition as supported by Microsoft Windows */ > > > +#define ACPI_SMBUS_MS_HID "SMB0001" > > > I agree with the reasoning but why do you add the ID here also? Wouldn't > > it suffice if added only to the i2c-scmi.c driver? > > This has been done by analogy with existing ACPI_SMBUS_IBM_HID which is > defined in acpi_drivers.h. I think that it may look confusing when some > HIDs are defined in i2c-scmi.c and others - in acpi_drivers.h. Could you > please clarify this moment and confirm that all HIDs must be defined > only in i2c-scmi.c? I will rework the patch then. If there is only one file using a HID, then I think it is fine to keep it in the driver itself. If multiple files share the HID then it makes sense to put it to include/acpi/acpi_drivers.h. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] i2c: i2c-scmi: add a MS HID 2017-03-31 7:47 ` Mika Westerberg @ 2017-03-31 8:04 ` Viktor Krasnov 0 siblings, 0 replies; 9+ messages in thread From: Viktor Krasnov @ 2017-03-31 8:04 UTC (permalink / raw) To: Mika Westerberg Cc: Wolfram Sang, jdelvare, linux-i2c, Edgar Cherkasov, Michael Brunner, linux-acpi, Andy Shevchenko, Jarkko Nikula Hello Mika, > If there is only one file using a HID, then I think it is fine to keep > it in the driver itself. If multiple files share the HID then it makes > sense to put it to include/acpi/acpi_drivers.h. Understood, thanks for the detailed explanation. Indeed, ACPI_SMBUS_IBM_HID is referenced in several files, so it explains why this HID is defined in include/acpi/acpi_drivers.h and not in i2c-scmi.c itself. Ok, I will correct the patch, retest it and resend in next few days. Best regards, Viktor Krasnov. On Fri, 2017-03-31 at 10:47 +0300, Mika Westerberg wrote: > On Fri, Mar 31, 2017 at 10:31:07AM +0300, Viktor Krasnov wrote: > > Hello Mika, > > > > thanks for the review. > > > > > > --- a/include/acpi/acpi_drivers.h > > > > +++ b/include/acpi/acpi_drivers.h > > > > @@ -60,6 +60,8 @@ > > > > #define ACPI_DOCK_HID "LNXDOCK" > > > > /* Quirk for broken IBM BIOSes */ > > > > #define ACPI_SMBUS_IBM_HID "SMBUSIBM" > > > > +/* SMBUS HID definition as supported by Microsoft Windows */ > > > > +#define ACPI_SMBUS_MS_HID "SMB0001" > > > > > I agree with the reasoning but why do you add the ID here also? Wouldn't > > > it suffice if added only to the i2c-scmi.c driver? > > > > This has been done by analogy with existing ACPI_SMBUS_IBM_HID which is > > defined in acpi_drivers.h. I think that it may look confusing when some > > HIDs are defined in i2c-scmi.c and others - in acpi_drivers.h. Could you > > please clarify this moment and confirm that all HIDs must be defined > > only in i2c-scmi.c? I will rework the patch then. > > If there is only one file using a HID, then I think it is fine to keep > it in the driver itself. If multiple files share the HID then it makes > sense to put it to include/acpi/acpi_drivers.h. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] i2c: i2c-scmi: add a MS HID 2017-03-30 15:52 ` Wolfram Sang 2017-03-31 7:20 ` Mika Westerberg @ 2017-03-31 10:01 ` Andy Shevchenko 1 sibling, 0 replies; 9+ messages in thread From: Andy Shevchenko @ 2017-03-31 10:01 UTC (permalink / raw) To: Wolfram Sang, Viktor Krasnov Cc: jdelvare, linux-i2c, Edgar Cherkasov, Michael Brunner, linux-acpi, Mika Westerberg, Jarkko Nikula On Thu, 2017-03-30 at 17:52 +0200, Wolfram Sang wrote: > On Thu, Mar 23, 2017 at 09:47:47PM +0100, Wolfram Sang wrote: > > On Fri, Mar 10, 2017 at 04:58:35PM +0300, Viktor Krasnov wrote: > > > Signed-off-by: Edgar Cherkasov <echerkasov@dev.rtsoft.ru> > > > Signed-off-by: Michael Brunner <Michael.Brunner@kontron.com> > > > Acked-by: Viktor Krasnov <vkrasnov@dev.rtsoft.ru> A bit of offtopic here, but I have noticed kontron.com address in SoB lines. I would like to know if Kontron will follow ACPI spec when assembling firmwares for its board [1]? Table name LPW8 for SSDT is not allowed by spec and can't be just decompiled without hacking either table or decompiler. [1] http://www.kontron.com/products/boards-and-standard-form-factors/sma rc/smarc-sxbt.html P.S. We may continue privately to not contaminate mailing list with unnecessary noise. -- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] i2c: i2c-scmi: add a MS HID 2017-03-10 13:58 [PATCH] i2c: i2c-scmi: add a MS HID Viktor Krasnov 2017-03-23 20:47 ` Wolfram Sang @ 2017-03-24 9:16 ` Jean Delvare 1 sibling, 0 replies; 9+ messages in thread From: Jean Delvare @ 2017-03-24 9:16 UTC (permalink / raw) To: Viktor Krasnov; +Cc: wsa, linux-i2c, Edgar Cherkasov, Michael Brunner On Fri, 10 Mar 2017 16:58:35 +0300, Viktor Krasnov wrote: > From: Edgar Cherkasov <echerkasov@dev.rtsoft.ru> > > Description of the problem: > - i2c-scmi driver contains only two identifiers "SMBUS01" and "SMBUSIBM"; > - the fist HID (SMBUS01) is clearly defined in "SMBus Control Method > Interface Specification, version 1.0": "Each device must specify > 'SMBUS01' as its _HID and use a unique _UID value"; > - unfortunately, BIOS vendors (like AMI) seem to ignore this requirement > and implement "SMB0001" HID instead of "SMBUS01"; > - I speculate that they do this because only "SMB0001" is hard coded in > Windows SMBus driver produced by Microsoft. > > This leads to following situation: > - SMBus works out of box in Windows but not in Linux; > - board vendors are forced to add correct "SMBUS01" HID to BIOS to make > SMBus work in Linux. Moreover the same board vendors complain that > tools (3-rd party ASL compiler) do not like the "SMBUS01" identifier > and produce errors. So they need to constantly patch the compiler for > each new version of BIOS. > > As it is very unlikely that BIOS vendors implement a correct HID in > future, I would propose to consider whether it is possible to work around > the problem by adding MS HID to the Linux i2c-scmi driver. > > Signed-off-by: Edgar Cherkasov <echerkasov@dev.rtsoft.ru> > Signed-off-by: Michael Brunner <Michael.Brunner@kontron.com> > Acked-by: Viktor Krasnov <vkrasnov@dev.rtsoft.ru> > --- > drivers/i2c/busses/i2c-scmi.c | 1 + > include/acpi/acpi_drivers.h | 2 ++ > 2 files changed, 3 insertions(+) > > diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c > index dfc98df..fb9aee0 100644 > --- a/drivers/i2c/busses/i2c-scmi.c > +++ b/drivers/i2c/busses/i2c-scmi.c > @@ -51,6 +51,7 @@ struct acpi_smbus_cmi { > static const struct acpi_device_id acpi_smbus_cmi_ids[] = { > {"SMBUS01", (kernel_ulong_t)&smbus_methods}, > {ACPI_SMBUS_IBM_HID, (kernel_ulong_t)&ibm_smbus_methods}, > + {ACPI_SMBUS_MS_HID, (kernel_ulong_t)&smbus_methods}, > {"", 0} > }; > MODULE_DEVICE_TABLE(acpi, acpi_smbus_cmi_ids); > diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h > index 29c6912..d34538b 100644 > --- a/include/acpi/acpi_drivers.h > +++ b/include/acpi/acpi_drivers.h > @@ -60,6 +60,8 @@ > #define ACPI_DOCK_HID "LNXDOCK" > /* Quirk for broken IBM BIOSes */ > #define ACPI_SMBUS_IBM_HID "SMBUSIBM" > +/* SMBUS HID definition as supported by Microsoft Windows */ > +#define ACPI_SMBUS_MS_HID "SMB0001" > > /* > * For fixed hardware buttons, we fabricate acpi_devices with HID No objection from me. Reviewed-by: Jean Delvare <jdelvare@suse.de> -- Jean Delvare SUSE L3 Support ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-03-31 10:02 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-03-10 13:58 [PATCH] i2c: i2c-scmi: add a MS HID Viktor Krasnov 2017-03-23 20:47 ` Wolfram Sang 2017-03-30 15:52 ` Wolfram Sang 2017-03-31 7:20 ` Mika Westerberg 2017-03-31 7:31 ` Viktor Krasnov 2017-03-31 7:47 ` Mika Westerberg 2017-03-31 8:04 ` Viktor Krasnov 2017-03-31 10:01 ` Andy Shevchenko 2017-03-24 9:16 ` Jean Delvare
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.