Re: [PATCH v3 3/4] i2c: core: Allow drivers to specify index for irq to get from of / ACPI

[Wolfram Sang <wsa@the-dreams.de>]

[-- Attachment #1: Type: text/plain, Size: 2437 bytes --]

Hi Hans,

> The problem here is that the i2c system is somewhat special in that
> it does irq mapping on behalf of the driver and does not even bother
> to call the driver's probe() if the acpi_dev_gpio_irq_get() call
> returns -EPROBE_DEFER.

Yes, the client->irq handling is cruft, I am fully aware of that.

> >Or maybe you simply want to be early and don't want to get deferred? Are
> >we talking then about boot optimizations or necessities?
> 
> The problem which I'm trying to fix is not having to write a (complex)
> gpio driver for an undocumented PMIC which I (and AFAICT no-one) needs (*)
> just because the i2c-core needs to be "special" and do the acpi_dev_gpio_irq_get
> for me even though in this specific driver I don't need the irq at index
> 0 at all. IOW the problem which I'm trying to fix is to get i2c_device_probe
> to not out-smart me and never call my driver's probe method for
> invalid reasons.

So, it is a necessity because you have an ACPI entry to a PMIC which is
totally undocumented. Point taken.

> 
> My previous patch for this added an irq_index member to i2c_driver instead,
> because my use-case does actually need an irq, but the one with
> resource-index 1, not the useless one at index 0. Which is yet another
> indication that the naive irq-handling in the i2c-core sometimes get
> somewhat in the way. In my experience many more complex i2c devices have
> more then 1 irq line.

I am aware of this problem as well.

> That previous patch works for me too (and even simplifies my driver somewhat),
> if you like it better I can go back to that. But thinking more about this
> I decided that having a "dear i2c-core please don't try to out-smart the
> driver, leave irq handling to me" flag would be better / more generally
> useful.

I agree. The last sentence made me understand that you want to flag
"this driver wants custom irq handling" more than "this driver does not
use irq" what I misinterpreted before. This is a much broader use case
and we can probably help more people with that. I like it. Maybe we
should name the flag something like "custom_irq_handling" to prevent
similar misunderstandings? Just an idea.

Sorry if you got annoyed by my questions, yet with the prospect of
not only adding code to the core but also adding something to
i2c_driver, I really want to make sure it is worth it.

Have a nice weekend,

   Wolfram

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]