On Nov 07 2016 or thereabouts, Wolfram Sang wrote: > On Thu, Oct 13, 2016 at 02:10:40PM +0200, Benjamin Tissoires wrote: > > The current SMBus Host Notify implementation relies on .alert() to > > relay its notifications. However, the use cases where SMBus Host > > Notify is needed currently is to signal data ready on touchpads. > > > > This is closer to an IRQ than a custom API through .alert(). > > Given that the 2 touchpad manufacturers (Synaptics and Elan) that > > use SMBus Host Notify don't put any data in the SMBus payload, the > > concept actually matches one to one. > > I see the advantages. The only question I have: What if we encounter > devices in the future which do put data in the payload? Can this > mechanism be extended to handle that? I had this very same problem when dealing with hid-rmi. In that case, we have an interrupt that contains data. I somewhat solved this by the following commit transposed in the hid-rmi world: https://github.com/bentiss/linux/commit/d24d938e1eabba026c3c5e4daeee3a16403295de The idea is the following: - we extend the internal call of i2c_handle_smbus_host_notify() by re-adding a data payload. - this call takes a spinlock, and copy the data into an internal placeholder, releases the spinlock and then triggers the irq - when the client gets an interrupt, it calls i2c_retrieve_host_notify_data() which is protected by the spinlock and returns the *current* data The only difficulty for the client now is that it might access newer data than the origin interrupt. This can be solved by 2 solutions. Either we use a kfifo in i2c_core, to the risk of having some complex processing. Either we just let the client dealing with that by implementing itself the kfifo. The data just needs to be retrieved in the non threaded part of the interrupt handler, and stored by the client itself. So all in all, I think adding the data will be just adding a spinlock, a placeholder and a new internal API to retrieve it. I am not sure we have other means of extending the IRQ processing, but I am open to suggestions. Cheers, Benjamin > > > > > Benefits are multiple: > > - simpler code and API: the client will just have an IRQ, and > > nothing needs to be added in the adapter beside internally > > enabling it. > > - no more specific workqueue, the threading is handled by IRQ core > > directly (when required) > > - no more races when removing the device (the drivers are already > > required to disable irq on remove) > > - simpler handling for drivers: use plain regular IRQs > > - no more dependency on i2c-smbus for i2c-i801 (and any other adapter) > > - the IRQ domain is created automatically when the adapter exports > > the Host Notify capability > > - the IRQ are assign only if ACPI, OF and the caller did not assign > > one already > > - the domain is automatically destroyed on remove > > - fewer lines of code (minus 20, yeah!) > > > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx> > > Thanks for keeping at it! > -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
