[PATCH 1/2] i2c: add SMBus Host Notify support

2015-06-23 Thread Benjamin Tissoires
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

2015-06-23 Thread Benjamin Tissoires
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

2015-06-23 Thread Benjamin Tissoires
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

2015-06-23 Thread Wolfram Sang
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

2015-06-23 Thread Wolfram Sang
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

2015-06-23 Thread Lucas De Marchi
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

2015-06-23 Thread christian . ruppert
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