[PATCH v3 0/6] enable I2C devices behind I2C bus on Gen2

[PATCH v3 0/6] enable I2C devices behind I2C bus on Gen2

[Andy Shevchenko]

There is a board in the wild, i.e. Intel Galileo Gen2, that has ACPI enumerated
devices behind I2C bus.

This patch series dedicated to enable those devices. Meanwhile it also changes
I2C core to cope with ACPI 6.0 specification (patch 1).

The MFD framework is also updated to cope with interesting implementation of
the cell descriptions under ACPI MFD (patch 2).

The patches 5 and 6 are pretty independent and could be applied ahead, though
they don't make much sense without previous ones.

Srinivas, it would be nice to see your tag (ideally Tested-by) to be sure we
don't break ISH stuff.

Since it touches multiple subsystems someone needs to create an immutable
branch. I don't actually know whose subsystem better here. Wolfram?

Tested on the actual Intel Galileo Gen2 by Ismo (gpio expanders) and me (at24).

Changelog v3:
- append ACKs from Rafael (from ACPI angle)
- drop upstreamed patches (GPIO pca953x)

Changelog v2:
- append tags
- re-make patch 3 (suggested by Lee)
- improve patch 8 (suggested by Thierry)

Andy Shevchenko (5):
  mfd: core: redo ACPI matching of the children devices
  mfd: intel_quark_i2c_gpio: load gpio driver first
  mfd: intel_quark_i2c_gpio: support devices behind i2c bus
  at24: enable ACPI device found on Galileo Gen2
  pwm-pca9685: enable ACPI device found on Galileo Gen2

Mika Westerberg (1):
  i2c / ACPI: Rework I2C device scanning

 Documentation/acpi/enumeration.txt | 11 +++--
 drivers/i2c/i2c-core.c             | 82 +++++++++++++++++++++++++++-----------
 drivers/mfd/intel_quark_i2c_gpio.c | 33 ++++++++++-----
 drivers/mfd/mfd-core.c             | 52 ++++++++++++++++--------
 drivers/misc/eeprom/at24.c         | 22 ++++++++--
 drivers/pwm/Kconfig                |  2 +-
 drivers/pwm/pwm-pca9685.c          | 20 ++++++++--
 include/linux/mfd/core.h           | 10 ++++-
 8 files changed, 170 insertions(+), 62 deletions(-)

-- 
2.5.3

[PATCH v3 1/6] i2c / ACPI: Rework I2C device scanning

[Andy Shevchenko]

From: Mika Westerberg <mika.westerberg@linux.intel.com>

The way we currently scan I2C devices behind an I2C host controller does not
work in cases where the I2C device in question is not declared directly below
the host controller ACPI node.

This is perfectly legal according the ACPI 6.0 specification and some existing
systems are doing this.

To be able to enumerate all devices which are connected to a certain I2C host
controller we need to rework the current I2C scanning routine a bit. Instead of
scanning directly below the host controller we scan the whole ACPI namespace
for present devices with valid I2cSerialBus() connection pointing to the host
controller in question.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/i2c-core.c | 82 ++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 59 insertions(+), 23 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index a732107..24e516e 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -99,27 +99,40 @@ struct gsb_buffer {
 	};
 } __packed;
 
-static int acpi_i2c_add_resource(struct acpi_resource *ares, void *data)
+struct acpi_i2c_lookup {
+	struct i2c_board_info *info;
+	acpi_handle adapter_handle;
+	acpi_handle device_handle;
+};
+
+static int acpi_i2c_find_address(struct acpi_resource *ares, void *data)
 {
-	struct i2c_board_info *info = data;
+	struct acpi_i2c_lookup *lookup = data;
+	struct i2c_board_info *info = lookup->info;
+	struct acpi_resource_i2c_serialbus *sb;
+	acpi_handle adapter_handle;
+	acpi_status status;
 
-	if (ares->type == ACPI_RESOURCE_TYPE_SERIAL_BUS) {
-		struct acpi_resource_i2c_serialbus *sb;
+	if (info->addr || ares->type != ACPI_RESOURCE_TYPE_SERIAL_BUS)
+		return 1;
 
-		sb = &ares->data.i2c_serial_bus;
-		if (!info->addr && sb->type == ACPI_RESOURCE_SERIAL_TYPE_I2C) {
-			info->addr = sb->slave_address;
-			if (sb->access_mode == ACPI_I2C_10BIT_MODE)
-				info->flags |= I2C_CLIENT_TEN;
-		}
-	} else if (!info->irq) {
-		struct resource r;
+	sb = &ares->data.i2c_serial_bus;
+	if (sb->type != ACPI_RESOURCE_SERIAL_TYPE_I2C)
+		return 1;
 
-		if (acpi_dev_resource_interrupt(ares, 0, &r))
-			info->irq = r.start;
+	/*
+	 * Extract the ResourceSource and make sure that the handle matches
+	 * with the I2C adapter handle.
+	 */
+	status = acpi_get_handle(lookup->device_handle,
+				 sb->resource_source.string_ptr,
+				 &adapter_handle);
+	if (ACPI_SUCCESS(status) && adapter_handle == lookup->adapter_handle) {
+		info->addr = sb->slave_address;
+		if (sb->access_mode == ACPI_I2C_10BIT_MODE)
+			info->flags |= I2C_CLIENT_TEN;
 	}
 
-	/* Tell the ACPI core to skip this resource */
 	return 1;
 }
 
@@ -128,6 +141,8 @@ static acpi_status acpi_i2c_add_device(acpi_handle handle, u32 level,
 {
 	struct i2c_adapter *adapter = data;
 	struct list_head resource_list;
+	struct acpi_i2c_lookup lookup;
+	struct resource_entry *entry;
 	struct i2c_board_info info;
 	struct acpi_device *adev;
 	int ret;
@@ -140,14 +155,37 @@ static acpi_status acpi_i2c_add_device(acpi_handle handle, u32 level,
 	memset(&info, 0, sizeof(info));
 	info.fwnode = acpi_fwnode_handle(adev);
 
+	memset(&lookup, 0, sizeof(lookup));
+	lookup.adapter_handle = ACPI_HANDLE(adapter->dev.parent);
+	lookup.device_handle = handle;
+	lookup.info = &info;
+
+	/*
+	 * Look up for I2cSerialBus resource with ResourceSource that
+	 * matches with this adapter.
+	 */
 	INIT_LIST_HEAD(&resource_list);
 	ret = acpi_dev_get_resources(adev, &resource_list,
-				     acpi_i2c_add_resource, &info);
+				     acpi_i2c_find_address, &lookup);
 	acpi_dev_free_resource_list(&resource_list);
 
 	if (ret < 0 || !info.addr)
 		return AE_OK;
 
+	/* Then fill IRQ number if any */
+	ret = acpi_dev_get_resources(adev, &resource_list, NULL, NULL);
+	if (ret < 0)
+		return AE_OK;
+
+	resource_list_for_each_entry(entry, &resource_list) {
+		if (resource_type(entry->res) == IORESOURCE_IRQ) {
+			info.irq = entry->res->start;
+			break;
+		}
+	}
+
+	acpi_dev_free_resource_list(&resource_list);
+
 	adev->power.flags.ignore_parent = true;
 	strlcpy(info.type, dev_name(&adev->dev), sizeof(info.type));
 	if (!i2c_new_device(adapter, &info)) {
@@ -160,6 +198,8 @@ static acpi_status acpi_i2c_add_device(acpi_handle handle, u32 level,
 	return AE_OK;
 }
 
+#define ACPI_I2C_MAX_SCAN_DEPTH 32
+
 /**
  * acpi_i2c_register_devices - enumerate I2C slave devices behind adapter
  * @adap: pointer to adapter
@@ -170,17 +210,13 @@ static acpi_status acpi_i2c_add_device(acpi_handle handle, u32 level,
  */
 static void acpi_i2c_register_devices(struct i2c_adapter *adap)
 {
-	acpi_handle handle;
 	acpi_status status;
 
-	if (!adap->dev.parent)
-		return;
-
-	handle = ACPI_HANDLE(adap->dev.parent);
-	if (!handle)
+	if (!adap->dev.parent || !has_acpi_companion(adap->dev.parent))
 		return;
 
-	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, 1,
+	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
+				     ACPI_I2C_MAX_SCAN_DEPTH,
 				     acpi_i2c_add_device, NULL,
 				     adap, NULL);
 	if (ACPI_FAILURE(status))
-- 
2.5.3

Re: [PATCH v3 1/6] i2c / ACPI: Rework I2C device scanning

[Wolfram Sang]

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

On Wed, Oct 07, 2015 at 01:18:44PM +0300, Andy Shevchenko wrote:
> From: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> The way we currently scan I2C devices behind an I2C host controller does not
> work in cases where the I2C device in question is not declared directly below
> the host controller ACPI node.
> 
> This is perfectly legal according the ACPI 6.0 specification and some existing
> systems are doing this.
> 
> To be able to enumerate all devices which are connected to a certain I2C host
> controller we need to rework the current I2C scanning routine a bit. Instead of
> scanning directly below the host controller we scan the whole ACPI namespace
> for present devices with valid I2cSerialBus() connection pointing to the host
> controller in question.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied to for-next, thanks!

I am going to pick up patch 5 as well. I hope the others can go via mfd
and there is no build dependency on them?


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

[PATCH v3 2/6] mfd: core: redo ACPI matching of the children devices

[Andy Shevchenko]

There is at least one board on the market, i.e. Intel Galileo Gen2, that uses
_ADR to distinguish the devices under one actual device. Due to this we have to
improve the quirk in the MFD core to handle that board.

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 Documentation/acpi/enumeration.txt | 11 +++++---
 drivers/mfd/mfd-core.c             | 52 ++++++++++++++++++++++++++------------
 include/linux/mfd/core.h           | 10 ++++++--
 3 files changed, 52 insertions(+), 21 deletions(-)

diff --git a/Documentation/acpi/enumeration.txt b/Documentation/acpi/enumeration.txt
index b731b29..a91ec5a 100644
--- a/Documentation/acpi/enumeration.txt
+++ b/Documentation/acpi/enumeration.txt
@@ -347,13 +347,18 @@ For the first case, the MFD drivers do not need to do anything. The
 resulting child platform device will have its ACPI_COMPANION() set to point
 to the parent device.
 
-If the ACPI namespace has a device that we can match using an ACPI id,
-the id should be set like:
+If the ACPI namespace has a device that we can match using an ACPI id or ACPI
+adr, the cell should be set like:
+
+	static struct mfd_cell_acpi_match my_subdevice_cell_acpi_match = {
+		.pnpid = "XYZ0001",
+		.adr = 0,
+	};
 
 	static struct mfd_cell my_subdevice_cell = {
 		.name = "my_subdevice",
 		/* set the resources relative to the parent */
-		.acpi_pnpid = "XYZ0001",
+		.acpi_match = &my_subdevice_cell_acpi_match,
 	};
 
 The ACPI id "XYZ0001" is then used to lookup an ACPI device directly under
diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index c17635d..60b60dc 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -82,29 +82,49 @@ static int mfd_platform_add_cell(struct platform_device *pdev,
 static void mfd_acpi_add_device(const struct mfd_cell *cell,
 				struct platform_device *pdev)
 {
-	struct acpi_device *parent_adev;
+	const struct mfd_cell_acpi_match *match = cell->acpi_match;
+	struct acpi_device *parent, *child;
 	struct acpi_device *adev;
 
-	parent_adev = ACPI_COMPANION(pdev->dev.parent);
-	if (!parent_adev)
+	parent = ACPI_COMPANION(pdev->dev.parent);
+	if (!parent)
 		return;
 
 	/*
-	 * MFD child device gets its ACPI handle either from the ACPI
-	 * device directly under the parent that matches the acpi_pnpid or
-	 * it will use the parent handle if is no acpi_pnpid is given.
+	 * MFD child device gets its ACPI handle either from the ACPI device
+	 * directly under the parent that matches the either _HID or _CID, or
+	 * _ADR or it will use the parent handle if is no ID is given.
+	 *
+	 * Note that use of _ADR is a grey area in the ACPI specification,
+	 * though Intel Galileo Gen2 is using it to distinguish the children
+	 * devices.
 	 */
-	adev = parent_adev;
-	if (cell->acpi_pnpid) {
-		struct acpi_device_id ids[2] = {};
-		struct acpi_device *child_adev;
-
-		strlcpy(ids[0].id, cell->acpi_pnpid, sizeof(ids[0].id));
-		list_for_each_entry(child_adev, &parent_adev->children, node)
-			if (acpi_match_device_ids(child_adev, ids)) {
-				adev = child_adev;
-				break;
+	adev = parent;
+	if (match) {
+		if (match->pnpid) {
+			struct acpi_device_id ids[2] = {};
+
+			strlcpy(ids[0].id, match->pnpid, sizeof(ids[0].id));
+			list_for_each_entry(child, &parent->children, node) {
+				if (acpi_match_device_ids(child, ids)) {
+					adev = child;
+					break;
+				}
+			}
+		} else {
+			unsigned long long adr;
+			acpi_status status;
+
+			list_for_each_entry(child, &parent->children, node) {
+				status = acpi_evaluate_integer(child->handle,
+							       "_ADR", NULL,
+							       &adr);
+				if (ACPI_SUCCESS(status) && match->adr == adr) {
+					adev = child;
+					break;
+				}
 			}
+		}
 	}
 
 	ACPI_COMPANION_SET(&pdev->dev, adev);
diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h
index a76bc10..27dac3f 100644
--- a/include/linux/mfd/core.h
+++ b/include/linux/mfd/core.h
@@ -18,6 +18,12 @@
 
 struct irq_domain;
 
+/* Matches ACPI PNP id, either _HID or _CID, or ACPI _ADR */
+struct mfd_cell_acpi_match {
+	const char			*pnpid;
+	const unsigned long long	adr;
+};
+
 /*
  * This struct describes the MFD part ("cell").
  * After registration the copy of this structure will become the platform data
@@ -44,8 +50,8 @@ struct mfd_cell {
 	 */
 	const char		*of_compatible;
 
-	/* Matches ACPI PNP id, either _HID or _CID */
-	const char		*acpi_pnpid;
+	/* Matches ACPI */
+	const struct mfd_cell_acpi_match	*acpi_match;
 
 	/*
 	 * These resources can be specified relative to the parent device.
-- 
2.5.3

Re: [PATCH v3 2/6] mfd: core: redo ACPI matching of the children devices

[Lee Jones]

On Wed, 07 Oct 2015, Andy Shevchenko wrote:

> There is at least one board on the market, i.e. Intel Galileo Gen2, that uses
> _ADR to distinguish the devices under one actual device. Due to this we have to
> improve the quirk in the MFD core to handle that board.
> 
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  Documentation/acpi/enumeration.txt | 11 +++++---
>  drivers/mfd/mfd-core.c             | 52 ++++++++++++++++++++++++++------------
>  include/linux/mfd/core.h           | 10 ++++++--
>  3 files changed, 52 insertions(+), 21 deletions(-)

For my own reference:
  Acked-by: Lee Jones <lee.jones@linaro.org>

> diff --git a/Documentation/acpi/enumeration.txt b/Documentation/acpi/enumeration.txt
> index b731b29..a91ec5a 100644
> --- a/Documentation/acpi/enumeration.txt
> +++ b/Documentation/acpi/enumeration.txt
> @@ -347,13 +347,18 @@ For the first case, the MFD drivers do not need to do anything. The
>  resulting child platform device will have its ACPI_COMPANION() set to point
>  to the parent device.
>  
> -If the ACPI namespace has a device that we can match using an ACPI id,
> -the id should be set like:
> +If the ACPI namespace has a device that we can match using an ACPI id or ACPI
> +adr, the cell should be set like:
> +
> +	static struct mfd_cell_acpi_match my_subdevice_cell_acpi_match = {
> +		.pnpid = "XYZ0001",
> +		.adr = 0,
> +	};
>  
>  	static struct mfd_cell my_subdevice_cell = {
>  		.name = "my_subdevice",
>  		/* set the resources relative to the parent */
> -		.acpi_pnpid = "XYZ0001",
> +		.acpi_match = &my_subdevice_cell_acpi_match,
>  	};
>  
>  The ACPI id "XYZ0001" is then used to lookup an ACPI device directly under
> diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
> index c17635d..60b60dc 100644
> --- a/drivers/mfd/mfd-core.c
> +++ b/drivers/mfd/mfd-core.c
> @@ -82,29 +82,49 @@ static int mfd_platform_add_cell(struct platform_device *pdev,
>  static void mfd_acpi_add_device(const struct mfd_cell *cell,
>  				struct platform_device *pdev)
>  {
> -	struct acpi_device *parent_adev;
> +	const struct mfd_cell_acpi_match *match = cell->acpi_match;
> +	struct acpi_device *parent, *child;
>  	struct acpi_device *adev;
>  
> -	parent_adev = ACPI_COMPANION(pdev->dev.parent);
> -	if (!parent_adev)
> +	parent = ACPI_COMPANION(pdev->dev.parent);
> +	if (!parent)
>  		return;
>  
>  	/*
> -	 * MFD child device gets its ACPI handle either from the ACPI
> -	 * device directly under the parent that matches the acpi_pnpid or
> -	 * it will use the parent handle if is no acpi_pnpid is given.
> +	 * MFD child device gets its ACPI handle either from the ACPI device
> +	 * directly under the parent that matches the either _HID or _CID, or
> +	 * _ADR or it will use the parent handle if is no ID is given.
> +	 *
> +	 * Note that use of _ADR is a grey area in the ACPI specification,
> +	 * though Intel Galileo Gen2 is using it to distinguish the children
> +	 * devices.
>  	 */
> -	adev = parent_adev;
> -	if (cell->acpi_pnpid) {
> -		struct acpi_device_id ids[2] = {};
> -		struct acpi_device *child_adev;
> -
> -		strlcpy(ids[0].id, cell->acpi_pnpid, sizeof(ids[0].id));
> -		list_for_each_entry(child_adev, &parent_adev->children, node)
> -			if (acpi_match_device_ids(child_adev, ids)) {
> -				adev = child_adev;
> -				break;
> +	adev = parent;
> +	if (match) {
> +		if (match->pnpid) {
> +			struct acpi_device_id ids[2] = {};
> +
> +			strlcpy(ids[0].id, match->pnpid, sizeof(ids[0].id));
> +			list_for_each_entry(child, &parent->children, node) {
> +				if (acpi_match_device_ids(child, ids)) {
> +					adev = child;
> +					break;
> +				}
> +			}
> +		} else {
> +			unsigned long long adr;
> +			acpi_status status;
> +
> +			list_for_each_entry(child, &parent->children, node) {
> +				status = acpi_evaluate_integer(child->handle,
> +							       "_ADR", NULL,
> +							       &adr);
> +				if (ACPI_SUCCESS(status) && match->adr == adr) {
> +					adev = child;
> +					break;
> +				}
>  			}
> +		}
>  	}
>  
>  	ACPI_COMPANION_SET(&pdev->dev, adev);
> diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h
> index a76bc10..27dac3f 100644
> --- a/include/linux/mfd/core.h
> +++ b/include/linux/mfd/core.h
> @@ -18,6 +18,12 @@
>  
>  struct irq_domain;
>  
> +/* Matches ACPI PNP id, either _HID or _CID, or ACPI _ADR */
> +struct mfd_cell_acpi_match {
> +	const char			*pnpid;
> +	const unsigned long long	adr;
> +};
> +
>  /*
>   * This struct describes the MFD part ("cell").
>   * After registration the copy of this structure will become the platform data
> @@ -44,8 +50,8 @@ struct mfd_cell {
>  	 */
>  	const char		*of_compatible;
>  
> -	/* Matches ACPI PNP id, either _HID or _CID */
> -	const char		*acpi_pnpid;
> +	/* Matches ACPI */
> +	const struct mfd_cell_acpi_match	*acpi_match;
>  
>  	/*
>  	 * These resources can be specified relative to the parent device.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v3 3/6] mfd: intel_quark_i2c_gpio: load gpio driver first

[Andy Shevchenko]

On Intel Galileo boards the GPIO expander is connected to i2c bus. Moreover it
is able to generate interrupt, but interrupt line is connected to GPIO. That's
why we have to have GPIO driver in place when we will probe i2c host with
device connected to it.

Acked-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/intel_quark_i2c_gpio.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
index 1ce1603..958c134 100644
--- a/drivers/mfd/intel_quark_i2c_gpio.c
+++ b/drivers/mfd/intel_quark_i2c_gpio.c
@@ -90,19 +90,19 @@ static struct resource intel_quark_gpio_res[] = {
 
 static struct mfd_cell intel_quark_mfd_cells[] = {
 	{
-		.id = MFD_I2C_BAR,
-		.name = "i2c_designware",
-		.num_resources = ARRAY_SIZE(intel_quark_i2c_res),
-		.resources = intel_quark_i2c_res,
-		.ignore_resource_conflicts = true,
-	},
-	{
 		.id = MFD_GPIO_BAR,
 		.name = "gpio-dwapb",
 		.num_resources = ARRAY_SIZE(intel_quark_gpio_res),
 		.resources = intel_quark_gpio_res,
 		.ignore_resource_conflicts = true,
 	},
+	{
+		.id = MFD_I2C_BAR,
+		.name = "i2c_designware",
+		.num_resources = ARRAY_SIZE(intel_quark_i2c_res),
+		.resources = intel_quark_i2c_res,
+		.ignore_resource_conflicts = true,
+	},
 };
 
 static const struct pci_device_id intel_quark_mfd_ids[] = {
@@ -248,12 +248,11 @@ static int intel_quark_mfd_probe(struct pci_dev *pdev,
 
 	dev_set_drvdata(&pdev->dev, quark_mfd);
 
-	ret = intel_quark_i2c_setup(pdev, &intel_quark_mfd_cells[MFD_I2C_BAR]);
+	ret = intel_quark_i2c_setup(pdev, &intel_quark_mfd_cells[1]);
 	if (ret)
 		return ret;
 
-	ret = intel_quark_gpio_setup(pdev,
-				     &intel_quark_mfd_cells[MFD_GPIO_BAR]);
+	ret = intel_quark_gpio_setup(pdev, &intel_quark_mfd_cells[0]);
 	if (ret)
 		return ret;
 
-- 
2.5.3

[PATCH v3 4/6] mfd: intel_quark_i2c_gpio: support devices behind i2c bus

[Andy Shevchenko]

On Intel Galileo Gen2 the GPIO expanders are connected to the i2c bus. For
those devices the ACPI table has specific parameters that refer to an actual
i2c host controller. Since MFD now copes with that specific configuration we
have to provide a necessary information how to distinguish devices in ACPI
namespace. Here the _ADR values are provided.

Acked-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/intel_quark_i2c_gpio.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
index 958c134..0421374 100644
--- a/drivers/mfd/intel_quark_i2c_gpio.c
+++ b/drivers/mfd/intel_quark_i2c_gpio.c
@@ -31,6 +31,10 @@
 #define MFD_I2C_BAR		0
 #define MFD_GPIO_BAR		1
 
+/* ACPI _ADR value to match the child node */
+#define MFD_ACPI_MATCH_GPIO	0ULL
+#define MFD_ACPI_MATCH_I2C	1ULL
+
 /* The base GPIO number under GPIOLIB framework */
 #define INTEL_QUARK_MFD_GPIO_BASE	8
 
@@ -82,16 +86,25 @@ static struct resource intel_quark_i2c_res[] = {
 	},
 };
 
+static struct mfd_cell_acpi_match intel_quark_acpi_match_i2c = {
+	.adr = MFD_ACPI_MATCH_I2C,
+};
+
 static struct resource intel_quark_gpio_res[] = {
 	[INTEL_QUARK_IORES_MEM] = {
 		.flags = IORESOURCE_MEM,
 	},
 };
 
+static struct mfd_cell_acpi_match intel_quark_acpi_match_gpio = {
+	.adr = MFD_ACPI_MATCH_GPIO,
+};
+
 static struct mfd_cell intel_quark_mfd_cells[] = {
 	{
 		.id = MFD_GPIO_BAR,
 		.name = "gpio-dwapb",
+		.acpi_match = &intel_quark_acpi_match_gpio,
 		.num_resources = ARRAY_SIZE(intel_quark_gpio_res),
 		.resources = intel_quark_gpio_res,
 		.ignore_resource_conflicts = true,
@@ -99,6 +112,7 @@ static struct mfd_cell intel_quark_mfd_cells[] = {
 	{
 		.id = MFD_I2C_BAR,
 		.name = "i2c_designware",
+		.acpi_match = &intel_quark_acpi_match_i2c,
 		.num_resources = ARRAY_SIZE(intel_quark_i2c_res),
 		.resources = intel_quark_i2c_res,
 		.ignore_resource_conflicts = true,
-- 
2.5.3

[PATCH v3 5/6] at24: enable ACPI device found on Galileo Gen2

[Andy Shevchenko]

There is a 24c08 chip connected to i2c bus on Intel Galileo Gen2 board. Enable
it via ACPI ID INT3499.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/misc/eeprom/at24.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index c6cb7f8..ed009a3 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -21,6 +21,7 @@
 #include <linux/bitops.h>
 #include <linux/jiffies.h>
 #include <linux/of.h>
+#include <linux/acpi.h>
 #include <linux/i2c.h>
 #include <linux/platform_data/at24.h>
 
@@ -131,6 +132,12 @@ static const struct i2c_device_id at24_ids[] = {
 };
 MODULE_DEVICE_TABLE(i2c, at24_ids);
 
+static const struct acpi_device_id at24_acpi_ids[] = {
+	{ "INT3499", AT24_DEVICE_MAGIC(8192 / 8, 0) },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, at24_acpi_ids);
+
 /*-------------------------------------------------------------------------*/
 
 /*
@@ -473,15 +480,23 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	struct at24_data *at24;
 	int err;
 	unsigned i, num_addresses;
-	kernel_ulong_t magic;
+	kernel_ulong_t magic = 0;
 
 	if (client->dev.platform_data) {
 		chip = *(struct at24_platform_data *)client->dev.platform_data;
 	} else {
-		if (!id->driver_data)
+		if (id) {
+			magic = id->driver_data;
+		} else {
+			const struct acpi_device_id *id;
+
+			id = acpi_match_device(at24_acpi_ids, &client->dev);
+			if (id)
+				magic = id->driver_data;
+		}
+		if (!magic)
 			return -ENODEV;
 
-		magic = id->driver_data;
 		chip.byte_len = BIT(magic & AT24_BITMASK(AT24_SIZE_BYTELEN));
 		magic >>= AT24_SIZE_BYTELEN;
 		chip.flags = magic & AT24_BITMASK(AT24_SIZE_FLAGS);
@@ -661,6 +676,7 @@ static int at24_remove(struct i2c_client *client)
 static struct i2c_driver at24_driver = {
 	.driver = {
 		.name = "at24",
+		.acpi_match_table = ACPI_PTR(at24_acpi_ids),
 	},
 	.probe = at24_probe,
 	.remove = at24_remove,
-- 
2.5.3

Re: [PATCH v3 5/6] at24: enable ACPI device found on Galileo Gen2

[Wolfram Sang]

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

On Wed, Oct 07, 2015 at 01:18:48PM +0300, Andy Shevchenko wrote:
> There is a 24c08 chip connected to i2c bus on Intel Galileo Gen2 board. Enable
> it via ACPI ID INT3499.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Sparse rightfully says:

drivers/misc/eeprom/at24.c:491:53: warning: symbol 'id' shadows an earlier one
drivers/misc/eeprom/at24.c:474:50: originally declared here


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

[PATCH v3 6/6] pwm-pca9685: enable ACPI device found on Galileo Gen2

[Andy Shevchenko]

There is a chip connected to i2c bus on Intel Galileo Gen2 board. Enable it via
ACPI ID INT3492.

Cc: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pwm/Kconfig       |  2 +-
 drivers/pwm/pwm-pca9685.c | 20 ++++++++++++++++----
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 0cfaf6b..2f4641a 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -272,7 +272,7 @@ config PWM_MXS
 
 config PWM_PCA9685
 	tristate "NXP PCA9685 PWM driver"
-	depends on OF && I2C
+	depends on I2C
 	select REGMAP_I2C
 	help
 	  Generic PWM framework driver for NXP PCA9685 LED controller.
diff --git a/drivers/pwm/pwm-pca9685.c b/drivers/pwm/pwm-pca9685.c
index 70448a6..117fccf 100644
--- a/drivers/pwm/pwm-pca9685.c
+++ b/drivers/pwm/pwm-pca9685.c
@@ -19,9 +19,11 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/acpi.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/pwm.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
@@ -297,7 +299,6 @@ static const struct regmap_config pca9685_regmap_i2c_config = {
 static int pca9685_pwm_probe(struct i2c_client *client,
 				const struct i2c_device_id *id)
 {
-	struct device_node *np = client->dev.of_node;
 	struct pca9685 *pca;
 	int ret;
 	int mode2;
@@ -320,12 +321,12 @@ static int pca9685_pwm_probe(struct i2c_client *client,
 
 	regmap_read(pca->regmap, PCA9685_MODE2, &mode2);
 
-	if (of_property_read_bool(np, "invert"))
+	if (device_property_read_bool(&client->dev, "invert"))
 		mode2 |= MODE2_INVRT;
 	else
 		mode2 &= ~MODE2_INVRT;
 
-	if (of_property_read_bool(np, "open-drain"))
+	if (device_property_read_bool(&client->dev, "open-drain"))
 		mode2 &= ~MODE2_OUTDRV;
 	else
 		mode2 |= MODE2_OUTDRV;
@@ -363,16 +364,27 @@ static const struct i2c_device_id pca9685_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, pca9685_id);
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id pca9685_acpi_ids[] = {
+	{ "INT3492", 0 },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(acpi, pca9685_acpi_ids);
+#endif
+
+#ifdef CONFIG_OF
 static const struct of_device_id pca9685_dt_ids[] = {
 	{ .compatible = "nxp,pca9685-pwm", },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, pca9685_dt_ids);
+#endif
 
 static struct i2c_driver pca9685_i2c_driver = {
 	.driver = {
 		.name = "pca9685-pwm",
-		.of_match_table = pca9685_dt_ids,
+		.acpi_match_table = ACPI_PTR(pca9685_acpi_ids),
+		.of_match_table = of_match_ptr(pca9685_dt_ids),
 	},
 	.probe = pca9685_pwm_probe,
 	.remove = pca9685_pwm_remove,
-- 
2.5.3

Re: [PATCH v3 6/6] pwm-pca9685: enable ACPI device found on Galileo Gen2

[Andy Shevchenko]

On Wed, 2015-10-07 at 13:18 +0300, Andy Shevchenko wrote:
> There is a chip connected to i2c bus on Intel Galileo Gen2 board. 
> Enable it via
> ACPI ID INT3492.

Thierry, can you Ack or take this one, please?

> 
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/pwm/Kconfig       |  2 +-
>  drivers/pwm/pwm-pca9685.c | 20 ++++++++++++++++----
>  2 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index 0cfaf6b..2f4641a 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -272,7 +272,7 @@ config PWM_MXS
>  
>  config PWM_PCA9685
>  	tristate "NXP PCA9685 PWM driver"
> -	depends on OF && I2C
> +	depends on I2C
>  	select REGMAP_I2C
>  	help
>  	  Generic PWM framework driver for NXP PCA9685 LED 
> controller.
> diff --git a/drivers/pwm/pwm-pca9685.c b/drivers/pwm/pwm-pca9685.c
> index 70448a6..117fccf 100644
> --- a/drivers/pwm/pwm-pca9685.c
> +++ b/drivers/pwm/pwm-pca9685.c
> @@ -19,9 +19,11 @@
>   * this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include <linux/acpi.h>
>  #include <linux/i2c.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> +#include <linux/property.h>
>  #include <linux/pwm.h>
>  #include <linux/regmap.h>
>  #include <linux/slab.h>
> @@ -297,7 +299,6 @@ static const struct regmap_config 
> pca9685_regmap_i2c_config = {
>  static int pca9685_pwm_probe(struct i2c_client *client,
>  				const struct i2c_device_id *id)
>  {
> -	struct device_node *np = client->dev.of_node;
>  	struct pca9685 *pca;
>  	int ret;
>  	int mode2;
> @@ -320,12 +321,12 @@ static int pca9685_pwm_probe(struct i2c_client 
> *client,
>  
>  	regmap_read(pca->regmap, PCA9685_MODE2, &mode2);
>  
> -	if (of_property_read_bool(np, "invert"))
> +	if (device_property_read_bool(&client->dev, "invert"))
>  		mode2 |= MODE2_INVRT;
>  	else
>  		mode2 &= ~MODE2_INVRT;
>  
> -	if (of_property_read_bool(np, "open-drain"))
> +	if (device_property_read_bool(&client->dev, "open-drain"))
>  		mode2 &= ~MODE2_OUTDRV;
>  	else
>  		mode2 |= MODE2_OUTDRV;
> @@ -363,16 +364,27 @@ static const struct i2c_device_id pca9685_id[] 
> = {
>  };
>  MODULE_DEVICE_TABLE(i2c, pca9685_id);
>  
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id pca9685_acpi_ids[] = {
> +	{ "INT3492", 0 },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(acpi, pca9685_acpi_ids);
> +#endif
> +
> +#ifdef CONFIG_OF
>  static const struct of_device_id pca9685_dt_ids[] = {
>  	{ .compatible = "nxp,pca9685-pwm", },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, pca9685_dt_ids);
> +#endif
>  
>  static struct i2c_driver pca9685_i2c_driver = {
>  	.driver = {
>  		.name = "pca9685-pwm",
> -		.of_match_table = pca9685_dt_ids,
> +		.acpi_match_table = ACPI_PTR(pca9685_acpi_ids),
> +		.of_match_table = of_match_ptr(pca9685_dt_ids),
>  	},
>  	.probe = pca9685_pwm_probe,
>  	.remove = pca9685_pwm_remove,

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v3 0/6] enable I2C devices behind I2C bus on Gen2

[Andy Shevchenko]

On Wed, 2015-10-07 at 13:18 +0300, Andy Shevchenko wrote:
> There is a board in the wild, i.e. Intel Galileo Gen2, that has ACPI
> enumerated
> devices behind I2C bus.

Lee, since Wolfram is going to apply patches 1 and 5, how could we
proceed with the rest? Patches are indeed build independent, though
they are unified by enabling logically piece-by-piece. It would be
great if you think you may apply them to your v4.4 queue.


> This patch series dedicated to enable those devices. Meanwhile it
> also changes
> I2C core to cope with ACPI 6.0 specification (patch 1).
> 
> The MFD framework is also updated to cope with interesting
> implementation of
> the cell descriptions under ACPI MFD (patch 2).
> 
> The patches 5 and 6 are pretty independent and could be applied
> ahead, though
> they don't make much sense without previous ones.
> 
> Srinivas, it would be nice to see your tag (ideally Tested-by) to be
> sure we
> don't break ISH stuff.
> 
> Since it touches multiple subsystems someone needs to create an
> immutable
> branch. I don't actually know whose subsystem better here. Wolfram?
> 
> Tested on the actual Intel Galileo Gen2 by Ismo (gpio expanders) and
> me (at24).
> 
> Changelog v3:
> - append ACKs from Rafael (from ACPI angle)
> - drop upstreamed patches (GPIO pca953x)
> 
> Changelog v2:
> - append tags
> - re-make patch 3 (suggested by Lee)
> - improve patch 8 (suggested by Thierry)
> 
> Andy Shevchenko (5):
>   mfd: core: redo ACPI matching of the children devices
>   mfd: intel_quark_i2c_gpio: load gpio driver first
>   mfd: intel_quark_i2c_gpio: support devices behind i2c bus
>   at24: enable ACPI device found on Galileo Gen2
>   pwm-pca9685: enable ACPI device found on Galileo Gen2
> 
> Mika Westerberg (1):
>   i2c / ACPI: Rework I2C device scanning
> 
>  Documentation/acpi/enumeration.txt | 11 +++--
>  drivers/i2c/i2c-core.c             | 82 +++++++++++++++++++++++++++-
> ----------
>  drivers/mfd/intel_quark_i2c_gpio.c | 33 ++++++++++-----
>  drivers/mfd/mfd-core.c             | 52 ++++++++++++++++--------
>  drivers/misc/eeprom/at24.c         | 22 ++++++++--
>  drivers/pwm/Kconfig                |  2 +-
>  drivers/pwm/pwm-pca9685.c          | 20 ++++++++--
>  include/linux/mfd/core.h           | 10 ++++-
>  8 files changed, 170 insertions(+), 62 deletions(-)
> 

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v3 0/6] enable I2C devices behind I2C bus on Gen2

[Lee Jones]

On Wed, 21 Oct 2015, Andy Shevchenko wrote:

> On Wed, 2015-10-07 at 13:18 +0300, Andy Shevchenko wrote:
> > There is a board in the wild, i.e. Intel Galileo Gen2, that has ACPI
> > enumerated
> > devices behind I2C bus.
> 
> Lee, since Wolfram is going to apply patches 1 and 5, how could we
> proceed with the rest? Patches are indeed build independent, though
> they are unified by enabling logically piece-by-piece. It would be
> great if you think you may apply them to your v4.4 queue.

So we're just waiting for Thierry's Ack now, right?

> > This patch series dedicated to enable those devices. Meanwhile it
> > also changes
> > I2C core to cope with ACPI 6.0 specification (patch 1).
> > 
> > The MFD framework is also updated to cope with interesting
> > implementation of
> > the cell descriptions under ACPI MFD (patch 2).
> > 
> > The patches 5 and 6 are pretty independent and could be applied
> > ahead, though
> > they don't make much sense without previous ones.
> > 
> > Srinivas, it would be nice to see your tag (ideally Tested-by) to be
> > sure we
> > don't break ISH stuff.
> > 
> > Since it touches multiple subsystems someone needs to create an
> > immutable
> > branch. I don't actually know whose subsystem better here. Wolfram?
> > 
> > Tested on the actual Intel Galileo Gen2 by Ismo (gpio expanders) and
> > me (at24).
> > 
> > Changelog v3:
> > - append ACKs from Rafael (from ACPI angle)
> > - drop upstreamed patches (GPIO pca953x)
> > 
> > Changelog v2:
> > - append tags
> > - re-make patch 3 (suggested by Lee)
> > - improve patch 8 (suggested by Thierry)
> > 
> > Andy Shevchenko (5):
> >   mfd: core: redo ACPI matching of the children devices
> >   mfd: intel_quark_i2c_gpio: load gpio driver first
> >   mfd: intel_quark_i2c_gpio: support devices behind i2c bus
> >   at24: enable ACPI device found on Galileo Gen2
> >   pwm-pca9685: enable ACPI device found on Galileo Gen2
> > 
> > Mika Westerberg (1):
> >   i2c / ACPI: Rework I2C device scanning
> > 
> >  Documentation/acpi/enumeration.txt | 11 +++--
> >  drivers/i2c/i2c-core.c             | 82 +++++++++++++++++++++++++++-
> > ----------
> >  drivers/mfd/intel_quark_i2c_gpio.c | 33 ++++++++++-----
> >  drivers/mfd/mfd-core.c             | 52 ++++++++++++++++--------
> >  drivers/misc/eeprom/at24.c         | 22 ++++++++--
> >  drivers/pwm/Kconfig                |  2 +-
> >  drivers/pwm/pwm-pca9685.c          | 20 ++++++++--
> >  include/linux/mfd/core.h           | 10 ++++-
> >  8 files changed, 170 insertions(+), 62 deletions(-)
> > 
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 0/6] enable I2C devices behind I2C bus on Gen2

[Andy Shevchenko]

On Fri, Oct 30, 2015 at 8:54 PM, Lee Jones <lee.jones@linaro.org> wrote:
> On Wed, 21 Oct 2015, Andy Shevchenko wrote:
>
>> On Wed, 2015-10-07 at 13:18 +0300, Andy Shevchenko wrote:
>> > There is a board in the wild, i.e. Intel Galileo Gen2, that has ACPI
>> > enumerated
>> > devices behind I2C bus.
>>
>> Lee, since Wolfram is going to apply patches 1 and 5, how could we
>> proceed with the rest? Patches are indeed build independent, though
>> they are unified by enabling logically piece-by-piece. It would be
>> great if you think you may apply them to your v4.4 queue.
>
> So we're just waiting for Thierry's Ack now, right?

That's right. Either Ack, or he might apply patch by himself. It
wouldn't be harmful in any case.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 0/6] enable I2C devices behind I2C bus on Gen2

[Lee Jones]

On Fri, 30 Oct 2015, Andy Shevchenko wrote:

> On Fri, Oct 30, 2015 at 8:54 PM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Wed, 21 Oct 2015, Andy Shevchenko wrote:
> >
> >> On Wed, 2015-10-07 at 13:18 +0300, Andy Shevchenko wrote:
> >> > There is a board in the wild, i.e. Intel Galileo Gen2, that has ACPI
> >> > enumerated
> >> > devices behind I2C bus.
> >>
> >> Lee, since Wolfram is going to apply patches 1 and 5, how could we
> >> proceed with the rest? Patches are indeed build independent, though
> >> they are unified by enabling logically piece-by-piece. It would be
> >> great if you think you may apply them to your v4.4 queue.
> >
> > So we're just waiting for Thierry's Ack now, right?
> 
> That's right. Either Ack, or he might apply patch by himself. It
> wouldn't be harmful in any case.

I'll give him another few days.  If we don't hear anything then I'll
apply them all bar the PWM patch.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog