Hi Prarit, On Wed, 9 Apr 2014 12:22:43 -0400, Prarit Bhargava wrote: > RFC and a work in progress ... I need to go through and do a bunch of error > condition checking, more testing, etc. I'm just throwing this out there to see > if anyone has any major concerns about doing something like this. > > TODO: > > - decipher error returns from ACPI AML operations (and implement those too) > - additional error checking from i2c and acpi function calls (this code is > not robust enough) > - testing > > P. > > ----8<---- > > Some ACPI tables on new systems implement an SBUS device in ACPI. This results > in a conflict with the ACPI tables and the i2c-i801 driver over the address > space. As a result the i2c-i801 driver will not load. To workaround this, we > have users use the kernel parameter "acpi_enforce_resources=lax". This, > isn't a good solution as I've seen other conflicts on some systems that are > also overriden. I thought about implementing an i2c-core kernel parameter and > a wrapper function for acpi_check_resource_conflict() but that seems rather > clunky and doesn't resolve the issue of the shared resource between the ACPI > and the device. > > There isn't any documentaton on Intel's website about the SBUS device but from > reading the acpidump it looks like the operations are straightforward. > > SSXB > transmit one byte > SRXB > receive one byte > SWRB > write one byte > SRDB > read one byte > SWRW > write one word > SRDW > read one word > SBLW > write block > SBRW > read block > > I've implemented these as an i2c algorithm so that users who cannot load the > regular i801 i2c driver can at least get the functionality they are looking > for. Writing a driver for the ACPI interface to the SMBus is IMHO a very good idea, thanks for doing that. However I have two technical concerns with your approach: 1* What you wrote is NOT an "i2c algo". i2c "algorithms" are abstracted I2C implementations, not correlated with any hardware or BIOS specific interfaces. If you check other drivers under i2c/algos you'll see they do NOT call i2c_add_adapter (although they may implement helper wrappers around that function.) Hardware-specific drivers which build on top of the algo driver are responsible for that. What you wrote is much more like i2c/busses/i2c-scmi.c, so it is actually an i2c _bus_ driver. 2* You are creating a link between i2c-i801 and your driver, where IMHO there should not be any. Both drivers are independent, and from the kernel's perspective, they are not even for the same hardware. In your case the ACPI interface is backed by an i801-like chip, but the same interface could totally be implemented on top of any other chip. So I invite you to make your driver an independent i2c bus driver much like i2c-scmi. Thanks, -- Jean Delvare SUSE L3 Support -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
