[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHb3i=tVphSZiPKohnVX3Q0AXD0LYQYStNjzXzrOKFPvKL3odg@mail.gmail.com>
Date: Tue, 11 Oct 2022 09:08:32 +0300
From: Tali Perry <tali.perry1@...il.com>
To: Jonathan Neuschäfer <j.neuschaefer@....net>
Cc: linux-i2c@...r.kernel.org, openbmc@...ts.ozlabs.org,
Avi Fishman <avifishman70@...il.com>,
Tomer Maimon <tmaimon77@...il.com>,
Patrick Venture <venture@...gle.com>,
Nancy Yuen <yuenn@...gle.com>,
Benjamin Fair <benjaminfair@...gle.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] i2c: npcm7xx: Annotate register field definitions
with longer names
Reviewed-by: Tali Perry <tali.perry1@...il.com>
Thanks Jonathan!
On Sat, Oct 8, 2022 at 3:59 PM Jonathan Neuschäfer
<j.neuschaefer@....net> 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@....net>
> ---
> 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
>
Powered by blists - more mailing lists