From: Neil Armstrong <narmstrong@baylibre.com>
To: Lucas Tanure <tanure@linux.com>,
Kevin Hilman <khilman@baylibre.com>,
Jerome Brunet <jbrunet@baylibre.com>,
Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: linux-i2c@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] i2c: meson: Use _SHIFT and _MASK for register definitions
Date: Mon, 11 Apr 2022 09:27:46 +0200 [thread overview]
Message-ID: <5d5d4590-18fc-1d94-08e3-7ffde8d9146d@baylibre.com> (raw)
In-Reply-To: <20220409164334.157664-2-tanure@linux.com>
On 09/04/2022 18:43, Lucas Tanure wrote:
> Differentiate between masks and shifts
>
> Signed-off-by: Lucas Tanure <tanure@linux.com>
> ---
> drivers/i2c/busses/i2c-meson.c | 41 ++++++++++++++++++----------------
> 1 file changed, 22 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
> index 07eb819072c4..4b4a5b2d77ab 100644
> --- a/drivers/i2c/busses/i2c-meson.c
> +++ b/drivers/i2c/busses/i2c-meson.c
> @@ -30,18 +30,21 @@
> #define REG_TOK_RDATA1 0x1c
>
> /* Control register fields */
> -#define REG_CTRL_START BIT(0)
> -#define REG_CTRL_ACK_IGNORE BIT(1)
> -#define REG_CTRL_STATUS BIT(2)
> -#define REG_CTRL_ERROR BIT(3)
> -#define REG_CTRL_CLKDIV GENMASK(21, 12)
> -#define REG_CTRL_CLKDIVEXT GENMASK(29, 28)
> -
> -#define REG_SLV_ADDR GENMASK(7, 0)
> -#define REG_SLV_SDA_FILTER GENMASK(10, 8)
> -#define REG_SLV_SCL_FILTER GENMASK(13, 11)
> -#define REG_SLV_SCL_LOW GENMASK(27, 16)
> -#define REG_SLV_SCL_LOW_EN BIT(28)
> +#define REG_CTRL_START BIT(0)
> +#define REG_CTRL_ACK_IGNORE BIT(1)
> +#define REG_CTRL_STATUS BIT(2)
> +#define REG_CTRL_ERROR BIT(3)
> +#define REG_CTRL_CLKDIV_SHIFT 12
> +#define REG_CTRL_CLKDIV_MASK GENMASK(21, REG_CTRL_CLKDIV_SHIFT)
> +#define REG_CTRL_CLKDIVEXT_SHIFT 28
> +#define REG_CTRL_CLKDIVEXT_MASK GENMASK(29, REG_CTRL_CLKDIVEXT_SHIFT)
> +
> +#define REG_SLV_ADDR_MASK GENMASK(7, 0)
> +#define REG_SLV_SDA_FILTER_MASK GENMASK(10, 8)
> +#define REG_SLV_SCL_FILTER_MASK GENMASK(13, 11)
> +#define REG_SLV_SCL_LOW_SHIFT 16
> +#define REG_SLV_SCL_LOW_MASK GENMASK(27, REG_SLV_SCL_LOW_SHIFT)
> +#define REG_SLV_SCL_LOW_EN BIT(28)
>
> #define I2C_TIMEOUT_MS 500
> #define FILTER_DELAY 15
> @@ -149,11 +152,11 @@ static void meson_i2c_set_clk_div(struct meson_i2c *i2c, unsigned int freq)
> div = GENMASK(11, 0);
> }
>
> - meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_CLKDIV,
> - FIELD_PREP(REG_CTRL_CLKDIV, div & GENMASK(9, 0)));
> + meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_CLKDIV_MASK,
> + FIELD_PREP(REG_CTRL_CLKDIV_MASK, div & GENMASK(9, 0)));
>
> - meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_CLKDIVEXT,
> - FIELD_PREP(REG_CTRL_CLKDIVEXT, div >> 10));
> + meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_CLKDIVEXT_MASK,
> + FIELD_PREP(REG_CTRL_CLKDIVEXT_MASK, div >> 10));
>
> /* Disable HIGH/LOW mode */
> meson_i2c_set_mask(i2c, REG_SLAVE_ADDR, REG_SLV_SCL_LOW_EN, 0);
> @@ -292,8 +295,8 @@ static void meson_i2c_do_start(struct meson_i2c *i2c, struct i2c_msg *msg)
> TOKEN_SLAVE_ADDR_WRITE;
>
>
> - meson_i2c_set_mask(i2c, REG_SLAVE_ADDR, REG_SLV_ADDR,
> - FIELD_PREP(REG_SLV_ADDR, msg->addr << 1));
> + meson_i2c_set_mask(i2c, REG_SLAVE_ADDR, REG_SLV_ADDR_MASK,
> + FIELD_PREP(REG_SLV_ADDR_MASK, msg->addr << 1));
>
> meson_i2c_add_token(i2c, TOKEN_START);
> meson_i2c_add_token(i2c, token);
> @@ -467,7 +470,7 @@ static int meson_i2c_probe(struct platform_device *pdev)
>
> /* Disable filtering */
> meson_i2c_set_mask(i2c, REG_SLAVE_ADDR,
> - REG_SLV_SDA_FILTER | REG_SLV_SCL_FILTER, 0);
> + REG_SLV_SDA_FILTER_MASK | REG_SLV_SCL_FILTER_MASK, 0);
>
> meson_i2c_set_clk_div(i2c, timings.bus_freq_hz);
>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
next prev parent reply other threads:[~2022-04-11 7:27 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-09 16:43 [PATCH v2 0/2] Ensure High and Low periods of SCL are correct Lucas Tanure
2022-04-09 16:43 ` [PATCH v2 1/2] i2c: meson: Use _SHIFT and _MASK for register definitions Lucas Tanure
2022-04-11 7:27 ` Neil Armstrong [this message]
2022-04-15 21:45 ` Wolfram Sang
2022-04-09 16:43 ` [PATCH v2 2/2] i2c: meson: Use 50% duty cycle for I2C clock Lucas Tanure
2022-04-11 7:29 ` Neil Armstrong
2022-04-15 21:45 ` Wolfram Sang
2022-04-11 7:30 ` [PATCH v2 0/2] Ensure High and Low periods of SCL are correct Neil Armstrong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5d5d4590-18fc-1d94-08e3-7ffde8d9146d@baylibre.com \
--to=narmstrong@baylibre.com \
--cc=jbrunet@baylibre.com \
--cc=khilman@baylibre.com \
--cc=linux-amlogic@lists.infradead.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=martin.blumenstingl@googlemail.com \
--cc=tanure@linux.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox