Re: [PATCH 3/3] i2c: meson: fixup rate calculation with filter delay

2020-10-08 Thread Wolfram Sang
On Wed, Oct 07, 2020 at 10:07:51AM +0200, Jerome Brunet wrote:
> From: Nicolas Belin 
> 
> Apparently, 15 cycles of the peripheral clock are used by the controller
> for sampling and filtering. Because this was not known before, the rate
> calculation is slightly off.
> 
> Clean up and fix the calculation taking this filtering delay into account.
> 
> Fixes: 30021e3707a7 ("i2c: add support for Amlogic Meson I2C controller")
> Signed-off-by: Nicolas Belin 
> Signed-off-by: Jerome Brunet 

Applied to for-current, thanks!



signature.asc
Description: PGP signature


[PATCH 3/3] i2c: meson: fixup rate calculation with filter delay

2020-10-07 Thread Jerome Brunet
From: Nicolas Belin 

Apparently, 15 cycles of the peripheral clock are used by the controller
for sampling and filtering. Because this was not known before, the rate
calculation is slightly off.

Clean up and fix the calculation taking this filtering delay into account.

Fixes: 30021e3707a7 ("i2c: add support for Amlogic Meson I2C controller")
Signed-off-by: Nicolas Belin 
Signed-off-by: Jerome Brunet 
---
 drivers/i2c/busses/i2c-meson.c | 23 ---
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index e7ec2ab2a220..ef73a42577cc 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -34,10 +34,8 @@
 #define REG_CTRL_ACK_IGNOREBIT(1)
 #define REG_CTRL_STATUSBIT(2)
 #define REG_CTRL_ERROR BIT(3)
-#define REG_CTRL_CLKDIV_SHIFT  12
-#define REG_CTRL_CLKDIV_MASK   GENMASK(21, 12)
-#define REG_CTRL_CLKDIVEXT_SHIFT 28
-#define REG_CTRL_CLKDIVEXT_MASKGENMASK(29, 28)
+#define REG_CTRL_CLKDIVGENMASK(21, 12)
+#define REG_CTRL_CLKDIVEXT GENMASK(29, 28)
 
 #define REG_SLV_ADDR   GENMASK(7, 0)
 #define REG_SLV_SDA_FILTER GENMASK(10, 8)
@@ -46,6 +44,7 @@
 #define REG_SLV_SCL_LOW_EN BIT(28)
 
 #define I2C_TIMEOUT_MS 500
+#define FILTER_DELAY   15
 
 enum {
TOKEN_END = 0,
@@ -140,19 +139,21 @@ static void meson_i2c_set_clk_div(struct meson_i2c *i2c, 
unsigned int freq)
unsigned long clk_rate = clk_get_rate(i2c->clk);
unsigned int div;
 
-   div = DIV_ROUND_UP(clk_rate, freq * i2c->data->div_factor);
+   div = DIV_ROUND_UP(clk_rate, freq);
+   div -= FILTER_DELAY;
+   div = DIV_ROUND_UP(div, i2c->data->div_factor);
 
/* clock divider has 12 bits */
-   if (div >= (1 << 12)) {
+   if (div > GENMASK(11, 0)) {
dev_err(i2c->dev, "requested bus frequency too low\n");
-   div = (1 << 12) - 1;
+   div = GENMASK(11, 0);
}
 
-   meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_CLKDIV_MASK,
-  (div & GENMASK(9, 0)) << REG_CTRL_CLKDIV_SHIFT);
+   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_CLKDIVEXT_MASK,
-  (div >> 10) << REG_CTRL_CLKDIVEXT_SHIFT);
+   meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_CLKDIVEXT,
+  FIELD_PREP(REG_CTRL_CLKDIVEXT, div >> 10));
 
/* Disable HIGH/LOW mode */
meson_i2c_set_mask(i2c, REG_SLAVE_ADDR, REG_SLV_SCL_LOW_EN, 0);
-- 
2.25.4