Re: [PATCH v2 2/5] iio: light: si1133: remove unused macros

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 28 Apr 2026 15:18:34 +0200
Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:

> From: Joshua Crofts <joshua.crofts1@gmail.com>
> 
> Remove unused macros to improve readability.
Hi Joshua,

Don't update macros then remove them - that's just increasing
noise in the update patch. Reorder this patch to be before patch 1.

Otherwise, I think a few of these can stay. See below

Thanks,

Jonathan

> 
> No functional change.
> 
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> ---
>  drivers/iio/light/si1133.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/iio/light/si1133.c b/drivers/iio/light/si1133.c
> index 52e4269dc35014b87b7c46a120e64723be716768..ada8da122b76c9a0a55751baa6522a145f0fc7e8 100644
> --- a/drivers/iio/light/si1133.c
> +++ b/drivers/iio/light/si1133.c
> @@ -22,8 +22,6 @@
>  #define SI1133_REG_PART_ID		0x00
>  #define SI1133_REG_REV_ID		0x01
>  #define SI1133_REG_MFR_ID		0x02
> -#define SI1133_REG_INFO0		0x03
> -#define SI1133_REG_INFO1		0x04

This one isn't in the category of things Andy was referring to.
It's a hardware register address definition.  We don't have hard rules on whether
a complete set of these should be defined, or just the ones used but it is definitely
acceptable to just define them all up front as then easy to check them against
a datasheet.

>  
>  #define SI1133_PART_ID			0x33
>  
> @@ -40,7 +38,6 @@
>  #define SI1133_CMD_RESET_CTR		0x00
>  #define SI1133_CMD_RESET_SW		0x01
>  #define SI1133_CMD_FORCE		0x11
> -#define SI1133_CMD_START_AUTONOMOUS	0x13
Similar for this.

>  #define SI1133_CMD_PARAM_SET		0x80
>  #define SI1133_CMD_PARAM_QUERY		0x40
>  #define SI1133_CMD_PARAM_MASK		0x3F
> @@ -86,13 +83,9 @@
>  #define SI1133_CMD_MINSLEEP_US_LOW	5000
>  #define SI1133_CMD_MINSLEEP_US_HIGH	7500
>  #define SI1133_CMD_TIMEOUT_MS		25
> -#define SI1133_CMD_LUX_TIMEOUT_MS	5000
> -#define SI1133_CMD_TIMEOUT_US		(SI1133_CMD_TIMEOUT_MS * 1000)

Dropping these and the one below is good as those aren't register or command
defines. This last one is entirely pointless as better to have the rescale
inline and only one definition up here.

>  
>  #define SI1133_REG_HOSTOUT(x)		((x) + 0x13)
>  
> -#define SI1133_MEASUREMENT_FREQUENCY 1250
> -
>  #define SI1133_X_ORDER_MASK            0x0070
>  #define SI1133_Y_ORDER_MASK            0x0007
>  #define si1133_get_x_order(m)          (((m) & SI1133_X_ORDER_MASK) >> 4)
>