* [RFC 1/4] i2c: core: Allow i2c_board_info to specify that the core should not try to find an IRQ
2020-11-05 8:00 [RFC 0/4] platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated i2c-clients Hans de Goede
@ 2020-11-05 8:00 ` Hans de Goede
2020-11-05 8:00 ` [RFC 2/4] platform/x86: i2c-multi-instantiate: Set i2c_board_info.irq to -ENOENT when no IRQ is specified Hans de Goede
` (3 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Hans de Goede @ 2020-11-05 8:00 UTC (permalink / raw)
To: Mark Gross, Wolfram Sang
Cc: Hans de Goede, Andy Shevchenko, platform-driver-x86, linux-i2c,
linux-acpi
When I2C devices are enumerated/instantiated through ACPI tables then
a single ACPI device may describe multiple separate I2C connected ICs.
This is handled by the drivers/platform/x86/i2c-multi-instantiate.c
code which contains a table which maps the ACPI-device-id to the
information necessary to instantiate the i2c-clients (type and IRQ
routing for each described IC).
In some cases the i2c-driver may need access to the ACPI-fwnode as
that may contain ACPI-methods supplying e.g. orientation-matrix info
for accelerometers.
Currently setting i2c_board_info.fwnode to point to the ACPI-fwnode
will cause the i2c-core to call i2c_acpi_get_irq() for any i2c-clients
for which i2c-multi-instantiate.c has not passed an IRQ in
i2c_board_info.irq, messing up the IRQ routing done by
i2c-multi-instantiate.c.
Make i2c_device_probe() accept a client->init_irq value < 0 to skip
the i2c-core IRQ handling, while still setting client->irq to 0
after checking for this, since most i2c-drivers expect client->irq == 0
for clients without an IRQ.
This allows i2c-multi-instantiate.c to set i2c_board_info.irq = -ENOENT
for clients without an IRQ and pass the ACPI-fwnode without issues.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/i2c/i2c-core-base.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 573b5da145d1..1887e2267031 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -467,11 +467,10 @@ static int i2c_device_probe(struct device *dev)
goto put_sync_adapter;
}
- if (irq < 0)
- irq = 0;
-
client->irq = irq;
}
+ if (client->irq < 0)
+ client->irq = 0;
driver = to_i2c_driver(dev->driver);
--
2.28.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC 2/4] platform/x86: i2c-multi-instantiate: Set i2c_board_info.irq to -ENOENT when no IRQ is specified
2020-11-05 8:00 [RFC 0/4] platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated i2c-clients Hans de Goede
2020-11-05 8:00 ` [RFC 1/4] i2c: core: Allow i2c_board_info to specify that the core should not try to find an IRQ Hans de Goede
@ 2020-11-05 8:00 ` Hans de Goede
2020-11-05 8:00 ` [RFC 3/4] platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated I2C-clients Hans de Goede
` (2 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Hans de Goede @ 2020-11-05 8:00 UTC (permalink / raw)
To: Mark Gross, Wolfram Sang
Cc: Hans de Goede, Andy Shevchenko, platform-driver-x86, linux-i2c,
linux-acpi
In some cases the i2c-driver may need access to the ACPI-fwnode as
that may contain ACPI-methods supplying e.g. orientation-matrix info
for accelerometers.
Setting i2c_board_info.fwnode to point to the ACPI-fwnode, while
leaving i2c_board_info.irq set to 0 (in the IRQ_RESOURCE_NONE case)
will cause the i2c-core to assign the first IRQ described in the ACPI
resources to the client, which we do not want.
Set i2c_board_info.irq to -ENOENT instead of 0 in the IRQ_RESOURCE_NONE
case, to avoid this issue.
This is a preparation patch for passing the fwnode to i2c_acpi_new_device.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/platform/x86/i2c-multi-instantiate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/platform/x86/i2c-multi-instantiate.c b/drivers/platform/x86/i2c-multi-instantiate.c
index 6acc8457866e..cb4688bdd6b6 100644
--- a/drivers/platform/x86/i2c-multi-instantiate.c
+++ b/drivers/platform/x86/i2c-multi-instantiate.c
@@ -113,7 +113,7 @@ static int i2c_multi_inst_probe(struct platform_device *pdev)
board_info.irq = ret;
break;
default:
- board_info.irq = 0;
+ board_info.irq = -ENOENT;
break;
}
multi->clients[i] = i2c_acpi_new_device(dev, i, &board_info);
--
2.28.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC 3/4] platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated I2C-clients
2020-11-05 8:00 [RFC 0/4] platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated i2c-clients Hans de Goede
2020-11-05 8:00 ` [RFC 1/4] i2c: core: Allow i2c_board_info to specify that the core should not try to find an IRQ Hans de Goede
2020-11-05 8:00 ` [RFC 2/4] platform/x86: i2c-multi-instantiate: Set i2c_board_info.irq to -ENOENT when no IRQ is specified Hans de Goede
@ 2020-11-05 8:00 ` Hans de Goede
2020-11-05 10:31 ` Andy Shevchenko
2020-11-05 8:00 ` [RFC 4/4] platform/x86: i2c-multi-instantiate: Add IRQ_RESOURCE_GPIO_OPTIONAL IRQ type Hans de Goede
2020-11-05 10:38 ` [RFC 0/4] platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated i2c-clients Andy Shevchenko
4 siblings, 1 reply; 14+ messages in thread
From: Hans de Goede @ 2020-11-05 8:00 UTC (permalink / raw)
To: Mark Gross, Wolfram Sang
Cc: Hans de Goede, Andy Shevchenko, platform-driver-x86, linux-i2c,
linux-acpi
The ACPI fwnode may contain additional info which is useful for the
I2C-driver. E.g. some accelerometer ACPI fwnode's contain an ACPI method
providing rotation/mount matrix info.
Pass the ACPI-fwnode to the instantiated I2C-clients by setting
i2c_board_info.fwnode, so that the I2C-drivers can access this info.
Now that we set i2c_board_info.irq to -ENOENT if there is no IRQ,
avoiding the I2C-core assigning the first IRQ described in the ACPI
resources to the client, this is safe to do.
Setting the fwnode also influences acpi_device_[uevent_]modalias and
acpi_dev_pm_attach, but these both call acpi_device_is_first_physical_node
and are a no-op if this returns false.
The first physical node for the ACPI fwnode is actually the ACPI core
instantiated platform-device to which the I2C-multi-instantiate driver
binds, so acpi_device_is_first_physical_node always returns false for
the instantiated I2C-clients and thus we can safely pass the fwnode.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/platform/x86/i2c-multi-instantiate.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/platform/x86/i2c-multi-instantiate.c b/drivers/platform/x86/i2c-multi-instantiate.c
index cb4688bdd6b6..cbccfcbed44c 100644
--- a/drivers/platform/x86/i2c-multi-instantiate.c
+++ b/drivers/platform/x86/i2c-multi-instantiate.c
@@ -93,6 +93,7 @@ static int i2c_multi_inst_probe(struct platform_device *pdev)
snprintf(name, sizeof(name), "%s-%s.%d", dev_name(dev),
inst_data[i].type, i);
board_info.dev_name = name;
+ board_info.fwnode = dev->fwnode;
switch (inst_data[i].flags & IRQ_RESOURCE_TYPE) {
case IRQ_RESOURCE_GPIO:
ret = acpi_dev_gpio_irq_get(adev, inst_data[i].irq_idx);
--
2.28.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [RFC 3/4] platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated I2C-clients
2020-11-05 8:00 ` [RFC 3/4] platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated I2C-clients Hans de Goede
@ 2020-11-05 10:31 ` Andy Shevchenko
0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2020-11-05 10:31 UTC (permalink / raw)
To: Hans de Goede
Cc: Mark Gross, Wolfram Sang, Andy Shevchenko, Platform Driver,
linux-i2c, ACPI Devel Maling List
On Thu, Nov 5, 2020 at 10:00 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> The ACPI fwnode may contain additional info which is useful for the
> I2C-driver. E.g. some accelerometer ACPI fwnode's contain an ACPI method
> providing rotation/mount matrix info.
>
> Pass the ACPI-fwnode to the instantiated I2C-clients by setting
> i2c_board_info.fwnode, so that the I2C-drivers can access this info.
>
> Now that we set i2c_board_info.irq to -ENOENT if there is no IRQ,
> avoiding the I2C-core assigning the first IRQ described in the ACPI
> resources to the client, this is safe to do.
>
> Setting the fwnode also influences acpi_device_[uevent_]modalias and
> acpi_dev_pm_attach, but these both call acpi_device_is_first_physical_node
> and are a no-op if this returns false.
Perhaps add () to the mentioned function calls.
> The first physical node for the ACPI fwnode is actually the ACPI core
> instantiated platform-device to which the I2C-multi-instantiate driver
> binds, so acpi_device_is_first_physical_node always returns false for
> the instantiated I2C-clients and thus we can safely pass the fwnode.
Ditto.
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/platform/x86/i2c-multi-instantiate.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/platform/x86/i2c-multi-instantiate.c b/drivers/platform/x86/i2c-multi-instantiate.c
> index cb4688bdd6b6..cbccfcbed44c 100644
> --- a/drivers/platform/x86/i2c-multi-instantiate.c
> +++ b/drivers/platform/x86/i2c-multi-instantiate.c
> @@ -93,6 +93,7 @@ static int i2c_multi_inst_probe(struct platform_device *pdev)
> snprintf(name, sizeof(name), "%s-%s.%d", dev_name(dev),
> inst_data[i].type, i);
> board_info.dev_name = name;
> + board_info.fwnode = dev->fwnode;
> switch (inst_data[i].flags & IRQ_RESOURCE_TYPE) {
> case IRQ_RESOURCE_GPIO:
> ret = acpi_dev_gpio_irq_get(adev, inst_data[i].irq_idx);
> --
> 2.28.0
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFC 4/4] platform/x86: i2c-multi-instantiate: Add IRQ_RESOURCE_GPIO_OPTIONAL IRQ type
2020-11-05 8:00 [RFC 0/4] platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated i2c-clients Hans de Goede
` (2 preceding siblings ...)
2020-11-05 8:00 ` [RFC 3/4] platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated I2C-clients Hans de Goede
@ 2020-11-05 8:00 ` Hans de Goede
2020-11-05 10:36 ` Andy Shevchenko
2020-11-05 10:38 ` [RFC 0/4] platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated i2c-clients Andy Shevchenko
4 siblings, 1 reply; 14+ messages in thread
From: Hans de Goede @ 2020-11-05 8:00 UTC (permalink / raw)
To: Mark Gross, Wolfram Sang
Cc: Hans de Goede, Andy Shevchenko, platform-driver-x86, linux-i2c,
linux-acpi
Most I2C-drivers treat IRQs as optional and in some cases ACPI
devices managed by i2c-multi-instantiate.c may have a GpioInt resource
specified on some systems, while it is not there on others.
Add a new IRQ_RESOURCE_GPIO_OPTIONAL IRQ type, which still tries to get
a GpioInt IRQ, but does not consider it a fatal error when this fails.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/platform/x86/i2c-multi-instantiate.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/platform/x86/i2c-multi-instantiate.c b/drivers/platform/x86/i2c-multi-instantiate.c
index cbccfcbed44c..55c6d6e8d576 100644
--- a/drivers/platform/x86/i2c-multi-instantiate.c
+++ b/drivers/platform/x86/i2c-multi-instantiate.c
@@ -15,10 +15,11 @@
#include <linux/platform_device.h>
#include <linux/types.h>
-#define IRQ_RESOURCE_TYPE GENMASK(1, 0)
-#define IRQ_RESOURCE_NONE 0
-#define IRQ_RESOURCE_GPIO 1
-#define IRQ_RESOURCE_APIC 2
+#define IRQ_RESOURCE_TYPE GENMASK(1, 0)
+#define IRQ_RESOURCE_NONE 0
+#define IRQ_RESOURCE_GPIO 1
+#define IRQ_RESOURCE_GPIO_OPTIONAL 2
+#define IRQ_RESOURCE_APIC 3
struct i2c_inst_data {
const char *type;
@@ -64,6 +65,7 @@ static int i2c_multi_inst_probe(struct platform_device *pdev)
struct i2c_board_info board_info = {};
struct device *dev = &pdev->dev;
struct acpi_device *adev;
+ bool irq_optional;
char name[32];
int i, ret;
@@ -94,10 +96,14 @@ static int i2c_multi_inst_probe(struct platform_device *pdev)
inst_data[i].type, i);
board_info.dev_name = name;
board_info.fwnode = dev->fwnode;
+ irq_optional = false;
switch (inst_data[i].flags & IRQ_RESOURCE_TYPE) {
+ case IRQ_RESOURCE_GPIO_OPTIONAL:
+ irq_optional = true;
+ fallthrough;
case IRQ_RESOURCE_GPIO:
ret = acpi_dev_gpio_irq_get(adev, inst_data[i].irq_idx);
- if (ret < 0) {
+ if (ret < 0 && (!irq_optional || ret != -ENOENT)) {
dev_err(dev, "Error requesting irq at index %d: %d\n",
inst_data[i].irq_idx, ret);
goto error;
--
2.28.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [RFC 4/4] platform/x86: i2c-multi-instantiate: Add IRQ_RESOURCE_GPIO_OPTIONAL IRQ type
2020-11-05 8:00 ` [RFC 4/4] platform/x86: i2c-multi-instantiate: Add IRQ_RESOURCE_GPIO_OPTIONAL IRQ type Hans de Goede
@ 2020-11-05 10:36 ` Andy Shevchenko
0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2020-11-05 10:36 UTC (permalink / raw)
To: Hans de Goede
Cc: Mark Gross, Wolfram Sang, Andy Shevchenko, Platform Driver,
linux-i2c, ACPI Devel Maling List
On Thu, Nov 5, 2020 at 10:00 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Most I2C-drivers treat IRQs as optional and in some cases ACPI
> devices managed by i2c-multi-instantiate.c may have a GpioInt resource
> specified on some systems, while it is not there on others.
GpioInt()
> Add a new IRQ_RESOURCE_GPIO_OPTIONAL IRQ type, which still tries to get
> a GpioInt IRQ, but does not consider it a fatal error when this fails.
GpioInt()
...
> -#define IRQ_RESOURCE_TYPE GENMASK(1, 0)
> -#define IRQ_RESOURCE_NONE 0
> -#define IRQ_RESOURCE_GPIO 1
> -#define IRQ_RESOURCE_APIC 2
> +#define IRQ_RESOURCE_TYPE GENMASK(1, 0)
> +#define IRQ_RESOURCE_NONE 0
> +#define IRQ_RESOURCE_GPIO 1
> +#define IRQ_RESOURCE_GPIO_OPTIONAL 2
> +#define IRQ_RESOURCE_APIC 3
I think flag is cleaner:
#define IRQ_RESOURCE_OPTIONAL BIT(2)
...
> case IRQ_RESOURCE_GPIO:
> ret = acpi_dev_gpio_irq_get(adev, inst_data[i].irq_idx);
> - if (ret < 0) {
> + if (ret < 0 && (!irq_optional || ret != -ENOENT)) {
ret < 0 && !((inst_data[i].flags & IRQ_RESOURCE_OPTIONAL) && ret == -ENOENT)
> dev_err(dev, "Error requesting irq at index %d: %d\n",
> inst_data[i].irq_idx, ret);
> goto error;
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 0/4] platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated i2c-clients
2020-11-05 8:00 [RFC 0/4] platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated i2c-clients Hans de Goede
` (3 preceding siblings ...)
2020-11-05 8:00 ` [RFC 4/4] platform/x86: i2c-multi-instantiate: Add IRQ_RESOURCE_GPIO_OPTIONAL IRQ type Hans de Goede
@ 2020-11-05 10:38 ` Andy Shevchenko
2020-11-09 11:33 ` Hans de Goede
4 siblings, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2020-11-05 10:38 UTC (permalink / raw)
To: Hans de Goede, Krogerus, Heikki
Cc: Mark Gross, Wolfram Sang, Andy Shevchenko, Platform Driver,
linux-i2c, ACPI Devel Maling List
On Thu, Nov 5, 2020 at 10:00 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi All,
>
> As the subject says this series is mostly about passing the ACPI fwnode to
> i2c-clients instantiated by the i2c-multi-instantiate code.
>
> As discussed here:
> https://bugzilla.kernel.org/show_bug.cgi?id=198671
>
> BOSC0200 ACPI devices may sometimes describe 2 accelerometers in a single
> ACPI device, while working on this I noticed that BOSC0200 ACPI nodes
> contain ACCEL_MOUNT_MATRIX info (unlike all the other ACPI ids for bmc150
> accelerometers). Which is why I wanted to pass the fwnode so that we
> could use this info in the bmc150-accel driver.
>
> The plan was to use i2c-multi-instantiate for this, but doing so will
> change the modalias and /lib/udev/hwdb.d/60-sensor.hwdb matches on
> the modalias for various quirks setting ACCEL_MOUNT_MATRIX. So then the
> plan became to first add support for the mount-matrix provided inside
> the BOSC0200 ACPI node, making the udev info unnecessary. But for at
> least 1 model (and probably more) the BOSC0200 ACPI node and hwdb info
> does not match and since the hwdb info is added by users of the actual
> devices we can assume it is correct, so it seems that we cannot always
> trust the ACPI provided info. This is ok, the hwdb info overrides it
> (iio-sensor-proxy prefers the udev provided mount-matrix over the
> one provided by the driver) but this means that we MUST keep the
> existing hwdb matches working, which means that we cannot use
> i2c-multi-instantiate for this.
>
> Instead I will dust of an old patch for this from Jeremy Cline:
> https://patchwork.kernel.org/project/linux-iio/patch/010001602cf53153-39ad69f1-1b39-4e6d-a748-9455a16c2fbd-000000@email.amazonses.com/
>
> Which deals with there being 2 accelerometers inside the bmc150-accel
> driver.
>
> But before coming to the conclusion that i2c-multi-instantiate
> would not work I had already written this series. Since this might
> be useful for some other case in the future I'm sending this out
> as a RFC now, mostly so that it gets added to the archives.
I think they are in pretty good shape (only the 4th required a bit of
attention).
Please, send as non-RFC and also Cc Heikki (just in case if he has
comments wrt INT3515).
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC 0/4] platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated i2c-clients
2020-11-05 10:38 ` [RFC 0/4] platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated i2c-clients Andy Shevchenko
@ 2020-11-09 11:33 ` Hans de Goede
2020-11-10 10:10 ` Andy Shevchenko
0 siblings, 1 reply; 14+ messages in thread
From: Hans de Goede @ 2020-11-09 11:33 UTC (permalink / raw)
To: Andy Shevchenko, Krogerus, Heikki
Cc: Mark Gross, Wolfram Sang, Andy Shevchenko, Platform Driver,
linux-i2c, ACPI Devel Maling List
Hi,
On 11/5/20 11:38 AM, Andy Shevchenko wrote:
> On Thu, Nov 5, 2020 at 10:00 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi All,
>>
>> As the subject says this series is mostly about passing the ACPI fwnode to
>> i2c-clients instantiated by the i2c-multi-instantiate code.
>>
>> As discussed here:
>> https://bugzilla.kernel.org/show_bug.cgi?id=198671
>>
>> BOSC0200 ACPI devices may sometimes describe 2 accelerometers in a single
>> ACPI device, while working on this I noticed that BOSC0200 ACPI nodes
>> contain ACCEL_MOUNT_MATRIX info (unlike all the other ACPI ids for bmc150
>> accelerometers). Which is why I wanted to pass the fwnode so that we
>> could use this info in the bmc150-accel driver.
>>
>> The plan was to use i2c-multi-instantiate for this, but doing so will
>> change the modalias and /lib/udev/hwdb.d/60-sensor.hwdb matches on
>> the modalias for various quirks setting ACCEL_MOUNT_MATRIX. So then the
>> plan became to first add support for the mount-matrix provided inside
>> the BOSC0200 ACPI node, making the udev info unnecessary. But for at
>> least 1 model (and probably more) the BOSC0200 ACPI node and hwdb info
>> does not match and since the hwdb info is added by users of the actual
>> devices we can assume it is correct, so it seems that we cannot always
>> trust the ACPI provided info. This is ok, the hwdb info overrides it
>> (iio-sensor-proxy prefers the udev provided mount-matrix over the
>> one provided by the driver) but this means that we MUST keep the
>> existing hwdb matches working, which means that we cannot use
>> i2c-multi-instantiate for this.
>>
>> Instead I will dust of an old patch for this from Jeremy Cline:
>> https://patchwork.kernel.org/project/linux-iio/patch/010001602cf53153-39ad69f1-1b39-4e6d-a748-9455a16c2fbd-000000@email.amazonses.com/
>>
>> Which deals with there being 2 accelerometers inside the bmc150-accel
>> driver.
>>
>> But before coming to the conclusion that i2c-multi-instantiate
>> would not work I had already written this series. Since this might
>> be useful for some other case in the future I'm sending this out
>> as a RFC now, mostly so that it gets added to the archives.
>
> I think they are in pretty good shape (only the 4th required a bit of
> attention).
FWIW I agree with the changes which you suggest for the 4th patch.
> Please, send as non-RFC and also Cc Heikki (just in case if he has
> comments wrt INT3515).
But do we really want to land these changes, while ATM we do not
really have any need for them ? Esp. the
"platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated I2C-clients"
Change is not without a chance of regressions. The acpi_device_is_first_physical_node()
behavior surprised me a bit while working on the BOSC0200 changes. So I'm not
100% sure I have managed to see / think of all implications of this change.
Heikki do you now (or in the near future) need access to the fwnode for
the TypeC controllers handled by the i2c-multi-instantiate code ?
Note that if we do decide to move forward with this set, it should probably
be merged in its entirety by Wolfram as it also makes i2c-core changes
(or Wolfram could just merge the i2c-core change and provide an immutable
branch for me to merge into pdx86/for-next.
And then your (Andy's) cleanup series can be applied on top of this once merged.
Regards,
Hans
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 0/4] platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated i2c-clients
2020-11-09 11:33 ` Hans de Goede
@ 2020-11-10 10:10 ` Andy Shevchenko
2020-11-10 11:14 ` Hans de Goede
0 siblings, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2020-11-10 10:10 UTC (permalink / raw)
To: Hans de Goede
Cc: Krogerus, Heikki, Mark Gross, Wolfram Sang, Andy Shevchenko,
Platform Driver, linux-i2c, ACPI Devel Maling List
On Mon, Nov 9, 2020 at 1:33 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 11/5/20 11:38 AM, Andy Shevchenko wrote:
> > On Thu, Nov 5, 2020 at 10:00 AM Hans de Goede <hdegoede@redhat.com> wrote:
...
> >> But before coming to the conclusion that i2c-multi-instantiate
> >> would not work I had already written this series. Since this might
> >> be useful for some other case in the future I'm sending this out
> >> as a RFC now, mostly so that it gets added to the archives.
> >
> > I think they are in pretty good shape (only the 4th required a bit of
> > attention).
>
> FWIW I agree with the changes which you suggest for the 4th patch.
>
> > Please, send as non-RFC and also Cc Heikki (just in case if he has
> > comments wrt INT3515).
>
> But do we really want to land these changes, while ATM we do not
> really have any need for them ? Esp. the
>
> "platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated I2C-clients"
>
> Change is not without a chance of regressions. The acpi_device_is_first_physical_node()
> behavior surprised me a bit while working on the BOSC0200 changes. So I'm not
> 100% sure I have managed to see / think of all implications of this change.
I think in general the direction to switch to fwnode is a good one. I
was thinking about moving i2c core to use swnodes in which case they
will utilize fwnode pointer. But it might have complications, you are
right.
> Heikki do you now (or in the near future) need access to the fwnode for
> the TypeC controllers handled by the i2c-multi-instantiate code ?
>
> Note that if we do decide to move forward with this set, it should probably
> be merged in its entirety by Wolfram as it also makes i2c-core changes
> (or Wolfram could just merge the i2c-core change and provide an immutable
> branch for me to merge into pdx86/for-next.
>
> And then your (Andy's) cleanup series can be applied on top of this once merged.
Fine to me.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 0/4] platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated i2c-clients
2020-11-10 10:10 ` Andy Shevchenko
@ 2020-11-10 11:14 ` Hans de Goede
2020-11-10 14:47 ` Andy Shevchenko
0 siblings, 1 reply; 14+ messages in thread
From: Hans de Goede @ 2020-11-10 11:14 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Krogerus, Heikki, Mark Gross, Wolfram Sang, Andy Shevchenko,
Platform Driver, linux-i2c, ACPI Devel Maling List
Hi,
On 11/10/20 11:10 AM, Andy Shevchenko wrote:
> On Mon, Nov 9, 2020 at 1:33 PM Hans de Goede <hdegoede@redhat.com> wrote:
>> On 11/5/20 11:38 AM, Andy Shevchenko wrote:
>>> On Thu, Nov 5, 2020 at 10:00 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> ...
>
>>>> But before coming to the conclusion that i2c-multi-instantiate
>>>> would not work I had already written this series. Since this might
>>>> be useful for some other case in the future I'm sending this out
>>>> as a RFC now, mostly so that it gets added to the archives.
>>>
>>> I think they are in pretty good shape (only the 4th required a bit of
>>> attention).
>>
>> FWIW I agree with the changes which you suggest for the 4th patch.
>>
>>> Please, send as non-RFC and also Cc Heikki (just in case if he has
>>> comments wrt INT3515).
>>
>> But do we really want to land these changes, while ATM we do not
>> really have any need for them ? Esp. the
>>
>> "platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated I2C-clients"
>>
>> Change is not without a chance of regressions. The acpi_device_is_first_physical_node()
>> behavior surprised me a bit while working on the BOSC0200 changes. So I'm not
>> 100% sure I have managed to see / think of all implications of this change.
>
> I think in general the direction to switch to fwnode is a good one. I
> was thinking about moving i2c core to use swnodes in which case they
> will utilize fwnode pointer. But it might have complications, you are
> right.
So do you agree to just keep this series in the archives (in case we need
it later) for now ? Or would you still like me to post a non RFC version ?
Regards,
Hans
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 0/4] platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated i2c-clients
2020-11-10 11:14 ` Hans de Goede
@ 2020-11-10 14:47 ` Andy Shevchenko
2020-11-24 10:35 ` Hans de Goede
0 siblings, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2020-11-10 14:47 UTC (permalink / raw)
To: Hans de Goede
Cc: Krogerus, Heikki, Mark Gross, Wolfram Sang, Andy Shevchenko,
Platform Driver, linux-i2c, ACPI Devel Maling List
On Tue, Nov 10, 2020 at 1:14 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 11/10/20 11:10 AM, Andy Shevchenko wrote:
> > On Mon, Nov 9, 2020 at 1:33 PM Hans de Goede <hdegoede@redhat.com> wrote:
...
> > I think in general the direction to switch to fwnode is a good one. I
> > was thinking about moving i2c core to use swnodes in which case they
> > will utilize fwnode pointer. But it might have complications, you are
> > right.
>
> So do you agree to just keep this series in the archives (in case we need
> it later) for now ? Or would you still like me to post a non RFC version ?
If nobody else has a different opinion (Heikki, Wolfram, Rafael?), I'm
fine with it to be in archives for the time being.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 0/4] platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated i2c-clients
2020-11-10 14:47 ` Andy Shevchenko
@ 2020-11-24 10:35 ` Hans de Goede
2020-11-24 11:35 ` Andy Shevchenko
0 siblings, 1 reply; 14+ messages in thread
From: Hans de Goede @ 2020-11-24 10:35 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Krogerus, Heikki, Mark Gross, Wolfram Sang, Andy Shevchenko,
Platform Driver, linux-i2c, ACPI Devel Maling List
Hi,
On 11/10/20 3:47 PM, Andy Shevchenko wrote:
> On Tue, Nov 10, 2020 at 1:14 PM Hans de Goede <hdegoede@redhat.com> wrote:
>> On 11/10/20 11:10 AM, Andy Shevchenko wrote:
>>> On Mon, Nov 9, 2020 at 1:33 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> ...
>
>>> I think in general the direction to switch to fwnode is a good one. I
>>> was thinking about moving i2c core to use swnodes in which case they
>>> will utilize fwnode pointer. But it might have complications, you are
>>> right.
>>
>> So do you agree to just keep this series in the archives (in case we need
>> it later) for now ? Or would you still like me to post a non RFC version ?
>
> If nobody else has a different opinion (Heikki, Wolfram, Rafael?), I'm
> fine with it to be in archives for the time being.
Since no-one else has responded, lets just keep this series for the
archives.
Andy, that also means that there no longer is a reason to hold of merging
your i2c-multi-instantiate cleanup series (minus patch 3 as discussed),
so I've merged that into my review-hans branch now.
Regards,
Hans
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 0/4] platform/x86: i2c-multi-instantiate: Pass ACPI fwnode to instantiated i2c-clients
2020-11-24 10:35 ` Hans de Goede
@ 2020-11-24 11:35 ` Andy Shevchenko
0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2020-11-24 11:35 UTC (permalink / raw)
To: Hans de Goede
Cc: Krogerus, Heikki, Mark Gross, Wolfram Sang, Andy Shevchenko,
Platform Driver, linux-i2c, ACPI Devel Maling List
On Tue, Nov 24, 2020 at 12:35 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 11/10/20 3:47 PM, Andy Shevchenko wrote:
> > On Tue, Nov 10, 2020 at 1:14 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >> On 11/10/20 11:10 AM, Andy Shevchenko wrote:
> >>> On Mon, Nov 9, 2020 at 1:33 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >
> > ...
> >
> >>> I think in general the direction to switch to fwnode is a good one. I
> >>> was thinking about moving i2c core to use swnodes in which case they
> >>> will utilize fwnode pointer. But it might have complications, you are
> >>> right.
> >>
> >> So do you agree to just keep this series in the archives (in case we need
> >> it later) for now ? Or would you still like me to post a non RFC version ?
> >
> > If nobody else has a different opinion (Heikki, Wolfram, Rafael?), I'm
> > fine with it to be in archives for the time being.
>
> Since no-one else has responded, lets just keep this series for the
> archives.
>
> Andy, that also means that there no longer is a reason to hold of merging
> your i2c-multi-instantiate cleanup series (minus patch 3 as discussed),
> so I've merged that into my review-hans branch now.
Cool, thanks!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread