linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/4] i2c-multi-instantiate pseudo driver
@ 2018-08-09 11:40 Hans de Goede
  2018-08-09 11:40 ` [PATCH v6 1/4] ACPI / scan: Initialize status to ACPI_STA_DEFAULT Hans de Goede
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Hans de Goede @ 2018-08-09 11:40 UTC (permalink / raw)
  To: Rafael J . Wysocki, Len Brown, Andy Shevchenko, Mika Westerberg,
	Darren Hart, Wolfram Sang
  Cc: Hans de Goede, Srinivas Pandruvada, linux-acpi,
	platform-driver-x86, linux-kernel, Heikki Krogerus, linux-i2c

Hi All,

Here is v6 of my patch-series for dealing with multiple independent i2c
devices being described in a single ACPI fwnode.

Changes in v6:
-Added some Reviewed-by and Acked-by-s to patches 2-4
-4th patch "platform/x86: Add ACPI i2c-multi-instantiate pseudo driver":
 -s/irq_idx/gpio_irq_idx/

Changes in v5:
-Only changes in the 4th patch "platform/x86: Add ACPI i2c-multi-instantiate pseudo driver":
 -s/no_clients/num_clients/

Changes in v4:
-Rewrite commit message of first patch as this is NOT a bug fix for
 acpi_is_indirect_io_slave() as I thought at first, it is still necessary
 as a preperation patch for the rest of the series
-MAINTAINER and Kconfig entry fixes
-Some minor coding style fixes

Changes in v3:
-Drop the patch adding the I2C_CLIENT_IGNORE_BUSY flag/hack
-Add patches to make ACPI fwnode-s with affected acpi_ids get enumerated as
 platform device instead of i2c-client
-Change the i2c-multi-instantiate driver to a platform driver

Changes in v2:
-Rebase on 4.18-rc2

Changes in v1:
-Second attempt at dealing with this using a new design where a separate
 driver (which can be build as a module) is used to instantiate the
 i2c-clients instead of modifying the i2c core
-Add an I2C_CLIENT_IGNORE_BUSY flag to make it possible for this driver to bind
 to the client instantiated for the first i2c resource and still create a new
 client using the i2c_client_id for the actual driver for the first resource,
 resulting in 2 clients at the same address (of which one is inactive)

Regards,

Hans


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v6 1/4] ACPI / scan: Initialize status to ACPI_STA_DEFAULT
  2018-08-09 11:40 [PATCH v6 0/4] i2c-multi-instantiate pseudo driver Hans de Goede
@ 2018-08-09 11:40 ` Hans de Goede
  2018-08-09 11:40 ` [PATCH v6 2/4] ACPI / scan: Create platform device for fwnodes with multiple i2c devices Hans de Goede
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Hans de Goede @ 2018-08-09 11:40 UTC (permalink / raw)
  To: Rafael J . Wysocki, Len Brown, Andy Shevchenko, Mika Westerberg,
	Darren Hart, Wolfram Sang
  Cc: Hans de Goede, Srinivas Pandruvada, linux-acpi,
	platform-driver-x86, linux-kernel, Heikki Krogerus, linux-i2c

Since commit 63347db0affa ("ACPI / scan: Use acpi_bus_get_status() to
initialize ACPI_TYPE_DEVICE devs") the status field of normal acpi_devices
gets set to 0 by acpi_bus_type_and_status() and filled with its actual
value later when acpi_add_single_object() calls acpi_bus_get_status().

This means that any acpi_match_device_ids() calls in between will always
fail with -ENOENT.

We already have a workaround for this, which temporary forces status to
ACPI_STA_DEFAULT in drivers/acpi/x86/utils.c: acpi_device_always_present()
and the next commit in this series adds another acpi_match_device_ids()
call between status being initialized as 0 and the acpi_bus_get_status()
call.

Rather then adding another workaround, this commit makes
acpi_bus_type_and_status() initialize status to ACPI_STA_DEFAULT, this is
safe to do as the only code looking at status between the initialization
and the acpi_bus_get_status() call is those acpi_match_device_ids() calls.

Note this does mean that we need to (re)set status to 0 in case the
acpi_bus_get_status() call fails.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
-New patch in v3 of this patch-set

Changes in v4:
-This is not a fix for acpi_is_indirect_io_slave() as I thought at first,
 acpi_is_indirect_io_slave() calls acpi_match_device_ids() on its parent
 device, where status is already set properly. Rewrite the commit message
 accordingly.
---
 drivers/acpi/scan.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 970dd87d347c..6799d00dd790 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1612,7 +1612,8 @@ static int acpi_add_single_object(struct acpi_device **child,
 	 * Note this must be done before the get power-/wakeup_dev-flags calls.
 	 */
 	if (type == ACPI_BUS_TYPE_DEVICE)
-		acpi_bus_get_status(device);
+		if (acpi_bus_get_status(device) < 0)
+			acpi_set_device_status(device, 0);
 
 	acpi_bus_get_power_flags(device);
 	acpi_bus_get_wakeup_device_flags(device);
@@ -1690,7 +1691,7 @@ static int acpi_bus_type_and_status(acpi_handle handle, int *type,
 		 * acpi_add_single_object updates this once we've an acpi_device
 		 * so that acpi_bus_get_status' quirk handling can be used.
 		 */
-		*sta = 0;
+		*sta = ACPI_STA_DEFAULT;
 		break;
 	case ACPI_TYPE_PROCESSOR:
 		*type = ACPI_BUS_TYPE_PROCESSOR;
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v6 2/4] ACPI / scan: Create platform device for fwnodes with multiple i2c devices
  2018-08-09 11:40 [PATCH v6 0/4] i2c-multi-instantiate pseudo driver Hans de Goede
  2018-08-09 11:40 ` [PATCH v6 1/4] ACPI / scan: Initialize status to ACPI_STA_DEFAULT Hans de Goede
@ 2018-08-09 11:40 ` Hans de Goede
  2018-08-09 11:40 ` [PATCH v6 3/4] ACPI / x86-utils: Remove status workaround from acpi_device_always_present() Hans de Goede
  2018-08-09 11:40 ` [PATCH v6 4/4] platform/x86: Add ACPI i2c-multi-instantiate pseudo driver Hans de Goede
  3 siblings, 0 replies; 8+ messages in thread
From: Hans de Goede @ 2018-08-09 11:40 UTC (permalink / raw)
  To: Rafael J . Wysocki, Len Brown, Andy Shevchenko, Mika Westerberg,
	Darren Hart, Wolfram Sang
  Cc: Hans de Goede, Srinivas Pandruvada, linux-acpi,
	platform-driver-x86, linux-kernel, Heikki Krogerus, linux-i2c

Some devices have multiple I2cSerialBus resources and for things to work
an i2c-client must be instantiated for each, each with its own
i2c_device_id.

Normally we only instantiate an i2c-client for the first resource, using
the ACPI HID as id.

This commit adds a list of HIDs of devices, which need multiple i2c-clients
instantiated from a single fwnode, to acpi_device_enumeration_by_parent and
makes acpi_device_enumeration_by_parent return false for these devices so
that a platform device will be instantiated.

This allows the drivers/platform/x86/i2c-multi-instantiate.c driver, which
knows which i2c_device_id to use for each resource, to bind to the fwnode
and initiate an i2c-client for each resource.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
-New patch in v3 of this patch-set

Changes in v4:
-Some small code-style fixes
---
 drivers/acpi/scan.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 6799d00dd790..b51913372e21 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1540,6 +1540,18 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device)
 {
 	struct list_head resource_list;
 	bool is_serial_bus_slave = false;
+	/*
+	 * These devices have multiple I2cSerialBus resources and an i2c-client
+	 * must be instantiated for each, each with its own i2c_device_id.
+	 * Normally we only instantiate an i2c-client for the first resource,
+	 * using the ACPI HID as id. These special cases are handled by the
+	 * drivers/platform/x86/i2c-multi-instantiate.c driver, which knows
+	 * which i2c_device_id to use for each resource.
+	 */
+	static const struct acpi_device_id i2c_multi_instantiate_ids[] = {
+		{"BSG1160", },
+		{}
+	};
 
 	if (acpi_is_indirect_io_slave(device))
 		return true;
@@ -1551,6 +1563,10 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device)
 	     fwnode_property_present(&device->fwnode, "baud")))
 		return true;
 
+	/* Instantiate a pdev for the i2c-multi-instantiate drv to bind to */
+	if (!acpi_match_device_ids(device, i2c_multi_instantiate_ids))
+		return false;
+
 	INIT_LIST_HEAD(&resource_list);
 	acpi_dev_get_resources(device, &resource_list,
 			       acpi_check_serial_bus_slave,
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v6 3/4] ACPI / x86-utils: Remove status workaround from acpi_device_always_present()
  2018-08-09 11:40 [PATCH v6 0/4] i2c-multi-instantiate pseudo driver Hans de Goede
  2018-08-09 11:40 ` [PATCH v6 1/4] ACPI / scan: Initialize status to ACPI_STA_DEFAULT Hans de Goede
  2018-08-09 11:40 ` [PATCH v6 2/4] ACPI / scan: Create platform device for fwnodes with multiple i2c devices Hans de Goede
@ 2018-08-09 11:40 ` Hans de Goede
  2018-08-09 11:40 ` [PATCH v6 4/4] platform/x86: Add ACPI i2c-multi-instantiate pseudo driver Hans de Goede
  3 siblings, 0 replies; 8+ messages in thread
From: Hans de Goede @ 2018-08-09 11:40 UTC (permalink / raw)
  To: Rafael J . Wysocki, Len Brown, Andy Shevchenko, Mika Westerberg,
	Darren Hart, Wolfram Sang
  Cc: Hans de Goede, Srinivas Pandruvada, linux-acpi,
	platform-driver-x86, linux-kernel, Heikki Krogerus, linux-i2c

Now that we init the status field to ACPI_STA_DEFAULT rather then to 0,
the workaround for acpi_match_device_ids() always returning -ENOENT when
status is 0 is no longer needed.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
-New patch in v3 of this patch-set
---
 drivers/acpi/x86/utils.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/acpi/x86/utils.c b/drivers/acpi/x86/utils.c
index ec5b0f190231..a4fb97d64b3b 100644
--- a/drivers/acpi/x86/utils.c
+++ b/drivers/acpi/x86/utils.c
@@ -103,13 +103,9 @@ static const struct always_present_id always_present_ids[] = {
 
 bool acpi_device_always_present(struct acpi_device *adev)
 {
-	u32 *status = (u32 *)&adev->status;
-	u32 old_status = *status;
 	bool ret = false;
 	unsigned int i;
 
-	/* acpi_match_device_ids checks status, so set it to default */
-	*status = ACPI_STA_DEFAULT;
 	for (i = 0; i < ARRAY_SIZE(always_present_ids); i++) {
 		if (acpi_match_device_ids(adev, always_present_ids[i].hid))
 			continue;
@@ -125,15 +121,9 @@ bool acpi_device_always_present(struct acpi_device *adev)
 		    !dmi_check_system(always_present_ids[i].dmi_ids))
 			continue;
 
-		if (old_status != ACPI_STA_DEFAULT) /* Log only once */
-			dev_info(&adev->dev,
-				 "Device [%s] is in always present list\n",
-				 adev->pnp.bus_id);
-
 		ret = true;
 		break;
 	}
-	*status = old_status;
 
 	return ret;
 }
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v6 4/4] platform/x86: Add ACPI i2c-multi-instantiate pseudo driver
  2018-08-09 11:40 [PATCH v6 0/4] i2c-multi-instantiate pseudo driver Hans de Goede
                   ` (2 preceding siblings ...)
  2018-08-09 11:40 ` [PATCH v6 3/4] ACPI / x86-utils: Remove status workaround from acpi_device_always_present() Hans de Goede
@ 2018-08-09 11:40 ` Hans de Goede
  2018-08-09 11:55   ` Rafael J. Wysocki
  3 siblings, 1 reply; 8+ messages in thread
From: Hans de Goede @ 2018-08-09 11:40 UTC (permalink / raw)
  To: Rafael J . Wysocki, Len Brown, Andy Shevchenko, Mika Westerberg,
	Darren Hart, Wolfram Sang
  Cc: Hans de Goede, Srinivas Pandruvada, linux-acpi,
	platform-driver-x86, linux-kernel, Heikki Krogerus, linux-i2c

On systems with ACPI instantiated i2c-clients, normally there is 1 fw_node
per i2c-device and that fw-node contains 1 I2cSerialBus resource for that 1
i2c-device.

But in some rare cases the manufacturer has decided to describe multiple
i2c-devices in a single ACPI fwnode with multiple I2cSerialBus resources.

An earlier attempt to fix this in the i2c-core resulted in a lot of extra
code to support this corner-case.

This commit introduces a new i2c-multi-instantiate driver which fixes this
in a different way. This new driver can be built as a module which will
only loaded on affected systems.

This driver will instantiate a new i2c-client per I2cSerialBus resource,
using the driver_data from the acpi_device_id it is binding to to tell it
which chip-type (and optional irq-resource) to use when instantiating.

Note this driver depends on a platform device being instantiated for the
ACPI fwnode, see the i2c_multi_instantiate_ids list of ACPI device-ids in
drivers/acpi/scan.c: acpi_device_enumeration_by_parent().

Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Rebase on top of 4.18-rc2

Changes in v3:
-Change from an i2c-driver using a hack to allow having multiple i2c clients
 at the same address to a platform-driver

Changes in v4:
-Tweak MAINTAINERS entry a bit

Changes in v5:
-s/no_clients/num_clients/
-Change patch Subject prefix to platform/x86

Changes in v6:
-s/irq_idx/gpio_irq_idx/
---
 MAINTAINERS                                  |   6 +
 drivers/platform/x86/Kconfig                 |  11 ++
 drivers/platform/x86/Makefile                |   1 +
 drivers/platform/x86/i2c-multi-instantiate.c | 132 +++++++++++++++++++
 4 files changed, 150 insertions(+)
 create mode 100644 drivers/platform/x86/i2c-multi-instantiate.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 9b377508f24f..dbe7836e4f6b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -367,6 +367,12 @@ L:	linux-acpi@vger.kernel.org
 S:	Maintained
 F:	drivers/acpi/arm64
 
+ACPI I2C MULTI INSTANTIATE DRIVER
+M:	Hans de Goede <hdegoede@redhat.com>
+L:	platform-driver-x86@vger.kernel.org
+S:	Maintained
+F:	drivers/platform/x86/i2c-multi-instantiate.c
+
 ACPI PMIC DRIVERS
 M:	"Rafael J. Wysocki" <rjw@rjwysocki.net>
 M:	Len Brown <lenb@kernel.org>
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 85a93453237c..64c82592d4b6 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1219,6 +1219,17 @@ config INTEL_CHTDC_TI_PWRBTN
 	  To compile this driver as a module, choose M here: the module
 	  will be called intel_chtdc_ti_pwrbtn.
 
+config I2C_MULTI_INSTANTIATE
+	tristate "I2C multi instantiate pseudo device driver"
+	depends on I2C && ACPI
+	help
+	  Some ACPI-based systems list multiple i2c-devices in a single ACPI
+	  firmware-node. This driver will instantiate separate i2c-clients
+	  for each device in the firmware-node.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called i2c-multi-instantiate.
+
 endif # X86_PLATFORM_DEVICES
 
 config PMC_ATOM
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 8d9477114fb5..e6d1becf81ce 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -91,3 +91,4 @@ obj-$(CONFIG_PMC_ATOM)		+= pmc_atom.o
 obj-$(CONFIG_MLX_PLATFORM)	+= mlx-platform.o
 obj-$(CONFIG_INTEL_TURBO_MAX_3) += intel_turbo_max_3.o
 obj-$(CONFIG_INTEL_CHTDC_TI_PWRBTN)	+= intel_chtdc_ti_pwrbtn.o
+obj-$(CONFIG_I2C_MULTI_INSTANTIATE)	+= i2c-multi-instantiate.o
diff --git a/drivers/platform/x86/i2c-multi-instantiate.c b/drivers/platform/x86/i2c-multi-instantiate.c
new file mode 100644
index 000000000000..5456581b473c
--- /dev/null
+++ b/drivers/platform/x86/i2c-multi-instantiate.c
@@ -0,0 +1,132 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * I2C multi-instantiate driver, pseudo driver to instantiate multiple
+ * i2c-clients from a single fwnode.
+ *
+ * Copyright 2018 Hans de Goede <hdegoede@redhat.com>
+ */
+
+#include <linux/acpi.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+struct i2c_inst_data {
+	const char *type;
+	int gpio_irq_idx;
+};
+
+struct i2c_multi_inst_data {
+	int num_clients;
+	struct i2c_client *clients[0];
+};
+
+static int i2c_multi_inst_probe(struct platform_device *pdev)
+{
+	struct i2c_multi_inst_data *multi;
+	const struct acpi_device_id *match;
+	const struct i2c_inst_data *inst_data;
+	struct i2c_board_info board_info = {};
+	struct device *dev = &pdev->dev;
+	struct acpi_device *adev;
+	char name[32];
+	int i, ret;
+
+	match = acpi_match_device(dev->driver->acpi_match_table, dev);
+	if (!match) {
+		dev_err(dev, "Error ACPI match data is missing\n");
+		return -ENODEV;
+	}
+	inst_data = (const struct i2c_inst_data *)match->driver_data;
+
+	adev = ACPI_COMPANION(dev);
+
+	/* Count number of clients to instantiate */
+	for (i = 0; inst_data[i].type; i++) {}
+
+	multi = devm_kmalloc(dev,
+			offsetof(struct i2c_multi_inst_data, clients[i]),
+			GFP_KERNEL);
+	if (!multi)
+		return -ENOMEM;
+
+	multi->num_clients = i;
+
+	for (i = 0; i < multi->num_clients; 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);
+		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);
+			if (ret < 0) {
+				dev_err(dev, "Error requesting irq at index %d: %d\n",
+					inst_data[i].gpio_irq_idx, ret);
+				goto error;
+			}
+			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);
+			ret = -ENODEV;
+			goto error;
+		}
+	}
+
+	platform_set_drvdata(pdev, multi);
+	return 0;
+
+error:
+	while (--i >= 0)
+		i2c_unregister_device(multi->clients[i]);
+
+	return ret;
+}
+
+static int i2c_multi_inst_remove(struct platform_device *pdev)
+{
+	struct i2c_multi_inst_data *multi = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i < multi->num_clients; i++)
+		i2c_unregister_device(multi->clients[i]);
+
+	return 0;
+}
+
+static const struct i2c_inst_data bsg1160_data[]  = {
+	{ "bmc150_accel", 0 },
+	{ "bmc150_magn", -1 },
+	{ "bmg160", -1 },
+	{}
+};
+
+/*
+ * 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 },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, i2c_multi_inst_acpi_ids);
+
+static struct platform_driver i2c_multi_inst_driver = {
+	.driver	= {
+		.name = "I2C multi instantiate pseudo device driver",
+		.acpi_match_table = ACPI_PTR(i2c_multi_inst_acpi_ids),
+	},
+	.probe = i2c_multi_inst_probe,
+	.remove = i2c_multi_inst_remove,
+};
+module_platform_driver(i2c_multi_inst_driver);
+
+MODULE_DESCRIPTION("I2C multi instantiate pseudo device driver");
+MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
+MODULE_LICENSE("GPL");
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v6 4/4] platform/x86: Add ACPI i2c-multi-instantiate pseudo driver
  2018-08-09 11:40 ` [PATCH v6 4/4] platform/x86: Add ACPI i2c-multi-instantiate pseudo driver Hans de Goede
@ 2018-08-09 11:55   ` Rafael J. Wysocki
  2018-08-09 12:08     ` Hans de Goede
  0 siblings, 1 reply; 8+ messages in thread
From: Rafael J. Wysocki @ 2018-08-09 11:55 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Rafael J . Wysocki, Len Brown, Andy Shevchenko, Mika Westerberg,
	Darren Hart, Wolfram Sang, Srinivas Pandruvada,
	ACPI Devel Maling List, Platform Driver,
	Linux Kernel Mailing List, Heikki Krogerus, linux-i2c

On Thu, Aug 9, 2018 at 1:40 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> On systems with ACPI instantiated i2c-clients, normally there is 1 fw_node
> per i2c-device and that fw-node contains 1 I2cSerialBus resource for that 1
> i2c-device.
>
> But in some rare cases the manufacturer has decided to describe multiple
> i2c-devices in a single ACPI fwnode with multiple I2cSerialBus resources.
>
> An earlier attempt to fix this in the i2c-core resulted in a lot of extra
> code to support this corner-case.
>
> This commit introduces a new i2c-multi-instantiate driver which fixes this
> in a different way. This new driver can be built as a module which will
> only loaded on affected systems.
>
> This driver will instantiate a new i2c-client per I2cSerialBus resource,
> using the driver_data from the acpi_device_id it is binding to to tell it
> which chip-type (and optional irq-resource) to use when instantiating.
>
> Note this driver depends on a platform device being instantiated for the
> ACPI fwnode, see the i2c_multi_instantiate_ids list of ACPI device-ids in
> drivers/acpi/scan.c: acpi_device_enumeration_by_parent().
>
> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-by: Wolfram Sang <wsa@the-dreams.de>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Rebase on top of 4.18-rc2
>
> Changes in v3:
> -Change from an i2c-driver using a hack to allow having multiple i2c clients
>  at the same address to a platform-driver
>
> Changes in v4:
> -Tweak MAINTAINERS entry a bit
>
> Changes in v5:
> -s/no_clients/num_clients/
> -Change patch Subject prefix to platform/x86
>
> Changes in v6:
> -s/irq_idx/gpio_irq_idx/

And this is the only patch that has changed in v6, right?

Anyway, if you have further updates on top of this series, please send
them as incremental patches.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v6 4/4] platform/x86: Add ACPI i2c-multi-instantiate pseudo driver
  2018-08-09 11:55   ` Rafael J. Wysocki
@ 2018-08-09 12:08     ` Hans de Goede
  2018-08-12 11:14       ` Rafael J. Wysocki
  0 siblings, 1 reply; 8+ messages in thread
From: Hans de Goede @ 2018-08-09 12:08 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Rafael J . Wysocki, Len Brown, Andy Shevchenko, Mika Westerberg,
	Darren Hart, Wolfram Sang, Srinivas Pandruvada,
	ACPI Devel Maling List, Platform Driver,
	Linux Kernel Mailing List, Heikki Krogerus, linux-i2c

Hi,

On 09-08-18 13:55, Rafael J. Wysocki wrote:
> On Thu, Aug 9, 2018 at 1:40 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> On systems with ACPI instantiated i2c-clients, normally there is 1 fw_node
>> per i2c-device and that fw-node contains 1 I2cSerialBus resource for that 1
>> i2c-device.
>>
>> But in some rare cases the manufacturer has decided to describe multiple
>> i2c-devices in a single ACPI fwnode with multiple I2cSerialBus resources.
>>
>> An earlier attempt to fix this in the i2c-core resulted in a lot of extra
>> code to support this corner-case.
>>
>> This commit introduces a new i2c-multi-instantiate driver which fixes this
>> in a different way. This new driver can be built as a module which will
>> only loaded on affected systems.
>>
>> This driver will instantiate a new i2c-client per I2cSerialBus resource,
>> using the driver_data from the acpi_device_id it is binding to to tell it
>> which chip-type (and optional irq-resource) to use when instantiating.
>>
>> Note this driver depends on a platform device being instantiated for the
>> ACPI fwnode, see the i2c_multi_instantiate_ids list of ACPI device-ids in
>> drivers/acpi/scan.c: acpi_device_enumeration_by_parent().
>>
>> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Acked-by: Wolfram Sang <wsa@the-dreams.de>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> -Rebase on top of 4.18-rc2
>>
>> Changes in v3:
>> -Change from an i2c-driver using a hack to allow having multiple i2c clients
>>   at the same address to a platform-driver
>>
>> Changes in v4:
>> -Tweak MAINTAINERS entry a bit
>>
>> Changes in v5:
>> -s/no_clients/num_clients/
>> -Change patch Subject prefix to platform/x86
>>
>> Changes in v6:
>> -s/irq_idx/gpio_irq_idx/
> 
> And this is the only patch that has changed in v6, right?

Correct.

> Anyway, if you have further updates on top of this series, please send
> them as incremental patches.

Ok, will do.

Thanks & Regards,

Hans


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v6 4/4] platform/x86: Add ACPI i2c-multi-instantiate pseudo driver
  2018-08-09 12:08     ` Hans de Goede
@ 2018-08-12 11:14       ` Rafael J. Wysocki
  0 siblings, 0 replies; 8+ messages in thread
From: Rafael J. Wysocki @ 2018-08-12 11:14 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Rafael J. Wysocki, Len Brown, Andy Shevchenko, Mika Westerberg,
	Darren Hart, Wolfram Sang, Srinivas Pandruvada,
	ACPI Devel Maling List, Platform Driver,
	Linux Kernel Mailing List, Heikki Krogerus, linux-i2c

On Thursday, August 9, 2018 2:08:41 PM CEST Hans de Goede wrote:
> Hi,
> 
> On 09-08-18 13:55, Rafael J. Wysocki wrote:
> > On Thu, Aug 9, 2018 at 1:40 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> >> On systems with ACPI instantiated i2c-clients, normally there is 1 fw_node
> >> per i2c-device and that fw-node contains 1 I2cSerialBus resource for that 1
> >> i2c-device.
> >>
> >> But in some rare cases the manufacturer has decided to describe multiple
> >> i2c-devices in a single ACPI fwnode with multiple I2cSerialBus resources.
> >>
> >> An earlier attempt to fix this in the i2c-core resulted in a lot of extra
> >> code to support this corner-case.
> >>
> >> This commit introduces a new i2c-multi-instantiate driver which fixes this
> >> in a different way. This new driver can be built as a module which will
> >> only loaded on affected systems.
> >>
> >> This driver will instantiate a new i2c-client per I2cSerialBus resource,
> >> using the driver_data from the acpi_device_id it is binding to to tell it
> >> which chip-type (and optional irq-resource) to use when instantiating.
> >>
> >> Note this driver depends on a platform device being instantiated for the
> >> ACPI fwnode, see the i2c_multi_instantiate_ids list of ACPI device-ids in
> >> drivers/acpi/scan.c: acpi_device_enumeration_by_parent().
> >>
> >> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >> Acked-by: Wolfram Sang <wsa@the-dreams.de>
> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >> ---
> >> Changes in v2:
> >> -Rebase on top of 4.18-rc2
> >>
> >> Changes in v3:
> >> -Change from an i2c-driver using a hack to allow having multiple i2c clients
> >>   at the same address to a platform-driver
> >>
> >> Changes in v4:
> >> -Tweak MAINTAINERS entry a bit
> >>
> >> Changes in v5:
> >> -s/no_clients/num_clients/
> >> -Change patch Subject prefix to platform/x86
> >>
> >> Changes in v6:
> >> -s/irq_idx/gpio_irq_idx/
> > 
> > And this is the only patch that has changed in v6, right?
> 
> Correct.
> 
> > Anyway, if you have further updates on top of this series, please send
> > them as incremental patches.
> 
> Ok, will do.

The series has been applied, thanks!



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-08-12 11:16 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-09 11:40 [PATCH v6 0/4] i2c-multi-instantiate pseudo driver Hans de Goede
2018-08-09 11:40 ` [PATCH v6 1/4] ACPI / scan: Initialize status to ACPI_STA_DEFAULT Hans de Goede
2018-08-09 11:40 ` [PATCH v6 2/4] ACPI / scan: Create platform device for fwnodes with multiple i2c devices Hans de Goede
2018-08-09 11:40 ` [PATCH v6 3/4] ACPI / x86-utils: Remove status workaround from acpi_device_always_present() Hans de Goede
2018-08-09 11:40 ` [PATCH v6 4/4] platform/x86: Add ACPI i2c-multi-instantiate pseudo driver Hans de Goede
2018-08-09 11:55   ` Rafael J. Wysocki
2018-08-09 12:08     ` Hans de Goede
2018-08-12 11:14       ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).