[PATCH 1/2] i2c: do not leave semaphore armed when copying properties fails

[PATCH 1/2] i2c: do not leave semaphore armed when copying properties fails

[Dmitry Torokhov]

We should not leave i2c_register_board_info() early, without unlocking the
__i2c_board_lock.

Fixes: b0c1e95ab44f ("i2c: copy device properties when using ...")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/i2c/i2c-boardinfo.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/i2c-boardinfo.c b/drivers/i2c/i2c-boardinfo.c
index 5b8f6c3a6950..0e285c68b2ff 100644
--- a/drivers/i2c/i2c-boardinfo.c
+++ b/drivers/i2c/i2c-boardinfo.c
@@ -84,8 +84,10 @@ int i2c_register_board_info(int busnum, struct i2c_board_info const *info, unsig
 		if (info->properties) {
 			devinfo->board_info.properties =
 					property_entries_dup(info->properties);
-			if (IS_ERR(devinfo->board_info.properties))
-				return PTR_ERR(devinfo->board_info.properties);
+			if (IS_ERR(devinfo->board_info.properties)) {
+				status = PTR_ERR(devinfo->board_info.properties);
+				break;
+			}
 		}
 
 		list_add_tail(&devinfo->list, &__i2c_board_list);
-- 
2.12.0.246.ga2ecc84866-goog

[PATCH 2/2] i2c: allow attaching IRQ resources to i2c_board_info

[Dmitry Torokhov]

Simple integer for interrupt number is not expressive enough, as it does
not convey interrupt trigger type that should be used. Let's allow
attaching array of resources to the board info and have i2c core parse
first IRQ resource and set up interrupt trigger as needed.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/i2c/i2c-boardinfo.c | 12 ++++++++++++
 drivers/i2c/i2c-core.c      | 30 ++++++++++++++++++++++++++++++
 include/linux/i2c.h         |  4 ++++
 3 files changed, 46 insertions(+)

diff --git a/drivers/i2c/i2c-boardinfo.c b/drivers/i2c/i2c-boardinfo.c
index 0e285c68b2ff..31186ead5a40 100644
--- a/drivers/i2c/i2c-boardinfo.c
+++ b/drivers/i2c/i2c-boardinfo.c
@@ -90,6 +90,18 @@ int i2c_register_board_info(int busnum, struct i2c_board_info const *info, unsig
 			}
 		}
 
+		if (info->resources) {
+			devinfo->board_info.resources =
+				kmemdup(info->resources,
+					info->num_resources *
+						sizeof(*info->resources),
+					GFP_KERNEL);
+			if (!devinfo->board_info.resources) {
+				status = -ENOMEM;
+				break;
+			}
+		}
+
 		list_add_tail(&devinfo->list, &__i2c_board_list);
 	}
 
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 3897e78e5e9a..34a5115484dd 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1277,6 +1277,32 @@ static void i2c_dev_set_name(struct i2c_adapter *adap,
 		     i2c_encode_flags_to_addr(client));
 }
 
+static int i2c_dev_irq_from_resources(const struct resource *resources,
+				      unsigned int num_resources)
+{
+	struct irq_data *irqd;
+	int i;
+
+	for (i = 0; i < num_resources; i++) {
+		const struct resource *r = &resources[i];
+
+		if (resource_type(r) != IORESOURCE_IRQ)
+			continue;
+
+		if (r->flags & IORESOURCE_BITS) {
+			irqd = irq_get_irq_data(r->start);
+			if (!irqd)
+				break;
+
+			irqd_set_trigger_type(irqd, r->flags & IORESOURCE_BITS);
+		}
+
+		return r->start;
+	}
+
+	return 0;
+}
+
 /**
  * i2c_new_device - instantiate an i2c device
  * @adap: the adapter managing the device
@@ -1312,7 +1338,11 @@ 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;
+	if (!client->irq)
+		client->irq = i2c_dev_irq_from_resources(info->resources,
+							 info->num_resources);
 
 	strlcpy(client->name, info->type, sizeof(client->name));
 
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index ec8f866a5656..2cc3988d127b 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -312,6 +312,8 @@ static inline int i2c_slave_event(struct i2c_client *client,
  * @of_node: pointer to OpenFirmware device node
  * @fwnode: device node supplied by the platform firmware
  * @properties: additional device properties for the device
+ * @resources: resources associated with the device
+ * @num_resources: number of resources in the @resources array
  * @irq: stored in i2c_client.irq
  *
  * I2C doesn't actually support hardware probing, although controllers and
@@ -334,6 +336,8 @@ struct i2c_board_info {
 	struct device_node *of_node;
 	struct fwnode_handle *fwnode;
 	const struct property_entry *properties;
+	const struct resource *resources;
+	unsigned int	num_resources;
 	int		irq;
 };
 
-- 
2.12.0.246.ga2ecc84866-goog

Re: [PATCH 1/2] i2c: do not leave semaphore armed when copying properties fails

[Wolfram Sang]

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

On Wed, Mar 08, 2017 at 10:41:01AM -0800, Dmitry Torokhov wrote:
> We should not leave i2c_register_board_info() early, without unlocking the
> __i2c_board_lock.
> 
> Fixes: b0c1e95ab44f ("i2c: copy device properties when using ...")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

So, it seems that patches 1+2 are related. Because I'd like to have
patch 2 sitting in for-next for a whole cycle for sure, my plan is to
revert the faulty b0c1e95ab44f from for-current and apply the fixed
version (b0c1e95ab44f + this patch squashed) to for-next as well.

Is that okay with you?


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

Re: [PATCH 1/2] i2c: do not leave semaphore armed when copying properties fails

[Dmitry Torokhov]

Hi Wolfram,

> On Wed, Mar 08, 2017 at 10:41:01AM -0800, Dmitry Torokhov wrote:
> > We should not leave i2c_register_board_info() early, without unlocking the
> > __i2c_board_lock.
> > 
> > Fixes: b0c1e95ab44f ("i2c: copy device properties when using ...")
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> So, it seems that patches 1+2 are related. Because I'd like to have
> patch 2 sitting in for-next for a whole cycle for sure, my plan is to
> revert the faulty b0c1e95ab44f from for-current and apply the fixed
> version (b0c1e95ab44f + this patch squashed) to for-next as well.
> 
> Is that okay with you?

I am perfectly fine with reverting b0c1e95ab44f from for-current,
however I wonder if we could have an immutable branch off 4.11-rc2 (or
-rc1) containing fixed version of patch copying property + patch adding
resources + patch exporting i2c_client_type (I will CC you on that
shortly), which we could share between your tree and mine so I can get
in changes to a few drivers on my side (eeti_ts, atmel, etc).

If you are OK with this I can prepare said branch.

Thanks!

-- 
Dmitry

Re: [PATCH 1/2] i2c: do not leave semaphore armed when copying properties fails

[Wolfram Sang]

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


> I am perfectly fine with reverting b0c1e95ab44f from for-current,
> however I wonder if we could have an immutable branch off 4.11-rc2 (or

Yes, sure. Please prepare a branch and once I reviewed all patches
touching i2c core, we can (from my side at least) declare it immutable
and I will pull it into i2c. I'll try to review the resources patch this
weekend.


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

Re: [PATCH 1/2] i2c: do not leave semaphore armed when copying properties fails

[Dmitry Torokhov]

Hi Wolfram,

On Fri, Mar 10, 2017 at 12:16:53AM +0100, Wolfram Sang wrote:
> 
> > I am perfectly fine with reverting b0c1e95ab44f from for-current,
> > however I wonder if we could have an immutable branch off 4.11-rc2 (or
> 
> Yes, sure. Please prepare a branch and once I reviewed all patches
> touching i2c core, we can (from my side at least) declare it immutable
> and I will pull it into i2c. I'll try to review the resources patch this
> weekend.
> 

Did you have a chance at looking the I2C IRQ resources patch? I prepared
a branch containing the 3 patches:

$ git log --oneline --reverse v4.11-rc3..HEAD
51f19be7f637 i2c: export i2c_client_type structure
5b57e4dd278e i2c: copy device properties when using i2c_register_board_info()
7f4bf035cb84 i2c: allow attaching IRQ resources to i2c_board_info

You can find it at:

git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git
4.11-rc3-i2c-irq-resources

or:

https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git/log/?h=4.11-rc3-i2c-irq-resources

If you are happy with the branch I'll add your acked-bys and mark it as
immutable so we can share it between our trees.

Thanks!

-- 
Dmitry