[PATCH 1/2] i2c: add SMBus Host Notify support
SMBus Host Notify allows a slave device to act as a master on a bus to notify the host of an interrupt. The functionality is directly implemented in the firmware so we just export a toggle function and rely on the adapter to actually support this. Signed-off-by: Benjamin Tissoires --- Documentation/i2c/smbus-protocol | 4 drivers/i2c/i2c-core.c | 26 ++ include/linux/i2c.h | 8 include/uapi/linux/i2c.h | 1 + 4 files changed, 39 insertions(+) diff --git a/Documentation/i2c/smbus-protocol b/Documentation/i2c/smbus-protocol index 6012b12..af4e4b9 100644 --- a/Documentation/i2c/smbus-protocol +++ b/Documentation/i2c/smbus-protocol @@ -199,6 +199,10 @@ alerting device's address. [S] [HostAddr] [Wr] A [DevAddr] A [DataLow] A [DataHigh] A [P] +I2C drivers for devices which can trigger SMBus Host Notify should call +i2c_smbus_toggle_host_notify() to enable SMBUS Host Notify on the adapter +and implement the optional alert() callback. + Packet Error Checking (PEC) === diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index 987c124..eaaf5b0 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -2959,6 +2959,32 @@ int i2c_slave_unregister(struct i2c_client *client) EXPORT_SYMBOL_GPL(i2c_slave_unregister); #endif +int i2c_smbus_toggle_host_notify(struct i2c_client *client, bool state) +{ + int ret; + + if (!client) + return -EINVAL; + + if (!(client->flags & I2C_CLIENT_TEN)) { + /* Enforce stricter address checking */ + ret = i2c_check_addr_validity(client->addr); + if (ret) + return ret; + } + + if (!client->adapter->algo->toggle_smbus_host_notify) + return -EOPNOTSUPP; + + i2c_lock_adapter(client->adapter); + ret = client->adapter->algo->toggle_smbus_host_notify(client->adapter, + state); + i2c_unlock_adapter(client->adapter); + + return ret; +} +EXPORT_SYMBOL_GPL(i2c_smbus_toggle_host_notify); + MODULE_AUTHOR("Simon G. Vogl "); MODULE_DESCRIPTION("I2C-Bus main module"); MODULE_LICENSE("GPL"); diff --git a/include/linux/i2c.h b/include/linux/i2c.h index e83a738..7ffc970 100644 --- a/include/linux/i2c.h +++ b/include/linux/i2c.h @@ -177,6 +177,8 @@ struct i2c_driver { * The format and meaning of the data value depends on the protocol. * For the SMBus alert protocol, there is a single bit of data passed * as the alert response's low bit ("event flag"). +* For the SMBus Host Notify protocol, the data corresponds to the +* 16-bits payload data reported by the external device. */ void (*alert)(struct i2c_client *, unsigned int data); @@ -270,6 +272,9 @@ static inline int i2c_slave_event(struct i2c_client *client, } #endif + +extern int i2c_smbus_toggle_host_notify(struct i2c_client *client, bool state); + /** * struct i2c_board_info - template for device creation * @type: chip type, to initialize i2c_client.name @@ -378,6 +383,8 @@ i2c_register_board_info(int busnum, struct i2c_board_info const *info, * from the I2C_FUNC_* flags. * @reg_slave: Register given client to I2C slave mode of this adapter * @unreg_slave: Unregister given client from I2C slave mode of this adapter + * @toggle_smbus_host_notify: toggle the SMBus Host Notify support of this + * adapter. * * The following structs are for those who like to implement new bus drivers: * i2c_algorithm is the interface to a class of hardware solutions which can @@ -408,6 +415,7 @@ struct i2c_algorithm { int (*reg_slave)(struct i2c_client *client); int (*unreg_slave)(struct i2c_client *client); #endif + int (*toggle_smbus_host_notify)(struct i2c_adapter *adap, bool state); }; /** diff --git a/include/uapi/linux/i2c.h b/include/uapi/linux/i2c.h index 0e949cb..c72708e 100644 --- a/include/uapi/linux/i2c.h +++ b/include/uapi/linux/i2c.h @@ -100,6 +100,7 @@ struct i2c_msg { #define I2C_FUNC_SMBUS_WRITE_BLOCK_DATA 0x0200 #define I2C_FUNC_SMBUS_READ_I2C_BLOCK 0x0400 /* I2C-like block xfer */ #define I2C_FUNC_SMBUS_WRITE_I2C_BLOCK 0x0800 /* w/ 1-byte reg. addr. */ +#define I2C_FUNC_SMBUS_HOST_NOTIFY 0x1000 #define I2C_FUNC_SMBUS_BYTE(I2C_FUNC_SMBUS_READ_BYTE | \ I2C_FUNC_SMBUS_WRITE_BYTE) -- 2.4.3 -- 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
[PATCH 0/2] I2C/SMBus: add support for Host Notify in i2c_i801
Hi, I am currently working on the Synaptics touchpads that can be found on many Lenovo laptops. These touchpads are capable of talking over the PS/2 bus but also over SMBus. The PS/2 bus is somewhat limited in what it allows to do and using the SMBus protocol would allow us to be close to what the Windows driver implements (and thus benefit from the firmware QA that is made for this system). The current WIP is posted here: https://github.com/bentiss/linux branch synaptics-rmi4-smbus-v4.1-15-06-23 We are still missing a few bits here and there to actually switch the default to this driver (PS/2 pass-through for example), so I am splitting the series in the hope of receiving feedbacks earlier. The Synaptics touchpads are using the SMBus Host Notification feature which an implementation is proposed in this series. I am not sure if the Host Notification feature can be emulated by all controllers so I based my work on what is available on the i801 PCH: an hardware feature introduced in ICH 3. Any comments welcome! Cheers, Benjamin Benjamin Tissoires (2): i2c: add SMBus Host Notify support i2c: i801: add support of Host Notify Documentation/i2c/smbus-protocol | 4 + drivers/i2c/busses/i2c-i801.c| 223 +++ drivers/i2c/i2c-core.c | 60 +++ drivers/i2c/i2c-smbus.c | 17 +-- include/linux/i2c.h | 11 ++ include/uapi/linux/i2c.h | 1 + 6 files changed, 256 insertions(+), 60 deletions(-) -- 2.4.3 -- 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
[PATCH 2/2] i2c: i801: add support of Host Notify
The i801 chip can handle the Host Notify feature since ICH 3 as mentioned in http://www.intel.com/content/dam/doc/datasheet/82801ca-io-controller-hub-3-datasheet.pdf Implement .toggle_smbus_host_notify() and rely on .alert() to notify the actual client that it has a notification. With a T440s and a Synaptics touchpad that implements Host Notify, the payload data is always 0x, so I am not sure if the device actually sends the payload or if there is a problem regarding the implementation. Part of this code is inspired by i2c-smbus.c. Signed-off-by: Benjamin Tissoires --- drivers/i2c/busses/i2c-i801.c | 223 +- drivers/i2c/i2c-core.c| 34 +++ drivers/i2c/i2c-smbus.c | 17 +--- include/linux/i2c.h | 3 + 4 files changed, 217 insertions(+), 60 deletions(-) diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c index 5ecbb3f..22a3218 100644 --- a/drivers/i2c/busses/i2c-i801.c +++ b/drivers/i2c/busses/i2c-i801.c @@ -20,46 +20,46 @@ /* * Supports the following Intel I/O Controller Hubs (ICH): * - * I/O Block I2C - * region SMBus Block proc. block - * Chip name PCI ID sizePEC buffer callread - * --- - * 82801AA (ICH) 0x2413 16 no no no no - * 82801AB (ICH0) 0x2423 16 no no no no - * 82801BA (ICH2) 0x2443 16 no no no no - * 82801CA (ICH3) 0x2483 32 softno no no - * 82801DB (ICH4) 0x24c3 32 hardyes no no - * 82801E (ICH5) 0x24d3 32 hardyes yes yes - * 6300ESB 0x25a4 32 hardyes yes yes - * 82801F (ICH6) 0x266a 32 hardyes yes yes - * 6310ESB/6320ESB 0x269b 32 hardyes yes yes - * 82801G (ICH7) 0x27da 32 hardyes yes yes - * 82801H (ICH8) 0x283e 32 hardyes yes yes - * 82801I (ICH9) 0x2930 32 hardyes yes yes - * EP80579 (Tolapai) 0x5032 32 hardyes yes yes - * ICH10 0x3a30 32 hardyes yes yes - * ICH10 0x3a60 32 hardyes yes yes - * 5/3400 Series (PCH) 0x3b30 32 hardyes yes yes - * 6 Series (PCH) 0x1c22 32 hardyes yes yes - * Patsburg (PCH) 0x1d22 32 hardyes yes yes - * Patsburg (PCH) IDF 0x1d70 32 hardyes yes yes - * Patsburg (PCH) IDF 0x1d71 32 hardyes yes yes - * Patsburg (PCH) IDF 0x1d72 32 hardyes yes yes - * DH89xxCC (PCH) 0x2330 32 hardyes yes yes - * Panther Point (PCH) 0x1e22 32 hardyes yes yes - * Lynx Point (PCH)0x8c22 32 hardyes yes yes - * Lynx Point-LP (PCH) 0x9c22 32 hardyes yes yes - * Avoton (SOC)0x1f3c 32 hardyes yes yes - * Wellsburg (PCH) 0x8d22 32 hardyes yes yes - * Wellsburg (PCH) MS 0x8d7d 32 hardyes yes yes - * Wellsburg (PCH) MS 0x8d7e 32 hardyes yes yes - * Wellsburg (PCH) MS 0x8d7f 32 hardyes yes yes - * Coleto Creek (PCH) 0x23b0 32 hardyes yes yes - * Wildcat Point (PCH) 0x8ca2 32 hardyes yes yes - * Wildcat Point-LP (PCH) 0x9ca2 32 hardyes yes yes - * BayTrail (SOC) 0x0f12 32 hardyes yes yes - * Sunrise Point-H (PCH) 0xa123 32 hardyes yes yes - * Sunrise Point-LP (PCH) 0x9d23 32 hardyes yes yes + * I/O SMBus Block I2C + * region Slave HostSMBus Block proc. block + * Chip name PCI ID sizemodenotify PEC buffer callread + * -- + * 82801AA (ICH) 0x2413 16 no no no no no no + * 82801AB (ICH0) 0x2423 16 no no no no no no + * 82801BA (ICH2) 0x2443 16 yes no no no no no + * 82801CA (ICH3) 0x2483 32 yes yes softno no no + * 82801DB (ICH4) 0x24c3 32 yes yes hardyes no no + * 82801E (ICH5
Re: [PATCH v5] i2c: busses: i2c-bcm2835: limits cdiv to allowed values
On Thu, Jun 18, 2015 at 11:10:11AM +0200, Silvan Wicki wrote: > Checks if the cdiv value is in between min (0x2) and max (0xFFFE) > supported values by the bcm2835. If not, it returns -ENODEV. > > See page 33/34 of BCM2835-ARM-Peripherals.pdf for the DIV register. > https://www.raspberrypi.org/wp-content/uploads/2012/02/BCM2835-ARM-Peripherals.pdf > > Signed-off-by: Silvan Wicki Applied to for-next, thanks! But I needed to rebase on top of your "i2c: bcm2835: clear reserved bits in S-Register" patch. A small patch series would have been a good choice here. signature.asc Description: Digital signature
Re: [PATCH 0/4] generic timeout handling for Renesas drivers
On Sat, Jun 20, 2015 at 09:03:18PM +0200, Wolfram Sang wrote: > Here is a small patch series to make I2C timeout handling easier for users. It > is not so amazingly huge anymore and it can be modified via i2c-dev if wanted. > While at it, fix the type of the timeout variable to the proper one. > This time build-tested twice! > > Wolfram Sang (4): > i2c: rcar: use adapter default for timeout > i2c: rcar: use proper type for timeout > i2c: sh_mobile: use adapter default for timeout > i2c: sh_mobile: use proper type for timeout Applied to for-next, thanks! signature.asc Description: Digital signature
Re: [PATCH] i2c: designware: use enable on resume instead initialization
On Tue, Jun 23, 2015 at 1:45 PM, wrote: > Hello, > > Christian Ruppert/ALi_GVA/ALi wrote on 10.06.2015 17:05:16: >> Mika Westerberg wrote on 10.06. >> 2015 09:07:22: >> > On Tue, Jun 09, 2015 at 03:29:01PM -0300, Lucas De Marchi wrote: >> > > Hi Mika, >> > > >> > > On Tue, Jun 9, 2015 at 5:51 AM, Mika Westerberg >> > > wrote: >> > > > On Mon, Jun 08, 2015 at 02:50:28PM -0300, >> lucas.de.mar...@gmail.com wrote: >> > > >> From: Fabio Mello >> > > >> >> > > >> According to documentation and tests, initialization is not >> > > >> necessary on module resume, since the controller keeps its state >> > > >> between disable/enable. Change the target address is also > allowed. >> > > >> >> > > >> So, this patch replaces the initialization on module resume with > a >> > > >> simple enable, and removes the (non required anymore) enables and >> > > >> disables. >> > > >> >> > > >> Signed-off-by: Fabio Mello >> > > >> Signed-off-by: Lucas De Marchi >> > > >> --- >> > > >> >> > > >> These pictures explain a little more the consequence of letting > the >> > > >> enable+disable in the code: >> > > >> >> > > >> http://pub.politreco.com/paste/TEK0011-before.jpg >> > > >> http://pub.politreco.com/paste/TEK0007-after.jpg >> > > >> >> > > >> The yellow line is a GPIO toggle in userspace to mark when we >> > start and finish >> > > >> the i2c transactions. The blue line is the SCL in that i2c >> > bus. Take a look on >> > > >> the huge pauses we have between any 2 transactions. These >> > pauses are removed >> > > >> with this patch and we are able to read our sensor's values in >> > 950usec rather >> > > >> than 5.24msec we had before. We are testing this using a >> > Minnowboard Max that >> > > >> has a designware i2c controller. >> > > > >> > > > Did you test this on any other platform than Intel Baytrail? >> > > >> > > No. The only soc we have here with this controller is the Baytrail. >> > >> > My concern is that this patch might break some non-Intel platform. It >> > would be nice if someone (Christian?) could try this out. >> >> Ouch, this one brings back painful memories. Take a look at patch >> 38d7fadef4973bb94e36897fcb6bb6a12fdd10c9 (https://git.kernel.org/ >> cgit/linux/kernel/git/torvalds/linux.git/commit/? >> id=38d7fadef4973bb94e36897fcb6bb6a12fdd10c9) for the context. >> >> In brief: >> - Before patch 38d7fade, the driver disabled the hardware after >> successful transfers. I do not know the reason for this and I cannot >> judge whether this is necessary or not. I thus decided not to modify >> this behaviour in patch 38d7fade. >> >> - After patch 38d7fade, the driver disabled the dw controller after >> all transfers, in particular in the case of unsuccessful transfers. >> This modification was necessary because of a race condition >> triggered by an i2c slave device which interrupted transfers in the >> middle. In this case, the dw controller (at least our version) seems >> to send spurious interrupts later if it is not disabled. The >> interrupt handler is not designed to be called on already aborted >> transfers, however, which leads to undesirable behaviour if the >> interrupt occurs at the wrong moment (system hangs in irq loop). >> >> I will try to dig out the test setup we used to validate the patch >> at the time but given the fact that this was two years ago this >> might take a little while. In the meantime, do you have any means to >> stress test the case of unexpected events on the bus (client aborts >> transfer, timeout etc.)? An alternative might be to only disable the >> controller in the case of errors and leave it active after >> successful transfers. We should understand why the controller was >> disabled after successful transfers in the first place, however. >> Maybe some quirk with older versions of the hardware? Mika, do you >> have any memories about this? > > As promised I tried to dig out the test setup we used to validate patch > 38d7fade at the time but without success. (I half expected that after such > a long time...) > > So I said to myself, let's give the patch a try nevertheless and see if it > works in our system at least in the nominal case (no i2c bus errors). > > The result is not very encouraging: Out of five (identical) designware i2c > controllers we have on my test SOC, the first one initialises properly but > the second one gets stuck in the famous irq loop right away when the > module is enabled in i2c_dw_init. The system never gets around to try Are you using the pci or platform driver? I noticed yesterday the pci version is failing here with a NULL pointer dereference. > initialising the remaining three controllers due to the irq loop. I didn't > have the time to investigate the details yet but I suspect this is > triggered by some nastily behaved device on the bus. For example, some of > our external devices are notorious for keeping i2c lines tied to zero > before being properly powered on/reset/initialised by their respective > drivers (whic
Re: [PATCH] i2c: designware: use enable on resume instead initialization
Hello, Christian Ruppert/ALi_GVA/ALi wrote on 10.06.2015 17:05:16: > Mika Westerberg wrote on 10.06. > 2015 09:07:22: > > On Tue, Jun 09, 2015 at 03:29:01PM -0300, Lucas De Marchi wrote: > > > Hi Mika, > > > > > > On Tue, Jun 9, 2015 at 5:51 AM, Mika Westerberg > > > wrote: > > > > On Mon, Jun 08, 2015 at 02:50:28PM -0300, > lucas.de.mar...@gmail.com wrote: > > > >> From: Fabio Mello > > > >> > > > >> According to documentation and tests, initialization is not > > > >> necessary on module resume, since the controller keeps its state > > > >> between disable/enable. Change the target address is also allowed. > > > >> > > > >> So, this patch replaces the initialization on module resume with a > > > >> simple enable, and removes the (non required anymore) enables and > > > >> disables. > > > >> > > > >> Signed-off-by: Fabio Mello > > > >> Signed-off-by: Lucas De Marchi > > > >> --- > > > >> > > > >> These pictures explain a little more the consequence of letting the > > > >> enable+disable in the code: > > > >> > > > >> http://pub.politreco.com/paste/TEK0011-before.jpg > > > >> http://pub.politreco.com/paste/TEK0007-after.jpg > > > >> > > > >> The yellow line is a GPIO toggle in userspace to mark when we > > start and finish > > > >> the i2c transactions. The blue line is the SCL in that i2c > > bus. Take a look on > > > >> the huge pauses we have between any 2 transactions. These > > pauses are removed > > > >> with this patch and we are able to read our sensor's values in > > 950usec rather > > > >> than 5.24msec we had before. We are testing this using a > > Minnowboard Max that > > > >> has a designware i2c controller. > > > > > > > > Did you test this on any other platform than Intel Baytrail? > > > > > > No. The only soc we have here with this controller is the Baytrail. > > > > My concern is that this patch might break some non-Intel platform. It > > would be nice if someone (Christian?) could try this out. > > Ouch, this one brings back painful memories. Take a look at patch > 38d7fadef4973bb94e36897fcb6bb6a12fdd10c9 (https://git.kernel.org/ > cgit/linux/kernel/git/torvalds/linux.git/commit/? > id=38d7fadef4973bb94e36897fcb6bb6a12fdd10c9) for the context. > > In brief: > - Before patch 38d7fade, the driver disabled the hardware after > successful transfers. I do not know the reason for this and I cannot > judge whether this is necessary or not. I thus decided not to modify > this behaviour in patch 38d7fade. > > - After patch 38d7fade, the driver disabled the dw controller after > all transfers, in particular in the case of unsuccessful transfers. > This modification was necessary because of a race condition > triggered by an i2c slave device which interrupted transfers in the > middle. In this case, the dw controller (at least our version) seems > to send spurious interrupts later if it is not disabled. The > interrupt handler is not designed to be called on already aborted > transfers, however, which leads to undesirable behaviour if the > interrupt occurs at the wrong moment (system hangs in irq loop). > > I will try to dig out the test setup we used to validate the patch > at the time but given the fact that this was two years ago this > might take a little while. In the meantime, do you have any means to > stress test the case of unexpected events on the bus (client aborts > transfer, timeout etc.)? An alternative might be to only disable the > controller in the case of errors and leave it active after > successful transfers. We should understand why the controller was > disabled after successful transfers in the first place, however. > Maybe some quirk with older versions of the hardware? Mika, do you > have any memories about this? As promised I tried to dig out the test setup we used to validate patch 38d7fade at the time but without success. (I half expected that after such a long time...) So I said to myself, let's give the patch a try nevertheless and see if it works in our system at least in the nominal case (no i2c bus errors). The result is not very encouraging: Out of five (identical) designware i2c controllers we have on my test SOC, the first one initialises properly but the second one gets stuck in the famous irq loop right away when the module is enabled in i2c_dw_init. The system never gets around to try initialising the remaining three controllers due to the irq loop. I didn't have the time to investigate the details yet but I suspect this is triggered by some nastily behaved device on the bus. For example, some of our external devices are notorious for keeping i2c lines tied to zero before being properly powered on/reset/initialised by their respective drivers (which in turn depend on the i2c master to be initialised first, obviously). I'll grab an oscilloscope and dump the waves to confirm this suspicion on the occasion. However, similar situations might occur in multi-master setups (which we don't have). I