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@opensource.cirrus.com
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/?i...
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