Re: [PATCH 2/2] i2c: npcm7xx: Annotate register field definitions with longer names

[Tali Perry]

Reviewed-by: Tali Perry <tali.perry1@xxxxxxxxx>

Thanks Jonathan!

On Sat, Oct 8, 2022 at 3:59 PM Jonathan Neuschäfer
<j.neuschaefer@xxxxxxx> wrote:
>
> To make the code easier to understand, add longer names to the
> definitions of register fields. These longer names are based on source
> code published by DELL/AESS for WPCM450, but should apply just as well
> to NPCM7xx and NPCM8xx.
>
> Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@xxxxxxx>
> ---
>  drivers/i2c/busses/i2c-npcm7xx.c | 56 ++++++++++++++++----------------
>  1 file changed, 28 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-npcm7xx.c b/drivers/i2c/busses/i2c-npcm7xx.c
> index 9a7a2d0bf5765..bbc7359e67f74 100644
> --- a/drivers/i2c/busses/i2c-npcm7xx.c
> +++ b/drivers/i2c/busses/i2c-npcm7xx.c
> @@ -146,50 +146,50 @@ static const int npcm_i2caddr[I2C_NUM_OWN_ADDR] = {
>  #endif
>
>  /* NPCM_I2CST reg fields */
> -#define NPCM_I2CST_XMIT                        BIT(0)
> -#define NPCM_I2CST_MASTER              BIT(1)
> -#define NPCM_I2CST_NMATCH              BIT(2)
> -#define NPCM_I2CST_STASTR              BIT(3)
> -#define NPCM_I2CST_NEGACK              BIT(4)
> -#define NPCM_I2CST_BER                 BIT(5)
> -#define NPCM_I2CST_SDAST               BIT(6)
> -#define NPCM_I2CST_SLVSTP              BIT(7)
> +#define NPCM_I2CST_XMIT                        BIT(0)  /* Transmit mode */
> +#define NPCM_I2CST_MASTER              BIT(1)  /* Master mode */
> +#define NPCM_I2CST_NMATCH              BIT(2)  /* New match */
> +#define NPCM_I2CST_STASTR              BIT(3)  /* Stall after start */
> +#define NPCM_I2CST_NEGACK              BIT(4)  /* Negative ACK */
> +#define NPCM_I2CST_BER                 BIT(5)  /* Bus error */
> +#define NPCM_I2CST_SDAST               BIT(6)  /* SDA status */
> +#define NPCM_I2CST_SLVSTP              BIT(7)  /* Slave stop */
>
>  /* NPCM_I2CCST reg fields */
> -#define NPCM_I2CCST_BUSY               BIT(0)
> -#define NPCM_I2CCST_BB                 BIT(1)
> -#define NPCM_I2CCST_MATCH              BIT(2)
> -#define NPCM_I2CCST_GCMATCH            BIT(3)
> -#define NPCM_I2CCST_TSDA               BIT(4)
> -#define NPCM_I2CCST_TGSCL              BIT(5)
> -#define NPCM_I2CCST_MATCHAF            BIT(6)
> -#define NPCM_I2CCST_ARPMATCH           BIT(7)
> +#define NPCM_I2CCST_BUSY               BIT(0)  /* Busy */
> +#define NPCM_I2CCST_BB                 BIT(1)  /* Bus busy */
> +#define NPCM_I2CCST_MATCH              BIT(2)  /* Address match */
> +#define NPCM_I2CCST_GCMATCH            BIT(3)  /* Global call match */
> +#define NPCM_I2CCST_TSDA               BIT(4)  /* Test SDA line */
> +#define NPCM_I2CCST_TGSCL              BIT(5)  /* Toggle SCL line */
> +#define NPCM_I2CCST_MATCHAF            BIT(6)  /* Match address field */
> +#define NPCM_I2CCST_ARPMATCH           BIT(7)  /* ARP address match */
>
>  /* NPCM_I2CCTL1 reg fields */
> -#define NPCM_I2CCTL1_START             BIT(0)
> -#define NPCM_I2CCTL1_STOP              BIT(1)
> -#define NPCM_I2CCTL1_INTEN             BIT(2)
> +#define NPCM_I2CCTL1_START             BIT(0)  /* Generate start condition */
> +#define NPCM_I2CCTL1_STOP              BIT(1)  /* Generate stop condition */
> +#define NPCM_I2CCTL1_INTEN             BIT(2)  /* Interrupt enable */
>  #define NPCM_I2CCTL1_EOBINTE           BIT(3)
>  #define NPCM_I2CCTL1_ACK               BIT(4)
> -#define NPCM_I2CCTL1_GCMEN             BIT(5)
> -#define NPCM_I2CCTL1_NMINTE            BIT(6)
> -#define NPCM_I2CCTL1_STASTRE           BIT(7)
> +#define NPCM_I2CCTL1_GCMEN             BIT(5)  /* Global call match enable */
> +#define NPCM_I2CCTL1_NMINTE            BIT(6)  /* New match interrupt enable */
> +#define NPCM_I2CCTL1_STASTRE           BIT(7)  /* Stall after start enable */
>
>  /* RW1S fields (inside a RW reg): */
>  #define NPCM_I2CCTL1_RWS   \
>         (NPCM_I2CCTL1_START | NPCM_I2CCTL1_STOP | NPCM_I2CCTL1_ACK)
>
>  /* npcm_i2caddr reg fields */
> -#define NPCM_I2CADDR_A                 GENMASK(6, 0)
> -#define NPCM_I2CADDR_SAEN              BIT(7)
> +#define NPCM_I2CADDR_A                 GENMASK(6, 0)   /* Address */
> +#define NPCM_I2CADDR_SAEN              BIT(7)          /* Slave address enable */
>
>  /* NPCM_I2CCTL2 reg fields */
> -#define I2CCTL2_ENABLE                 BIT(0)
> -#define I2CCTL2_SCLFRQ6_0              GENMASK(7, 1)
> +#define I2CCTL2_ENABLE                 BIT(0)          /* Module enable */
> +#define I2CCTL2_SCLFRQ6_0              GENMASK(7, 1)   /* Bits 0:6 of frequency divisor */
>
>  /* NPCM_I2CCTL3 reg fields */
> -#define I2CCTL3_SCLFRQ8_7              GENMASK(1, 0)
> -#define I2CCTL3_ARPMEN                 BIT(2)
> +#define I2CCTL3_SCLFRQ8_7              GENMASK(1, 0)   /* Bits 7:8 of frequency divisor */
> +#define I2CCTL3_ARPMEN                 BIT(2)  /* ARP match enable */
>  #define I2CCTL3_IDL_START              BIT(3)
>  #define I2CCTL3_400K_MODE              BIT(4)
>  #define I2CCTL3_BNK_SEL                        BIT(5)
> --
> 2.35.1
>