Re: [PATCH] Input: synaptics-rmi4 - Fix error handling in I2C transport driver

On 10/01/2016 12:04 PM, Dmitry Torokhov wrote:

On Sat, Oct 01, 2016 at 10:27:42AM -0700, Andrew Duggan wrote:

quoted

On Fri, Sep 30, 2016, at 08:44 PM, Guenter Roeck wrote:

quoted

On 09/30/2016 04:02 PM, Dmitry Torokhov wrote:

quoted

On Fri, Sep 30, 2016 at 03:54:03PM -0700, Dmitry Torokhov wrote:

quoted

On Thu, Sep 29, 2016 at 10:55:40AM -0700, Bjorn Andersson wrote:

quoted

On Wed 28 Sep 17:37 PDT 2016, Guenter Roeck wrote:

quoted

Instantiating the rmi4 I2C transport driver without interrupts assigned
(for example using manual i2c instantiation from the command line)
caused the driver to fail to load, but it does not clean up its
regulator or transport device registrations. Result is a crash at a later
time, for example when rebooting the system.

Fixes: 946c8432aab0 ("Input: synaptics-rmi4 - support regulator supplies")

Sorry for that.

quoted

Fixes: fdf51604f104 ("Input: synaptics-rmi4 - add I2C transport driver")
Cc: Bjorn Andersson <redacted>

Reviewed-by: Bjorn Andersson <redacted>

Applied, thank you.

I take it back. rmi_i2c_init_irq() uses devm* so this whole thing mixes
up devm* and manual unregistering and unwind order is completely
broken.

Oops ...

quoted

1. Why do we register interrupt from transport drivers and not make it
part of rmi_register_transport_device()?

Not all RMI devices will have access to interrupts (ie HID and SMBus).
The same goes for regulators. Here is a reference to a previous
discussion regarding both:
https://lkml.org/lkml/2016/5/9/1055

Yeah, I am wondering if we should not revisit this and have SMBus (and
possibly HID) actually provide us with an interrupt.

In the meantime we can just ignore interrupt value if it is set to 0.

I have another follow-up patch doing that (I have a test setup which
doesn't support interrupts); I just wasn't sure if there was interest,
so I did not yet send it out. I'll be happy to do that, though I would
prefer to keep it separate (it isn't a bug fix, after all).

Thanks,
Guenter