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

[Jonathan Cameron]

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(-)
>