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@xxxxxxxxxx>
> ---
> 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 */
>
