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

From: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
To: Wolfram Sang <wsa@kernel.org>,
	Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: linux-i2c@vger.kernel.org, linux-acpi@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Subject: Re: [PATCH 1/2] i2c: consider devices with of_match_table during i2c device probing
Date: Wed, 26 Aug 2020 14:07:11 +0900	[thread overview]
Message-ID: <20200826055951.GA407@jagdpanzerIV.localdomain> (raw)
In-Reply-To: <20200826042938.3259-1-sergey.senozhatsky@gmail.com>

On (20/08/26 13:29), 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.

Or, alternatively, we can drop i2c_acpi_match_device() and use
i2c_device_match() in i2c_device_probe().

---

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 34a9609f256d..14bfc5c83f84 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -96,7 +96,6 @@ static int i2c_device_match(struct device *dev, struct device_driver *drv)
 	struct i2c_client	*client = i2c_verify_client(dev);
 	struct i2c_driver	*driver;
 
-
 	/* Attempt an OF style match */
 	if (i2c_of_match_device(drv->of_match_table, client))
 		return 1;
@@ -480,8 +479,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_of_match_device(dev->driver->of_match_table, client)) {
+	    !(client && i2c_device_match(&client->dev, dev->driver))) {
 		status = -ENODEV;
 		goto put_sync_adapter;
 	}
diff --git a/drivers/i2c/i2c-core.h b/drivers/i2c/i2c-core.h
index 94ff1693b391..8ce261167a2d 100644
--- a/drivers/i2c/i2c-core.h
+++ b/drivers/i2c/i2c-core.h
@@ -59,20 +59,11 @@ 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);
 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)
-{
-	return NULL;
-}
 
 static inline int i2c_acpi_get_irq(struct i2c_client *client)
 {
