linux-kernel - Re: [PATCH v4 1/6] iio: accel: bma400: Reorganize and rename register and field macros

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251012165228.4650cdac@jic23-huawei>
Date: Sun, 12 Oct 2025 16:52:28 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Akshay Jindal <akshayaj.lkd@...il.com>
Cc: dan@...obertson.com, dlechner@...libre.com, nuno.sa@...log.com,
 andy@...nel.org, shuah@...nel.org, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 1/6] iio: accel: bma400: Reorganize and rename
 register and field macros

On Tue,  7 Oct 2025 11:25:01 +0530
Akshay Jindal <akshayaj.lkd@...il.com> wrote:

> Reorganize register and field macros to improve consistency with the
> datasheet and naming style:
> 
> - Move field macros next to their corresponding register macros
> - Reorder register macros to follow address order from the datasheet
> - Rename field macros to include the register name in the macro name
> - Add a _REG suffix to register macros where missing
> 
> No functional changes are intended.
> 
> Signed-off-by: Akshay Jindal <akshayaj.lkd@...il.com>
> ---
>  drivers/iio/accel/bma400.h      | 110 +++++++++--------
>  drivers/iio/accel/bma400_core.c | 212 ++++++++++++++++----------------
>  2 files changed, 163 insertions(+), 159 deletions(-)
> 
> diff --git a/drivers/iio/accel/bma400.h b/drivers/iio/accel/bma400.h
> index 932358b45f17..ae3411c090c9 100644
> --- a/drivers/iio/accel/bma400.h
> +++ b/drivers/iio/accel/bma400.h
> @@ -16,31 +16,37 @@
>   * Read-Only Registers
>   */
>  
> +/* Chip ID of BMA 400 devices found in the chip ID register. */
> +#define BMA400_ID_REG_VAL           0x90
> +
>  /* Status and ID registers */
>  #define BMA400_CHIP_ID_REG          0x00
>  #define BMA400_ERR_REG              0x02
>  #define BMA400_STATUS_REG           0x03
>  
>  /* Acceleration registers */
> -#define BMA400_X_AXIS_LSB_REG       0x04
> -#define BMA400_X_AXIS_MSB_REG       0x05
> -#define BMA400_Y_AXIS_LSB_REG       0x06
> -#define BMA400_Y_AXIS_MSB_REG       0x07
> -#define BMA400_Z_AXIS_LSB_REG       0x08
> -#define BMA400_Z_AXIS_MSB_REG       0x09
> +#define BMA400_ACC_X_LSB_REG		0x04
> +#define BMA400_ACC_X_MSB_REG		0x05
> +#define BMA400_ACC_Y_LSB_REG		0x06
> +#define BMA400_ACC_Y_MSB_REG		0x07
> +#define BMA400_ACC_Z_LSB_REG		0x08
> +#define BMA400_ACC_Z_MSB_REG		0x09
>  
>  /* Sensor time registers */
> -#define BMA400_SENSOR_TIME0         0x0a
> -#define BMA400_SENSOR_TIME1         0x0b
> -#define BMA400_SENSOR_TIME2         0x0c
> +#define BMA400_SENSOR_TIME0_REG         0x0a
> +#define BMA400_SENSOR_TIME1_REG         0x0b
> +#define BMA400_SENSOR_TIME2_REG         0x0c
>  
>  /* Event and interrupt registers */
>  #define BMA400_EVENT_REG            0x0d
> +
>  #define BMA400_INT_STAT0_REG        0x0e
>  #define BMA400_INT_STAT1_REG        0x0f
>  #define BMA400_INT_STAT2_REG        0x10
> -#define BMA400_INT12_MAP_REG        0x23
> -#define BMA400_INT_ENG_OVRUN_MSK    BIT(4)
> +#define BMA400_ENG_OVRUN_INT_STAT_MASK		BIT(4)
> +#define BMA400_STEP_INT_STAT_MASK		GENMASK(9, 8)
> +#define BMA400_S_TAP_INT_STAT_MASK		BIT(10)
> +#define BMA400_D_TAP_INT_STAT_MASK		BIT(11)

Discussion on naming format and association with registers continues on v3
(busy week so I only just got back to reviewing!)

Also a bit on the weird 2 registers as one thing going on here.

>  
>  /* Temperature register */
>  #define BMA400_TEMP_DATA_REG        0x11
> @@ -55,70 +61,68 @@
>  #define BMA400_STEP_CNT1_REG        0x16
>  #define BMA400_STEP_CNT3_REG        0x17
>  #define BMA400_STEP_STAT_REG        0x18
> -#define BMA400_STEP_INT_MSK         BIT(0)
>  #define BMA400_STEP_RAW_LEN         0x03
> -#define BMA400_STEP_STAT_MASK       GENMASK(9, 8)

Other than those outstanding bits this looks fine (though I'll take one more close
read once we have those questions resolved).

Jonathan