* [PULL v2] First set of iio new device support etc for the 5.5 cycle @ 2019-10-12 11:19 Jonathan Cameron 2019-10-12 15:27 ` Greg KH 0 siblings, 1 reply; 7+ messages in thread From: Jonathan Cameron @ 2019-10-12 11:19 UTC (permalink / raw) To: gregkh, linux-iio 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PULL v2] First set of iio new device support etc for the 5.5 cycle 2019-10-12 11:19 [PULL v2] First set of iio new device support etc for the 5.5 cycle Jonathan Cameron @ 2019-10-12 15:27 ` Greg KH 2019-10-12 15:28 ` Greg KH 0 siblings, 1 reply; 7+ messages in thread From: Greg KH @ 2019-10-12 15:27 UTC (permalink / raw) To: Jonathan Cameron; +Cc: linux-iio 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PULL v2] First set of iio new device support etc for the 5.5 cycle 2019-10-12 15:27 ` Greg KH @ 2019-10-12 15:28 ` Greg KH 2019-10-12 16:06 ` Jonathan Cameron 0 siblings, 1 reply; 7+ messages in thread From: Greg KH @ 2019-10-12 15:28 UTC (permalink / raw) To: Jonathan Cameron; +Cc: linux-iio 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PULL v2] First set of iio new device support etc for the 5.5 cycle 2019-10-12 15:28 ` Greg KH @ 2019-10-12 16:06 ` Jonathan Cameron 2019-10-12 18:41 ` Alexandru Ardelean 0 siblings, 1 reply; 7+ messages in thread From: Jonathan Cameron @ 2019-10-12 16:06 UTC (permalink / raw) To: Greg KH; +Cc: linux-iio, Alexandru Ardelean 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PULL v2] First set of iio new device support etc for the 5.5 cycle 2019-10-12 16:06 ` Jonathan Cameron @ 2019-10-12 18:41 ` Alexandru Ardelean 2019-10-13 7:59 ` Greg KH 2019-10-13 8:09 ` Jonathan Cameron 0 siblings, 2 replies; 7+ messages in thread From: Alexandru Ardelean @ 2019-10-12 18:41 UTC (permalink / raw) To: Jonathan Cameron; +Cc: Greg KH, linux-iio, 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PULL v2] First set of iio new device support etc for the 5.5 cycle 2019-10-12 18:41 ` Alexandru Ardelean @ 2019-10-13 7:59 ` Greg KH 2019-10-13 8:09 ` Jonathan Cameron 1 sibling, 0 replies; 7+ messages in thread From: Greg KH @ 2019-10-13 7:59 UTC (permalink / raw) To: Alexandru Ardelean; +Cc: Jonathan Cameron, linux-iio, Alexandru Ardelean 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PULL v2] First set of iio new device support etc for the 5.5 cycle 2019-10-12 18:41 ` Alexandru Ardelean 2019-10-13 7:59 ` Greg KH @ 2019-10-13 8:09 ` Jonathan Cameron 1 sibling, 0 replies; 7+ messages in thread From: Jonathan Cameron @ 2019-10-13 8:09 UTC (permalink / raw) To: Alexandru Ardelean; +Cc: Greg KH, linux-iio, Alexandru Ardelean 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-10-13 8:09 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-12 11:19 [PULL v2] First set of iio new device support etc for the 5.5 cycle Jonathan Cameron 2019-10-12 15:27 ` Greg KH 2019-10-12 15:28 ` Greg KH 2019-10-12 16:06 ` Jonathan Cameron 2019-10-12 18:41 ` Alexandru Ardelean 2019-10-13 7:59 ` Greg KH 2019-10-13 8:09 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).