[PULL v2] First set of iio new device support etc for the 5.5 cycle

[PULL v2] First set of iio new device support etc for the 5.5 cycle

[Jonathan Cameron]

The following changes since commit b73b93a2af3392b9b7b8ba7e818ee767499f9655:

  iio: adc: ad7192: Add sysfs ABI documentation (2019-09-08 10:34:49 +0100)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git tags/iio-for-5.5a-take2

for you to fetch changes up to 4e716deff4c0b5bdb1bb7c41bdfe0fac2cf64c27:

  iio: imu: adis16400: fix compiler warnings (2019-10-12 12:11:37 +0100)

----------------------------------------------------------------
First set of IIO new device support, cleanups and features for the 5.5 cycle

Second version of pull has an additional patch to fix some build
warnings from the adis16400.

The usual mixed backs of new device support being added to drivers,
long term reworks continuing and little per driver cleanups and
features.

Also a few trivial counter subsystem tidy ups on behalf of William.

Core new feature
* Device label support.  A long requested feature no one got around to
  implementing before.  Allows DT based provision of a 'label' that
  identifies a device uniquely within a system.  This differs from existing
  'name' which is meant to be the part number.
New device support
* ingenic-adc
  - Support for the JZ4770 SoC ADC including bindings.
* inv_mpu6050
  - Add support for magnetometer in MPU925x parts.
    Fiddly to do as this is actually a separate device sitting inside the
    package, but with the master device being able to schedule reads etc.
    Will only run if the auxiliary bus is not in use for any other devices.

Features
* ad7192
  - Userspace calibration controls to do zero and full scale.
* st_lsm6dsx
  - Enable latched interrupts by default for sensors events with related clear.
  - Motion events and related wakeup source.  This needed quite a bit of
    refactoring as well.

Cleanups and minor features
* ad7192
  - sysfs ABI docs
* ad7949
  - Remove code to readback configuration word as driver never actually enabled
    it.
  - Fix incorrect xfer length.  Not actually known to cause problems other
    than wasted bus usage.
* adis library and drivers
  - Locking rework to simplify locking in general and avoid using the
    core mlock except for it's intending use to protect IIO state changes.
* adis16080
  - Replace core mlock usage with local lock with more appropriate scope.
* adis16130
  - Remove pointless mlock usage.
* adis16240
  - Remove include of gpio.h as no gpio usage.
* adis16400
  - Fix some uninitialized variable build warnings.
* atlas-ph-sensor
  - Improve logical ordering of buffer predisable / postenable functions.
    This is part of a longer term rework Alexandru is driving towards.
* bh1750
  - Fix up a static compiler warning and make the code more readable.
  - yaml conversion of binding + MAINTAINERS entry.
* bmp280
  - Drop a stray newline.
* cm36651
  - Drop a redundant assignment
* itg3200
  - Alignment cleanup.
* max31856
  - Add missing of_node and parent references, useful to identify the device.
* sc27xx_adc
  - Use devm_hwspin_lock_request_specific rather than local rolled version.
* stm32-lptimer counter
  - kernel-doc warning.
* stm32-timer counter
  - kernel-doc warning.
  - Alignment cleanup.
* sx9500
  - Improve logical ordering of buffer predisable / postenable functions.
    This is part of a longer term rework Alexandru is driving towards.
* tcs3414
  - Improve logical ordering of buffer predisable / postenable functions.
    This is part of a longer term rework Alexandru is driving towards.

----------------------------------------------------------------
Alexandru Ardelean (16):
      iio: tcs3414: fix iio_triggered_buffer_{pre,post}enable positions
      iio: gyro: adis16130: remove mlock usage
      iio: gyro: adis16080: replace mlock with own lock
      iio: proximity: sx9500: fix iio_triggered_buffer_{predisable,postenable} positions
      iio: imu: adis: rename txrx_lock -> state_lock
      iio: imu: adis: add unlocked read/write function versions
      iio: imu: adis[16480]: group RW into a single lock in adis_enable_irq()
      iio: imu: adis: create an unlocked version of adis_check_status()
      iio: imu: adis: create an unlocked version of adis_reset()
      iio: imu: adis: protect initial startup routine with state lock
      iio: imu: adis: group single conversion under a single state lock
      iio: imu: adis16400: rework locks using ADIS library's state lock
      iio: gyro: adis16136: rework locks using ADIS library's state lock
      iio: imu: adis16480: use state lock for filter freq set
      iio: chemical: atlas-ph-sensor: fix iio_triggered_buffer_predisable() position
      iio: imu: adis16400: fix compiler warnings

Andrea Merello (3):
      iio: ad7949: kill pointless "readback"-handling code
      iio: max31856: add missing of_node and parent references to iio_dev
      iio: ad7949: fix incorrect SPI xfer len

Artur Rojek (3):
      dt-bindings: iio/adc: Add a compatible string for JZ4770 SoC ADC
      dt-bindings: iio/adc: Add AUX2 channel idx for JZ4770 SoC ADC
      IIO: Ingenic JZ47xx: Add support for JZ4770 SoC ADC.

Baolin Wang (1):
      iio: adc: sc27xx: Use devm_hwspin_lock_request_specific() to simplify code

Bartosz Golaszewski (1):
      iio: pressure: bmp280: remove stray newline

Colin Ian King (3):
      iio: light: cm36651: redundant assignment to variable ret
      counter: stm32: clean up indentation issue
      iio: gyro: clean up indentation issue

Fabrice Gasnier (2):
      counter: stm32-timer-cnt: fix a kernel-doc warning
      counter: stm32-lptimer-cnt: fix a kernel-doc warning

Jean-Baptiste Maneyrol (7):
      iio: imu: inv_mpu6050: disable i2c mux for MPU925x
      iio: imu: inv_mpu6050: add header include protection macro
      iio: imu: inv_mpu6050: add defines for supporting 9-axis chips
      iio: imu: inv_mpu6050: fix objects syntax in Makefile
      iio: imu: inv_mpu6050: helpers for using i2c master on auxiliary bus
      iio: imu: inv_mpu6050: add MPU925x magnetometer support
      iio: imu: inv_mpu6050: add fifo support for magnetometer data

Krzysztof Wilczynski (1):
      iio: light: bh1750: Resolve compiler warning and make code more readable

Lorenzo Bianconi (2):
      iio: imu: st_lsm6dsx: enable LIR for sensor events
      iio: imu: st_lsm6dsx: enable clear on read for latched interrupts

Mircea Caprioru (2):
      iio: adc: ad_sigma_delta: Export ad_sd_calibrate
      staging: iio: adc: ad7192: Add system calibration support

Phil Reid (2):
      dt-binding: iio: Add optional label property
      iio: core: Add optional symbolic label to device attributes

Rohit Sarkar (1):
      staging: iio: ADIS16240: Remove unused include

Sean Nyekjaer (5):
      iio: imu: st_lsm6dsx: move interrupt thread to core
      iio: imu: st_lsm6dsx: add motion events
      iio: imu: st_lsm6dsx: add wakeup-source option
      iio: imu: st_lsm6dsx: always enter interrupt thread
      iio: imu: st_lsm6dsx: add motion report function and call from interrupt

Tomasz Duszynski (2):
      dt-bindings: iio: light: bh1750: convert bindings to yaml
      MAINTAINERS: add entry for ROHM BH1750 driver

 Documentation/ABI/testing/sysfs-bus-iio-adc-ad7192 |  24 ++
 .../devicetree/bindings/iio/adc/ingenic,adc.txt    |   1 +
 .../devicetree/bindings/iio/iio-bindings.txt       |   5 +
 .../devicetree/bindings/iio/light/bh1750.txt       |  18 -
 .../devicetree/bindings/iio/light/bh1750.yaml      |  43 ++
 MAINTAINERS                                        |   6 +
 drivers/counter/stm32-lptimer-cnt.c                |   2 +-
 drivers/counter/stm32-timer-cnt.c                  |   6 +-
 drivers/iio/adc/ad7949.c                           |  33 +-
 drivers/iio/adc/ad_sigma_delta.c                   |   3 +-
 drivers/iio/adc/ingenic-adc.c                      | 149 +++++--
 drivers/iio/adc/sc27xx_adc.c                       |  16 +-
 drivers/iio/chemical/atlas-ph-sensor.c             |   8 +-
 drivers/iio/gyro/adis16080.c                       |   8 +-
 drivers/iio/gyro/adis16130.c                       |   2 -
 drivers/iio/gyro/adis16136.c                       |  31 +-
 drivers/iio/gyro/itg3200_core.c                    |   2 +-
 drivers/iio/imu/adis.c                             |  95 ++---
 drivers/iio/imu/adis16400.c                        |  61 ++-
 drivers/iio/imu/adis16480.c                        |  17 +-
 drivers/iio/imu/adis_buffer.c                      |   4 +-
 drivers/iio/imu/inv_mpu6050/Makefile               |   7 +-
 drivers/iio/imu/inv_mpu6050/inv_mpu_aux.c          | 204 ++++++++++
 drivers/iio/imu/inv_mpu6050/inv_mpu_aux.h          |  19 +
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c         | 152 +++++++-
 drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c          |  60 ++-
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h          |  70 +++-
 drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c         | 356 +++++++++++++++++
 drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h         |  36 ++
 drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c         |  11 +-
 drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c      |  86 +++-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h            |  50 +++
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c     |  78 +---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c       | 434 ++++++++++++++++++++-
 drivers/iio/industrialio-core.c                    |  17 +
 drivers/iio/light/bh1750.c                         |   4 +-
 drivers/iio/light/cm36651.c                        |   2 +-
 drivers/iio/light/tcs3414.c                        |  30 +-
 drivers/iio/pressure/bmp280-core.c                 |   1 -
 drivers/iio/proximity/sx9500.c                     |  16 +-
 drivers/iio/temperature/max31856.c                 |   2 +
 drivers/staging/iio/accel/adis16240.c              |   1 -
 drivers/staging/iio/adc/ad7192.c                   |  79 +++-
 include/dt-bindings/iio/adc/ingenic,adc.h          |   1 +
 include/linux/iio/adc/ad_sigma_delta.h             |   2 +
 include/linux/iio/iio.h                            |   2 +
 include/linux/iio/imu/adis.h                       | 154 +++++++-
 47 files changed, 2082 insertions(+), 326 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/light/bh1750.txt
 create mode 100644 Documentation/devicetree/bindings/iio/light/bh1750.yaml
 create mode 100644 drivers/iio/imu/inv_mpu6050/inv_mpu_aux.c
 create mode 100644 drivers/iio/imu/inv_mpu6050/inv_mpu_aux.h
 create mode 100644 drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c
 create mode 100644 drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h

Re: [PULL v2] First set of iio new device support etc for the 5.5 cycle

[Greg KH]

On Sat, Oct 12, 2019 at 12:19:46PM +0100, Jonathan Cameron wrote:
> The following changes since commit b73b93a2af3392b9b7b8ba7e818ee767499f9655:
> 
>   iio: adc: ad7192: Add sysfs ABI documentation (2019-09-08 10:34:49 +0100)
> 
> are available in the Git repository at:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git tags/iio-for-5.5a-take2

Better, but I see this now:

drivers/iio/imu/adis.c: In function ‘__adis_check_status’:
drivers/iio/imu/adis.c:295:9: warning: ‘status’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  295 |  status &= adis->data->status_error_mask;
      |         ^~


I'll take this, can you just send a follow-on patch for this?

thanks,

greg k-h

Re: [PULL v2] First set of iio new device support etc for the 5.5 cycle

[Greg KH]

On Sat, Oct 12, 2019 at 05:27:44PM +0200, Greg KH wrote:
> On Sat, Oct 12, 2019 at 12:19:46PM +0100, Jonathan Cameron wrote:
> > The following changes since commit b73b93a2af3392b9b7b8ba7e818ee767499f9655:
> > 
> >   iio: adc: ad7192: Add sysfs ABI documentation (2019-09-08 10:34:49 +0100)
> > 
> > are available in the Git repository at:
> > 
> >   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git tags/iio-for-5.5a-take2
> 
> Better, but I see this now:
> 
> drivers/iio/imu/adis.c: In function ‘__adis_check_status’:
> drivers/iio/imu/adis.c:295:9: warning: ‘status’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>   295 |  status &= adis->data->status_error_mask;
>       |         ^~
> 
> 
> I'll take this, can you just send a follow-on patch for this?

Also I see:

drivers/iio/imu/adis16480.c: In function ‘adis16480_enable_irq’:
drivers/iio/imu/adis16480.c:950:6: warning: ‘val’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  950 |  val &= ~ADIS16480_DRDY_EN_MSK;
      |      ^~
  CC [M]  drivers/iio/magnetometer/hmc5843_i2c.o
drivers/iio/imu/adis16480.c: In function ‘adis16480_write_raw’:
drivers/iio/imu/adis16480.c:571:7: warning: ‘val’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  571 |   val &= ~enable_mask;
      |       ^~
drivers/iio/imu/adis16480.c:557:11: note: ‘val’ was declared here
  557 |  uint16_t val;
      |           ^~~


So did you really fix anything here?

I'll drop this pull again.

What version of gcc are you using?  Might I suggest a newer one (i.e. a
modern one?)

thanks,

greg k-h

Re: [PULL v2] First set of iio new device support etc for the 5.5 cycle

[Jonathan Cameron]

On Sat, 12 Oct 2019 17:28:41 +0200
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Sat, Oct 12, 2019 at 05:27:44PM +0200, Greg KH wrote:
> > On Sat, Oct 12, 2019 at 12:19:46PM +0100, Jonathan Cameron wrote:  
> > > The following changes since commit b73b93a2af3392b9b7b8ba7e818ee767499f9655:
> > > 
> > >   iio: adc: ad7192: Add sysfs ABI documentation (2019-09-08 10:34:49 +0100)
> > > 
> > > are available in the Git repository at:
> > > 
> > >   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git tags/iio-for-5.5a-take2  
> > 
> > Better, but I see this now:
> > 
> > drivers/iio/imu/adis.c: In function ‘__adis_check_status’:
> > drivers/iio/imu/adis.c:295:9: warning: ‘status’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> >   295 |  status &= adis->data->status_error_mask;
> >       |         ^~
> > 
> > 
> > I'll take this, can you just send a follow-on patch for this?  
> 
> Also I see:
> 
> drivers/iio/imu/adis16480.c: In function ‘adis16480_enable_irq’:
> drivers/iio/imu/adis16480.c:950:6: warning: ‘val’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>   950 |  val &= ~ADIS16480_DRDY_EN_MSK;
>       |      ^~
>   CC [M]  drivers/iio/magnetometer/hmc5843_i2c.o
> drivers/iio/imu/adis16480.c: In function ‘adis16480_write_raw’:
> drivers/iio/imu/adis16480.c:571:7: warning: ‘val’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>   571 |   val &= ~enable_mask;
>       |       ^~
> drivers/iio/imu/adis16480.c:557:11: note: ‘val’ was declared here
>   557 |  uint16_t val;
>       |           ^~~
> 
> 
> So did you really fix anything here?
> 
> I'll drop this pull again.
> 
> What version of gcc are you using?  Might I suggest a newer one (i.e. a
> modern one?)

Ah. This is my mistake.  I did see all of these, but still thought we were
in the category of tidying up some compiler version caused issues.

The adis16400 came up in my local tests, so I previously pinged Alex on
basis it was something to do in a follow up. The other two showed up, but
again I still thought these were compiler version issues, particularly
as 0-day didn't highlight them (there were several other issues it
did highlight this week). Hence again I requested a follow up to tidy
it up.

Anyhow, did some digging.  The issue here was a 'fix' I put in to an initial
0-day issue in the inline functions that Alex added.  Note that one
appears to be compiler version dependent as it didn't turn up in my
local builds. There are now inline functions that check if (ret)
and don't set the value if ret is non 0.

Out in the drivers, the check is the more specific (unnecessarily)
if (ret < 0) and hence the compiler is concluded that there might be a path to
val not being set.  Previously it was giving up figuring this out.
So reality is they are a false positive (sort of as in reality ret
is never positive) but the compiler has made a reasonable point
that it can't see that.

Never mind, I'll do a new pull request once fixes are in place.
Given there are two obvious ways of suppressing this and it's Alex's
driver I'll wait until he has time to take a look.

Sorry for wasting your time.

Jonathan

Re: [PULL v2] First set of iio new device support etc for the 5.5 cycle

[Alexandru Ardelean]

On Sat, Oct 12, 2019 at 7:18 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Sat, 12 Oct 2019 17:28:41 +0200
> Greg KH <gregkh@linuxfoundation.org> wrote:
>
> > On Sat, Oct 12, 2019 at 05:27:44PM +0200, Greg KH wrote:
> > > On Sat, Oct 12, 2019 at 12:19:46PM +0100, Jonathan Cameron wrote:
> > > > The following changes since commit b73b93a2af3392b9b7b8ba7e818ee767499f9655:
> > > >
> > > >   iio: adc: ad7192: Add sysfs ABI documentation (2019-09-08 10:34:49 +0100)
> > > >
> > > > are available in the Git repository at:
> > > >
> > > >   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git tags/iio-for-5.5a-take2
> > >
> > > Better, but I see this now:
> > >
> > > drivers/iio/imu/adis.c: In function ‘__adis_check_status’:
> > > drivers/iio/imu/adis.c:295:9: warning: ‘status’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> > >   295 |  status &= adis->data->status_error_mask;
> > >       |         ^~
> > >
> > >
> > > I'll take this, can you just send a follow-on patch for this?
> >
> > Also I see:
> >
> > drivers/iio/imu/adis16480.c: In function ‘adis16480_enable_irq’:
> > drivers/iio/imu/adis16480.c:950:6: warning: ‘val’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> >   950 |  val &= ~ADIS16480_DRDY_EN_MSK;
> >       |      ^~
> >   CC [M]  drivers/iio/magnetometer/hmc5843_i2c.o
> > drivers/iio/imu/adis16480.c: In function ‘adis16480_write_raw’:
> > drivers/iio/imu/adis16480.c:571:7: warning: ‘val’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> >   571 |   val &= ~enable_mask;
> >       |       ^~
> > drivers/iio/imu/adis16480.c:557:11: note: ‘val’ was declared here
> >   557 |  uint16_t val;
> >       |           ^~~
> >
> >
> > So did you really fix anything here?
> >
> > I'll drop this pull again.
> >
> > What version of gcc are you using?  Might I suggest a newer one (i.e. a
> > modern one?)
>
> Ah. This is my mistake.  I did see all of these, but still thought we were
> in the category of tidying up some compiler version caused issues.
>
> The adis16400 came up in my local tests, so I previously pinged Alex on
> basis it was something to do in a follow up. The other two showed up, but
> again I still thought these were compiler version issues, particularly
> as 0-day didn't highlight them (there were several other issues it
> did highlight this week). Hence again I requested a follow up to tidy
> it up.
>
> Anyhow, did some digging.  The issue here was a 'fix' I put in to an initial
> 0-day issue in the inline functions that Alex added.  Note that one
> appears to be compiler version dependent as it didn't turn up in my
> local builds. There are now inline functions that check if (ret)
> and don't set the value if ret is non 0.
>
> Out in the drivers, the check is the more specific (unnecessarily)
> if (ret < 0) and hence the compiler is concluded that there might be a path to
> val not being set.  Previously it was giving up figuring this out.
> So reality is they are a false positive (sort of as in reality ret
> is never positive) but the compiler has made a reasonable point
> that it can't see that.
>
> Never mind, I'll do a new pull request once fixes are in place.
> Given there are two obvious ways of suppressing this and it's Alex's
> driver I'll wait until he has time to take a look.
>
> Sorry for wasting your time.
>

If it helps, let's drop the ADIS patches for this round, and I can
take a closer look as well.
The cleanup does seem to have revealed a few gaps in our CI in
relation to upstreaming things.

We use Travis-CI for our stuff and stuff is public:
https://travis-ci.org/analogdevicesinc/linux

So, if anyone sees anything we should do better, I'm open to
improvements/suggestions.
I am in the process of adding sparse-builds, maybe some build
hardening is next (stronger compile/build flags), adding our patches
on top of a newer kernel (that's partially done).
That should help us catch things a bit earlier.

Sorry for the noise from my side as-well.
Alex


> Jonathan

Re: [PULL v2] First set of iio new device support etc for the 5.5 cycle

[Greg KH]

On Sat, Oct 12, 2019 at 09:41:45PM +0300, Alexandru Ardelean wrote:
> On Sat, Oct 12, 2019 at 7:18 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Sat, 12 Oct 2019 17:28:41 +0200
> > Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > > On Sat, Oct 12, 2019 at 05:27:44PM +0200, Greg KH wrote:
> > > > On Sat, Oct 12, 2019 at 12:19:46PM +0100, Jonathan Cameron wrote:
> > > > > The following changes since commit b73b93a2af3392b9b7b8ba7e818ee767499f9655:
> > > > >
> > > > >   iio: adc: ad7192: Add sysfs ABI documentation (2019-09-08 10:34:49 +0100)
> > > > >
> > > > > are available in the Git repository at:
> > > > >
> > > > >   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git tags/iio-for-5.5a-take2
> > > >
> > > > Better, but I see this now:
> > > >
> > > > drivers/iio/imu/adis.c: In function ‘__adis_check_status’:
> > > > drivers/iio/imu/adis.c:295:9: warning: ‘status’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> > > >   295 |  status &= adis->data->status_error_mask;
> > > >       |         ^~
> > > >
> > > >
> > > > I'll take this, can you just send a follow-on patch for this?
> > >
> > > Also I see:
> > >
> > > drivers/iio/imu/adis16480.c: In function ‘adis16480_enable_irq’:
> > > drivers/iio/imu/adis16480.c:950:6: warning: ‘val’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> > >   950 |  val &= ~ADIS16480_DRDY_EN_MSK;
> > >       |      ^~
> > >   CC [M]  drivers/iio/magnetometer/hmc5843_i2c.o
> > > drivers/iio/imu/adis16480.c: In function ‘adis16480_write_raw’:
> > > drivers/iio/imu/adis16480.c:571:7: warning: ‘val’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> > >   571 |   val &= ~enable_mask;
> > >       |       ^~
> > > drivers/iio/imu/adis16480.c:557:11: note: ‘val’ was declared here
> > >   557 |  uint16_t val;
> > >       |           ^~~
> > >
> > >
> > > So did you really fix anything here?
> > >
> > > I'll drop this pull again.
> > >
> > > What version of gcc are you using?  Might I suggest a newer one (i.e. a
> > > modern one?)
> >
> > Ah. This is my mistake.  I did see all of these, but still thought we were
> > in the category of tidying up some compiler version caused issues.
> >
> > The adis16400 came up in my local tests, so I previously pinged Alex on
> > basis it was something to do in a follow up. The other two showed up, but
> > again I still thought these were compiler version issues, particularly
> > as 0-day didn't highlight them (there were several other issues it
> > did highlight this week). Hence again I requested a follow up to tidy
> > it up.
> >
> > Anyhow, did some digging.  The issue here was a 'fix' I put in to an initial
> > 0-day issue in the inline functions that Alex added.  Note that one
> > appears to be compiler version dependent as it didn't turn up in my
> > local builds. There are now inline functions that check if (ret)
> > and don't set the value if ret is non 0.

Oddly, 0-day is not always seeing stuff like this, and I don't know why.

> > Out in the drivers, the check is the more specific (unnecessarily)
> > if (ret < 0) and hence the compiler is concluded that there might be a path to
> > val not being set.  Previously it was giving up figuring this out.
> > So reality is they are a false positive (sort of as in reality ret
> > is never positive) but the compiler has made a reasonable point
> > that it can't see that.
> >
> > Never mind, I'll do a new pull request once fixes are in place.
> > Given there are two obvious ways of suppressing this and it's Alex's
> > driver I'll wait until he has time to take a look.
> >
> > Sorry for wasting your time.
> >
> 
> If it helps, let's drop the ADIS patches for this round, and I can
> take a closer look as well.
> The cleanup does seem to have revealed a few gaps in our CI in
> relation to upstreaming things.
> 
> We use Travis-CI for our stuff and stuff is public:
> https://travis-ci.org/analogdevicesinc/linux
> 
> So, if anyone sees anything we should do better, I'm open to
> improvements/suggestions.

Building 'allmodconfig' with a recent version of gcc?  That's all I did
here to catch these warnings, nothing special :)

thanks,

greg k-h

Re: [PULL v2] First set of iio new device support etc for the 5.5 cycle

[Jonathan Cameron]

On Sat, 12 Oct 2019 21:41:45 +0300
Alexandru Ardelean <ardeleanalex@gmail.com> wrote:

> On Sat, Oct 12, 2019 at 7:18 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Sat, 12 Oct 2019 17:28:41 +0200
> > Greg KH <gregkh@linuxfoundation.org> wrote:
> >  
> > > On Sat, Oct 12, 2019 at 05:27:44PM +0200, Greg KH wrote:  
> > > > On Sat, Oct 12, 2019 at 12:19:46PM +0100, Jonathan Cameron wrote:  
> > > > > The following changes since commit b73b93a2af3392b9b7b8ba7e818ee767499f9655:
> > > > >
> > > > >   iio: adc: ad7192: Add sysfs ABI documentation (2019-09-08 10:34:49 +0100)
> > > > >
> > > > > are available in the Git repository at:
> > > > >
> > > > >   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git tags/iio-for-5.5a-take2  
> > > >
> > > > Better, but I see this now:
> > > >
> > > > drivers/iio/imu/adis.c: In function ‘__adis_check_status’:
> > > > drivers/iio/imu/adis.c:295:9: warning: ‘status’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> > > >   295 |  status &= adis->data->status_error_mask;
> > > >       |         ^~
> > > >
> > > >
> > > > I'll take this, can you just send a follow-on patch for this?  
> > >
> > > Also I see:
> > >
> > > drivers/iio/imu/adis16480.c: In function ‘adis16480_enable_irq’:
> > > drivers/iio/imu/adis16480.c:950:6: warning: ‘val’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> > >   950 |  val &= ~ADIS16480_DRDY_EN_MSK;
> > >       |      ^~
> > >   CC [M]  drivers/iio/magnetometer/hmc5843_i2c.o
> > > drivers/iio/imu/adis16480.c: In function ‘adis16480_write_raw’:
> > > drivers/iio/imu/adis16480.c:571:7: warning: ‘val’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> > >   571 |   val &= ~enable_mask;
> > >       |       ^~
> > > drivers/iio/imu/adis16480.c:557:11: note: ‘val’ was declared here
> > >   557 |  uint16_t val;
> > >       |           ^~~
> > >
> > >
> > > So did you really fix anything here?
> > >
> > > I'll drop this pull again.
> > >
> > > What version of gcc are you using?  Might I suggest a newer one (i.e. a
> > > modern one?)  
> >
> > Ah. This is my mistake.  I did see all of these, but still thought we were
> > in the category of tidying up some compiler version caused issues.
> >
> > The adis16400 came up in my local tests, so I previously pinged Alex on
> > basis it was something to do in a follow up. The other two showed up, but
> > again I still thought these were compiler version issues, particularly
> > as 0-day didn't highlight them (there were several other issues it
> > did highlight this week). Hence again I requested a follow up to tidy
> > it up.
> >
> > Anyhow, did some digging.  The issue here was a 'fix' I put in to an initial
> > 0-day issue in the inline functions that Alex added.  Note that one
> > appears to be compiler version dependent as it didn't turn up in my
> > local builds. There are now inline functions that check if (ret)
> > and don't set the value if ret is non 0.
> >
> > Out in the drivers, the check is the more specific (unnecessarily)
> > if (ret < 0) and hence the compiler is concluded that there might be a path to
> > val not being set.  Previously it was giving up figuring this out.
> > So reality is they are a false positive (sort of as in reality ret
> > is never positive) but the compiler has made a reasonable point
> > that it can't see that.
> >
> > Never mind, I'll do a new pull request once fixes are in place.
> > Given there are two obvious ways of suppressing this and it's Alex's
> > driver I'll wait until he has time to take a look.
> >
> > Sorry for wasting your time.
> >  
> 
> If it helps, let's drop the ADIS patches for this round, and I can
> take a closer look as well.

Makes sense.   As I had queued a few other changes you sent
later on top of this series, I'll drop those for now as well
rather than trying to work out which ones have dependencies on this and
which don't.

So I'll drop all of:

iio: gyro: adis16260: replace mlock with ADIS lib's state_lock
iio: imu: adis: add a note better explaining state_lock
iio: imu: adis: update `adis_data` struct doc-string
iio: imu: adis: add doc-string for `adis` struct
iio: imu: adis16400: fix compiler warnings

+ original series
iio: imu: adis16480: use state lock for filter freq set	
iio: gyro: adis16136: rework locks using ADIS library's state lock
iio: imu: adis16400: rework locks using ADIS library's state lock
iio: imu: adis: group single conversion under a single state lock
iio: imu: adis: protect initial startup routine with state lock
iio: imu: adis: create an unlocked version of adis_reset()
iio: imu: adis: create an unlocked version of adis_check_status()
iio: imu: adis[16480]: group RW into a single lock in adis_enable_irq()
iio: imu: adis: add unlocked read/write function versions
iio: imu: adis: rename txrx_lock -> state_lock

If you could do me a single series with those as well that would
be great.

Thanks for sorting this out,

Jonathan

> The cleanup does seem to have revealed a few gaps in our CI in
> relation to upstreaming things.
> 
> We use Travis-CI for our stuff and stuff is public:
> https://travis-ci.org/analogdevicesinc/linux
> 
> So, if anyone sees anything we should do better, I'm open to
> improvements/suggestions.
> I am in the process of adding sparse-builds, maybe some build
> hardening is next (stronger compile/build flags), adding our patches
> on top of a newer kernel (that's partially done).
> That should help us catch things a bit earlier.
> 
> Sorry for the noise from my side as-well.
> Alex
> 
> 
> > Jonathan