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

[Wolfram Sang]

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@xxxxxxxxx>

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: signature.asc
Description: PGP signature