Hi Stefan, On 11/24/22 12:07, Stefan Binding wrote: > This allows the i2c driver to obtain the ACPI_COMPANION. > > Signed-off-by: Stefan Binding <sbinding@xxxxxxxxxxxxxxxxxxxxx> > --- > drivers/platform/x86/serial-multi-instantiate.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/platform/x86/serial-multi-instantiate.c b/drivers/platform/x86/serial-multi-instantiate.c > index 5362f1a7b77c..15ef2f3c442e 100644 > --- a/drivers/platform/x86/serial-multi-instantiate.c > +++ b/drivers/platform/x86/serial-multi-instantiate.c > @@ -194,6 +194,7 @@ static int smi_i2c_probe(struct platform_device *pdev, struct smi *smi, > strscpy(board_info.type, inst_array[i].type, I2C_NAME_SIZE); > snprintf(name, sizeof(name), "%s-%s.%d", dev_name(dev), inst_array[i].type, i); > board_info.dev_name = name; > + board_info.fwnode = acpi_fwnode_handle(adev); > > ret = smi_get_irq(pdev, adev, &inst_array[i]); > if (ret < 0) I'm afraid that making this change is not as straight forward as it looks. I know that I have tried to do this in the past and it failed. IIRC there were 3 problems: 1. I was expecting this to also allow the driver for the instantiated i2c-client to be able to bind using an acpi_match_table but that unfortunately does not work. acpi_match_table matches only work for the first physical_node linked under /sys/bus/acpi/devices/xxxx:xx/physical_node and that is the platform device to which serial-multi-instantiate.c binds. The i2c_client becomes the second physical node. Note this is not really an issue, just something to be aware of. 2. This causes the i2c-core to use the first IRQ resource in the ACPI fwnode as client->irq for any clients for which we do not set an IRQ when instantiating. Which may very well be wrong. Sometimes that IRQ is only valid for the first i2c-client which we instantiate; and not for the others! And sometimes it is a problem because it may point to an irqchip for which we never wrote a driver leading to all probes of the i2c-client failing with -EPROBE_DEFER, see: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d1d84bb95364ed604015c2b788caaf3dbca0262f Note that patch has been reverted since that specific -EPROBE_DEFER issue has been solved by making the ACPI core instantiate a platform_device instead of an i2c_client (in this case we did not need the actual i2c_client at all). The current i2c-core code has a (!client-irq) test guarding its code of trying to use the first ACPI fwnode IRQ resource. So we could disable this by setting client->irq = -ENOENT in serial-multi-instantiate.c when (inst->flags & IRQ_RESOURCE_TYPE) == IRQ_RESOURCE_NONE). But that will introduce a new problem. Many i2c-drivers check if there is an IRQ for them to use by doing: "if (client->irq) request_irq(client->irq, ...)" but then with error checking/so setting client->irq to -ENOENT will cause the request_irq to fail, leading the probe to fail. So before you can write a patch setting client->irq = -ENOENT when (inst->flags & IRQ_RESOURCE_TYPE) == IRQ_RESOURCE_NONE), you would first need to patch all i2c-drivers for clients instantiated through serial-multi-instantiate.c changing: if (client->irq) { ... } to: if (client->irq > 0) { ... } Note this is not as bad as it sounds, since there are only a few drivers for clients instantiated by serial-multi-instantiate.c . 3. Some drivers may check for an ACPI companion device and then change their behavior. So all drivers for clients instantiated through serial-multi-instantiate.c will need to be audited for this (and a summary of this audit needs to be added to the commit msg). Regards, Hans
