Skip to contentSkip to conversation

i2c-multi-instantiate: Adapt for INT3515 and alike

PATCH In review Andy Shevchenko 7 participants 38 messages Nov 28, 2018 – Dec 3, 2018

Subsystems (9)
USB ACPI IIO I2C subsystem host drivers +5 more subsystems
lore
Share & tools
lore

Andy Shevchenko

author
#0 lore raw

v5 cover i2c-multi-instantiate: Adapt for INT3515 and alike

Currently i2c-multi-instantiate driver does not support the case of INT3515 USB PD device, where:

- Interrupt() is used instead of GpioInt()
- All slaves may be the same from IP point of view
- There can be variadic amount of slaves

This series is addressing all above.

Note, series has been smoke tested (Heikki would do BAT or more) on Intel Coffee Lake system.

The idea is to push this either through PDx86 tree (needs Rafael's ACKs) or ACPI. In any case it needs tags from Heikki, Hans, Mika, Wolfram and Jonathan.

Testing and comments are warmly welcome.

In v5:
- unmess with patches 3 and 4 (there were squashed by my mistake)
- other than above no code change from v4

In v4:
- directly return -EPROBE_DEFER from i2c_acpi_new_device()
- due to above split error handling simplification to separate patches

In v3:
- revisit error codes returned by i2c_acpi_new_device() (Hans, Mika)
- add documentation to exported i2c_acpi_get_i2c_resource() (Mika)
- drop Hans' Rb tag in patch 5 due to changes
- append Mika's Ab tag to patch 6

In v2:
- drop patches to copy fwnode and to handle BOSC0200 device
- append check for amount of devices listed in the driver's mapping
- add Rb tag given by Hans

Andy Shevchenko (15):
platform/x86: intel_cht_int33fe: Remove duplicate NULL check platform/x86: intel_cht_int33fe: Accept errors of

    i2c_acpi_new_device()
  platform/x86: i2c-multi-instantiate: Accept errors of
    i2c_acpi_new_device()

platform/x86: i2c-multi-instantiate: Defer probe when no adapter found
i2c: acpi: Return error pointers from i2c_acpi_new_device() platform/x86: intel_cht_int33fe: Get rid of obsolete conditional platform/x86: i2c-multi-instantiate: Get rid of obsolete conditional
i2c: acpi: Use ACPI_FAILURE instead of !ACPI_SUCCESS
i2c: acpi: Introduce i2c_acpi_get_i2c_resource() helper platform/x86: i2c-multi-instantiate: Count I2cSerialBus() resources platform/x86: i2c-multi-instantiate: Distinguish IRQ resource type platform/x86: i2c-multi-instantiate: Introduce IOAPIC IRQ support platform/x86: i2c-multi-instantiate: Allow to have same slaves ACPI / scan: Create platform device for INT3515 ACPI nodes
iio: inv_mpu6050: Use i2c_acpi_get_i2c_resource() helper

 drivers/acpi/scan.c                          |  1 +
 drivers/i2c/i2c-core-acpi.c                  | 64 +++++++++----
 drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c   | 16 ++--
 drivers/platform/x86/i2c-multi-instantiate.c | 97 ++++++++++++++++----
 drivers/platform/x86/intel_cht_int33fe.c     | 20 ++--
 drivers/usb/typec/tps6598x.c                 |  8 +-
 include/linux/acpi.h                         | 11 +++
 7 files changed, 157 insertions(+), 60 deletions(-)

--
2.19.2

author
#1 lore raw

PATCH v5 1/15 platform/x86: intel_cht_int33fe: Remove duplicate NULL check

Since i2c_unregister_device() became NULL-aware we may remove duplicate NULL check.

Signed-off-by Andy Shevchenko <[email protected]>
Reviewed-by Hans de Goede <[email protected]>
---
 drivers/platform/x86/intel_cht_int33fe.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/platform/x86/intel_cht_int33fe.c +2 -4
index 464fe93657b5..431151d4e611 100644
@@ -213,8 +213,7 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 	i2c_unregister_device(data->fusb302);
 
 out_unregister_max17047:
-	if (data->max17047)
-		i2c_unregister_device(data->max17047);
+	i2c_unregister_device(data->max17047);
 
 	device_connections_remove(data->connections);
 

@@ -227,8 +226,7 @@ static int cht_int33fe_remove(struct platform_device *pdev)
 
 	i2c_unregister_device(data->pi3usb30532);
 	i2c_unregister_device(data->fusb302);
-	if (data->max17047)
-		i2c_unregister_device(data->max17047);
+	i2c_unregister_device(data->max17047);
 
 	device_connections_remove(data->connections);
Signature · 1 line
2.19.2
author
#2 lore raw

PATCH v5 2/15 platform/x86: intel_cht_int33fe: Accept errors of i2c_acpi_new_device()

In reply to Andy Shevchenko

In the future i2c_acpi_new_device() will return error pointer in some cases. Prepare intel_cht_int33fe driver to support that.

Signed-off-by Andy Shevchenko <[email protected]>
Reviewed-by Hans de Goede <[email protected]>
---
 drivers/platform/x86/intel_cht_int33fe.c | 28 +++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)
drivers/platform/x86/intel_cht_int33fe.c +23 -5
index 431151d4e611..367d6e304ade 100644
@@ -168,8 +168,14 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 		board_info.dev_name = "max17047";
 		board_info.properties = max17047_props;
 		data->max17047 = i2c_acpi_new_device(dev, 1, &board_info);
-		if (!data->max17047)
-			return -EPROBE_DEFER; /* Wait for i2c-adapter to load */
+		if (IS_ERR(data->max17047))
+			ret = PTR_ERR(data->max17047);
+		else if (!data->max17047)
+			ret = -EPROBE_DEFER; /* Wait for i2c-adapter to load */
+		else
+			ret = 0;
+		if (ret)
+			return ret;
 	}
 
 	data->connections[0].endpoint[0] = "port0";

@@ -194,7 +200,13 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 	board_info.irq = fusb302_irq;
 
 	data->fusb302 = i2c_acpi_new_device(dev, 2, &board_info);
-	if (!data->fusb302)
+	if (IS_ERR(data->fusb302))
+		ret = PTR_ERR(data->fusb302);
+	else if (!data->fusb302)
+		ret = -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
+	else
+		ret = 0;
+	if (ret)
 		goto out_unregister_max17047;
 
 	memset(&board_info, 0, sizeof(board_info));

@@ -202,7 +214,13 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 	strlcpy(board_info.type, "pi3usb30532", I2C_NAME_SIZE);
 
 	data->pi3usb30532 = i2c_acpi_new_device(dev, 3, &board_info);
-	if (!data->pi3usb30532)
+	if (IS_ERR(data->pi3usb30532))
+		ret = PTR_ERR(data->pi3usb30532);
+	else if (!data->pi3usb30532)
+		ret = -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
+	else
+		ret = 0;
+	if (ret)
 		goto out_unregister_fusb302;
 
 	platform_set_drvdata(pdev, data);

@@ -217,7 +235,7 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 
 	device_connections_remove(data->connections);
 
-	return -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
+	return ret;
 }
 
 static int cht_int33fe_remove(struct platform_device *pdev)
Signature · 1 line
2.19.2
maintainer
#3 lore raw

Re: [PATCH v5 02/15] platform/x86: intel_cht_int33fe: Accept errors of i2c_acpi_new_device()

Quoted 5 lines · On Wed, 28 Nov 2018 13:45:22 +0200 Andy Shevchenko…

In the future i2c_acpi_new_device() will return error pointer in some cases. Prepare intel_cht_int33fe driver to support that.

Signed-off-by Andy Shevchenko <[email protected]>
Reviewed-by Hans de Goede <[email protected]>

Really trivial comment inline. I haven't checked back to see if there has been any previous discussion on that bit of code.

Otherwise looks sensible to me.

Jonathan

Quoted 9 lines
---
 drivers/platform/x86/intel_cht_int33fe.c | 28 +++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)
drivers/platform/x86/intel_cht_int33fe.c +0 -0
index 431151d4e611..367d6e304ade 100644

drivers/platform/x86/intel_cht_int33fe.c static int cht_int33fe_probe(struct platform_device *pdev) · line 172

Quoted 7 lines
drivers/platform/x86/intel_cht_int33fe.c +2 -2
@@ -168,8 +168,14 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 		board_info.dev_name = "max17047";
 		board_info.properties = max17047_props;
 		data->max17047 = i2c_acpi_new_device(dev, 1, &board_info);
-		if (!data->max17047)
-			return -EPROBE_DEFER; /* Wait for i2c-adapter to load */
+		if (IS_ERR(data->max17047))
+			ret = PTR_ERR(data->max17047);

Every so slightly nicer to just return directly in these error cases?

drivers/platform/x86/intel_cht_int33fe.c static int cht_int33fe_probe(struct platform_device *pdev) · line 176

Quoted 4 lines
drivers/platform/x86/intel_cht_int33fe.c +4 -0
+		else if (!data->max17047)
+			ret = -EPROBE_DEFER; /* Wait for i2c-adapter to load */
+		else
+			ret = 0;

Particularly so as then you don't need to set this ret as it's set in all paths where it is used below anyway...

Quoted 44 lines
drivers/platform/x86/intel_cht_int33fe.c +17 -3
+		if (ret)
+			return ret;
 	}
 
 	data->connections[0].endpoint[0] = "port0";

@@ -194,7 +200,13 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 	board_info.irq = fusb302_irq;
 
 	data->fusb302 = i2c_acpi_new_device(dev, 2, &board_info);
-	if (!data->fusb302)
+	if (IS_ERR(data->fusb302))
+		ret = PTR_ERR(data->fusb302);
+	else if (!data->fusb302)
+		ret = -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
+	else
+		ret = 0;
+	if (ret)
 		goto out_unregister_max17047;
 
 	memset(&board_info, 0, sizeof(board_info));

@@ -202,7 +214,13 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 	strlcpy(board_info.type, "pi3usb30532", I2C_NAME_SIZE);
 
 	data->pi3usb30532 = i2c_acpi_new_device(dev, 3, &board_info);
-	if (!data->pi3usb30532)
+	if (IS_ERR(data->pi3usb30532))
+		ret = PTR_ERR(data->pi3usb30532);
+	else if (!data->pi3usb30532)
+		ret = -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
+	else
+		ret = 0;
+	if (ret)
 		goto out_unregister_fusb302;
 
 	platform_set_drvdata(pdev, data);

@@ -217,7 +235,7 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 
 	device_connections_remove(data->connections);
 
-	return -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
+	return ret;
 }
 
 static int cht_int33fe_remove(struct platform_device *pdev)
author
#4 lore raw

Re: [PATCH v5 02/15] platform/x86: intel_cht_int33fe: Accept errors of i2c_acpi_new_device()

Quoted 20 lines · Jonathan Cameron
Quoted 5 lines · On Wed, 28 Nov 2018 13:45:22 +0200 Andy Shevchenko…

In the future i2c_acpi_new_device() will return error pointer in some cases. Prepare intel_cht_int33fe driver to support that.

Signed-off-by Andy Shevchenko <[email protected]>
Reviewed-by Hans de Goede <[email protected]>

Really trivial comment inline. I haven't checked back to see if there has been any previous discussion on that bit of code.

Otherwise looks sensible to me.

drivers/platform/x86/intel_cht_int33fe.c static int cht_int33fe_probe(struct platform_device *pdev) · line 172

Quoted 5 lines
drivers/platform/x86/intel_cht_int33fe.c +2 -2
 		data->max17047 = i2c_acpi_new_device(dev, 1, &board_info);
-		if (!data->max17047)
-			return -EPROBE_DEFER; /* Wait for i2c-adapter to load */
+		if (IS_ERR(data->max17047))
+			ret = PTR_ERR(data->max17047);

Every so slightly nicer to just return directly in these error cases?

Quoted 7 lines

drivers/platform/x86/intel_cht_int33fe.c static int cht_int33fe_probe(struct platform_device *pdev) · line 176

Quoted 4 lines
drivers/platform/x86/intel_cht_int33fe.c +4 -0
+		else if (!data->max17047)
+			ret = -EPROBE_DEFER; /* Wait for i2c-adapter to load */
+		else
+			ret = 0;

Particularly so as then you don't need to set this ret as it's set in all paths where it is used below anyway...

It doesn't matter much, since the logically it's split to 3 stages:
- preparation
- change API
- drop obsoleted pieces of code

This one is preparation stage. After 3rd stage it will be as you suggested.

Quoted 2 lines
Quoted 2 lines
drivers/platform/x86/intel_cht_int33fe.c +2 -0
+		if (ret)
+			return ret;
Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#5 lore raw

PATCH v5 3/15 platform/x86: i2c-multi-instantiate: Accept errors of i2c_acpi_new_device()

In reply to Andy Shevchenko

In the future i2c_acpi_new_device() will return error pointer in some cases. Prepare i2c-multi-instantiate driver to support that.

Signed-off-by Andy Shevchenko <[email protected]>
Reviewed-by Hans de Goede <[email protected]>
---
 drivers/platform/x86/i2c-multi-instantiate.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/platform/x86/i2c-multi-instantiate.c +7 -2
index 5456581b473c..e3345da82c84 100644
@@ -72,9 +72,14 @@ static int i2c_multi_inst_probe(struct platform_device *pdev)
 			board_info.irq = ret;
 		}
 		multi->clients[i] = i2c_acpi_new_device(dev, i, &board_info);
-		if (!multi->clients[i]) {
-			dev_err(dev, "Error creating i2c-client, idx %d\n", i);
+		if (IS_ERR(multi->clients[i]))
+			ret = PTR_ERR(multi->clients[i]);
+		else if (!multi->clients[i])
 			ret = -ENODEV;
+		else
+			ret = 0;
+		if (ret) {
+			dev_err(dev, "Error creating i2c-client, idx %d\n", i);
 			goto error;
 		}
 	}
Signature · 1 line
2.19.2
author
#6 lore raw

PATCH v5 4/15 platform/x86: i2c-multi-instantiate: Defer probe when no adapter found

In reply to Andy Shevchenko

Likewise the rest of the i2c_acpi_new_device() users, defer the probe of the i2c-multi-intantiate driver in case adapter is not yet found.

Signed-off-by Andy Shevchenko <[email protected]>
Reviewed-by Hans de Goede <[email protected]>
---
 drivers/platform/x86/i2c-multi-instantiate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/platform/x86/i2c-multi-instantiate.c +1 -1
index e3345da82c84..16a0eabe1e31 100644
@@ -75,7 +75,7 @@ static int i2c_multi_inst_probe(struct platform_device *pdev)
 		if (IS_ERR(multi->clients[i]))
 			ret = PTR_ERR(multi->clients[i]);
 		else if (!multi->clients[i])
-			ret = -ENODEV;
+			ret = -EPROBE_DEFER; /* Wait for i2c-adapter to load */
 		else
 			ret = 0;
 		if (ret) {
Signature · 1 line
2.19.2
author
#7 lore raw

PATCH v5 5/15 i2c: acpi: Return error pointers from i2c_acpi_new_device()

In reply to Andy Shevchenko

The caller would like to know the reason why the i2c_acpi_new_device() fails. For example, if adapter is not available, it might be in the future and we would like to re-probe the clients again. But at the same time we would like to bail out if the error seems unrecoverable, such as invalid argument supplied. To achieve this, return error pointer in some cases.

Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/i2c/i2c-core-acpi.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)
drivers/i2c/i2c-core-acpi.c +15 -6
index 32affd3fa8bd..689c0c467e97 100644
@@ -386,20 +386,22 @@ struct notifier_block i2c_acpi_notifier = {
  *
  * Also see i2c_new_device, which this function calls to create the i2c-client.
  *
- * Returns a pointer to the new i2c-client, or NULL if the adapter is not found.
+ * Returns a pointer to the new i2c-client, or error pointer in case of failure.
+ * Specifically, -EPROBE_DEFER is returned if the adapter is not found.
  */
 struct i2c_client *i2c_acpi_new_device(struct device *dev, int index,
 				       struct i2c_board_info *info)
 {
 	struct i2c_acpi_lookup lookup;
 	struct i2c_adapter *adapter;
+	struct i2c_client *client;
 	struct acpi_device *adev;
 	LIST_HEAD(resource_list);
 	int ret;
 
 	adev = ACPI_COMPANION(dev);
 	if (!adev)
-		return NULL;
+		return ERR_PTR(-EINVAL);
 
 	memset(&lookup, 0, sizeof(lookup));
 	lookup.info = info;

@@ -408,16 +410,23 @@ struct i2c_client *i2c_acpi_new_device(struct device *dev, int index,
 
 	ret = acpi_dev_get_resources(adev, &resource_list,
 				     i2c_acpi_fill_info, &lookup);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
 	acpi_dev_free_resource_list(&resource_list);
 
-	if (ret < 0 || !info->addr)
-		return NULL;
+	if (!info->addr)
+		return ERR_PTR(-EADDRNOTAVAIL);
 
 	adapter = i2c_acpi_find_adapter_by_handle(lookup.adapter_handle);
 	if (!adapter)
-		return NULL;
+		return ERR_PTR(-EPROBE_DEFER);
+
+	client = i2c_new_device(adapter, info);
+	if (!client)
+		return ERR_PTR(-ENODEV);
 
-	return i2c_new_device(adapter, info);
+	return client;
 }
 EXPORT_SYMBOL_GPL(i2c_acpi_new_device);
Signature · 1 line
2.19.2
Acked-by on v5 5/15
#8 lore raw

Re: [PATCH v5 05/15] i2c: acpi: Return error pointers from i2c_acpi_new_device()

Quoted 7 lines · Andy Shevchenko

The caller would like to know the reason why the i2c_acpi_new_device() fails. For example, if adapter is not available, it might be in the future and we would like to re-probe the clients again. But at the same time we would like to bail out if the error seems unrecoverable, such as invalid argument supplied. To achieve this, return error pointer in some cases.

Signed-off-by Andy Shevchenko <[email protected]>
Acked-by Mika Westerberg <[email protected]>
Acked-by on v5 5/15
#9 lore raw

Re: [PATCH v5 05/15] i2c: acpi: Return error pointers from i2c_acpi_new_device()

In reply to Andy Shevchenko
Quoted 7 lines · Andy Shevchenko

The caller would like to know the reason why the i2c_acpi_new_device() fails. For example, if adapter is not available, it might be in the future and we would like to re-probe the clients again. But at the same time we would like to bail out if the error seems unrecoverable, such as invalid argument supplied. To achieve this, return error pointer in some cases.

Signed-off-by Andy Shevchenko <[email protected]>
Acked-by Wolfram Sang <[email protected]>
author
#10 lore raw

PATCH v5 6/15 platform/x86: intel_cht_int33fe: Get rid of obsolete conditional

In reply to Andy Shevchenko

Now, when i2c_acpi_new_device() never returns NULL, there is no point to check for it. Besides that, i2c_acpi_new_device() returns -EPROBE_DEFER directly and caller doesn't need to guess is better.

Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/platform/x86/intel_cht_int33fe.c | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)
drivers/platform/x86/intel_cht_int33fe.c +5 -19
index 367d6e304ade..616b8853a91f 100644
@@ -169,13 +169,7 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 		board_info.properties = max17047_props;
 		data->max17047 = i2c_acpi_new_device(dev, 1, &board_info);
 		if (IS_ERR(data->max17047))
-			ret = PTR_ERR(data->max17047);
-		else if (!data->max17047)
-			ret = -EPROBE_DEFER; /* Wait for i2c-adapter to load */
-		else
-			ret = 0;
-		if (ret)
-			return ret;
+			return PTR_ERR(data->max17047);
 	}
 
 	data->connections[0].endpoint[0] = "port0";

@@ -200,28 +194,20 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 	board_info.irq = fusb302_irq;
 
 	data->fusb302 = i2c_acpi_new_device(dev, 2, &board_info);
-	if (IS_ERR(data->fusb302))
+	if (IS_ERR(data->fusb302)) {
 		ret = PTR_ERR(data->fusb302);
-	else if (!data->fusb302)
-		ret = -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
-	else
-		ret = 0;
-	if (ret)
 		goto out_unregister_max17047;
+	}
 
 	memset(&board_info, 0, sizeof(board_info));
 	board_info.dev_name = "pi3usb30532";
 	strlcpy(board_info.type, "pi3usb30532", I2C_NAME_SIZE);
 
 	data->pi3usb30532 = i2c_acpi_new_device(dev, 3, &board_info);
-	if (IS_ERR(data->pi3usb30532))
+	if (IS_ERR(data->pi3usb30532)) {
 		ret = PTR_ERR(data->pi3usb30532);
-	else if (!data->pi3usb30532)
-		ret = -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
-	else
-		ret = 0;
-	if (ret)
 		goto out_unregister_fusb302;
+	}
 
 	platform_set_drvdata(pdev, data);
Signature · 1 line
2.19.2
maintainer
#11 lore raw

Re: [PATCH v5 06/15] platform/x86: intel_cht_int33fe: Get rid of obsolete conditional

Quoted 5 lines · On Wed, 28 Nov 2018 13:45:26 +0200 Andy Shevchenko…

Now, when i2c_acpi_new_device() never returns NULL, there is no point to check for it. Besides that, i2c_acpi_new_device() returns -EPROBE_DEFER directly and caller doesn't need to guess is better.

Signed-off-by Andy Shevchenko <[email protected]>

Ah, the code I commented on earlier goes away anyway. Not to worry then!

Jonathan

Quoted 56 lines
---
 drivers/platform/x86/intel_cht_int33fe.c | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)
drivers/platform/x86/intel_cht_int33fe.c +5 -19
index 367d6e304ade..616b8853a91f 100644
@@ -169,13 +169,7 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 		board_info.properties = max17047_props;
 		data->max17047 = i2c_acpi_new_device(dev, 1, &board_info);
 		if (IS_ERR(data->max17047))
-			ret = PTR_ERR(data->max17047);
-		else if (!data->max17047)
-			ret = -EPROBE_DEFER; /* Wait for i2c-adapter to load */
-		else
-			ret = 0;
-		if (ret)
-			return ret;
+			return PTR_ERR(data->max17047);
 	}
 
 	data->connections[0].endpoint[0] = "port0";

@@ -200,28 +194,20 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 	board_info.irq = fusb302_irq;
 
 	data->fusb302 = i2c_acpi_new_device(dev, 2, &board_info);
-	if (IS_ERR(data->fusb302))
+	if (IS_ERR(data->fusb302)) {
 		ret = PTR_ERR(data->fusb302);
-	else if (!data->fusb302)
-		ret = -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
-	else
-		ret = 0;
-	if (ret)
 		goto out_unregister_max17047;
+	}
 
 	memset(&board_info, 0, sizeof(board_info));
 	board_info.dev_name = "pi3usb30532";
 	strlcpy(board_info.type, "pi3usb30532", I2C_NAME_SIZE);
 
 	data->pi3usb30532 = i2c_acpi_new_device(dev, 3, &board_info);
-	if (IS_ERR(data->pi3usb30532))
+	if (IS_ERR(data->pi3usb30532)) {
 		ret = PTR_ERR(data->pi3usb30532);
-	else if (!data->pi3usb30532)
-		ret = -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
-	else
-		ret = 0;
-	if (ret)
 		goto out_unregister_fusb302;
+	}
 
 	platform_set_drvdata(pdev, data);
author
#12 lore raw

PATCH v5 7/15 platform/x86: i2c-multi-instantiate: Get rid of obsolete conditional

In reply to Andy Shevchenko

Now, when i2c_acpi_new_device() never returns NULL, there is no point to check for it. Besides that, i2c_acpi_new_device() returns -EPROBE_DEFER directly and caller doesn't need to guess is better.

Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/platform/x86/i2c-multi-instantiate.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)
drivers/platform/x86/i2c-multi-instantiate.c +3 -7
index 16a0eabe1e31..724535673089 100644
@@ -72,14 +72,10 @@ static int i2c_multi_inst_probe(struct platform_device *pdev)
 			board_info.irq = ret;
 		}
 		multi->clients[i] = i2c_acpi_new_device(dev, i, &board_info);
-		if (IS_ERR(multi->clients[i]))
+		if (IS_ERR(multi->clients[i])) {
 			ret = PTR_ERR(multi->clients[i]);
-		else if (!multi->clients[i])
-			ret = -EPROBE_DEFER; /* Wait for i2c-adapter to load */
-		else
-			ret = 0;
-		if (ret) {
-			dev_err(dev, "Error creating i2c-client, idx %d\n", i);
+			if (ret != -EPROBE_DEFER)
+				dev_err(dev, "Error creating i2c-client, idx %d\n", i);
 			goto error;
 		}
 	}
Signature · 1 line
2.19.2
author
#13 lore raw

PATCH v5 8/15 i2c: acpi: Use ACPI_FAILURE instead of !ACPI_SUCCESS

In reply to Andy Shevchenko

Convert to use ACPI_FAILURE instead of !ACPI_SUCCESS.

Signed-off-by Andy Shevchenko <[email protected]>
Reviewed-by Hans de Goede <[email protected]>
Acked-by Mika Westerberg <[email protected]>
---
 drivers/i2c/i2c-core-acpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i2c/i2c-core-acpi.c +1 -1
index 689c0c467e97..8a88586e0902 100644
@@ -65,7 +65,7 @@ static int i2c_acpi_fill_info(struct acpi_resource *ares, void *data)
 	status = acpi_get_handle(lookup->device_handle,
 				 sb->resource_source.string_ptr,
 				 &lookup->adapter_handle);
-	if (!ACPI_SUCCESS(status))
+	if (ACPI_FAILURE(status))
 		return 1;
 
 	info->addr = sb->slave_address;
Signature · 1 line
2.19.2
author
#15 lore raw

PATCH v5 9/15 i2c: acpi: Introduce i2c_acpi_get_i2c_resource() helper

In reply to Andy Shevchenko

Besides current two users one more is coming. Definitely makes sense to introduce a helper.

No functional change intended.

Signed-off-by Andy Shevchenko <[email protected]>
Reviewed-by Hans de Goede <[email protected]>
---
 drivers/i2c/i2c-core-acpi.c | 41 ++++++++++++++++++++++++++-----------
 include/linux/acpi.h        | 11 ++++++++++
 2 files changed, 40 insertions(+), 12 deletions(-)
drivers/i2c/i2c-core-acpi.c +29 -12
index 8a88586e0902..272800692088 100644
@@ -45,6 +45,33 @@ struct i2c_acpi_lookup {
 	u32 min_speed;
 };
 
+/**
+ * i2c_acpi_get_i2c_resource - Gets I2cSerialBus resource if type matches
+ * @ares:	ACPI resource
+ * @i2c:	Pointer to I2cSerialBus resource will be returned here
+ *
+ * Checks if the given ACPI resource is of type I2cSerialBus.
+ * In this case, returns a pointer to it to the caller.
+ *
+ * Returns true if resource type is of I2cSerialBus, otherwise false.
+ */
+bool i2c_acpi_get_i2c_resource(struct acpi_resource *ares,
+			       struct acpi_resource_i2c_serialbus **i2c)
+{
+	struct acpi_resource_i2c_serialbus *sb;
+
+	if (ares->type != ACPI_RESOURCE_TYPE_SERIAL_BUS)
+		return false;
+
+	sb = &ares->data.i2c_serial_bus;
+	if (sb->type != ACPI_RESOURCE_SERIAL_TYPE_I2C)
+		return false;
+
+	*i2c = sb;
+	return true;
+}
+EXPORT_SYMBOL_GPL(i2c_acpi_get_i2c_resource);
+
 static int i2c_acpi_fill_info(struct acpi_resource *ares, void *data)
 {
 	struct i2c_acpi_lookup *lookup = data;

@@ -52,11 +79,7 @@ static int i2c_acpi_fill_info(struct acpi_resource *ares, void *data)
 	struct acpi_resource_i2c_serialbus *sb;
 	acpi_status status;
 
-	if (info->addr || ares->type != ACPI_RESOURCE_TYPE_SERIAL_BUS)
-		return 1;
-
-	sb = &ares->data.i2c_serial_bus;
-	if (sb->type != ACPI_RESOURCE_SERIAL_TYPE_I2C)
+	if (info->addr || !i2c_acpi_get_i2c_resource(ares, &sb))
 		return 1;
 
 	if (lookup->index != -1 && lookup->n++ != lookup->index)

@@ -534,13 +557,7 @@ i2c_acpi_space_handler(u32 function, acpi_physical_address command,
 		goto err;
 	}
 
-	if (!value64 || ares->type != ACPI_RESOURCE_TYPE_SERIAL_BUS) {
-		ret = AE_BAD_PARAMETER;
-		goto err;
-	}
-
-	sb = &ares->data.i2c_serial_bus;
-	if (sb->type != ACPI_RESOURCE_SERIAL_TYPE_I2C) {
+	if (!value64 || !i2c_acpi_get_i2c_resource(ares, &sb)) {
 		ret = AE_BAD_PARAMETER;
 		goto err;
 	}
include/linux/acpi.h +11 -0
index ed80f147bd50..6afc6e3c4c5c 100644
@@ -1054,6 +1054,17 @@ static inline int acpi_dev_gpio_irq_get(struct acpi_device *adev, int index)
 }
 #endif
 
+#if defined(CONFIG_ACPI) && IS_ENABLED(CONFIG_I2C)
+bool i2c_acpi_get_i2c_resource(struct acpi_resource *ares,
+			       struct acpi_resource_i2c_serialbus **i2c);
+#else
+static inline bool i2c_acpi_get_i2c_resource(struct acpi_resource *ares,
+					     struct acpi_resource_i2c_serialbus **i2c)
+{
+	return false;
+}
+#endif
+
 /* Device properties */
 
 #ifdef CONFIG_ACPI
Signature · 1 line
2.19.2
#17 lore raw

Re: [PATCH v5 09/15] i2c: acpi: Introduce i2c_acpi_get_i2c_resource() helper

In reply to Andy Shevchenko
Quoted 10 lines · Andy Shevchenko

Besides current two users one more is coming. Definitely makes sense to introduce a helper.

No functional change intended.

Signed-off-by Andy Shevchenko <[email protected]>
Reviewed-by Hans de Goede <[email protected]>
---
 drivers/i2c/i2c-core-acpi.c | 41 ++++++++++++++++++++++++++-----------
 include/linux/acpi.h        | 11 ++++++++++

Any reason this is not in i2c.h?

author
#18 lore raw

Re: [PATCH v5 09/15] i2c: acpi: Introduce i2c_acpi_get_i2c_resource() helper

Quoted 6 lines · Wolfram Sang
Quoted 5 lines · Andy Shevchenko

Besides current two users one more is coming. Definitely makes sense to introduce a helper.

No functional change intended.

Quoted 7 lines
Quoted 5 lines
Signed-off-by Andy Shevchenko <[email protected]>
Reviewed-by Hans de Goede <[email protected]>
---
 drivers/i2c/i2c-core-acpi.c | 41 ++++++++++++++++++++++++++-----------
 include/linux/acpi.h        | 11 ++++++++++

Any reason this is not in i2c.h?

Yes. As I explained earlier to some people there are facts affecting this:
- the function is operate on top of solely ACPI structures
- there is already similar function for GPIO and it stays like that

On top of this recently I've discovered, that i2c.h has separate #ifdef for I2C and ACPI which theoretically might produce a linker error in some cases.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#19 lore raw

Re: [PATCH v5 09/15] i2c: acpi: Introduce i2c_acpi_get_i2c_resource() helper

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
 drivers/i2c/i2c-core-acpi.c | 41 ++++++++++++++++++++++++++-----------
 include/linux/acpi.h        | 11 ++++++++++

Any reason this is not in i2c.h?

Yes. As I explained earlier to some people there are facts affecting this:
- the function is operate on top of solely ACPI structures

And moving the function itself to the ACPI realm then?

I don't say this is a show-stopper for this series, but I just wonder...

Quoted 4 lines

- there is already similar function for GPIO and it stays like that

On top of this recently I've discovered, that i2c.h has separate #ifdef for I2C and ACPI which theoretically might produce a linker error in some cases.

Is this something we have to live with or which can be cleaned up somewhen?

author
#20 lore raw

Re: [PATCH v5 09/15] i2c: acpi: Introduce i2c_acpi_get_i2c_resource() helper

Quoted 9 lines · Wolfram Sang
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
 drivers/i2c/i2c-core-acpi.c | 41 ++++++++++++++++++++++++++-----------
 include/linux/acpi.h        | 11 ++++++++++

Any reason this is not in i2c.h?

Yes. As I explained earlier to some people there are facts affecting this:
- the function is operate on top of solely ACPI structures

And moving the function itself to the ACPI realm then?

Unfortunately I don't see the place where it fits good. See below.

Quoted 9 lines

I don't say this is a show-stopper for this series, but I just wonder...

Quoted 4 lines

- there is already similar function for GPIO and it stays like that

On top of this recently I've discovered, that i2c.h has separate #ifdef for I2C and ACPI which theoretically might produce a linker error in some cases.

Is this something we have to live with or which can be cleaned up somewhen?

My opinion that we might need something like
drivers/acpi/acpi_i2c_lib.c
drivers/acpi/acpi_gpio_lib.c
etc.

But better to ask Rafael and Mika what they think about this.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#21 lore raw

Re: [PATCH v5 09/15] i2c: acpi: Introduce i2c_acpi_get_i2c_resource() helper

Quoted 9 lines
Quoted 3 lines

Is this something we have to live with or which can be cleaned up somewhen?

My opinion that we might need something like
drivers/acpi/acpi_i2c_lib.c
drivers/acpi/acpi_gpio_lib.c
etc.

Collect all of them in drivers/acpi/acpi_libs.c?

Quoted 1 line

But better to ask Rafael and Mika what they think about this.

Sure!

#22 lore raw

Re: [PATCH v5 09/15] i2c: acpi: Introduce i2c_acpi_get_i2c_resource() helper

Quoted 4 lines · Wolfram Sang
Quoted 4 lines
Quoted 3 lines

Is this something we have to live with or which can be cleaned up somewhen?

Quoted 9 lines
Quoted 5 lines

My opinion that we might need something like
drivers/acpi/acpi_i2c_lib.c
drivers/acpi/acpi_gpio_lib.c
etc.

Collect all of them in drivers/acpi/acpi_libs.c?

Quoted 1 line

But better to ask Rafael and Mika what they think about this.

IMHO all the bus specific ACPI things should go under those buses (so the opposite what is proposed here) but I also don't think any of this is show stopper for the patch series under discussion ;-)

author
#23 lore raw

Re: [PATCH v5 09/15] i2c: acpi: Introduce i2c_acpi_get_i2c_resource() helper

Quoted 11 lines · Mika Westerberg
Quoted 10 lines · Wolfram Sang
Quoted 9 lines
Quoted 3 lines

Is this something we have to live with or which can be cleaned up somewhen?

My opinion that we might need something like
drivers/acpi/acpi_i2c_lib.c
drivers/acpi/acpi_gpio_lib.c
etc.

Quoted 7 lines
Quoted 3 lines

Collect all of them in drivers/acpi/acpi_libs.c?

Quoted 1 line

But better to ask Rafael and Mika what they think about this.

IMHO all the bus specific ACPI things should go under those buses (so the opposite what is proposed here) but I also don't think any of this is show stopper for the patch series under discussion ;-)

Thank you guys for your input, let me fix this later, out of this series.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#25 lore raw

PATCH v5 10/15 platform/x86: i2c-multi-instantiate: Count I2cSerialBus() resources

In reply to Andy Shevchenko

Instead of relying on hard coded and thus expected number of I2C clients, count the real amount provided by firmware.

This allows to support non-fixed amount of the slaves.

Signed-off-by Andy Shevchenko <[email protected]>
Reviewed-by Hans de Goede <[email protected]>
---
 drivers/platform/x86/i2c-multi-instantiate.c | 41 ++++++++++++++++++--
 1 file changed, 37 insertions(+), 4 deletions(-)
drivers/platform/x86/i2c-multi-instantiate.c +37 -4
index 724535673089..d99bbaed0ca2 100644
@@ -12,6 +12,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/types.h>
 
 struct i2c_inst_data {
 	const char *type;

@@ -23,6 +24,31 @@ struct i2c_multi_inst_data {
 	struct i2c_client *clients[0];
 };
 
+static int i2c_multi_inst_count(struct acpi_resource *ares, void *data)
+{
+	struct acpi_resource_i2c_serialbus *sb;
+	int *count = data;
+
+	if (i2c_acpi_get_i2c_resource(ares, &sb))
+		*count = *count + 1;
+
+	return 1;
+}
+
+static int i2c_multi_inst_count_resources(struct acpi_device *adev)
+{
+	LIST_HEAD(r);
+	int count = 0;
+	int ret;
+
+	ret = acpi_dev_get_resources(adev, &r, i2c_multi_inst_count, &count);
+	if (ret < 0)
+		return ret;
+
+	acpi_dev_free_resource_list(&r);
+	return count;
+}
+
 static int i2c_multi_inst_probe(struct platform_device *pdev)
 {
 	struct i2c_multi_inst_data *multi;

@@ -44,17 +70,19 @@ static int i2c_multi_inst_probe(struct platform_device *pdev)
 	adev = ACPI_COMPANION(dev);
 
 	/* Count number of clients to instantiate */
-	for (i = 0; inst_data[i].type; i++) {}
+	ret = i2c_multi_inst_count_resources(adev);
+	if (ret < 0)
+		return ret;
 
 	multi = devm_kmalloc(dev,
-			offsetof(struct i2c_multi_inst_data, clients[i]),
+			offsetof(struct i2c_multi_inst_data, clients[ret]),
 			GFP_KERNEL);
 	if (!multi)
 		return -ENOMEM;
 
-	multi->num_clients = i;
+	multi->num_clients = ret;
 
-	for (i = 0; i < multi->num_clients; i++) {
+	for (i = 0; i < multi->num_clients && inst_data[i].type; i++) {
 		memset(&board_info, 0, sizeof(board_info));
 		strlcpy(board_info.type, inst_data[i].type, I2C_NAME_SIZE);
 		snprintf(name, sizeof(name), "%s-%s", match->id,

@@ -79,6 +107,11 @@ static int i2c_multi_inst_probe(struct platform_device *pdev)
 			goto error;
 		}
 	}
+	if (i < multi->num_clients) {
+		dev_err(dev, "Error finding driver, idx %d\n", i);
+		ret = -ENODEV;
+		goto error;
+	}
 
 	platform_set_drvdata(pdev, multi);
 	return 0;
Signature · 1 line
2.19.2
author
#26 lore raw

PATCH v5 11/15 platform/x86: i2c-multi-instantiate: Distinguish IRQ resource type

In reply to Andy Shevchenko

As a preparatory patch switch the driver to distinguish IRQ resource type. For now, only GpioInt() is supported.

Signed-off-by Andy Shevchenko <[email protected]>
Reviewed-by Hans de Goede <[email protected]>
---
 drivers/platform/x86/i2c-multi-instantiate.c | 27 +++++++++++++-------
 1 file changed, 18 insertions(+), 9 deletions(-)
drivers/platform/x86/i2c-multi-instantiate.c +18 -9
index d99bbaed0ca2..99db3e336718 100644
@@ -7,6 +7,7 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/bits.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>

@@ -14,9 +15,14 @@
 #include <linux/platform_device.h>
 #include <linux/types.h>
 
+#define IRQ_RESOURCE_TYPE	GENMASK(1, 0)
+#define IRQ_RESOURCE_NONE	0
+#define IRQ_RESOURCE_GPIO	1
+
 struct i2c_inst_data {
 	const char *type;
-	int gpio_irq_idx;
+	unsigned int flags;
+	int irq_idx;
 };
 
 struct i2c_multi_inst_data {

@@ -88,16 +94,19 @@ static int i2c_multi_inst_probe(struct platform_device *pdev)
 		snprintf(name, sizeof(name), "%s-%s", match->id,
 			 inst_data[i].type);
 		board_info.dev_name = name;
-		board_info.irq = 0;
-		if (inst_data[i].gpio_irq_idx != -1) {
-			ret = acpi_dev_gpio_irq_get(adev,
-						    inst_data[i].gpio_irq_idx);
+		switch (inst_data[i].flags & IRQ_RESOURCE_TYPE) {
+		case IRQ_RESOURCE_GPIO:
+			ret = acpi_dev_gpio_irq_get(adev, inst_data[i].irq_idx);
 			if (ret < 0) {
 				dev_err(dev, "Error requesting irq at index %d: %d\n",
-					inst_data[i].gpio_irq_idx, ret);
+					inst_data[i].irq_idx, ret);
 				goto error;
 			}
 			board_info.irq = ret;
+			break;
+		default:
+			board_info.irq = 0;
+			break;
 		}
 		multi->clients[i] = i2c_acpi_new_device(dev, i, &board_info);
 		if (IS_ERR(multi->clients[i])) {

@@ -135,9 +144,9 @@ static int i2c_multi_inst_remove(struct platform_device *pdev)
 }
 
 static const struct i2c_inst_data bsg1160_data[]  = {
-	{ "bmc150_accel", 0 },
-	{ "bmc150_magn", -1 },
-	{ "bmg160", -1 },
+	{ "bmc150_accel", IRQ_RESOURCE_GPIO, 0 },
+	{ "bmc150_magn" },
+	{ "bmg160" },
 	{}
 };
Signature · 1 line
2.19.2
author
#27 lore raw

PATCH v5 12/15 platform/x86: i2c-multi-instantiate: Introduce IOAPIC IRQ support

In reply to Andy Shevchenko

If ACPI table provides an Interrupt() resource we may consider to use it instead of GpioInt() one.

Here we leave an error condition, when getting IRQ resource, to the driver to decide how to proceed, because some drivers may consider IRQ resource optional.

Signed-off-by Andy Shevchenko <[email protected]>
Reviewed-by Hans de Goede <[email protected]>
---
 drivers/platform/x86/i2c-multi-instantiate.c | 9 +++++++++
 1 file changed, 9 insertions(+)
drivers/platform/x86/i2c-multi-instantiate.c +9 -0
index 99db3e336718..365457f9c424 100644
@@ -18,6 +18,7 @@
 #define IRQ_RESOURCE_TYPE	GENMASK(1, 0)
 #define IRQ_RESOURCE_NONE	0
 #define IRQ_RESOURCE_GPIO	1
+#define IRQ_RESOURCE_APIC	2
 
 struct i2c_inst_data {
 	const char *type;

@@ -104,6 +105,14 @@ static int i2c_multi_inst_probe(struct platform_device *pdev)
 			}
 			board_info.irq = ret;
 			break;
+		case IRQ_RESOURCE_APIC:
+			ret = platform_get_irq(pdev, inst_data[i].irq_idx);
+			if (ret < 0) {
+				dev_dbg(dev, "Error requesting irq at index %d: %d\n",
+					inst_data[i].irq_idx, ret);
+			}
+			board_info.irq = ret;
+			break;
 		default:
 			board_info.irq = 0;
 			break;
Signature · 1 line
2.19.2
author
#28 lore raw

PATCH v5 13/15 platform/x86: i2c-multi-instantiate: Allow to have same slaves

In reply to Andy Shevchenko

Currently the driver will not enumerate the devices where I2C slaves are of the same type.

Add an instance number to make them unique.

Signed-off-by Andy Shevchenko <[email protected]>
Reviewed-by Hans de Goede <[email protected]>
---
 drivers/platform/x86/i2c-multi-instantiate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/platform/x86/i2c-multi-instantiate.c +2 -2
index 365457f9c424..2439d4c71118 100644
@@ -92,8 +92,8 @@ static int i2c_multi_inst_probe(struct platform_device *pdev)
 	for (i = 0; i < multi->num_clients && inst_data[i].type; i++) {
 		memset(&board_info, 0, sizeof(board_info));
 		strlcpy(board_info.type, inst_data[i].type, I2C_NAME_SIZE);
-		snprintf(name, sizeof(name), "%s-%s", match->id,
-			 inst_data[i].type);
+		snprintf(name, sizeof(name), "%s-%s.%d", match->id,
+			 inst_data[i].type, i);
 		board_info.dev_name = name;
 		switch (inst_data[i].flags & IRQ_RESOURCE_TYPE) {
 		case IRQ_RESOURCE_GPIO:
Signature · 1 line
2.19.2
author
#29 lore raw

PATCH v5 14/15 ACPI / scan: Create platform device for INT3515 ACPI nodes

In reply to Andy Shevchenko

The ACPI device with INT3515 _HID is representing a complex USB PD hardware infrastructure which includes several I2C slave ICs.

We add an ID to the I2C multi instantiate list to enumerate all I2C slaves correctly.

Signed-off-by Andy Shevchenko <[email protected]>
Reviewed-by Hans de Goede <[email protected]>
---
 drivers/acpi/scan.c                          | 1 +
 drivers/platform/x86/i2c-multi-instantiate.c | 9 +++++++++
 drivers/usb/typec/tps6598x.c                 | 8 ++++----
 3 files changed, 14 insertions(+), 4 deletions(-)
drivers/acpi/scan.c +1 -0
index bd1c59fb0e17..e9eda5558c1f 100644
@@ -1541,6 +1541,7 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device)
 	static const struct acpi_device_id i2c_multi_instantiate_ids[] = {
 		{"BSG1160", },
 		{"INT33FE", },
+		{"INT3515", },
 		{}
 	};
drivers/platform/x86/i2c-multi-instantiate.c +9 -0
index 2439d4c71118..3d893e0ac250 100644
@@ -159,12 +159,21 @@ static const struct i2c_inst_data bsg1160_data[]  = {
 	{}
 };
 
+static const struct i2c_inst_data int3515_data[]  = {
+	{ "tps6598x", IRQ_RESOURCE_APIC, 0 },
+	{ "tps6598x", IRQ_RESOURCE_APIC, 1 },
+	{ "tps6598x", IRQ_RESOURCE_APIC, 2 },
+	{ "tps6598x", IRQ_RESOURCE_APIC, 3 },
+	{}
+};
+
 /*
  * Note new device-ids must also be added to i2c_multi_instantiate_ids in
  * drivers/acpi/scan.c: acpi_device_enumeration_by_parent().
  */
 static const struct acpi_device_id i2c_multi_inst_acpi_ids[] = {
 	{ "BSG1160", (unsigned long)bsg1160_data },
+	{ "INT3515", (unsigned long)int3515_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(acpi, i2c_multi_inst_acpi_ids);
drivers/usb/typec/tps6598x.c +4 -4
index c84c8c189e90..1c0033ad8738 100644
@@ -501,19 +501,19 @@ static int tps6598x_remove(struct i2c_client *client)
 	return 0;
 }
 
-static const struct acpi_device_id tps6598x_acpi_match[] = {
-	{ "INT3515", 0 },
+static const struct i2c_device_id tps6598x_id[] = {
+	{ "tps6598x" },
 	{ }
 };
-MODULE_DEVICE_TABLE(acpi, tps6598x_acpi_match);
+MODULE_DEVICE_TABLE(i2c, tps6598x_id);
 
 static struct i2c_driver tps6598x_i2c_driver = {
 	.driver = {
 		.name = "tps6598x",
-		.acpi_match_table = tps6598x_acpi_match,
 	},
 	.probe_new = tps6598x_probe,
 	.remove = tps6598x_remove,
+	.id_table = tps6598x_id,
 };
 module_i2c_driver(tps6598x_i2c_driver);
Signature · 1 line
2.19.2
maintainer Acked-by on v5 14/15
#30 lore raw

Re: [PATCH v5 14/15] ACPI / scan: Create platform device for INT3515 ACPI nodes

Quoted 8 lines · Andy Shevchenko

The ACPI device with INT3515 _HID is representing a complex USB PD hardware infrastructure which includes several I2C slave ICs.

We add an ID to the I2C multi instantiate list to enumerate all I2C slaves correctly.

Signed-off-by Andy Shevchenko <[email protected]>
Reviewed-by Hans de Goede <[email protected]>
Acked-by Heikki Krogerus <[email protected]>
Quoted 74 lines
---
 drivers/acpi/scan.c                          | 1 +
 drivers/platform/x86/i2c-multi-instantiate.c | 9 +++++++++
 drivers/usb/typec/tps6598x.c                 | 8 ++++----
 3 files changed, 14 insertions(+), 4 deletions(-)
drivers/acpi/scan.c +1 -0
index bd1c59fb0e17..e9eda5558c1f 100644
@@ -1541,6 +1541,7 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device)
 	static const struct acpi_device_id i2c_multi_instantiate_ids[] = {
 		{"BSG1160", },
 		{"INT33FE", },
+		{"INT3515", },
 		{}
 	};
drivers/platform/x86/i2c-multi-instantiate.c +9 -0
index 2439d4c71118..3d893e0ac250 100644
@@ -159,12 +159,21 @@ static const struct i2c_inst_data bsg1160_data[]  = {
 	{}
 };
 
+static const struct i2c_inst_data int3515_data[]  = {
+	{ "tps6598x", IRQ_RESOURCE_APIC, 0 },
+	{ "tps6598x", IRQ_RESOURCE_APIC, 1 },
+	{ "tps6598x", IRQ_RESOURCE_APIC, 2 },
+	{ "tps6598x", IRQ_RESOURCE_APIC, 3 },
+	{}
+};
+
 /*
  * Note new device-ids must also be added to i2c_multi_instantiate_ids in
  * drivers/acpi/scan.c: acpi_device_enumeration_by_parent().
  */
 static const struct acpi_device_id i2c_multi_inst_acpi_ids[] = {
 	{ "BSG1160", (unsigned long)bsg1160_data },
+	{ "INT3515", (unsigned long)int3515_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(acpi, i2c_multi_inst_acpi_ids);
drivers/usb/typec/tps6598x.c +4 -4
index c84c8c189e90..1c0033ad8738 100644
@@ -501,19 +501,19 @@ static int tps6598x_remove(struct i2c_client *client)
 	return 0;
 }
 
-static const struct acpi_device_id tps6598x_acpi_match[] = {
-	{ "INT3515", 0 },
+static const struct i2c_device_id tps6598x_id[] = {
+	{ "tps6598x" },
 	{ }
 };
-MODULE_DEVICE_TABLE(acpi, tps6598x_acpi_match);
+MODULE_DEVICE_TABLE(i2c, tps6598x_id);
 
 static struct i2c_driver tps6598x_i2c_driver = {
 	.driver = {
 		.name = "tps6598x",
-		.acpi_match_table = tps6598x_acpi_match,
 	},
 	.probe_new = tps6598x_probe,
 	.remove = tps6598x_remove,
+	.id_table = tps6598x_id,
 };
 module_i2c_driver(tps6598x_i2c_driver);
Signature · 1 line
2.19.2
Signature · 1 line
heikki
maintainer Acked-by on v5 14/15
#31 lore raw

Re: [PATCH v5 14/15] ACPI / scan: Create platform device for INT3515 ACPI nodes

In reply to Andy Shevchenko
Quoted 9 lines · Andy Shevchenko

The ACPI device with INT3515 _HID is representing a complex USB PD hardware infrastructure which includes several I2C slave ICs.

We add an ID to the I2C multi instantiate list to enumerate all I2C slaves correctly.

Signed-off-by Andy Shevchenko <[email protected]>
Reviewed-by Hans de Goede <[email protected]>
Acked-by Rafael J. Wysocki <[email protected]>

and I'm assuming that you will route the whole series through the platform/x86 drivers tree.

author
#32 lore raw

Re: [PATCH v5 14/15] ACPI / scan: Create platform device for INT3515 ACPI nodes

Quoted 9 lines · Rafael J. Wysocki
Quoted 7 lines · Andy Shevchenko

The ACPI device with INT3515 _HID is representing a complex USB PD hardware infrastructure which includes several I2C slave ICs.

We add an ID to the I2C multi instantiate list to enumerate all I2C slaves correctly.

Quoted 7 lines
Quoted 2 lines
Signed-off-by Andy Shevchenko <[email protected]>
Reviewed-by Hans de Goede <[email protected]>
Acked-by Rafael J. Wysocki <[email protected]>

and I'm assuming that you will route the whole series through the platform/x86 drivers tree.

Thanks, Rafael, will route through PDx86!

Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#33 lore raw

PATCH v5 15/15 iio: inv_mpu6050: Use i2c_acpi_get_i2c_resource() helper

In reply to Andy Shevchenko

ACPI provides a generic helper to get I2C Serial Bus resources. Use it instead of open coded variant.

Signed-off-by Andy Shevchenko <[email protected]>
Reviewed-by Hans de Goede <[email protected]>
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)
drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c +6 -10
index d78a10403bac..a961b5a06fe6 100644
@@ -91,18 +91,14 @@ static int asus_acpi_get_sensor_info(struct acpi_device *adev,
 
 static int acpi_i2c_check_resource(struct acpi_resource *ares, void *data)
 {
+	struct acpi_resource_i2c_serialbus *sb;
 	u32 *addr = data;
 
-	if (ares->type == ACPI_RESOURCE_TYPE_SERIAL_BUS) {
-		struct acpi_resource_i2c_serialbus *sb;
-
-		sb = &ares->data.i2c_serial_bus;
-		if (sb->type == ACPI_RESOURCE_SERIAL_TYPE_I2C) {
-			if (*addr)
-				*addr |= (sb->slave_address << 16);
-			else
-				*addr = sb->slave_address;
-		}
+	if (i2c_acpi_get_i2c_resource(ares, &sb)) {
+		if (*addr)
+			*addr |= (sb->slave_address << 16);
+		else
+			*addr = sb->slave_address;
 	}
 
 	/* Tell the ACPI core that we already copied this address */
Signature · 1 line
2.19.2
maintainer Acked-by on v5 15/15
#34 lore raw

Re: [PATCH v5 15/15] iio: inv_mpu6050: Use i2c_acpi_get_i2c_resource() helper

+CC linux-iio mostly so I can track it and because others may care.

Quoted 5 lines · On Wed, 28 Nov 2018 13:45:35 +0200 Andy Shevchenko…

ACPI provides a generic helper to get I2C Serial Bus resources. Use it instead of open coded variant.

Signed-off-by Andy Shevchenko <[email protected]>
Reviewed-by Hans de Goede <[email protected]>
Acked-by Jonathan Cameron <[email protected]>

Thanks,

Jonathan

Quoted 33 lines
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)
drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c +6 -10
index d78a10403bac..a961b5a06fe6 100644
@@ -91,18 +91,14 @@ static int asus_acpi_get_sensor_info(struct acpi_device *adev,
 
 static int acpi_i2c_check_resource(struct acpi_resource *ares, void *data)
 {
+	struct acpi_resource_i2c_serialbus *sb;
 	u32 *addr = data;
 
-	if (ares->type == ACPI_RESOURCE_TYPE_SERIAL_BUS) {
-		struct acpi_resource_i2c_serialbus *sb;
-
-		sb = &ares->data.i2c_serial_bus;
-		if (sb->type == ACPI_RESOURCE_SERIAL_TYPE_I2C) {
-			if (*addr)
-				*addr |= (sb->slave_address << 16);
-			else
-				*addr = sb->slave_address;
-		}
+	if (i2c_acpi_get_i2c_resource(ares, &sb)) {
+		if (*addr)
+			*addr |= (sb->slave_address << 16);
+		else
+			*addr = sb->slave_address;
 	}
 
 	/* Tell the ACPI core that we already copied this address */
maintainer Reviewed-by on the v5 series
#35 lore raw
In reply to Andy Shevchenko
Quoted 9 lines · Andy Shevchenko

Currently i2c-multi-instantiate driver does not support the case of INT3515 USB PD device, where:

- Interrupt() is used instead of GpioInt()
- All slaves may be the same from IP point of view
- There can be variadic amount of slaves

This series is addressing all above.

Quoted 7 lines

Note, series has been smoke tested (Heikki would do BAT or more) on Intel Coffee Lake system.

The idea is to push this either through PDx86 tree (needs Rafael's ACKs) or ACPI. In any case it needs tags from Heikki, Hans, Mika, Wolfram and Jonathan.

Testing and comments are warmly welcome.

Looks good to me. The series, FWIW:

Reviewed-by Heikki Krogerus <[email protected]>
Quoted 49 lines

In v5:
- unmess with patches 3 and 4 (there were squashed by my mistake)
- other than above no code change from v4

In v4:
- directly return -EPROBE_DEFER from i2c_acpi_new_device()
- due to above split error handling simplification to separate patches

In v3:
- revisit error codes returned by i2c_acpi_new_device() (Hans, Mika)
- add documentation to exported i2c_acpi_get_i2c_resource() (Mika)
- drop Hans' Rb tag in patch 5 due to changes
- append Mika's Ab tag to patch 6

In v2:
- drop patches to copy fwnode and to handle BOSC0200 device
- append check for amount of devices listed in the driver's mapping
- add Rb tag given by Hans

Andy Shevchenko (15):
platform/x86: intel_cht_int33fe: Remove duplicate NULL check platform/x86: intel_cht_int33fe: Accept errors of

    i2c_acpi_new_device()
  platform/x86: i2c-multi-instantiate: Accept errors of
    i2c_acpi_new_device()

platform/x86: i2c-multi-instantiate: Defer probe when no adapter found
i2c: acpi: Return error pointers from i2c_acpi_new_device() platform/x86: intel_cht_int33fe: Get rid of obsolete conditional platform/x86: i2c-multi-instantiate: Get rid of obsolete conditional
i2c: acpi: Use ACPI_FAILURE instead of !ACPI_SUCCESS
i2c: acpi: Introduce i2c_acpi_get_i2c_resource() helper platform/x86: i2c-multi-instantiate: Count I2cSerialBus() resources platform/x86: i2c-multi-instantiate: Distinguish IRQ resource type platform/x86: i2c-multi-instantiate: Introduce IOAPIC IRQ support platform/x86: i2c-multi-instantiate: Allow to have same slaves ACPI / scan: Create platform device for INT3515 ACPI nodes
iio: inv_mpu6050: Use i2c_acpi_get_i2c_resource() helper

 drivers/acpi/scan.c                          |  1 +
 drivers/i2c/i2c-core-acpi.c                  | 64 +++++++++----
 drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c   | 16 ++--
 drivers/platform/x86/i2c-multi-instantiate.c | 97 ++++++++++++++++----
 drivers/platform/x86/intel_cht_int33fe.c     | 20 ++--
 drivers/usb/typec/tps6598x.c                 |  8 +-
 include/linux/acpi.h                         | 11 +++
 7 files changed, 157 insertions(+), 60 deletions(-)

--
2.19.2

Signature · 1 line
heikki
Reviewed-by on the v5 series
#36 lore raw
In reply to Andy Shevchenko

Hi,

Quoted 12 lines · Andy Shevchenko

Currently i2c-multi-instantiate driver does not support the case of INT3515 USB PD device, where:

- Interrupt() is used instead of GpioInt()
- All slaves may be the same from IP point of view
- There can be variadic amount of slaves

This series is addressing all above.

Note, series has been smoke tested (Heikki would do BAT or more) on Intel Coffee Lake system.

Quoted 8 lines

The idea is to push this either through PDx86 tree (needs Rafael's ACKs) or ACPI. In any case it needs tags from Heikki, Hans, Mika, Wolfram and Jonathan.

Testing and comments are warmly welcome.

In v5:
- unmess with patches 3 and 4 (there were squashed by my mistake)
- other than above no code change from v4

v5 looks good to me, you can add my:

Reviewed-by Hans de Goede <[email protected]>

To the patches which don't have this yet.

Regards,

Hans

Quoted 47 lines

In v4:
- directly return -EPROBE_DEFER from i2c_acpi_new_device()
- due to above split error handling simplification to separate patches

In v3:
- revisit error codes returned by i2c_acpi_new_device() (Hans, Mika)
- add documentation to exported i2c_acpi_get_i2c_resource() (Mika)
- drop Hans' Rb tag in patch 5 due to changes
- append Mika's Ab tag to patch 6

In v2:
- drop patches to copy fwnode and to handle BOSC0200 device
- append check for amount of devices listed in the driver's mapping
- add Rb tag given by Hans

Andy Shevchenko (15):
platform/x86: intel_cht_int33fe: Remove duplicate NULL check platform/x86: intel_cht_int33fe: Accept errors of

     i2c_acpi_new_device()
   platform/x86: i2c-multi-instantiate: Accept errors of
     i2c_acpi_new_device()

platform/x86: i2c-multi-instantiate: Defer probe when no adapter found
i2c: acpi: Return error pointers from i2c_acpi_new_device() platform/x86: intel_cht_int33fe: Get rid of obsolete conditional platform/x86: i2c-multi-instantiate: Get rid of obsolete conditional
i2c: acpi: Use ACPI_FAILURE instead of !ACPI_SUCCESS
i2c: acpi: Introduce i2c_acpi_get_i2c_resource() helper platform/x86: i2c-multi-instantiate: Count I2cSerialBus() resources platform/x86: i2c-multi-instantiate: Distinguish IRQ resource type platform/x86: i2c-multi-instantiate: Introduce IOAPIC IRQ support platform/x86: i2c-multi-instantiate: Allow to have same slaves ACPI / scan: Create platform device for INT3515 ACPI nodes
iio: inv_mpu6050: Use i2c_acpi_get_i2c_resource() helper

  drivers/acpi/scan.c                          |  1 +
  drivers/i2c/i2c-core-acpi.c                  | 64 +++++++++----
  drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c   | 16 ++--
  drivers/platform/x86/i2c-multi-instantiate.c | 97 ++++++++++++++++----
  drivers/platform/x86/intel_cht_int33fe.c     | 20 ++--
  drivers/usb/typec/tps6598x.c                 |  8 +-
  include/linux/acpi.h                         | 11 +++
  7 files changed, 157 insertions(+), 60 deletions(-)

--
2.19.2

author
#37 lore raw
In reply to Andy Shevchenko
Quoted 9 lines · Andy Shevchenko

Currently i2c-multi-instantiate driver does not support the case of INT3515 USB PD device, where:

- Interrupt() is used instead of GpioInt()
- All slaves may be the same from IP point of view
- There can be variadic amount of slaves

This series is addressing all above.

Quoted 7 lines

Note, series has been smoke tested (Heikki would do BAT or more) on Intel Coffee Lake system.

The idea is to push this either through PDx86 tree (needs Rafael's ACKs) or ACPI. In any case it needs tags from Heikki, Hans, Mika, Wolfram and Jonathan.

Testing and comments are warmly welcome.

Thank you everyone, who helped with this series, pushed to my review and testing queue!

Quoted 51 lines

In v5:
- unmess with patches 3 and 4 (there were squashed by my mistake)
- other than above no code change from v4

In v4:
- directly return -EPROBE_DEFER from i2c_acpi_new_device()
- due to above split error handling simplification to separate patches

In v3:
- revisit error codes returned by i2c_acpi_new_device() (Hans, Mika)
- add documentation to exported i2c_acpi_get_i2c_resource() (Mika)
- drop Hans' Rb tag in patch 5 due to changes
- append Mika's Ab tag to patch 6

In v2:
- drop patches to copy fwnode and to handle BOSC0200 device
- append check for amount of devices listed in the driver's mapping
- add Rb tag given by Hans

Andy Shevchenko (15):
platform/x86: intel_cht_int33fe: Remove duplicate NULL check platform/x86: intel_cht_int33fe: Accept errors of

    i2c_acpi_new_device()
  platform/x86: i2c-multi-instantiate: Accept errors of
    i2c_acpi_new_device()

platform/x86: i2c-multi-instantiate: Defer probe when no adapter found
i2c: acpi: Return error pointers from i2c_acpi_new_device() platform/x86: intel_cht_int33fe: Get rid of obsolete conditional platform/x86: i2c-multi-instantiate: Get rid of obsolete conditional
i2c: acpi: Use ACPI_FAILURE instead of !ACPI_SUCCESS
i2c: acpi: Introduce i2c_acpi_get_i2c_resource() helper platform/x86: i2c-multi-instantiate: Count I2cSerialBus() resources platform/x86: i2c-multi-instantiate: Distinguish IRQ resource type platform/x86: i2c-multi-instantiate: Introduce IOAPIC IRQ support platform/x86: i2c-multi-instantiate: Allow to have same slaves ACPI / scan: Create platform device for INT3515 ACPI nodes
iio: inv_mpu6050: Use i2c_acpi_get_i2c_resource() helper

 drivers/acpi/scan.c                          |  1 +
 drivers/i2c/i2c-core-acpi.c                  | 64 +++++++++----
 drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c   | 16 ++--
 drivers/platform/x86/i2c-multi-instantiate.c | 97 ++++++++++++++++----
 drivers/platform/x86/intel_cht_int33fe.c     | 20 ++--
 drivers/usb/typec/tps6598x.c                 |  8 +-
 include/linux/acpi.h                         | 11 +++
 7 files changed, 157 insertions(+), 60 deletions(-)

--
2.19.2

Signature · 2 lines
With Best Regards,
Andy Shevchenko