Re: [PATCH 1/2] platform/x86: i2c-multi-instantiate: Add flag for passing fwnode

[Andy Shevchenko]

On Mon, Apr 27, 2020 at 6:06 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> On 4/27/20 3:18 PM, Andy Shevchenko wrote:
> > On Mon, Apr 27, 2020 at 3:51 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> >> On 4/26/20 7:59 PM, Andy Shevchenko wrote:
> >>> On Sun, Apr 26, 2020 at 1:47 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:

> >>>> In some cases the driver for the i2c_client-s which i2c-multi-instantiate
> >>>> instantiates may need access some fields / methods from to the ACPI fwnode
> >>>> for which i2c_clients are being instantiated.
> >>>>
> >>>> An example of this are CPLM3218 ACPI device-s. These contain CPM0 and
> >>>> CPM1 packages with various information (e.g. register init values) which
> >>>> the driver needs.
> >>>>
> >>>> Passing the fwnode through the i2c_board_info struct also gives the
> >>>> i2c-core access to it, and if we do not pass an IRQ then the i2c-core
> >>>> will use the fwnode to get an IRQ, see i2c_acpi_get_irq().
> >>>
> >>> I'm wondering, can we rather do it in the same way like we do for
> >>> GPIO/APIC case here.
> >>> Introduce IRQ_RESOURCE_SHARED (or so) and
> >>>
> >>> case _SHARED:
> >>>    irq = i2c_acpi_get_irq();
> >>> ...
> >>>
> >>> ?
> >>
> >> I think you are miss-understanding the problem. The problem is not that
> >> we want to share the IRQ, the problem is that we want to pass the single
> >> IRQ in the resources to only 1 of the instantiated I2C-clients. But if we
> >> do not pass an IRQ (we leave it at 0) and we do pass the fwnode then
> >> i2c-core-base.c will see that there is an ACPI-node attached to the
> >> device and will call i2c_acpi_get_irq().
> >
> > Do we know ahead which device should take IRQ resource and which should not?
> > Can we use current _NONE flag for them?
>
> The problem is not internal to i2c-multi-instantiate.c, the problem
> (once we pass a fwnode) is the API between i2c-multi-instantiate.c and
> the i2c-core. For the IRQ_RESOURCE_NONE case i2c-multi-instantiate.c
> sets board_info.irq to 0, which is the correct way to specify that
> we do not have an IRQ, but if don't pass an IRQ then the i2c-core
> will try to find one itself.  And once we pass the fwnode, then
> the "try to find one itself" code will call i2c_acpi_get_irq()
> and find the same IRQ for clients we instantiate, leading to
> the earlier mentioned IRQ conflict.

I'm missing something here. Why we need to pass an fwnode in the first place?
Seems you would like to access to methods from the driver.
But if you simple enumerate the driver in ACPI multi-instantiate won't
be needed.

As far as I understand, the actual driver consumes *both* IÂC
resources. It's not a multi-instantiate in this case.


-- 
With Best Regards,
Andy Shevchenko