Re: [PATCH 1/2] i2c: consider devices with of_match_table during i2c device probing

Wolfram Sang
2020-08-26T05:08:59Z

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

On Wed, Aug 26, 2020 at 01:29:37PM +0900, Sergey Senozhatsky wrote:
> Unlike acpi_match_device(), acpi_driver_match_device() does
> consider devices that provide of_match_table and performs
> of_compatible() matching for such devices. The key point here is
> that ACPI of_compatible() matching - acpi_of_match_device() - is
> part of ACPI and does not depend on CONFIG_OF.
>
> Consider the following case:
> o !CONFIG_OF system
> o probing of i2c device that provides .of_match_table, but no .id_table
>
> i2c_device_probe()
> ...
> if (!driver->id_table &&
> !i2c_acpi_match_device(dev->driver->acpi_match_table, client) &&
> !i2c_of_match_device(dev->driver->of_match_table, client)) {
> status = -ENODEV;
> goto put_sync_adapter;
> }
>
> i2c_of_match_device() depends on CONFIG_OF and, thus, is always false.
> i2c_acpi_match_device() does ACPI match only, no of_comtatible() matching
> takes place, even though the device provides .of_match_table and ACPI,
> technically, is capable of matching such device. The result is -ENODEV.
> Probing will succeed, however, if we'd use .of_match_table aware ACPI
> matching.
>
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

We have currently this in for-current which is even removing
i2c_acpi_match_device():

http://patchwork.ozlabs.org/project/linux-i2c/list/?series=196990&state=*

From a glimpse, this is basically equal but CCing Andy for the details.

> ---
> drivers/i2c/i2c-core-acpi.c | 10 ++++------
> drivers/i2c/i2c-core-base.c | 2 +-
> drivers/i2c/i2c-core.h | 10 +++-------
> 3 files changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
> index 2ade99b105b9..1dd152ae75e5 100644
> --- a/drivers/i2c/i2c-core-acpi.c
> +++ b/drivers/i2c/i2c-core-acpi.c
> @@ -276,14 +276,12 @@ void i2c_acpi_register_devices(struct i2c_adapter *adap)
> dev_warn(&adap->dev, "failed to enumerate I2C slaves\n");
> }
>
> -const struct acpi_device_id *
> -i2c_acpi_match_device(const struct acpi_device_id *matches,
> - struct i2c_client *client)
> +bool i2c_acpi_match_device(struct device *dev, struct i2c_client *client)
> {
> - if (!(client && matches))
> - return NULL;
> + if (!client)
> + return false;
>
> - return acpi_match_device(matches, &client->dev);
> + return acpi_driver_match_device(&client->dev, dev->driver);
> }
>
> static const struct acpi_device_id i2c_acpi_force_400khz_device_ids[] = {
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 34a9609f256d..35ec6335852b 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -480,7 +480,7 @@ static int i2c_device_probe(struct device *dev)
> * or ACPI ID table is supplied for the probing device.
> */
> if (!driver->id_table &&
> - !i2c_acpi_match_device(dev->driver->acpi_match_table, client) &&
> + !i2c_acpi_match_device(dev, client) &&
> !i2c_of_match_device(dev->driver->of_match_table, client)) {
> status = -ENODEV;
> goto put_sync_adapter;
> diff --git a/drivers/i2c/i2c-core.h b/drivers/i2c/i2c-core.h
> index 94ff1693b391..7ee6a6e3fb8d 100644
> --- a/drivers/i2c/i2c-core.h
> +++ b/drivers/i2c/i2c-core.h
> @@ -59,19 +59,15 @@ static inline int __i2c_check_suspended(struct i2c_adapter *adap)
> }
>
> #ifdef CONFIG_ACPI
> -const struct acpi_device_id *
> -i2c_acpi_match_device(const struct acpi_device_id *matches,
> - struct i2c_client *client);
> +bool i2c_acpi_match_device(struct device *dev, struct i2c_client *client);
> void i2c_acpi_register_devices(struct i2c_adapter *adap);
>
> int i2c_acpi_get_irq(struct i2c_client *client);
> #else /* CONFIG_ACPI */
> static inline void i2c_acpi_register_devices(struct i2c_adapter *adap) { }
> -static inline const struct acpi_device_id *
> -i2c_acpi_match_device(const struct acpi_device_id *matches,
> - struct i2c_client *client)
> +bool i2c_acpi_match_device(struct device *dev, struct i2c_client *client)
> {
> - return NULL;
> + return false;
> }
>
> static inline int i2c_acpi_get_irq(struct i2c_client *client)
> --
> 2.28.0
>

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