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

[Andy Shevchenko]

On Wed, Apr 20, 2022 at 11:11 PM 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.

You forgot to add tags from the previous round of review, please be
respectful to reviewers.


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


-- 
With Best Regards,
Andy Shevchenko