linux-kernel - Re: [PATCH 1/2] i2c: consider devices with of_match

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200826050851.GA1081@ninjato>
Date:   Wed, 26 Aug 2020 07:08:51 +0200
From:   Wolfram Sang <wsa@...nel.org>
To:     Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Mika Westerberg <mika.westerberg@...ux.intel.com>,
        linux-i2c@...r.kernel.org, linux-acpi@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] i2c: consider devices with of_match_table during i2c
 device probing

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@...il.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
> 

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)