Re: [PATCH v4 0/9] iio: accel: bma400: Add buffer, step and activity/inactivity

From: Jonathan Cameron
Date: Sun May 01 2022 - 12:28:05 EST


On Thu, 21 Apr 2022 02:40:56 +0530
Jagath Jog J <jagathjog1996@xxxxxxxxx> wrote:

> This patch series adds trigger buffer support with data ready interrupt,
> separate channel for step counter, an event for step change interrupt,
> activity recognition and activity/inactivity event support.

Hi Jagath,

This is coming together nicely. I'm fine with all the patches
I haven't sent specific replies for.

Thanks,

Jonathan

>
> changes since v3
> 1. Removed all the unnecessary mutex locking for regmap.
> 2. Corrected the mutex locking and unlocking for device private data
> members.
> 3. Mutex locking and unlocking is used to protect the device private
> structure members.
> 4. Using DMA safe buffer for regmap_bulk_write() and regmap_bulk_read().
>
> 1/9: Fixed the comment.
>
> 3/9: Added () for the function name in the comment.
>
> 4/9: Handling error cases with goto in bma400_trigger_handler().
> Mutex locking and unlocking is used to protect the data->buffer.
> Using DMA safe buffer for regmap_bulk_read().
> Mutex locking and unlocking is used to protect the data->status in
> bma400_interrupt.
>
> 5/9: Using DMA safe buffers to read steps value by allocating memory internally.
> Using DMA safe buffers for regmap_bulk_write().
> Removed the lock for regmap().
>
> 6/9: Removed the duplication of code for enabling step, added function to handle
> the step enable.
>
> 7/9: Removed the lock for regmap().
> Mutex locking and unlocking is used to protect the data members.
>
> 8/9: Removed the lock for regmap().
>
> 9/9. Added __be16 duration in struct bma400_data.
> Fixed the warning - impossible condition '(reg < 0) => (0-255 < 0)'
> Fixed error: call to __compiletime_assert_272
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
>
> changes since v2
> 1. Reordering of header includes in the separate patch.
> 2. Matching the IIO syntax for multiline comment.
> 3. Following the preference in the interrupt handler for returning.
> 4. Add support for activity recognition.
> 5. Add support for debugfs to access registers from userspace.
> 6. Add support for activity and inactivity events
>
> changes since v1
> 1. Added comment section that describes the math for scale calculation.
> 2. Added separate devm_add_action_or_reset() calls to disable regulator
> and to put the sensor in power down mode.
> 3. Remove the err_reg_disable and out, goto labels and returning directly
> if error occurs.
> 4. Added mutex calls while putting sensor in power down.
> 5. Added ___cacheline_aligned for device data.
> 6. Ordering the header includes.
> 7. Handling erroneous and spurious interrupts in the interrupt handler
> by returning IRQ_NONE.
> 8. Using dev_err_probe() instead of dev_err().
> 9. Configured the interrupt to open drain.
> 10. Using le16_to_cpu() to fix the sparse warning.
> 11. Checking the step change event is enabled or not.
> 12. Enabling the step change event will also enable the step channel.
> 13. Using FIELD_GET() instead of bitwise operation.
> 14. Removal of dead code in the _event_config().
>
> Jagath Jog J (9):
> iio: accel: bma400: Fix the scale min and max macro values
> iio: accel: bma400: Reordering of header files
> iio: accel: bma400: conversion to device-managed function
> iio: accel: bma400: Add triggered buffer support
> iio: accel: bma400: Add separate channel for step counter
> iio: accel: bma400: Add step change event
> iio: accel: bma400: Add activity recognition support
> iio: accel: bma400: Add debugfs register access support
> iio: accel: bma400: Add support for activity and inactivity events
>
> drivers/iio/accel/Kconfig | 2 +
> drivers/iio/accel/bma400.h | 50 ++-
> drivers/iio/accel/bma400_core.c | 694 +++++++++++++++++++++++++++++---
> drivers/iio/accel/bma400_i2c.c | 10 +-
> drivers/iio/accel/bma400_spi.c | 8 +-
> 5 files changed, 697 insertions(+), 67 deletions(-)
>