[v6,1/4] i2c-core-acpi : Add i2c_acpi_set_connection

Message ID 20171225155723.6338-1-m.capdeville@no-log.org
State Superseded
Headers show
Series
  • [v6,1/4] i2c-core-acpi : Add i2c_acpi_set_connection
Related show

Commit Message

CAPDEVILLE Marc Dec. 25, 2017, 3:57 p.m.
Add a methode to allow clients to change their connection address on
same adapter.

On ACPI enumerated device, when a device support smbus alert protocol,
there are two acpi serial bus connection description. The order in which
connection is given is not well defined and devices may be enumerated
with the wrong address (the first one). So let the driver detect the
unsupported address and request i2c acpi core to choose the second one
at probing time.

Signed-off-by: Marc CAPDEVILLE <m.capdeville@no-log.org>
---
 drivers/i2c/i2c-core-acpi.c | 50 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c.h         | 10 +++++++++
 2 files changed, 60 insertions(+)

Comments

Rafael J. Wysocki Dec. 28, 2017, 1:04 a.m. | #1
On Monday, December 25, 2017 4:57:20 PM CET Marc CAPDEVILLE wrote:
> Add a methode to allow clients to change their connection address on
> same adapter.
> 
> On ACPI enumerated device, when a device support smbus alert protocol,
> there are two acpi serial bus connection description. The order in which
> connection is given is not well defined and devices may be enumerated
> with the wrong address (the first one). So let the driver detect the
> unsupported address and request i2c acpi core to choose the second one
> at probing time.

By convention, the ordering of resources in _CRS is meaningful, but it is
a contract between AML and an OS driver written to work with it.

> Signed-off-by: Marc CAPDEVILLE <m.capdeville@no-log.org>
> ---
>  drivers/i2c/i2c-core-acpi.c | 50 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/i2c.h         | 10 +++++++++
>  2 files changed, 60 insertions(+)
> 
> diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
> index a9126b3cda61..47bc0da12055 100644
> --- a/drivers/i2c/i2c-core-acpi.c
> +++ b/drivers/i2c/i2c-core-acpi.c
> @@ -421,6 +421,56 @@ struct i2c_client *i2c_acpi_new_device(struct device *dev, int index,
>  }
>  EXPORT_SYMBOL_GPL(i2c_acpi_new_device);
>  

Please add a kerneldoc description of this function.

It is exported and needs to be documented properly.

> +int i2c_acpi_set_connection(struct i2c_client *client, int index)
> +{
> +	struct i2c_acpi_lookup lookup;
> +	struct i2c_adapter *adapter;
> +	struct acpi_device *adev;
> +	struct i2c_board_info info;
> +	LIST_HEAD(resource_list);
> +	int ret;
> +
> +	if (!client)
> +		return -ENODEV;
> +
> +	adev = ACPI_COMPANION(&client->dev);
> +	if (!adev)
> +		return -ENODEV;
> +
> +	memset(&info,  0, sizeof(info));
> +	memset(&lookup, 0, sizeof(lookup));
> +	lookup.info = &info;
> +	lookup.device_handle = acpi_device_handle(adev);
> +	lookup.index = index;
> +
> +	ret = acpi_dev_get_resources(adev, &resource_list,
> +				     i2c_acpi_fill_info, &lookup);
> +	acpi_dev_free_resource_list(&resource_list);
> +
> +	adapter = i2c_acpi_find_adapter_by_handle(lookup.adapter_handle);
> +
> +	if (ret < 0 || !info.addr)
> +		return -EINVAL;

Why do you make this check after calling i2c_acpi_find_adapter_by_handle()?

> +
> +	/* Only accept connection on same adapter */
> +	if (adapter != client->adapter)
> +		return -EINVAL;
> +
> +	ret = i2c_check_addr_validity(info.addr, info.flags);
> +	if (ret) {
> +		dev_err(&client->dev, "Invalid %d-bit I2C address 0x%02hx\n",
> +			info.flags & I2C_CLIENT_TEN ? 10 : 7, info.addr);
> +		return -EINVAL;
> +	}
> +
> +	/* Set new address and flags */
> +	client->addr = info.addr;
> +	client->flags = info.flags;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(i2c_acpi_set_connection);
> +
>  #ifdef CONFIG_ACPI_I2C_OPREGION
>  static int acpi_gsb_i2c_read_bytes(struct i2c_client *client,
>  		u8 cmd, u8 *data, u8 data_len)
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 0f774406fad0..618b453901da 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -595,6 +595,8 @@ struct i2c_adapter {
>  	const struct i2c_adapter_quirks *quirks;
>  
>  	struct irq_domain *host_notify_domain;
> +
> +	struct i2c_client *smbus_ara;	/* ARA for SMBUS if present */

Not used in this patch and how is it related to the other changes here?

>  };
>  #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
>  
> @@ -839,16 +841,24 @@ static inline const struct of_device_id
>  u32 i2c_acpi_find_bus_speed(struct device *dev);
>  struct i2c_client *i2c_acpi_new_device(struct device *dev, int index,
>  				       struct i2c_board_info *info);
> +
> +int i2c_acpi_set_connection(struct i2c_client *client, int index);
>  #else
>  static inline u32 i2c_acpi_find_bus_speed(struct device *dev)
>  {
>  	return 0;
>  }
> +
>  static inline struct i2c_client *i2c_acpi_new_device(struct device *dev,
>  					int index, struct i2c_board_info *info)
>  {
>  	return NULL;
>  }
> +
> +static inline int i2c_acpi_set_connection(struct i2c_client *client, int index)
> +{
> +	return -EINVAL;
> +}
>  #endif /* CONFIG_ACPI */
>  
>  #endif /* _LINUX_I2C_H */
>

Patch

diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
index a9126b3cda61..47bc0da12055 100644
--- a/drivers/i2c/i2c-core-acpi.c
+++ b/drivers/i2c/i2c-core-acpi.c
@@ -421,6 +421,56 @@  struct i2c_client *i2c_acpi_new_device(struct device *dev, int index,
 }
 EXPORT_SYMBOL_GPL(i2c_acpi_new_device);
 
+int i2c_acpi_set_connection(struct i2c_client *client, int index)
+{
+	struct i2c_acpi_lookup lookup;
+	struct i2c_adapter *adapter;
+	struct acpi_device *adev;
+	struct i2c_board_info info;
+	LIST_HEAD(resource_list);
+	int ret;
+
+	if (!client)
+		return -ENODEV;
+
+	adev = ACPI_COMPANION(&client->dev);
+	if (!adev)
+		return -ENODEV;
+
+	memset(&info,  0, sizeof(info));
+	memset(&lookup, 0, sizeof(lookup));
+	lookup.info = &info;
+	lookup.device_handle = acpi_device_handle(adev);
+	lookup.index = index;
+
+	ret = acpi_dev_get_resources(adev, &resource_list,
+				     i2c_acpi_fill_info, &lookup);
+	acpi_dev_free_resource_list(&resource_list);
+
+	adapter = i2c_acpi_find_adapter_by_handle(lookup.adapter_handle);
+
+	if (ret < 0 || !info.addr)
+		return -EINVAL;
+
+	/* Only accept connection on same adapter */
+	if (adapter != client->adapter)
+		return -EINVAL;
+
+	ret = i2c_check_addr_validity(info.addr, info.flags);
+	if (ret) {
+		dev_err(&client->dev, "Invalid %d-bit I2C address 0x%02hx\n",
+			info.flags & I2C_CLIENT_TEN ? 10 : 7, info.addr);
+		return -EINVAL;
+	}
+
+	/* Set new address and flags */
+	client->addr = info.addr;
+	client->flags = info.flags;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(i2c_acpi_set_connection);
+
 #ifdef CONFIG_ACPI_I2C_OPREGION
 static int acpi_gsb_i2c_read_bytes(struct i2c_client *client,
 		u8 cmd, u8 *data, u8 data_len)
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 0f774406fad0..618b453901da 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -595,6 +595,8 @@  struct i2c_adapter {
 	const struct i2c_adapter_quirks *quirks;
 
 	struct irq_domain *host_notify_domain;
+
+	struct i2c_client *smbus_ara;	/* ARA for SMBUS if present */
 };
 #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
 
@@ -839,16 +841,24 @@  static inline const struct of_device_id
 u32 i2c_acpi_find_bus_speed(struct device *dev);
 struct i2c_client *i2c_acpi_new_device(struct device *dev, int index,
 				       struct i2c_board_info *info);
+
+int i2c_acpi_set_connection(struct i2c_client *client, int index);
 #else
 static inline u32 i2c_acpi_find_bus_speed(struct device *dev)
 {
 	return 0;
 }
+
 static inline struct i2c_client *i2c_acpi_new_device(struct device *dev,
 					int index, struct i2c_board_info *info)
 {
 	return NULL;
 }
+
+static inline int i2c_acpi_set_connection(struct i2c_client *client, int index)
+{
+	return -EINVAL;
+}
 #endif /* CONFIG_ACPI */
 
 #endif /* _LINUX_I2C_H */