linux-kernel - [PATCH v5 0/2] iio: light: Support AMS AS73211 digital XYZ sensor

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20200802163735.76617-1-ceggers@arri.de>
Date:   Sun, 2 Aug 2020 18:37:33 +0200
From:   Christian Eggers <ceggers@...i.de>
To:     Rob Herring <robh+dt@...nel.org>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        Jonathan Cameron <jic23@...nel.org>
CC:     Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        <linux-iio@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        "Christian Eggers" <ceggers@...i.de>
Subject: [PATCH v5 0/2] iio: light: Support AMS AS73211 digital XYZ sensor

This series adds support for the AMS AS73211 digital XYZ sensor. Some
comments to the review emails below...

Again, many thanks for the comprehensive reviews.

Changes in v5:
---------------
- [1/2] Reviewed by Rob Herring
- [2/2] Added KHZ_PER_HR define
- [2/2] Added AS73211_SAMPLE_FREQ_BASE define
- [2/2] Slight changes in comments
- [2/2] Claim direct mode in write_raw()
- [2/2] Saturate only in case of overflow
- [2/2] Don't set indio_dev->dev.parent
- [2/2] Fix error path (order) in probe()

Changes in v4:
---------------
- Integrated 2nd review from Andy Shevchenko
- Use more devm_ functions in as73211_probe()

Changes in v3:
---------------
- Integrated comments from Andy Shevchenko
- Integrated comments from Jonathan Cameron

Changes in v2:
---------------
- Fix $id in dt binding
- Document full I2C address range in "reg" property
- Move "buffer" member out of "struct as73211_data"
- Fix sparse warnings by using correct data types


On Friday, 31 July 2020, 17:41:47 CEST, Andy Shevchenko wrote:
> On Fri, Jul 31, 2020 at 1:52 PM Christian Eggers <ceggers@...i.de> wrote:
> > devm_iio_device_alloc() doesn't pass 'dev' to iio_device_alloc().
> > I already looked around, but I didn't find. And after debugging
> > v5.4, devm_iio_device_alloc() definitely doesn't do it.
> 
> Why are you talking about v5.4? We are in v5.8 cycle contributing to v5.9.
v5.4-rt is the version for my product release. Current base for these
patches is 5.8-rc6


On Friday, 31 July 2020, 18:19:55 CEST, Jonathan Cameron wrote:
> > On Fri, Jul 31, 2020 at 1:52 PM Christian Eggers <ceggers@...i.de> wrote:
> > Recently IIO gained some features among which I think the one that
> > assigns parent devices.
> 
> yup. This should be in linux-next now for the coming merge window (which
> probably opens on Sunday).
Thanks for the hint. Is there a particular tree which is preferred for
IIO development?


On Saturday, 1 August 2020, 17:29:58 CEST, Jonathan Cameron wrote:
> On Fri, 31 Jul 2020 09:01:14 +0200 Christian Eggers <ceggers@...i.de> wrote:
> > +static int as73211_write_raw(struct iio_dev *indio_dev, struct
> > iio_chan_spec const *chan, +			      int val, int val2, long mask)
> > +{
> > +	...
> > +	/* Need to switch to config mode ... */
> 
> Is this safe whilst we are doing a capture?
> You may want to claim_direct_mode for write_raw to ensure we don't get such
> a race.
> 
That's an important point! As I use iio-trig-sysfs, I never had any
problems with this. But if for instance iio-trig-hrtimer is active, this
could become a problem.

In v5, I have claimed direct mode. For the application this means, that
buffer/enable has to be set to '0' before any settings can be
changed. As libiio doesn't support enabling/disabling a buffer directly,
the buffer needs to be destroyed and newly constructed.