On Tue, Oct 11, 2016 at 04:20:52PM +0200, Benjamin Tissoires wrote:

On Oct 10 2016 or thereabouts, Dmitry Torokhov wrote:

quoted

Hi Benjamin,

On Mon, Oct 10, 2016 at 9:42 AM, Benjamin Tissoires
[off-list ref] wrote:

quoted

The Thinkpad series 13 uses Host Notify to report the interrupt.
Add elan_smb_alert() to handle those interrupts and disable the irq
handling on this case.

Signed-off-by: Benjamin Tissoires <redacted>

Why do we have to do this in the driver instead of having I2C core set
it up for us? I expect we'd be repeating this block of code for every
driver that has an option of using SMbus notify.

I didn't want to assign blindly an IRQ for Host Notify because it's a
device (as I2C client) feature. Not all SMBus clients on the Host Notify
capable bus are capable of Host Notify, so I thought it was the
responsibility of the driver to assign it.

Right, it is device's feature and it is up to the driver to decide if it
should be using interrupt-driven model or not. From I2C core I'd try the
following logic: if client->irq is not already set up and
ACPI/DT/platform data does not have any mention of IERQ and device is on
Host Notify capable bus then assign IRQ and let the dirver decide if it
wants to use it or not. Having client->irq present does not mean that we
have to use it. Driver knows the device best.

However, I can see your point, though I'd need some inputs (and I'll
have to resend the series as the Intel bot showed me 2 mistakes).

So, if i2c-core gets to assign itself the IRQ for Host Notify, do we:
1) assign an IRQ to any SMBus device on a Host Notify capable adapter
   that doesn't have a valid provided IRQ?
2) have a new field in struct i2c_board_info that explicitly requires
   Host Notify as an IRQ?
3) do not touch anything in i2c_core, let the caller of i2c_new_device
   fill in the irq by a call to 
   i2c_smbus_host_notify_to_irq(adapter, address)?

1) has the advantage of being transparent for everybody, except we will
provide IRQs to devices that don't have one (this can be ignored), but
this may also lead to errors when IRQ is not correctly set by the caller
where it should be, and the driver/developer doesn't understand why it
is never triggered.

2) is a nice compromise, but we will need some OF binding, add some work
in for the callers of i2c_new_device() and will not work nicely with
sysfs instantiated i2c devices (the new_device sysfs entry). The sysfs
could be solved by adding some new address namespace, but that doesn't
make sense I think

3) requires less changes in i2c_core but it's going to be even harder
to add a device through sysfs that is Host Notify capable given that we
can't specify the IRQ.

After thinking a bit, I think I'd lean toward 1), but I am open to other
options as well.

Yes, I think #1 is what we should do (but ultimately it is up to Wolfram
to decide).

Thanks.

-- 
Dmitry