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

2015-10-12 Thread Dustin Byford 
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 
> > Signed-off-by: Dustin Byford 
> 
> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2015-10-12 Thread Rafael J. Wysocki 
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 
> Signed-off-by: Dustin Byford 

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 = >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 = >data.i2c_serial_bus;
> + if (sb->type != ACPI_RESOURCE_SERIAL_TYPE_I2C)
> + return 1;
>  
> - if (acpi_dev_resource_interrupt(ares, 0, ))
> - 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,
> +  _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(, 0, sizeof(info));
>   info.fwnode = acpi_fwnode_handle(adev);
>  
> + memset(, 0, sizeof(lookup));
> + lookup.adapter_handle = ACPI_HANDLE(adapter->dev.parent);
> + lookup.device_handle = handle;
> + lookup.info = 
> +
> + /*
> +  * Look up for I2cSerialBus resource with ResourceSource that
> +  * matches with this adapter.
> +  */
>   INIT_LIST_HEAD(_list);
>   ret = acpi_dev_get_resources(adev, _list,
> -  acpi_i2c_add_resource, );
> +  acpi_i2c_find_address, );
>   acpi_dev_free_resource_list(_list);
>  
>   if (ret < 0 || !info.addr)
>   return AE_OK;
>  
> + /* Then fill IRQ number if any */
> + ret = acpi_dev_get_resources(adev, _list, NULL, NULL);
> + if (ret < 0)
> + return AE_OK;
> +
> + resource_list_for_each_entry(entry, _list) {
> + if (resource_type(entry->res) == IORESOURCE_IRQ) {
> + info.irq = entry->res->start;
> + break;
> + }
> + }
> +
> + acpi_dev_free_resource_list(_list);
> +
>   adev->power.flags.ignore_parent = true;
>   strlcpy(info.type, dev_name(>dev), sizeof(info.type));
>   if (!i2c_new_device(adapter, )) {
> @@ -160,6 +198,8 @@ static

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

2015-10-12 Thread Dustin Byford 
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 
> > > Signed-off-by: Dustin Byford 
> > 
> > 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2015-10-12 Thread Andy Shevchenko 
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 
> > Signed-off-by: Dustin Byford 
> 
> 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 = >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 = >data.i2c_serial_bus;
> > +   if (sb->type != ACPI_RESOURCE_SERIAL_TYPE_I2C)
> > +   return 1;
> >  
> > -   if (acpi_dev_resource_interrupt(ares, 0, ))
> > -   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,
> > +_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(, 0, sizeof(info));
> > info.fwnode = acpi_fwnode_handle(adev);
> >  
> > +   memset(, 0, sizeof(lookup));
> > +   lookup.adapter_handle = ACPI_HANDLE(adapter->dev.parent);
> > +   lookup.device_handle = handle;
> > +   lookup.info = 
> > +
> > +   /*
> > +* Look up for I2cSerialBus resource with ResourceSource 
> > that
> > +* matches with this adapter.
> > +*/
> > INIT_LIST_HEAD(_list);
> > ret = acpi_dev_get_resources(adev, _list,
> > -acpi_i2c_add_resource, 
> > );
> > +acpi_i2c_find_address, 
> > );
> > acpi_dev_free_resource_list(_list);
> >  
> > if (ret < 0 || !info.addr)
> > return AE_OK;
> >  
> > +   /* Then fill IRQ number if any */
> > +   ret = acpi_dev_get_resources(adev, _list, NULL, 
> > NULL);
> > +   if (ret < 0)
> > +   return AE_OK;
> > +
> > +   resource_list_for_each_entry(entry, _list) {
> > +   if (resource_type(entry->res) == IORESOURCE_IRQ) {
> > +   info.irq =

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

2015-10-12 Thread Mika Westerberg 
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 
> Signed-off-by: Dustin Byford 

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 = >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 = >data.i2c_serial_bus;
> + if (sb->type != ACPI_RESOURCE_SERIAL_TYPE_I2C)
> + return 1;
>  
> - if (acpi_dev_resource_interrupt(ares, 0, ))
> - 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,
> +  _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(, 0, sizeof(info));
>   info.fwnode = acpi_fwnode_handle(adev);
>  
> + memset(, 0, sizeof(lookup));
> + lookup.adapter_handle = ACPI_HANDLE(adapter->dev.parent);
> + lookup.device_handle = handle;
> + lookup.info = 
> +
> + /*
> +  * Look up for I2cSerialBus resource with ResourceSource that
> +  * matches with this adapter.
> +  */
>   INIT_LIST_HEAD(_list);
>   ret = acpi_dev_get_resources(adev, _list,
> -  acpi_i2c_add_resource, );
> +  acpi_i2c_find_address, );
>   acpi_dev_free_resource_list(_list);
>  
>   if (ret < 0 || !info.addr)
>   return AE_OK;
>  
> + /* Then fill IRQ number if any */
> + ret = acpi_dev_get_resources(adev, _list, NULL, NULL);
> + if (ret < 0)
> + return AE_OK;
> +
> + resource_list_for_each_entry(entry, _list) {
> + if (resource_type(entry->res) == IORESOURCE_IRQ) {
> + info.irq = entry->res->start;
> + break;
> + }
> + }
> +
> + acpi_dev_free_resource_list(_list);
> +
>   adev->power.flags.ignore_parent = true;
>   strlcpy(info.type, dev_name(>dev), sizeof(info.type));
>   if (!i2c_new_device(adapter, )) {
> @@ -160,6 +198,8

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

2015-10-12 Thread Mika Westerberg 
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 
> Signed-off-by: Dustin Byford 

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 = >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 = >data.i2c_serial_bus;
> + if (sb->type != ACPI_RESOURCE_SERIAL_TYPE_I2C)
> + return 1;
>  
> - if (acpi_dev_resource_interrupt(ares, 0, ))
> - 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,
> +  _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(, 0, sizeof(info));
>   info.fwnode = acpi_fwnode_handle(adev);
>  
> + memset(, 0, sizeof(lookup));
> + lookup.adapter_handle = ACPI_HANDLE(adapter->dev.parent);
> + lookup.device_handle = handle;
> + lookup.info = 
> +
> + /*
> +  * Look up for I2cSerialBus resource with ResourceSource that
> +  * matches with this adapter.
> +  */
>   INIT_LIST_HEAD(_list);
>   ret = acpi_dev_get_resources(adev, _list,
> -  acpi_i2c_add_resource, );
> +  acpi_i2c_find_address, );
>   acpi_dev_free_resource_list(_list);
>  
>   if (ret < 0 || !info.addr)
>   return AE_OK;
>  
> + /* Then fill IRQ number if any */
> + ret = acpi_dev_get_resources(adev, _list, NULL, NULL);
> + if (ret < 0)
> + return AE_OK;
> +
> + resource_list_for_each_entry(entry, _list) {
> + if (resource_type(entry->res) == IORESOURCE_IRQ) {
> + info.irq = entry->res->start;
> + break;
> + }
> + }
> +
> + acpi_dev_free_resource_list(_list);
> +
>   adev->power.flags.ignore_parent = true;
>   strlcpy(info.type, dev_name(>dev), sizeof(info.type));
>

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

2015-10-12 Thread Andy Shevchenko 
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 
> > Signed-off-by: Dustin Byford 
> 
> 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 = >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 = >data.i2c_serial_bus;
> > +   if (sb->type != ACPI_RESOURCE_SERIAL_TYPE_I2C)
> > +   return 1;
> >  
> > -   if (acpi_dev_resource_interrupt(ares, 0, ))
> > -   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,
> > +_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(, 0, sizeof(info));
> > info.fwnode = acpi_fwnode_handle(adev);
> >  
> > +   memset(, 0, sizeof(lookup));
> > +   lookup.adapter_handle = ACPI_HANDLE(adapter->dev.parent);
> > +   lookup.device_handle = handle;
> > +   lookup.info = 
> > +
> > +   /*
> > +* Look up for I2cSerialBus resource with ResourceSource 
> > that
> > +* matches with this adapter.
> > +*/
> > INIT_LIST_HEAD(_list);
> > ret = acpi_dev_get_resources(adev, _list,
> > -acpi_i2c_add_resource, 
> > );
> > +acpi_i2c_find_address, 
> > );
> > acpi_dev_free_resource_list(_list);
> >  
> > if (ret < 0 || !info.addr)
> > return AE_OK;
> >  
> > +   /* Then fill IRQ number if any */
> > +   ret = acpi_dev_get_resources(adev, _list, NULL, 
> > NULL);
> > +   if (ret < 0)
> > +   return AE_OK;
> > +
> > +   resource_list_for_each_entry(entry, _list) {
> > +   if (resource_type(entry->res) ==

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

2015-10-12 Thread Dustin Byford 
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 
> > > Signed-off-by: Dustin Byford 
> > 
> > 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2015-10-12 Thread Rafael J. Wysocki 
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 
> Signed-off-by: Dustin Byford 

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 = >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 = >data.i2c_serial_bus;
> + if (sb->type != ACPI_RESOURCE_SERIAL_TYPE_I2C)
> + return 1;
>  
> - if (acpi_dev_resource_interrupt(ares, 0, ))
> - 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,
> +  _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(, 0, sizeof(info));
>   info.fwnode = acpi_fwnode_handle(adev);
>  
> + memset(, 0, sizeof(lookup));
> + lookup.adapter_handle = ACPI_HANDLE(adapter->dev.parent);
> + lookup.device_handle = handle;
> + lookup.info = 
> +
> + /*
> +  * Look up for I2cSerialBus resource with ResourceSource that
> +  * matches with this adapter.
> +  */
>   INIT_LIST_HEAD(_list);
>   ret = acpi_dev_get_resources(adev, _list,
> -  acpi_i2c_add_resource, );
> +  acpi_i2c_find_address, );
>   acpi_dev_free_resource_list(_list);
>  
>   if (ret < 0 || !info.addr)
>   return AE_OK;
>  
> + /* Then fill IRQ number if any */
> + ret = acpi_dev_get_resources(adev, _list, NULL, NULL);
> + if (ret < 0)
> + return AE_OK;
> +
> + resource_list_for_each_entry(entry, _list) {
> + if (resource_type(entry->res) == IORESOURCE_IRQ) {
> + info.irq = entry->res->start;
> + break;
> + }
> + }
> +
> + acpi_dev_free_resource_list(_list);
> +
>   adev->power.flags.ignore_parent = true;
>   strlcpy(info.type, dev_name(>dev), sizeof(info.type));
>   if

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

2015-10-12 Thread Dustin Byford 
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 
> > Signed-off-by: Dustin Byford 
> 
> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/