On Fri, Dec 4, 2020 at 8:51 AM Hui Wang <hui.wang@xxxxxxxxxxxxx> wrote:
>
> We are working on some latest Thinkpad Yoga and Carbon laptops, the
> touchscreen doesn't work on those machines. And we found the
> touchscreen module is I2C wacom WACF2200 (056A:5276).
>
> The problem is in the acpi_pnp.c, the WACFXXX is in the
> acpi_pnp_device_ids[], so a pnp device will be built and attach to the
> acpi_dev as the 1st physical_node, later when I2C subsystem starts to
> initialize, it will build an I2C_dev and attach to the acpi_dev as the
> 2nd physical_node. When I2C bus needs to match the acpi_id_table, it
> will call acpi_companion_match(), because the 1st physical_node is not
> I2C_dev, it fails to match, then the i2c driver (hid_i2c) will not be
> called.
>
> To fix it, adding a special treatment in the companion_match(): if the
> 1st dev is on pnp bus and the device in question is not on pnp bus,
> skip the 1st physical device, just use the device in question to
> match.
>
> We could refer to the pnpacpi_add_device() in the
> pnp/pnpacpi/core.c, pnp device will not be built if the acpi_dev
> is already attached to a physical device, so a pnp device has
> lower priority than other devices, it is safe to skip it in
> the companion_match().
>
> Signed-off-by: Hui Wang <hui.wang@xxxxxxxxxxxxx>
> ---
> drivers/acpi/bus.c | 26 ++++++++++++++++++++++----
> 1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 1682f8b454a2..8aa0a861ca29 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -582,6 +582,15 @@ bool acpi_device_is_first_physical_node(struct acpi_device *adev,
> return !!acpi_primary_dev_companion(adev, dev);
> }
>
> +/* Could move this function to linux/pnp.h in the future */
> +static bool acpi_dev_is_on_pnp_bus(const struct device *dev)
> +{
> + if (dev->bus)
> + return !strcmp(dev->bus->name, "pnp");
> + else
Unnecessary else.
> + return false;
> +}
> +
> /*
> * acpi_companion_match() - Can we match via ACPI companion device
> * @dev: Device in question
> @@ -597,7 +606,9 @@ bool acpi_device_is_first_physical_node(struct acpi_device *adev,
> * companion. A typical case is an MFD device where all the sub-devices share
> * the parent's ACPI companion. In such cases we can only allow the primary
> * (first) physical device to be matched with the help of the companion's PNP
> - * IDs.
> + * IDs. And another case is a pnp device is attached to ACPI device first, then
> + * other function devices are attached too, in this case, the primary physical
> + * device (pnp) is ignored, just use the device in question to match.
> *
> * Additional physical devices sharing the ACPI companion can still use
> * resources available from it but they will be matched normally using functions
> @@ -605,7 +616,7 @@ bool acpi_device_is_first_physical_node(struct acpi_device *adev,
> */
> struct acpi_device *acpi_companion_match(const struct device *dev)
> {
> - struct acpi_device *adev;
> + struct acpi_device *adev, *radev;
>
> adev = ACPI_COMPANION(dev);
> if (!adev)
> @@ -614,7 +625,15 @@ struct acpi_device *acpi_companion_match(const struct device *dev)
> if (list_empty(&adev->pnp.ids))
> return NULL;
>
> - return acpi_primary_dev_companion(adev, dev);
> + radev = acpi_primary_dev_companion(adev, dev);
> + if (radev == NULL) {
> + const struct device *first_dev = acpi_get_first_physical_node(adev);
> +
> + if (acpi_dev_is_on_pnp_bus(first_dev) && !acpi_dev_is_on_pnp_bus(dev))
> + radev = adev;
> + }
> +
> + return radev;
> }
This is too convoluted IMV.
Would dropping the device ID in question from acpi_pnp_device_ids[]
make the problem go away?
If so, why don't you do just that?
