linux-kernel - Re: [PATCH v4] iio : Add cm3218 smbus ara and acpi support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171120105756.GC22431@lahna.fi.intel.com>
Date:   Mon, 20 Nov 2017 12:57:56 +0200
From:   Mika Westerberg <mika.westerberg@...ux.intel.com>
To:     Jonathan Cameron <jic23@...nel.org>
Cc:     Wolfram Sang <wsa@...-dreams.de>,
        Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        Marc CAPDEVILLE <m.capdeville@...log.org>,
        Kevin Tsai <ktsai@...ellamicro.com>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        Benjamin Tissoires <benjamin.tissoires@...hat.com>,
        Jarkko Nikula <jarkko.nikula@...ux.intel.com>
Subject: Re: [PATCH v4] iio : Add cm3218 smbus ara and acpi support

+Jarkko

On Sun, Nov 19, 2017 at 04:35:51PM +0000, Jonathan Cameron wrote:
> On Thu, 2 Nov 2017 16:04:07 +0100
> Wolfram Sang <wsa@...-dreams.de> wrote:
> 
> > On Thu, Nov 02, 2017 at 02:35:50PM +0000, Jonathan Cameron wrote:
> > > On Fri, 27 Oct 2017 18:27:02 +0200
> > > Marc CAPDEVILLE <m.capdeville@...log.org> wrote:
> > >   
> > > > On asus T100, Capella cm3218 chip is implemented as ambiant light
> > > > sensor. This chip expose an smbus ARA protocol device on standard
> > > > address 0x0c. The chip is not functional before all alerts are
> > > > acknowledged.
> > > > On asus T100, this device is enumerated on ACPI bus and the
> > > > description give tow I2C connection. The first is the connection to
> > > > the ARA device and the second gives the real address of the device.
> > > > 
> > > > So, on device probe,If the i2c address is the ARA address and the
> > > > device is enumerated via acpi, we lookup for the real address in
> > > > the ACPI resource list and change it in the client structure.
> > > > if an interrupt resource is given, and only for cm3218 chip,
> > > > we declare an smbus_alert device.
> > > > 
> > > > Signed-off-by: Marc CAPDEVILLE <m.capdeville@...log.org>  
> > > 
> > > Wolfram - this needs input from you on how to neatly handle
> > > an ACPI registered ARA.  
> > 
> > ACPI is really not my field. Try asking the I2C ACPI maintainers or
> > Benjamin Tissoires <benjamin.tissoires@...hat.com> who did work on SMBus
> > interrupts recently.
> > 
> Hi Mika, Benjamin,
> 
> So we've lost most of the context in this thread, but the basic question
> is how to handle smbus ARA support with ACPI. 
> 
> https://patchwork.kernel.org/patch/10030309/
> 
> Has the proposal made in this driver which is really not terribly nice
> (as it registers the ARA device by messing with the address and registering
> a second device).
> 
> As I understood it the ARA device registration should be handled by the
> i2c master, but there are very few examples.
> 
> Phil pointed out that equivalent OF support recently got taken from him..
> https://www.spinics.net/lists/devicetree/msg191947.html
> https://www.spinics.net/lists/linux-i2c/msg31173.html
> 
> Any thoughts on the right way to do this?

There does not seem to be any way in ACPI to tell which "connection" is
used to describe ARA so that part currently is something each driver
needs to handle as they know the device the best. I don't think we have
any means to handle it in generic way in I2C core except to provide some
helpers that work on top of i2c_setup_smbus_alert() but understand ACPI
resources. Say provide function like this:

  int acpi_i2c_setup_smbus_alert(struct i2c_adapter *adapter, int index);

Which then extracts automatically I2cSerialBus connection from "index"
and calls i2c_setup_smbus_alert() accordingly.

In the long run we could introduce _DSD property that can be used to
name the connection in the same way DT does;

    Name (_CRS, ResourceTemplate () {
        I2cSerialBus () { ... } // ARA
        I2cSerialBus () { ... } // normal device address
    })

    Name (_DSD, Package () {
        ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
        Package () {
            Package () {"smbus_alert", 0} // Where 0 means the first I2cSerialBus
            ...
        }
    })

But it does not help the existing systems.