Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1494460

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

Show all headers | View raw

On Sat, Oct 01, 2016 at 10:27:42AM -0700, Andrew Duggan wrote:
> 
> 
> On Fri, Sep 30, 2016, at 08:44 PM, Guenter Roeck wrote:
> > On 09/30/2016 04:02 PM, Dmitry Torokhov wrote:
> > > On Fri, Sep 30, 2016 at 03:54:03PM -0700, Dmitry Torokhov wrote:
> > >> On Thu, Sep 29, 2016 at 10:55:40AM -0700, Bjorn Andersson wrote:
> > >>> On Wed 28 Sep 17:37 PDT 2016, Guenter Roeck wrote:
> > >>>
> > >>>> 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.
> > >>>
> > >>>> Fixes: fdf51604f104 ("Input: synaptics-rmi4 - add I2C transport driver")
> > >>>> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> > >>>
> > >>> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > >>
> > >> 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 ...
> > 
> > > 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.

> 
> > 
> > rmi_register_transport_device() doesn't take dev as parameter.
> > 
> > > 2. If we need to use some non-devm-ised resources we should use
> > > devm_add_action[_or_reset] to work these operations into devm stream.
> > 
> 
> Since the regulator functions have their own devm_ versions I would
> suggest switching to those functions to avoid dealing with
> unregistering. 

There is no devm* API for enabling regulators, just acquiring them.

Thanks,

-- 
Dmitry

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH] Input: synaptics-rmi4 - Fix error handling in I2C transport driver Guenter Roeck <linux@roeck-us.net> - 2016-09-29 02:40 +0200
  Re: [PATCH] Input: synaptics-rmi4 - Fix error handling in I2C  transport driver Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-09-29 20:00 +0200
    Re: [PATCH] Input: synaptics-rmi4 - Fix error handling in I2C  transport driver Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2016-10-01 01:00 +0200
      Re: [PATCH] Input: synaptics-rmi4 - Fix error handling in I2C  transport driver Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2016-10-01 01:10 +0200
        Re: [PATCH] Input: synaptics-rmi4 - Fix error handling in I2C  transport driver Guenter Roeck <linux@roeck-us.net> - 2016-10-01 05:50 +0200
          Re: [PATCH] Input: synaptics-rmi4 - Fix error handling in I2C transport  driver Andrew Duggan <andrew@duggan.us> - 2016-10-01 19:40 +0200
            Re: [PATCH] Input: synaptics-rmi4 - Fix error handling in I2C  transport driver Guenter Roeck <linux@roeck-us.net> - 2016-10-01 19:50 +0200
              Re: [PATCH] Input: synaptics-rmi4 - Fix error handling in I2C transport  driver Andrew Duggan <andrew@duggan.us> - 2016-10-01 20:00 +0200
            Re: [PATCH] Input: synaptics-rmi4 - Fix error handling in I2C  transport driver Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2016-10-01 21:10 +0200
              Re: [PATCH] Input: synaptics-rmi4 - Fix error handling in I2C  transport driver Guenter Roeck <linux@roeck-us.net> - 2016-10-01 22:50 +0200
      Re: [PATCH] Input: synaptics-rmi4 - Fix error handling in I2C  transport driver Guenter Roeck <linux@roeck-us.net> - 2016-10-01 05:10 +0200

csiph-web