Re: [PATCH] ACPI / scan: Don't create platform device for INT3515 ACPI nodes

[Andy Shevchenko]

On Wed, Dec 23, 2020 at 4:40 PM Heikki Krogerus
<heikki.krogerus@xxxxxxxxxxxxxxx> wrote:
>
> There are several reports about the tps6598x causing
> interrupt flood on boards with the INT3515 ACPI node, which
> then causes instability. There appears to be several
> problems with the interrupt. One problem is that the
> I2CSerialBus resources do not always map to the Interrupt
> resource with the same index, but that is not the only
> problem. We have not been able to come up with a solution
> for all the issues, and because of that disabling the device
> for now.
>
> The PD controller on these platforms is autonomous, and the
> purpose for the driver is primarily to supply status to the
> userspace, so this will not affect any functionality.
>
> Reported-by: Moody Salem <moody@xxxxxxxxxxx>
> Fixes: a3dd034a1707 ("ACPI / scan: Create platform device for INT3515 ACPI nodes")
> Cc: stable@xxxxxxxxxxxxxxx

> Link: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1883511

BugLink: ?

> Signed-off-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> ---
>  drivers/platform/x86/i2c-multi-instantiate.c | 31 +++++++++++++++-----
>  1 file changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/platform/x86/i2c-multi-instantiate.c b/drivers/platform/x86/i2c-multi-instantiate.c
> index 6acc8457866e1..e1df665d3ad31 100644
> --- a/drivers/platform/x86/i2c-multi-instantiate.c
> +++ b/drivers/platform/x86/i2c-multi-instantiate.c
> @@ -166,13 +166,29 @@ static const struct i2c_inst_data bsg2150_data[]  = {
>         {}
>  };
>
> -static const struct i2c_inst_data int3515_data[]  = {
> -       { "tps6598x", IRQ_RESOURCE_APIC, 0 },
> -       { "tps6598x", IRQ_RESOURCE_APIC, 1 },
> -       { "tps6598x", IRQ_RESOURCE_APIC, 2 },
> -       { "tps6598x", IRQ_RESOURCE_APIC, 3 },
> -       {}
> -};
> +/*
> + * Device with _HID INT3515 (TI PD controllers) has some unresolved interrupt
> + * issues. The most common problem seen is interrupt flood.
> + *
> + * There are at least two known causes. Firstly, on some boards, the
> + * I2CSerialBus resource index does not match the Interrupt resource, i.e. they
> + * are not one-to-one mapped like in the array below. Secondly, on some boards
> + * the irq line from the PD controller is not actually connected at all. But the

irq -> IRQ

> + * interrupt flood is also seen on some boards where those are not a problem, so
> + * there are some other problems as well.
> + *
> + * Because of the issues with the interrupt, the device is disabled for now. If
> + * you wish to debug the issues, uncomment the below, and add an entry for the
> + * INT3515 device to the i2c_multi_instance__ids table.

Extra _ (underscore).

> + *
> + * static const struct i2c_inst_data int3515_data[]  = {
> + *     { "tps6598x", IRQ_RESOURCE_APIC, 0 },
> + *     { "tps6598x", IRQ_RESOURCE_APIC, 1 },
> + *     { "tps6598x", IRQ_RESOURCE_APIC, 2 },
> + *     { "tps6598x", IRQ_RESOURCE_APIC, 3 },
> + *     { }
> + * };
> + */
>
>  /*
>   * Note new device-ids must also be added to i2c_multi_instantiate_ids in
> @@ -181,7 +197,6 @@ static const struct i2c_inst_data int3515_data[]  = {
>  static const struct acpi_device_id i2c_multi_inst_acpi_ids[] = {
>         { "BSG1160", (unsigned long)bsg1160_data },
>         { "BSG2150", (unsigned long)bsg2150_data },
> -       { "INT3515", (unsigned long)int3515_data },

Perhaps also comment it out and refer to the above note?

>         { }
>  };
>  MODULE_DEVICE_TABLE(acpi, i2c_multi_inst_acpi_ids);
> --
> 2.29.2
>


-- 
With Best Regards,
Andy Shevchenko