[PATCH v2 00/10] i2c: meson: series with improvements

[PATCH v2 00/10] i2c: meson: series with improvements

[Heiner Kallweit]

This patch series includes several improvements for the i2c-meson
driver.

Changes in v2:
- removed patches 8 and 12
- removed one small change in patch 6
- don't print error message in patch 7

Heiner Kallweit (12):
  i2c: meson: use min instead of min_t where min_t isn't needed
  i2c: meson: remove member irq from struct meson_i2c
  i2c: meson: use i2c core for DT clock-frequency parsing
  i2c: meson: use full 12 bits for clock divider
  i2c: meson: set clock divider in probe instead of setting it for each transfer
  i2c: meson: remove variable count from meson_i2c_xfer
  i2c: meson: improve interrupt handler and detect spurious interrupts
  i2c: meson: don't create separate token chain just for the stop command
  i2c: meson: remove meson_i2c_write_tokens
  i2c: meson: improve and simplify interrupt handler

 drivers/i2c/busses/i2c-meson.c | 136 ++++++++++++++++-------------------------
 1 file changed, 51 insertions(+), 85 deletions(-)

-- 
2.12.0

[PATCH v2 00/10] i2c: meson: series with improvements

[Heiner Kallweit]

This patch series includes several improvements for the i2c-meson
driver.

Changes in v2:
- removed patches 8 and 12
- removed one small change in patch 6
- don't print error message in patch 7

Heiner Kallweit (12):
  i2c: meson: use min instead of min_t where min_t isn't needed
  i2c: meson: remove member irq from struct meson_i2c
  i2c: meson: use i2c core for DT clock-frequency parsing
  i2c: meson: use full 12 bits for clock divider
  i2c: meson: set clock divider in probe instead of setting it for each transfer
  i2c: meson: remove variable count from meson_i2c_xfer
  i2c: meson: improve interrupt handler and detect spurious interrupts
  i2c: meson: don't create separate token chain just for the stop command
  i2c: meson: remove meson_i2c_write_tokens
  i2c: meson: improve and simplify interrupt handler

 drivers/i2c/busses/i2c-meson.c | 136 ++++++++++++++++-------------------------
 1 file changed, 51 insertions(+), 85 deletions(-)

-- 
2.12.0

[PATCH v2 01/10] i2c: meson: i2c: meson: use min instead of min_t where min_t isn't needed

[Heiner Kallweit]

Use min instead of min_t where min_t isn't needed.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- no changes
---
 drivers/i2c/busses/i2c-meson.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index 73b97c71..40e5da9a 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -156,10 +156,10 @@ static void meson_i2c_get_data(struct meson_i2c *i2c, char *buf, int len)
 	dev_dbg(i2c->dev, "%s: data %08x %08x len %d\n", __func__,
 		rdata0, rdata1, len);
 
-	for (i = 0; i < min_t(int, 4, len); i++)
+	for (i = 0; i < min(4, len); i++)
 		*buf++ = (rdata0 >> i * 8) & 0xff;
 
-	for (i = 4; i < min_t(int, 8, len); i++)
+	for (i = 4; i < min(8, len); i++)
 		*buf++ = (rdata1 >> (i - 4) * 8) & 0xff;
 }
 
@@ -168,10 +168,10 @@ static void meson_i2c_put_data(struct meson_i2c *i2c, char *buf, int len)
 	u32 wdata0 = 0, wdata1 = 0;
 	int i;
 
-	for (i = 0; i < min_t(int, 4, len); i++)
+	for (i = 0; i < min(4, len); i++)
 		wdata0 |= *buf++ << (i * 8);
 
-	for (i = 4; i < min_t(int, 8, len); i++)
+	for (i = 4; i < min(8, len); i++)
 		wdata1 |= *buf++ << ((i - 4) * 8);
 
 	writel(wdata0, i2c->regs + REG_TOK_WDATA0);
@@ -186,7 +186,7 @@ static void meson_i2c_prepare_xfer(struct meson_i2c *i2c)
 	bool write = !(i2c->msg->flags & I2C_M_RD);
 	int i;
 
-	i2c->count = min_t(int, i2c->msg->len - i2c->pos, 8);
+	i2c->count = min(i2c->msg->len - i2c->pos, 8);
 
 	for (i = 0; i < i2c->count - 1; i++)
 		meson_i2c_add_token(i2c, TOKEN_DATA);
-- 
2.12.0

[PATCH v2 01/10] i2c: meson: i2c: meson: use min instead of min_t where min_t isn't needed

[Heiner Kallweit]

Use min instead of min_t where min_t isn't needed.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- no changes
---
 drivers/i2c/busses/i2c-meson.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index 73b97c71..40e5da9a 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -156,10 +156,10 @@ static void meson_i2c_get_data(struct meson_i2c *i2c, char *buf, int len)
 	dev_dbg(i2c->dev, "%s: data %08x %08x len %d\n", __func__,
 		rdata0, rdata1, len);
 
-	for (i = 0; i < min_t(int, 4, len); i++)
+	for (i = 0; i < min(4, len); i++)
 		*buf++ = (rdata0 >> i * 8) & 0xff;
 
-	for (i = 4; i < min_t(int, 8, len); i++)
+	for (i = 4; i < min(8, len); i++)
 		*buf++ = (rdata1 >> (i - 4) * 8) & 0xff;
 }
 
@@ -168,10 +168,10 @@ static void meson_i2c_put_data(struct meson_i2c *i2c, char *buf, int len)
 	u32 wdata0 = 0, wdata1 = 0;
 	int i;
 
-	for (i = 0; i < min_t(int, 4, len); i++)
+	for (i = 0; i < min(4, len); i++)
 		wdata0 |= *buf++ << (i * 8);
 
-	for (i = 4; i < min_t(int, 8, len); i++)
+	for (i = 4; i < min(8, len); i++)
 		wdata1 |= *buf++ << ((i - 4) * 8);
 
 	writel(wdata0, i2c->regs + REG_TOK_WDATA0);
@@ -186,7 +186,7 @@ static void meson_i2c_prepare_xfer(struct meson_i2c *i2c)
 	bool write = !(i2c->msg->flags & I2C_M_RD);
 	int i;
 
-	i2c->count = min_t(int, i2c->msg->len - i2c->pos, 8);
+	i2c->count = min(i2c->msg->len - i2c->pos, 8);
 
 	for (i = 0; i < i2c->count - 1; i++)
 		meson_i2c_add_token(i2c, TOKEN_DATA);
-- 
2.12.0

[PATCH v2 02/10] i2c: meson: remove member irq from struct meson_i2c

[Heiner Kallweit]

Member irq can be replaced with a local variable in probe
because it's nowhere else accessed.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Jerome Brunet <jbrunet@baylibre.com>
---
v2:
- added Reviewed-by
---
 drivers/i2c/busses/i2c-meson.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index 40e5da9a..50059d09 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -82,7 +82,6 @@ struct meson_i2c {
 	struct device		*dev;
 	void __iomem		*regs;
 	struct clk		*clk;
-	int			irq;
 
 	struct i2c_msg		*msg;
 	int			state;
@@ -391,7 +390,7 @@ static int meson_i2c_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	struct meson_i2c *i2c;
 	struct resource *mem;
-	int ret = 0;
+	int irq, ret = 0;
 
 	i2c = devm_kzalloc(&pdev->dev, sizeof(struct meson_i2c), GFP_KERNEL);
 	if (!i2c)
@@ -418,14 +417,14 @@ static int meson_i2c_probe(struct platform_device *pdev)
 	if (IS_ERR(i2c->regs))
 		return PTR_ERR(i2c->regs);
 
-	i2c->irq = platform_get_irq(pdev, 0);
-	if (i2c->irq < 0) {
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
 		dev_err(&pdev->dev, "can't find IRQ\n");
-		return i2c->irq;
+		return irq;
 	}
 
-	ret = devm_request_irq(&pdev->dev, i2c->irq, meson_i2c_irq,
-			       0, dev_name(&pdev->dev), i2c);
+	ret = devm_request_irq(&pdev->dev, irq, meson_i2c_irq, 0,
+			       dev_name(&pdev->dev), i2c);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "can't request IRQ\n");
 		return ret;
-- 
2.12.0

[PATCH v2 02/10] i2c: meson: remove member irq from struct meson_i2c

[Heiner Kallweit]

Member irq can be replaced with a local variable in probe
because it's nowhere else accessed.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Jerome Brunet <jbrunet@baylibre.com>
---
v2:
- added Reviewed-by
---
 drivers/i2c/busses/i2c-meson.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index 40e5da9a..50059d09 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -82,7 +82,6 @@ struct meson_i2c {
 	struct device		*dev;
 	void __iomem		*regs;
 	struct clk		*clk;
-	int			irq;
 
 	struct i2c_msg		*msg;
 	int			state;
@@ -391,7 +390,7 @@ static int meson_i2c_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	struct meson_i2c *i2c;
 	struct resource *mem;
-	int ret = 0;
+	int irq, ret = 0;
 
 	i2c = devm_kzalloc(&pdev->dev, sizeof(struct meson_i2c), GFP_KERNEL);
 	if (!i2c)
@@ -418,14 +417,14 @@ static int meson_i2c_probe(struct platform_device *pdev)
 	if (IS_ERR(i2c->regs))
 		return PTR_ERR(i2c->regs);
 
-	i2c->irq = platform_get_irq(pdev, 0);
-	if (i2c->irq < 0) {
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
 		dev_err(&pdev->dev, "can't find IRQ\n");
-		return i2c->irq;
+		return irq;
 	}
 
-	ret = devm_request_irq(&pdev->dev, i2c->irq, meson_i2c_irq,
-			       0, dev_name(&pdev->dev), i2c);
+	ret = devm_request_irq(&pdev->dev, irq, meson_i2c_irq, 0,
+			       dev_name(&pdev->dev), i2c);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "can't request IRQ\n");
 		return ret;
-- 
2.12.0

[PATCH v2 03/10] i2c: meson: use i2c core for DT clock-frequency parsing

[Heiner Kallweit]

We don't have to parse the DT manually to retrieve the bus frequency
and we don't have to maintain an own default for the bus frequency.
Let the i2c core do this for us.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Jerome Brunet <jbrunet@baylibre.com>
---
v2:
- added Reviewed-by
---
 drivers/i2c/busses/i2c-meson.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index 50059d09..5e243efa 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -38,7 +38,6 @@
 #define REG_CTRL_CLKDIV_MASK	((BIT(10) - 1) << REG_CTRL_CLKDIV_SHIFT)
 
 #define I2C_TIMEOUT_MS		500
-#define DEFAULT_FREQ		100000
 
 enum {
 	TOKEN_END = 0,
@@ -73,7 +72,7 @@ enum {
  * @error:	Flag set when an error is received
  * @lock:	To avoid race conditions between irq handler and xfer code
  * @done:	Completion used to wait for transfer termination
- * @frequency:	Operating frequency of I2C bus clock
+ * @timings:	Struct including the bus frequency
  * @tokens:	Sequence of tokens to be written to the device
  * @num_tokens:	Number of tokens
  */
@@ -92,7 +91,7 @@ struct meson_i2c {
 
 	spinlock_t		lock;
 	struct completion	done;
-	unsigned int		frequency;
+	struct i2c_timings	timings;
 	u32			tokens[2];
 	int			num_tokens;
 };
@@ -136,12 +135,12 @@ static void meson_i2c_set_clk_div(struct meson_i2c *i2c)
 	unsigned long clk_rate = clk_get_rate(i2c->clk);
 	unsigned int div;
 
-	div = DIV_ROUND_UP(clk_rate, i2c->frequency * 4);
+	div = DIV_ROUND_UP(clk_rate, i2c->timings.bus_freq_hz * 4);
 	meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_CLKDIV_MASK,
 			   div << REG_CTRL_CLKDIV_SHIFT);
 
 	dev_dbg(i2c->dev, "%s: clk %lu, freq %u, div %u\n", __func__,
-		clk_rate, i2c->frequency, div);
+		clk_rate, i2c->timings.bus_freq_hz, div);
 }
 
 static void meson_i2c_get_data(struct meson_i2c *i2c, char *buf, int len)
@@ -396,9 +395,7 @@ static int meson_i2c_probe(struct platform_device *pdev)
 	if (!i2c)
 		return -ENOMEM;
 
-	if (of_property_read_u32(pdev->dev.of_node, "clock-frequency",
-				 &i2c->frequency))
-		i2c->frequency = DEFAULT_FREQ;
+	i2c_parse_fw_timings(&pdev->dev, &i2c->timings, true);
 
 	i2c->dev = &pdev->dev;
 	platform_set_drvdata(pdev, i2c);
-- 
2.12.0

[PATCH v2 03/10] i2c: meson: use i2c core for DT clock-frequency parsing

[Heiner Kallweit]

We don't have to parse the DT manually to retrieve the bus frequency
and we don't have to maintain an own default for the bus frequency.
Let the i2c core do this for us.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Jerome Brunet <jbrunet@baylibre.com>
---
v2:
- added Reviewed-by
---
 drivers/i2c/busses/i2c-meson.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index 50059d09..5e243efa 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -38,7 +38,6 @@
 #define REG_CTRL_CLKDIV_MASK	((BIT(10) - 1) << REG_CTRL_CLKDIV_SHIFT)
 
 #define I2C_TIMEOUT_MS		500
-#define DEFAULT_FREQ		100000
 
 enum {
 	TOKEN_END = 0,
@@ -73,7 +72,7 @@ enum {
  * @error:	Flag set when an error is received
  * @lock:	To avoid race conditions between irq handler and xfer code
  * @done:	Completion used to wait for transfer termination
- * @frequency:	Operating frequency of I2C bus clock
+ * @timings:	Struct including the bus frequency
  * @tokens:	Sequence of tokens to be written to the device
  * @num_tokens:	Number of tokens
  */
@@ -92,7 +91,7 @@ struct meson_i2c {
 
 	spinlock_t		lock;
 	struct completion	done;
-	unsigned int		frequency;
+	struct i2c_timings	timings;
 	u32			tokens[2];
 	int			num_tokens;
 };
@@ -136,12 +135,12 @@ static void meson_i2c_set_clk_div(struct meson_i2c *i2c)
 	unsigned long clk_rate = clk_get_rate(i2c->clk);
 	unsigned int div;
 
-	div = DIV_ROUND_UP(clk_rate, i2c->frequency * 4);
+	div = DIV_ROUND_UP(clk_rate, i2c->timings.bus_freq_hz * 4);
 	meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_CLKDIV_MASK,
 			   div << REG_CTRL_CLKDIV_SHIFT);
 
 	dev_dbg(i2c->dev, "%s: clk %lu, freq %u, div %u\n", __func__,
-		clk_rate, i2c->frequency, div);
+		clk_rate, i2c->timings.bus_freq_hz, div);
 }
 
 static void meson_i2c_get_data(struct meson_i2c *i2c, char *buf, int len)
@@ -396,9 +395,7 @@ static int meson_i2c_probe(struct platform_device *pdev)
 	if (!i2c)
 		return -ENOMEM;
 
-	if (of_property_read_u32(pdev->dev.of_node, "clock-frequency",
-				 &i2c->frequency))
-		i2c->frequency = DEFAULT_FREQ;
+	i2c_parse_fw_timings(&pdev->dev, &i2c->timings, true);
 
 	i2c->dev = &pdev->dev;
 	platform_set_drvdata(pdev, i2c);
-- 
2.12.0

Re: [PATCH v2 03/10] i2c: meson: use i2c core for DT clock-frequency parsing

[Neil Armstrong]

On 03/09/2017 07:39 AM, Heiner Kallweit wrote:
> We don't have to parse the DT manually to retrieve the bus frequency
> and we don't have to maintain an own default for the bus frequency.
> Let the i2c core do this for us.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> Reviewed-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
> v2:
> - added Reviewed-by
> ---
>  drivers/i2c/busses/i2c-meson.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
> index 50059d09..5e243efa 100644
> --- a/drivers/i2c/busses/i2c-meson.c
> +++ b/drivers/i2c/busses/i2c-meson.c
> @@ -38,7 +38,6 @@
>  #define REG_CTRL_CLKDIV_MASK	((BIT(10) - 1) << REG_CTRL_CLKDIV_SHIFT)
>  
>  #define I2C_TIMEOUT_MS		500
> -#define DEFAULT_FREQ		100000
>  
>  enum {
>  	TOKEN_END = 0,
> @@ -73,7 +72,7 @@ enum {
>   * @error:	Flag set when an error is received
>   * @lock:	To avoid race conditions between irq handler and xfer code
>   * @done:	Completion used to wait for transfer termination
> - * @frequency:	Operating frequency of I2C bus clock
> + * @timings:	Struct including the bus frequency
>   * @tokens:	Sequence of tokens to be written to the device
>   * @num_tokens:	Number of tokens
>   */
> @@ -92,7 +91,7 @@ struct meson_i2c {
>  
>  	spinlock_t		lock;
>  	struct completion	done;
> -	unsigned int		frequency;
> +	struct i2c_timings	timings;
>  	u32			tokens[2];
>  	int			num_tokens;
>  };
> @@ -136,12 +135,12 @@ static void meson_i2c_set_clk_div(struct meson_i2c *i2c)
>  	unsigned long clk_rate = clk_get_rate(i2c->clk);
>  	unsigned int div;
>  
> -	div = DIV_ROUND_UP(clk_rate, i2c->frequency * 4);
> +	div = DIV_ROUND_UP(clk_rate, i2c->timings.bus_freq_hz * 4);
>  	meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_CLKDIV_MASK,
>  			   div << REG_CTRL_CLKDIV_SHIFT);
>  
>  	dev_dbg(i2c->dev, "%s: clk %lu, freq %u, div %u\n", __func__,
> -		clk_rate, i2c->frequency, div);
> +		clk_rate, i2c->timings.bus_freq_hz, div);
>  }
>  
>  static void meson_i2c_get_data(struct meson_i2c *i2c, char *buf, int len)
> @@ -396,9 +395,7 @@ static int meson_i2c_probe(struct platform_device *pdev)
>  	if (!i2c)
>  		return -ENOMEM;
>  
> -	if (of_property_read_u32(pdev->dev.of_node, "clock-frequency",
> -				 &i2c->frequency))
> -		i2c->frequency = DEFAULT_FREQ;
> +	i2c_parse_fw_timings(&pdev->dev, &i2c->timings, true);
>  
>  	i2c->dev = &pdev->dev;
>  	platform_set_drvdata(pdev, i2c);
> 

Hi Heiner,

Please update the bindings in Documentation/devicetree/bindings/i2c/i2c-meson.txt
for these new optional attributes by adding :

---><-----
Please also refer to i2c.txt in this directory for common i2c
controller binding usage.
---><-----

With this additional patch :
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

Neil

[PATCH v2 03/10] i2c: meson: use i2c core for DT clock-frequency parsing

[Neil Armstrong]

On 03/09/2017 07:39 AM, Heiner Kallweit wrote:
> We don't have to parse the DT manually to retrieve the bus frequency
> and we don't have to maintain an own default for the bus frequency.
> Let the i2c core do this for us.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> Reviewed-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
> v2:
> - added Reviewed-by
> ---
>  drivers/i2c/busses/i2c-meson.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
> index 50059d09..5e243efa 100644
> --- a/drivers/i2c/busses/i2c-meson.c
> +++ b/drivers/i2c/busses/i2c-meson.c
> @@ -38,7 +38,6 @@
>  #define REG_CTRL_CLKDIV_MASK	((BIT(10) - 1) << REG_CTRL_CLKDIV_SHIFT)
>  
>  #define I2C_TIMEOUT_MS		500
> -#define DEFAULT_FREQ		100000
>  
>  enum {
>  	TOKEN_END = 0,
> @@ -73,7 +72,7 @@ enum {
>   * @error:	Flag set when an error is received
>   * @lock:	To avoid race conditions between irq handler and xfer code
>   * @done:	Completion used to wait for transfer termination
> - * @frequency:	Operating frequency of I2C bus clock
> + * @timings:	Struct including the bus frequency
>   * @tokens:	Sequence of tokens to be written to the device
>   * @num_tokens:	Number of tokens
>   */
> @@ -92,7 +91,7 @@ struct meson_i2c {
>  
>  	spinlock_t		lock;
>  	struct completion	done;
> -	unsigned int		frequency;
> +	struct i2c_timings	timings;
>  	u32			tokens[2];
>  	int			num_tokens;
>  };
> @@ -136,12 +135,12 @@ static void meson_i2c_set_clk_div(struct meson_i2c *i2c)
>  	unsigned long clk_rate = clk_get_rate(i2c->clk);
>  	unsigned int div;
>  
> -	div = DIV_ROUND_UP(clk_rate, i2c->frequency * 4);
> +	div = DIV_ROUND_UP(clk_rate, i2c->timings.bus_freq_hz * 4);
>  	meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_CLKDIV_MASK,
>  			   div << REG_CTRL_CLKDIV_SHIFT);
>  
>  	dev_dbg(i2c->dev, "%s: clk %lu, freq %u, div %u\n", __func__,
> -		clk_rate, i2c->frequency, div);
> +		clk_rate, i2c->timings.bus_freq_hz, div);
>  }
>  
>  static void meson_i2c_get_data(struct meson_i2c *i2c, char *buf, int len)
> @@ -396,9 +395,7 @@ static int meson_i2c_probe(struct platform_device *pdev)
>  	if (!i2c)
>  		return -ENOMEM;
>  
> -	if (of_property_read_u32(pdev->dev.of_node, "clock-frequency",
> -				 &i2c->frequency))
> -		i2c->frequency = DEFAULT_FREQ;
> +	i2c_parse_fw_timings(&pdev->dev, &i2c->timings, true);
>  
>  	i2c->dev = &pdev->dev;
>  	platform_set_drvdata(pdev, i2c);
> 

Hi Heiner,

Please update the bindings in Documentation/devicetree/bindings/i2c/i2c-meson.txt
for these new optional attributes by adding :

---><-----
Please also refer to i2c.txt in this directory for common i2c
controller binding usage.
---><-----

With this additional patch :
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

Neil

Re: [PATCH v2 03/10] i2c: meson: use i2c core for DT clock-frequency parsing

[Heiner Kallweit]

Am 09.03.2017 um 09:26 schrieb Neil Armstrong:
> On 03/09/2017 07:39 AM, Heiner Kallweit wrote:
>> We don't have to parse the DT manually to retrieve the bus frequency
>> and we don't have to maintain an own default for the bus frequency.
>> Let the i2c core do this for us.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> Reviewed-by: Jerome Brunet <jbrunet@baylibre.com>
>> ---
>> v2:
>> - added Reviewed-by
>> ---
>>  drivers/i2c/busses/i2c-meson.c | 13 +++++--------
>>  1 file changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
>> index 50059d09..5e243efa 100644
>> --- a/drivers/i2c/busses/i2c-meson.c
>> +++ b/drivers/i2c/busses/i2c-meson.c
>> @@ -38,7 +38,6 @@
>>  #define REG_CTRL_CLKDIV_MASK	((BIT(10) - 1) << REG_CTRL_CLKDIV_SHIFT)
>>  
>>  #define I2C_TIMEOUT_MS		500
>> -#define DEFAULT_FREQ		100000
>>  
>>  enum {
>>  	TOKEN_END = 0,
>> @@ -73,7 +72,7 @@ enum {
>>   * @error:	Flag set when an error is received
>>   * @lock:	To avoid race conditions between irq handler and xfer code
>>   * @done:	Completion used to wait for transfer termination
>> - * @frequency:	Operating frequency of I2C bus clock
>> + * @timings:	Struct including the bus frequency
>>   * @tokens:	Sequence of tokens to be written to the device
>>   * @num_tokens:	Number of tokens
>>   */
>> @@ -92,7 +91,7 @@ struct meson_i2c {
>>  
>>  	spinlock_t		lock;
>>  	struct completion	done;
>> -	unsigned int		frequency;
>> +	struct i2c_timings	timings;
>>  	u32			tokens[2];
>>  	int			num_tokens;
>>  };
>> @@ -136,12 +135,12 @@ static void meson_i2c_set_clk_div(struct meson_i2c *i2c)
>>  	unsigned long clk_rate = clk_get_rate(i2c->clk);
>>  	unsigned int div;
>>  
>> -	div = DIV_ROUND_UP(clk_rate, i2c->frequency * 4);
>> +	div = DIV_ROUND_UP(clk_rate, i2c->timings.bus_freq_hz * 4);
>>  	meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_CLKDIV_MASK,
>>  			   div << REG_CTRL_CLKDIV_SHIFT);
>>  
>>  	dev_dbg(i2c->dev, "%s: clk %lu, freq %u, div %u\n", __func__,
>> -		clk_rate, i2c->frequency, div);
>> +		clk_rate, i2c->timings.bus_freq_hz, div);
>>  }
>>  
>>  static void meson_i2c_get_data(struct meson_i2c *i2c, char *buf, int len)
>> @@ -396,9 +395,7 @@ static int meson_i2c_probe(struct platform_device *pdev)
>>  	if (!i2c)
>>  		return -ENOMEM;
>>  
>> -	if (of_property_read_u32(pdev->dev.of_node, "clock-frequency",
>> -				 &i2c->frequency))
>> -		i2c->frequency = DEFAULT_FREQ;
>> +	i2c_parse_fw_timings(&pdev->dev, &i2c->timings, true);
>>  
>>  	i2c->dev = &pdev->dev;
>>  	platform_set_drvdata(pdev, i2c);
>>
> 
> Hi Heiner,
> 
> Please update the bindings in Documentation/devicetree/bindings/i2c/i2c-meson.txt
> for these new optional attributes by adding :
> 
Hi Neil,

not sure which new attributes you mean. i2c_parse_fw_timings parses additional
attributes (apart from bus frequency), that right. But they are not used by
the driver and therefore effectively aren't supported.

Heiner

> ---><-----
> Please also refer to i2c.txt in this directory for common i2c
> controller binding usage.
> ---><-----
> 
> With this additional patch :
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
> 
> Neil
> 

[PATCH v2 03/10] i2c: meson: use i2c core for DT clock-frequency parsing

[Heiner Kallweit]

Am 09.03.2017 um 09:26 schrieb Neil Armstrong:
> On 03/09/2017 07:39 AM, Heiner Kallweit wrote:
>> We don't have to parse the DT manually to retrieve the bus frequency
>> and we don't have to maintain an own default for the bus frequency.
>> Let the i2c core do this for us.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> Reviewed-by: Jerome Brunet <jbrunet@baylibre.com>
>> ---
>> v2:
>> - added Reviewed-by
>> ---
>>  drivers/i2c/busses/i2c-meson.c | 13 +++++--------
>>  1 file changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
>> index 50059d09..5e243efa 100644
>> --- a/drivers/i2c/busses/i2c-meson.c
>> +++ b/drivers/i2c/busses/i2c-meson.c
>> @@ -38,7 +38,6 @@
>>  #define REG_CTRL_CLKDIV_MASK	((BIT(10) - 1) << REG_CTRL_CLKDIV_SHIFT)
>>  
>>  #define I2C_TIMEOUT_MS		500
>> -#define DEFAULT_FREQ		100000
>>  
>>  enum {
>>  	TOKEN_END = 0,
>> @@ -73,7 +72,7 @@ enum {
>>   * @error:	Flag set when an error is received
>>   * @lock:	To avoid race conditions between irq handler and xfer code
>>   * @done:	Completion used to wait for transfer termination
>> - * @frequency:	Operating frequency of I2C bus clock
>> + * @timings:	Struct including the bus frequency
>>   * @tokens:	Sequence of tokens to be written to the device
>>   * @num_tokens:	Number of tokens
>>   */
>> @@ -92,7 +91,7 @@ struct meson_i2c {
>>  
>>  	spinlock_t		lock;
>>  	struct completion	done;
>> -	unsigned int		frequency;
>> +	struct i2c_timings	timings;
>>  	u32			tokens[2];
>>  	int			num_tokens;
>>  };
>> @@ -136,12 +135,12 @@ static void meson_i2c_set_clk_div(struct meson_i2c *i2c)
>>  	unsigned long clk_rate = clk_get_rate(i2c->clk);
>>  	unsigned int div;
>>  
>> -	div = DIV_ROUND_UP(clk_rate, i2c->frequency * 4);
>> +	div = DIV_ROUND_UP(clk_rate, i2c->timings.bus_freq_hz * 4);
>>  	meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_CLKDIV_MASK,
>>  			   div << REG_CTRL_CLKDIV_SHIFT);
>>  
>>  	dev_dbg(i2c->dev, "%s: clk %lu, freq %u, div %u\n", __func__,
>> -		clk_rate, i2c->frequency, div);
>> +		clk_rate, i2c->timings.bus_freq_hz, div);
>>  }
>>  
>>  static void meson_i2c_get_data(struct meson_i2c *i2c, char *buf, int len)
>> @@ -396,9 +395,7 @@ static int meson_i2c_probe(struct platform_device *pdev)
>>  	if (!i2c)
>>  		return -ENOMEM;
>>  
>> -	if (of_property_read_u32(pdev->dev.of_node, "clock-frequency",
>> -				 &i2c->frequency))
>> -		i2c->frequency = DEFAULT_FREQ;
>> +	i2c_parse_fw_timings(&pdev->dev, &i2c->timings, true);
>>  
>>  	i2c->dev = &pdev->dev;
>>  	platform_set_drvdata(pdev, i2c);
>>
> 
> Hi Heiner,
> 
> Please update the bindings in Documentation/devicetree/bindings/i2c/i2c-meson.txt
> for these new optional attributes by adding :
> 
Hi Neil,

not sure which new attributes you mean. i2c_parse_fw_timings parses additional
attributes (apart from bus frequency), that right. But they are not used by
the driver and therefore effectively aren't supported.

Heiner

> ---><-----
> Please also refer to i2c.txt in this directory for common i2c
> controller binding usage.
> ---><-----
> 
> With this additional patch :
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
> 
> Neil
> 

[PATCH v2 04/10] i2c: meson: use full 12 bits for clock divider

[Heiner Kallweit]

The clock divider has 12 bits, splitted into a 10 bit field and a
2 bit field. The extra 2 bits aren't used currently.

Change this to use the full 12 bits and warn if the requested
frequency is too low.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Acked-by: Jerome Brunet <jbrunet@baylibre.com>
---
v2:
- added Acked-by
---
 drivers/i2c/busses/i2c-meson.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index 5e243efa..a5764be5 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -35,7 +35,9 @@
 #define REG_CTRL_STATUS		BIT(2)
 #define REG_CTRL_ERROR		BIT(3)
 #define REG_CTRL_CLKDIV_SHIFT	12
-#define REG_CTRL_CLKDIV_MASK	((BIT(10) - 1) << REG_CTRL_CLKDIV_SHIFT)
+#define REG_CTRL_CLKDIV_MASK	GENMASK(21, 12)
+#define REG_CTRL_CLKDIVEXT_SHIFT 28
+#define REG_CTRL_CLKDIVEXT_MASK	GENMASK(29, 28)
 
 #define I2C_TIMEOUT_MS		500
 
@@ -136,8 +138,15 @@ static void meson_i2c_set_clk_div(struct meson_i2c *i2c)
 	unsigned int div;
 
 	div = DIV_ROUND_UP(clk_rate, i2c->timings.bus_freq_hz * 4);
+
+	/* clock divider has 12 bits */
+	WARN_ON(div >= (1 << 12));
+
 	meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_CLKDIV_MASK,
-			   div << REG_CTRL_CLKDIV_SHIFT);
+			   (div & GENMASK(9, 0)) << REG_CTRL_CLKDIV_SHIFT);
+
+	meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_CLKDIVEXT_MASK,
+			   (div >> 10) << REG_CTRL_CLKDIVEXT_SHIFT);
 
 	dev_dbg(i2c->dev, "%s: clk %lu, freq %u, div %u\n", __func__,
 		clk_rate, i2c->timings.bus_freq_hz, div);
-- 
2.12.0

[PATCH v2 04/10] i2c: meson: use full 12 bits for clock divider

[Heiner Kallweit]

The clock divider has 12 bits, splitted into a 10 bit field and a
2 bit field. The extra 2 bits aren't used currently.

Change this to use the full 12 bits and warn if the requested
frequency is too low.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Acked-by: Jerome Brunet <jbrunet@baylibre.com>
---
v2:
- added Acked-by
---
 drivers/i2c/busses/i2c-meson.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index 5e243efa..a5764be5 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -35,7 +35,9 @@
 #define REG_CTRL_STATUS		BIT(2)
 #define REG_CTRL_ERROR		BIT(3)
 #define REG_CTRL_CLKDIV_SHIFT	12
-#define REG_CTRL_CLKDIV_MASK	((BIT(10) - 1) << REG_CTRL_CLKDIV_SHIFT)
+#define REG_CTRL_CLKDIV_MASK	GENMASK(21, 12)
+#define REG_CTRL_CLKDIVEXT_SHIFT 28
+#define REG_CTRL_CLKDIVEXT_MASK	GENMASK(29, 28)
 
 #define I2C_TIMEOUT_MS		500
 
@@ -136,8 +138,15 @@ static void meson_i2c_set_clk_div(struct meson_i2c *i2c)
 	unsigned int div;
 
 	div = DIV_ROUND_UP(clk_rate, i2c->timings.bus_freq_hz * 4);
+
+	/* clock divider has 12 bits */
+	WARN_ON(div >= (1 << 12));
+
 	meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_CLKDIV_MASK,
-			   div << REG_CTRL_CLKDIV_SHIFT);
+			   (div & GENMASK(9, 0)) << REG_CTRL_CLKDIV_SHIFT);
+
+	meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_CLKDIVEXT_MASK,
+			   (div >> 10) << REG_CTRL_CLKDIVEXT_SHIFT);
 
 	dev_dbg(i2c->dev, "%s: clk %lu, freq %u, div %u\n", __func__,
 		clk_rate, i2c->timings.bus_freq_hz, div);
-- 
2.12.0

[PATCH v2 05/10] i2c: meson: set clock divider in probe instead of setting it for each transfer

[Heiner Kallweit]

The bus frequency won't change, therefore we can set the clock divider
in probe already and we don't have to set it for each transfer.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- no changes
---
 drivers/i2c/busses/i2c-meson.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index a5764be5..594fec22 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -74,7 +74,6 @@ enum {
  * @error:	Flag set when an error is received
  * @lock:	To avoid race conditions between irq handler and xfer code
  * @done:	Completion used to wait for transfer termination
- * @timings:	Struct including the bus frequency
  * @tokens:	Sequence of tokens to be written to the device
  * @num_tokens:	Number of tokens
  */
@@ -93,7 +92,6 @@ struct meson_i2c {
 
 	spinlock_t		lock;
 	struct completion	done;
-	struct i2c_timings	timings;
 	u32			tokens[2];
 	int			num_tokens;
 };
@@ -132,12 +130,12 @@ static void meson_i2c_write_tokens(struct meson_i2c *i2c)
 	writel(i2c->tokens[1], i2c->regs + REG_TOK_LIST1);
 }
 
-static void meson_i2c_set_clk_div(struct meson_i2c *i2c)
+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, i2c->timings.bus_freq_hz * 4);
+	div = DIV_ROUND_UP(clk_rate, freq * 4);
 
 	/* clock divider has 12 bits */
 	WARN_ON(div >= (1 << 12));
@@ -149,7 +147,7 @@ static void meson_i2c_set_clk_div(struct meson_i2c *i2c)
 			   (div >> 10) << REG_CTRL_CLKDIVEXT_SHIFT);
 
 	dev_dbg(i2c->dev, "%s: clk %lu, freq %u, div %u\n", __func__,
-		clk_rate, i2c->timings.bus_freq_hz, div);
+		clk_rate, freq, div);
 }
 
 static void meson_i2c_get_data(struct meson_i2c *i2c, char *buf, int len)
@@ -369,7 +367,6 @@ static int meson_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 	int i, ret = 0, count = 0;
 
 	clk_enable(i2c->clk);
-	meson_i2c_set_clk_div(i2c);
 
 	for (i = 0; i < num; i++) {
 		ret = meson_i2c_xfer_msg(i2c, msgs + i, i == num - 1);
@@ -396,6 +393,7 @@ static const struct i2c_algorithm meson_i2c_algorithm = {
 static int meson_i2c_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
+	struct i2c_timings timings;
 	struct meson_i2c *i2c;
 	struct resource *mem;
 	int irq, ret = 0;
@@ -404,7 +402,7 @@ static int meson_i2c_probe(struct platform_device *pdev)
 	if (!i2c)
 		return -ENOMEM;
 
-	i2c_parse_fw_timings(&pdev->dev, &i2c->timings, true);
+	i2c_parse_fw_timings(&pdev->dev, &timings, true);
 
 	i2c->dev = &pdev->dev;
 	platform_set_drvdata(pdev, i2c);
@@ -462,6 +460,8 @@ static int meson_i2c_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	meson_i2c_set_clk_div(i2c, timings.bus_freq_hz);
+
 	return 0;
 }
 
-- 
2.12.0

[PATCH v2 05/10] i2c: meson: set clock divider in probe instead of setting it for each transfer

[Heiner Kallweit]

The bus frequency won't change, therefore we can set the clock divider
in probe already and we don't have to set it for each transfer.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- no changes
---
 drivers/i2c/busses/i2c-meson.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index a5764be5..594fec22 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -74,7 +74,6 @@ enum {
  * @error:	Flag set when an error is received
  * @lock:	To avoid race conditions between irq handler and xfer code
  * @done:	Completion used to wait for transfer termination
- * @timings:	Struct including the bus frequency
  * @tokens:	Sequence of tokens to be written to the device
  * @num_tokens:	Number of tokens
  */
@@ -93,7 +92,6 @@ struct meson_i2c {
 
 	spinlock_t		lock;
 	struct completion	done;
-	struct i2c_timings	timings;
 	u32			tokens[2];
 	int			num_tokens;
 };
@@ -132,12 +130,12 @@ static void meson_i2c_write_tokens(struct meson_i2c *i2c)
 	writel(i2c->tokens[1], i2c->regs + REG_TOK_LIST1);
 }
 
-static void meson_i2c_set_clk_div(struct meson_i2c *i2c)
+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, i2c->timings.bus_freq_hz * 4);
+	div = DIV_ROUND_UP(clk_rate, freq * 4);
 
 	/* clock divider has 12 bits */
 	WARN_ON(div >= (1 << 12));
@@ -149,7 +147,7 @@ static void meson_i2c_set_clk_div(struct meson_i2c *i2c)
 			   (div >> 10) << REG_CTRL_CLKDIVEXT_SHIFT);
 
 	dev_dbg(i2c->dev, "%s: clk %lu, freq %u, div %u\n", __func__,
-		clk_rate, i2c->timings.bus_freq_hz, div);
+		clk_rate, freq, div);
 }
 
 static void meson_i2c_get_data(struct meson_i2c *i2c, char *buf, int len)
@@ -369,7 +367,6 @@ static int meson_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 	int i, ret = 0, count = 0;
 
 	clk_enable(i2c->clk);
-	meson_i2c_set_clk_div(i2c);
 
 	for (i = 0; i < num; i++) {
 		ret = meson_i2c_xfer_msg(i2c, msgs + i, i == num - 1);
@@ -396,6 +393,7 @@ static const struct i2c_algorithm meson_i2c_algorithm = {
 static int meson_i2c_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
+	struct i2c_timings timings;
 	struct meson_i2c *i2c;
 	struct resource *mem;
 	int irq, ret = 0;
@@ -404,7 +402,7 @@ static int meson_i2c_probe(struct platform_device *pdev)
 	if (!i2c)
 		return -ENOMEM;
 
-	i2c_parse_fw_timings(&pdev->dev, &i2c->timings, true);
+	i2c_parse_fw_timings(&pdev->dev, &timings, true);
 
 	i2c->dev = &pdev->dev;
 	platform_set_drvdata(pdev, i2c);
@@ -462,6 +460,8 @@ static int meson_i2c_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	meson_i2c_set_clk_div(i2c, timings.bus_freq_hz);
+
 	return 0;
 }
 
-- 
2.12.0

Re: [PATCH v2 05/10] i2c: meson: set clock divider in probe instead of setting it for each transfer

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> The bus frequency won't change, therefore we can set the clock divider
> in probe already and we don't have to set it for each transfer.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> v2:
> - no changes
> ---
>  drivers/i2c/busses/i2c-meson.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
> index a5764be5..594fec22 100644
> --- a/drivers/i2c/busses/i2c-meson.c
> +++ b/drivers/i2c/busses/i2c-meson.c
> @@ -74,7 +74,6 @@ enum {
>   * @error:	Flag set when an error is received
>   * @lock:	To avoid race conditions between irq handler and xfer code
>   * @done:	Completion used to wait for transfer termination
> - * @timings:	Struct including the bus frequency

You just added this field in PATCH 03/10.  Why not do this patch
first, which will simplify the other.

Kevin

>   * @tokens:	Sequence of tokens to be written to the device
>   * @num_tokens:	Number of tokens
>   */
> @@ -93,7 +92,6 @@ struct meson_i2c {
>  
>  	spinlock_t		lock;
>  	struct completion	done;
> -	struct i2c_timings	timings;
>  	u32			tokens[2];
>  	int			num_tokens;
>  };
> @@ -132,12 +130,12 @@ static void meson_i2c_write_tokens(struct meson_i2c *i2c)
>  	writel(i2c->tokens[1], i2c->regs + REG_TOK_LIST1);
>  }
>  
> -static void meson_i2c_set_clk_div(struct meson_i2c *i2c)
> +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, i2c->timings.bus_freq_hz * 4);
> +	div = DIV_ROUND_UP(clk_rate, freq * 4);
>  
>  	/* clock divider has 12 bits */
>  	WARN_ON(div >= (1 << 12));
> @@ -149,7 +147,7 @@ static void meson_i2c_set_clk_div(struct meson_i2c *i2c)
>  			   (div >> 10) << REG_CTRL_CLKDIVEXT_SHIFT);
>  
>  	dev_dbg(i2c->dev, "%s: clk %lu, freq %u, div %u\n", __func__,
> -		clk_rate, i2c->timings.bus_freq_hz, div);
> +		clk_rate, freq, div);
>  }
>  
>  static void meson_i2c_get_data(struct meson_i2c *i2c, char *buf, int len)
> @@ -369,7 +367,6 @@ static int meson_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
>  	int i, ret = 0, count = 0;
>  
>  	clk_enable(i2c->clk);
> -	meson_i2c_set_clk_div(i2c);
>  
>  	for (i = 0; i < num; i++) {
>  		ret = meson_i2c_xfer_msg(i2c, msgs + i, i == num - 1);
> @@ -396,6 +393,7 @@ static const struct i2c_algorithm meson_i2c_algorithm = {
>  static int meson_i2c_probe(struct platform_device *pdev)
>  {
>  	struct device_node *np = pdev->dev.of_node;
> +	struct i2c_timings timings;
>  	struct meson_i2c *i2c;
>  	struct resource *mem;
>  	int irq, ret = 0;
> @@ -404,7 +402,7 @@ static int meson_i2c_probe(struct platform_device *pdev)
>  	if (!i2c)
>  		return -ENOMEM;
>  
> -	i2c_parse_fw_timings(&pdev->dev, &i2c->timings, true);
> +	i2c_parse_fw_timings(&pdev->dev, &timings, true);
>  
>  	i2c->dev = &pdev->dev;
>  	platform_set_drvdata(pdev, i2c);
> @@ -462,6 +460,8 @@ static int meson_i2c_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> +	meson_i2c_set_clk_div(i2c, timings.bus_freq_hz);
> +
>  	return 0;
>  }

[PATCH v2 05/10] i2c: meson: set clock divider in probe instead of setting it for each transfer

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> The bus frequency won't change, therefore we can set the clock divider
> in probe already and we don't have to set it for each transfer.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> v2:
> - no changes
> ---
>  drivers/i2c/busses/i2c-meson.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
> index a5764be5..594fec22 100644
> --- a/drivers/i2c/busses/i2c-meson.c
> +++ b/drivers/i2c/busses/i2c-meson.c
> @@ -74,7 +74,6 @@ enum {
>   * @error:	Flag set when an error is received
>   * @lock:	To avoid race conditions between irq handler and xfer code
>   * @done:	Completion used to wait for transfer termination
> - * @timings:	Struct including the bus frequency

You just added this field in PATCH 03/10.  Why not do this patch
first, which will simplify the other.

Kevin

>   * @tokens:	Sequence of tokens to be written to the device
>   * @num_tokens:	Number of tokens
>   */
> @@ -93,7 +92,6 @@ struct meson_i2c {
>  
>  	spinlock_t		lock;
>  	struct completion	done;
> -	struct i2c_timings	timings;
>  	u32			tokens[2];
>  	int			num_tokens;
>  };
> @@ -132,12 +130,12 @@ static void meson_i2c_write_tokens(struct meson_i2c *i2c)
>  	writel(i2c->tokens[1], i2c->regs + REG_TOK_LIST1);
>  }
>  
> -static void meson_i2c_set_clk_div(struct meson_i2c *i2c)
> +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, i2c->timings.bus_freq_hz * 4);
> +	div = DIV_ROUND_UP(clk_rate, freq * 4);
>  
>  	/* clock divider has 12 bits */
>  	WARN_ON(div >= (1 << 12));
> @@ -149,7 +147,7 @@ static void meson_i2c_set_clk_div(struct meson_i2c *i2c)
>  			   (div >> 10) << REG_CTRL_CLKDIVEXT_SHIFT);
>  
>  	dev_dbg(i2c->dev, "%s: clk %lu, freq %u, div %u\n", __func__,
> -		clk_rate, i2c->timings.bus_freq_hz, div);
> +		clk_rate, freq, div);
>  }
>  
>  static void meson_i2c_get_data(struct meson_i2c *i2c, char *buf, int len)
> @@ -369,7 +367,6 @@ static int meson_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
>  	int i, ret = 0, count = 0;
>  
>  	clk_enable(i2c->clk);
> -	meson_i2c_set_clk_div(i2c);
>  
>  	for (i = 0; i < num; i++) {
>  		ret = meson_i2c_xfer_msg(i2c, msgs + i, i == num - 1);
> @@ -396,6 +393,7 @@ static const struct i2c_algorithm meson_i2c_algorithm = {
>  static int meson_i2c_probe(struct platform_device *pdev)
>  {
>  	struct device_node *np = pdev->dev.of_node;
> +	struct i2c_timings timings;
>  	struct meson_i2c *i2c;
>  	struct resource *mem;
>  	int irq, ret = 0;
> @@ -404,7 +402,7 @@ static int meson_i2c_probe(struct platform_device *pdev)
>  	if (!i2c)
>  		return -ENOMEM;
>  
> -	i2c_parse_fw_timings(&pdev->dev, &i2c->timings, true);
> +	i2c_parse_fw_timings(&pdev->dev, &timings, true);
>  
>  	i2c->dev = &pdev->dev;
>  	platform_set_drvdata(pdev, i2c);
> @@ -462,6 +460,8 @@ static int meson_i2c_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> +	meson_i2c_set_clk_div(i2c, timings.bus_freq_hz);
> +
>  	return 0;
>  }

Re: [PATCH v2 05/10] i2c: meson: set clock divider in probe instead of setting it for each transfer

[Heiner Kallweit]

Am 11.03.2017 um 00:17 schrieb Kevin Hilman:
> Heiner Kallweit <hkallweit1@gmail.com> writes:
> 
>> The bus frequency won't change, therefore we can set the clock divider
>> in probe already and we don't have to set it for each transfer.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>> v2:
>> - no changes
>> ---
>>  drivers/i2c/busses/i2c-meson.c | 14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
>> index a5764be5..594fec22 100644
>> --- a/drivers/i2c/busses/i2c-meson.c
>> +++ b/drivers/i2c/busses/i2c-meson.c
>> @@ -74,7 +74,6 @@ enum {
>>   * @error:	Flag set when an error is received
>>   * @lock:	To avoid race conditions between irq handler and xfer code
>>   * @done:	Completion used to wait for transfer termination
>> - * @timings:	Struct including the bus frequency
> 
> You just added this field in PATCH 03/10.  Why not do this patch
> first, which will simplify the other.
> 
Yes, most likely this would be better. Will keep this in mind for the
upcoming v3.

> Kevin
> 
>>   * @tokens:	Sequence of tokens to be written to the device
>>   * @num_tokens:	Number of tokens
>>   */
>> @@ -93,7 +92,6 @@ struct meson_i2c {
>>  
>>  	spinlock_t		lock;
>>  	struct completion	done;
>> -	struct i2c_timings	timings;
>>  	u32			tokens[2];
>>  	int			num_tokens;
>>  };
>> @@ -132,12 +130,12 @@ static void meson_i2c_write_tokens(struct meson_i2c *i2c)
>>  	writel(i2c->tokens[1], i2c->regs + REG_TOK_LIST1);
>>  }
>>  
>> -static void meson_i2c_set_clk_div(struct meson_i2c *i2c)
>> +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, i2c->timings.bus_freq_hz * 4);
>> +	div = DIV_ROUND_UP(clk_rate, freq * 4);
>>  
>>  	/* clock divider has 12 bits */
>>  	WARN_ON(div >= (1 << 12));
>> @@ -149,7 +147,7 @@ static void meson_i2c_set_clk_div(struct meson_i2c *i2c)
>>  			   (div >> 10) << REG_CTRL_CLKDIVEXT_SHIFT);
>>  
>>  	dev_dbg(i2c->dev, "%s: clk %lu, freq %u, div %u\n", __func__,
>> -		clk_rate, i2c->timings.bus_freq_hz, div);
>> +		clk_rate, freq, div);
>>  }
>>  
>>  static void meson_i2c_get_data(struct meson_i2c *i2c, char *buf, int len)
>> @@ -369,7 +367,6 @@ static int meson_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
>>  	int i, ret = 0, count = 0;
>>  
>>  	clk_enable(i2c->clk);
>> -	meson_i2c_set_clk_div(i2c);
>>  
>>  	for (i = 0; i < num; i++) {
>>  		ret = meson_i2c_xfer_msg(i2c, msgs + i, i == num - 1);
>> @@ -396,6 +393,7 @@ static const struct i2c_algorithm meson_i2c_algorithm = {
>>  static int meson_i2c_probe(struct platform_device *pdev)
>>  {
>>  	struct device_node *np = pdev->dev.of_node;
>> +	struct i2c_timings timings;
>>  	struct meson_i2c *i2c;
>>  	struct resource *mem;
>>  	int irq, ret = 0;
>> @@ -404,7 +402,7 @@ static int meson_i2c_probe(struct platform_device *pdev)
>>  	if (!i2c)
>>  		return -ENOMEM;
>>  
>> -	i2c_parse_fw_timings(&pdev->dev, &i2c->timings, true);
>> +	i2c_parse_fw_timings(&pdev->dev, &timings, true);
>>  
>>  	i2c->dev = &pdev->dev;
>>  	platform_set_drvdata(pdev, i2c);
>> @@ -462,6 +460,8 @@ static int meson_i2c_probe(struct platform_device *pdev)
>>  		return ret;
>>  	}
>>  
>> +	meson_i2c_set_clk_div(i2c, timings.bus_freq_hz);
>> +
>>  	return 0;
>>  }
> 

[PATCH v2 05/10] i2c: meson: set clock divider in probe instead of setting it for each transfer

[Heiner Kallweit]

Am 11.03.2017 um 00:17 schrieb Kevin Hilman:
> Heiner Kallweit <hkallweit1@gmail.com> writes:
> 
>> The bus frequency won't change, therefore we can set the clock divider
>> in probe already and we don't have to set it for each transfer.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>> v2:
>> - no changes
>> ---
>>  drivers/i2c/busses/i2c-meson.c | 14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
>> index a5764be5..594fec22 100644
>> --- a/drivers/i2c/busses/i2c-meson.c
>> +++ b/drivers/i2c/busses/i2c-meson.c
>> @@ -74,7 +74,6 @@ enum {
>>   * @error:	Flag set when an error is received
>>   * @lock:	To avoid race conditions between irq handler and xfer code
>>   * @done:	Completion used to wait for transfer termination
>> - * @timings:	Struct including the bus frequency
> 
> You just added this field in PATCH 03/10.  Why not do this patch
> first, which will simplify the other.
> 
Yes, most likely this would be better. Will keep this in mind for the
upcoming v3.

> Kevin
> 
>>   * @tokens:	Sequence of tokens to be written to the device
>>   * @num_tokens:	Number of tokens
>>   */
>> @@ -93,7 +92,6 @@ struct meson_i2c {
>>  
>>  	spinlock_t		lock;
>>  	struct completion	done;
>> -	struct i2c_timings	timings;
>>  	u32			tokens[2];
>>  	int			num_tokens;
>>  };
>> @@ -132,12 +130,12 @@ static void meson_i2c_write_tokens(struct meson_i2c *i2c)
>>  	writel(i2c->tokens[1], i2c->regs + REG_TOK_LIST1);
>>  }
>>  
>> -static void meson_i2c_set_clk_div(struct meson_i2c *i2c)
>> +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, i2c->timings.bus_freq_hz * 4);
>> +	div = DIV_ROUND_UP(clk_rate, freq * 4);
>>  
>>  	/* clock divider has 12 bits */
>>  	WARN_ON(div >= (1 << 12));
>> @@ -149,7 +147,7 @@ static void meson_i2c_set_clk_div(struct meson_i2c *i2c)
>>  			   (div >> 10) << REG_CTRL_CLKDIVEXT_SHIFT);
>>  
>>  	dev_dbg(i2c->dev, "%s: clk %lu, freq %u, div %u\n", __func__,
>> -		clk_rate, i2c->timings.bus_freq_hz, div);
>> +		clk_rate, freq, div);
>>  }
>>  
>>  static void meson_i2c_get_data(struct meson_i2c *i2c, char *buf, int len)
>> @@ -369,7 +367,6 @@ static int meson_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
>>  	int i, ret = 0, count = 0;
>>  
>>  	clk_enable(i2c->clk);
>> -	meson_i2c_set_clk_div(i2c);
>>  
>>  	for (i = 0; i < num; i++) {
>>  		ret = meson_i2c_xfer_msg(i2c, msgs + i, i == num - 1);
>> @@ -396,6 +393,7 @@ static const struct i2c_algorithm meson_i2c_algorithm = {
>>  static int meson_i2c_probe(struct platform_device *pdev)
>>  {
>>  	struct device_node *np = pdev->dev.of_node;
>> +	struct i2c_timings timings;
>>  	struct meson_i2c *i2c;
>>  	struct resource *mem;
>>  	int irq, ret = 0;
>> @@ -404,7 +402,7 @@ static int meson_i2c_probe(struct platform_device *pdev)
>>  	if (!i2c)
>>  		return -ENOMEM;
>>  
>> -	i2c_parse_fw_timings(&pdev->dev, &i2c->timings, true);
>> +	i2c_parse_fw_timings(&pdev->dev, &timings, true);
>>  
>>  	i2c->dev = &pdev->dev;
>>  	platform_set_drvdata(pdev, i2c);
>> @@ -462,6 +460,8 @@ static int meson_i2c_probe(struct platform_device *pdev)
>>  		return ret;
>>  	}
>>  
>> +	meson_i2c_set_clk_div(i2c, timings.bus_freq_hz);
>> +
>>  	return 0;
>>  }
> 

[PATCH v2 06/10] i2c: meson: remove variable count from meson_i2c_xfer

[Heiner Kallweit]

Variable count has always the same value as i, so we don't need it.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- remove one small change from v1
---
 drivers/i2c/busses/i2c-meson.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index 594fec22..56e15a74 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -364,7 +364,7 @@ static int meson_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 			  int num)
 {
 	struct meson_i2c *i2c = adap->algo_data;
-	int i, ret = 0, count = 0;
+	int i, ret = 0;
 
 	clk_enable(i2c->clk);
 
@@ -372,12 +372,11 @@ static int meson_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 		ret = meson_i2c_xfer_msg(i2c, msgs + i, i == num - 1);
 		if (ret)
 			break;
-		count++;
 	}
 
 	clk_disable(i2c->clk);
 
-	return ret ? ret : count;
+	return ret ?: i;
 }
 
 static u32 meson_i2c_func(struct i2c_adapter *adap)
-- 
2.12.0

[PATCH v2 06/10] i2c: meson: remove variable count from meson_i2c_xfer

[Heiner Kallweit]

Variable count has always the same value as i, so we don't need it.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- remove one small change from v1
---
 drivers/i2c/busses/i2c-meson.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index 594fec22..56e15a74 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -364,7 +364,7 @@ static int meson_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 			  int num)
 {
 	struct meson_i2c *i2c = adap->algo_data;
-	int i, ret = 0, count = 0;
+	int i, ret = 0;
 
 	clk_enable(i2c->clk);
 
@@ -372,12 +372,11 @@ static int meson_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 		ret = meson_i2c_xfer_msg(i2c, msgs + i, i == num - 1);
 		if (ret)
 			break;
-		count++;
 	}
 
 	clk_disable(i2c->clk);
 
-	return ret ? ret : count;
+	return ret ?: i;
 }
 
 static u32 meson_i2c_func(struct i2c_adapter *adap)
-- 
2.12.0

[PATCH v2 07/10] i2c: meson: improve interrupt handler and detect spurious interrupts

[Heiner Kallweit]

If state is STATE_IDLE no interrupt should occur. Detect this case and
warn.
In addition move resetting REG_CTRL_START bit to the start of the
interrupt handler and remove a unneeded REG_CTRL_START bit reset
in meson_i2c_xfer_msg.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- don't print an error if spurious interrupt is detected
---
 drivers/i2c/busses/i2c-meson.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index 56e15a74..74d433cd 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -228,12 +228,18 @@ static irqreturn_t meson_i2c_irq(int irqno, void *dev_id)
 	spin_lock(&i2c->lock);
 
 	meson_i2c_reset_tokens(i2c);
+	meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_START, 0);
 	ctrl = readl(i2c->regs + REG_CTRL);
 
 	dev_dbg(i2c->dev, "irq: state %d, pos %d, count %d, ctrl %08x\n",
 		i2c->state, i2c->pos, i2c->count, ctrl);
 
-	if (ctrl & REG_CTRL_ERROR && i2c->state != STATE_IDLE) {
+	if (i2c->state == STATE_IDLE) {
+		spin_unlock(&i2c->lock);
+		return IRQ_NONE;
+	}
+
+	if (ctrl & REG_CTRL_ERROR) {
 		/*
 		 * The bit is set when the IGNORE_NAK bit is cleared
 		 * and the device didn't respond. In this case, the
@@ -276,15 +282,12 @@ static irqreturn_t meson_i2c_irq(int irqno, void *dev_id)
 		i2c->state = STATE_IDLE;
 		complete(&i2c->done);
 		break;
-	case STATE_IDLE:
-		break;
 	}
 
 out:
 	if (i2c->state != STATE_IDLE) {
 		/* Restart the processing */
 		meson_i2c_write_tokens(i2c);
-		meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_START, 0);
 		meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_START,
 				   REG_CTRL_START);
 	}
@@ -344,9 +347,6 @@ static int meson_i2c_xfer_msg(struct meson_i2c *i2c, struct i2c_msg *msg,
 	 */
 	spin_lock_irqsave(&i2c->lock, flags);
 
-	/* Abort any active operation */
-	meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_START, 0);
-
 	if (!time_left) {
 		i2c->state = STATE_IDLE;
 		ret = -ETIMEDOUT;
-- 
2.12.0

[PATCH v2 07/10] i2c: meson: improve interrupt handler and detect spurious interrupts

[Heiner Kallweit]

If state is STATE_IDLE no interrupt should occur. Detect this case and
warn.
In addition move resetting REG_CTRL_START bit to the start of the
interrupt handler and remove a unneeded REG_CTRL_START bit reset
in meson_i2c_xfer_msg.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- don't print an error if spurious interrupt is detected
---
 drivers/i2c/busses/i2c-meson.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index 56e15a74..74d433cd 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -228,12 +228,18 @@ static irqreturn_t meson_i2c_irq(int irqno, void *dev_id)
 	spin_lock(&i2c->lock);
 
 	meson_i2c_reset_tokens(i2c);
+	meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_START, 0);
 	ctrl = readl(i2c->regs + REG_CTRL);
 
 	dev_dbg(i2c->dev, "irq: state %d, pos %d, count %d, ctrl %08x\n",
 		i2c->state, i2c->pos, i2c->count, ctrl);
 
-	if (ctrl & REG_CTRL_ERROR && i2c->state != STATE_IDLE) {
+	if (i2c->state == STATE_IDLE) {
+		spin_unlock(&i2c->lock);
+		return IRQ_NONE;
+	}
+
+	if (ctrl & REG_CTRL_ERROR) {
 		/*
 		 * The bit is set when the IGNORE_NAK bit is cleared
 		 * and the device didn't respond. In this case, the
@@ -276,15 +282,12 @@ static irqreturn_t meson_i2c_irq(int irqno, void *dev_id)
 		i2c->state = STATE_IDLE;
 		complete(&i2c->done);
 		break;
-	case STATE_IDLE:
-		break;
 	}
 
 out:
 	if (i2c->state != STATE_IDLE) {
 		/* Restart the processing */
 		meson_i2c_write_tokens(i2c);
-		meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_START, 0);
 		meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_START,
 				   REG_CTRL_START);
 	}
@@ -344,9 +347,6 @@ static int meson_i2c_xfer_msg(struct meson_i2c *i2c, struct i2c_msg *msg,
 	 */
 	spin_lock_irqsave(&i2c->lock, flags);
 
-	/* Abort any active operation */
-	meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_START, 0);
-
 	if (!time_left) {
 		i2c->state = STATE_IDLE;
 		ret = -ETIMEDOUT;
-- 
2.12.0

[PATCH v2 08/10] i2c: meson: don't create separate token chain just for the stop command

[Heiner Kallweit]

We can directly add the stop token to the token chain including the
last transfer chunk. This is more efficient than creating a separate
token chain just for the stop command.
And it allows us to get rid of state STATE_STOP completely.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- rebased
---
 drivers/i2c/busses/i2c-meson.c | 23 +++++------------------
 1 file changed, 5 insertions(+), 18 deletions(-)

diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index 74d433cd..3e6f5e50 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -55,7 +55,6 @@ enum {
 	STATE_IDLE,
 	STATE_READ,
 	STATE_WRITE,
-	STATE_STOP,
 };
 
 /**
@@ -205,19 +204,9 @@ static void meson_i2c_prepare_xfer(struct meson_i2c *i2c)
 
 	if (write)
 		meson_i2c_put_data(i2c, i2c->msg->buf + i2c->pos, i2c->count);
-}
-
-static void meson_i2c_stop(struct meson_i2c *i2c)
-{
-	dev_dbg(i2c->dev, "%s: last %d\n", __func__, i2c->last);
 
-	if (i2c->last) {
-		i2c->state = STATE_STOP;
+	if (i2c->last && i2c->pos + i2c->count >= i2c->msg->len)
 		meson_i2c_add_token(i2c, TOKEN_STOP);
-	} else {
-		i2c->state = STATE_IDLE;
-		complete(&i2c->done);
-	}
 }
 
 static irqreturn_t meson_i2c_irq(int irqno, void *dev_id)
@@ -262,7 +251,8 @@ static irqreturn_t meson_i2c_irq(int irqno, void *dev_id)
 		}
 
 		if (i2c->pos >= i2c->msg->len) {
-			meson_i2c_stop(i2c);
+			i2c->state = STATE_IDLE;
+			complete(&i2c->done);
 			break;
 		}
 
@@ -272,16 +262,13 @@ static irqreturn_t meson_i2c_irq(int irqno, void *dev_id)
 		i2c->pos += i2c->count;
 
 		if (i2c->pos >= i2c->msg->len) {
-			meson_i2c_stop(i2c);
+			i2c->state = STATE_IDLE;
+			complete(&i2c->done);
 			break;
 		}
 
 		meson_i2c_prepare_xfer(i2c);
 		break;
-	case STATE_STOP:
-		i2c->state = STATE_IDLE;
-		complete(&i2c->done);
-		break;
 	}
 
 out:
-- 
2.12.0

[PATCH v2 08/10] i2c: meson: don't create separate token chain just for the stop command

[Heiner Kallweit]

We can directly add the stop token to the token chain including the
last transfer chunk. This is more efficient than creating a separate
token chain just for the stop command.
And it allows us to get rid of state STATE_STOP completely.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- rebased
---
 drivers/i2c/busses/i2c-meson.c | 23 +++++------------------
 1 file changed, 5 insertions(+), 18 deletions(-)

diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index 74d433cd..3e6f5e50 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -55,7 +55,6 @@ enum {
 	STATE_IDLE,
 	STATE_READ,
 	STATE_WRITE,
-	STATE_STOP,
 };
 
 /**
@@ -205,19 +204,9 @@ static void meson_i2c_prepare_xfer(struct meson_i2c *i2c)
 
 	if (write)
 		meson_i2c_put_data(i2c, i2c->msg->buf + i2c->pos, i2c->count);
-}
-
-static void meson_i2c_stop(struct meson_i2c *i2c)
-{
-	dev_dbg(i2c->dev, "%s: last %d\n", __func__, i2c->last);
 
-	if (i2c->last) {
-		i2c->state = STATE_STOP;
+	if (i2c->last && i2c->pos + i2c->count >= i2c->msg->len)
 		meson_i2c_add_token(i2c, TOKEN_STOP);
-	} else {
-		i2c->state = STATE_IDLE;
-		complete(&i2c->done);
-	}
 }
 
 static irqreturn_t meson_i2c_irq(int irqno, void *dev_id)
@@ -262,7 +251,8 @@ static irqreturn_t meson_i2c_irq(int irqno, void *dev_id)
 		}
 
 		if (i2c->pos >= i2c->msg->len) {
-			meson_i2c_stop(i2c);
+			i2c->state = STATE_IDLE;
+			complete(&i2c->done);
 			break;
 		}
 
@@ -272,16 +262,13 @@ static irqreturn_t meson_i2c_irq(int irqno, void *dev_id)
 		i2c->pos += i2c->count;
 
 		if (i2c->pos >= i2c->msg->len) {
-			meson_i2c_stop(i2c);
+			i2c->state = STATE_IDLE;
+			complete(&i2c->done);
 			break;
 		}
 
 		meson_i2c_prepare_xfer(i2c);
 		break;
-	case STATE_STOP:
-		i2c->state = STATE_IDLE;
-		complete(&i2c->done);
-		break;
 	}
 
 out:
-- 
2.12.0

[PATCH v2 09/10] i2c: meson: remove meson_i2c_write_tokens

[Heiner Kallweit]

meson_i2c_write_tokens is always called directly after
meson_i2c_prepare_xfer (and only then). So we can simplify the code by
removing meson_i2c_write_tokens and moving the two statements of
meson_i2c_write_tokens to the end of meson_i2c_prepare_xfer. 

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- rebased
---
 drivers/i2c/busses/i2c-meson.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index 3e6f5e50..8f4658de 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -123,12 +123,6 @@ static void meson_i2c_add_token(struct meson_i2c *i2c, int token)
 	i2c->num_tokens++;
 }
 
-static void meson_i2c_write_tokens(struct meson_i2c *i2c)
-{
-	writel(i2c->tokens[0], i2c->regs + REG_TOK_LIST0);
-	writel(i2c->tokens[1], i2c->regs + REG_TOK_LIST1);
-}
-
 static void meson_i2c_set_clk_div(struct meson_i2c *i2c, unsigned int freq)
 {
 	unsigned long clk_rate = clk_get_rate(i2c->clk);
@@ -207,6 +201,9 @@ static void meson_i2c_prepare_xfer(struct meson_i2c *i2c)
 
 	if (i2c->last && i2c->pos + i2c->count >= i2c->msg->len)
 		meson_i2c_add_token(i2c, TOKEN_STOP);
+
+	writel(i2c->tokens[0], i2c->regs + REG_TOK_LIST0);
+	writel(i2c->tokens[1], i2c->regs + REG_TOK_LIST1);
 }
 
 static irqreturn_t meson_i2c_irq(int irqno, void *dev_id)
@@ -272,12 +269,10 @@ static irqreturn_t meson_i2c_irq(int irqno, void *dev_id)
 	}
 
 out:
-	if (i2c->state != STATE_IDLE) {
+	if (i2c->state != STATE_IDLE)
 		/* Restart the processing */
-		meson_i2c_write_tokens(i2c);
 		meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_START,
 				   REG_CTRL_START);
-	}
 
 	spin_unlock(&i2c->lock);
 
@@ -318,7 +313,6 @@ static int meson_i2c_xfer_msg(struct meson_i2c *i2c, struct i2c_msg *msg,
 
 	i2c->state = (msg->flags & I2C_M_RD) ? STATE_READ : STATE_WRITE;
 	meson_i2c_prepare_xfer(i2c);
-	meson_i2c_write_tokens(i2c);
 	reinit_completion(&i2c->done);
 
 	/* Start the transfer */
-- 
2.12.0

[PATCH v2 09/10] i2c: meson: remove meson_i2c_write_tokens

[Heiner Kallweit]

meson_i2c_write_tokens is always called directly after
meson_i2c_prepare_xfer (and only then). So we can simplify the code by
removing meson_i2c_write_tokens and moving the two statements of
meson_i2c_write_tokens to the end of meson_i2c_prepare_xfer. 

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- rebased
---
 drivers/i2c/busses/i2c-meson.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index 3e6f5e50..8f4658de 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -123,12 +123,6 @@ static void meson_i2c_add_token(struct meson_i2c *i2c, int token)
 	i2c->num_tokens++;
 }
 
-static void meson_i2c_write_tokens(struct meson_i2c *i2c)
-{
-	writel(i2c->tokens[0], i2c->regs + REG_TOK_LIST0);
-	writel(i2c->tokens[1], i2c->regs + REG_TOK_LIST1);
-}
-
 static void meson_i2c_set_clk_div(struct meson_i2c *i2c, unsigned int freq)
 {
 	unsigned long clk_rate = clk_get_rate(i2c->clk);
@@ -207,6 +201,9 @@ static void meson_i2c_prepare_xfer(struct meson_i2c *i2c)
 
 	if (i2c->last && i2c->pos + i2c->count >= i2c->msg->len)
 		meson_i2c_add_token(i2c, TOKEN_STOP);
+
+	writel(i2c->tokens[0], i2c->regs + REG_TOK_LIST0);
+	writel(i2c->tokens[1], i2c->regs + REG_TOK_LIST1);
 }
 
 static irqreturn_t meson_i2c_irq(int irqno, void *dev_id)
@@ -272,12 +269,10 @@ static irqreturn_t meson_i2c_irq(int irqno, void *dev_id)
 	}
 
 out:
-	if (i2c->state != STATE_IDLE) {
+	if (i2c->state != STATE_IDLE)
 		/* Restart the processing */
-		meson_i2c_write_tokens(i2c);
 		meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_START,
 				   REG_CTRL_START);
-	}
 
 	spin_unlock(&i2c->lock);
 
@@ -318,7 +313,6 @@ static int meson_i2c_xfer_msg(struct meson_i2c *i2c, struct i2c_msg *msg,
 
 	i2c->state = (msg->flags & I2C_M_RD) ? STATE_READ : STATE_WRITE;
 	meson_i2c_prepare_xfer(i2c);
-	meson_i2c_write_tokens(i2c);
 	reinit_completion(&i2c->done);
 
 	/* Start the transfer */
-- 
2.12.0

[PATCH v2 10/10] i2c: meson: improve and simplify interrupt handler

[Heiner Kallweit]

The preceding changes in this patch series now allow to simplify
the interrupt handler significantly.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- rebased
---
 drivers/i2c/busses/i2c-meson.c | 40 ++++++++++------------------------------
 1 file changed, 10 insertions(+), 30 deletions(-)

diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index 8f4658de..25415690 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -239,41 +239,21 @@ static irqreturn_t meson_i2c_irq(int irqno, void *dev_id)
 		goto out;
 	}
 
-	switch (i2c->state) {
-	case STATE_READ:
-		if (i2c->count > 0) {
-			meson_i2c_get_data(i2c, i2c->msg->buf + i2c->pos,
-					   i2c->count);
-			i2c->pos += i2c->count;
-		}
-
-		if (i2c->pos >= i2c->msg->len) {
-			i2c->state = STATE_IDLE;
-			complete(&i2c->done);
-			break;
-		}
-
-		meson_i2c_prepare_xfer(i2c);
-		break;
-	case STATE_WRITE:
-		i2c->pos += i2c->count;
+	if (i2c->state == STATE_READ && i2c->count)
+		meson_i2c_get_data(i2c, i2c->msg->buf + i2c->pos, i2c->count);
 
-		if (i2c->pos >= i2c->msg->len) {
-			i2c->state = STATE_IDLE;
-			complete(&i2c->done);
-			break;
-		}
+	i2c->pos += i2c->count;
 
-		meson_i2c_prepare_xfer(i2c);
-		break;
+	if (i2c->pos >= i2c->msg->len) {
+		i2c->state = STATE_IDLE;
+		complete(&i2c->done);
+		goto out;
 	}
 
+	/* Restart the processing */
+	meson_i2c_prepare_xfer(i2c);
+	meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_START, REG_CTRL_START);
 out:
-	if (i2c->state != STATE_IDLE)
-		/* Restart the processing */
-		meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_START,
-				   REG_CTRL_START);
-
 	spin_unlock(&i2c->lock);
 
 	return IRQ_HANDLED;
-- 
2.12.0

[PATCH v2 10/10] i2c: meson: improve and simplify interrupt handler

[Heiner Kallweit]

The preceding changes in this patch series now allow to simplify
the interrupt handler significantly.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- rebased
---
 drivers/i2c/busses/i2c-meson.c | 40 ++++++++++------------------------------
 1 file changed, 10 insertions(+), 30 deletions(-)

diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index 8f4658de..25415690 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -239,41 +239,21 @@ static irqreturn_t meson_i2c_irq(int irqno, void *dev_id)
 		goto out;
 	}
 
-	switch (i2c->state) {
-	case STATE_READ:
-		if (i2c->count > 0) {
-			meson_i2c_get_data(i2c, i2c->msg->buf + i2c->pos,
-					   i2c->count);
-			i2c->pos += i2c->count;
-		}
-
-		if (i2c->pos >= i2c->msg->len) {
-			i2c->state = STATE_IDLE;
-			complete(&i2c->done);
-			break;
-		}
-
-		meson_i2c_prepare_xfer(i2c);
-		break;
-	case STATE_WRITE:
-		i2c->pos += i2c->count;
+	if (i2c->state == STATE_READ && i2c->count)
+		meson_i2c_get_data(i2c, i2c->msg->buf + i2c->pos, i2c->count);
 
-		if (i2c->pos >= i2c->msg->len) {
-			i2c->state = STATE_IDLE;
-			complete(&i2c->done);
-			break;
-		}
+	i2c->pos += i2c->count;
 
-		meson_i2c_prepare_xfer(i2c);
-		break;
+	if (i2c->pos >= i2c->msg->len) {
+		i2c->state = STATE_IDLE;
+		complete(&i2c->done);
+		goto out;
 	}
 
+	/* Restart the processing */
+	meson_i2c_prepare_xfer(i2c);
+	meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_START, REG_CTRL_START);
 out:
-	if (i2c->state != STATE_IDLE)
-		/* Restart the processing */
-		meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_START,
-				   REG_CTRL_START);
-
 	spin_unlock(&i2c->lock);
 
 	return IRQ_HANDLED;
-- 
2.12.0