Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1243753 > unrolled thread

[PATCH 1/2] i2c: scan entire ACPI namespace for I2C connections

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/2] i2c: scan entire ACPI namespace for I2C connections Dustin Byford <dustin@cumulusnetworks.com> - 2015-10-10 02:50 +0200
    Re: [PATCH 1/2] i2c: scan entire ACPI namespace for I2C connections Mika Westerberg <mika.westerberg@linux.intel.com> - 2015-10-12 12:50 +0200
      Re: [PATCH 1/2] i2c: scan entire ACPI namespace for I2C connections Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2015-10-12 13:30 +0200
        Re: [PATCH 1/2] i2c: scan entire ACPI namespace for I2C connections Dustin Byford <dustin@cumulusnetworks.com> - 2015-10-12 19:10 +0200
    Re: [PATCH 1/2] i2c: scan entire ACPI namespace for I2C connections "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-10-12 20:40 +0200
      Re: [PATCH 1/2] i2c: scan entire ACPI namespace for I2C connections Dustin Byford <dustin@cumulusnetworks.com> - 2015-10-12 21:00 +0200

#1243753 — [PATCH 1/2] i2c: scan entire ACPI namespace for I2C connections

An I2cSerialBus connection resource descriptor may indicate a
ResourceSource (a string uniquely identifying the I2C bus controller)
anywhere in the ACPI namespace.  However, when enumerating connections to a
I2C bus controller, i2c-core.c:acpi_i2c_register_devices() as only
searching devices that are descendants of the bus controller.

This change corrects acpi_i2c_register_devices() to walk the entire ACPI
namespace searching for I2C connections.

Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Dustin Byford <dustin@cumulusnetworks.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 5f89f1e..3a4c54e 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.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1244586

On Fri, Oct 09, 2015 at 05:41:46PM -0700, Dustin Byford wrote:
> An I2cSerialBus connection resource descriptor may indicate a
> ResourceSource (a string uniquely identifying the I2C bus controller)
> anywhere in the ACPI namespace.  However, when enumerating connections to a
> I2C bus controller, i2c-core.c:acpi_i2c_register_devices() as only
> searching devices that are descendants of the bus controller.
> 
> This change corrects acpi_i2c_register_devices() to walk the entire ACPI
> namespace searching for I2C connections.
> 
> Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Dustin Byford <dustin@cumulusnetworks.com>

This patch is already included in Andy's latest series adding support
for Intel Galileo here:

https://patchwork.ozlabs.org/patch/527231/

Maybe we can add your Tested-by/Reviewed-by to that patch instead?

> ---
>  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 5f89f1e..3a4c54e 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.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1244603

On Mon, 2015-10-12 at 13:46 +0300, Mika Westerberg wrote:
> On Fri, Oct 09, 2015 at 05:41:46PM -0700, Dustin Byford wrote:
> > An I2cSerialBus connection resource descriptor may indicate a
> > ResourceSource (a string uniquely identifying the I2C bus 
> > controller)
> > anywhere in the ACPI namespace.  However, when enumerating 
> > connections to a
> > I2C bus controller, i2c-core.c:acpi_i2c_register_devices() as only
> > searching devices that are descendants of the bus controller.
> > 
> > This change corrects acpi_i2c_register_devices() to walk the entire 
> > ACPI
> > namespace searching for I2C connections.
> > 
> > Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Signed-off-by: Dustin Byford <dustin@cumulusnetworks.com>
> 
> This patch is already included in Andy's latest series adding support
> for Intel Galileo here:
> 
> https://patchwork.ozlabs.org/patch/527231/
> 
> Maybe we can add your Tested-by/Reviewed-by to that patch instead?

Indeed, the Tested-by tag would be much appreciated!

> 
> > ---
> >  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 5f89f1e..3a4c54e 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))

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1244933

On Mon Oct 12 14:20, Andy Shevchenko wrote:
> On Mon, 2015-10-12 at 13:46 +0300, Mika Westerberg wrote:
> > On Fri, Oct 09, 2015 at 05:41:46PM -0700, Dustin Byford wrote:
> > > An I2cSerialBus connection resource descriptor may indicate a
> > > ResourceSource (a string uniquely identifying the I2C bus 
> > > controller)
> > > anywhere in the ACPI namespace.  However, when enumerating 
> > > connections to a
> > > I2C bus controller, i2c-core.c:acpi_i2c_register_devices() as only
> > > searching devices that are descendants of the bus controller.
> > > 
> > > This change corrects acpi_i2c_register_devices() to walk the entire 
> > > ACPI
> > > namespace searching for I2C connections.
> > > 
> > > Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > > Signed-off-by: Dustin Byford <dustin@cumulusnetworks.com>
> > 
> > This patch is already included in Andy's latest series adding support
> > for Intel Galileo here:
> > 
> > https://patchwork.ozlabs.org/patch/527231/
> > 
> > Maybe we can add your Tested-by/Reviewed-by to that patch instead?
> 
> Indeed, the Tested-by tag would be much appreciated!

OK, that works for me.  I didn't realize it had been submitted elsewhere
or I wouldn't have sent the duplicate.

Thanks.

		--Dustin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1244989

On Friday, October 09, 2015 05:41:46 PM Dustin Byford wrote:
> An I2cSerialBus connection resource descriptor may indicate a
> ResourceSource (a string uniquely identifying the I2C bus controller)
> anywhere in the ACPI namespace.  However, when enumerating connections to a
> I2C bus controller, i2c-core.c:acpi_i2c_register_devices() as only
> searching devices that are descendants of the bus controller.
> 
> This change corrects acpi_i2c_register_devices() to walk the entire ACPI
> namespace searching for I2C connections.
> 
> Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Dustin Byford <dustin@cumulusnetworks.com>

This one has already been submitted by Andy and I've ACKed it.

I'm not sure what to do here, though.

I guess I can apply this one and put it into a branch for others to pull from.

Thoughts?

> ---
>  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 5f89f1e..3a4c54e 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))
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1245029

On Mon Oct 12 21:01, Rafael J. Wysocki wrote:
> On Friday, October 09, 2015 05:41:46 PM Dustin Byford wrote:
> > An I2cSerialBus connection resource descriptor may indicate a
> > ResourceSource (a string uniquely identifying the I2C bus controller)
> > anywhere in the ACPI namespace.  However, when enumerating connections to a
> > I2C bus controller, i2c-core.c:acpi_i2c_register_devices() as only
> > searching devices that are descendants of the bus controller.
> > 
> > This change corrects acpi_i2c_register_devices() to walk the entire ACPI
> > namespace searching for I2C connections.
> > 
> > Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Signed-off-by: Dustin Byford <dustin@cumulusnetworks.com>
> 
> This one has already been submitted by Andy and I've ACKed it.

Sorry, I missed that before I sent the patch.

> I'm not sure what to do here, though.
> 
> I guess I can apply this one and put it into a branch for others to pull from.
>
> Thoughts?

I'd be OK if you just drop this patch and I won't include it in my next
revision.

		--Dustin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web