[PATCH v3 0/8] iio: tcs3472: cleanups, devm conversion and wait time

Aldo Conte <[email protected]>
Newsgroups dev.linux.lists.linux-kernel-mentees,org.kernel.vger.linux-iio,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
This is v3 of the TCS3472 series, addressing the review comments from
Andy Shevchenko, Jonathan Cameron and Joshua Crofts on v2.

The series has been reorganized into 8 patches as
requested by the reviewers: cleanup/bug-fix patches separated from
refactors, dev_info() conversion split out of the devm patch, and a
new precursor bug fix for powerdown on probe failure.

Patch 1: powers down the chip if probe fails after the chip has been
         enabled. Pre-existing bug, found by code inspection.
         v3:
         - New patch in v3, split out as a precursor bug fix.

Patch 2: sorts the #include block alphabetically.
         v3:
         - No changes since v2.

Patch 3: converts the remaining mutex_lock/unlock pairs to
         guard(mutex)(), with small cleanups that guard() enables
         (early returns on I2C error, !! -> ? 1 : 0).
         v3:
         - In write_event_config, powerdown, resume: early return on
           I2C error.
         - In read_event_config: !! -> ? 1 : 0.
         - Removed stray blank line in trigger_handler.
         - Subject shortened (no function list).

Patch 4: uses ! instead of explicit NULL check in probe.
         v3:
         - New patch (suggested by Joshua).

Patch 5: converts the driver to devm for resource allocation, drops
         the explicit remove() callback, and adds
         tcs3472_powerdown_action() to power down the chip on cleanup.
         v3:
         - Powerdown bug fix moved to patch 1.
         - Powerdown/resume RMW refactor moved to guard patch.
         - TCS3472_ENABLE_RUN define moved to wait time patch.
         - dev_info() conversion split into patch 6.
         - devm_mutex_init() used for the lock (suggested by Joshua).

Patch 6: uses the local 'struct device *dev' for the remaining
         dev_info() calls in probe.
         v3:
         - New patch, split out from the devm conversion.

Patch 7: moves the trailing return -EINVAL in tcs3472_read_raw() and
         tcs3472_write_raw() into explicit default: cases.
         v3:
         - No code changes since v2.
         - Carried over Reviewed-by tags from Andy and Joshua.

Patch 8: implements wait time support exposed through
         sampling_frequency, closing a long-standing TODO. WTIME is
         recomputed when integration_time changes to preserve the
         requested frequency. WEN is disabled when the frequency is
         too high; WLONG is enabled when the period exceeds WTIME
         range.
         v3:
         - TCS3472_ENABLE_RUN: new macro style (name+tab+backslash).
         - cycle_us: div64_u64() and (u64)val cast for 32-bit safety.
         - wait_us: kept s64 with comment explaining the range.
         - 'if (ret)' instead of 'if (ret < 0)' after i2c writes.
         - New helper tcs3472_cycle_to_freq() deduplicates val/val2.
         - !! -> ? 1 : 0 in probe (WLONG extraction).
         - Fix in tcs3472_resume: preserve WEN from data->enable
           instead of forcing TCS3472_ENABLE_RUN, so a user-set
           sampling_frequency that disabled WEN is honored.

Testing:

All eight patches were tested individually on a Raspberry Pi 3B with
an Adafruit TCS3472 breakout connected to I2C-1 at address 0x29.
Each patch was checked out and built separately to ensure the driver
remains functional in every intermediate state of the series.

Patch 2 (sort headers):
  - Driver loads cleanly; RGBC channel reads return reasonable values.
  - No warnings in dmesg.

Patch 3 (guard(mutex)):
  - Threshold value writes (RISING=1000, FALLING=200) read back
    correctly; AIHT/AILT registers on the chip mirror the values.
  - Threshold enable toggles the AIEN bit in ENABLE
    (sysfs '1' -> 0x13, sysfs '0' -> 0x03).
  - rmmod completes with no lock leak in dmesg.

Patch 4 (! instead of NULL check):
  - Driver loads cleanly; behavior identical to previous patch.

Patch 5 (devm conversion):
  - Probe completes with ENABLE = 0x03 (PON|AEN, WEN not yet
    introduced).
  - RGBC reads return sensible values (e.g. clear=525, red=269,
    green=151, blue=97).
  - calibscale, integration_time, threshold value/period, and
    threshold enable all work as before.
  - After echo 0x29 to delete_device, ENABLE reads back as 0x00,
    confirming that devm_add_action_or_reset() invokes
    tcs3472_powerdown() on cleanup.
  - rmmod completes with no warning.

Patch 6 (use 'dev' for dev_info):
  - Driver loads cleanly; "TCS34721/34725 found" or "TCS34723/34727
    found" appears in dmesg as before.

Patch 7 (default case):
  - All read_raw and write_raw cases handle attributes identically
    to the previous patch.
  - Invalid inputs still rejected with -EINVAL through the explicit
    default: case.

Patch 8 (wait time / sampling_frequency):
  - Initial state after probe: ATIME=0x00, WTIME=0xff, CONFIG=0x00,
    ENABLE=0x0b (PON|AEN|WEN), sampling_frequency=1.614987 Hz.
  - Write 1 Hz: WTIME=0x60, CONFIG=0x00, ENABLE=0x0b,
    sampling_frequency reads back 0.999200 Hz.
  - Write 5 Hz (too high for any wait time): WEN cleared,
    ENABLE=0x03, sampling_frequency reads back 1.621271 Hz.
  - Write 0.5 Hz (period exceeds WTIME range): WLONG enabled,
    CONFIG=0x02, WTIME=0xd0, WEN re-enabled, ENABLE=0x0b,
    sampling_frequency reads back 0.500200 Hz.
  - After setting 0.5 Hz, changing integration_time to 0.024 s:
    WTIME automatically recomputed to 0xbb, sampling_frequency reads
    back 0.496622 Hz (closest achievable to 0.5 Hz at the new ATIME).
  - Invalid inputs (-1.0, 0.0) rejected with -EINVAL; chip state
    unchanged.
  - Suspend/resume cycle preserves WEN state: after disabling WEN
    via high sampling_frequency, suspend then resume leaves ENABLE
    at 0x03 (WEN not forced back on).
  - After rmmod, ENABLE reads back as 0x00.

Aldo Conte (8):
  iio: tcs3472: power down chip on probe failure
  iio: tcs3472: sort headers alphabetically
  iio: tcs3472: convert remaining locking to guard(mutex)
  iio: tcs3472: use ! instead of explicit NULL check
  iio: tcs3472: use devm for resource management
  iio: tcs3472: use local struct device * for remaining cases
  iio: tcs3472: move standalone return to default case
  iio: tcs3472: implement wait time and sampling frequency

 drivers/iio/light/tcs3472.c | 416 ++++++++++++++++++++++++++----------
 1 file changed, 303 insertions(+), 113 deletions(-)

-- 
2.54.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.