[PATCH v1 0/8] enable I2C devices behind I2C bus on Gen2

[PATCH v1 0/8] 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/8).

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

The patches 7 and 8 are pretty independent, though they don't make much sense
without previous ones applied.

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. Lee, Wolfram?

Apparently we would like to get ACKs / comments from the rest.

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

Andy Shevchenko (7):
  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
  gpio: pca953x: store driver_data for future use
  gpio: pca953x: support ACPI devices found on Galileo Gen2
  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/gpio/gpio-pca953x.c        | 36 +++++++++++++----
 drivers/i2c/i2c-core.c             | 82 +++++++++++++++++++++++++++-----------
 drivers/mfd/intel_quark_i2c_gpio.c | 62 ++++++++++++++++++----------
 drivers/mfd/mfd-core.c             | 52 ++++++++++++++++--------
 drivers/misc/eeprom/at24.c         | 22 ++++++++--
 drivers/pwm/Kconfig                |  2 +-
 drivers/pwm/pwm-pca9685.c          | 14 +++++--
 include/linux/mfd/core.h           | 10 ++++-
 9 files changed, 212 insertions(+), 79 deletions(-)

-- 
2.5.1

[PATCH v1 1/8] 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>
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.1

[PATCH v1 2/8] 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.

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.1

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

[Lee Jones]

On Tue, 22 Sep 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.

This will require an ACPI Ack.

> 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.

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

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

[Andy Shevchenko]

On Tue, 2015-09-22 at 23:15 +0100, Lee Jones wrote:
> On Tue, 22 Sep 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.
> 
> This will require an ACPI Ack.

Rafael is Cc'ed, so, I hope he will comment on this.

> 
> > 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.
> 

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

[PATCH v1 3/8] 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.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/intel_quark_i2c_gpio.c | 48 +++++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
index 1ce1603..89bd4bf 100644
--- a/drivers/mfd/intel_quark_i2c_gpio.c
+++ b/drivers/mfd/intel_quark_i2c_gpio.c
@@ -82,27 +82,26 @@ static struct resource intel_quark_i2c_res[] = {
 	},
 };
 
+static struct mfd_cell intel_quark_mfd_i2c_cell = {
+	.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 struct resource intel_quark_gpio_res[] = {
 	[INTEL_QUARK_IORES_MEM] = {
 		.flags = IORESOURCE_MEM,
 	},
 };
 
-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,
-	},
+static struct mfd_cell intel_quark_mfd_gpio_cell = {
+	.id = MFD_GPIO_BAR,
+	.name = "gpio-dwapb",
+	.num_resources = ARRAY_SIZE(intel_quark_gpio_res),
+	.resources = intel_quark_gpio_res,
+	.ignore_resource_conflicts = true,
 };
 
 static const struct pci_device_id intel_quark_mfd_ids[] = {
@@ -248,18 +247,25 @@ 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_i2c_cell);
 	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_gpio_cell);
 	if (ret)
 		return ret;
 
-	return mfd_add_devices(&pdev->dev, 0, intel_quark_mfd_cells,
-			       ARRAY_SIZE(intel_quark_mfd_cells), NULL, 0,
-			       NULL);
+	ret = mfd_add_devices(&pdev->dev, 0, &intel_quark_mfd_gpio_cell, 1,
+			      NULL, 0, NULL);
+	if (ret)
+		return ret;
+
+	ret = mfd_add_devices(&pdev->dev, 0, &intel_quark_mfd_i2c_cell, 1,
+			      NULL, 0, NULL);
+	if (ret)
+		mfd_remove_devices(&pdev->dev);
+
+	return ret;
 }
 
 static void intel_quark_mfd_remove(struct pci_dev *pdev)
-- 
2.5.1

Re: [PATCH v1 3/8] mfd: intel_quark_i2c_gpio: load gpio driver first

[Lee Jones]

On Tue, 22 Sep 2015, Andy Shevchenko wrote:

> 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.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/intel_quark_i2c_gpio.c | 48 +++++++++++++++++++++-----------------
>  1 file changed, 27 insertions(+), 21 deletions(-)

--- a/drivers/mfd/intel_quark_i2c_gpio.c
+++ b/drivers/mfd/intel_quark_i2c_gpio.c
@@ -28,8 +28,8 @@
 #include <linux/platform_data/i2c-designware.h>
 
 /* PCI BAR for register base address */
-#define MFD_I2C_BAR            0
-#define MFD_GPIO_BAR           1
+#define MFD_GPIO_BAR           0
+#define MFD_I2C_BAR            1
 
 /* The base GPIO number under GPIOLIB framework */
 #define INTEL_QUARK_MFD_GPIO_BASE      8

?

> diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
> index 1ce1603..89bd4bf 100644
> --- a/drivers/mfd/intel_quark_i2c_gpio.c
> +++ b/drivers/mfd/intel_quark_i2c_gpio.c
> @@ -82,27 +82,26 @@ static struct resource intel_quark_i2c_res[] = {
>  	},
>  };
>  
> +static struct mfd_cell intel_quark_mfd_i2c_cell = {
> +	.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 struct resource intel_quark_gpio_res[] = {
>  	[INTEL_QUARK_IORES_MEM] = {
>  		.flags = IORESOURCE_MEM,
>  	},
>  };
>  
> -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,
> -	},
> +static struct mfd_cell intel_quark_mfd_gpio_cell = {
> +	.id = MFD_GPIO_BAR,
> +	.name = "gpio-dwapb",
> +	.num_resources = ARRAY_SIZE(intel_quark_gpio_res),
> +	.resources = intel_quark_gpio_res,
> +	.ignore_resource_conflicts = true,
>  };
>  
>  static const struct pci_device_id intel_quark_mfd_ids[] = {
> @@ -248,18 +247,25 @@ 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_i2c_cell);
>  	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_gpio_cell);
>  	if (ret)
>  		return ret;
>  
> -	return mfd_add_devices(&pdev->dev, 0, intel_quark_mfd_cells,
> -			       ARRAY_SIZE(intel_quark_mfd_cells), NULL, 0,
> -			       NULL);
> +	ret = mfd_add_devices(&pdev->dev, 0, &intel_quark_mfd_gpio_cell, 1,
> +			      NULL, 0, NULL);
> +	if (ret)
> +		return ret;
> +
> +	ret = mfd_add_devices(&pdev->dev, 0, &intel_quark_mfd_i2c_cell, 1,
> +			      NULL, 0, NULL);
> +	if (ret)
> +		mfd_remove_devices(&pdev->dev);
> +
> +	return ret;
>  }
>  
>  static void intel_quark_mfd_remove(struct pci_dev *pdev)

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 3/8] mfd: intel_quark_i2c_gpio: load gpio driver first

[Andy Shevchenko]

On Tue, 2015-09-22 at 23:23 +0100, Lee Jones wrote:
> On Tue, 22 Sep 2015, Andy Shevchenko wrote:
> 
> > 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.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  drivers/mfd/intel_quark_i2c_gpio.c | 48 +++++++++++++++++++++-----
> > ------------
> >  1 file changed, 27 insertions(+), 21 deletions(-)
> 
> --- a/drivers/mfd/intel_quark_i2c_gpio.c
> +++ b/drivers/mfd/intel_quark_i2c_gpio.c
> @@ -28,8 +28,8 @@
>  #include <linux/platform_data/i2c-designware.h>
>  
>  /* PCI BAR for register base address */
> -#define MFD_I2C_BAR            0
> -#define MFD_GPIO_BAR           1
> +#define MFD_GPIO_BAR           0
> +#define MFD_I2C_BAR            1
>  
>  /* The base GPIO number under GPIOLIB framework */
>  #define INTEL_QUARK_MFD_GPIO_BASE      8
> 
> ?

No, PCI bars are left without changes, but _ADR values are different.
Otherwise it will not work.

Here is the excerpt from DSDT
    Device (GIP0)
    {   
        Name (_ADR, 0x00150002)  // _ADR: Address
        Name (_STA, 0x0F)  // _STA: Status
        Name (_PRW, Package (0x02)  // _PRW: Power Resources for Wake
          {   
              0x0F, 
              0x03
          })
        Device (GPO)
        {   
          Name (_ADR, Zero)  // _ADR: Address
          Name (_STA, 0x0F)  // _STA: Status
          Name (_PRW, Package (0x02)  // _PRW: Power Resources for Wake
            {   
                0x0F, 
                0x03
            })
        }

        Device (I2C)
        {   
          Name (_ADR, One)  // _ADR: Address
          Name (_STA, 0x0F)  // _STA: Status
          Name (_PRW, Package (0x02)  // _PRW: Power Resources for Wake
            {   
                0x0F, 
                0x03
            })
        }
    }

> 
> > diff --git a/drivers/mfd/intel_quark_i2c_gpio.c 
> > b/drivers/mfd/intel_quark_i2c_gpio.c
> > index 1ce1603..89bd4bf 100644
> > --- a/drivers/mfd/intel_quark_i2c_gpio.c
> > +++ b/drivers/mfd/intel_quark_i2c_gpio.c
> > @@ -82,27 +82,26 @@ static struct resource intel_quark_i2c_res[] = 
> > {
> >  	},
> >  };
> >  
> > +static struct mfd_cell intel_quark_mfd_i2c_cell = {
> > +	.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 struct resource intel_quark_gpio_res[] = {
> >  	[INTEL_QUARK_IORES_MEM] = {
> >  		.flags = IORESOURCE_MEM,
> >  	},
> >  };
> >  
> > -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,
> > -	},
> > +static struct mfd_cell intel_quark_mfd_gpio_cell = {
> > +	.id = MFD_GPIO_BAR,
> > +	.name = "gpio-dwapb",
> > +	.num_resources = ARRAY_SIZE(intel_quark_gpio_res),
> > +	.resources = intel_quark_gpio_res,
> > +	.ignore_resource_conflicts = true,
> >  };
> >  
> >  static const struct pci_device_id intel_quark_mfd_ids[] = {
> > @@ -248,18 +247,25 @@ 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_i2c_cell);
> >  	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_gpio_cell);
> >  	if (ret)
> >  		return ret;
> >  
> > -	return mfd_add_devices(&pdev->dev, 0, 
> > intel_quark_mfd_cells,
> > -			       ARRAY_SIZE(intel_quark_mfd_cells), 
> > NULL, 0,
> > -			       NULL);
> > +	ret = mfd_add_devices(&pdev->dev, 0, 
> > &intel_quark_mfd_gpio_cell, 1,
> > +			      NULL, 0, NULL);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = mfd_add_devices(&pdev->dev, 0, 
> > &intel_quark_mfd_i2c_cell, 1,
> > +			      NULL, 0, NULL);
> > +	if (ret)
> > +		mfd_remove_devices(&pdev->dev);
> > +
> > +	return ret;
> >  }
> >  
> >  static void intel_quark_mfd_remove(struct pci_dev *pdev)
> 

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

Re: [PATCH v1 3/8] mfd: intel_quark_i2c_gpio: load gpio driver first

[Lee Jones]

On Tue, 22 Sep 2015, Andy Shevchenko wrote:

> 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.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/intel_quark_i2c_gpio.c | 48 +++++++++++++++++++++-----------------
>  1 file changed, 27 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
> index 1ce1603..89bd4bf 100644
> --- a/drivers/mfd/intel_quark_i2c_gpio.c
> +++ b/drivers/mfd/intel_quark_i2c_gpio.c
> @@ -82,27 +82,26 @@ static struct resource intel_quark_i2c_res[] = {
>  	},
>  };
>  
> +static struct mfd_cell intel_quark_mfd_i2c_cell = {
> +	.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 struct resource intel_quark_gpio_res[] = {
>  	[INTEL_QUARK_IORES_MEM] = {
>  		.flags = IORESOURCE_MEM,
>  	},
>  };
>  
> -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,
> -	},
> +static struct mfd_cell intel_quark_mfd_gpio_cell = {
> +	.id = MFD_GPIO_BAR,
> +	.name = "gpio-dwapb",
> +	.num_resources = ARRAY_SIZE(intel_quark_gpio_res),
> +	.resources = intel_quark_gpio_res,
> +	.ignore_resource_conflicts = true,
>  };
>  
>  static const struct pci_device_id intel_quark_mfd_ids[] = {
> @@ -248,18 +247,25 @@ 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_i2c_cell);
>  	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_gpio_cell);
>  	if (ret)
>  		return ret;
>  
> -	return mfd_add_devices(&pdev->dev, 0, intel_quark_mfd_cells,
> -			       ARRAY_SIZE(intel_quark_mfd_cells), NULL, 0,
> -			       NULL);
> +	ret = mfd_add_devices(&pdev->dev, 0, &intel_quark_mfd_gpio_cell, 1,
> +			      NULL, 0, NULL);
> +	if (ret)
> +		return ret;
> +
> +	ret = mfd_add_devices(&pdev->dev, 0, &intel_quark_mfd_i2c_cell, 1,
> +			      NULL, 0, NULL);
> +	if (ret)
> +		mfd_remove_devices(&pdev->dev);
> +
> +	return ret;
>  }
>  
>  static void intel_quark_mfd_remove(struct pci_dev *pdev)

--- 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[] = {

?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 4/8] 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.

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 89bd4bf..e9fd33b 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,9 +86,14 @@ 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 mfd_cell intel_quark_mfd_i2c_cell = {
 	.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,
@@ -96,9 +105,14 @@ static struct resource intel_quark_gpio_res[] = {
 	},
 };
 
+static struct mfd_cell_acpi_match intel_quark_acpi_match_gpio = {
+	.adr = MFD_ACPI_MATCH_GPIO,
+};
+
 static struct mfd_cell intel_quark_mfd_gpio_cell = {
 	.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,
-- 
2.5.1

Re: [PATCH v1 4/8] mfd: intel_quark_i2c_gpio: support devices behind i2c bus

[Lee Jones]

On Tue, 22 Sep 2015, Andy Shevchenko wrote:

> 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.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/intel_quark_i2c_gpio.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

Acked-by: Lee Jones <lee.jones@linaro.org>

> diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
> index 89bd4bf..e9fd33b 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,9 +86,14 @@ 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 mfd_cell intel_quark_mfd_i2c_cell = {
>  	.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,
> @@ -96,9 +105,14 @@ static struct resource intel_quark_gpio_res[] = {
>  	},
>  };
>  
> +static struct mfd_cell_acpi_match intel_quark_acpi_match_gpio = {
> +	.adr = MFD_ACPI_MATCH_GPIO,
> +};
> +
>  static struct mfd_cell intel_quark_mfd_gpio_cell = {
>  	.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,

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

[PATCH v1 5/8] gpio: pca953x: store driver_data for future use

[Andy Shevchenko]

Instead of using id->driver_data directly we copied it to the internal
structure. This will help to adapt driver for ACPI use.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-pca953x.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 50caeb1..242e244 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -42,6 +42,9 @@
 #define PCA_INT			0x0100
 #define PCA953X_TYPE		0x1000
 #define PCA957X_TYPE		0x2000
+#define PCA_TYPE_MASK		0xF000
+
+#define PCA_CHIP_TYPE(x)	((x) & PCA_TYPE_MASK)
 
 static const struct i2c_device_id pca953x_id[] = {
 	{ "pca9505", 40 | PCA953X_TYPE | PCA_INT, },
@@ -95,6 +98,7 @@ struct pca953x_chip {
 	struct gpio_chip gpio_chip;
 	const char *const *names;
 	int	chip_type;
+	unsigned long driver_data;
 };
 
 static inline struct pca953x_chip *to_pca(struct gpio_chip *gc)
@@ -517,14 +521,13 @@ static irqreturn_t pca953x_irq_handler(int irq, void *devid)
 }
 
 static int pca953x_irq_setup(struct pca953x_chip *chip,
-			     const struct i2c_device_id *id,
 			     int irq_base)
 {
 	struct i2c_client *client = chip->client;
 	int ret, i, offset = 0;
 
 	if (client->irq && irq_base != -1
-			&& (id->driver_data & PCA_INT)) {
+			&& (chip->driver_data & PCA_INT)) {
 
 		switch (chip->chip_type) {
 		case PCA953X_TYPE:
@@ -581,12 +584,11 @@ static int pca953x_irq_setup(struct pca953x_chip *chip,
 
 #else /* CONFIG_GPIO_PCA953X_IRQ */
 static int pca953x_irq_setup(struct pca953x_chip *chip,
-			     const struct i2c_device_id *id,
 			     int irq_base)
 {
 	struct i2c_client *client = chip->client;
 
-	if (irq_base != -1 && (id->driver_data & PCA_INT))
+	if (irq_base != -1 && (chip->driver_data & PCA_INT))
 		dev_warn(&client->dev, "interrupt support not compiled in\n");
 
 	return 0;
@@ -673,14 +675,15 @@ static int pca953x_probe(struct i2c_client *client,
 
 	chip->client = client;
 
-	chip->chip_type = id->driver_data & (PCA953X_TYPE | PCA957X_TYPE);
+	chip->driver_data = id->driver_data;
+	chip->chip_type = PCA_CHIP_TYPE(chip->driver_data);
 
 	mutex_init(&chip->i2c_lock);
 
 	/* initialize cached registers from their original values.
 	 * we can't share this chip with another i2c master.
 	 */
-	pca953x_setup_gpio(chip, id->driver_data & PCA_GPIO_MASK);
+	pca953x_setup_gpio(chip, chip->driver_data & PCA_GPIO_MASK);
 
 	if (chip->chip_type == PCA953X_TYPE)
 		ret = device_pca953x_init(chip, invert);
@@ -693,7 +696,7 @@ static int pca953x_probe(struct i2c_client *client,
 	if (ret)
 		return ret;
 
-	ret = pca953x_irq_setup(chip, id, irq_base);
+	ret = pca953x_irq_setup(chip, irq_base);
 	if (ret)
 		return ret;
 
-- 
2.5.1

Re: [PATCH v1 5/8] gpio: pca953x: store driver_data for future use

[Linus Walleij]

On Tue, Sep 22, 2015 at 3:10 AM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Instead of using id->driver_data directly we copied it to the internal
> structure. This will help to adapt driver for ACPI use.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Gregory, Grygorii, Graeme: can I get some help in reviewing these
PCA patches for ACPI support?

Yours,
Linus Walleij

Re: [PATCH v1 5/8] gpio: pca953x: store driver_data for future use

[Andy Shevchenko]

On Fri, 2015-10-02 at 03:53 -0700, Linus Walleij wrote:
> On Tue, Sep 22, 2015 at 3:10 AM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> 
> > Instead of using id->driver_data directly we copied it to the 
> > internal
> > structure. This will help to adapt driver for ACPI use.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Gregory, Grygorii, Graeme: can I get some help in reviewing these
> PCA patches for ACPI support?

I would like to notice that there is v2 of the series (GPIO patches
weren't changed AFAIR).

http://www.spinics.net/lists/kernel/msg2087827.html

> 
> Yours,
> Linus Walleij

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

[PATCH v1 6/8] gpio: pca953x: support ACPI devices found on Galileo Gen2

[Andy Shevchenko]

This patch adds a support of the expandes found on Intel Galileo Gen2 board.
The platform information comes from ACPI.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-pca953x.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 242e244..0220b38 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -21,6 +21,7 @@
 #ifdef CONFIG_OF_GPIO
 #include <linux/of_platform.h>
 #endif
+#include <linux/acpi.h>
 
 #define PCA953X_INPUT		0
 #define PCA953X_OUTPUT		1
@@ -75,6 +76,12 @@ static const struct i2c_device_id pca953x_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, pca953x_id);
 
+static const struct acpi_device_id pca953x_acpi_ids[] = {
+	{ "INT3491", 16 | PCA953X_TYPE | PCA_INT, },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, pca953x_acpi_ids);
+
 #define MAX_BANK 5
 #define BANK_SZ 8
 
@@ -675,7 +682,18 @@ static int pca953x_probe(struct i2c_client *client,
 
 	chip->client = client;
 
-	chip->driver_data = id->driver_data;
+	if (id) {
+		chip->driver_data = id->driver_data;
+	} else {
+		const struct acpi_device_id *id;
+
+		id = acpi_match_device(pca953x_acpi_ids, &client->dev);
+		if (!id)
+			return -ENODEV;
+
+		chip->driver_data = id->driver_data;
+	}
+
 	chip->chip_type = PCA_CHIP_TYPE(chip->driver_data);
 
 	mutex_init(&chip->i2c_lock);
@@ -768,6 +786,7 @@ static struct i2c_driver pca953x_driver = {
 	.driver = {
 		.name	= "pca953x",
 		.of_match_table = pca953x_dt_ids,
+		.acpi_match_table = ACPI_PTR(pca953x_acpi_ids),
 	},
 	.probe		= pca953x_probe,
 	.remove		= pca953x_remove,
-- 
2.5.1

Re: [PATCH v1 6/8] gpio: pca953x: support ACPI devices found on Galileo Gen2

[Linus Walleij]

On Tue, Sep 22, 2015 at 3:10 AM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> This patch adds a support of the expandes found on Intel Galileo Gen2 board.
> The platform information comes from ACPI.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Paging Gregory, Grygorii, Graeme on this patch too.

Yours,
Linus Walleij

[PATCH v1 7/8] 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.1

[PATCH v1 8/8] 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 | 14 +++++++++++---
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 062630a..bb114ef 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -242,7 +242,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..38ea28b 100644
--- a/drivers/pwm/pwm-pca9685.c
+++ b/drivers/pwm/pwm-pca9685.c
@@ -26,6 +26,8 @@
 #include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/delay.h>
+#include <linux/acpi.h>
+#include <linux/property.h>
 
 /*
  * Because the PCA9685 has only one prescaler per chip, changing the period of
@@ -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,6 +364,12 @@ static const struct i2c_device_id pca9685_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, pca9685_id);
 
+static const struct acpi_device_id pca9685_acpi_ids[] = {
+	{ "INT3492", 0 },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(acpi, pca9685_acpi_ids);
+
 static const struct of_device_id pca9685_dt_ids[] = {
 	{ .compatible = "nxp,pca9685-pwm", },
 	{ /* sentinel */ }
@@ -372,6 +379,7 @@ MODULE_DEVICE_TABLE(of, pca9685_dt_ids);
 static struct i2c_driver pca9685_i2c_driver = {
 	.driver = {
 		.name = "pca9685-pwm",
+		.acpi_match_table = ACPI_PTR(pca9685_acpi_ids),
 		.of_match_table = pca9685_dt_ids,
 	},
 	.probe = pca9685_pwm_probe,
-- 
2.5.1

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

[Thierry Reding]

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

On Tue, Sep 22, 2015 at 01:10:19PM +0300, Andy Shevchenko wrote:
> 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 | 14 +++++++++++---
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index 062630a..bb114ef 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -242,7 +242,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..38ea28b 100644
> --- a/drivers/pwm/pwm-pca9685.c
> +++ b/drivers/pwm/pwm-pca9685.c
> @@ -26,6 +26,8 @@
>  #include <linux/regmap.h>
>  #include <linux/slab.h>
>  #include <linux/delay.h>
> +#include <linux/acpi.h>
> +#include <linux/property.h>

These should be kept sorted. I know that delay.h isn't properly ordered
either, I missed that during patch review. Please keep new ones ordered
alphabetically and I'll sort out the delay.h via a separate patch.

>  
>  /*
>   * Because the PCA9685 has only one prescaler per chip, changing the period of
> @@ -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,6 +364,12 @@ static const struct i2c_device_id pca9685_id[] = {
>  };
>  MODULE_DEVICE_TABLE(i2c, pca9685_id);
>  
> +static const struct acpi_device_id pca9685_acpi_ids[] = {
> +	{ "INT3492", 0 },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(acpi, pca9685_acpi_ids);
> +
>  static const struct of_device_id pca9685_dt_ids[] = {
>  	{ .compatible = "nxp,pca9685-pwm", },
>  	{ /* sentinel */ }
> @@ -372,6 +379,7 @@ MODULE_DEVICE_TABLE(of, pca9685_dt_ids);
>  static struct i2c_driver pca9685_i2c_driver = {
>  	.driver = {
>  		.name = "pca9685-pwm",
> +		.acpi_match_table = ACPI_PTR(pca9685_acpi_ids),

I think you now need #ifdef protection for the ACPI ID table, otherwise
the compiler will warn that the table is unused for !ACPI.

>  		.of_match_table = pca9685_dt_ids,

Similarly to the above, this should use of_match_ptr(), which in turn
will require #ifdef protection for the table to avoid warnings.

Thierry

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

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

[Andy Shevchenko]

On Tue, 2015-09-22 at 16:37 +0200, Thierry Reding wrote:
> On Tue, Sep 22, 2015 at 01:10:19PM +0300, Andy Shevchenko wrote:

> > --- a/drivers/pwm/pwm-pca9685.c
> > +++ b/drivers/pwm/pwm-pca9685.c
> > @@ -26,6 +26,8 @@
> >  #include <linux/regmap.h>
> >  #include <linux/slab.h>
> >  #include <linux/delay.h>
> > +#include <linux/acpi.h>
> > +#include <linux/property.h>
> 
> These should be kept sorted. I know that delay.h isn't properly 
> ordered
> either, I missed that during patch review. Please keep new ones 
> ordered
> alphabetically and I'll sort out the delay.h via a separate patch.

Will do in next version.

> @@ -363,6 +364,12 @@ static const struct i2c_device_id pca9685_id[]
> > = {
> >  };
> >  MODULE_DEVICE_TABLE(i2c, pca9685_id);
> >  
> > +static const struct acpi_device_id pca9685_acpi_ids[] = {
> > +	{ "INT3492", 0 },
> > +	{ /* sentinel */ },
> > +};
> > +MODULE_DEVICE_TABLE(acpi, pca9685_acpi_ids);
> > +
> >  static const struct of_device_id pca9685_dt_ids[] = {
> >  	{ .compatible = "nxp,pca9685-pwm", },
> >  	{ /* sentinel */ }
> > @@ -372,6 +379,7 @@ MODULE_DEVICE_TABLE(of, pca9685_dt_ids);
> >  static struct i2c_driver pca9685_i2c_driver = {
> >  	.driver = {
> >  		.name = "pca9685-pwm",
> > +		.acpi_match_table = ACPI_PTR(pca9685_acpi_ids),
> 
> I think you now need #ifdef protection for the ACPI ID table, 
> otherwise
> the compiler will warn that the table is unused for !ACPI.

No, there is no warning, just checked a build with CONFIG_ACPI=n.

Tried even with C=1 W=2, and driver compiled in and a module.
In all variants no warning regarding the topic is issued.

$ gcc --version
gcc (Debian 5.2.1-17) 5.2.1 20150911

Perhaps this would explain what is happening there.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=28901

So, I will add #ifdef in the code as well, though I'm not a big fan of
conditional compilation.

> 
> >  		.of_match_table = pca9685_dt_ids,
> 
> Similarly to the above, this should use of_match_ptr(), which in turn
> will require #ifdef protection for the table to avoid warnings.

Hmm... my patch do not touches that part. Perhaps another patch for
this?

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

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

[Thierry Reding]

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

On Wed, Sep 23, 2015 at 11:41:26AM +0300, Andy Shevchenko wrote:
> On Tue, 2015-09-22 at 16:37 +0200, Thierry Reding wrote:
> > On Tue, Sep 22, 2015 at 01:10:19PM +0300, Andy Shevchenko wrote:
> 
> > > --- a/drivers/pwm/pwm-pca9685.c
> > > +++ b/drivers/pwm/pwm-pca9685.c
> > > @@ -26,6 +26,8 @@
> > >  #include <linux/regmap.h>
> > >  #include <linux/slab.h>
> > >  #include <linux/delay.h>
> > > +#include <linux/acpi.h>
> > > +#include <linux/property.h>
> > 
> > These should be kept sorted. I know that delay.h isn't properly 
> > ordered
> > either, I missed that during patch review. Please keep new ones 
> > ordered
> > alphabetically and I'll sort out the delay.h via a separate patch.
> 
> Will do in next version.
> 
> > @@ -363,6 +364,12 @@ static const struct i2c_device_id pca9685_id[]
> > > = {
> > >  };
> > >  MODULE_DEVICE_TABLE(i2c, pca9685_id);
> > >  
> > > +static const struct acpi_device_id pca9685_acpi_ids[] = {
> > > +	{ "INT3492", 0 },
> > > +	{ /* sentinel */ },
> > > +};
> > > +MODULE_DEVICE_TABLE(acpi, pca9685_acpi_ids);
> > > +
> > >  static const struct of_device_id pca9685_dt_ids[] = {
> > >  	{ .compatible = "nxp,pca9685-pwm", },
> > >  	{ /* sentinel */ }
> > > @@ -372,6 +379,7 @@ MODULE_DEVICE_TABLE(of, pca9685_dt_ids);
> > >  static struct i2c_driver pca9685_i2c_driver = {
> > >  	.driver = {
> > >  		.name = "pca9685-pwm",
> > > +		.acpi_match_table = ACPI_PTR(pca9685_acpi_ids),
> > 
> > I think you now need #ifdef protection for the ACPI ID table, 
> > otherwise
> > the compiler will warn that the table is unused for !ACPI.
> 
> No, there is no warning, just checked a build with CONFIG_ACPI=n.
> 
> Tried even with C=1 W=2, and driver compiled in and a module.
> In all variants no warning regarding the topic is issued.
> 
> $ gcc --version
> gcc (Debian 5.2.1-17) 5.2.1 20150911
> 
> Perhaps this would explain what is happening there.
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=28901
> 
> So, I will add #ifdef in the code as well, though I'm not a big fan of
> conditional compilation.

I'm pretty sure I've seen warnings for this with 5.2.0, but I'll put
this in my tree to check. Irrespective I think it should have the #ifdef
protection because I'm very certain that the warning is there with some
versions of GCC that people might still be using. And I don't much like
conditional compilation either, but anything producing a warning will
cause someone to write a patch to fix it, so I just want to be proactive
in avoiding that kind of churn.

> > 
> > >  		.of_match_table = pca9685_dt_ids,
> > 
> > Similarly to the above, this should use of_match_ptr(), which in turn
> > will require #ifdef protection for the table to avoid warnings.
> 
> Hmm... my patch do not touches that part. Perhaps another patch for
> this?

Your patch does touch that part by removing the dependency on OF. That
makes it possible to build this code with !OF, which in turn would make
the OF match table unused.

Thierry

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

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

[Andy Shevchenko]

On Wed, 2015-09-23 at 14:48 +0200, Thierry Reding wrote:
> > > > 

> On Wed, Sep 23, 2015 at 11:41:26AM +0300, Andy Shevchenko wrote:
> > On Tue, 2015-09-22 at 16:37 +0200, Thierry Reding wrote:
> > > On Tue, Sep 22, 2015 at 01:10:19PM +0300, Andy Shevchenko wrote:
> > 

> > > 
> > > >  		.of_match_table = pca9685_dt_ids,
> > > 
> > > Similarly to the above, this should use of_match_ptr(), which in 
> > > turn
> > > will require #ifdef protection for the table to avoid warnings.
> > 
> > Hmm... my patch do not touches that part. Perhaps another patch for
> > this?
> 
> Your patch does touch that part by removing the dependency on OF. 
> That
> makes it possible to build this code with !OF, which in turn would 
> make
> the OF match table unused.

Ah, you're right. Will amend this part as well.
Thanks!

> 
> Thierry

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