Linux I2C development
 help / color / mirror / Atom feed
* Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on Dell machines
From: Wolfram Sang @ 2017-01-04 17:46 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Pali Rohár, Dmitry Torokhov, Michał Kępień,
	Jean Delvare, Steven Honeyman, Valdis.Kletnieks, Jochen Eisinger,
	Gabriele Mazzotta, Andy Lutomirski, Mario_Limonciello, Alex Hung,
	Takashi Iwai, linux-i2c, linux-kernel, platform-driver-x86
In-Reply-To: <20170104173928.GW5767@mail.corp.redhat.com>


> How about:
> ---
> From daa7571bbf337704332c0cfeec9b8fd5aeae596f Mon Sep 17 00:00:00 2001
> From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Date: Wed, 4 Jan 2017 18:26:54 +0100
> Subject: [PATCH] I2C: add the source of the IRQ in struct i2c_client
> 
> With commit 4d5538f5882a ("i2c: use an IRQ to report Host Notify events,
> not alert"), the IRQ provided in struct i2c_client might be assigned while
> it has not been explicitly declared by either the platform information
> or OF or ACPI.
> Some drivers (lis3lv02d) rely on the fact that the IRQ gets assigned or
> not to trigger a different behavior (exposing /dev/freefall in this case).
> 
> Provide a way for others to know who set the IRQ and so they can behave
> accordingly.
> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
>  drivers/i2c/i2c-core.c |  7 +++++++
>  include/linux/i2c.h    | 11 +++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index cf9e396..226c75d 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -935,8 +935,12 @@ static int i2c_device_probe(struct device *dev)
>  			irq = of_irq_get_byname(dev->of_node, "irq");
>  			if (irq == -EINVAL || irq == -ENODATA)
>  				irq = of_irq_get(dev->of_node, 0);
> +			if (irq > 0)
> +				client->irq_source = I2C_IRQ_SOURCE_OF;
>  		} else if (ACPI_COMPANION(dev)) {
>  			irq = acpi_dev_gpio_irq_get(ACPI_COMPANION(dev), 0);
> +			if (irq > 0)
> +				client->irq_source = I2C_IRQ_SOURCE_ACPI;
>  		}
>  		if (irq == -EPROBE_DEFER)
>  			return irq;
> @@ -947,6 +951,8 @@ static int i2c_device_probe(struct device *dev)
>  		if (irq < 0) {
>  			dev_dbg(dev, "Using Host Notify IRQ\n");
>  			irq = i2c_smbus_host_notify_to_irq(client);
> +			if (irq > 0)
> +				client->irq_source = I2C_IRQ_SOURCE_HOST_NOTIFY;
>  		}
>  		if (irq < 0)
>  			irq = 0;
> @@ -1317,6 +1323,7 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
>  	client->flags = info->flags;
>  	client->addr = info->addr;
>  	client->irq = info->irq;
> +	client->irq_source = I2C_IRQ_SOURCE_PLATFORM;
>  
>  	strlcpy(client->name, info->type, sizeof(client->name));
>  
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index b2109c5..7d0368d 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -213,6 +213,13 @@ struct i2c_driver {
>  };
>  #define to_i2c_driver(d) container_of(d, struct i2c_driver, driver)
>  
> +enum i2c_irq_source {
> +	I2C_IRQ_SOURCE_PLATFORM,
> +	I2C_IRQ_SOURCE_OF,
> +	I2C_IRQ_SOURCE_ACPI,
> +	I2C_IRQ_SOURCE_HOST_NOTIFY,
> +};
> +
>  /**
>   * struct i2c_client - represent an I2C slave device
>   * @flags: I2C_CLIENT_TEN indicates the device uses a ten bit chip address;
> @@ -227,6 +234,9 @@ struct i2c_driver {
>   *	userspace_devices list
>   * @slave_cb: Callback when I2C slave mode of an adapter is used. The adapter
>   *	calls it to pass on slave events to the slave driver.
> + * @irq_source: Enum which provides the source of the IRQ. Useful to know
> + * 	if the IRQ was issued from Host Notify or if it was provided by an other
> + * 	component.

I'd think some documentation somewhere makes sense why we need to
distinguish this in some cases?

>   *
>   * An i2c_client identifies a single device (i.e. chip) connected to an
>   * i2c bus. The behaviour exposed to Linux is defined by the driver
> @@ -245,6 +255,7 @@ struct i2c_client {
>  #if IS_ENABLED(CONFIG_I2C_SLAVE)
>  	i2c_slave_cb_t slave_cb;	/* callback for slave mode	*/
>  #endif
> +	enum i2c_irq_source irq_source;	/* which component assigned the irq */
>  };
>  #define to_i2c_client(d) container_of(d, struct i2c_client, dev)
> 
> Dmitry, Wolfram, Jean, would this be acceptable for you?

Adding something to i2c_driver is not exactly cheap, but from what I
glimpsed from this thread, this is one of the cleanest solution to this
problem?

^ permalink raw reply

* Re: [PATCH] iio: adc: axp288: Drop bogus AXP288_ADC_TS_PIN_CTRL register modifications
From: Jacob Pan @ 2017-01-04 17:55 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Jonathan Cameron, Chen-Yu Tsai, Lars-Peter Clausen,
	Peter Meerwald-Stadler, russianneuromancer @ ya . ru, linux-iio,
	linux-i2c, jacob.jun.pan
In-Reply-To: <221c8f4e-0757-19bb-90db-4e34677734c5@redhat.com>

On Tue, 3 Jan 2017 23:10:57 +0100
Hans de Goede <hdegoede@redhat.com> wrote:

> > It could have been a quirk we had to do on our platforms, I just
> > cannot recall the details. Are you testing this on x86 platforms?  
> 
> Yes, I've successfully tested this on 2 different models cherrytrail
> tablets.

[Jacob Pan] I am ok with your change. I last tested on baytrail
tablets, I don't know if you have one available to verify. That would
be ideal.

Thanks,

^ permalink raw reply

* Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on Dell machines
From: Dmitry Torokhov @ 2017-01-04 17:54 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Benjamin Tissoires, Pali Rohár, Michał Kępień,
	Jean Delvare, Steven Honeyman, Valdis.Kletnieks, Jochen Eisinger,
	Gabriele Mazzotta, Andy Lutomirski, Mario_Limonciello, Alex Hung,
	Takashi Iwai, linux-i2c, linux-kernel, platform-driver-x86
In-Reply-To: <20170104174619.feke27q3ehwkylfs@ninjato>

On Wed, Jan 04, 2017 at 06:46:19PM +0100, Wolfram Sang wrote:
> 
> > How about:
> > ---
> > From daa7571bbf337704332c0cfeec9b8fd5aeae596f Mon Sep 17 00:00:00 2001
> > From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > Date: Wed, 4 Jan 2017 18:26:54 +0100
> > Subject: [PATCH] I2C: add the source of the IRQ in struct i2c_client
> > 
> > With commit 4d5538f5882a ("i2c: use an IRQ to report Host Notify events,
> > not alert"), the IRQ provided in struct i2c_client might be assigned while
> > it has not been explicitly declared by either the platform information
> > or OF or ACPI.
> > Some drivers (lis3lv02d) rely on the fact that the IRQ gets assigned or
> > not to trigger a different behavior (exposing /dev/freefall in this case).
> > 
> > Provide a way for others to know who set the IRQ and so they can behave
> > accordingly.
> > 
> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > ---
> >  drivers/i2c/i2c-core.c |  7 +++++++
> >  include/linux/i2c.h    | 11 +++++++++++
> >  2 files changed, 18 insertions(+)
> > 
> > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> > index cf9e396..226c75d 100644
> > --- a/drivers/i2c/i2c-core.c
> > +++ b/drivers/i2c/i2c-core.c
> > @@ -935,8 +935,12 @@ static int i2c_device_probe(struct device *dev)
> >  			irq = of_irq_get_byname(dev->of_node, "irq");
> >  			if (irq == -EINVAL || irq == -ENODATA)
> >  				irq = of_irq_get(dev->of_node, 0);
> > +			if (irq > 0)
> > +				client->irq_source = I2C_IRQ_SOURCE_OF;
> >  		} else if (ACPI_COMPANION(dev)) {
> >  			irq = acpi_dev_gpio_irq_get(ACPI_COMPANION(dev), 0);
> > +			if (irq > 0)
> > +				client->irq_source = I2C_IRQ_SOURCE_ACPI;
> >  		}
> >  		if (irq == -EPROBE_DEFER)
> >  			return irq;
> > @@ -947,6 +951,8 @@ static int i2c_device_probe(struct device *dev)
> >  		if (irq < 0) {
> >  			dev_dbg(dev, "Using Host Notify IRQ\n");
> >  			irq = i2c_smbus_host_notify_to_irq(client);
> > +			if (irq > 0)
> > +				client->irq_source = I2C_IRQ_SOURCE_HOST_NOTIFY;
> >  		}
> >  		if (irq < 0)
> >  			irq = 0;
> > @@ -1317,6 +1323,7 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
> >  	client->flags = info->flags;
> >  	client->addr = info->addr;
> >  	client->irq = info->irq;
> > +	client->irq_source = I2C_IRQ_SOURCE_PLATFORM;
> >  
> >  	strlcpy(client->name, info->type, sizeof(client->name));
> >  
> > diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> > index b2109c5..7d0368d 100644
> > --- a/include/linux/i2c.h
> > +++ b/include/linux/i2c.h
> > @@ -213,6 +213,13 @@ struct i2c_driver {
> >  };
> >  #define to_i2c_driver(d) container_of(d, struct i2c_driver, driver)
> >  
> > +enum i2c_irq_source {
> > +	I2C_IRQ_SOURCE_PLATFORM,
> > +	I2C_IRQ_SOURCE_OF,
> > +	I2C_IRQ_SOURCE_ACPI,
> > +	I2C_IRQ_SOURCE_HOST_NOTIFY,
> > +};
> > +
> >  /**
> >   * struct i2c_client - represent an I2C slave device
> >   * @flags: I2C_CLIENT_TEN indicates the device uses a ten bit chip address;
> > @@ -227,6 +234,9 @@ struct i2c_driver {
> >   *	userspace_devices list
> >   * @slave_cb: Callback when I2C slave mode of an adapter is used. The adapter
> >   *	calls it to pass on slave events to the slave driver.
> > + * @irq_source: Enum which provides the source of the IRQ. Useful to know
> > + * 	if the IRQ was issued from Host Notify or if it was provided by an other
> > + * 	component.
> 
> I'd think some documentation somewhere makes sense why we need to
> distinguish this in some cases?

I'd rather drivers be oblivious of the source of interrupt. If they need
to distinguish between them that means that our IRQ abstration failed.

> 
> >   *
> >   * An i2c_client identifies a single device (i.e. chip) connected to an
> >   * i2c bus. The behaviour exposed to Linux is defined by the driver
> > @@ -245,6 +255,7 @@ struct i2c_client {
> >  #if IS_ENABLED(CONFIG_I2C_SLAVE)
> >  	i2c_slave_cb_t slave_cb;	/* callback for slave mode	*/
> >  #endif
> > +	enum i2c_irq_source irq_source;	/* which component assigned the irq */
> >  };
> >  #define to_i2c_client(d) container_of(d, struct i2c_client, dev)
> > 
> > Dmitry, Wolfram, Jean, would this be acceptable for you?
> 
> Adding something to i2c_driver is not exactly cheap, but from what I
> glimpsed from this thread, this is one of the cleanest solution to this
> problem?
> 

As Benjamin said, it is really property of device [instance], not
driver. I.e. driver could handle both wired IRQ and HostNotify-based
scheme similarly, it is device (and board) that knows how stuff is
connected.

Maybe we could do something like this (untested):


>From e362a0277fd1bd6112f258664d8831d9bc6b78da Mon Sep 17 00:00:00 2001
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: Wed, 4 Jan 2017 09:33:43 -0800
Subject: [PATCH] i2c: do not enable fall back to Host Notify by default

Falling back unconditionally to HostNotify as primary client's interrupt
breaks some drivers which alter their functionality depending on whether
interrupt is present or not, so let's introduce a board flag telling I2C
core explicitly if we want wired interrupt or HostNotify-based one:
I2C_CLIENT_HOST_NOTIFY.

For DT-based systems we introduce "host-notofy" property that we convert
to I2C_CLIENT_HOST_NOTIFY board flag.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 Documentation/devicetree/bindings/i2c/i2c.txt |  8 ++++++++
 drivers/i2c/i2c-core.c                        | 17 ++++++++---------
 include/linux/i2c.h                           |  1 +
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
index 5fa691e6f638..cee9d5055fa2 100644
--- a/Documentation/devicetree/bindings/i2c/i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c.txt
@@ -62,6 +62,9 @@ wants to support one of the below features, it should adapt the bindings below.
 	"irq" and "wakeup" names are recognized by I2C core, other names are
 	left to individual drivers.
 
+- host-notify
+	device uses SMBus host notify protocol instead of interrupt line.
+
 - multi-master
 	states that there is another master active on this bus. The OS can use
 	this information to adapt power management to keep the arbitration awake
@@ -81,6 +84,11 @@ Binding may contain optional "interrupts" property, describing interrupts
 used by the device. I2C core will assign "irq" interrupt (or the very first
 interrupt if not using interrupt names) as primary interrupt for the slave.
 
+Alternatively, devices supporting SMbus Host Notify, and connected to
+adapters that support this feature, may use "host-notify" property. I2C
+core will create a virtual interrupt for Host Notify and assign it as
+primary interrupt for the slave.
+
 Also, if device is marked as a wakeup source, I2C core will set up "wakeup"
 interrupt for the device. If "wakeup" interrupt name is not present in the
 binding, then primary interrupt will be used as wakeup interrupt.
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index cf9e396d7702..250969fa7670 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -931,7 +931,10 @@ static int i2c_device_probe(struct device *dev)
 	if (!client->irq) {
 		int irq = -ENOENT;
 
-		if (dev->of_node) {
+		if (client->flags & I2C_CLIENT_HOST_HOTIFY) {
+			dev_dbg(dev, "Using Host Notify IRQ\n");
+			irq = i2c_smbus_host_notify_to_irq(client);
+		} else if (dev->of_node) {
 			irq = of_irq_get_byname(dev->of_node, "irq");
 			if (irq == -EINVAL || irq == -ENODATA)
 				irq = of_irq_get(dev->of_node, 0);
@@ -940,14 +943,7 @@ static int i2c_device_probe(struct device *dev)
 		}
 		if (irq == -EPROBE_DEFER)
 			return irq;
-		/*
-		 * ACPI and OF did not find any useful IRQ, try to see
-		 * if Host Notify can be used.
-		 */
-		if (irq < 0) {
-			dev_dbg(dev, "Using Host Notify IRQ\n");
-			irq = i2c_smbus_host_notify_to_irq(client);
-		}
+
 		if (irq < 0)
 			irq = 0;
 
@@ -1716,6 +1712,9 @@ static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
 	info.of_node = of_node_get(node);
 	info.archdata = &dev_ad;
 
+	if (of_read_property_bool(node, "host-notify"))
+		info.flags |= I2C_CLIENT_HOST_NOTIFY;
+
 	if (of_get_property(node, "wakeup-source", NULL))
 		info.flags |= I2C_CLIENT_WAKE;
 
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index b2109c522dec..4b45ec46161f 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -665,6 +665,7 @@ i2c_unlock_adapter(struct i2c_adapter *adapter)
 #define I2C_CLIENT_TEN		0x10	/* we have a ten bit chip address */
 					/* Must equal I2C_M_TEN below */
 #define I2C_CLIENT_SLAVE	0x20	/* we are the slave */
+#define I2C_CLIENT_HOST_NOTIFY	0x40	/* We want to use I2C host notify */
 #define I2C_CLIENT_WAKE		0x80	/* for board_info; true iff can wake */
 #define I2C_CLIENT_SCCB		0x9000	/* Use Omnivision SCCB protocol */
 					/* Must match I2C_M_STOP|IGNORE_NAK */
-- 
2.11.0.390.gc69c2f50cf-goog


-- 
Dmitry

^ permalink raw reply related

* Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on Dell machines
From: Jean Delvare @ 2017-01-04 18:50 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Pali Rohár, Benjamin Tissoires, Michał Kępień,
	Wolfram Sang, Steven Honeyman, Valdis.Kletnieks, Jochen Eisinger,
	Gabriele Mazzotta, Andy Lutomirski, Mario_Limonciello, Alex Hung,
	Takashi Iwai, linux-i2c, linux-kernel, platform-driver-x86
In-Reply-To: <20170103205937.GA2289@dtor-ws>

On Tue, 3 Jan 2017 12:59:37 -0800, Dmitry Torokhov wrote:
> On Tue, Jan 03, 2017 at 09:39:13PM +0100, Pali Rohár wrote:
> > Some distributions blacklist i2c-i801.ko module... And 
> 
> Any particular reason for that?

At some point in time, the i2c-i801 driver caused problems on a few
systems. They decided that blacklisting the driver for everybody was
the easiest way to fix the problem. Of course this doesn't make any
sense and should be reverted. The root cause of the problem should have
been investigated back then. Maybe it's fixed by now and they will
never now...

-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply

* Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on Dell machines
From: Benjamin Tissoires @ 2017-01-04 21:49 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Wolfram Sang, Pali Rohár, Michał Kępień,
	Jean Delvare, Steven Honeyman, Valdis.Kletnieks, Jochen Eisinger,
	Gabriele Mazzotta, Andy Lutomirski, Mario_Limonciello, Alex Hung,
	Takashi Iwai, linux-i2c, linux-kernel, platform-driver-x86
In-Reply-To: <20170104175456.GA40341@dtor-ws>

On Jan 04 2017 or thereabouts, Dmitry Torokhov wrote:
> On Wed, Jan 04, 2017 at 06:46:19PM +0100, Wolfram Sang wrote:
> > 
> > > How about:
> > > ---
> > > From daa7571bbf337704332c0cfeec9b8fd5aeae596f Mon Sep 17 00:00:00 2001
> > > From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > > Date: Wed, 4 Jan 2017 18:26:54 +0100
> > > Subject: [PATCH] I2C: add the source of the IRQ in struct i2c_client
> > > 
> > > With commit 4d5538f5882a ("i2c: use an IRQ to report Host Notify events,
> > > not alert"), the IRQ provided in struct i2c_client might be assigned while
> > > it has not been explicitly declared by either the platform information
> > > or OF or ACPI.
> > > Some drivers (lis3lv02d) rely on the fact that the IRQ gets assigned or
> > > not to trigger a different behavior (exposing /dev/freefall in this case).
> > > 
> > > Provide a way for others to know who set the IRQ and so they can behave
> > > accordingly.
> > > 
> > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > > ---
> > >  drivers/i2c/i2c-core.c |  7 +++++++
> > >  include/linux/i2c.h    | 11 +++++++++++
> > >  2 files changed, 18 insertions(+)
> > > 
> > > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> > > index cf9e396..226c75d 100644
> > > --- a/drivers/i2c/i2c-core.c
> > > +++ b/drivers/i2c/i2c-core.c
> > > @@ -935,8 +935,12 @@ static int i2c_device_probe(struct device *dev)
> > >  			irq = of_irq_get_byname(dev->of_node, "irq");
> > >  			if (irq == -EINVAL || irq == -ENODATA)
> > >  				irq = of_irq_get(dev->of_node, 0);
> > > +			if (irq > 0)
> > > +				client->irq_source = I2C_IRQ_SOURCE_OF;
> > >  		} else if (ACPI_COMPANION(dev)) {
> > >  			irq = acpi_dev_gpio_irq_get(ACPI_COMPANION(dev), 0);
> > > +			if (irq > 0)
> > > +				client->irq_source = I2C_IRQ_SOURCE_ACPI;
> > >  		}
> > >  		if (irq == -EPROBE_DEFER)
> > >  			return irq;
> > > @@ -947,6 +951,8 @@ static int i2c_device_probe(struct device *dev)
> > >  		if (irq < 0) {
> > >  			dev_dbg(dev, "Using Host Notify IRQ\n");
> > >  			irq = i2c_smbus_host_notify_to_irq(client);
> > > +			if (irq > 0)
> > > +				client->irq_source = I2C_IRQ_SOURCE_HOST_NOTIFY;
> > >  		}
> > >  		if (irq < 0)
> > >  			irq = 0;
> > > @@ -1317,6 +1323,7 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
> > >  	client->flags = info->flags;
> > >  	client->addr = info->addr;
> > >  	client->irq = info->irq;
> > > +	client->irq_source = I2C_IRQ_SOURCE_PLATFORM;
> > >  
> > >  	strlcpy(client->name, info->type, sizeof(client->name));
> > >  
> > > diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> > > index b2109c5..7d0368d 100644
> > > --- a/include/linux/i2c.h
> > > +++ b/include/linux/i2c.h
> > > @@ -213,6 +213,13 @@ struct i2c_driver {
> > >  };
> > >  #define to_i2c_driver(d) container_of(d, struct i2c_driver, driver)
> > >  
> > > +enum i2c_irq_source {
> > > +	I2C_IRQ_SOURCE_PLATFORM,
> > > +	I2C_IRQ_SOURCE_OF,
> > > +	I2C_IRQ_SOURCE_ACPI,
> > > +	I2C_IRQ_SOURCE_HOST_NOTIFY,
> > > +};
> > > +
> > >  /**
> > >   * struct i2c_client - represent an I2C slave device
> > >   * @flags: I2C_CLIENT_TEN indicates the device uses a ten bit chip address;
> > > @@ -227,6 +234,9 @@ struct i2c_driver {
> > >   *	userspace_devices list
> > >   * @slave_cb: Callback when I2C slave mode of an adapter is used. The adapter
> > >   *	calls it to pass on slave events to the slave driver.
> > > + * @irq_source: Enum which provides the source of the IRQ. Useful to know
> > > + * 	if the IRQ was issued from Host Notify or if it was provided by an other
> > > + * 	component.
> > 
> > I'd think some documentation somewhere makes sense why we need to
> > distinguish this in some cases?
> 
> I'd rather drivers be oblivious of the source of interrupt. If they need
> to distinguish between them that means that our IRQ abstration failed.
> 
> > 
> > >   *
> > >   * An i2c_client identifies a single device (i.e. chip) connected to an
> > >   * i2c bus. The behaviour exposed to Linux is defined by the driver
> > > @@ -245,6 +255,7 @@ struct i2c_client {
> > >  #if IS_ENABLED(CONFIG_I2C_SLAVE)
> > >  	i2c_slave_cb_t slave_cb;	/* callback for slave mode	*/
> > >  #endif
> > > +	enum i2c_irq_source irq_source;	/* which component assigned the irq */
> > >  };
> > >  #define to_i2c_client(d) container_of(d, struct i2c_client, dev)
> > > 
> > > Dmitry, Wolfram, Jean, would this be acceptable for you?
> > 
> > Adding something to i2c_driver is not exactly cheap, but from what I
> > glimpsed from this thread, this is one of the cleanest solution to this
> > problem?
> > 
> 
> As Benjamin said, it is really property of device [instance], not
> driver. I.e. driver could handle both wired IRQ and HostNotify-based
> scheme similarly, it is device (and board) that knows how stuff is
> connected.
> 
> Maybe we could do something like this (untested):
> 
> 
> From e362a0277fd1bd6112f258664d8831d9bc6b78da Mon Sep 17 00:00:00 2001
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Date: Wed, 4 Jan 2017 09:33:43 -0800
> Subject: [PATCH] i2c: do not enable fall back to Host Notify by default
> 
> Falling back unconditionally to HostNotify as primary client's interrupt
> breaks some drivers which alter their functionality depending on whether
> interrupt is present or not, so let's introduce a board flag telling I2C
> core explicitly if we want wired interrupt or HostNotify-based one:
> I2C_CLIENT_HOST_NOTIFY.
> 
> For DT-based systems we introduce "host-notofy" property that we convert

typo: s/host-notofy/host-notify/

> to I2C_CLIENT_HOST_NOTIFY board flag.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  Documentation/devicetree/bindings/i2c/i2c.txt |  8 ++++++++
>  drivers/i2c/i2c-core.c                        | 17 ++++++++---------
>  include/linux/i2c.h                           |  1 +
>  3 files changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
> index 5fa691e6f638..cee9d5055fa2 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c.txt
> @@ -62,6 +62,9 @@ wants to support one of the below features, it should adapt the bindings below.
>  	"irq" and "wakeup" names are recognized by I2C core, other names are
>  	left to individual drivers.
>  
> +- host-notify
> +	device uses SMBus host notify protocol instead of interrupt line.
> +
>  - multi-master
>  	states that there is another master active on this bus. The OS can use
>  	this information to adapt power management to keep the arbitration awake
> @@ -81,6 +84,11 @@ Binding may contain optional "interrupts" property, describing interrupts
>  used by the device. I2C core will assign "irq" interrupt (or the very first
>  interrupt if not using interrupt names) as primary interrupt for the slave.
>  
> +Alternatively, devices supporting SMbus Host Notify, and connected to
> +adapters that support this feature, may use "host-notify" property. I2C
> +core will create a virtual interrupt for Host Notify and assign it as
> +primary interrupt for the slave.
> +
>  Also, if device is marked as a wakeup source, I2C core will set up "wakeup"
>  interrupt for the device. If "wakeup" interrupt name is not present in the
>  binding, then primary interrupt will be used as wakeup interrupt.
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index cf9e396d7702..250969fa7670 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -931,7 +931,10 @@ static int i2c_device_probe(struct device *dev)
>  	if (!client->irq) {
>  		int irq = -ENOENT;
>  
> -		if (dev->of_node) {
> +		if (client->flags & I2C_CLIENT_HOST_HOTIFY) {

typo: s/I2C_CLIENT_HOST_HOTIFY/I2C_CLIENT_HOST_NOTIFY/

With these fixed, the code is:
Tested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

I tested both with and without the I2C_CLIENT_HOST_HOTIFY flag on the
Thinkpad T450s, and everything is in order.

Thanks Dmitry for the patch!

Cheers,
Benjamin


> +			dev_dbg(dev, "Using Host Notify IRQ\n");
> +			irq = i2c_smbus_host_notify_to_irq(client);
> +		} else if (dev->of_node) {
>  			irq = of_irq_get_byname(dev->of_node, "irq");
>  			if (irq == -EINVAL || irq == -ENODATA)
>  				irq = of_irq_get(dev->of_node, 0);
> @@ -940,14 +943,7 @@ static int i2c_device_probe(struct device *dev)
>  		}
>  		if (irq == -EPROBE_DEFER)
>  			return irq;
> -		/*
> -		 * ACPI and OF did not find any useful IRQ, try to see
> -		 * if Host Notify can be used.
> -		 */
> -		if (irq < 0) {
> -			dev_dbg(dev, "Using Host Notify IRQ\n");
> -			irq = i2c_smbus_host_notify_to_irq(client);
> -		}
> +
>  		if (irq < 0)
>  			irq = 0;
>  
> @@ -1716,6 +1712,9 @@ static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
>  	info.of_node = of_node_get(node);
>  	info.archdata = &dev_ad;
>  
> +	if (of_read_property_bool(node, "host-notify"))
> +		info.flags |= I2C_CLIENT_HOST_NOTIFY;
> +
>  	if (of_get_property(node, "wakeup-source", NULL))
>  		info.flags |= I2C_CLIENT_WAKE;
>  
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index b2109c522dec..4b45ec46161f 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -665,6 +665,7 @@ i2c_unlock_adapter(struct i2c_adapter *adapter)
>  #define I2C_CLIENT_TEN		0x10	/* we have a ten bit chip address */
>  					/* Must equal I2C_M_TEN below */
>  #define I2C_CLIENT_SLAVE	0x20	/* we are the slave */
> +#define I2C_CLIENT_HOST_NOTIFY	0x40	/* We want to use I2C host notify */
>  #define I2C_CLIENT_WAKE		0x80	/* for board_info; true iff can wake */
>  #define I2C_CLIENT_SCCB		0x9000	/* Use Omnivision SCCB protocol */
>  					/* Must match I2C_M_STOP|IGNORE_NAK */
> -- 
> 2.11.0.390.gc69c2f50cf-goog
> 
> 
> -- 
> Dmitry

^ permalink raw reply

* Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on Dell machines
From: Wolfram Sang @ 2017-01-04 21:55 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Benjamin Tissoires, Pali Rohár, Michał Kępień,
	Jean Delvare, Steven Honeyman, Valdis.Kletnieks, Jochen Eisinger,
	Gabriele Mazzotta, Andy Lutomirski, Mario_Limonciello, Alex Hung,
	Takashi Iwai, linux-i2c, linux-kernel, platform-driver-x86
In-Reply-To: <20170104175456.GA40341@dtor-ws>

[-- Attachment #1: Type: text/plain, Size: 1259 bytes --]


> From e362a0277fd1bd6112f258664d8831d9bc6b78da Mon Sep 17 00:00:00 2001
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Date: Wed, 4 Jan 2017 09:33:43 -0800
> Subject: [PATCH] i2c: do not enable fall back to Host Notify by default
> 
> Falling back unconditionally to HostNotify as primary client's interrupt
> breaks some drivers which alter their functionality depending on whether
> interrupt is present or not, so let's introduce a board flag telling I2C
> core explicitly if we want wired interrupt or HostNotify-based one:
> I2C_CLIENT_HOST_NOTIFY.
> 
> For DT-based systems we introduce "host-notofy" property that we convert
> to I2C_CLIENT_HOST_NOTIFY board flag.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Yay, this looks better to me. One nit:

> +Alternatively, devices supporting SMbus Host Notify, and connected to
> +adapters that support this feature, may use "host-notify" property. I2C
> +core will create a virtual interrupt for Host Notify and assign it as
> +primary interrupt for the slave.

This paragraph sounds Linux-ish while binding docs should be OS
agnostic. Maybe we can shorten the second sentence to "It will be
assigned then as the primary interrupt for the slave."?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* [PATCH] i2c: do not enable fall back to Host Notify by default
From: Dmitry Torokhov @ 2017-01-04 21:58 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Rob Herring, Benjamin Tissoires, Pali Rohár,
	Michał Kępień, Jean Delvare, Takashi Iwai,
	linux-i2c, devicetree, linux-kernel

Falling back unconditionally to HostNotify as primary client's interrupt
breaks some drivers which alter their functionality depending on whether
interrupt is present or not, so let's introduce a board flag telling I2C
core explicitly if we want wired interrupt or HostNotify-based one:
I2C_CLIENT_HOST_NOTIFY.

For DT-based systems we introduce "host-notify" property that we convert
to I2C_CLIENT_HOST_NOTIFY board flag.

Tested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 Documentation/devicetree/bindings/i2c/i2c.txt |  8 ++++++++
 drivers/i2c/i2c-core.c                        | 17 ++++++++---------
 include/linux/i2c.h                           |  1 +
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
index 5fa691e6f638..cee9d5055fa2 100644
--- a/Documentation/devicetree/bindings/i2c/i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c.txt
@@ -62,6 +62,9 @@ wants to support one of the below features, it should adapt the bindings below.
 	"irq" and "wakeup" names are recognized by I2C core, other names are
 	left to individual drivers.
 
+- host-notify
+	device uses SMBus host notify protocol instead of interrupt line.
+
 - multi-master
 	states that there is another master active on this bus. The OS can use
 	this information to adapt power management to keep the arbitration awake
@@ -81,6 +84,11 @@ Binding may contain optional "interrupts" property, describing interrupts
 used by the device. I2C core will assign "irq" interrupt (or the very first
 interrupt if not using interrupt names) as primary interrupt for the slave.
 
+Alternatively, devices supporting SMbus Host Notify, and connected to
+adapters that support this feature, may use "host-notify" property. I2C
+core will create a virtual interrupt for Host Notify and assign it as
+primary interrupt for the slave.
+
 Also, if device is marked as a wakeup source, I2C core will set up "wakeup"
 interrupt for the device. If "wakeup" interrupt name is not present in the
 binding, then primary interrupt will be used as wakeup interrupt.
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index cf9e396d7702..fbd1db014768 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -931,7 +931,10 @@ static int i2c_device_probe(struct device *dev)
 	if (!client->irq) {
 		int irq = -ENOENT;
 
-		if (dev->of_node) {
+		if (client->flags & I2C_CLIENT_HOST_NOTIFY) {
+			dev_dbg(dev, "Using Host Notify IRQ\n");
+			irq = i2c_smbus_host_notify_to_irq(client);
+		} else if (dev->of_node) {
 			irq = of_irq_get_byname(dev->of_node, "irq");
 			if (irq == -EINVAL || irq == -ENODATA)
 				irq = of_irq_get(dev->of_node, 0);
@@ -940,14 +943,7 @@ static int i2c_device_probe(struct device *dev)
 		}
 		if (irq == -EPROBE_DEFER)
 			return irq;
-		/*
-		 * ACPI and OF did not find any useful IRQ, try to see
-		 * if Host Notify can be used.
-		 */
-		if (irq < 0) {
-			dev_dbg(dev, "Using Host Notify IRQ\n");
-			irq = i2c_smbus_host_notify_to_irq(client);
-		}
+
 		if (irq < 0)
 			irq = 0;
 
@@ -1716,6 +1712,9 @@ static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
 	info.of_node = of_node_get(node);
 	info.archdata = &dev_ad;
 
+	if (of_read_property_bool(node, "host-notify"))
+		info.flags |= I2C_CLIENT_HOST_NOTIFY;
+
 	if (of_get_property(node, "wakeup-source", NULL))
 		info.flags |= I2C_CLIENT_WAKE;
 
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index b2109c522dec..4b45ec46161f 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -665,6 +665,7 @@ i2c_unlock_adapter(struct i2c_adapter *adapter)
 #define I2C_CLIENT_TEN		0x10	/* we have a ten bit chip address */
 					/* Must equal I2C_M_TEN below */
 #define I2C_CLIENT_SLAVE	0x20	/* we are the slave */
+#define I2C_CLIENT_HOST_NOTIFY	0x40	/* We want to use I2C host notify */
 #define I2C_CLIENT_WAKE		0x80	/* for board_info; true iff can wake */
 #define I2C_CLIENT_SCCB		0x9000	/* Use Omnivision SCCB protocol */
 					/* Must match I2C_M_STOP|IGNORE_NAK */
-- 
2.11.0.390.gc69c2f50cf-goog


-- 
Dmitry

^ permalink raw reply related

* Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on Dell machines
From: Dmitry Torokhov @ 2017-01-04 22:00 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Benjamin Tissoires, Pali Rohár, Michał Kępień,
	Jean Delvare, Steven Honeyman, Valdis.Kletnieks, Jochen Eisinger,
	Gabriele Mazzotta, Andy Lutomirski, Mario_Limonciello, Alex Hung,
	Takashi Iwai, linux-i2c, linux-kernel, platform-driver-x86
In-Reply-To: <20170104215546.GA1433@katana>

On Wed, Jan 04, 2017 at 10:55:47PM +0100, Wolfram Sang wrote:
> 
> > From e362a0277fd1bd6112f258664d8831d9bc6b78da Mon Sep 17 00:00:00 2001
> > From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Date: Wed, 4 Jan 2017 09:33:43 -0800
> > Subject: [PATCH] i2c: do not enable fall back to Host Notify by default
> > 
> > Falling back unconditionally to HostNotify as primary client's interrupt
> > breaks some drivers which alter their functionality depending on whether
> > interrupt is present or not, so let's introduce a board flag telling I2C
> > core explicitly if we want wired interrupt or HostNotify-based one:
> > I2C_CLIENT_HOST_NOTIFY.
> > 
> > For DT-based systems we introduce "host-notofy" property that we convert
> > to I2C_CLIENT_HOST_NOTIFY board flag.
> > 
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> Yay, this looks better to me. One nit:
> 
> > +Alternatively, devices supporting SMbus Host Notify, and connected to
> > +adapters that support this feature, may use "host-notify" property. I2C
> > +core will create a virtual interrupt for Host Notify and assign it as
> > +primary interrupt for the slave.
> 
> This paragraph sounds Linux-ish while binding docs should be OS
> agnostic. Maybe we can shorten the second sentence to "It will be
> assigned then as the primary interrupt for the slave."?

Heh, I just sent out patch to DT folks. Provided that they are fine with
the new property I can either send V2 or you could edit when applying,
whatever is easier for you.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on Dell machines
From: Dmitry Torokhov @ 2017-01-04 21:55 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Wolfram Sang, Pali Rohár, Michał Kępień,
	Jean Delvare, Steven Honeyman, Valdis.Kletnieks, Jochen Eisinger,
	Gabriele Mazzotta, Andy Lutomirski, Mario_Limonciello, Alex Hung,
	Takashi Iwai, linux-i2c, linux-kernel, platform-driver-x86
In-Reply-To: <20170104214906.GA15702@mail.corp.redhat.com>

On Wed, Jan 04, 2017 at 10:49:06PM +0100, Benjamin Tissoires wrote:
> On Jan 04 2017 or thereabouts, Dmitry Torokhov wrote:
> > On Wed, Jan 04, 2017 at 06:46:19PM +0100, Wolfram Sang wrote:
> > > 
> > > > How about:
> > > > ---
> > > > From daa7571bbf337704332c0cfeec9b8fd5aeae596f Mon Sep 17 00:00:00 2001
> > > > From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > > > Date: Wed, 4 Jan 2017 18:26:54 +0100
> > > > Subject: [PATCH] I2C: add the source of the IRQ in struct i2c_client
> > > > 
> > > > With commit 4d5538f5882a ("i2c: use an IRQ to report Host Notify events,
> > > > not alert"), the IRQ provided in struct i2c_client might be assigned while
> > > > it has not been explicitly declared by either the platform information
> > > > or OF or ACPI.
> > > > Some drivers (lis3lv02d) rely on the fact that the IRQ gets assigned or
> > > > not to trigger a different behavior (exposing /dev/freefall in this case).
> > > > 
> > > > Provide a way for others to know who set the IRQ and so they can behave
> > > > accordingly.
> > > > 
> > > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > > > ---
> > > >  drivers/i2c/i2c-core.c |  7 +++++++
> > > >  include/linux/i2c.h    | 11 +++++++++++
> > > >  2 files changed, 18 insertions(+)
> > > > 
> > > > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> > > > index cf9e396..226c75d 100644
> > > > --- a/drivers/i2c/i2c-core.c
> > > > +++ b/drivers/i2c/i2c-core.c
> > > > @@ -935,8 +935,12 @@ static int i2c_device_probe(struct device *dev)
> > > >  			irq = of_irq_get_byname(dev->of_node, "irq");
> > > >  			if (irq == -EINVAL || irq == -ENODATA)
> > > >  				irq = of_irq_get(dev->of_node, 0);
> > > > +			if (irq > 0)
> > > > +				client->irq_source = I2C_IRQ_SOURCE_OF;
> > > >  		} else if (ACPI_COMPANION(dev)) {
> > > >  			irq = acpi_dev_gpio_irq_get(ACPI_COMPANION(dev), 0);
> > > > +			if (irq > 0)
> > > > +				client->irq_source = I2C_IRQ_SOURCE_ACPI;
> > > >  		}
> > > >  		if (irq == -EPROBE_DEFER)
> > > >  			return irq;
> > > > @@ -947,6 +951,8 @@ static int i2c_device_probe(struct device *dev)
> > > >  		if (irq < 0) {
> > > >  			dev_dbg(dev, "Using Host Notify IRQ\n");
> > > >  			irq = i2c_smbus_host_notify_to_irq(client);
> > > > +			if (irq > 0)
> > > > +				client->irq_source = I2C_IRQ_SOURCE_HOST_NOTIFY;
> > > >  		}
> > > >  		if (irq < 0)
> > > >  			irq = 0;
> > > > @@ -1317,6 +1323,7 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
> > > >  	client->flags = info->flags;
> > > >  	client->addr = info->addr;
> > > >  	client->irq = info->irq;
> > > > +	client->irq_source = I2C_IRQ_SOURCE_PLATFORM;
> > > >  
> > > >  	strlcpy(client->name, info->type, sizeof(client->name));
> > > >  
> > > > diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> > > > index b2109c5..7d0368d 100644
> > > > --- a/include/linux/i2c.h
> > > > +++ b/include/linux/i2c.h
> > > > @@ -213,6 +213,13 @@ struct i2c_driver {
> > > >  };
> > > >  #define to_i2c_driver(d) container_of(d, struct i2c_driver, driver)
> > > >  
> > > > +enum i2c_irq_source {
> > > > +	I2C_IRQ_SOURCE_PLATFORM,
> > > > +	I2C_IRQ_SOURCE_OF,
> > > > +	I2C_IRQ_SOURCE_ACPI,
> > > > +	I2C_IRQ_SOURCE_HOST_NOTIFY,
> > > > +};
> > > > +
> > > >  /**
> > > >   * struct i2c_client - represent an I2C slave device
> > > >   * @flags: I2C_CLIENT_TEN indicates the device uses a ten bit chip address;
> > > > @@ -227,6 +234,9 @@ struct i2c_driver {
> > > >   *	userspace_devices list
> > > >   * @slave_cb: Callback when I2C slave mode of an adapter is used. The adapter
> > > >   *	calls it to pass on slave events to the slave driver.
> > > > + * @irq_source: Enum which provides the source of the IRQ. Useful to know
> > > > + * 	if the IRQ was issued from Host Notify or if it was provided by an other
> > > > + * 	component.
> > > 
> > > I'd think some documentation somewhere makes sense why we need to
> > > distinguish this in some cases?
> > 
> > I'd rather drivers be oblivious of the source of interrupt. If they need
> > to distinguish between them that means that our IRQ abstration failed.
> > 
> > > 
> > > >   *
> > > >   * An i2c_client identifies a single device (i.e. chip) connected to an
> > > >   * i2c bus. The behaviour exposed to Linux is defined by the driver
> > > > @@ -245,6 +255,7 @@ struct i2c_client {
> > > >  #if IS_ENABLED(CONFIG_I2C_SLAVE)
> > > >  	i2c_slave_cb_t slave_cb;	/* callback for slave mode	*/
> > > >  #endif
> > > > +	enum i2c_irq_source irq_source;	/* which component assigned the irq */
> > > >  };
> > > >  #define to_i2c_client(d) container_of(d, struct i2c_client, dev)
> > > > 
> > > > Dmitry, Wolfram, Jean, would this be acceptable for you?
> > > 
> > > Adding something to i2c_driver is not exactly cheap, but from what I
> > > glimpsed from this thread, this is one of the cleanest solution to this
> > > problem?
> > > 
> > 
> > As Benjamin said, it is really property of device [instance], not
> > driver. I.e. driver could handle both wired IRQ and HostNotify-based
> > scheme similarly, it is device (and board) that knows how stuff is
> > connected.
> > 
> > Maybe we could do something like this (untested):
> > 
> > 
> > From e362a0277fd1bd6112f258664d8831d9bc6b78da Mon Sep 17 00:00:00 2001
> > From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Date: Wed, 4 Jan 2017 09:33:43 -0800
> > Subject: [PATCH] i2c: do not enable fall back to Host Notify by default
> > 
> > Falling back unconditionally to HostNotify as primary client's interrupt
> > breaks some drivers which alter their functionality depending on whether
> > interrupt is present or not, so let's introduce a board flag telling I2C
> > core explicitly if we want wired interrupt or HostNotify-based one:
> > I2C_CLIENT_HOST_NOTIFY.
> > 
> > For DT-based systems we introduce "host-notofy" property that we convert
> 
> typo: s/host-notofy/host-notify/
> 
> > to I2C_CLIENT_HOST_NOTIFY board flag.
> > 
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/i2c/i2c.txt |  8 ++++++++
> >  drivers/i2c/i2c-core.c                        | 17 ++++++++---------
> >  include/linux/i2c.h                           |  1 +
> >  3 files changed, 17 insertions(+), 9 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
> > index 5fa691e6f638..cee9d5055fa2 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c.txt
> > +++ b/Documentation/devicetree/bindings/i2c/i2c.txt
> > @@ -62,6 +62,9 @@ wants to support one of the below features, it should adapt the bindings below.
> >  	"irq" and "wakeup" names are recognized by I2C core, other names are
> >  	left to individual drivers.
> >  
> > +- host-notify
> > +	device uses SMBus host notify protocol instead of interrupt line.
> > +
> >  - multi-master
> >  	states that there is another master active on this bus. The OS can use
> >  	this information to adapt power management to keep the arbitration awake
> > @@ -81,6 +84,11 @@ Binding may contain optional "interrupts" property, describing interrupts
> >  used by the device. I2C core will assign "irq" interrupt (or the very first
> >  interrupt if not using interrupt names) as primary interrupt for the slave.
> >  
> > +Alternatively, devices supporting SMbus Host Notify, and connected to
> > +adapters that support this feature, may use "host-notify" property. I2C
> > +core will create a virtual interrupt for Host Notify and assign it as
> > +primary interrupt for the slave.
> > +
> >  Also, if device is marked as a wakeup source, I2C core will set up "wakeup"
> >  interrupt for the device. If "wakeup" interrupt name is not present in the
> >  binding, then primary interrupt will be used as wakeup interrupt.
> > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> > index cf9e396d7702..250969fa7670 100644
> > --- a/drivers/i2c/i2c-core.c
> > +++ b/drivers/i2c/i2c-core.c
> > @@ -931,7 +931,10 @@ static int i2c_device_probe(struct device *dev)
> >  	if (!client->irq) {
> >  		int irq = -ENOENT;
> >  
> > -		if (dev->of_node) {
> > +		if (client->flags & I2C_CLIENT_HOST_HOTIFY) {
> 
> typo: s/I2C_CLIENT_HOST_HOTIFY/I2C_CLIENT_HOST_NOTIFY/
> 
> With these fixed, the code is:
> Tested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> I tested both with and without the I2C_CLIENT_HOST_HOTIFY flag on the
> Thinkpad T450s, and everything is in order.
> 
> Thanks Dmitry for the patch!

Thanks Benjamin. Let me submit the patch "officially" and CC Rob & DT
folks on binding change.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on Dell machines
From: Dmitry Torokhov @ 2017-01-04 22:03 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Benjamin Tissoires, Pali Rohár, Michał Kępień,
	Jean Delvare, Steven Honeyman, Valdis.Kletnieks, Jochen Eisinger,
	Gabriele Mazzotta, Andy Lutomirski, Mario_Limonciello, Alex Hung,
	Takashi Iwai, linux-i2c, linux-kernel, platform-driver-x86
In-Reply-To: <20170104220030.GD40341@dtor-ws>

On Wed, Jan 04, 2017 at 02:00:30PM -0800, Dmitry Torokhov wrote:
> On Wed, Jan 04, 2017 at 10:55:47PM +0100, Wolfram Sang wrote:
> > 
> > > From e362a0277fd1bd6112f258664d8831d9bc6b78da Mon Sep 17 00:00:00 2001
> > > From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > Date: Wed, 4 Jan 2017 09:33:43 -0800
> > > Subject: [PATCH] i2c: do not enable fall back to Host Notify by default
> > > 
> > > Falling back unconditionally to HostNotify as primary client's interrupt
> > > breaks some drivers which alter their functionality depending on whether
> > > interrupt is present or not, so let's introduce a board flag telling I2C
> > > core explicitly if we want wired interrupt or HostNotify-based one:
> > > I2C_CLIENT_HOST_NOTIFY.
> > > 
> > > For DT-based systems we introduce "host-notofy" property that we convert
> > > to I2C_CLIENT_HOST_NOTIFY board flag.
> > > 
> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > 
> > Yay, this looks better to me. One nit:
> > 
> > > +Alternatively, devices supporting SMbus Host Notify, and connected to
> > > +adapters that support this feature, may use "host-notify" property. I2C
> > > +core will create a virtual interrupt for Host Notify and assign it as
> > > +primary interrupt for the slave.
> > 
> > This paragraph sounds Linux-ish while binding docs should be OS
> > agnostic. Maybe we can shorten the second sentence to "It will be
> > assigned then as the primary interrupt for the slave."?
> 
> Heh, I just sent out patch to DT folks. Provided that they are fine with
> the new property I can either send V2 or you could edit when applying,
> whatever is easier for you.

That said, both variants are Linux-ish to me: I do not believe that
"primary slave interrupt" for I2C devices is a generic concept. Is it?
Do BSD and Windows use it?

Also, it matches the paragraph above that also talks about I2C core.

In any case, it is your decision, I'm just making random noise here.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] i2c: do not enable fall back to Host Notify by default
From: kbuild test robot @ 2017-01-05  2:20 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: kbuild-all, Wolfram Sang, Benjamin Tissoires, Pali Rohár,
	Michał Kępień, Jean Delvare, Steven Honeyman,
	Valdis.Kletnieks, Jochen Eisinger, Gabriele Mazzotta,
	Andy Lutomirski, Mario_Limonciello, Alex Hung, Takashi Iwai,
	linux-i2c, linux-kernel, platform-driver-x86
In-Reply-To: <20170104175456.GA40341@dtor-ws>

[-- Attachment #1: Type: text/plain, Size: 1902 bytes --]

Hi Dmitry,

[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on v4.10-rc2 next-20170104]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Dmitry-Torokhov/i2c-do-not-enable-fall-back-to-Host-Notify-by-default/20170105-095405
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: x86_64-randconfig-x017-201701 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/i2c/i2c-core.c: In function 'i2c_device_probe':
   drivers/i2c/i2c-core.c:934:23: error: 'I2C_CLIENT_HOST_HOTIFY' undeclared (first use in this function)
      if (client->flags & I2C_CLIENT_HOST_HOTIFY) {
                          ^~~~~~~~~~~~~~~~~~~~~~
   drivers/i2c/i2c-core.c:934:23: note: each undeclared identifier is reported only once for each function it appears in
   drivers/i2c/i2c-core.c: In function 'of_i2c_register_device':
>> drivers/i2c/i2c-core.c:1715:6: error: implicit declaration of function 'of_read_property_bool' [-Werror=implicit-function-declaration]
     if (of_read_property_bool(node, "host-notify"))
         ^~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/of_read_property_bool +1715 drivers/i2c/i2c-core.c

  1709		}
  1710	
  1711		info.addr = addr;
  1712		info.of_node = of_node_get(node);
  1713		info.archdata = &dev_ad;
  1714	
> 1715		if (of_read_property_bool(node, "host-notify"))
  1716			info.flags |= I2C_CLIENT_HOST_NOTIFY;
  1717	
  1718		if (of_get_property(node, "wakeup-source", NULL))

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24760 bytes --]

^ permalink raw reply

* Re: [PATCH] i2c: do not enable fall back to Host Notify by default
From: kbuild test robot @ 2017-01-05  2:21 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: kbuild-all, Wolfram Sang, Benjamin Tissoires, Pali Rohár,
	Michał Kępień, Jean Delvare, Steven Honeyman,
	Valdis.Kletnieks, Jochen Eisinger, Gabriele Mazzotta,
	Andy Lutomirski, Mario_Limonciello, Alex Hung, Takashi Iwai,
	linux-i2c, linux-kernel, platform-driver-x86
In-Reply-To: <20170104175456.GA40341@dtor-ws>

[-- Attachment #1: Type: text/plain, Size: 1581 bytes --]

Hi Dmitry,

[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on v4.10-rc2 next-20170104]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Dmitry-Torokhov/i2c-do-not-enable-fall-back-to-Host-Notify-by-default/20170105-095405
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: x86_64-randconfig-x011-201701 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/i2c/i2c-core.c: In function 'i2c_device_probe':
>> drivers/i2c/i2c-core.c:934:23: error: 'I2C_CLIENT_HOST_HOTIFY' undeclared (first use in this function)
      if (client->flags & I2C_CLIENT_HOST_HOTIFY) {
                          ^~~~~~~~~~~~~~~~~~~~~~
   drivers/i2c/i2c-core.c:934:23: note: each undeclared identifier is reported only once for each function it appears in

vim +/I2C_CLIENT_HOST_HOTIFY +934 drivers/i2c/i2c-core.c

   928		if (!client)
   929			return 0;
   930	
   931		if (!client->irq) {
   932			int irq = -ENOENT;
   933	
 > 934			if (client->flags & I2C_CLIENT_HOST_HOTIFY) {
   935				dev_dbg(dev, "Using Host Notify IRQ\n");
   936				irq = i2c_smbus_host_notify_to_irq(client);
   937			} else if (dev->of_node) {

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24858 bytes --]

^ permalink raw reply

* Re: [PATCH 5/5] i2c: mux: pca954x: Add irq_mask_en to delay enabling irqs
From: Phil Reid @ 2017-01-05  3:31 UTC (permalink / raw)
  To: Peter Rosin, wsa-z923LK4zBo2bacvFa/9K2g,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <2bc78ee3-be23-40b8-54ca-90bb2cc05a47-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

On 4/01/2017 22:21, Peter Rosin wrote:
> On 2017-01-04 10:29, Phil Reid wrote:
>> Unfortunately some hardware device will assert their irq line immediately
>> on power on and provide no mechanism to mask the irq. As the i2c muxes
>> provide no method to mask irq line this provides a work around by keeping
>> the parent irq masked until enough device drivers have loaded to service
>> all pending interrupts.
>>
>> For example the the ltc1760 assert its SMBALERT irq immediately on power
>> on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
>> device is registered irq are enabled and fire continuously as the second
>> device driver has not yet loaded. Setting this parameter to 0x3 while
>> delay the irq being enabled until both devices are ready.
>
> I think this also needs a comment in the code, including a description
> of the limitations. If the interrupt is shared between two devices on
> the same bus, you would have the exact same problem and this workaround
> would be no good...
>
> Overall, this series fixes the issues I had with the patch from half
> a year ago or so. Thanks!
>
> With nitpicks fixed,
> Acked-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
>
G'day Pater,

Thanks for the review. v2 on it's way.


-- 
Regards
Phil Reid

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v2 0/5] i2c: mux: pca954x: Add interrupt controller support
From: Phil Reid @ 2017-01-05  4:10 UTC (permalink / raw)
  To: peda-koto5C5qi+TLoDKTGw+V6w, wsa-z923LK4zBo2bacvFa/9K2g,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Various muxes can aggregate multiple interrupts from each i2c bus.
All of the muxes with interrupt support combine the active low irq lines
using an internal 'and' function and generate a combined active low
output. The muxes do provide the ability to read a control register to
determine which irq is active. By making the mux an irq controller isr
latenct can potentially be reduced by reading the status register and 
then only calling the registered isr on that bus segment.

In addition an additional enable mask is added to work around devices
that assert irq immediately before being setup buy disabling the irq
from the mux until all devices are registered.

Changes from v1:
- Update for new ACPI table
- Fix typo in documentation
- Fix typo in function names
- Fix typo in irq name
- Added spaces around '+' / '='
- Change goto label names
- Change property name from i2c-mux-irq-mask-en to nxp,irq-mask-enable
- Change variable name irq_mask_en to irq_mask_enable
- Add commentt about irq_mask_enable
- Added Acked-By's


Phil Reid (5):
  i2c: mux: pca954x: Add missing pca9542 definition to chip_desc
  dt: bindings: i2c-mux-pca954x: Add documentation for interrupt
    controller
  i2c: mux: pca954x: Add interrupt controller support
  dt: bindings: i2c-mux-pca954x: Add documentation for
    i2c-mux-irq-mask-en
  i2c: mux: pca954x: Add irq_mask_en to delay enabling irqs

 .../devicetree/bindings/i2c/i2c-mux-pca954x.txt    |  17 ++-
 drivers/i2c/muxes/i2c-mux-pca954x.c                | 156 ++++++++++++++++++++-
 2 files changed, 168 insertions(+), 5 deletions(-)

-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v2 4/5] dt: bindings: i2c-mux-pca954x: Add documentation for i2c-mux-irq-mask-en
From: Phil Reid @ 2017-01-05  4:11 UTC (permalink / raw)
  To: peda-koto5C5qi+TLoDKTGw+V6w, wsa-z923LK4zBo2bacvFa/9K2g,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1483589463-35380-1-git-send-email-preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>

Unfortunately some hardware device will assert their irq line immediately
on power on and provide no mechanism to mask the irq. As the i2c muxes
provide no method to mask irq line this provides a work around by keeping
the parent irq masked until enough device drivers have loaded to service
all pending interrupts.

For example the the ltc1760 assert its SMBALERT irq immediately on power
on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
device is registered irq are enabled and fire continuously as the second
device driver has not yet loaded. Setting this parameter to 0x3 while
delay the irq being enabled until both devices are ready.

Acked-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
---
 Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
index aa09704..6de1e8e 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
@@ -19,6 +19,8 @@ Optional Properties:
   - i2c-mux-idle-disconnect: Boolean; if defined, forces mux to disconnect all
     children in idle state. This is necessary for example, if there are several
     multiplexers on the bus and the devices behind them use same I2C addresses.
+  - nxp,irq-mask-enable: BitMask; Defines a mask for which irq lines need to be
+    unmasked before the parent irq line in enabled.
   - interrupt-parent: Phandle for the interrupt controller that services
     interrupts for this device.
   - interrupts: Interrupt mapping for IRQ.
@@ -36,6 +38,7 @@ Example:
 		#size-cells = <0>;
 		reg = <0x74>;
 
+		nxp,irq-mask-enable = <0x3>;
 		interrupt-parent = <&ipic>;
 		interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
 		interrupt-controller;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v2 5/5] i2c: mux: pca954x: Add irq_mask_en to delay enabling irqs
From: Phil Reid @ 2017-01-05  4:11 UTC (permalink / raw)
  To: peda-koto5C5qi+TLoDKTGw+V6w, wsa-z923LK4zBo2bacvFa/9K2g,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1483589463-35380-1-git-send-email-preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>

Unfortunately some hardware device will assert their irq line immediately
on power on and provide no mechanism to mask the irq. As the i2c muxes
provide no method to mask irq line this provides a work around by keeping
the parent irq masked until enough device drivers have loaded to service
all pending interrupts.

For example the the ltc1760 assert its SMBALERT irq immediately on power
on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
device is registered irq are enabled and fire continuously as the second
device driver has not yet loaded. Setting this parameter to 0x3 while
delay the irq being enabled until both devices are ready.

Acked-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 84fc767..581a75d 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -75,6 +75,19 @@ struct chip_desc {
 	} muxtype;
 };
 
+/*
+ * irq_mask_enable: Provides a mechanism to work around hardware that asserts
+ * their irq immediately on power on. It allows the enabling of the  irq to be
+ * delayed until the corresponding bits in the the irq_mask are set thru
+ * irq_unmask.
+ * For example the the ltc1760 assert its SMBALERT irq immediately on power
+ * on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
+ * device is registered irq are enabled and fire continuously as the second
+ * device driver has not yet loaded. Setting this parameter to 0x3 while
+ * delay the irq being enabled until both devices are ready.
+ * This workaround will not work if two devices share an interrupt on the
+ * same bus segment.
+ */
 struct pca954x {
 	const struct chip_desc *chip;
 
@@ -84,6 +97,7 @@ struct pca954x {
 
 	struct irq_domain *irq;
 	unsigned int irq_mask;
+	unsigned int irq_mask_enable;
 };
 
 /* Provide specs for the PCA954x types we know about */
@@ -270,9 +284,12 @@ static void pca954x_irq_unmask(struct irq_data *idata)
 	struct pca954x *data = irq_data_get_irq_chip_data(idata);
 	unsigned int pos = idata->hwirq;
 
-	if (!data->irq_mask)
+	if (!data->irq_mask_enable && !data->irq_mask)
 		enable_irq(data->client->irq);
 	data->irq_mask |= BIT(pos);
+	if (data->irq_mask_enable &&
+		(data->irq_mask & data->irq_mask) == data->irq_mask_enable)
+		enable_irq(data->client->irq);
 }
 
 static int pca954x_irq_set_type(struct irq_data *idata, unsigned int type)
@@ -395,6 +412,9 @@ static int pca954x_probe(struct i2c_client *client,
 	idle_disconnect_dt = of_node &&
 		of_property_read_bool(of_node, "i2c-mux-idle-disconnect");
 
+	of_property_read_u32(of_node, "nxp,irq-mask-enable",
+			     &data->irq_mask_enable);
+
 	ret = pca954x_irq_setup(muxc);
 	if (ret)
 		goto fail_del_adapters;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v2 2/5] dt: bindings: i2c-mux-pca954x: Add documentation for interrupt controller
From: Phil Reid @ 2017-01-05  4:11 UTC (permalink / raw)
  To: peda, wsa, robh+dt, mark.rutland, preid, linux-i2c, devicetree
In-Reply-To: <1483589463-35380-1-git-send-email-preid@electromag.com.au>

Various muxes can aggregate multiple irq lines and provide a control
register to determine the active line. Add bindings for interrupt
controller support.

Acked-by: Peter Rosin <peda@axentia.se>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
index cf53d5f..aa09704 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
@@ -19,7 +19,14 @@ Optional Properties:
   - i2c-mux-idle-disconnect: Boolean; if defined, forces mux to disconnect all
     children in idle state. This is necessary for example, if there are several
     multiplexers on the bus and the devices behind them use same I2C addresses.
-
+  - interrupt-parent: Phandle for the interrupt controller that services
+    interrupts for this device.
+  - interrupts: Interrupt mapping for IRQ.
+  - interrupt-controller: Marks the device node as an interrupt controller.
+  - #interrupt-cells : Should be two.
+    - first cell is the pin number
+    - second cell is used to specify flags.
+    See also Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
 
 Example:
 
@@ -29,6 +36,11 @@ Example:
 		#size-cells = <0>;
 		reg = <0x74>;
 
+		interrupt-parent = <&ipic>;
+		interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+
 		i2c@2 {
 			#address-cells = <1>;
 			#size-cells = <0>;
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v2 1/5] i2c: mux: pca954x: Add missing pca9542 definition to chip_desc
From: Phil Reid @ 2017-01-05  4:10 UTC (permalink / raw)
  To: peda, wsa, robh+dt, mark.rutland, preid, linux-i2c, devicetree
In-Reply-To: <1483589463-35380-1-git-send-email-preid@electromag.com.au>

The spec for the pca954x was missing. This chip is the same as the pca9540
except that it has interrupt lines. While the i2c_device_id table mapped
the pca9542 to the pca9540 definition the compatible table did not. In
preparation for irq support add the pca9542 definition.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index dd18b9c..bbf088e 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -84,6 +84,11 @@ struct pca954x {
 		.enable = 0x4,
 		.muxtype = pca954x_ismux,
 	},
+	[pca_9542] = {
+		.nchans = 2,
+		.enable = 0x4,
+		.muxtype = pca954x_ismux,
+	},
 	[pca_9543] = {
 		.nchans = 2,
 		.muxtype = pca954x_isswi,
@@ -110,7 +115,7 @@ struct pca954x {
 
 static const struct i2c_device_id pca954x_id[] = {
 	{ "pca9540", pca_9540 },
-	{ "pca9542", pca_9540 },
+	{ "pca9542", pca_9542 },
 	{ "pca9543", pca_9543 },
 	{ "pca9544", pca_9544 },
 	{ "pca9545", pca_9545 },
@@ -124,7 +129,7 @@ struct pca954x {
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id pca954x_acpi_ids[] = {
 	{ .id = "PCA9540", .driver_data = pca_9540 },
-	{ .id = "PCA9542", .driver_data = pca_9540 },
+	{ .id = "PCA9542", .driver_data = pca_9542 },
 	{ .id = "PCA9543", .driver_data = pca_9543 },
 	{ .id = "PCA9544", .driver_data = pca_9544 },
 	{ .id = "PCA9545", .driver_data = pca_9545 },
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v2 3/5] i2c: mux: pca954x: Add interrupt controller support
From: Phil Reid @ 2017-01-05  4:11 UTC (permalink / raw)
  To: peda, wsa, robh+dt, mark.rutland, preid, linux-i2c, devicetree
In-Reply-To: <1483589463-35380-1-git-send-email-preid@electromag.com.au>

Various muxes can aggregate multiple interrupts from each i2c bus.
All of the muxes with interrupt support combine the active low irq lines
using an internal 'and' function and generate a combined active low
output. The muxes do provide the ability to read a control register to
determine which irq is active. By making the mux an irq controller isr
latency can potentially be reduced by reading the status register and
then only calling the registered isr on that bus segment.

As there is no irq masking on the mux irq are disabled until irq_unmask is
called at least once.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 127 +++++++++++++++++++++++++++++++++++-
 1 file changed, 125 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index bbf088e..84fc767 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -41,14 +41,19 @@
 #include <linux/i2c.h>
 #include <linux/i2c-mux.h>
 #include <linux/i2c/pca954x.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_irq.h>
 #include <linux/pm.h>
 #include <linux/slab.h>
 
 #define PCA954X_MAX_NCHANS 8
 
+#define PCA954X_IRQ_OFFSET 4
+
 enum pca_type {
 	pca_9540,
 	pca_9542,
@@ -63,6 +68,7 @@ enum pca_type {
 struct chip_desc {
 	u8 nchans;
 	u8 enable;	/* used for muxes only */
+	u8 has_irq;
 	enum muxtype {
 		pca954x_ismux = 0,
 		pca954x_isswi
@@ -75,6 +81,9 @@ struct pca954x {
 	u8 last_chan;		/* last register value */
 	u8 deselect;
 	struct i2c_client *client;
+
+	struct irq_domain *irq;
+	unsigned int irq_mask;
 };
 
 /* Provide specs for the PCA954x types we know about */
@@ -87,19 +96,23 @@ struct pca954x {
 	[pca_9542] = {
 		.nchans = 2,
 		.enable = 0x4,
+		.has_irq = 1,
 		.muxtype = pca954x_ismux,
 	},
 	[pca_9543] = {
 		.nchans = 2,
+		.has_irq = 1,
 		.muxtype = pca954x_isswi,
 	},
 	[pca_9544] = {
 		.nchans = 4,
 		.enable = 0x4,
+		.has_irq = 1,
 		.muxtype = pca954x_ismux,
 	},
 	[pca_9545] = {
 		.nchans = 4,
+		.has_irq = 1,
 		.muxtype = pca954x_isswi,
 	},
 	[pca_9547] = {
@@ -222,6 +235,102 @@ static int pca954x_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
 	return pca954x_reg_write(muxc->parent, client, data->last_chan);
 }
 
+static irqreturn_t pca954x_irq_handler(int irq, void *dev_id)
+{
+	struct pca954x *data = dev_id;
+	unsigned int child_irq;
+	int ret, i, handled;
+
+	ret = i2c_smbus_read_byte(data->client);
+	if (ret < 0)
+		return IRQ_NONE;
+
+	for (i = 0; i < data->chip->nchans; i++) {
+		if (ret & BIT(PCA954X_IRQ_OFFSET + i)) {
+			child_irq = irq_linear_revmap(data->irq, i);
+			handle_nested_irq(child_irq);
+			handled++;
+		}
+	}
+	return handled ? IRQ_HANDLED : IRQ_NONE;
+}
+
+static void pca954x_irq_mask(struct irq_data *idata)
+{
+	struct pca954x *data = irq_data_get_irq_chip_data(idata);
+	unsigned int pos = idata->hwirq;
+
+	data->irq_mask &= ~BIT(pos);
+	if (!data->irq_mask)
+		disable_irq(data->client->irq);
+}
+
+static void pca954x_irq_unmask(struct irq_data *idata)
+{
+	struct pca954x *data = irq_data_get_irq_chip_data(idata);
+	unsigned int pos = idata->hwirq;
+
+	if (!data->irq_mask)
+		enable_irq(data->client->irq);
+	data->irq_mask |= BIT(pos);
+}
+
+static int pca954x_irq_set_type(struct irq_data *idata, unsigned int type)
+{
+	if ((type & IRQ_TYPE_SENSE_MASK) != IRQ_TYPE_LEVEL_LOW)
+		return -EINVAL;
+	return 0;
+}
+
+static struct irq_chip pca954x_irq_chip = {
+	.name = "i2c-mux-pca954x",
+	.irq_mask = pca954x_irq_mask,
+	.irq_unmask = pca954x_irq_unmask,
+	.irq_set_type = pca954x_irq_set_type,
+};
+
+static int pca954x_irq_setup(struct i2c_mux_core *muxc)
+{
+	struct pca954x *data = i2c_mux_priv(muxc);
+	struct i2c_client *client = data->client;
+	int c, err, irq;
+
+	if (!data->chip->has_irq || client->irq <= 0)
+		return 0;
+
+	data->irq = irq_domain_add_linear(client->dev.of_node,
+					  data->chip->nchans,
+					  &irq_domain_simple_ops, data);
+	if (!data->irq)
+		return -ENODEV;
+
+	for (c = 0; c < data->chip->nchans; c++) {
+		irq = irq_create_mapping(data->irq, c);
+		irq_set_chip_data(irq, data);
+		irq_set_chip_and_handler(irq, &pca954x_irq_chip,
+			handle_simple_irq);
+	}
+
+	err = devm_request_threaded_irq(&client->dev, data->client->irq, NULL,
+					pca954x_irq_handler,
+					IRQF_ONESHOT | IRQF_SHARED,
+					"pca954x", data);
+	if (err)
+		goto err_req_irq;
+
+	disable_irq(data->client->irq);
+
+	return 0;
+err_req_irq:
+	for (c = 0; c < data->chip->nchans; c++) {
+		irq = irq_find_mapping(data->irq, c);
+		irq_dispose_mapping(irq);
+	}
+	irq_domain_remove(data->irq);
+
+	return err;
+}
+
 /*
  * I2C init/probing/exit functions
  */
@@ -286,6 +395,10 @@ static int pca954x_probe(struct i2c_client *client,
 	idle_disconnect_dt = of_node &&
 		of_property_read_bool(of_node, "i2c-mux-idle-disconnect");
 
+	ret = pca954x_irq_setup(muxc);
+	if (ret)
+		goto fail_del_adapters;
+
 	/* Now create an adapter for each channel */
 	for (num = 0; num < data->chip->nchans; num++) {
 		bool idle_disconnect_pd = false;
@@ -311,7 +424,7 @@ static int pca954x_probe(struct i2c_client *client,
 			dev_err(&client->dev,
 				"failed to register multiplexed adapter"
 				" %d as bus %d\n", num, force);
-			goto virt_reg_failed;
+			goto fail_del_adapters;
 		}
 	}
 
@@ -322,7 +435,7 @@ static int pca954x_probe(struct i2c_client *client,
 
 	return 0;
 
-virt_reg_failed:
+fail_del_adapters:
 	i2c_mux_del_adapters(muxc);
 	return ret;
 }
@@ -330,6 +443,16 @@ static int pca954x_probe(struct i2c_client *client,
 static int pca954x_remove(struct i2c_client *client)
 {
 	struct i2c_mux_core *muxc = i2c_get_clientdata(client);
+	struct pca954x *data = i2c_mux_priv(muxc);
+	int c, irq;
+
+	if (data->irq) {
+		for (c = 0; c < data->chip->nchans; c++) {
+			irq = irq_find_mapping(data->irq, c);
+			irq_dispose_mapping(irq);
+		}
+		irq_domain_remove(data->irq);
+	}
 
 	i2c_mux_del_adapters(muxc);
 	return 0;
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v2] i2c: do not enable fall back to Host Notify by default
From: Dmitry Torokhov @ 2017-01-05  4:57 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Rob Herring, Benjamin Tissoires, Pali Rohár,
	Michał Kępień, Jean Delvare, Takashi Iwai,
	linux-i2c, devicetree, linux-kernel

Falling back unconditionally to HostNotify as primary client's interrupt
breaks some drivers which alter their functionality depending on whether
interrupt is present or not, so let's introduce a board flag telling I2C
core explicitly if we want wired interrupt or HostNotify-based one:
I2C_CLIENT_HOST_NOTIFY.

For DT-based systems we introduce "host-notify" property that we convert
to I2C_CLIENT_HOST_NOTIFY board flag.

Tested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

v1->v2:

- of_read_property_bool -> of_property_read_bool
- did not change binding wording to avoit mentioning I2C core because we
  use the same wording (mentioning I2C core) for wired interrupts

 Documentation/devicetree/bindings/i2c/i2c.txt |  8 ++++++++
 drivers/i2c/i2c-core.c                        | 17 ++++++++---------
 include/linux/i2c.h                           |  1 +
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
index 5fa691e6f638..cee9d5055fa2 100644
--- a/Documentation/devicetree/bindings/i2c/i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c.txt
@@ -62,6 +62,9 @@ wants to support one of the below features, it should adapt the bindings below.
 	"irq" and "wakeup" names are recognized by I2C core, other names are
 	left to individual drivers.
 
+- host-notify
+	device uses SMBus host notify protocol instead of interrupt line.
+
 - multi-master
 	states that there is another master active on this bus. The OS can use
 	this information to adapt power management to keep the arbitration awake
@@ -81,6 +84,11 @@ Binding may contain optional "interrupts" property, describing interrupts
 used by the device. I2C core will assign "irq" interrupt (or the very first
 interrupt if not using interrupt names) as primary interrupt for the slave.
 
+Alternatively, devices supporting SMbus Host Notify, and connected to
+adapters that support this feature, may use "host-notify" property. I2C
+core will create a virtual interrupt for Host Notify and assign it as
+primary interrupt for the slave.
+
 Also, if device is marked as a wakeup source, I2C core will set up "wakeup"
 interrupt for the device. If "wakeup" interrupt name is not present in the
 binding, then primary interrupt will be used as wakeup interrupt.
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index cf9e396d7702..7b117240f1ea 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -931,7 +931,10 @@ static int i2c_device_probe(struct device *dev)
 	if (!client->irq) {
 		int irq = -ENOENT;
 
-		if (dev->of_node) {
+		if (client->flags & I2C_CLIENT_HOST_NOTIFY) {
+			dev_dbg(dev, "Using Host Notify IRQ\n");
+			irq = i2c_smbus_host_notify_to_irq(client);
+		} else if (dev->of_node) {
 			irq = of_irq_get_byname(dev->of_node, "irq");
 			if (irq == -EINVAL || irq == -ENODATA)
 				irq = of_irq_get(dev->of_node, 0);
@@ -940,14 +943,7 @@ static int i2c_device_probe(struct device *dev)
 		}
 		if (irq == -EPROBE_DEFER)
 			return irq;
-		/*
-		 * ACPI and OF did not find any useful IRQ, try to see
-		 * if Host Notify can be used.
-		 */
-		if (irq < 0) {
-			dev_dbg(dev, "Using Host Notify IRQ\n");
-			irq = i2c_smbus_host_notify_to_irq(client);
-		}
+
 		if (irq < 0)
 			irq = 0;
 
@@ -1716,6 +1712,9 @@ static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
 	info.of_node = of_node_get(node);
 	info.archdata = &dev_ad;
 
+	if (of_property_read_bool(node, "host-notify"))
+		info.flags |= I2C_CLIENT_HOST_NOTIFY;
+
 	if (of_get_property(node, "wakeup-source", NULL))
 		info.flags |= I2C_CLIENT_WAKE;
 
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index b2109c522dec..4b45ec46161f 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -665,6 +665,7 @@ i2c_unlock_adapter(struct i2c_adapter *adapter)
 #define I2C_CLIENT_TEN		0x10	/* we have a ten bit chip address */
 					/* Must equal I2C_M_TEN below */
 #define I2C_CLIENT_SLAVE	0x20	/* we are the slave */
+#define I2C_CLIENT_HOST_NOTIFY	0x40	/* We want to use I2C host notify */
 #define I2C_CLIENT_WAKE		0x80	/* for board_info; true iff can wake */
 #define I2C_CLIENT_SCCB		0x9000	/* Use Omnivision SCCB protocol */
 					/* Must match I2C_M_STOP|IGNORE_NAK */
-- 
2.11.0.390.gc69c2f50cf-goog


-- 
Dmitry

^ permalink raw reply related

* Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on Dell machines
From: Pali Rohár @ 2017-01-05  8:54 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Wolfram Sang, Benjamin Tissoires, Michał Kępień,
	Jean Delvare, Steven Honeyman, Valdis.Kletnieks, Jochen Eisinger,
	Gabriele Mazzotta, Andy Lutomirski, Mario_Limonciello, Alex Hung,
	Takashi Iwai, linux-i2c, linux-kernel, platform-driver-x86
In-Reply-To: <20170104175456.GA40341@dtor-ws>

On Wednesday 04 January 2017 09:54:56 Dmitry Torokhov wrote:
> On Wed, Jan 04, 2017 at 06:46:19PM +0100, Wolfram Sang wrote:
> > 
> > > How about:
> > > ---
> > > From daa7571bbf337704332c0cfeec9b8fd5aeae596f Mon Sep 17 00:00:00 2001
> > > From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > > Date: Wed, 4 Jan 2017 18:26:54 +0100
> > > Subject: [PATCH] I2C: add the source of the IRQ in struct i2c_client
> > > 
> > > With commit 4d5538f5882a ("i2c: use an IRQ to report Host Notify events,
> > > not alert"), the IRQ provided in struct i2c_client might be assigned while
> > > it has not been explicitly declared by either the platform information
> > > or OF or ACPI.
> > > Some drivers (lis3lv02d) rely on the fact that the IRQ gets assigned or
> > > not to trigger a different behavior (exposing /dev/freefall in this case).
> > > 
> > > Provide a way for others to know who set the IRQ and so they can behave
> > > accordingly.
> > > 
> > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > > ---
> > >  drivers/i2c/i2c-core.c |  7 +++++++
> > >  include/linux/i2c.h    | 11 +++++++++++
> > >  2 files changed, 18 insertions(+)
> > > 
> > > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> > > index cf9e396..226c75d 100644
> > > --- a/drivers/i2c/i2c-core.c
> > > +++ b/drivers/i2c/i2c-core.c
> > > @@ -935,8 +935,12 @@ static int i2c_device_probe(struct device *dev)
> > >  			irq = of_irq_get_byname(dev->of_node, "irq");
> > >  			if (irq == -EINVAL || irq == -ENODATA)
> > >  				irq = of_irq_get(dev->of_node, 0);
> > > +			if (irq > 0)
> > > +				client->irq_source = I2C_IRQ_SOURCE_OF;
> > >  		} else if (ACPI_COMPANION(dev)) {
> > >  			irq = acpi_dev_gpio_irq_get(ACPI_COMPANION(dev), 0);
> > > +			if (irq > 0)
> > > +				client->irq_source = I2C_IRQ_SOURCE_ACPI;
> > >  		}
> > >  		if (irq == -EPROBE_DEFER)
> > >  			return irq;
> > > @@ -947,6 +951,8 @@ static int i2c_device_probe(struct device *dev)
> > >  		if (irq < 0) {
> > >  			dev_dbg(dev, "Using Host Notify IRQ\n");
> > >  			irq = i2c_smbus_host_notify_to_irq(client);
> > > +			if (irq > 0)
> > > +				client->irq_source = I2C_IRQ_SOURCE_HOST_NOTIFY;
> > >  		}
> > >  		if (irq < 0)
> > >  			irq = 0;
> > > @@ -1317,6 +1323,7 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
> > >  	client->flags = info->flags;
> > >  	client->addr = info->addr;
> > >  	client->irq = info->irq;
> > > +	client->irq_source = I2C_IRQ_SOURCE_PLATFORM;
> > >  
> > >  	strlcpy(client->name, info->type, sizeof(client->name));
> > >  
> > > diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> > > index b2109c5..7d0368d 100644
> > > --- a/include/linux/i2c.h
> > > +++ b/include/linux/i2c.h
> > > @@ -213,6 +213,13 @@ struct i2c_driver {
> > >  };
> > >  #define to_i2c_driver(d) container_of(d, struct i2c_driver, driver)
> > >  
> > > +enum i2c_irq_source {
> > > +	I2C_IRQ_SOURCE_PLATFORM,
> > > +	I2C_IRQ_SOURCE_OF,
> > > +	I2C_IRQ_SOURCE_ACPI,
> > > +	I2C_IRQ_SOURCE_HOST_NOTIFY,
> > > +};
> > > +
> > >  /**
> > >   * struct i2c_client - represent an I2C slave device
> > >   * @flags: I2C_CLIENT_TEN indicates the device uses a ten bit chip address;
> > > @@ -227,6 +234,9 @@ struct i2c_driver {
> > >   *	userspace_devices list
> > >   * @slave_cb: Callback when I2C slave mode of an adapter is used. The adapter
> > >   *	calls it to pass on slave events to the slave driver.
> > > + * @irq_source: Enum which provides the source of the IRQ. Useful to know
> > > + * 	if the IRQ was issued from Host Notify or if it was provided by an other
> > > + * 	component.
> > 
> > I'd think some documentation somewhere makes sense why we need to
> > distinguish this in some cases?
> 
> I'd rather drivers be oblivious of the source of interrupt. If they need
> to distinguish between them that means that our IRQ abstration failed.
> 
> > 
> > >   *
> > >   * An i2c_client identifies a single device (i.e. chip) connected to an
> > >   * i2c bus. The behaviour exposed to Linux is defined by the driver
> > > @@ -245,6 +255,7 @@ struct i2c_client {
> > >  #if IS_ENABLED(CONFIG_I2C_SLAVE)
> > >  	i2c_slave_cb_t slave_cb;	/* callback for slave mode	*/
> > >  #endif
> > > +	enum i2c_irq_source irq_source;	/* which component assigned the irq */
> > >  };
> > >  #define to_i2c_client(d) container_of(d, struct i2c_client, dev)
> > > 
> > > Dmitry, Wolfram, Jean, would this be acceptable for you?
> > 
> > Adding something to i2c_driver is not exactly cheap, but from what I
> > glimpsed from this thread, this is one of the cleanest solution to this
> > problem?
> > 
> 
> As Benjamin said, it is really property of device [instance], not
> driver. I.e. driver could handle both wired IRQ and HostNotify-based
> scheme similarly, it is device (and board) that knows how stuff is
> connected.
> 
> Maybe we could do something like this (untested):
> 
> 
> From e362a0277fd1bd6112f258664d8831d9bc6b78da Mon Sep 17 00:00:00 2001
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Date: Wed, 4 Jan 2017 09:33:43 -0800
> Subject: [PATCH] i2c: do not enable fall back to Host Notify by default
> 
> Falling back unconditionally to HostNotify as primary client's interrupt
> breaks some drivers which alter their functionality depending on whether
> interrupt is present or not, so let's introduce a board flag telling I2C
> core explicitly if we want wired interrupt or HostNotify-based one:
> I2C_CLIENT_HOST_NOTIFY.
> 
> For DT-based systems we introduce "host-notofy" property that we convert
> to I2C_CLIENT_HOST_NOTIFY board flag.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  Documentation/devicetree/bindings/i2c/i2c.txt |  8 ++++++++
>  drivers/i2c/i2c-core.c                        | 17 ++++++++---------
>  include/linux/i2c.h                           |  1 +
>  3 files changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
> index 5fa691e6f638..cee9d5055fa2 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c.txt
> @@ -62,6 +62,9 @@ wants to support one of the below features, it should adapt the bindings below.
>  	"irq" and "wakeup" names are recognized by I2C core, other names are
>  	left to individual drivers.
>  
> +- host-notify
> +	device uses SMBus host notify protocol instead of interrupt line.
> +
>  - multi-master
>  	states that there is another master active on this bus. The OS can use
>  	this information to adapt power management to keep the arbitration awake
> @@ -81,6 +84,11 @@ Binding may contain optional "interrupts" property, describing interrupts
>  used by the device. I2C core will assign "irq" interrupt (or the very first
>  interrupt if not using interrupt names) as primary interrupt for the slave.
>  
> +Alternatively, devices supporting SMbus Host Notify, and connected to
> +adapters that support this feature, may use "host-notify" property. I2C
> +core will create a virtual interrupt for Host Notify and assign it as
> +primary interrupt for the slave.
> +
>  Also, if device is marked as a wakeup source, I2C core will set up "wakeup"
>  interrupt for the device. If "wakeup" interrupt name is not present in the
>  binding, then primary interrupt will be used as wakeup interrupt.
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index cf9e396d7702..250969fa7670 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -931,7 +931,10 @@ static int i2c_device_probe(struct device *dev)
>  	if (!client->irq) {
>  		int irq = -ENOENT;
>  
> -		if (dev->of_node) {
> +		if (client->flags & I2C_CLIENT_HOST_HOTIFY) {
> +			dev_dbg(dev, "Using Host Notify IRQ\n");
> +			irq = i2c_smbus_host_notify_to_irq(client);
> +		} else if (dev->of_node) {
>  			irq = of_irq_get_byname(dev->of_node, "irq");
>  			if (irq == -EINVAL || irq == -ENODATA)
>  				irq = of_irq_get(dev->of_node, 0);
> @@ -940,14 +943,7 @@ static int i2c_device_probe(struct device *dev)
>  		}
>  		if (irq == -EPROBE_DEFER)
>  			return irq;
> -		/*
> -		 * ACPI and OF did not find any useful IRQ, try to see
> -		 * if Host Notify can be used.
> -		 */
> -		if (irq < 0) {
> -			dev_dbg(dev, "Using Host Notify IRQ\n");
> -			irq = i2c_smbus_host_notify_to_irq(client);
> -		}
> +
>  		if (irq < 0)
>  			irq = 0;
>  
> @@ -1716,6 +1712,9 @@ static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
>  	info.of_node = of_node_get(node);
>  	info.archdata = &dev_ad;
>  
> +	if (of_read_property_bool(node, "host-notify"))
> +		info.flags |= I2C_CLIENT_HOST_NOTIFY;
> +
>  	if (of_get_property(node, "wakeup-source", NULL))
>  		info.flags |= I2C_CLIENT_WAKE;
>  
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index b2109c522dec..4b45ec46161f 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -665,6 +665,7 @@ i2c_unlock_adapter(struct i2c_adapter *adapter)
>  #define I2C_CLIENT_TEN		0x10	/* we have a ten bit chip address */
>  					/* Must equal I2C_M_TEN below */
>  #define I2C_CLIENT_SLAVE	0x20	/* we are the slave */
> +#define I2C_CLIENT_HOST_NOTIFY	0x40	/* We want to use I2C host notify */
>  #define I2C_CLIENT_WAKE		0x80	/* for board_info; true iff can wake */
>  #define I2C_CLIENT_SCCB		0x9000	/* Use Omnivision SCCB protocol */
>  					/* Must match I2C_M_STOP|IGNORE_NAK */
> -- 
> 2.11.0.390.gc69c2f50cf-goog
> 
> 

Looks good, this seems to be elegant solution to our problem.

But then it is needed to patch those touchpad drivers to add that
I2C_CLIENT_HOST_NOTIFY flag, right?

-- 
Pali Rohár
pali.rohar@gmail.com

^ permalink raw reply

* [PATCH v8 0/5] Add support for the STM32F4 I2C
From: M'boumba Cedric Madianga @ 2017-01-05  9:07 UTC (permalink / raw)
  To: wsa, robh+dt, mcoquelin.stm32, alexandre.torgue, linus.walleij,
	patrice.chotard, linux, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel, u.kleine-koenig
  Cc: M'boumba Cedric Madianga

This patchset adds support for the I2C controller embedded in STM32F4xx SoC.
It enables I2C transfer in interrupt mode with Standard-mode and Fast-mode bus
speed.

Changes since v7:
- Remove unneeded parenthesis in some macro definitions (Uwe)
- Fix some typo (s/KhzkHZ, s/PEC/POC) (Uwe)
- Fix alignment issues in some structures declaration (Uwe)
- Clarify comments to argue i2c_timing values chosen (Uwe)
- Raise an error if parent clk rate is out of scope during I2C hw config (Uwe)
- Use dev_dbg instead of dev_err message when I2C bus is busy  (Uwe)
- Add more comments about stuff done by stm32f4_i2c_handle_rx_btf() (Uwe)
- Simplify stm32f4_i2c_isr_error() routine implementation by removing possible
  status checking (Uwe)
- Rework stm32f4_i2c_isr_error() routine by removing the loop to check which status occured  (Uwe)
- Add open-drain property for SCL pins  (Uwe)
- Rework unneeded mul_ccr field from i2c_timing structure
- Remove min_ccr field from i2c_timing structure as default scl_period is chosen
  to have a correct minimal ccr value
- Execute hw_config once during probe
- Remove soft_reset after an I2C error as all errors are now handled and
  hw_config is done once during probe
- Generate STOP by software when Acknowledge failure occurs
- Set the max speed mode for I2C pins
- Add bias-disable property for I2C pins
- Use intrinsic limitation of APB bus to set I2C max input clk

M'boumba Cedric Madianga (5):
  dt-bindings: Document the STM32 I2C bindings
  i2c: Add STM32F4 I2C driver
  ARM: dts: stm32: Add I2C1 support for STM32F429 SoC
  ARM: dts: stm32: Add I2C1 support for STM32429 eval board
  ARM: configs: stm32: Add I2C support for STM32 defconfig

 .../devicetree/bindings/i2c/i2c-stm32.txt          |  33 +
 arch/arm/boot/dts/stm32429i-eval.dts               |   6 +
 arch/arm/boot/dts/stm32f429.dtsi                   |  23 +
 arch/arm/configs/stm32_defconfig                   |   3 +
 drivers/i2c/busses/Kconfig                         |  10 +
 drivers/i2c/busses/Makefile                        |   1 +
 drivers/i2c/busses/i2c-stm32f4.c                   | 866 +++++++++++++++++++++
 7 files changed, 942 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-stm32.txt
 create mode 100644 drivers/i2c/busses/i2c-stm32f4.c

-- 
1.9.1

^ permalink raw reply

* [PATCH v8 1/5] dt-bindings: Document the STM32 I2C bindings
From: M'boumba Cedric Madianga @ 2017-01-05  9:07 UTC (permalink / raw)
  To: wsa, robh+dt, mcoquelin.stm32, alexandre.torgue, linus.walleij,
	patrice.chotard, linux, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel, u.kleine-koenig
  Cc: M'boumba Cedric Madianga
In-Reply-To: <1483607246-14771-1-git-send-email-cedric.madianga@gmail.com>

This patch adds documentation of device tree bindings for the STM32 I2C
controller.

Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/i2c/i2c-stm32.txt          | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-stm32.txt

diff --git a/Documentation/devicetree/bindings/i2c/i2c-stm32.txt b/Documentation/devicetree/bindings/i2c/i2c-stm32.txt
new file mode 100644
index 0000000..78eaf7b
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-stm32.txt
@@ -0,0 +1,33 @@
+* I2C controller embedded in STMicroelectronics STM32 I2C platform
+
+Required properties :
+- compatible : Must be "st,stm32f4-i2c"
+- reg : Offset and length of the register set for the device
+- interrupts : Must contain the interrupt id for I2C event and then the
+  interrupt id for I2C error.
+- resets: Must contain the phandle to the reset controller.
+- clocks: Must contain the input clock of the I2C instance.
+- A pinctrl state named "default" must be defined to set pins in mode of
+  operation for I2C transfer
+- #address-cells = <1>;
+- #size-cells = <0>;
+
+Optional properties :
+- clock-frequency : Desired I2C bus clock frequency in Hz. If not specified,
+  the default 100 kHz frequency will be used. As only Normal and Fast modes
+  are supported, possible values are 100000 and 400000.
+
+Example :
+
+	i2c@40005400 {
+		compatible = "st,stm32f4-i2c";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x40005400 0x400>;
+		interrupts = <31>,
+			     <32>;
+		resets = <&rcc 277>;
+		clocks = <&rcc 0 149>;
+		pinctrl-0 = <&i2c1_sda_pin>, <&i2c1_scl_pin>;
+		pinctrl-names = "default";
+	};
-- 
1.9.1

^ permalink raw reply related

* [PATCH v8 3/5] ARM: dts: stm32: Add I2C1 support for STM32F429 SoC
From: M'boumba Cedric Madianga @ 2017-01-05  9:07 UTC (permalink / raw)
  To: wsa, robh+dt, mcoquelin.stm32, alexandre.torgue, linus.walleij,
	patrice.chotard, linux, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel, u.kleine-koenig
  Cc: M'boumba Cedric Madianga
In-Reply-To: <1483607246-14771-1-git-send-email-cedric.madianga@gmail.com>

This patch adds I2C1 support for STM32F429 SoC

Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
---
 arch/arm/boot/dts/stm32f429.dtsi | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
index e4dae0e..5b063e9 100644
--- a/arch/arm/boot/dts/stm32f429.dtsi
+++ b/arch/arm/boot/dts/stm32f429.dtsi
@@ -48,6 +48,7 @@
 #include "skeleton.dtsi"
 #include "armv7-m.dtsi"
 #include <dt-bindings/pinctrl/stm32f429-pinfunc.h>
+#include <dt-bindings/mfd/stm32f4-rcc.h>
 
 / {
 	clocks {
@@ -153,6 +154,18 @@
 			status = "disabled";
 		};
 
+		i2c1: i2c@40005400 {
+			compatible = "st,stm32f4-i2c";
+			reg = <0x40005400 0x400>;
+			interrupts = <31>,
+				     <32>;
+			resets = <&rcc STM32F4_APB1_RESET(I2C1)>;
+			clocks = <&rcc 0 STM32F4_APB1_CLOCK(I2C1)>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
 		usart7: serial@40007800 {
 			compatible = "st,stm32-usart", "st,stm32-uart";
 			reg = <0x40007800 0x400>;
@@ -355,6 +368,16 @@
 					slew-rate = <2>;
 				};
 			};
+
+			i2c1_pins_b: i2c1@0 {
+				pins {
+					pinmux = <STM32F429_PB9_FUNC_I2C1_SDA>,
+						 <STM32F429_PB6_FUNC_I2C1_SCL>;
+					bias-disable;
+					drive-open-drain;
+					slew-rate = <3>;
+				};
+			};
 		};
 
 		rcc: rcc@40023810 {
-- 
1.9.1

^ permalink raw reply related

* [PATCH v8 2/5] i2c: Add STM32F4 I2C driver
From: M'boumba Cedric Madianga @ 2017-01-05  9:07 UTC (permalink / raw)
  To: wsa, robh+dt, mcoquelin.stm32, alexandre.torgue, linus.walleij,
	patrice.chotard, linux, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel, u.kleine-koenig
  Cc: M'boumba Cedric Madianga
In-Reply-To: <1483607246-14771-1-git-send-email-cedric.madianga@gmail.com>

This patch adds support for the STM32F4 I2C controller.

Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
---
 drivers/i2c/busses/Kconfig       |  10 +
 drivers/i2c/busses/Makefile      |   1 +
 drivers/i2c/busses/i2c-stm32f4.c | 866 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 877 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-stm32f4.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 0cdc844..2719208 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -886,6 +886,16 @@ config I2C_ST
 	  This driver can also be built as module. If so, the module
 	  will be called i2c-st.
 
+config I2C_STM32F4
+	tristate "STMicroelectronics STM32F4 I2C support"
+	depends on ARCH_STM32 || COMPILE_TEST
+	help
+	  Enable this option to add support for STM32 I2C controller embedded
+	  in STM32F4 SoCs.
+
+	  This driver can also be built as module. If so, the module
+	  will be called i2c-stm32f4.
+
 config I2C_STU300
 	tristate "ST Microelectronics DDC I2C interface"
 	depends on MACH_U300
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 1c1bac8..a2c6ff5 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -85,6 +85,7 @@ obj-$(CONFIG_I2C_SH_MOBILE)	+= i2c-sh_mobile.o
 obj-$(CONFIG_I2C_SIMTEC)	+= i2c-simtec.o
 obj-$(CONFIG_I2C_SIRF)		+= i2c-sirf.o
 obj-$(CONFIG_I2C_ST)		+= i2c-st.o
+obj-$(CONFIG_I2C_STM32F4)	+= i2c-stm32f4.o
 obj-$(CONFIG_I2C_STU300)	+= i2c-stu300.o
 obj-$(CONFIG_I2C_SUN6I_P2WI)	+= i2c-sun6i-p2wi.o
 obj-$(CONFIG_I2C_TEGRA)		+= i2c-tegra.o
diff --git a/drivers/i2c/busses/i2c-stm32f4.c b/drivers/i2c/busses/i2c-stm32f4.c
new file mode 100644
index 0000000..23e5757
--- /dev/null
+++ b/drivers/i2c/busses/i2c-stm32f4.c
@@ -0,0 +1,866 @@
+/*
+ * Driver for STMicroelectronics STM32 I2C controller
+ *
+ * This I2C controller is described in the STM32F429/439 Soc reference manual.
+ * Please see below a link to the documentation:
+ * http://www.st.com/resource/en/reference_manual/DM00031020.pdf
+ *
+ * Copyright (C) M'boumba Cedric Madianga 2016
+ * Author: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
+ *
+ * This driver is based on i2c-st.c
+ *
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+
+/* STM32F4 I2C offset registers */
+#define STM32F4_I2C_CR1			0x00
+#define STM32F4_I2C_CR2			0x04
+#define STM32F4_I2C_DR			0x10
+#define STM32F4_I2C_SR1			0x14
+#define STM32F4_I2C_SR2			0x18
+#define STM32F4_I2C_CCR			0x1C
+#define STM32F4_I2C_TRISE		0x20
+#define STM32F4_I2C_FLTR		0x24
+
+/* STM32F4 I2C control 1*/
+#define STM32F4_I2C_CR1_POS		BIT(11)
+#define STM32F4_I2C_CR1_ACK		BIT(10)
+#define STM32F4_I2C_CR1_STOP		BIT(9)
+#define STM32F4_I2C_CR1_START		BIT(8)
+#define STM32F4_I2C_CR1_PE		BIT(0)
+
+/* STM32F4 I2C control 2 */
+#define STM32F4_I2C_CR2_FREQ_MASK	GENMASK(5, 0)
+#define STM32F4_I2C_CR2_FREQ(n)		((n) & STM32F4_I2C_CR2_FREQ_MASK)
+#define STM32F4_I2C_CR2_ITBUFEN		BIT(10)
+#define STM32F4_I2C_CR2_ITEVTEN		BIT(9)
+#define STM32F4_I2C_CR2_ITERREN		BIT(8)
+#define STM32F4_I2C_CR2_IRQ_MASK	(STM32F4_I2C_CR2_ITBUFEN | \
+					 STM32F4_I2C_CR2_ITEVTEN | \
+					 STM32F4_I2C_CR2_ITERREN)
+
+/* STM32F4 I2C Status 1 */
+#define STM32F4_I2C_SR1_AF		BIT(10)
+#define STM32F4_I2C_SR1_ARLO		BIT(9)
+#define STM32F4_I2C_SR1_BERR		BIT(8)
+#define STM32F4_I2C_SR1_TXE		BIT(7)
+#define STM32F4_I2C_SR1_RXNE		BIT(6)
+#define STM32F4_I2C_SR1_BTF		BIT(2)
+#define STM32F4_I2C_SR1_ADDR		BIT(1)
+#define STM32F4_I2C_SR1_SB		BIT(0)
+#define STM32F4_I2C_SR1_ITEVTEN_MASK	(STM32F4_I2C_SR1_BTF | \
+					 STM32F4_I2C_SR1_ADDR | \
+					 STM32F4_I2C_SR1_SB)
+#define STM32F4_I2C_SR1_ITBUFEN_MASK	(STM32F4_I2C_SR1_TXE | \
+					 STM32F4_I2C_SR1_RXNE)
+#define STM32F4_I2C_SR1_ITERREN_MASK	(STM32F4_I2C_SR1_AF | \
+					 STM32F4_I2C_SR1_ARLO | \
+					 STM32F4_I2C_SR1_BERR)
+
+/* STM32F4 I2C Status 2 */
+#define STM32F4_I2C_SR2_BUSY		BIT(1)
+
+/* STM32F4 I2C Control Clock */
+#define STM32F4_I2C_CCR_CCR_MASK	GENMASK(11, 0)
+#define STM32F4_I2C_CCR_CCR(n)		((n) & STM32F4_I2C_CCR_CCR_MASK)
+#define STM32F4_I2C_CCR_FS		BIT(15)
+#define STM32F4_I2C_CCR_DUTY		BIT(14)
+
+/* STM32F4 I2C Trise */
+#define STM32F4_I2C_TRISE_VALUE_MASK	GENMASK(5, 0)
+#define STM32F4_I2C_TRISE_VALUE(n)	((n) & STM32F4_I2C_TRISE_VALUE_MASK)
+
+/* STM32F4 I2C Filter */
+#define STM32F4_I2C_FLTR_DNF_MASK	GENMASK(3, 0)
+#define STM32F4_I2C_FLTR_DNF(n)		((n) & STM32F4_I2C_FLTR_DNF_MASK)
+#define STM32F4_I2C_FLTR_ANOFF		BIT(4)
+
+#define STM32F4_I2C_MIN_FREQ		2U
+#define STM32F4_I2C_MAX_FREQ		46U
+#define HZ_TO_MHZ			1000000
+
+enum stm32f4_i2c_speed {
+	STM32F4_I2C_SPEED_STANDARD, /* 100 kHz */
+	STM32F4_I2C_SPEED_FAST, /* 400 kHz */
+	STM32F4_I2C_SPEED_END,
+};
+
+/**
+ * struct stm32f4_i2c_timings - per-Mode tuning parameters
+ * @duty: Fast mode duty cycle
+ * @scl_period: SCL period in microsecond to reach 100kHz/400kHz SCL frequency
+ */
+struct stm32f4_i2c_timings {
+	u32 duty;
+	u32 scl_period;
+};
+
+/**
+ * struct stm32f4_i2c_msg - client specific data
+ * @addr: 8-bit slave addr, including r/w bit
+ * @count: number of bytes to be transferred
+ * @buf: data buffer
+ * @result: result of the transfer
+ * @stop: last I2C msg to be sent, i.e. STOP to be generated
+ */
+struct stm32f4_i2c_msg {
+	u8 addr;
+	u32 count;
+	u8 *buf;
+	int result;
+	bool stop;
+};
+
+/**
+ * struct stm32f4_i2c_dev - private data of the controller
+ * @adap: I2C adapter for this controller
+ * @dev: device for this controller
+ * @base: virtual memory area
+ * @complete: completion of I2C message
+ * @clk: hw i2c clock
+ * speed: I2C clock frequency of the controller. Standard or Fast only supported
+ * @msg: I2C transfer information
+ */
+struct stm32f4_i2c_dev {
+	struct i2c_adapter adap;
+	struct device *dev;
+	void __iomem *base;
+	struct completion complete;
+	struct clk *clk;
+	int speed;
+	struct stm32f4_i2c_msg msg;
+};
+
+/*
+ * In standard mode:
+ * SCL period = SCL high period = SCL low period = CCR * I2C parent clk period
+ *
+ * In fast mode:
+ * If Duty = 0; SCL high period = 1  * CCR * I2C parent clk period
+ *		SCL low period  = 2  * CCR * I2C parent clk period
+ * If Duty = 1; SCL high period = 9  * CCR * I2C parent clk period
+ *		SCL low period  = 16 * CCR * I2C parent clk period
+ * In order to reach 400 kHz with lower I2C parent clk frequencies we always set
+ * Duty = 1
+ *
+ * For both modes, we have CCR = SCL period * I2C parent clk frequency
+ * with scl_period = 5 microseconds in Standard mode and scl_period = 1
+ * microsecond in Fast Mode in order to satisfy scl_high and scl_low periods
+ * constraints defined by i2c bus specification
+ */
+static struct stm32f4_i2c_timings i2c_timings[] = {
+	[STM32F4_I2C_SPEED_STANDARD] = {
+		.duty			= 0,
+		.scl_period		= 5,
+	},
+	[STM32F4_I2C_SPEED_FAST] = {
+		.duty			= 1,
+		.scl_period		= 1,
+	},
+};
+
+static inline void stm32f4_i2c_set_bits(void __iomem *reg, u32 mask)
+{
+	writel_relaxed(readl_relaxed(reg) | mask, reg);
+}
+
+static inline void stm32f4_i2c_clr_bits(void __iomem *reg, u32 mask)
+{
+	writel_relaxed(readl_relaxed(reg) & ~mask, reg);
+}
+
+static void stm32f4_i2c_disable_irq(struct stm32f4_i2c_dev *i2c_dev)
+{
+	void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
+
+	stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_IRQ_MASK);
+}
+
+static int stm32f4_i2c_set_periph_clk_freq(struct stm32f4_i2c_dev *i2c_dev)
+{
+	u32 clk_rate, cr2, freq;
+
+	/*
+	 * The minimum allowed frequency is 2MHz, the maximum APB frequency is
+	 * limited by an intrinsic value of 46 MHz
+	 */
+	clk_rate = clk_get_rate(i2c_dev->clk);
+	freq = DIV_ROUND_UP(clk_rate, HZ_TO_MHZ);
+	if (freq < STM32F4_I2C_MIN_FREQ || freq > STM32F4_I2C_MAX_FREQ) {
+		dev_err(i2c_dev->dev, "out of scope parent clk freq at %dMHz\n",
+			freq);
+		return -EINVAL;
+	}
+
+	cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
+	cr2 &= ~STM32F4_I2C_CR2_FREQ_MASK;
+	cr2 |= STM32F4_I2C_CR2_FREQ(freq);
+	writel_relaxed(cr2, i2c_dev->base + STM32F4_I2C_CR2);
+
+	return 0;
+}
+
+static void stm32f4_i2c_set_rise_time(struct stm32f4_i2c_dev *i2c_dev)
+{
+	u32 trise, freq, cr2;
+
+	/*
+	 * These bits must be programmed with the maximum SCL rise time given in
+	 * the I2C bus specification, incremented by 1.
+	 *
+	 * In standard mode, the maximum allowed SCL rise time is 1000 ns.
+	 * If, in the I2C_CR2 register, the value of FREQ[5:0] bits is equal to
+	 * 0x08 so period = 125 ns therefore the TRISE[5:0] bits must be
+	 * programmed with 09h.(1000 ns / 125 ns = 8 + 1)
+	 * So, for I2C standard mode TRISE = FREQ[5:0] + 1
+	 *
+	 * In fast mode, the maximum allowed SCL rise time is 300 ns.
+	 * If, in the I2C_CR2 register, the value of FREQ[5:0] bits is equal to
+	 * 0x08 so period = 125 ns therefore the TRISE[5:0] bits must be
+	 * programmed with 03h.(300 ns / 125 ns = 2 + 1)
+	 * So, for I2C fast mode TRISE = FREQ[5:0] * 300 / 1000 + 1
+	 */
+
+	cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
+	freq = cr2 & STM32F4_I2C_CR2_FREQ_MASK;
+
+	if (i2c_dev->speed == STM32F4_I2C_SPEED_STANDARD)
+		trise = freq + 1;
+	else
+		trise = freq * 300 / 1000 + 1;
+
+	writel_relaxed(STM32F4_I2C_TRISE_VALUE(trise),
+		       i2c_dev->base + STM32F4_I2C_TRISE);
+}
+
+static void stm32f4_i2c_set_speed_mode(struct stm32f4_i2c_dev *i2c_dev)
+{
+	struct stm32f4_i2c_timings *t = &i2c_timings[i2c_dev->speed];
+	u32 cr2, ccr, freq, val;
+
+	ccr = readl_relaxed(i2c_dev->base + STM32F4_I2C_CCR);
+	ccr &= ~(STM32F4_I2C_CCR_FS | STM32F4_I2C_CCR_DUTY |
+		 STM32F4_I2C_CCR_CCR_MASK);
+
+	/*
+	 * Please see the comments above regarding i2c_timings[] declaration
+	 * to understand the below calculation
+	 */
+	cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
+	freq = cr2 & STM32F4_I2C_CR2_FREQ_MASK;
+	val = freq * t->scl_period;
+	ccr |= STM32F4_I2C_CCR_CCR(val);
+
+	if (t->duty)
+		ccr |= STM32F4_I2C_CCR_FS | STM32F4_I2C_CCR_DUTY;
+
+	writel_relaxed(ccr, i2c_dev->base + STM32F4_I2C_CCR);
+}
+
+static void stm32f4_i2c_set_filter(struct stm32f4_i2c_dev *i2c_dev)
+{
+	u32 filter;
+
+	/* Enable analog noise filter and disable digital noise filter */
+	filter = readl_relaxed(i2c_dev->base + STM32F4_I2C_FLTR);
+	filter &= ~(STM32F4_I2C_FLTR_ANOFF | STM32F4_I2C_FLTR_DNF_MASK);
+	writel_relaxed(filter, i2c_dev->base + STM32F4_I2C_FLTR);
+}
+
+/**
+ * stm32f4_i2c_hw_config() - Prepare I2C block
+ * @i2c_dev: Controller's private data
+ */
+static int stm32f4_i2c_hw_config(struct stm32f4_i2c_dev *i2c_dev)
+{
+	void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
+	int ret = 0;
+
+	/* Disable I2C */
+	stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_PE);
+
+	ret = stm32f4_i2c_set_periph_clk_freq(i2c_dev);
+	if (ret)
+		return ret;
+
+	stm32f4_i2c_set_rise_time(i2c_dev);
+
+	stm32f4_i2c_set_speed_mode(i2c_dev);
+
+	stm32f4_i2c_set_filter(i2c_dev);
+
+	/* Enable I2C */
+	stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_PE);
+
+	return ret;
+}
+
+static int stm32f4_i2c_wait_free_bus(struct stm32f4_i2c_dev *i2c_dev)
+{
+	u32 status;
+	int ret;
+
+	ret = readl_relaxed_poll_timeout(i2c_dev->base + STM32F4_I2C_SR2,
+					 status,
+					 !(status & STM32F4_I2C_SR2_BUSY),
+					 10, 1000);
+	if (ret) {
+		dev_dbg(i2c_dev->dev, "bus not free\n");
+		ret = -EBUSY;
+	}
+
+	return ret;
+}
+
+/**
+ * stm32f4_i2c_write_ byte() - Write a byte in the data register
+ * @i2c_dev: Controller's private data
+ * @byte: Data to write in the register
+ */
+static void stm32f4_i2c_write_byte(struct stm32f4_i2c_dev *i2c_dev, u8 byte)
+{
+	writel_relaxed(byte, i2c_dev->base + STM32F4_I2C_DR);
+}
+
+/**
+ * stm32f4_i2c_write_msg() - Fill the data register in write mode
+ * @i2c_dev: Controller's private data
+ *
+ * This function fills the data register with I2C transfer buffer
+ */
+static void stm32f4_i2c_write_msg(struct stm32f4_i2c_dev *i2c_dev)
+{
+	struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
+
+	stm32f4_i2c_write_byte(i2c_dev, *msg->buf++);
+	msg->count--;
+}
+
+static void stm32f4_i2c_read_msg(struct stm32f4_i2c_dev *i2c_dev)
+{
+	struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
+	u32 rbuf;
+
+	rbuf = readl_relaxed(i2c_dev->base + STM32F4_I2C_DR);
+	*msg->buf++ = rbuf & 0xff;
+	msg->count--;
+}
+
+static void stm32f4_i2c_terminate_xfer(struct stm32f4_i2c_dev *i2c_dev)
+{
+	struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
+	void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
+
+	stm32f4_i2c_disable_irq(i2c_dev);
+
+	reg = i2c_dev->base + STM32F4_I2C_CR1;
+	if (msg->stop)
+		stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
+	else
+		stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
+
+	complete(&i2c_dev->complete);
+}
+
+/**
+ * stm32f4_i2c_handle_write() - Handle FIFO empty interrupt in case of write
+ * @i2c_dev: Controller's private data
+ */
+static void stm32f4_i2c_handle_write(struct stm32f4_i2c_dev *i2c_dev)
+{
+	struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
+	void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
+
+	if (msg->count) {
+		stm32f4_i2c_write_msg(i2c_dev);
+		if (!msg->count) {
+			/* Disable buffer interrupts for RXNE/TXE events */
+			stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
+		}
+	} else {
+		stm32f4_i2c_terminate_xfer(i2c_dev);
+	}
+}
+
+/**
+ * stm32f4_i2c_handle_read() - Handle FIFO empty interrupt in case of read
+ * @i2c_dev: Controller's private data
+ */
+static void stm32f4_i2c_handle_read(struct stm32f4_i2c_dev *i2c_dev)
+{
+	struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
+	void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
+
+	switch (msg->count) {
+	case 1:
+		stm32f4_i2c_disable_irq(i2c_dev);
+		stm32f4_i2c_read_msg(i2c_dev);
+		complete(&i2c_dev->complete);
+		break;
+	/*
+	 * For 2 or 3-byte reception, we do not have to read the data register
+	 * when RXNE occurs as we have to wait for byte transferred finished
+	 * event before reading data. So, here we just disable buffer
+	 * interrupt in order to avoid another system preemption due to RXNE
+	 * event
+	 */
+	case 2:
+	case 3:
+		stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
+		break;
+	/* For N byte reception with N > 3 we directly read data register */
+	default:
+		stm32f4_i2c_read_msg(i2c_dev);
+	}
+}
+
+/**
+ * stm32f4_i2c_handle_rx_btf() - Handle byte transfer finished interrupt
+ * in case of read
+ * @i2c_dev: Controller's private data
+ */
+static void stm32f4_i2c_handle_rx_btf(struct stm32f4_i2c_dev *i2c_dev)
+{
+	struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
+	void __iomem *reg;
+	u32 mask;
+	int i;
+
+	switch (msg->count) {
+	case 2:
+		/*
+		 * In order to correctly send the Stop or Repeated Start
+		 * condition on the I2C bus, the STOP/START bit has to be set
+		 * before reading the last two bytes.
+		 * After that, we could read the last two bytes, disable
+		 * remaining interrupts and notify the end of xfer to the
+		 * client
+		 */
+		reg = i2c_dev->base + STM32F4_I2C_CR1;
+		if (msg->stop)
+			stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
+		else
+			stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
+
+		for (i = 2; i > 0; i--)
+			stm32f4_i2c_read_msg(i2c_dev);
+
+		reg = i2c_dev->base + STM32F4_I2C_CR2;
+		mask = STM32F4_I2C_CR2_ITEVTEN | STM32F4_I2C_CR2_ITERREN;
+		stm32f4_i2c_clr_bits(reg, mask);
+
+		complete(&i2c_dev->complete);
+		break;
+	case 3:
+		/*
+		 * In order to correctly send the ACK on the I2C bus for the
+		 * last two bytes, we have to set ACK bit before reading the
+		 * third last data byte
+		 */
+		reg = i2c_dev->base + STM32F4_I2C_CR1;
+		stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
+		stm32f4_i2c_read_msg(i2c_dev);
+		break;
+	default:
+		stm32f4_i2c_read_msg(i2c_dev);
+	}
+}
+
+/**
+ * stm32f4_i2c_handle_rx_addr() - Handle address matched interrupt in case of
+ * master receiver
+ * @i2c_dev: Controller's private data
+ */
+static void stm32f4_i2c_handle_rx_addr(struct stm32f4_i2c_dev *i2c_dev)
+{
+	struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
+	void __iomem *reg;
+
+	switch (msg->count) {
+	case 0:
+		stm32f4_i2c_terminate_xfer(i2c_dev);
+		/* Clear ADDR flag */
+		readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
+		break;
+	case 1:
+		/*
+		 * Single byte reception:
+		 * Enable NACK, clear ADDR flag and generate STOP or RepSTART
+		 */
+		reg = i2c_dev->base + STM32F4_I2C_CR1;
+		stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
+		readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
+		if (msg->stop)
+			stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
+		else
+			stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
+		break;
+	case 2:
+		/*
+		 * 2-byte reception:
+		 * Enable NACK and set POS
+		 */
+		reg = i2c_dev->base + STM32F4_I2C_CR1;
+		stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
+		stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_POS);
+		readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
+		break;
+
+	default:
+		/* N-byte reception: Enable ACK */
+		reg = i2c_dev->base + STM32F4_I2C_CR1;
+		stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_ACK);
+		readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
+		break;
+	}
+}
+
+/**
+ * stm32f4_i2c_isr_event() - Interrupt routine for I2C bus event
+ * @irq: interrupt number
+ * @data: Controller's private data
+ */
+static irqreturn_t stm32f4_i2c_isr_event(int irq, void *data)
+{
+	struct stm32f4_i2c_dev *i2c_dev = data;
+	struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
+	u32 possible_status = STM32F4_I2C_SR1_ITEVTEN_MASK;
+	void __iomem *reg;
+	u32 status, ien, event;
+
+	ien = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
+	ien &= STM32F4_I2C_CR2_IRQ_MASK;
+
+	/* Update possible_status if buffer interrupt is enabled */
+	if (ien & STM32F4_I2C_CR2_ITBUFEN)
+		possible_status |= STM32F4_I2C_SR1_ITBUFEN_MASK;
+
+	status = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR1);
+	event = status & possible_status;
+	if (!event) {
+		dev_dbg(i2c_dev->dev,
+			"spurious evt irq (status=0x%08x, ien=0x%08x)\n",
+			status, ien);
+		return IRQ_NONE;
+	}
+
+	if (event & STM32F4_I2C_SR1_SB)
+		stm32f4_i2c_write_byte(i2c_dev, msg->addr);
+
+	if (event & STM32F4_I2C_SR1_ADDR) {
+		if (msg->addr & I2C_M_RD)
+			stm32f4_i2c_handle_rx_addr(i2c_dev);
+		else
+			readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
+
+		/* Enable buffer interrupts for RXNE/TXE events */
+		reg = i2c_dev->base + STM32F4_I2C_CR2;
+		stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
+	}
+
+	if ((event & STM32F4_I2C_SR1_TXE) && !(msg->addr & I2C_M_RD))
+		stm32f4_i2c_handle_write(i2c_dev);
+
+	if ((event & STM32F4_I2C_SR1_RXNE) && (msg->addr & I2C_M_RD))
+		stm32f4_i2c_handle_read(i2c_dev);
+
+	if (event & STM32F4_I2C_SR1_BTF) {
+		if (msg->addr & I2C_M_RD)
+			stm32f4_i2c_handle_rx_btf(i2c_dev);
+		else
+			stm32f4_i2c_handle_write(i2c_dev);
+	}
+
+	return IRQ_HANDLED;
+}
+
+/**
+ * stm32f4_i2c_isr_error() - Interrupt routine for I2C bus error
+ * @irq: interrupt number
+ * @data: Controller's private data
+ */
+static irqreturn_t stm32f4_i2c_isr_error(int irq, void *data)
+{
+	struct stm32f4_i2c_dev *i2c_dev = data;
+	struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
+	void __iomem *reg;
+	u32 status;
+
+	status = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR1);
+
+	/* Arbitration lost */
+	if (status & STM32F4_I2C_SR1_ARLO) {
+		reg = i2c_dev->base + STM32F4_I2C_SR1;
+		stm32f4_i2c_clr_bits(reg, STM32F4_I2C_SR1_ARLO);
+		msg->result = -EAGAIN;
+	}
+
+	/*
+	 * Acknowledge failure:
+	 * In master transmitter mode a Stop must be generated by software
+	 */
+	if (status & STM32F4_I2C_SR1_AF) {
+		if (!(msg->addr & I2C_M_RD)) {
+			reg = i2c_dev->base + STM32F4_I2C_CR1;
+			stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
+		}
+		reg = i2c_dev->base + STM32F4_I2C_SR1;
+		stm32f4_i2c_clr_bits(reg, STM32F4_I2C_SR1_AF);
+		msg->result = -EIO;
+	}
+
+	/* Bus error */
+	if (status & STM32F4_I2C_SR1_BERR) {
+		reg = i2c_dev->base + STM32F4_I2C_SR1;
+		stm32f4_i2c_clr_bits(reg, STM32F4_I2C_SR1_BERR);
+		msg->result = -EIO;
+	}
+
+	stm32f4_i2c_disable_irq(i2c_dev);
+	complete(&i2c_dev->complete);
+
+	return IRQ_HANDLED;
+}
+
+/**
+ * stm32f4_i2c_xfer_msg() - Transfer a single I2C message
+ * @i2c_dev: Controller's private data
+ * @msg: I2C message to transfer
+ * @is_first: first message of the sequence
+ * @is_last: last message of the sequence
+ */
+static int stm32f4_i2c_xfer_msg(struct stm32f4_i2c_dev *i2c_dev,
+				struct i2c_msg *msg, bool is_first,
+				bool is_last)
+{
+	struct stm32f4_i2c_msg *f4_msg = &i2c_dev->msg;
+	void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
+	unsigned long timeout;
+	u32 mask;
+	int ret;
+
+	f4_msg->addr = i2c_8bit_addr_from_msg(msg);
+	f4_msg->buf = msg->buf;
+	f4_msg->count = msg->len;
+	f4_msg->result = 0;
+	f4_msg->stop = is_last;
+
+	reinit_completion(&i2c_dev->complete);
+
+	/* Enable events and errors interrupts */
+	mask = STM32F4_I2C_CR2_ITEVTEN | STM32F4_I2C_CR2_ITERREN;
+	stm32f4_i2c_set_bits(i2c_dev->base + STM32F4_I2C_CR2, mask);
+
+	if (is_first) {
+		ret = stm32f4_i2c_wait_free_bus(i2c_dev);
+		if (ret)
+			return ret;
+
+		/* START generation */
+		stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
+	}
+
+	timeout = wait_for_completion_timeout(&i2c_dev->complete,
+					      i2c_dev->adap.timeout);
+	ret = f4_msg->result;
+
+	/* Disable POS position Ack */
+	stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_POS);
+
+	if (!timeout)
+		ret = -ETIMEDOUT;
+
+	return ret;
+}
+
+/**
+ * stm32f4_i2c_xfer() - Transfer combined I2C message
+ * @i2c_adap: Adapter pointer to the controller
+ * @msgs: Pointer to data to be written.
+ * @num: Number of messages to be executed
+ */
+static int stm32f4_i2c_xfer(struct i2c_adapter *i2c_adap, struct i2c_msg msgs[],
+			    int num)
+{
+	struct stm32f4_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
+	int ret, i;
+
+	ret = clk_enable(i2c_dev->clk);
+	if (ret) {
+		dev_err(i2c_dev->dev, "Failed to enable clock\n");
+		return ret;
+	}
+
+	for (i = 0; i < num && !ret; i++)
+		ret = stm32f4_i2c_xfer_msg(i2c_dev, &msgs[i], i == 0,
+					   i == num - 1);
+
+	clk_disable(i2c_dev->clk);
+
+	return (ret < 0) ? ret : num;
+}
+
+static u32 stm32f4_i2c_func(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static struct i2c_algorithm stm32f4_i2c_algo = {
+	.master_xfer = stm32f4_i2c_xfer,
+	.functionality = stm32f4_i2c_func,
+};
+
+static int stm32f4_i2c_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct stm32f4_i2c_dev *i2c_dev;
+	struct resource *res;
+	u32 irq_event, irq_error, clk_rate;
+	struct i2c_adapter *adap;
+	struct reset_control *rst;
+	int ret;
+
+	i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*i2c_dev), GFP_KERNEL);
+	if (!i2c_dev)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	i2c_dev->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(i2c_dev->base))
+		return PTR_ERR(i2c_dev->base);
+
+	irq_event = irq_of_parse_and_map(np, 0);
+	if (!irq_event) {
+		dev_err(&pdev->dev, "IRQ event missing or invalid\n");
+		return -EINVAL;
+	}
+
+	irq_error = irq_of_parse_and_map(np, 1);
+	if (!irq_error) {
+		dev_err(&pdev->dev, "IRQ error missing or invalid\n");
+		return -EINVAL;
+	}
+
+	i2c_dev->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(i2c_dev->clk)) {
+		dev_err(&pdev->dev, "Error: Missing controller clock\n");
+		return PTR_ERR(i2c_dev->clk);
+	}
+	ret = clk_prepare_enable(i2c_dev->clk);
+	if (ret) {
+		dev_err(i2c_dev->dev, "Failed to prepare_enable clock\n");
+		return ret;
+	}
+
+	rst = devm_reset_control_get(&pdev->dev, NULL);
+	if (IS_ERR(rst)) {
+		dev_err(&pdev->dev, "Error: Missing controller reset\n");
+		ret = PTR_ERR(rst);
+		goto clk_free;
+	}
+	reset_control_assert(rst);
+	udelay(2);
+	reset_control_deassert(rst);
+
+	i2c_dev->speed = STM32F4_I2C_SPEED_STANDARD;
+	ret = of_property_read_u32(np, "clock-frequency", &clk_rate);
+	if (!ret && clk_rate >= 40000)
+		i2c_dev->speed = STM32F4_I2C_SPEED_FAST;
+
+	i2c_dev->dev = &pdev->dev;
+
+	ret = devm_request_irq(&pdev->dev, irq_event, stm32f4_i2c_isr_event, 0,
+			       pdev->name, i2c_dev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to request irq event %i\n",
+			irq_event);
+		goto clk_free;
+	}
+
+	ret = devm_request_irq(&pdev->dev, irq_error, stm32f4_i2c_isr_error, 0,
+			       pdev->name, i2c_dev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to request irq error %i\n",
+			irq_error);
+		goto clk_free;
+	}
+
+	ret = stm32f4_i2c_hw_config(i2c_dev);
+	if (ret)
+		goto clk_free;
+
+	adap = &i2c_dev->adap;
+	i2c_set_adapdata(adap, i2c_dev);
+	snprintf(adap->name, sizeof(adap->name), "STM32 I2C(%pa)", &res->start);
+	adap->owner = THIS_MODULE;
+	adap->timeout = 2 * HZ;
+	adap->retries = 0;
+	adap->algo = &stm32f4_i2c_algo;
+	adap->dev.parent = &pdev->dev;
+	adap->dev.of_node = pdev->dev.of_node;
+
+	init_completion(&i2c_dev->complete);
+
+	ret = i2c_add_adapter(adap);
+	if (ret)
+		goto clk_free;
+
+	platform_set_drvdata(pdev, i2c_dev);
+
+	clk_disable(i2c_dev->clk);
+
+	dev_info(i2c_dev->dev, "STM32F4 I2C driver registered\n");
+
+	return 0;
+
+clk_free:
+	clk_disable_unprepare(i2c_dev->clk);
+	return ret;
+}
+
+static int stm32f4_i2c_remove(struct platform_device *pdev)
+{
+	struct stm32f4_i2c_dev *i2c_dev = platform_get_drvdata(pdev);
+
+	i2c_del_adapter(&i2c_dev->adap);
+
+	clk_unprepare(i2c_dev->clk);
+
+	return 0;
+}
+
+static const struct of_device_id stm32f4_i2c_match[] = {
+	{ .compatible = "st,stm32f4-i2c", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, stm32f4_i2c_match);
+
+static struct platform_driver stm32f4_i2c_driver = {
+	.driver = {
+		.name = "stm32f4-i2c",
+		.of_match_table = stm32f4_i2c_match,
+	},
+	.probe = stm32f4_i2c_probe,
+	.remove = stm32f4_i2c_remove,
+};
+
+module_platform_driver(stm32f4_i2c_driver);
+
+MODULE_AUTHOR("M'boumba Cedric Madianga <cedric.madianga@gmail.com>");
+MODULE_DESCRIPTION("STMicroelectronics STM32F4 I2C driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox