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

Groups > linux.kernel > #1550729

Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on Dell machines

From Benjamin Tissoires <benjamin.tissoires@redhat.com>
Newsgroups linux.kernel
Subject Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on Dell machines
Date 2017-01-04 13:40 +0100
Message-ID <sVT1f-4FT-21@gated-at.bofh.it> (permalink)
References (2 earlier) <sVE2e-37A-9@gated-at.bofh.it> <sVEbU-3aT-13@gated-at.bofh.it> <sVQwp-2U6-17@gated-at.bofh.it> <sVQZr-3pc-19@gated-at.bofh.it> <sVS5b-44p-29@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw

On Jan 04 2017 or thereabouts, Pali Rohár wrote:
> On Wednesday 04 January 2017 11:25:44 Benjamin Tissoires wrote:
> > On Jan 04 2017 or thereabouts, Andy Shevchenko wrote:
> > > On Tue, Jan 3, 2017 at 10:39 PM, Pali Rohár <pali.rohar@gmail.com> wrote:
> > > >
> > > > With Michał we already discussed about it, see emails. Basically you can
> > > > enable/disable kernel modules at compile time or blacklist at runtime
> > > > (or even chose what will be compiled into vmlinux and what as external
> > > > .ko module). Some distributions blacklist i2c-i801.ko module...
> > > 
> > > But you understand that any of compile/not compile is not an option, right?
> > > The case which we face will be both of them, if possible, will be
> > > compiled as modules.
> > > 
> > > Blacklisting means making your problem the actual user's one. Not good.
> > > 
> > > > And
> > > > there can be also problem with initialization of i2c-i801 driver (fix is
> > > > in commit a7ae81952cda, but does not have to work at every time!). So
> > > > that move on whitelisted machines can potentially cause disappearance of
> > > > /dev/freefall and users will not have hdd protection which is currently
> > > > working.
> > > 
> > 
> > I am seeing the same issues with psmouse and SMBus touchpads. The PS/2
> > device knows about the availability of a better but unlisted device at
> > the ACPI level.
> > 
> > The way I solved this to not have to deal with compile/not compile and
> > runtime errors is the same way Wolfram told you about: bus notifiers.
> > I also use an intermediate platform driver to not add i2c dependency on
> > psmouse.
> > 
> > For you the solution would be:
> > - In dell-smo8800, after checking the whitelist, add a platform driver
> >   "dell-lis3lv02d-platform", and add in the platform_data the I2C address
> >   of the chip.
> > - create a new driver dell-lis3lv02d-platform.ko which listens for the
> >   i2c bus creation and registers the lis3lv02d I2C node when it sees a
> >   matching adapter. (see [1] for my solution)
> > - in dell-lis3lv02d-platform.ko make sure to set the irq to -ENOENT so
> >   that lis3lv02d.ko doesn't create /dev/freefall which will still be
> >   handled by ACPI.
> > 
> > How does that sound?
> 
> Yes, something like this was already suggested. But it is more
> complicated as my approach and less error prone... See my notes in
> previous emails.

Sorry but I can't find your notes about errors in your previous emails.
This solution indeed is a little bit more complex than just adding the
bits in i2c-i801, but I don't really see the reason for an *adapter*
driver to instantiate specific *clients* depending on some DMI
matching.

It's a platform issue, so it should be solved at a platform level, not
at the i2c adapter level.

Plus the bus notifier solutions guarantees plain separation between the
elements and prevents any conflicts, with or without compile guards.

> 
> My current path (after fixing IRQ to -1) is smaller more intuitive and

This will only fix the fact that you have 2 concurrent drivers on the
same resource (freefall), not the fact that the global design of having
2 drivers which do not cooperate is wrong.

> do not introduce new complicated parts like bus notifier and new "fake"
> i2c driver...

It's not a "fake i2c driver". It's a platform driver. If you don't like
the idea of the platform driver, just add the bus notifier code in
dell-smo8800, but this will pull a new dependency on I2C in this ACPI
driver.

Cheers,
Benjamin

> > [1] https://github.com/bentiss/linux/blob/synaptics-rmi4-v4.9-rc7+/drivers/input/rmi4/rmi_platform.c 
> 
> -- 
> Pali Rohár
> pali.rohar@gmail.com

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

Thread

Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-01-03 19:40 +0100
  Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on Dell machines Pali Rohár <pali.rohar@gmail.com> - 2017-01-03 20:00 +0100
    RE: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on Dell  machines <Mario.Limonciello@dell.com> - 2017-01-03 20:10 +0100
    Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-01-03 20:50 +0100
      Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on Dell machines Pali Rohár <pali.rohar@gmail.com> - 2017-01-03 21:10 +0100
        Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-01-03 21:40 +0100
          Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on Dell machines Pali Rohár <pali.rohar@gmail.com> - 2017-01-03 21:50 +0100
            Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-01-03 22:10 +0100
              Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Pali Rohár <pali.rohar@gmail.com> - 2017-01-04 09:20 +0100
                Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-01-04 10:10 +0100
                Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Pali Rohár <pali.rohar@gmail.com> - 2017-01-04 10:20 +0100
                Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-01-04 11:20 +0100
                Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Pali Rohár <pali.rohar@gmail.com> - 2017-01-04 11:30 +0100
                Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-01-04 11:40 +0100
                Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Pali Rohár <pali.rohar@gmail.com> - 2017-01-04 12:30 +0100
                Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Pali Rohár <pali.rohar@gmail.com> - 2017-01-04 13:10 +0100
                Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-01-04 14:10 +0100
                Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Pali Rohár <pali.rohar@gmail.com> - 2017-01-04 17:10 +0100
                Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-01-04 18:40 +0100
                Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Wolfram Sang <wsa@the-dreams.de> - 2017-01-04 18:50 +0100
                Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-01-04 19:00 +0100
                Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-01-04 23:00 +0100
                Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-01-04 23:10 +0100
                Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-01-04 23:10 +0100
                Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Wolfram Sang <wsa@the-dreams.de> - 2017-01-04 23:10 +0100
                Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-01-04 23:10 +0100
                Re: [PATCH] i2c: do not enable fall back to Host Notify by default kbuild test robot <lkp@intel.com> - 2017-01-05 03:30 +0100
                Re: [PATCH] i2c: do not enable fall back to Host Notify by default kbuild test robot <lkp@intel.com> - 2017-01-05 03:30 +0100
                Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Pali Rohár <pali.rohar@gmail.com> - 2017-01-05 10:00 +0100
                Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-01-05 10:30 +0100
              Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Jean Delvare <jdelvare@suse.de> - 2017-01-04 20:10 +0100
            Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on Dell machines Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-01-04 11:00 +0100
              Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-01-04 11:30 +0100
                Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Pali Rohár <pali.rohar@gmail.com> - 2017-01-04 12:40 +0100
                Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-01-04 13:40 +0100
      Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-01-03 22:40 +0100
        Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-01-04 07:40 +0100
          Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Pali Rohár <pali.rohar@gmail.com> - 2017-01-04 10:30 +0100
          Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on  Dell machines Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-01-04 10:30 +0100
    Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on Dell machines Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-01-03 21:30 +0100

csiph-web