[PATCH v2 3/4] ARM: at91/dt: sama5d4: update i2c compatible string

[Ludovic Desroches]

A new compatible string has been introduced: atmel,sama5d4-i2c. It
allows to use the i2c-sda-hold-time-ns property if needed.

Signed-off-by: Ludovic Desroches <ludovic.desroc...@atmel.com>
---
 arch/arm/boot/dts/sama5d4.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/sama5d4.dtsi b/arch/arm/boot/dts/sama5d4.dtsi
index 2193637..83d7e7c 100644
--- a/arch/arm/boot/dts/sama5d4.dtsi
+++ b/arch/arm/boot/dts/sama5d4.dtsi
@@ -916,7 +916,7 @@
};
 
i2c0: i2c@f8014000 {
-   compatible = "atmel,at91sam9x5-i2c";
+   compatible = "atmel,sama5d4-i2c";
reg = <0xf8014000 0x4000>;
interrupts = <32 IRQ_TYPE_LEVEL_HIGH 6>;
dmas = <
@@ -935,7 +935,7 @@
};
 
i2c1: i2c@f8018000 {
-   compatible = "atmel,at91sam9x5-i2c";
+   compatible = "atmel,sama5d4-i2c";
reg = <0xf8018000 0x4000>;
interrupts = <33 IRQ_TYPE_LEVEL_HIGH 6>;
dmas = <
@@ -975,7 +975,7 @@
};
 
i2c2: i2c@f8024000 {
-   compatible = "atmel,at91sam9x5-i2c";
+   compatible = "atmel,sama5d4-i2c";
reg = <0xf8024000 0x4000>;
interrupts = <34 IRQ_TYPE_LEVEL_HIGH 6>;
dmas = <
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 4/4] ARM: at91/dt: sama5d2 Xplained: pmic needs a specific sda hold time

[Ludovic Desroches]

Data have to be held longer for the PMIC device. The ACT8945A
datasheet claims that minimum SDA data hold time is about 300 ns.

Signed-off-by: Ludovic Desroches <ludovic.desroc...@atmel.com>
---
 arch/arm/boot/dts/at91-sama5d2_xplained.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/at91-sama5d2_xplained.dts 
b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
index ad6de73..9bced00 100644
--- a/arch/arm/boot/dts/at91-sama5d2_xplained.dts
+++ b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
@@ -129,6 +129,7 @@
dmas = <0>, <0>;
pinctrl-names = "default";
pinctrl-0 = <_i2c0_default>;
+   i2c-sda-hold-time-ns = <350>;
status = "okay";
 
pmic: act8865@5b {
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/4] i2c: at91: update bindings documention

[Ludovic Desroches]

The i2c-sda-hold-time-ns property is supported from atmel,sama5d4-i2c.

Signed-off-by: Ludovic Desroches <ludovic.desroc...@atmel.com>
Acked-by: Rob Herring <r...@kernel.org>
---
 Documentation/devicetree/bindings/i2c/i2c-at91.txt | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-at91.txt 
b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
index 6e81dc1..ef973a0 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-at91.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
@@ -3,7 +3,7 @@ I2C for Atmel platforms
 Required properties :
 - compatible : Must be "atmel,at91rm9200-i2c", "atmel,at91sam9261-i2c",
  "atmel,at91sam9260-i2c", "atmel,at91sam9g20-i2c", "atmel,at91sam9g10-i2c",
- "atmel,at91sam9x5-i2c" or "atmel,sama5d2-i2c"
+ "atmel,at91sam9x5-i2c", "atmel,sama5d4-i2c" or "atmel,sama5d2-i2c"
 - reg: physical base address of the controller and length of memory mapped
  region.
 - interrupts: interrupt number to the cpu.
@@ -17,6 +17,8 @@ Optional properties:
 - dma-names: should contain "tx" and "rx".
 - atmel,fifo-size: maximum number of data the RX and TX FIFOs can store for 
FIFO
   capable I2C controllers.
+- i2c-sda-hold-time-ns: TWD hold time, only available for "atmel,sama5d4-i2c"
+  and "atmel,sama5d2-i2c".
 - Child nodes conforming to i2c bus binding
 
 Examples :
@@ -52,6 +54,7 @@ i2c0: i2c@f8034600 {
#size-cells = <0>;
clocks = <>;
atmel,fifo-size = <16>;
+   i2c-sda-hold-time-ns = <336>;
 
wm8731: wm8731@1a {
compatible = "wm8731";
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/4] i2c: at91: add upport for the HOLD field

[Ludovic Desroches]

Sorry forget this one, I have sent an old version.

On Thu, Dec 03, 2015 at 10:53:51AM +0100, Ludovic Desroches wrote:
> The hold field allows to configure the data hold time which can be set
> with the help of the generic binding 'i2c-sda-hold-time-ns'. This
> feature has been introduced with SAMA5D4 SoC family.
> 
> Signed-off-by: Ludovic Desroches <ludovic.desroc...@atmel.com>
> ---
>  drivers/i2c/busses/i2c-at91.c | 53 
> ---
>  1 file changed, 50 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
> index 10835d1..921d32b 100644
> --- a/drivers/i2c/busses/i2c-at91.c
> +++ b/drivers/i2c/busses/i2c-at91.c
> @@ -64,6 +64,8 @@
>  #define  AT91_TWI_IADR   0x000c  /* Internal Address Register */
>  
>  #define  AT91_TWI_CWGR   0x0010  /* Clock Waveform Generator Reg 
> */
> +#define  AT91_TWI_CWGR_HOLD_MAX  0x1f
> +#define  AT91_TWI_CWGR_HOLD(x)   (((x) & AT91_TWI_CWGR_HOLD_MAX) << 24)
>  
>  #define  AT91_TWI_SR 0x0020  /* Status Register */
>  #define  AT91_TWI_TXCOMP BIT(0)  /* Transmission Complete */
> @@ -110,6 +112,7 @@ struct at91_twi_pdata {
>   unsigned clk_offset;
>   bool has_unre_flag;
>   bool has_alt_cmd;
> + bool has_hold_field;
>   struct at_dma_slave dma_slave;
>  };
>  
> @@ -187,10 +190,11 @@ static void at91_init_twi_bus(struct at91_twi_dev *dev)
>   */
>  static void at91_calc_twi_clock(struct at91_twi_dev *dev, int twi_clk)
>  {
> - int ckdiv, cdiv, div;
> + int ckdiv, cdiv, div, hold = 0;
>   struct at91_twi_pdata *pdata = dev->pdata;
>   int offset = pdata->clk_offset;
>   int max_ckdiv = pdata->clk_max_div;
> + u32 twd_hold_time_ns = 0;
>  
>   div = max(0, (int)DIV_ROUND_UP(clk_get_rate(dev->clk),
>  2 * twi_clk) - offset);
> @@ -204,8 +208,33 @@ static void at91_calc_twi_clock(struct at91_twi_dev 
> *dev, int twi_clk)
>   cdiv = 255;
>   }
>  
> - dev->twi_cwgr_reg = (ckdiv << 16) | (cdiv << 8) | cdiv;
> - dev_dbg(dev->dev, "cdiv %d ckdiv %d\n", cdiv, ckdiv);
> + if (pdata->has_hold_field) {
> + of_property_read_u32(dev->dev->of_node, "i2c-sda-hold-time-ns",
> +  _hold_time_ns);
> +
> + /*
> +  * hold time = HOLD + 3 x T_peripheral_clock
> +  * Use clk rate in kHz to prevent overflows when computing
> +  * hold.
> +  */
> + hold = DIV_ROUND_UP(twd_hold_time_ns
> + * (clk_get_rate(dev->clk) / 1000), 100);
> + hold -= 3;
> + if (hold < 0)
> + hold = 0;
> + if (hold > AT91_TWI_CWGR_HOLD_MAX) {
> + dev_warn(dev->dev,
> +  "HOLD field set to its maximum value (%d 
> instead of %d)\n",
> +  AT91_TWI_CWGR_HOLD_MAX, hold);
> + hold = AT91_TWI_CWGR_HOLD_MAX;
> + }
> + }
> +
> + dev->twi_cwgr_reg = (ckdiv << 16) | (cdiv << 8) | cdiv
> + | AT91_TWI_CWGR_HOLD(hold);
> +
> + dev_dbg(dev->dev, "cdiv %d ckdiv %d hold %d (%d ns)\n",
> + cdiv, ckdiv, hold, twd_hold_time_ns);
>  }
>  
>  static void at91_twi_dma_cleanup(struct at91_twi_dev *dev)
> @@ -797,6 +826,7 @@ static struct at91_twi_pdata at91rm9200_config = {
>   .clk_offset = 3,
>   .has_unre_flag = true,
>   .has_alt_cmd = false,
> + .has_hold_field = false,
>  };
>  
>  static struct at91_twi_pdata at91sam9261_config = {
> @@ -804,6 +834,7 @@ static struct at91_twi_pdata at91sam9261_config = {
>   .clk_offset = 4,
>   .has_unre_flag = false,
>   .has_alt_cmd = false,
> + .has_hold_field = false,
>  };
>  
>  static struct at91_twi_pdata at91sam9260_config = {
> @@ -811,6 +842,7 @@ static struct at91_twi_pdata at91sam9260_config = {
>   .clk_offset = 4,
>   .has_unre_flag = false,
>   .has_alt_cmd = false,
> + .has_hold_field = false,
>  };
>  
>  static struct at91_twi_pdata at91sam9g20_config = {
> @@ -818,6 +850,7 @@ static struct at91_twi_pdata at91sam9g20_config = {
>   .clk_offset = 4,
>   .has_unre_flag = false,
>   .has_alt_cmd = false,
> + .has_hold_field = false,
>  };
>  
>  static struct at91_twi_pdata at91sam9g10_config = {
> @@ -825,6 +858,7 @@ static struct at91_twi_pd

[PATCH v2 1/4] i2c: at91: add support for the HOLD field

[Ludovic Desroches]

The hold field allows to configure the data hold time which can be set
with the help of the generic binding 'i2c-sda-hold-time-ns'. This
feature has been introduced with SAMA5D4 SoC family.

Signed-off-by: Ludovic Desroches <ludovic.desroc...@atmel.com>
---
 drivers/i2c/busses/i2c-at91.c | 53 ---
 1 file changed, 50 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index 10835d1..921d32b 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -64,6 +64,8 @@
 #defineAT91_TWI_IADR   0x000c  /* Internal Address Register */
 
 #defineAT91_TWI_CWGR   0x0010  /* Clock Waveform Generator Reg 
*/
+#defineAT91_TWI_CWGR_HOLD_MAX  0x1f
+#defineAT91_TWI_CWGR_HOLD(x)   (((x) & AT91_TWI_CWGR_HOLD_MAX) << 24)
 
 #defineAT91_TWI_SR 0x0020  /* Status Register */
 #defineAT91_TWI_TXCOMP BIT(0)  /* Transmission Complete */
@@ -110,6 +112,7 @@ struct at91_twi_pdata {
unsigned clk_offset;
bool has_unre_flag;
bool has_alt_cmd;
+   bool has_hold_field;
struct at_dma_slave dma_slave;
 };
 
@@ -187,10 +190,11 @@ static void at91_init_twi_bus(struct at91_twi_dev *dev)
  */
 static void at91_calc_twi_clock(struct at91_twi_dev *dev, int twi_clk)
 {
-   int ckdiv, cdiv, div;
+   int ckdiv, cdiv, div, hold = 0;
struct at91_twi_pdata *pdata = dev->pdata;
int offset = pdata->clk_offset;
int max_ckdiv = pdata->clk_max_div;
+   u32 twd_hold_time_ns = 0;
 
div = max(0, (int)DIV_ROUND_UP(clk_get_rate(dev->clk),
   2 * twi_clk) - offset);
@@ -204,8 +208,33 @@ static void at91_calc_twi_clock(struct at91_twi_dev *dev, 
int twi_clk)
cdiv = 255;
}
 
-   dev->twi_cwgr_reg = (ckdiv << 16) | (cdiv << 8) | cdiv;
-   dev_dbg(dev->dev, "cdiv %d ckdiv %d\n", cdiv, ckdiv);
+   if (pdata->has_hold_field) {
+   of_property_read_u32(dev->dev->of_node, "i2c-sda-hold-time-ns",
+_hold_time_ns);
+
+   /*
+* hold time = HOLD + 3 x T_peripheral_clock
+* Use clk rate in kHz to prevent overflows when computing
+* hold.
+*/
+   hold = DIV_ROUND_UP(twd_hold_time_ns
+   * (clk_get_rate(dev->clk) / 1000), 100);
+   hold -= 3;
+   if (hold < 0)
+   hold = 0;
+   if (hold > AT91_TWI_CWGR_HOLD_MAX) {
+   dev_warn(dev->dev,
+"HOLD field set to its maximum value (%d 
instead of %d)\n",
+AT91_TWI_CWGR_HOLD_MAX, hold);
+   hold = AT91_TWI_CWGR_HOLD_MAX;
+   }
+   }
+
+   dev->twi_cwgr_reg = (ckdiv << 16) | (cdiv << 8) | cdiv
+   | AT91_TWI_CWGR_HOLD(hold);
+
+   dev_dbg(dev->dev, "cdiv %d ckdiv %d hold %d (%d ns)\n",
+   cdiv, ckdiv, hold, twd_hold_time_ns);
 }
 
 static void at91_twi_dma_cleanup(struct at91_twi_dev *dev)
@@ -797,6 +826,7 @@ static struct at91_twi_pdata at91rm9200_config = {
.clk_offset = 3,
.has_unre_flag = true,
.has_alt_cmd = false,
+   .has_hold_field = false,
 };
 
 static struct at91_twi_pdata at91sam9261_config = {
@@ -804,6 +834,7 @@ static struct at91_twi_pdata at91sam9261_config = {
.clk_offset = 4,
.has_unre_flag = false,
.has_alt_cmd = false,
+   .has_hold_field = false,
 };
 
 static struct at91_twi_pdata at91sam9260_config = {
@@ -811,6 +842,7 @@ static struct at91_twi_pdata at91sam9260_config = {
.clk_offset = 4,
.has_unre_flag = false,
.has_alt_cmd = false,
+   .has_hold_field = false,
 };
 
 static struct at91_twi_pdata at91sam9g20_config = {
@@ -818,6 +850,7 @@ static struct at91_twi_pdata at91sam9g20_config = {
.clk_offset = 4,
.has_unre_flag = false,
.has_alt_cmd = false,
+   .has_hold_field = false,
 };
 
 static struct at91_twi_pdata at91sam9g10_config = {
@@ -825,6 +858,7 @@ static struct at91_twi_pdata at91sam9g10_config = {
.clk_offset = 4,
.has_unre_flag = false,
.has_alt_cmd = false,
+   .has_hold_field = false,
 };
 
 static const struct platform_device_id at91_twi_devtypes[] = {
@@ -854,6 +888,15 @@ static struct at91_twi_pdata at91sam9x5_config = {
.clk_offset = 4,
.has_unre_flag = false,
.has_alt_cmd = false,
+   .has_hold_field = false,
+};
+
+static struct at91_twi_pdata sama5d4_config = {
+   .clk_max_div = 7,
+   .clk_offset = 4,
+   .has_unre_flag = false,
+   .has_alt_cmd = fa

[PATCH v2 1/4] i2c: at91: add upport for the HOLD field

[Ludovic Desroches]

The hold field allows to configure the data hold time which can be set
with the help of the generic binding 'i2c-sda-hold-time-ns'. This
feature has been introduced with SAMA5D4 SoC family.

Signed-off-by: Ludovic Desroches <ludovic.desroc...@atmel.com>
---
 drivers/i2c/busses/i2c-at91.c | 53 ---
 1 file changed, 50 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index 10835d1..921d32b 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -64,6 +64,8 @@
 #defineAT91_TWI_IADR   0x000c  /* Internal Address Register */
 
 #defineAT91_TWI_CWGR   0x0010  /* Clock Waveform Generator Reg 
*/
+#defineAT91_TWI_CWGR_HOLD_MAX  0x1f
+#defineAT91_TWI_CWGR_HOLD(x)   (((x) & AT91_TWI_CWGR_HOLD_MAX) << 24)
 
 #defineAT91_TWI_SR 0x0020  /* Status Register */
 #defineAT91_TWI_TXCOMP BIT(0)  /* Transmission Complete */
@@ -110,6 +112,7 @@ struct at91_twi_pdata {
unsigned clk_offset;
bool has_unre_flag;
bool has_alt_cmd;
+   bool has_hold_field;
struct at_dma_slave dma_slave;
 };
 
@@ -187,10 +190,11 @@ static void at91_init_twi_bus(struct at91_twi_dev *dev)
  */
 static void at91_calc_twi_clock(struct at91_twi_dev *dev, int twi_clk)
 {
-   int ckdiv, cdiv, div;
+   int ckdiv, cdiv, div, hold = 0;
struct at91_twi_pdata *pdata = dev->pdata;
int offset = pdata->clk_offset;
int max_ckdiv = pdata->clk_max_div;
+   u32 twd_hold_time_ns = 0;
 
div = max(0, (int)DIV_ROUND_UP(clk_get_rate(dev->clk),
   2 * twi_clk) - offset);
@@ -204,8 +208,33 @@ static void at91_calc_twi_clock(struct at91_twi_dev *dev, 
int twi_clk)
cdiv = 255;
}
 
-   dev->twi_cwgr_reg = (ckdiv << 16) | (cdiv << 8) | cdiv;
-   dev_dbg(dev->dev, "cdiv %d ckdiv %d\n", cdiv, ckdiv);
+   if (pdata->has_hold_field) {
+   of_property_read_u32(dev->dev->of_node, "i2c-sda-hold-time-ns",
+_hold_time_ns);
+
+   /*
+* hold time = HOLD + 3 x T_peripheral_clock
+* Use clk rate in kHz to prevent overflows when computing
+* hold.
+*/
+   hold = DIV_ROUND_UP(twd_hold_time_ns
+   * (clk_get_rate(dev->clk) / 1000), 100);
+   hold -= 3;
+   if (hold < 0)
+   hold = 0;
+   if (hold > AT91_TWI_CWGR_HOLD_MAX) {
+   dev_warn(dev->dev,
+"HOLD field set to its maximum value (%d 
instead of %d)\n",
+AT91_TWI_CWGR_HOLD_MAX, hold);
+   hold = AT91_TWI_CWGR_HOLD_MAX;
+   }
+   }
+
+   dev->twi_cwgr_reg = (ckdiv << 16) | (cdiv << 8) | cdiv
+   | AT91_TWI_CWGR_HOLD(hold);
+
+   dev_dbg(dev->dev, "cdiv %d ckdiv %d hold %d (%d ns)\n",
+   cdiv, ckdiv, hold, twd_hold_time_ns);
 }
 
 static void at91_twi_dma_cleanup(struct at91_twi_dev *dev)
@@ -797,6 +826,7 @@ static struct at91_twi_pdata at91rm9200_config = {
.clk_offset = 3,
.has_unre_flag = true,
.has_alt_cmd = false,
+   .has_hold_field = false,
 };
 
 static struct at91_twi_pdata at91sam9261_config = {
@@ -804,6 +834,7 @@ static struct at91_twi_pdata at91sam9261_config = {
.clk_offset = 4,
.has_unre_flag = false,
.has_alt_cmd = false,
+   .has_hold_field = false,
 };
 
 static struct at91_twi_pdata at91sam9260_config = {
@@ -811,6 +842,7 @@ static struct at91_twi_pdata at91sam9260_config = {
.clk_offset = 4,
.has_unre_flag = false,
.has_alt_cmd = false,
+   .has_hold_field = false,
 };
 
 static struct at91_twi_pdata at91sam9g20_config = {
@@ -818,6 +850,7 @@ static struct at91_twi_pdata at91sam9g20_config = {
.clk_offset = 4,
.has_unre_flag = false,
.has_alt_cmd = false,
+   .has_hold_field = false,
 };
 
 static struct at91_twi_pdata at91sam9g10_config = {
@@ -825,6 +858,7 @@ static struct at91_twi_pdata at91sam9g10_config = {
.clk_offset = 4,
.has_unre_flag = false,
.has_alt_cmd = false,
+   .has_hold_field = false,
 };
 
 static const struct platform_device_id at91_twi_devtypes[] = {
@@ -854,6 +888,15 @@ static struct at91_twi_pdata at91sam9x5_config = {
.clk_offset = 4,
.has_unre_flag = false,
.has_alt_cmd = false,
+   .has_hold_field = false,
+};
+
+static struct at91_twi_pdata sama5d4_config = {
+   .clk_max_div = 7,
+   .clk_offset = 4,
+   .has_unre_flag = false,
+   .has_alt_cmd = fa

[PATCH v2 0/4] i2c: at91: add support for SDA HOLD

[Ludovic Desroches]

Changes:
- from v1:
  - fix typos,
  - change MAX_HOLD macro to follow Nicolas' advice,
  - make it clear that sama5d2 also support i2c-sda-hold-time-ns.

Ludovic Desroches (4):
  i2c: at91: add support for the HOLD field
  i2c: at91: update bindings documention
  ARM: at91/dt: sama5d4: update i2c compatible string
  ARM: at91/dt: sama5d2 Xplained: pmic needs a specific sda hold time

 Documentation/devicetree/bindings/i2c/i2c-at91.txt |  5 +-
 arch/arm/boot/dts/at91-sama5d2_xplained.dts|  1 +
 arch/arm/boot/dts/sama5d4.dtsi |  6 +--
 drivers/i2c/busses/i2c-at91.c  | 53 --
 4 files changed, 58 insertions(+), 7 deletions(-)

-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] i2c: at91: add upport for the HOLD field

[Ludovic Desroches]

The hold field allows to configure the data hold time which can be set
with the help of the generic binding 'i2c-sda-hold-time-ns'. This
feature has been introduced with SAMA5D4 SoC family.

Signed-off-by: Ludovic Desroches <ludovic.desroc...@atmel.com>
---
 drivers/i2c/busses/i2c-at91.c | 54 ---
 1 file changed, 51 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index 10835d1..09e1690 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -64,6 +64,7 @@
 #defineAT91_TWI_IADR   0x000c  /* Internal Address Register */
 
 #defineAT91_TWI_CWGR   0x0010  /* Clock Waveform Generator Reg 
*/
+#defineAT91_TWI_CWGR_HOLD(x)   (((x) & 0x1f) << 24)
 
 #defineAT91_TWI_SR 0x0020  /* Status Register */
 #defineAT91_TWI_TXCOMP BIT(0)  /* Transmission Complete */
@@ -110,6 +111,7 @@ struct at91_twi_pdata {
unsigned clk_offset;
bool has_unre_flag;
bool has_alt_cmd;
+   bool has_hold_field;
struct at_dma_slave dma_slave;
 };
 
@@ -187,10 +189,13 @@ static void at91_init_twi_bus(struct at91_twi_dev *dev)
  */
 static void at91_calc_twi_clock(struct at91_twi_dev *dev, int twi_clk)
 {
-   int ckdiv, cdiv, div;
+   int ckdiv, cdiv, div, hold = 0;
struct at91_twi_pdata *pdata = dev->pdata;
int offset = pdata->clk_offset;
int max_ckdiv = pdata->clk_max_div;
+   u32 twd_hold_time_ns = 0;
+
+#define MAX_HOLD 31
 
div = max(0, (int)DIV_ROUND_UP(clk_get_rate(dev->clk),
   2 * twi_clk) - offset);
@@ -204,8 +209,33 @@ static void at91_calc_twi_clock(struct at91_twi_dev *dev, 
int twi_clk)
cdiv = 255;
}
 
-   dev->twi_cwgr_reg = (ckdiv << 16) | (cdiv << 8) | cdiv;
-   dev_dbg(dev->dev, "cdiv %d ckdiv %d\n", cdiv, ckdiv);
+   if (pdata->has_hold_field) {
+   of_property_read_u32(dev->dev->of_node, "i2c-sda-hold-time-ns",
+_hold_time_ns);
+
+   /*
+* hold time = HOLD + 3 x T_peripheral_clock
+* Use clk rate in kHz to prevent overflows when computing
+* hold.
+*/
+   hold = DIV_ROUND_UP(twd_hold_time_ns
+   * (clk_get_rate(dev->clk) / 1000), 100);
+   hold -= 3;
+   if (hold < 0)
+   hold = 0;
+   if (hold > MAX_HOLD) {
+   dev_warn(dev->dev,
+"HOLD field set to its maximum value (%d 
instead of %d)\n",
+MAX_HOLD, hold);
+   hold = MAX_HOLD;
+   }
+   }
+
+   dev->twi_cwgr_reg = (ckdiv << 16) | (cdiv << 8) | cdiv
+   | AT91_TWI_CWGR_HOLD(hold);
+
+   dev_dbg(dev->dev, "cdiv %d ckdiv %d hold %d (%d ns)\n",
+   cdiv, ckdiv, hold, twd_hold_time_ns);
 }
 
 static void at91_twi_dma_cleanup(struct at91_twi_dev *dev)
@@ -797,6 +827,7 @@ static struct at91_twi_pdata at91rm9200_config = {
.clk_offset = 3,
.has_unre_flag = true,
.has_alt_cmd = false,
+   .has_hold_field = false,
 };
 
 static struct at91_twi_pdata at91sam9261_config = {
@@ -804,6 +835,7 @@ static struct at91_twi_pdata at91sam9261_config = {
.clk_offset = 4,
.has_unre_flag = false,
.has_alt_cmd = false,
+   .has_hold_field = false,
 };
 
 static struct at91_twi_pdata at91sam9260_config = {
@@ -811,6 +843,7 @@ static struct at91_twi_pdata at91sam9260_config = {
.clk_offset = 4,
.has_unre_flag = false,
.has_alt_cmd = false,
+   .has_hold_field = false,
 };
 
 static struct at91_twi_pdata at91sam9g20_config = {
@@ -818,6 +851,7 @@ static struct at91_twi_pdata at91sam9g20_config = {
.clk_offset = 4,
.has_unre_flag = false,
.has_alt_cmd = false,
+   .has_hold_field = false,
 };
 
 static struct at91_twi_pdata at91sam9g10_config = {
@@ -825,6 +859,7 @@ static struct at91_twi_pdata at91sam9g10_config = {
.clk_offset = 4,
.has_unre_flag = false,
.has_alt_cmd = false,
+   .has_hold_field = false,
 };
 
 static const struct platform_device_id at91_twi_devtypes[] = {
@@ -854,6 +889,15 @@ static struct at91_twi_pdata at91sam9x5_config = {
.clk_offset = 4,
.has_unre_flag = false,
.has_alt_cmd = false,
+   .has_hold_field = false,
+};
+
+static struct at91_twi_pdata sama5d4_config = {
+   .clk_max_div = 7,
+   .clk_offset = 4,
+   .has_unre_flag = false,
+   .has_alt_cmd = false,
+   .has_hold_field = true,
 };
 
 static struct at91_twi_pdata sama5d2_config =

[PATCH 3/4] ARM: at91/dt: sama5d4: update i2c compatible string

[Ludovic Desroches]

A new compatible string has been introduced: atmel,sama5d4-i2c. It
allows to use the i2c-sda-hold-time-ns property if needed.

Signed-off-by: Ludovic Desroches <ludovic.desroc...@atmel.com>
---
 arch/arm/boot/dts/sama5d4.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/sama5d4.dtsi b/arch/arm/boot/dts/sama5d4.dtsi
index 2193637..83d7e7c 100644
--- a/arch/arm/boot/dts/sama5d4.dtsi
+++ b/arch/arm/boot/dts/sama5d4.dtsi
@@ -916,7 +916,7 @@
};
 
i2c0: i2c@f8014000 {
-   compatible = "atmel,at91sam9x5-i2c";
+   compatible = "atmel,sama5d4-i2c";
reg = <0xf8014000 0x4000>;
interrupts = <32 IRQ_TYPE_LEVEL_HIGH 6>;
dmas = <
@@ -935,7 +935,7 @@
};
 
i2c1: i2c@f8018000 {
-   compatible = "atmel,at91sam9x5-i2c";
+   compatible = "atmel,sama5d4-i2c";
reg = <0xf8018000 0x4000>;
interrupts = <33 IRQ_TYPE_LEVEL_HIGH 6>;
dmas = <
@@ -975,7 +975,7 @@
};
 
i2c2: i2c@f8024000 {
-   compatible = "atmel,at91sam9x5-i2c";
+   compatible = "atmel,sama5d4-i2c";
reg = <0xf8024000 0x4000>;
interrupts = <34 IRQ_TYPE_LEVEL_HIGH 6>;
dmas = <
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] i2c: at91: update bindings documention

[Ludovic Desroches]

The i2c-sda-hold-time-ns property is supported from atmel,sama5d4-i2c.

Signed-off-by: Ludovic Desroches <ludovic.desroc...@atmel.com>
---
 Documentation/devicetree/bindings/i2c/i2c-at91.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-at91.txt 
b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
index 6e81dc1..67c6f2e 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-at91.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
@@ -3,7 +3,7 @@ I2C for Atmel platforms
 Required properties :
 - compatible : Must be "atmel,at91rm9200-i2c", "atmel,at91sam9261-i2c",
  "atmel,at91sam9260-i2c", "atmel,at91sam9g20-i2c", "atmel,at91sam9g10-i2c",
- "atmel,at91sam9x5-i2c" or "atmel,sama5d2-i2c"
+ "atmel,at91sam9x5-i2c", "atmel,sama5d4-i2c" or "atmel,sama5d2-i2c"
 - reg: physical base address of the controller and length of memory mapped
  region.
 - interrupts: interrupt number to the cpu.
@@ -17,6 +17,7 @@ Optional properties:
 - dma-names: should contain "tx" and "rx".
 - atmel,fifo-size: maximum number of data the RX and TX FIFOs can store for 
FIFO
   capable I2C controllers.
+- i2c-sda-hold-time-ns: TWD hold time, only available from "atmel,sama5d4-i2c".
 - Child nodes conforming to i2c bus binding
 
 Examples :
@@ -52,6 +53,7 @@ i2c0: i2c@f8034600 {
#size-cells = <0>;
clocks = <>;
atmel,fifo-size = <16>;
+   i2c-sda-hold-time-ns = <336>;
 
wm8731: wm8731@1a {
compatible = "wm8731";
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] ARM: at91/dt: sama5d2 Xplained: pmic needs a specific sda hold time

[Ludovic Desroches]

Data have to been hold longer for the PMIC device.

Signed-off-by: Ludovic Desroches <ludovic.desroc...@atmel.com>
---
 arch/arm/boot/dts/at91-sama5d2_xplained.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/at91-sama5d2_xplained.dts 
b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
index ad6de73..9bced00 100644
--- a/arch/arm/boot/dts/at91-sama5d2_xplained.dts
+++ b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
@@ -129,6 +129,7 @@
dmas = <0>, <0>;
pinctrl-names = "default";
pinctrl-0 = <_i2c0_default>;
+   i2c-sda-hold-time-ns = <350>;
status = "okay";
 
pmic: act8865@5b {
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] i2c: at91: add setting HOLD field of TWIHS_CWGR via DT

[Ludovic Desroches]

On Mon, Nov 30, 2015 at 04:13:20PM +0100, Wolfram Sang wrote:
> On Tue, Nov 24, 2015 at 02:47:40PM +0100, Ludovic Desroches wrote:
> > From: Wenyou Yang <wenyou.y...@atmel.com>
> > 
> > Add the HOLD field management. Some i2c devices need a longer data hold
> > time than the one given in the i2c bus specification. Since this value
> > depends on the i2c device connected to the bus, add a DT property to
> > configure it: "atmel,twd-hold-cycles".
> 
> We already have "i2c-sda-hold-time-ns". Can you use that one? Sorry that
> this is not obviously documented, I am working on it...
> 

I think we can use it, I'll rework the patch to convert duration in
number of cycles.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] ARM: dts: at91: specify DT property "atmel,twd-hold-cycles"

[Ludovic Desroches]

From: Wenyou Yang 

Specify the device tree property "atmel,twd-hold-cycles" to 25
to adapt to the PMIC ACT8945A.

Signed-off-by: Wenyou Yang 
---
 arch/arm/boot/dts/at91-sama5d2_xplained.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/at91-sama5d2_xplained.dts 
b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
index ad6de73..25ffd86 100644
--- a/arch/arm/boot/dts/at91-sama5d2_xplained.dts
+++ b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
@@ -129,6 +129,7 @@
dmas = <0>, <0>;
pinctrl-names = "default";
pinctrl-0 = <_i2c0_default>;
+   atmel,twd-hold-cycles = <25>;
status = "okay";
 
pmic: act8865@5b {
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] i2c: at91: add setting HOLD field of TWIHS_CWGR via DT

[Ludovic Desroches]

From: Wenyou Yang <wenyou.y...@atmel.com>

Add the HOLD field management. Some i2c devices need a longer data hold
time than the one given in the i2c bus specification. Since this value
depends on the i2c device connected to the bus, add a DT property to
configure it: "atmel,twd-hold-cycles".

Signed-off-by: Wenyou Yang <wenyou.y...@atmel.com>
Signed-off-by: Ludovic Desroches <ludovic.desroc...@atmel.com>
---
 drivers/i2c/busses/i2c-at91.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index 10835d1..b3595ea 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -64,6 +64,7 @@
 #defineAT91_TWI_IADR   0x000c  /* Internal Address Register */
 
 #defineAT91_TWI_CWGR   0x0010  /* Clock Waveform Generator Reg 
*/
+#defineAT91_TWI_CWGR_HOLD(x)   (((x) & 0x1f) << 24)
 
 #defineAT91_TWI_SR 0x0020  /* Status Register */
 #defineAT91_TWI_TXCOMP BIT(0)  /* Transmission Complete */
@@ -185,7 +186,8 @@ static void at91_init_twi_bus(struct at91_twi_dev *dev)
  * Calculate symmetric clock as stated in datasheet:
  * twi_clk = F_MAIN / (2 * (cdiv * (1 << ckdiv) + offset))
  */
-static void at91_calc_twi_clock(struct at91_twi_dev *dev, int twi_clk)
+static void at91_calc_twi_clock(struct at91_twi_dev *dev,
+   int twi_clk, u32 twd_hold)
 {
int ckdiv, cdiv, div;
struct at91_twi_pdata *pdata = dev->pdata;
@@ -204,7 +206,9 @@ static void at91_calc_twi_clock(struct at91_twi_dev *dev, 
int twi_clk)
cdiv = 255;
}
 
-   dev->twi_cwgr_reg = (ckdiv << 16) | (cdiv << 8) | cdiv;
+   dev->twi_cwgr_reg = (ckdiv << 16) | (cdiv << 8) | cdiv
+   | AT91_TWI_CWGR_HOLD(twd_hold);
+
dev_dbg(dev->dev, "cdiv %d ckdiv %d\n", cdiv, ckdiv);
 }
 
@@ -994,6 +998,7 @@ static int at91_twi_probe(struct platform_device *pdev)
int rc;
u32 phy_addr;
u32 bus_clk_rate;
+   u32 twd_hold_cycles = 0;
 
dev = devm_kzalloc(>dev, sizeof(*dev), GFP_KERNEL);
if (!dev)
@@ -1050,7 +1055,10 @@ static int at91_twi_probe(struct platform_device *pdev)
if (rc)
bus_clk_rate = DEFAULT_TWI_CLK_HZ;
 
-   at91_calc_twi_clock(dev, bus_clk_rate);
+   of_property_read_u32(dev->dev->of_node, "atmel,twd-hold-cycles",
+_hold_cycles);
+
+   at91_calc_twi_clock(dev, bus_clk_rate, twd_hold_cycles);
at91_init_twi_bus(dev);
 
snprintf(dev->adapter.name, sizeof(dev->adapter.name), "AT91");
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] i2c: at91: add DT property "atmel,twd-hold-cycles" to binding

[Ludovic Desroches]

From: Wenyou Yang <wenyou.y...@atmel.com>

Add a DT property "atmel,twd-hold-cycles" to specify the HOLD
filed of TWIHS_CWGR register to increase the TWD hold time.

Signed-off-by: Wenyou Yang <wenyou.y...@atmel.com>
Signed-off-by: Ludovic Desroches <ludovic.desroc...@atmel.com>
---
 Documentation/devicetree/bindings/i2c/i2c-at91.txt | 4 
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-at91.txt 
b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
index 6e81dc1..c81a0cb 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-at91.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
@@ -17,6 +17,9 @@ Optional properties:
 - dma-names: should contain "tx" and "rx".
 - atmel,fifo-size: maximum number of data the RX and TX FIFOs can store for 
FIFO
   capable I2C controllers.
+- atmel,twd-hold-cycles: number of cycles for TWD hold time whose value is
+  determinated by (atmel,twd-hold-cycles + 3) x t_peripheral_clock,
+  maximum value is 0x1f.
 - Child nodes conforming to i2c bus binding
 
 Examples :
@@ -29,6 +32,7 @@ i2c0: i2c@fff84000 {
#size-cells = <0>;
clocks = <_clk>;
clock-frequency = <40>;
+   atmel,twd-hold-cycles = <2>;
 
24c512@50 {
compatible = "24c512";
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] i2c: at91: manage unexpected RXRDY flag when starting a transfer

[Ludovic Desroches]

In some cases, we could start a new i2c transfer with the RXRDY flag
set. It is not a clean state and it leads to print annoying error
messages even if there no real issue. The cause is only having garbage
data in the Receive Holding Register because of a weird behavior of the
RXRDY flag.

Signed-off-by: Ludovic Desroches <ludovic.desroc...@atmel.com>
Fixes: 93563a6a71bb ("i2c: at91: fix a race condition when using the DMA
controller")
Reported-by: Peter Rosin <p...@lysator.liu.se>
Tested-by: Peter Rosin <p...@lysator.liu.se>
Cc: sta...@vger.kernel.org #4.1
---

Changes from v2:
- fix smatch warning: variable 'sr' set but not used

 drivers/i2c/busses/i2c-at91.c | 36 ++--
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index 94c087b..10835d1 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -347,8 +347,14 @@ error:
 
 static void at91_twi_read_next_byte(struct at91_twi_dev *dev)
 {
-   if (!dev->buf_len)
+   /*
+* If we are in this case, it means there is garbage data in RHR, so
+* delete them.
+*/
+   if (!dev->buf_len) {
+   at91_twi_read(dev, AT91_TWI_RHR);
return;
+   }
 
/* 8bit read works with and without FIFO */
*dev->buf = readb_relaxed(dev->base + AT91_TWI_RHR);
@@ -465,6 +471,24 @@ static irqreturn_t atmel_twi_interrupt(int irq, void 
*dev_id)
 
if (!irqstatus)
return IRQ_NONE;
+   /*
+* In reception, the behavior of the twi device (before sama5d2) is
+* weird. There is some magic about RXRDY flag! When a data has been
+* almost received, the reception of a new one is anticipated if there
+* is no stop command to send. That is the reason why ask for sending
+* the stop command not on the last data but on the second last one.
+*
+* Unfortunately, we could still have the RXRDY flag set even if the
+* transfer is done and we have read the last data. It might happen
+* when the i2c slave device sends too quickly data after receiving the
+* ack from the master. The data has been almost received before having
+* the order to send stop. In this case, sending the stop command could
+* cause a RXRDY interrupt with a TXCOMP one. It is better to manage
+* the RXRDY interrupt first in order to not keep garbage data in the
+* Receive Holding Register for the next transfer.
+*/
+   if (irqstatus & AT91_TWI_RXRDY)
+   at91_twi_read_next_byte(dev);
 
/*
 * When a NACK condition is detected, the I2C controller sets the NACK,
@@ -507,8 +531,6 @@ static irqreturn_t atmel_twi_interrupt(int irq, void 
*dev_id)
if (irqstatus & (AT91_TWI_TXCOMP | AT91_TWI_NACK)) {
at91_disable_twi_interrupts(dev);
complete(>cmd_complete);
-   } else if (irqstatus & AT91_TWI_RXRDY) {
-   at91_twi_read_next_byte(dev);
} else if (irqstatus & AT91_TWI_TXRDY) {
at91_twi_write_next_byte(dev);
}
@@ -525,7 +547,6 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
unsigned long time_left;
bool has_unre_flag = dev->pdata->has_unre_flag;
bool has_alt_cmd = dev->pdata->has_alt_cmd;
-   unsigned sr;
 
/*
 * WARNING: the TXCOMP bit in the Status Register is NOT a clear on
@@ -577,7 +598,7 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
dev->transfer_status = 0;
 
/* Clear pending interrupts, such as NACK. */
-   sr = at91_twi_read(dev, AT91_TWI_SR);
+   at91_twi_read(dev, AT91_TWI_SR);
 
if (dev->fifo_size) {
unsigned fifo_mr = at91_twi_read(dev, AT91_TWI_FMR);
@@ -600,11 +621,6 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
} else if (dev->msg->flags & I2C_M_RD) {
unsigned start_flags = AT91_TWI_START;
 
-   if (sr & AT91_TWI_RXRDY) {
-   dev_err(dev->dev, "RXRDY still set!");
-   at91_twi_read(dev, AT91_TWI_RHR);
-   }
-
/* if only one byte is to be read, immediately stop transfer */
if (!has_alt_cmd && dev->buf_len <= 1 &&
!(dev->msg->flags & I2C_M_RECV_LEN))
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/2] i2c: at91: manage unexpected RXRDY flag when starting a transfer

[Ludovic Desroches]

In some cases, we could start a new i2c transfer with the RXRDY flag
set. It is not a clean state and it leads to print annoying error
messages even if there no real issue. The cause is only having garbage
data in the Receive Holding Register because of a weird behavior of the
RXRDY flag.

Signed-off-by: Ludovic Desroches <ludovic.desroc...@atmel.com>
Fixes: 93563a6a71bb ("i2c: at91: fix a race condition when using the DMA
controller")
Reported-by: Peter Rosin <p...@lysator.liu.se>
Tested-by: Peter Rosin <p...@lysator.liu.se>
Cc: sta...@vger.kernel.org #4.1
---
 drivers/i2c/busses/i2c-at91.c | 33 +
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index 94c087b..bac0016 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -347,8 +347,14 @@ error:
 
 static void at91_twi_read_next_byte(struct at91_twi_dev *dev)
 {
-   if (!dev->buf_len)
+   /*
+* If we are in this case, it means there is garbage data in RHR, so
+* delete them.
+*/
+   if (!dev->buf_len) {
+   at91_twi_read(dev, AT91_TWI_RHR);
return;
+   }
 
/* 8bit read works with and without FIFO */
*dev->buf = readb_relaxed(dev->base + AT91_TWI_RHR);
@@ -465,6 +471,24 @@ static irqreturn_t atmel_twi_interrupt(int irq, void 
*dev_id)
 
if (!irqstatus)
return IRQ_NONE;
+   /*
+* In reception, the behavior of the twi device (before sama5d2) is
+* weird. There is some magic about RXRDY flag! When a data has been
+* almost received, the reception of a new one is anticipated if there
+* is no stop command to send. That is the reason why ask for sending
+* the stop command not on the last data but on the second last one.
+*
+* Unfortunately, we could still have the RXRDY flag set even if the
+* transfer is done and we have read the last data. It might happen
+* when the i2c slave device sends too quickly data after receiving the
+* ack from the master. The data has been almost received before having
+* the order to send stop. In this case, sending the stop command could
+* cause a RXRDY interrupt with a TXCOMP one. It is better to manage
+* the RXRDY interrupt first in order to not keep garbage data in the
+* Receive Holding Register for the next transfer.
+*/
+   if (irqstatus & AT91_TWI_RXRDY)
+   at91_twi_read_next_byte(dev);
 
/*
 * When a NACK condition is detected, the I2C controller sets the NACK,
@@ -507,8 +531,6 @@ static irqreturn_t atmel_twi_interrupt(int irq, void 
*dev_id)
if (irqstatus & (AT91_TWI_TXCOMP | AT91_TWI_NACK)) {
at91_disable_twi_interrupts(dev);
complete(>cmd_complete);
-   } else if (irqstatus & AT91_TWI_RXRDY) {
-   at91_twi_read_next_byte(dev);
} else if (irqstatus & AT91_TWI_TXRDY) {
at91_twi_write_next_byte(dev);
}
@@ -600,11 +622,6 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
} else if (dev->msg->flags & I2C_M_RD) {
unsigned start_flags = AT91_TWI_START;
 
-   if (sr & AT91_TWI_RXRDY) {
-   dev_err(dev->dev, "RXRDY still set!");
-   at91_twi_read(dev, AT91_TWI_RHR);
-   }
-
/* if only one byte is to be read, immediately stop transfer */
if (!has_alt_cmd && dev->buf_len <= 1 &&
!(dev->msg->flags & I2C_M_RECV_LEN))
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/2] i2c: at91: fix write transfers by clearing pending interrupt first

[Ludovic Desroches]

From: Cyrille Pitchen <cyrille.pitc...@atmel.com>

In some cases a NACK interrupt may be pending in the Status Register (SR)
as a result of a previous transfer. However at91_do_twi_transfer() did not
read the SR to clear pending interruptions before starting a new transfer.
Hence a NACK interrupt rose as soon as it was enabled again at the I2C
controller level, resulting in a wrong sequence of operations and strange
patterns of behaviour on the I2C bus, such as a clock stretch followed by
a restart of the transfer.

This first issue occurred with both DMA and PIO write transfers.

Also when a NACK error was detected during a PIO write transfer, the
interrupt handler used to wrongly start a new transfer by writing into the
Transmit Holding Register (THR). Then the I2C slave was likely to reply
with a second NACK.

This second issue is fixed in atmel_twi_interrupt() by handling the TXRDY
status bit only if both the TXCOMP and NACK status bits are cleared.

Tested with a at24 eeprom on sama5d36ek board running a linux-4.1-at91
kernel image. Adapted to linux-next.

Signed-off-by: Cyrille Pitchen <cyrille.pitc...@atmel.com>
Fixes: 93563a6a71bb ("i2c: at91: fix a race condition when using the DMA 
controller")
Reported-by: Peter Rosin <p...@lysator.liu.se>
Signed-off-by: Ludovic Desroches <ludovic.desroc...@atmel.com>
Tested-by: Peter Rosin <p...@lysator.liu.se>
Cc: sta...@vger.kernel.org #4.1
---
 drivers/i2c/busses/i2c-at91.c | 58 +--
 1 file changed, 50 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index 1c758cd..94c087b 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -465,19 +465,57 @@ static irqreturn_t atmel_twi_interrupt(int irq, void 
*dev_id)
 
if (!irqstatus)
return IRQ_NONE;
-   else if (irqstatus & AT91_TWI_RXRDY)
-   at91_twi_read_next_byte(dev);
-   else if (irqstatus & AT91_TWI_TXRDY)
-   at91_twi_write_next_byte(dev);
-
-   /* catch error flags */
-   dev->transfer_status |= status;
 
+   /*
+* When a NACK condition is detected, the I2C controller sets the NACK,
+* TXCOMP and TXRDY bits all together in the Status Register (SR).
+*
+* 1 - Handling NACK errors with CPU write transfer.
+*
+* In such case, we should not write the next byte into the Transmit
+* Holding Register (THR) otherwise the I2C controller would start a new
+* transfer and the I2C slave is likely to reply by another NACK.
+*
+* 2 - Handling NACK errors with DMA write transfer.
+*
+* By setting the TXRDY bit in the SR, the I2C controller also triggers
+* the DMA controller to write the next data into the THR. Then the
+* result depends on the hardware version of the I2C controller.
+*
+* 2a - Without support of the Alternative Command mode.
+*
+* This is the worst case: the DMA controller is triggered to write the
+* next data into the THR, hence starting a new transfer: the I2C slave
+* is likely to reply by another NACK.
+* Concurrently, this interrupt handler is likely to be called to manage
+* the first NACK before the I2C controller detects the second NACK and
+* sets once again the NACK bit into the SR.
+* When handling the first NACK, this interrupt handler disables the I2C
+* controller interruptions, especially the NACK interrupt.
+* Hence, the NACK bit is pending into the SR. This is why we should
+* read the SR to clear all pending interrupts at the beginning of
+* at91_do_twi_transfer() before actually starting a new transfer.
+*
+* 2b - With support of the Alternative Command mode.
+*
+* When a NACK condition is detected, the I2C controller also locks the
+* THR (and sets the LOCK bit in the SR): even though the DMA controller
+* is triggered by the TXRDY bit to write the next data into the THR,
+* this data actually won't go on the I2C bus hence a second NACK is not
+* generated.
+*/
if (irqstatus & (AT91_TWI_TXCOMP | AT91_TWI_NACK)) {
at91_disable_twi_interrupts(dev);
complete(>cmd_complete);
+   } else if (irqstatus & AT91_TWI_RXRDY) {
+   at91_twi_read_next_byte(dev);
+   } else if (irqstatus & AT91_TWI_TXRDY) {
+   at91_twi_write_next_byte(dev);
}
 
+   /* catch error flags */
+   dev->transfer_status |= status;
+
return IRQ_HANDLED;
 }
 
@@ -487,6 +525,7 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
unsigned long time_left;
bool has_unre_flag = dev->pdata->has_unre_flag;
bool has_alt_cmd = dev->pdata->

Re: [PATCH] i2c: at91: fix write transfers by clearing pending interrupt first

[Ludovic Desroches]

On Wed, Oct 21, 2015 at 09:42:40AM +0200, Peter Rosin wrote:
> On 2015-10-21 09:21, Peter Rosin wrote:
> > On 2015-10-20 15:27, Ludovic Desroches wrote:
> >> On Mon, Oct 19, 2015 at 12:49:03PM +0200, Peter Rosin wrote:
> >>> On 2015-10-19 10:51, Ludovic Desroches wrote:
> >>>> Hi Peter,
> >>>>
> >>>> On Fri, Oct 16, 2015 at 11:08:42AM +0200, Peter Rosin wrote:
> >>>>> On 2015-10-16 01:47, Peter Rosin wrote:
> >>>>>> On 2015-10-14 07:43, Ludovic Desroches wrote:
> >>>>>>> On Tue, Oct 13, 2015 at 08:01:34PM +0200, Peter Rosin wrote:
> >>>>>>>> On 2015-10-13 18:47, Cyrille Pitchen wrote:
> >>>>>>>>> Le 13/10/2015 17:19, Peter Rosin a écrit :
> >>>>>>>>>> On 2015-10-13 16:21, Ludovic Desroches wrote:
> >>>>
> >>>> [...]
> >>>>
> >>>>>> I have started to get this when I run with this patch:
> >>>>>>
> >>>>>> [   73.31] at91_i2c f0014000.i2c: RXRDY still set!
> >>>>>> [  198.20] at91_i2c f0014000.i2c: RXRDY still set!
> >>>>>> [  509.88] at91_i2c f0014000.i2c: RXRDY still set!
> >>>>>> [  615.75] at91_i2c f0014000.i2c: RXRDY still set!
> >>>>>> [  617.75] at91_i2c f0014000.i2c: RXRDY still set!
> >>>>>> [ 1766.64] at91_i2c f0014000.i2c: RXRDY still set!
> >>>>>> [ 2035.38] at91_i2c f0014000.i2c: RXRDY still set!
> >>>>>> [ 2227.19] at91_i2c f0014000.i2c: RXRDY still set!
> >>>>>> [ 2313.10] at91_i2c f0014000.i2c: RXRDY still set!
> >>>>>>
> >>>>>> My USB serial dongle was hung which was why I didn't notice until just 
> >>>>>> now.
> >>>>>>
> >>>>>> This is probably not when communication with the eeprom though, and 
> >>>>>> certainly not
> >>>>>> writing to it, but perhpaps when polling the temperature (using the 
> >>>>>> jc42 driver).
> >>>>>> I'll investigate further in the morning to see if I can pinpoint it.
> >>>>>
> >>>>> Yep, it's the jc42 accesses that triggers this (to the same chip as the
> >>>>> eeprom, but a different block of transistors I suppose).
> >>>>>
> >>>>> Looking at the i2c bus, the accesses normally go like this:
> >>>>>
> >>>>> [0.00] S 0x18 W 0x05 S 0x18 R 0xc1 0xbe NACK P
> >>>>> [0.000521] S 0x18 W 0x04 S 0x18 R 0x00 0x00 NACK P
> >>>>> [0.001024] S 0x18 W 0x03 S 0x18 R 0x00 0x00 NACK P
> >>>>> [0.001524] S 0x18 W 0x02 S 0x18 R 0x00 0x00 NACK P
> >>>>> [0.196991] S 0x18 W 0x05 S 0x18 R 0xc1 0xbe NACK P
> >>>>> [0.197514] S 0x18 W 0x04 S 0x18 R 0x00 0x00 NACK P
> >>>>> [0.198019] S 0x18 W 0x03 S 0x18 R 0x00 0x00 NACK P
> >>>>> [0.198520] S 0x18 W 0x02 S 0x18 R 0x00 0x00 NACK P
> >>>>>
> >>>>> I.e. chunks of four transfers every ~200 ms (I removed the 1Hz rate
> >>>>> limiter in the jc42 driver to get more frequent incidents).
> >>>>>
> >>>>> But when the diagnostic (RXRDY still set!) is output it continues
> >>>>> with this:
> >>>>>
> >>>>> [0.399755] S 0x18 W 0x05 S 0x18 R 0xc1 0xbe NACK P
> >>>>> [0.404998] S 0x18 W 0x04 S 0x18 R 0x00 0x00 NACK P
> >>>>> [0.405508] S 0x18 W 0x03 S 0x18 R 0x00 0x00 NACK P
> >>>>> [0.406008] S 0x18 W 0x02 S 0x18 R 0x00 0x00 NACK P
> >>>>>
> >>>>> Notice the ~5 ms delay (about the time it should take to output
> >>>>> the diagnostic at 115200 baud) after the access to register 0x05
> >>>>> at 0.399755.
> >>>>>
> >>>>> This is the only thing that I can observe on the bus, so it appears
> >>>>> to be harmless.
> >>>>>
> >>>>> It appears that the i2c access at 0.399755 finds the TWI
> >>>>> registers in an odd state, but nothing from the access at
> >>>>> 0.198520 appears to have gone wrong. Is this a race? Anyway,
> >>>>> the diagnostic is pretty frequent and annoying. printk_once?
> >>>>
> >>>> I'll try to reproduce it on my side. The only issue 

Re: [PATCH] i2c: at91: fix write transfers by clearing pending interrupt first

[Ludovic Desroches]

On Mon, Oct 19, 2015 at 12:49:03PM +0200, Peter Rosin wrote:
> On 2015-10-19 10:51, Ludovic Desroches wrote:
> > Hi Peter,
> > 
> > On Fri, Oct 16, 2015 at 11:08:42AM +0200, Peter Rosin wrote:
> >> On 2015-10-16 01:47, Peter Rosin wrote:
> >>> On 2015-10-14 07:43, Ludovic Desroches wrote:
> >>>> On Tue, Oct 13, 2015 at 08:01:34PM +0200, Peter Rosin wrote:
> >>>>> On 2015-10-13 18:47, Cyrille Pitchen wrote:
> >>>>>> Le 13/10/2015 17:19, Peter Rosin a écrit :
> >>>>>>> On 2015-10-13 16:21, Ludovic Desroches wrote:
> > 
> > [...]
> > 
> >>> I have started to get this when I run with this patch:
> >>>
> >>> [   73.31] at91_i2c f0014000.i2c: RXRDY still set!
> >>> [  198.20] at91_i2c f0014000.i2c: RXRDY still set!
> >>> [  509.88] at91_i2c f0014000.i2c: RXRDY still set!
> >>> [  615.75] at91_i2c f0014000.i2c: RXRDY still set!
> >>> [  617.75] at91_i2c f0014000.i2c: RXRDY still set!
> >>> [ 1766.64] at91_i2c f0014000.i2c: RXRDY still set!
> >>> [ 2035.38] at91_i2c f0014000.i2c: RXRDY still set!
> >>> [ 2227.19] at91_i2c f0014000.i2c: RXRDY still set!
> >>> [ 2313.10] at91_i2c f0014000.i2c: RXRDY still set!
> >>>
> >>> My USB serial dongle was hung which was why I didn't notice until just 
> >>> now.
> >>>
> >>> This is probably not when communication with the eeprom though, and 
> >>> certainly not
> >>> writing to it, but perhpaps when polling the temperature (using the jc42 
> >>> driver).
> >>> I'll investigate further in the morning to see if I can pinpoint it.
> >>
> >> Yep, it's the jc42 accesses that triggers this (to the same chip as the
> >> eeprom, but a different block of transistors I suppose).
> >>
> >> Looking at the i2c bus, the accesses normally go like this:
> >>
> >> [0.00] S 0x18 W 0x05 S 0x18 R 0xc1 0xbe NACK P
> >> [0.000521] S 0x18 W 0x04 S 0x18 R 0x00 0x00 NACK P
> >> [0.001024] S 0x18 W 0x03 S 0x18 R 0x00 0x00 NACK P
> >> [0.001524] S 0x18 W 0x02 S 0x18 R 0x00 0x00 NACK P
> >> [0.196991] S 0x18 W 0x05 S 0x18 R 0xc1 0xbe NACK P
> >> [0.197514] S 0x18 W 0x04 S 0x18 R 0x00 0x00 NACK P
> >> [0.198019] S 0x18 W 0x03 S 0x18 R 0x00 0x00 NACK P
> >> [0.198520] S 0x18 W 0x02 S 0x18 R 0x00 0x00 NACK P
> >>
> >> I.e. chunks of four transfers every ~200 ms (I removed the 1Hz rate
> >> limiter in the jc42 driver to get more frequent incidents).
> >>
> >> But when the diagnostic (RXRDY still set!) is output it continues
> >> with this:
> >>
> >> [0.399755] S 0x18 W 0x05 S 0x18 R 0xc1 0xbe NACK P
> >> [0.404998] S 0x18 W 0x04 S 0x18 R 0x00 0x00 NACK P
> >> [0.405508] S 0x18 W 0x03 S 0x18 R 0x00 0x00 NACK P
> >> [0.406008] S 0x18 W 0x02 S 0x18 R 0x00 0x00 NACK P
> >>
> >> Notice the ~5 ms delay (about the time it should take to output
> >> the diagnostic at 115200 baud) after the access to register 0x05
> >> at 0.399755.
> >>
> >> This is the only thing that I can observe on the bus, so it appears
> >> to be harmless.
> >>
> >> It appears that the i2c access at 0.399755 finds the TWI
> >> registers in an odd state, but nothing from the access at
> >> 0.198520 appears to have gone wrong. Is this a race? Anyway,
> >> the diagnostic is pretty frequent and annoying. printk_once?
> > 
> > I'll try to reproduce it on my side. The only issue you have is having
> > the message about RXRDY? I mean no issue with i2c transfers?
> 
> Exactly, the only issue is the message, the bus looks good and transfers
> work as they should AFAICT.
> > It is not the possible bug we had in mind, this bug will prevent the
> > master device to release the i2c bus. It will stop the transfer but
> > without sending a stop on the bus.
> 
> Agreed, this is not about the extra frame caused by the spurious write
> to the THR register. This is something else.
> 
> One suspicion is that the driver gets an unexpected irq from its own
> NACK (the one that it puts out to end the read) and races with the
> expected interrupt at TXCOMP?
>

I have discussed with the designer of this IP. So the NACK flag is only
used for 'real' nack not protocol ones.

Concerning the read issue, it have attached a patch (apply it on top of
Cyrille's patch). Could you have a try? I have reproduced your issue only one
time so it's hard for 

Re: [PATCH] i2c: at91: fix write transfers by clearing pending interrupt first

[Ludovic Desroches]

Hi Peter,

On Fri, Oct 16, 2015 at 11:08:42AM +0200, Peter Rosin wrote:
> On 2015-10-16 01:47, Peter Rosin wrote:
> > On 2015-10-14 07:43, Ludovic Desroches wrote:
> >> On Tue, Oct 13, 2015 at 08:01:34PM +0200, Peter Rosin wrote:
> >>> On 2015-10-13 18:47, Cyrille Pitchen wrote:
> >>>> Le 13/10/2015 17:19, Peter Rosin a écrit :
> >>>>> On 2015-10-13 16:21, Ludovic Desroches wrote:

[...]

> > I have started to get this when I run with this patch:
> > 
> > [   73.31] at91_i2c f0014000.i2c: RXRDY still set!
> > [  198.20] at91_i2c f0014000.i2c: RXRDY still set!
> > [  509.88] at91_i2c f0014000.i2c: RXRDY still set!
> > [  615.75] at91_i2c f0014000.i2c: RXRDY still set!
> > [  617.75] at91_i2c f0014000.i2c: RXRDY still set!
> > [ 1766.64] at91_i2c f0014000.i2c: RXRDY still set!
> > [ 2035.38] at91_i2c f0014000.i2c: RXRDY still set!
> > [ 2227.19] at91_i2c f0014000.i2c: RXRDY still set!
> > [ 2313.10] at91_i2c f0014000.i2c: RXRDY still set!
> > 
> > My USB serial dongle was hung which was why I didn't notice until just now.
> > 
> > This is probably not when communication with the eeprom though, and 
> > certainly not
> > writing to it, but perhpaps when polling the temperature (using the jc42 
> > driver).
> > I'll investigate further in the morning to see if I can pinpoint it.
> 
> Yep, it's the jc42 accesses that triggers this (to the same chip as the
> eeprom, but a different block of transistors I suppose).
> 
> Looking at the i2c bus, the accesses normally go like this:
> 
> [0.00] S 0x18 W 0x05 S 0x18 R 0xc1 0xbe NACK P
> [0.000521] S 0x18 W 0x04 S 0x18 R 0x00 0x00 NACK P
> [0.001024] S 0x18 W 0x03 S 0x18 R 0x00 0x00 NACK P
> [0.001524] S 0x18 W 0x02 S 0x18 R 0x00 0x00 NACK P
> [0.196991] S 0x18 W 0x05 S 0x18 R 0xc1 0xbe NACK P
> [0.197514] S 0x18 W 0x04 S 0x18 R 0x00 0x00 NACK P
> [0.198019] S 0x18 W 0x03 S 0x18 R 0x00 0x00 NACK P
> [0.198520] S 0x18 W 0x02 S 0x18 R 0x00 0x00 NACK P
> 
> I.e. chunks of four transfers every ~200 ms (I removed the 1Hz rate
> limiter in the jc42 driver to get more frequent incidents).
> 
> But when the diagnostic (RXRDY still set!) is output it continues
> with this:
> 
> [0.399755] S 0x18 W 0x05 S 0x18 R 0xc1 0xbe NACK P
> [0.404998] S 0x18 W 0x04 S 0x18 R 0x00 0x00 NACK P
> [0.405508] S 0x18 W 0x03 S 0x18 R 0x00 0x00 NACK P
> [0.406008] S 0x18 W 0x02 S 0x18 R 0x00 0x00 NACK P
> 
> Notice the ~5 ms delay (about the time it should take to output
> the diagnostic at 115200 baud) after the access to register 0x05
> at 0.399755.
> 
> This is the only thing that I can observe on the bus, so it appears
> to be harmless.
> 
> It appears that the i2c access at 0.399755 finds the TWI
> registers in an odd state, but nothing from the access at
> 0.198520 appears to have gone wrong. Is this a race? Anyway,
> the diagnostic is pretty frequent and annoying. printk_once?

I'll try to reproduce it on my side. The only issue you have is having
the message about RXRDY? I mean no issue with i2c transfers?

It is not the possible bug we had in mind, this bug will prevent the
master device to release the i2c bus. It will stop the transfer but
without sending a stop on the bus.

Regards

Ludovic
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: at91: fix write transfers by clearing pending interrupt first

[Ludovic Desroches]

On Tue, Oct 13, 2015 at 08:01:34PM +0200, Peter Rosin wrote:
> On 2015-10-13 18:47, Cyrille Pitchen wrote:
> > Le 13/10/2015 17:19, Peter Rosin a écrit :
> >> On 2015-10-13 16:21, Ludovic Desroches wrote:
> >>> From: Cyrille Pitchen <cyrille.pitc...@atmel.com>
> >>>
> >>> In some cases a NACK interrupt may be pending in the Status Register (SR)
> >>> as a result of a previous transfer. However at91_do_twi_transfer() did not
> >>> read the SR to clear pending interruptions before starting a new transfer.
> >>> Hence a NACK interrupt rose as soon as it was enabled again at the I2C
> >>> controller level, resulting in a wrong sequence of operations and strange
> >>> patterns of behaviour on the I2C bus, such as a clock stretch followed by
> >>> a restart of the transfer.
> >>>
> >>> This first issue occurred with both DMA and PIO write transfers.
> >>>
> >>> Also when a NACK error was detected during a PIO write transfer, the
> >>> interrupt handler used to wrongly start a new transfer by writing into the
> >>> Transmit Holding Register (THR). Then the I2C slave was likely to reply
> >>> with a second NACK.
> >>>
> >>> This second issue is fixed in atmel_twi_interrupt() by handling the TXRDY
> >>> status bit only if both the TXCOMP and NACK status bits are cleared.
> >>>
> >>> Tested with a at24 eeprom on sama5d36ek board running a linux-4.1-at91
> >>> kernel image. Adapted to linux-next.
> >>>
> >>> Signed-off-by: Cyrille Pitchen <cyrille.pitc...@atmel.com>
> >>> Fixes: 93563a6a71bb ("i2c: at91: fix a race condition when using the DMA 
> >>> controller")
> >>> Reported-by: Peter Rosin <p...@lysator.liu.se>
> >>> Signed-off-by: Ludovic Desroches <ludovic.desroc...@atmel.com>
> >>> Cc: sta...@vger.kernel.org #4.1
> >>
> >> The regression is gone with this patch (vanilla 4.2), thank you!
> >>
> >> However, looking at the bus, there are two NACKs after each
> >> successful chunk of memory written by the eeprom driver.
> >>
> >> Looking at the eeprom driver, I expect this on the bus:
> >>
> >> S 0x50 0x00 "hello there guys" P
> >> S 0x50 NACK P
> >> delay for at least 1 ms (since the eeprom driver has a msleep(1) call).
> >> S 0x50 NACK P
> >> delay for at least 1 ms
> >> ...
> >> ...
> >> S 0x50 NACK P
> >> delay for at least 1 ms
> >> S 0x50 0x10 "and girls\n" P
> >>
> >> This is not what I observe on the bus, most of the time there is a
> >> second NACK immediately following the first. I suspect that it is
> >> the i2c bus driver that somehow confuses itself and reissues the
> >> command for some reason?
> >>
> >> But this behavior has been there since the beginning, so it's probably
> >> orthogonal, and killing the data corrupting regression is much more
> >> important to me than fussing over a surplus failed transfer. Hence
> >>
> >> Tested-by: Peter Rosin <p...@lysator.liu.se>
> >>
> >> Cheers,
> >> Peter
> >>
> > 
> > Hi Peter,
> > 
> > sama5d3x and sama5d4x don't support the so called "Alternative Command mode"
> > whereas sama5d2x do. The Alternative Command mode comes with a new hardware
> > mechanism inside the I2C controller which locks the transmission of data on
> > the I2C bus when a NACK is detected. It means that even if a data is written
> > into the THR, the I2C controller doesn't push this data on the I2C bus but
> > retains the data in the THR (and its associated FIFO for sama5d2x and future
> > SoCs) until the driver unlocks the transmitter by writing the LOCKCLR (Lock
> > Clear) bit in the Control Register. Then and only then, the transmitter 
> > outputs
> > pending data again.
> > This new mechanism was introduced to cope with an unwanted DMA write into 
> > the
> > THR after a NACK. Indeed, as I've tried to explain in my patch, when a first
> > NACK is detected, the I2C controller sets the TXCOMP, NACK and TXRDY bits
> > alltogether in the Status Register. However setting the TXRDY bit also 
> > triggers
> > the DMA controller to write the next data into the THR. Pitifully, WITHOUT 
> > the
> > new lock mechanism, writing into the THR starts a new I2C frame. Since the
> > eeprom is likely not to be ready yet, it replies by a secon

[PATCH] i2c: at91: fix write transfers by clearing pending interrupt first

[Ludovic Desroches]

From: Cyrille Pitchen <cyrille.pitc...@atmel.com>

In some cases a NACK interrupt may be pending in the Status Register (SR)
as a result of a previous transfer. However at91_do_twi_transfer() did not
read the SR to clear pending interruptions before starting a new transfer.
Hence a NACK interrupt rose as soon as it was enabled again at the I2C
controller level, resulting in a wrong sequence of operations and strange
patterns of behaviour on the I2C bus, such as a clock stretch followed by
a restart of the transfer.

This first issue occurred with both DMA and PIO write transfers.

Also when a NACK error was detected during a PIO write transfer, the
interrupt handler used to wrongly start a new transfer by writing into the
Transmit Holding Register (THR). Then the I2C slave was likely to reply
with a second NACK.

This second issue is fixed in atmel_twi_interrupt() by handling the TXRDY
status bit only if both the TXCOMP and NACK status bits are cleared.

Tested with a at24 eeprom on sama5d36ek board running a linux-4.1-at91
kernel image. Adapted to linux-next.

Signed-off-by: Cyrille Pitchen <cyrille.pitc...@atmel.com>
Fixes: 93563a6a71bb ("i2c: at91: fix a race condition when using the DMA 
controller")
Reported-by: Peter Rosin <p...@lysator.liu.se>
Signed-off-by: Ludovic Desroches <ludovic.desroc...@atmel.com>
Cc: sta...@vger.kernel.org #4.1
---
 drivers/i2c/busses/i2c-at91.c | 58 +--
 1 file changed, 50 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index 1c758cd..94c087b 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -465,19 +465,57 @@ static irqreturn_t atmel_twi_interrupt(int irq, void 
*dev_id)
 
if (!irqstatus)
return IRQ_NONE;
-   else if (irqstatus & AT91_TWI_RXRDY)
-   at91_twi_read_next_byte(dev);
-   else if (irqstatus & AT91_TWI_TXRDY)
-   at91_twi_write_next_byte(dev);
-
-   /* catch error flags */
-   dev->transfer_status |= status;
 
+   /*
+* When a NACK condition is detected, the I2C controller sets the NACK,
+* TXCOMP and TXRDY bits all together in the Status Register (SR).
+*
+* 1 - Handling NACK errors with CPU write transfer.
+*
+* In such case, we should not write the next byte into the Transmit
+* Holding Register (THR) otherwise the I2C controller would start a new
+* transfer and the I2C slave is likely to reply by another NACK.
+*
+* 2 - Handling NACK errors with DMA write transfer.
+*
+* By setting the TXRDY bit in the SR, the I2C controller also triggers
+* the DMA controller to write the next data into the THR. Then the
+* result depends on the hardware version of the I2C controller.
+*
+* 2a - Without support of the Alternative Command mode.
+*
+* This is the worst case: the DMA controller is triggered to write the
+* next data into the THR, hence starting a new transfer: the I2C slave
+* is likely to reply by another NACK.
+* Concurrently, this interrupt handler is likely to be called to manage
+* the first NACK before the I2C controller detects the second NACK and
+* sets once again the NACK bit into the SR.
+* When handling the first NACK, this interrupt handler disables the I2C
+* controller interruptions, especially the NACK interrupt.
+* Hence, the NACK bit is pending into the SR. This is why we should
+* read the SR to clear all pending interrupts at the beginning of
+* at91_do_twi_transfer() before actually starting a new transfer.
+*
+* 2b - With support of the Alternative Command mode.
+*
+* When a NACK condition is detected, the I2C controller also locks the
+* THR (and sets the LOCK bit in the SR): even though the DMA controller
+* is triggered by the TXRDY bit to write the next data into the THR,
+* this data actually won't go on the I2C bus hence a second NACK is not
+* generated.
+*/
if (irqstatus & (AT91_TWI_TXCOMP | AT91_TWI_NACK)) {
at91_disable_twi_interrupts(dev);
complete(>cmd_complete);
+   } else if (irqstatus & AT91_TWI_RXRDY) {
+   at91_twi_read_next_byte(dev);
+   } else if (irqstatus & AT91_TWI_TXRDY) {
+   at91_twi_write_next_byte(dev);
}
 
+   /* catch error flags */
+   dev->transfer_status |= status;
+
return IRQ_HANDLED;
 }
 
@@ -487,6 +525,7 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
unsigned long time_left;
bool has_unre_flag = dev->pdata->has_unre_flag;
bool has_alt_cmd = dev->pdata->has_alt_cmd;
+   unsigned sr;
 
/*
 *

[PATCH 1/2] i2c: at91: add DT property for the HOLD field of TWIHS_CWGR

[Ludovic Desroches]

From: Wenyou Yang <wenyou.y...@atmel.com>

Add the HOLD field setting in order to support I2C slave devices which need
a longer hold time of the data.
Since it depends on the slave devices connected to the bus, add a DT
property "atmel,twd-hold-cycles" to specify this HOLD field.

Signed-off-by: Wenyou Yang <wenyou.y...@atmel.com>
Signed-off-by: Ludovic Desroches <ludovic.desroc...@atmel.com>
---
 drivers/i2c/busses/i2c-at91.c | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index 1c758cd..06e66ef 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -64,6 +64,7 @@
 #defineAT91_TWI_IADR   0x000c  /* Internal Address Register */
 
 #defineAT91_TWI_CWGR   0x0010  /* Clock Waveform Generator Reg 
*/
+#defineAT91_TWI_CWGR_HOLD(x)   (((x) & 0x1f) << 24)
 
 #defineAT91_TWI_SR 0x0020  /* Status Register */
 #defineAT91_TWI_TXCOMP BIT(0)  /* Transmission Complete */
@@ -185,7 +186,8 @@ static void at91_init_twi_bus(struct at91_twi_dev *dev)
  * Calculate symmetric clock as stated in datasheet:
  * twi_clk = F_MAIN / (2 * (cdiv * (1 << ckdiv) + offset))
  */
-static void at91_calc_twi_clock(struct at91_twi_dev *dev, int twi_clk)
+static void at91_calc_twi_clock(struct at91_twi_dev *dev,
+   int twi_clk, u32 twd_hold)
 {
int ckdiv, cdiv, div;
struct at91_twi_pdata *pdata = dev->pdata;
@@ -204,7 +206,9 @@ static void at91_calc_twi_clock(struct at91_twi_dev *dev, 
int twi_clk)
cdiv = 255;
}
 
-   dev->twi_cwgr_reg = (ckdiv << 16) | (cdiv << 8) | cdiv;
+   dev->twi_cwgr_reg = (ckdiv << 16) | (cdiv << 8) | cdiv
+   | AT91_TWI_CWGR_HOLD(twd_hold);
+
dev_dbg(dev->dev, "cdiv %d ckdiv %d\n", cdiv, ckdiv);
 }
 
@@ -936,6 +940,7 @@ static int at91_twi_probe(struct platform_device *pdev)
int rc;
u32 phy_addr;
u32 bus_clk_rate;
+   u32 twd_hold_cycles;
 
dev = devm_kzalloc(>dev, sizeof(*dev), GFP_KERNEL);
if (!dev)
@@ -992,7 +997,12 @@ static int at91_twi_probe(struct platform_device *pdev)
if (rc)
bus_clk_rate = DEFAULT_TWI_CLK_HZ;
 
-   at91_calc_twi_clock(dev, bus_clk_rate);
+   rc = of_property_read_u32(dev->dev->of_node,
+ "atmel,twd-hold-cycles", _hold_cycles);
+   if (rc)
+   twd_hold_cycles = 0;
+
+   at91_calc_twi_clock(dev, bus_clk_rate, twd_hold_cycles);
at91_init_twi_bus(dev);
 
snprintf(dev->adapter.name, sizeof(dev->adapter.name), "AT91");
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] i2c: at91: update documentation for new "atmel,twd-hold-cycles" property

[Ludovic Desroches]

From: Wenyou Yang <wenyou.y...@atmel.com>

Add a DT property "atmel,twd-hold-cycles" to specify the HOLD
field of TWIHS_CWGR register to increase the TWD hold time.

Signed-off-by: Wenyou Yang <wenyou.y...@atmel.com>
Signed-off-by: Ludovic Desroches <ludovic.desroc...@atmel.com>
---
 Documentation/devicetree/bindings/i2c/i2c-at91.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-at91.txt 
b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
index 6e81dc1..f14c0b2 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-at91.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
@@ -17,6 +17,8 @@ Optional properties:
 - dma-names: should contain "tx" and "rx".
 - atmel,fifo-size: maximum number of data the RX and TX FIFOs can store for 
FIFO
   capable I2C controllers.
+- atmel,twd-hold-cycles: increase the TWD hold time by
+  (atmel,twd-hold-cycles + 3) x t_periperal_clk, maximum value is 0x1f.
 - Child nodes conforming to i2c bus binding
 
 Examples :
@@ -29,6 +31,7 @@ i2c0: i2c@fff84000 {
#size-cells = <0>;
clocks = <_clk>;
clock-frequency = <40>;
+   atmel,twd-hold-cycles = <2>;
 
24c512@50 {
compatible = "24c512";
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] i2c: at91: fix code checker warnings

[Ludovic Desroches]

On Thu, Jun 11, 2015 at 11:16:32AM +0200, Cyrille Pitchen wrote:
 buf_len is a size_t, so unsigned but was tested with '= 0'.
 
 Signed-off-by: Cyrille Pitchen cyrille.pitc...@atmel.com

Acked-by: Ludovic Desroches ludovic.desroc...@atmel.com

Thanks

 ---
  drivers/i2c/busses/i2c-at91.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
 index 0d2dc7d..967c0cb 100644
 --- a/drivers/i2c/busses/i2c-at91.c
 +++ b/drivers/i2c/busses/i2c-at91.c
 @@ -232,7 +232,7 @@ static void at91_twi_dma_cleanup(struct at91_twi_dev *dev)
  
  static void at91_twi_write_next_byte(struct at91_twi_dev *dev)
  {
 - if (dev-buf_len = 0)
 + if (!dev-buf_len)
   return;
  
   /* 8bit write works with and without FIFO */
 @@ -275,7 +275,7 @@ static void at91_twi_write_data_dma(struct at91_twi_dev 
 *dev)
   struct dma_chan *chan_tx = dma-chan_tx;
   unsigned int sg_len = 1;
  
 - if (dev-buf_len = 0)
 + if (!dev-buf_len)
   return;
  
   dma-direction = DMA_TO_DEVICE;
 @@ -347,7 +347,7 @@ error:
  
  static void at91_twi_read_next_byte(struct at91_twi_dev *dev)
  {
 - if (dev-buf_len = 0)
 + if (!dev-buf_len)
   return;
  
   /* 8bit read works with and without FIFO */
 -- 
 1.8.2.2
 
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/6] i2c: at91: add support to FIFOs and alternative command

[Ludovic Desroches]

Hi Cyrille,

For the whole serie:
Acked-by: Ludovic Desroches ludovic.desroc...@atmel.com

If you send a new version, could you add stable in Cc to patch 3/6:
Cc: sta...@vger.kernel.org #3.10 and later
If not, Wolfram could you do it please?

Thanks

Ludovic

On Tue, Jun 02, 2015 at 03:18:30PM +0200, Cyrille Pitchen wrote:
 ChangeLog
 
 v3:
 - fix braces {} coding style issue
 - split the alternative command patch into 2 patches: the first one fixes
   a race condition whereas the second one is the actual alternative command
   patch
 
 v2:
 - fix typo in comment for AT91_TWI_SVEN.
 - document new device tree bindings like atmel,fifo-size.
 - explicitly set the has_alt_cmd boolean to false to already existing chip
   configs.
 - use the BIT() macro to define the register bits and do a little cleanup in a
   dedicated patch.
 - reword some comments to better explain why the TXCOMP interrupt is no longer
   enabled in at91_do_twi_transfer() but later in
   at91_twi_write_data_dma_callback() to avoid a race condition when DMA is 
 used.
 - remove useless TXCOMP interrupt enable line in at91_twi_write_next_byte()
   since this interrupt is also enabled by at91_do_twi_transfer() for PIO
   transfers.
 
 v1:
 This series of patches adds support of two new features which will be
 introduced with Atmel sama5d2x SoC.
 
 First, the alternative command mode eases the sending of STOP conditions.
 Before starting an I2C transaction, the size data to be transfered is
 written into the new Alternative Command Register. For each byte transferred,
 the I2C controller decreases this counter and automatically sends a STOP
 condition when the counter value reaches 0, that is to say when the last byte
 of the transaction has been sent/received. So there is no longer need to set
 the STOP bit into the Control Register.
 
 Then the use of FIFOs allows to reduce number I/O accesses: for instance,
 the TX FIFO allows to write up to 4 data in a single access to the Transmit
 Holding Register. Also the RX FIFO allows to read up to 4 data in a single
 access to the Receive Holding Register. Currently only DMA transfers take
 advantage of FIFOs.
 
 Cyrille Pitchen (6):
   i2c: at91: use BIT() macro to define register bits
   i2c: at91: update documentation for DT bindings
   i2c: at91: fix a race condition when using the DMA controller
   i2c: at91: add support for new alternative command mode
   i2c: at91: print hardware version
   i2c: at91: add support to FIFOs
 
  Documentation/devicetree/bindings/i2c/i2c-at91.txt |  29 +-
  drivers/i2c/busses/i2c-at91.c  | 354 
 +
  2 files changed, 321 insertions(+), 62 deletions(-)
 
 -- 
 1.8.2.2
 
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/6] i2c: at91: add support to FIFOs and alternative command

[Ludovic Desroches]

On Wed, Jun 03, 2015 at 12:36:28AM +0900, Wolfram Sang wrote:
 
  If you send a new version, could you add stable in Cc to patch 3/6:
 
 No need to send a new version just because of an additional ack. I can
 apply that. However...
 
  Cc: sta...@vger.kernel.org #3.10 and later
  If not, Wolfram could you do it please?
 
 The problem with 3/6 is that it seems to depend on 1/6 which is a
 cleanup patch. If cleaning up is not essential for the bugfix, the
 latter should be done first. To avoid exactly this problem.
 
 

I was thinking the same so I have tried to cherry-pick the patch on a 3.10.79
and it didn't cause any conflicts so I thought it could be sent to stable as
is.

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] i2c: at91: add support to FIFOs

[Ludovic Desroches]

Hi Cyrille,

Some comments otherwise

Acked-by: Ludovic Desroches ludovic.desroc...@atmel.com

On Fri, May 29, 2015 at 03:50:10PM +0200, Cyrille Pitchen wrote:
 When FIFOs are available and enabled, the driver now configures the Atmel
 eXtended DMA Controller to perform word accesses instead of byte accesses
 when possible.
 The actual access width depends on the size of the buffer to transmit.
 
 To enable FIFO support the atmel,fifo-size property must be set properly
 in the I2C controller node of the device tree.
 

Maybe we should describe this parameter in the device tree binding
documentation.

 Signed-off-by: Cyrille Pitchen cyrille.pitc...@atmel.com
 ---
  drivers/i2c/busses/i2c-at91.c | 146 
 +-
  1 file changed, 129 insertions(+), 17 deletions(-)
 
 diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
 index 1549b29..c061c19 100644
 --- a/drivers/i2c/busses/i2c-at91.c
 +++ b/drivers/i2c/busses/i2c-at91.c
 @@ -54,6 +54,8 @@
  #define  AT91_TWI_THRCLR (1  24) /* Transmit Holding Register 
 Clear */
  #define  AT91_TWI_RHRCLR (1  25) /* Receive Holding Register 
 Clear */
  #define  AT91_TWI_LOCKCLR(1  26) /* Lock Clear */
 +#define  AT91_TWI_FIFOEN (1  28) /* FIFO Enable */
 +#define  AT91_TWI_FIFODIS(1  29) /* FIFO Disable */
  

Use BIT() macro. Cleanup should be done in another patch, see comments
for patch 1/3.

  #define  AT91_TWI_MMR0x0004  /* Master Mode Register */
  #define  AT91_TWI_IADRSZ_1   0x0100  /* Internal Device Address Size 
 */
 @@ -85,6 +87,22 @@
  #define  AT91_TWI_ACR_DATAL(len) ((len)  0xff)
  #define  AT91_TWI_ACR_DIR(1  8)
  
 +#define  AT91_TWI_FMR0x0050  /* FIFO Mode Register */
 +#define  AT91_TWI_FMR_TXRDYM(mode)   (((mode)  0x3)  0)
 +#define  AT91_TWI_FMR_TXRDYM_MASK(0x3  0)
 +#define  AT91_TWI_FMR_RXRDYM(mode)   (((mode)  0x3)  4)
 +#define  AT91_TWI_FMR_RXRDYM_MASK(0x3  4)
 +#define  AT91_TWI_ONE_DATA   0x0
 +#define  AT91_TWI_TWO_DATA   0x1
 +#define  AT91_TWI_FOUR_DATA  0x2
 +
 +#define  AT91_TWI_FLR0x0054  /* FIFO Level Register */
 +
 +#define  AT91_TWI_FSR0x0060  /* FIFO Status Register */
 +#define  AT91_TWI_FIER   0x0064  /* FIFO Interrupt Enable 
 Register */
 +#define  AT91_TWI_FIDR   0x0068  /* FIFO Interrupt Disable 
 Register */
 +#define  AT91_TWI_FIMR   0x006c  /* FIFO Interrupt Mask Register 
 */
 +
  #define  AT91_TWI_VER0x00fc  /* Version Register */
  
  struct at91_twi_pdata {
 @@ -98,7 +116,7 @@ struct at91_twi_pdata {
  struct at91_twi_dma {
   struct dma_chan *chan_rx;
   struct dma_chan *chan_tx;
 - struct scatterlist sg;
 + struct scatterlist sg[2];
   struct dma_async_tx_descriptor *data_desc;
   enum dma_data_direction direction;
   bool buf_mapped;
 @@ -121,6 +139,7 @@ struct at91_twi_dev {
   struct at91_twi_pdata *pdata;
   bool use_dma;
   bool recv_len_abort;
 + u32 fifo_size;
   struct at91_twi_dma dma;
  };
  
 @@ -154,6 +173,9 @@ static void at91_init_twi_bus(struct at91_twi_dev *dev)
  {
   at91_disable_twi_interrupts(dev);
   at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_SWRST);
 + /* FIFO should be enabled immediately after the software reset */
 + if (dev-fifo_size)
 + at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_FIFOEN);
   at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_MSEN);
   at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_SVDIS);
   at91_twi_write(dev, AT91_TWI_CWGR, dev-twi_cwgr_reg);
 @@ -200,7 +222,7 @@ static void at91_twi_dma_cleanup(struct at91_twi_dev *dev)
   dma-xfer_in_progress = false;
   }
   if (dma-buf_mapped) {
 - dma_unmap_single(dev-dev, sg_dma_address(dma-sg),
 + dma_unmap_single(dev-dev, sg_dma_address(dma-sg[0]),
dev-buf_len, dma-direction);
   dma-buf_mapped = false;
   }
 @@ -213,7 +235,8 @@ static void at91_twi_write_next_byte(struct at91_twi_dev 
 *dev)
   if (dev-buf_len = 0)
   return;
  
 - at91_twi_write(dev, AT91_TWI_THR, *dev-buf);
 + /* 8bit write works with and without FIFO */
 + writeb_relaxed(*dev-buf, dev-base + AT91_TWI_THR);
  
   /* send stop when last byte has been written */
   if (--dev-buf_len == 0) {
 @@ -231,7 +254,7 @@ static void at91_twi_write_data_dma_callback(void *data)
  {
   struct at91_twi_dev *dev = (struct at91_twi_dev *)data;
  
 - dma_unmap_single(dev-dev, sg_dma_address(dev-dma.sg),
 + dma_unmap_single(dev-dev, sg_dma_address(dev-dma.sg[0]),
dev-buf_len, DMA_TO_DEVICE);
  
   /*
 @@ -252,6 +275,7 @@ static void at91_twi_write_data_dma(struct at91_twi_dev 
 *dev)
   struct dma_async_tx_descriptor *txdesc

Re: [PATCH 1/3] i2c: at91: add support for new alternative command mode

[Ludovic Desroches]

Hi Cyrille,

Some remarks, questions below.

On Fri, May 29, 2015 at 03:50:08PM +0200, Cyrille Pitchen wrote:
 The alternative command mode was introduced to simplify the transmission of
 STOP conditions and to solve timing and latency issues around them.
 
 This mode relies on a new register, the Alternative Command Register, which
 must be set at the same time as the Master Mode Register. This new register 
 was
 designed to allow simple setup of basic combined transactions built from
 up to two unitary transactions.
 
 Indeed, the ACR is split into two areas, which describe one unitary
 transaction each. Each area is filled with Data Length 8bit counter, a
 Direction and a PEC Request bit. The PEC bit is only used in SMBus mode and is
 not supported by this driver yet. Also when using alternative command mode, 
 the
 MREAD bit from the Master Mode Register is ignored. Instead the Direction bits
 from ACR are used to setup the direction, read or write, of each unitary
 transaction. Finally the 8bit counters must filled with the data length of
 their respective transaction. Then if only one transaction is to be used, the
 data length of the second one must be set to zero. At the moment, this driver
 uses only the first transaction.
 
 In addition to MMR and ACR, the Control Register also need to be written to
 enable the alternative command mode. That's the purpose of its ACMEN bit, 
 which
 stands for Alternative Command Mode Enable.
 
 Note that the alternative command mode is compatible with the use of the
 Internal Address Register. So combined transactions for eeprom read are
 actually implemented with the Internal Address Register. This register is 
 written
 with up to 3 bytes, which are the internal address sent to the slave through
 the first write transaction. Then the first area of the ACR describe the write
 transaction to follow, which carries the data to be read from the eeprom.
 The second area of the ACR is not used so its Data Length 8bit counter is
 cleared.
 
 For each byte sent or received by the device, the Data Length 8bit counter is
 decremented. When it reaches 0, a STOP condition is automatically sent.
 
 Signed-off-by: Cyrille Pitchen cyrille.pitc...@atmel.com
 ---
  drivers/i2c/busses/i2c-at91.c | 203 
 --
  1 file changed, 158 insertions(+), 45 deletions(-)
 
 diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
 index ff23d1b..b48be58 100644
 --- a/drivers/i2c/busses/i2c-at91.c
 +++ b/drivers/i2c/busses/i2c-at91.c
 @@ -41,12 +41,19 @@
  
  /* AT91 TWI register definitions */
  #define  AT91_TWI_CR 0x  /* Control Register */
 -#define  AT91_TWI_START  0x0001  /* Send a Start Condition */
 -#define  AT91_TWI_STOP   0x0002  /* Send a Stop Condition */
 -#define  AT91_TWI_MSEN   0x0004  /* Master Transfer Enable */
 -#define  AT91_TWI_SVDIS  0x0020  /* Slave Transfer Disable */
 -#define  AT91_TWI_QUICK  0x0040  /* SMBus quick command */
 -#define  AT91_TWI_SWRST  0x0080  /* Software Reset */
 +#define  AT91_TWI_START  (1  0)  /* Send a Start Condition */
 +#define  AT91_TWI_STOP   (1  1)  /* Send a Stop Condition */
 +#define  AT91_TWI_MSEN   (1  2)  /* Master Transfer Enable */
 +#define  AT91_TWI_MSDIS  (1  3)  /* Master Transfer Disable */
 +#define  AT91_TWI_SVEN   (1  4)  /* Slave Transfer Disable */

Typo in the comment.

 +#define  AT91_TWI_SVDIS  (1  5)  /* Slave Transfer Disable */
 +#define  AT91_TWI_QUICK  (1  6)  /* SMBus quick command */
 +#define  AT91_TWI_SWRST  (1  7)  /* Software Reset */
 +#define  AT91_TWI_ACMEN  (1  16) /* Alternative Command Mode 
 Enable */
 +#define  AT91_TWI_ACMDIS (1  17) /* Alternative Command Mode 
 Disable */
 +#define  AT91_TWI_THRCLR (1  24) /* Transmit Holding Register 
 Clear */
 +#define  AT91_TWI_RHRCLR (1  25) /* Receive Holding Register 
 Clear */
 +#define  AT91_TWI_LOCKCLR(1  26) /* Lock Clear */
  
  #define  AT91_TWI_MMR0x0004  /* Master Mode Register */
  #define  AT91_TWI_IADRSZ_1   0x0100  /* Internal Device Address Size 
 */
 @@ -57,13 +64,16 @@
  #define  AT91_TWI_CWGR   0x0010  /* Clock Waveform Generator Reg 
 */
  
  #define  AT91_TWI_SR 0x0020  /* Status Register */
 -#define  AT91_TWI_TXCOMP 0x0001  /* Transmission Complete */
 -#define  AT91_TWI_RXRDY  0x0002  /* Receive Holding Register 
 Ready */
 -#define  AT91_TWI_TXRDY  0x0004  /* Transmit Holding Register 
 Ready */
 +#define  AT91_TWI_TXCOMP (1  0)  /* Transmission Complete */
 +#define  AT91_TWI_RXRDY  (1  1)  /* Receive Holding Register 
 Ready */
 +#define  AT91_TWI_TXRDY  (1  2)  /* Transmit Holding Register 
 Ready */
 

Re: [PATCH 2/3] i2c: at91: print hardware version

[Ludovic Desroches]

On Fri, May 29, 2015 at 03:50:09PM +0200, Cyrille Pitchen wrote:
 The probe() function now prints the hardware version of the I2C controller
 
 Signed-off-by: Cyrille Pitchen cyrille.pitc...@atmel.com
Acked-by: Ludovic Desroches ludovic.desroc...@atmel.com

 ---
  drivers/i2c/busses/i2c-at91.c | 4 
  1 file changed, 4 insertions(+)
 
 diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
 index b48be58..1549b29 100644
 --- a/drivers/i2c/busses/i2c-at91.c
 +++ b/drivers/i2c/busses/i2c-at91.c
 @@ -85,6 +85,8 @@
  #define  AT91_TWI_ACR_DATAL(len) ((len)  0xff)
  #define  AT91_TWI_ACR_DIR(1  8)
  
 +#define  AT91_TWI_VER0x00fc  /* Version Register */
 +
  struct at91_twi_pdata {
   unsigned clk_max_div;
   unsigned clk_offset;
 @@ -867,6 +869,8 @@ static int at91_twi_probe(struct platform_device *pdev)
   return rc;
   }
  
 + dev_info(dev-dev, version: 0x%x\n, at91_twi_read(dev, AT91_TWI_VER));
 +
   rc = of_property_read_u32(dev-dev-of_node, clock-frequency,
   bus_clk_rate);
   if (rc)
 -- 
 1.8.2.2
 
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC V2 03/12] i2c: at91: make use of the new infrastructure for quirks

[Ludovic Desroches]

Hi Wolfram,

You can add my 

Acked-by and Tested-By: Ludovic Desroches ludovic.desroc...@atmel.com

Tested on sama5d3, some problems with at24 eeprom on sama5d4 but it
doesn't come from the i2c quirks patch series.

Regards

Ludovic

On Sun, Mar 08, 2015 at 09:28:45AM +0100, Wolfram Sang wrote:
 On Wed, Feb 25, 2015 at 05:01:54PM +0100, Wolfram Sang wrote:
  From: Wolfram Sang wsa+rene...@sang-engineering.com
  
  Signed-off-by: Wolfram Sang wsa+rene...@sang-engineering.com
 
 Hi Ludovic,
 
 if you have a few minutes, could you please test this series? I'd like to
 include it in 4.1. and because at91 is using the quirk infrastructure in
 a more complex way, it is a really good test candidate.
 
 Thanks,
 
Wolfram
 
  ---
   drivers/i2c/busses/i2c-at91.c | 32 +++-
   1 file changed, 11 insertions(+), 21 deletions(-)
  
  diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
  index 636fd2efad8850..b3a70e8fc653c5 100644
  --- a/drivers/i2c/busses/i2c-at91.c
  +++ b/drivers/i2c/busses/i2c-at91.c
  @@ -487,30 +487,10 @@ static int at91_twi_xfer(struct i2c_adapter *adap, 
  struct i2c_msg *msg, int num)
  if (ret  0)
  goto out;
   
  -   /*
  -* The hardware can handle at most two messages concatenated by a
  -* repeated start via it's internal address feature.
  -*/
  -   if (num  2) {
  -   dev_err(dev-dev,
  -   cannot handle more than two concatenated messages.\n);
  -   ret = 0;
  -   goto out;
  -   } else if (num == 2) {
  +   if (num == 2) {
  int internal_address = 0;
  int i;
   
  -   if (msg-flags  I2C_M_RD) {
  -   dev_err(dev-dev, first transfer must be write.\n);
  -   ret = -EINVAL;
  -   goto out;
  -   }
  -   if (msg-len  3) {
  -   dev_err(dev-dev, first message size must be = 3.\n);
  -   ret = -EINVAL;
  -   goto out;
  -   }
  -
  /* 1st msg is put into the internal address, start with 2nd */
  m_start = msg[1];
  for (i = 0; i  msg-len; ++i) {
  @@ -540,6 +520,15 @@ out:
  return ret;
   }
   
  +/*
  + * The hardware can handle at most two messages concatenated by a
  + * repeated start via it's internal address feature.
  + */
  +static struct i2c_adapter_quirks at91_twi_quirks = {
  +   .flags = I2C_AQ_COMB | I2C_AQ_COMB_WRITE_FIRST | I2C_AQ_COMB_SAME_ADDR,
  +   .max_comb_1st_msg_len = 3,
  +};
  +
   static u32 at91_twi_func(struct i2c_adapter *adapter)
   {
  return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL
  @@ -777,6 +766,7 @@ static int at91_twi_probe(struct platform_device *pdev)
  dev-adapter.owner = THIS_MODULE;
  dev-adapter.class = I2C_CLASS_DEPRECATED;
  dev-adapter.algo = at91_twi_algorithm;
  +   dev-adapter.quirks = at91_twi_quirks;
  dev-adapter.dev.parent = dev-dev;
  dev-adapter.nr = pdev-id;
  dev-adapter.timeout = AT91_I2C_TIMEOUT;
  -- 
  2.1.4
  


--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC V2 03/12] i2c: at91: make use of the new infrastructure for quirks

[Ludovic Desroches]

Hi Wolfram,

On Sun, Mar 08, 2015 at 09:28:45AM +0100, Wolfram Sang wrote:
 On Wed, Feb 25, 2015 at 05:01:54PM +0100, Wolfram Sang wrote:
  From: Wolfram Sang wsa+rene...@sang-engineering.com
  
  Signed-off-by: Wolfram Sang wsa+rene...@sang-engineering.com
 
 Hi Ludovic,
 
 if you have a few minutes, could you please test this series? I'd like to
 include it in 4.1. and because at91 is using the quirk infrastructure in
 a more complex way, it is a really good test candidate.

It was in the pipe. I have reviewed it, this second version seems to be
good. I am just waiting a bit more to give you my ack since I have some
issues to read an i2c eeprom (it works with a temperature sensor).
I am investigating if it doesn't come from a previous regression.

 
 Thanks,
 
Wolfram
 

Regards

Ludovic
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: at91: fixup return type of wait_for_completion_timeout

[Ludovic Desroches]

On Sun, Feb 08, 2015 at 11:12:07AM -0500, Nicholas Mc Guire wrote:
 Return type of wait_for_completion_timeout is unsigned long not int. This
 patch adds a timeout variable of appropriate type and fixes up the assignment.
 
 Signed-off-by: Nicholas Mc Guire hof...@osadl.org
Acked-by: Ludovic Desroches ludovic.desroc...@atmel.com

Thanks

 ---
 
 Patch was only compile tested with at91_dt_defconfig
 (implies CONFIG_I2C_AT91=y)
 
 Patch is against 3.19.0-rc7 (localversion-next is -next-20150204)
 
  drivers/i2c/busses/i2c-at91.c |7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
 index 636fd2e..79c6404 100644
 --- a/drivers/i2c/busses/i2c-at91.c
 +++ b/drivers/i2c/busses/i2c-at91.c
 @@ -381,6 +381,7 @@ static irqreturn_t atmel_twi_interrupt(int irq, void 
 *dev_id)
  static int at91_do_twi_transfer(struct at91_twi_dev *dev)
  {
   int ret;
 + unsigned long timeout;
   bool has_unre_flag = dev-pdata-has_unre_flag;
  
   dev_dbg(dev-dev, transfer: %s %d bytes.\n,
 @@ -436,9 +437,9 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
   }
   }
  
 - ret = wait_for_completion_timeout(dev-cmd_complete,
 -  dev-adapter.timeout);
 - if (ret == 0) {
 + timeout = wait_for_completion_timeout(dev-cmd_complete,
 +   dev-adapter.timeout);
 + if (timeout == 0) {
   dev_err(dev-dev, controller timed out\n);
   at91_init_twi_bus(dev);
   ret = -ETIMEDOUT;
 -- 
 1.7.10.4
 
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 02/11] i2c: add quirk checks to core

[Ludovic Desroches]

Hi Wolfram,

On Fri, Jan 09, 2015 at 06:21:32PM +0100, Wolfram Sang wrote:
 Let the core do the checks if HW quirks prevent a transfer. Saves code
 from drivers and adds consistency.
 
 Signed-off-by: Wolfram Sang w...@the-dreams.de
 ---
  drivers/i2c/i2c-core.c | 53 
 ++
  1 file changed, 53 insertions(+)
 
 diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
 index 39d25a8cb1ad..7b10a19abf5b 100644
 --- a/drivers/i2c/i2c-core.c
 +++ b/drivers/i2c/i2c-core.c
 @@ -2063,6 +2063,56 @@ module_exit(i2c_exit);
   * 
   */
  
 +/* Check if val is exceeding the quirk IFF quirk is non 0 */
 +#define i2c_quirk_exceeded(val, quirk) ((quirk)  ((val)  (quirk)))
 +
 +static int i2c_quirk_error(struct i2c_adapter *adap, struct i2c_msg *msg, 
 char *err_msg)
 +{
 + dev_err(adap-dev, quirk: %s (addr 0x%04x, size %u)\n, err_msg, 
 msg-addr, msg-len);
 + return -EOPNOTSUPP;
 +}
 +
 +static int i2c_check_for_quirks(struct i2c_adapter *adap, struct i2c_msg 
 *msgs, int num)
 +{
 + struct i2c_adapter_quirks *q = adap-quirks;
 + u16 max_read = q-max_read_len, max_write = q-max_write_len;
 + int max_num = q-max_num_msgs, i;
 +
 + if (q-flags  I2C_ADAPTER_QUIRK_COMB_WRITE_THEN_READ)
 + max_num = 2;
 +
 + if (i2c_quirk_exceeded(num, max_num))
 + return i2c_quirk_error(adap, msgs[0], too many messages);
 +
 + if (num == 2  q-flags  I2C_ADAPTER_QUIRK_COMB_WRITE_FIRST) {
 + if (msgs[0].flags  I2C_M_RD)
 + return i2c_quirk_error(adap, msgs[0], invalid first 
 write msg);
 +
 + max_write = q-max_comb_write_len;
 + }
 +
 + if (num == 2  q-flags  I2C_ADAPTER_QUIRK_COMB_READ_SECOND) {
 + if (!(msgs[1].flags  I2C_M_RD) || msgs[0].addr != msgs[1].addr)
 + return i2c_quirk_error(adap, msgs[1], invalid second 
 read msg);
 +
 + max_read = q-max_comb_read_len;
 + }
 +
 + for (i = 0; i  num; i++) {
 + u16 len = msgs[i].len;
 +
 + if (msgs[i].flags  I2C_M_RD) {
 + if (i2c_quirk_exceeded(len, max_read))
 + return i2c_quirk_error(adap, msgs[i], msg too 
 long);
 + } else {
 + if (i2c_quirk_exceeded(len, max_write))
 + return i2c_quirk_error(adap, msgs[i], msg too 
 long);
 + }
 + }
 +

I am not sure it will perfectly fit at91 quirks.

The hardware can handle two messages by using the internal address
feature. The internal address size is from one byte to three bytes. Then
the length of the first message is limited to three but we don't have
this constraint for the second one. If we have 'write then read' no problem
but if we have two write messages, the second one will cause a quirk
exceeded error.

Regards

Ludovic

 + return 0;
 +}
 +
  /**
   * __i2c_transfer - unlocked flavor of i2c_transfer
   * @adap: Handle to I2C bus
 @@ -2080,6 +2130,9 @@ int __i2c_transfer(struct i2c_adapter *adap, struct 
 i2c_msg *msgs, int num)
   unsigned long orig_jiffies;
   int ret, try;
  
 + if (adap-quirks  i2c_check_for_quirks(adap, msgs, num))
 + return -EOPNOTSUPP;
 +
   /* i2c_trace_msg gets enabled when tracepoint i2c_transfer gets
* enabled.  This is an efficient way of keeping the for-loop from
* being executed when not needed.
 -- 
 2.1.3
 
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] at91: i2c-at91: improve time-out handling

[Ludovic Desroches]

Hi Douglas,

On Thu, Jan 01, 2015 at 01:02:13PM -0500, Douglas Gilbert wrote:
 With lk 3.19.0-rc2 and a at91sam9g25 (9x5) based system I
 connected a NXP SC16IS750 I2C to serial bridge. After
 routing the 750's IRQ back to the sc16is7xx driver and some
 simple successful test, it was time for some intense testing:
 Tx looped back to Rx on the 750, open picocom on /dev/ttySC0
 at 38400, and use hexdump to blast a binary file (in hex) at
 ttySC0. The I2C SCL speed was 200,000 Hz.
 
 It worked as expected for a few seconds then it wedged the
 I2C bus. That was repeatable. In the cases that I checked SCL
 was high, SDA was low (driven by _both_ the G25's macrocell
 and the 750!!) and IRQ was active (low). This patch stopped
 the G25 macrocell from driving SDA low in the above wedge
 (and stopped copious error reports going to the log). I was
 surprised that a NXP I2C chip got into this situation, IMO
 SDA on a slave should have a driven low timeout. IMO all
 I2C master drivers should have provision to drive a gpio
 connected to a (or all the) slave's RESET line(s).
 
 
 ChangeLog:
when handling an I2C bus time-out, first clean-up the
DMA transfer, then do an I2C macrocell software reset
and restore some registers, including the interrupt
mask
 

I am wondering why you need to call at91_twi_irq_save() and
at91_twi_irq_restore(). The interrupts enabled in the driver are
AT91_TWI_TXCOMP, AT91_TWI_RXRDY and AT91_TWI_TXRDY and they are managed
in at91_do_twi_transfer() so they would be set correctly for the next
transfer.

Regards

Ludovic

 Signed-off-by: Douglas Gilbert dgilb...@interlog.com

 diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
 index 636fd2e..4d78708 100644
 --- a/drivers/i2c/busses/i2c-at91.c
 +++ b/drivers/i2c/busses/i2c-at91.c
 @@ -382,6 +382,7 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
  {
   int ret;
   bool has_unre_flag = dev-pdata-has_unre_flag;
 + bool timed_out = false;
  
   dev_dbg(dev-dev, transfer: %s %d bytes.\n,
   (dev-msg-flags  I2C_M_RD) ? read : write, dev-buf_len);
 @@ -440,7 +441,7 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
dev-adapter.timeout);
   if (ret == 0) {
   dev_err(dev-dev, controller timed out\n);
 - at91_init_twi_bus(dev);
 + timed_out = true;
   ret = -ETIMEDOUT;
   goto error;
   }
 @@ -471,6 +472,11 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
  
  error:
   at91_twi_dma_cleanup(dev);
 + if (timed_out) {
 + at91_twi_irq_save(dev);
 + at91_init_twi_bus(dev);
 + at91_twi_irq_restore(dev);
 + }
   return ret;
  }
  

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] i2c: at91: remove legacy DMA support

[Ludovic Desroches]

From: Arnd Bergmann a...@arndb.de

Since at91sam9g45 is now DT-only, all DMA capable users of this driver
are using the DT case, and the legacy support can be removed.

Signed-off-by: Arnd Bergmann a...@arndb.de
Signed-off-by: Ludovic Desroches ludovic.desroc...@atmel.com
---

Hi,

I have split the legacy dma support removing and probe deferring. I have also
fixed the compilation issue.

 drivers/i2c/busses/i2c-at91.c | 37 +++--
 1 file changed, 3 insertions(+), 34 deletions(-)

diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index 917d545..b69ea9b 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -72,7 +72,6 @@ struct at91_twi_pdata {
unsigned clk_max_div;
unsigned clk_offset;
bool has_unre_flag;
-   bool has_dma_support;
struct at_dma_slave dma_slave;
 };
 
@@ -541,35 +540,30 @@ static struct at91_twi_pdata at91rm9200_config = {
.clk_max_div = 5,
.clk_offset = 3,
.has_unre_flag = true,
-   .has_dma_support = false,
 };
 
 static struct at91_twi_pdata at91sam9261_config = {
.clk_max_div = 5,
.clk_offset = 4,
.has_unre_flag = false,
-   .has_dma_support = false,
 };
 
 static struct at91_twi_pdata at91sam9260_config = {
.clk_max_div = 7,
.clk_offset = 4,
.has_unre_flag = false,
-   .has_dma_support = false,
 };
 
 static struct at91_twi_pdata at91sam9g20_config = {
.clk_max_div = 7,
.clk_offset = 4,
.has_unre_flag = false,
-   .has_dma_support = false,
 };
 
 static struct at91_twi_pdata at91sam9g10_config = {
.clk_max_div = 7,
.clk_offset = 4,
.has_unre_flag = false,
-   .has_dma_support = false,
 };
 
 static const struct platform_device_id at91_twi_devtypes[] = {
@@ -598,7 +592,6 @@ static struct at91_twi_pdata at91sam9x5_config = {
.clk_max_div = 7,
.clk_offset = 4,
.has_unre_flag = false,
-   .has_dma_support = true,
 };
 
 static const struct of_device_id atmel_twi_dt_ids[] = {
@@ -627,30 +620,11 @@ static const struct of_device_id atmel_twi_dt_ids[] = {
 MODULE_DEVICE_TABLE(of, atmel_twi_dt_ids);
 #endif
 
-static bool filter(struct dma_chan *chan, void *pdata)
-{
-   struct at91_twi_pdata *sl_pdata = pdata;
-   struct at_dma_slave *sl;
-
-   if (!sl_pdata)
-   return false;
-
-   sl = sl_pdata-dma_slave;
-   if (sl  (sl-dma_dev == chan-device-dev)) {
-   chan-private = sl;
-   return true;
-   } else {
-   return false;
-   }
-}
-
 static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr)
 {
int ret = 0;
-   struct at91_twi_pdata *pdata = dev-pdata;
struct dma_slave_config slave_config;
struct at91_twi_dma *dma = dev-dma;
-   dma_cap_mask_t mask;
 
memset(slave_config, 0, sizeof(slave_config));
slave_config.src_addr = (dma_addr_t)phy_addr + AT91_TWI_RHR;
@@ -661,19 +635,14 @@ static int at91_twi_configure_dma(struct at91_twi_dev 
*dev, u32 phy_addr)
slave_config.dst_maxburst = 1;
slave_config.device_fc = false;
 
-   dma_cap_zero(mask);
-   dma_cap_set(DMA_SLAVE, mask);
-
-   dma-chan_tx = dma_request_slave_channel_compat(mask, filter, pdata,
-   dev-dev, tx);
+   dma-chan_tx = dma_request_slave_channel(dev-dev, tx);
if (!dma-chan_tx) {
dev_err(dev-dev, can't get a DMA channel for tx\n);
ret = -EBUSY;
goto error;
}
 
-   dma-chan_rx = dma_request_slave_channel_compat(mask, filter, pdata,
-   dev-dev, rx);
+   dma-chan_rx = dma_request_slave_channel(dev-dev, rx);
if (!dma-chan_rx) {
dev_err(dev-dev, can't get a DMA channel for rx\n);
ret = -EBUSY;
@@ -772,7 +741,7 @@ static int at91_twi_probe(struct platform_device *pdev)
}
clk_prepare_enable(dev-clk);
 
-   if (dev-pdata-has_dma_support) {
+   if (dev-dev-of_node) {
if (at91_twi_configure_dma(dev, phy_addr) == 0)
dev-use_dma = true;
}
-- 
2.0.3

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] i2c: at91: enable probe deferring on dma channel request

[Ludovic Desroches]

If dma controller is not probed, defer i2c probe.

Signed-off-by: Ludovic Desroches ludovic.desroc...@atmel.com
---

Arnd,

It's a combination of the first patch I sent and yours. As you said that my
patch looks wrong but actually it's ok I didn't dare to add your
signed-off-by.

 drivers/i2c/busses/i2c-at91.c | 25 ++---
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index b69ea9b..3e56d73 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -635,17 +635,17 @@ static int at91_twi_configure_dma(struct at91_twi_dev 
*dev, u32 phy_addr)
slave_config.dst_maxburst = 1;
slave_config.device_fc = false;
 
-   dma-chan_tx = dma_request_slave_channel(dev-dev, tx);
-   if (!dma-chan_tx) {
-   dev_err(dev-dev, can't get a DMA channel for tx\n);
-   ret = -EBUSY;
+   dma-chan_tx = dma_request_slave_channel_reason(dev-dev, tx);
+   if (IS_ERR(dma-chan_tx)) {
+   ret = PTR_ERR(dma-chan_tx);
+   dma-chan_tx = NULL;
goto error;
}
 
-   dma-chan_rx = dma_request_slave_channel(dev-dev, rx);
-   if (!dma-chan_rx) {
-   dev_err(dev-dev, can't get a DMA channel for rx\n);
-   ret = -EBUSY;
+   dma-chan_rx = dma_request_slave_channel_reason(dev-dev, rx);
+   if (IS_ERR(dma-chan_rx)) {
+   ret = PTR_ERR(dma-chan_rx);
+   dma-chan_rx = NULL;
goto error;
}
 
@@ -666,6 +666,7 @@ static int at91_twi_configure_dma(struct at91_twi_dev *dev, 
u32 phy_addr)
sg_init_table(dma-sg, 1);
dma-buf_mapped = false;
dma-xfer_in_progress = false;
+   dev-use_dma = true;
 
dev_info(dev-dev, using %s (tx) and %s (rx) for DMA transfers\n,
 dma_chan_name(dma-chan_tx), dma_chan_name(dma-chan_rx));
@@ -673,7 +674,8 @@ static int at91_twi_configure_dma(struct at91_twi_dev *dev, 
u32 phy_addr)
return ret;
 
 error:
-   dev_info(dev-dev, can't use DMA\n);
+   if (ret != -EPROBE_DEFER)
+   dev_info(dev-dev, can't use DMA, error %d\n, ret);
if (dma-chan_rx)
dma_release_channel(dma-chan_rx);
if (dma-chan_tx)
@@ -742,8 +744,9 @@ static int at91_twi_probe(struct platform_device *pdev)
clk_prepare_enable(dev-clk);
 
if (dev-dev-of_node) {
-   if (at91_twi_configure_dma(dev, phy_addr) == 0)
-   dev-use_dma = true;
+   rc = at91_twi_configure_dma(dev, phy_addr);
+   if (rc == -EPROBE_DEFER)
+   return rc;
}
 
rc = of_property_read_u32(dev-dev-of_node, clock-frequency,
-- 
2.0.3

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: at91: introduce probe deferring

[Ludovic Desroches]

On Wed, Nov 19, 2014 at 10:47:15AM +0100, Arnd Bergmann wrote:
 On Wednesday 19 November 2014 10:16:47 Wolfram Sang wrote:
  On Fri, Nov 14, 2014 at 02:47:59PM +0100, Ludovic Desroches wrote:
   Return probe defer if requesting a dma channel without a dma controller 
   probed.
   
   Signed-off-by: Ludovic Desroches ludovic.desroc...@atmel.com
   ---
drivers/i2c/busses/i2c-at91.c | 22 --
1 file changed, 16 insertions(+), 6 deletions(-)
   
   diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
   index 77fb647..df3f4c4 100644
   --- a/drivers/i2c/busses/i2c-at91.c
   +++ b/drivers/i2c/busses/i2c-at91.c
   @@ -679,14 +679,21 @@ static int at91_twi_configure_dma(struct 
   at91_twi_dev *dev, u32 phy_addr)
 dma_cap_zero(mask);
 dma_cap_set(DMA_SLAVE, mask);

   - dma-chan_tx = dma_request_slave_channel_compat(mask, filter, pdata,
   - dev-dev, tx);
   - if (!dma-chan_tx) {
   + dma-chan_tx = dma_request_slave_channel_reason(dev-dev, tx);
  
  Will it cause regressions if you drop the compat-version of requesting
  a channel?
 
 I got curious about this, since the patch looks obviously wrong, but
 actually it's ok. However the entire DMA support for non-DT platforms
 can be dropped in this driver, see patch below
 
   + if (IS_ERR(dma-chan_tx)) {
   + ret = PTR_ERR(dma-chan_tx);
   + if (ret == -EPROBE_DEFER) {
   + dev_warn(dev-dev, no DMA channel available at the 
   moment\n);
  
  I'd say drop this warning. The core usually prints when deferred probing
  takes place.
 
 Definitely yes.
 
   Arnd
 8---
 [PATCH] i2c: at91: remove legacy DMA supoprt
 
 Since at91sam9g45 is now DT-only, all DMA capable users of this driver are
 using the DT case, and the legacy support can be removed. While at it, fix
 the deferred probe case.
 
 Signed-off-by: Arnd Bergmann a...@arndb.de
Acked-by: Ludovic Desroches ludovic.desroc...@atmel.com

Thanks

 
 diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
 index 917d54588d95..534f4c07bfb6 100644
 --- a/drivers/i2c/busses/i2c-at91.c
 +++ b/drivers/i2c/busses/i2c-at91.c
 @@ -72,8 +72,6 @@ struct at91_twi_pdata {
   unsigned clk_max_div;
   unsigned clk_offset;
   bool has_unre_flag;
 - bool has_dma_support;
 - struct at_dma_slave dma_slave;
  };
  
  struct at91_twi_dma {
 @@ -541,35 +539,30 @@ static struct at91_twi_pdata at91rm9200_config = {
   .clk_max_div = 5,
   .clk_offset = 3,
   .has_unre_flag = true,
 - .has_dma_support = false,
  };
  
  static struct at91_twi_pdata at91sam9261_config = {
   .clk_max_div = 5,
   .clk_offset = 4,
   .has_unre_flag = false,
 - .has_dma_support = false,
  };
  
  static struct at91_twi_pdata at91sam9260_config = {
   .clk_max_div = 7,
   .clk_offset = 4,
   .has_unre_flag = false,
 - .has_dma_support = false,
  };
  
  static struct at91_twi_pdata at91sam9g20_config = {
   .clk_max_div = 7,
   .clk_offset = 4,
   .has_unre_flag = false,
 - .has_dma_support = false,
  };
  
  static struct at91_twi_pdata at91sam9g10_config = {
   .clk_max_div = 7,
   .clk_offset = 4,
   .has_unre_flag = false,
 - .has_dma_support = false,
  };
  
  static const struct platform_device_id at91_twi_devtypes[] = {
 @@ -598,7 +591,6 @@ static struct at91_twi_pdata at91sam9x5_config = {
   .clk_max_div = 7,
   .clk_offset = 4,
   .has_unre_flag = false,
 - .has_dma_support = true,
  };
  
  static const struct of_device_id atmel_twi_dt_ids[] = {
 @@ -627,30 +619,11 @@ static const struct of_device_id atmel_twi_dt_ids[] = {
  MODULE_DEVICE_TABLE(of, atmel_twi_dt_ids);
  #endif
  
 -static bool filter(struct dma_chan *chan, void *pdata)
 -{
 - struct at91_twi_pdata *sl_pdata = pdata;
 - struct at_dma_slave *sl;
 -
 - if (!sl_pdata)
 - return false;
 -
 - sl = sl_pdata-dma_slave;
 - if (sl  (sl-dma_dev == chan-device-dev)) {
 - chan-private = sl;
 - return true;
 - } else {
 - return false;
 - }
 -}
 -
  static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr)
  {
   int ret = 0;
 - struct at91_twi_pdata *pdata = dev-pdata;
   struct dma_slave_config slave_config;
   struct at91_twi_dma *dma = dev-dma;
 - dma_cap_mask_t mask;
  
   memset(slave_config, 0, sizeof(slave_config));
   slave_config.src_addr = (dma_addr_t)phy_addr + AT91_TWI_RHR;
 @@ -661,22 +634,17 @@ static int at91_twi_configure_dma(struct at91_twi_dev 
 *dev, u32 phy_addr)
   slave_config.dst_maxburst = 1;
   slave_config.device_fc = false;
  
 - dma_cap_zero(mask);
 - dma_cap_set(DMA_SLAVE, mask);
 -
 - dma-chan_tx = dma_request_slave_channel_compat(mask, filter, pdata,
 - dev-dev, tx);
 - if (!dma-chan_tx

[PATCH] i2c: at91: introduce probe deferring

[Ludovic Desroches]

Return probe defer if requesting a dma channel without a dma controller probed.

Signed-off-by: Ludovic Desroches ludovic.desroc...@atmel.com
---
 drivers/i2c/busses/i2c-at91.c | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index 77fb647..df3f4c4 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -679,14 +679,21 @@ static int at91_twi_configure_dma(struct at91_twi_dev 
*dev, u32 phy_addr)
dma_cap_zero(mask);
dma_cap_set(DMA_SLAVE, mask);
 
-   dma-chan_tx = dma_request_slave_channel_compat(mask, filter, pdata,
-   dev-dev, tx);
-   if (!dma-chan_tx) {
+   dma-chan_tx = dma_request_slave_channel_reason(dev-dev, tx);
+   if (IS_ERR(dma-chan_tx)) {
+   ret = PTR_ERR(dma-chan_tx);
+   if (ret == -EPROBE_DEFER) {
+   dev_warn(dev-dev, no DMA channel available at the 
moment\n);
+   return ret;
+   }
dev_err(dev-dev, can't get a DMA channel for tx\n);
-   ret = -EBUSY;
goto error;
}
 
+   /*
+* No reason to check EPROBE_DEFER here since we have already request
+* tx channel. If it fails here, it's for another reason.
+*/
dma-chan_rx = dma_request_slave_channel_compat(mask, filter, pdata,
dev-dev, rx);
if (!dma-chan_rx) {
@@ -722,7 +729,7 @@ error:
dev_info(dev-dev, can't use DMA\n);
if (dma-chan_rx)
dma_release_channel(dma-chan_rx);
-   if (dma-chan_tx)
+   if (!IS_ERR(dma-chan_tx))
dma_release_channel(dma-chan_tx);
return ret;
 }
@@ -788,8 +795,11 @@ static int at91_twi_probe(struct platform_device *pdev)
clk_prepare_enable(dev-clk);
 
if (dev-pdata-has_dma_support) {
-   if (at91_twi_configure_dma(dev, phy_addr) == 0)
+   rc = at91_twi_configure_dma(dev, phy_addr);
+   if (rc == 0)
dev-use_dma = true;
+   else if (rc == -EPROBE_DEFER)
+   return rc;
}
 
rc = of_property_read_u32(dev-dev-of_node, clock-frequency,
-- 
2.0.3

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/3] i2c/at91: add support PM functions

[Ludovic Desroches]

For the whole serie

Acked-by: Ludovic Desroches ludovic.desroc...@atmel.com

On Fri, Oct 24, 2014 at 02:50:14PM +0800, Wenyou Yang wrote:
 Hi Wolfram,
 
 The patches is to add the PM functions support for the at91 i2c controller.
 
 It is based on the i2c/for-next branch of 
 git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git.
 
 Best Regards,
 Wenyou Yang
 
 --
 Change log:
 v2.0
 
 According to the advice from Kevin Hilman, 
 1./ Wrap the runtime suspend/resume functions in CONFIG_PM instead of 
 CONFIG_PM_RUNTIME.
 2./ Call the runtime suspend/resume functions directly in the system 
 suspend/resume.
 
 v3.0
Covert the system suspend/resume to suspend_noirq/resume_noirq.
 
 Wenyou Yang (3):
   i2c/at91: add support for runtime PM
   i2c/at91: add support for system PM
   i2c/at91: adopt pinctrl support
 
  drivers/i2c/busses/i2c-at91.c |   71 
 +
  1 file changed, 65 insertions(+), 6 deletions(-)
 
 -- 
 1.7.9.5
 
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] i2c:at91: add bound checking on SMBus block length bytes

[Ludovic Desroches]

Hi,

On Thu, Oct 23, 2014 at 09:28:55AM -0400, Mark Roszko wrote:
 Hi Ludovic,
 
 Wolfram took in the patch quietly awhile back. It's in the 3.17 kernel
 and backported to some of the older trees.
 https://github.com/torvalds/linux/commit/75b81f339c6af43f6f4a1b3eabe0603321dade65

Great, sorry for the noise so. It seems I have checked it from a wrong
branch.

Ludovic

 
 Regards,
 Mark
 --
 To unsubscribe from this list: send the line unsubscribe linux-i2c in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] i2c:at91: add bound checking on SMBus block length bytes

[Ludovic Desroches]

Hi Wolfram,

Could you take this patch into your tree?

Thanks

Regards

Ludovic

On Wed, Aug 20, 2014 at 09:39:41PM -0400, Marek Roszko wrote:
 The driver was not bound checking the received length byte to ensure it was 
 within the
 the buffer size that is allocated for SMBus blocks. This resulted in buffer 
 overflows
 whenever an invalid length byte was received.
 It also failed to ensure the length byte was not zero. If it received zero, 
 it would end up
 in an infinite loop as the at91_twi_read_next_byte function returned 
 immediately without
 allowing RHR to be read to clear the RXRDY interrupt.
 
 Tested agaisnt a SMBus compliant battery.
 
 Signed-off-by: Marek Roszko mark.ros...@gmail.com
 Acked-by: Ludovic Desroches ludovic.desroc...@atmel.com
 ---
 Change from v1:
 fixed typo in commit message
 reworded message slightly to be specifically say length byte
 
  drivers/i2c/busses/i2c-at91.c |   28 
  1 file changed, 24 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
 index e95f9ba..ec4ff33 100644
 --- a/drivers/i2c/busses/i2c-at91.c
 +++ b/drivers/i2c/busses/i2c-at91.c
 @@ -101,6 +101,7 @@ struct at91_twi_dev {
   unsigned twi_cwgr_reg;
   struct at91_twi_pdata *pdata;
   bool use_dma;
 + bool recv_len_abort;
   struct at91_twi_dma dma;
  };
  
 @@ -267,12 +268,24 @@ static void at91_twi_read_next_byte(struct at91_twi_dev 
 *dev)
   *dev-buf = at91_twi_read(dev, AT91_TWI_RHR)  0xff;
   --dev-buf_len;
  
 + /* return if aborting, we only needed to read RHR to clear RXRDY*/
 + if (dev-recv_len_abort)
 + return;
 +
   /* handle I2C_SMBUS_BLOCK_DATA */
   if (unlikely(dev-msg-flags  I2C_M_RECV_LEN)) {
 - dev-msg-flags = ~I2C_M_RECV_LEN;
 - dev-buf_len += *dev-buf;
 - dev-msg-len = dev-buf_len + 1;
 - dev_dbg(dev-dev, received block length %d\n, dev-buf_len);
 + /* ensure length byte is a valid value */
 + if (*dev-buf = I2C_SMBUS_BLOCK_MAX  *dev-buf  0) {
 + dev-msg-flags = ~I2C_M_RECV_LEN;
 + dev-buf_len += *dev-buf;
 + dev-msg-len = dev-buf_len + 1;
 + dev_dbg(dev-dev, received block length %d\n,
 +  dev-buf_len);
 + } else {
 + /* abort and send the stop by reading one more byte */
 + dev-recv_len_abort = true;
 + dev-buf_len = 1;
 + }
   }
  
   /* send stop if second but last byte has been read */
 @@ -444,6 +457,12 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
   ret = -EIO;
   goto error;
   }
 + if (dev-recv_len_abort) {
 + dev_err(dev-dev, invalid smbus block length recvd\n);
 + ret = -EPROTO;
 + goto error;
 + }
 +
   dev_dbg(dev-dev, transfer complete\n);
  
   return 0;
 @@ -500,6 +519,7 @@ static int at91_twi_xfer(struct i2c_adapter *adap, struct 
 i2c_msg *msg, int num)
   dev-buf_len = m_start-len;
   dev-buf = m_start-buf;
   dev-msg = m_start;
 + dev-recv_len_abort = false;
  
   ret = at91_do_twi_transfer(dev);
  
 -- 
 1.7.10.4
 
 
 ___
 linux-arm-kernel mailing list
 linux-arm-ker...@lists.infradead.org
 http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] i2c/at91: add support for runtime PM

[Ludovic Desroches]

Hi Wenyou,

On Mon, Oct 20, 2014 at 11:42:12AM +0800, Wenyou Yang wrote:
 Drivers should put the device into low power states proactively whenever the
 device is not in use. Thus implement support for runtime PM and use the
 autosuspend feature to make sure that we can still perform well in case we see
 lots of i2c traffic within short period of time.
 
 Signed-off-by: Wenyou Yang wenyou.y...@atmel.com
 ---
  drivers/i2c/busses/i2c-at91.c |   48 
 -
  1 file changed, 38 insertions(+), 10 deletions(-)
 
 diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
 index 917d545..03b9f48 100644
 --- a/drivers/i2c/busses/i2c-at91.c
 +++ b/drivers/i2c/busses/i2c-at91.c
 @@ -31,10 +31,12 @@
  #include linux/platform_device.h
  #include linux/slab.h
  #include linux/platform_data/dma-atmel.h
 +#include linux/pm_runtime.h
  
  #define DEFAULT_TWI_CLK_HZ   10  /* max 400 Kbits/s */
  #define AT91_I2C_TIMEOUT msecs_to_jiffies(100)   /* transfer timeout */
  #define AT91_I2C_DMA_THRESHOLD   8   /* enable DMA 
 if transfer size is bigger than this threshold */
 +#define AUTOSUSPEND_TIMEOUT  2000
  
  /* AT91 TWI register definitions */
  #define  AT91_TWI_CR 0x  /* Control Register */
 @@ -475,12 +477,16 @@ error:
  static int at91_twi_xfer(struct i2c_adapter *adap, struct i2c_msg *msg, int 
 num)
  {
   struct at91_twi_dev *dev = i2c_get_adapdata(adap);
 - int ret;
 + int ret = 0;

Not necessary.

   unsigned int_addr_flag = 0;
   struct i2c_msg *m_start = msg;
  
   dev_dbg(adap-dev, at91_xfer: processing %d messages:\n, num);
  
 + ret = pm_runtime_get_sync(dev-dev);
 + if (ret  0)
 + goto out;
 +
   /*
* The hardware can handle at most two messages concatenated by a
* repeated start via it's internal address feature.
 @@ -488,18 +494,21 @@ static int at91_twi_xfer(struct i2c_adapter *adap, 
 struct i2c_msg *msg, int num)
   if (num  2) {
   dev_err(dev-dev,
   cannot handle more than two concatenated messages.\n);
 - return 0;
 + ret = 0;
 + goto out;
   } else if (num == 2) {
   int internal_address = 0;
   int i;
  
   if (msg-flags  I2C_M_RD) {
   dev_err(dev-dev, first transfer must be write.\n);
 - return -EINVAL;
 + ret = -EINVAL;
 + goto out;
   }
   if (msg-len  3) {
   dev_err(dev-dev, first message size must be = 3.\n);
 - return -EINVAL;
 + ret = -EINVAL;
 + goto out;
   }
  
   /* 1st msg is put into the internal address, start with 2nd */
 @@ -523,7 +532,13 @@ static int at91_twi_xfer(struct i2c_adapter *adap, 
 struct i2c_msg *msg, int num)
  
   ret = at91_do_twi_transfer(dev);
  
 - return (ret  0) ? ret : num;
 + if (ret == 0)

I don't figure out why you've changed this condition.

 + ret = num;
 +out:
 + pm_runtime_mark_last_busy(dev-dev);
 + pm_runtime_put_autosuspend(dev-dev);
 +
 + return ret;
  }
  
  static u32 at91_twi_func(struct i2c_adapter *adapter)
 @@ -795,11 +810,20 @@ static int at91_twi_probe(struct platform_device *pdev)
   dev-adapter.timeout = AT91_I2C_TIMEOUT;
   dev-adapter.dev.of_node = pdev-dev.of_node;
  
 + pm_runtime_set_autosuspend_delay(dev-dev, AUTOSUSPEND_TIMEOUT);
 + pm_runtime_use_autosuspend(dev-dev);
 + pm_runtime_set_active(dev-dev);
 + pm_runtime_enable(dev-dev);
 +
   rc = i2c_add_numbered_adapter(dev-adapter);
   if (rc) {
   dev_err(dev-dev, Adapter %s registration failed\n,
   dev-adapter.name);
   clk_disable_unprepare(dev-clk);
 +
 + pm_runtime_disable(dev-dev);
 + pm_runtime_set_suspended(dev-dev);
 +
   return rc;
   }
  
 @@ -814,16 +838,19 @@ static int at91_twi_remove(struct platform_device *pdev)
   i2c_del_adapter(dev-adapter);
   clk_disable_unprepare(dev-clk);
  
 + pm_runtime_disable(dev-dev);
 + pm_runtime_set_suspended(dev-dev);
 +
   return 0;
  }
  
  #ifdef CONFIG_PM
 -
 +#ifdef CONFIG_PM_RUNTIME
  static int at91_twi_runtime_suspend(struct device *dev)
  {
   struct at91_twi_dev *twi_dev = dev_get_drvdata(dev);
  
 - clk_disable(twi_dev-clk);
 + clk_disable_unprepare(twi_dev-clk);
  
   return 0;
  }
 @@ -832,12 +859,13 @@ static int at91_twi_runtime_resume(struct device *dev)
  {
   struct at91_twi_dev *twi_dev = dev_get_drvdata(dev);
  
 - return clk_enable(twi_dev-clk);
 + return clk_prepare_enable(twi_dev-clk);
  }
 +#endif
  
  static const struct dev_pm_ops at91_twi_pm = {
 - .runtime_suspend= at91_twi_runtime_suspend,
 

Re: [PATCH 2/3] i2c/at91: add support for system PM

[Ludovic Desroches]

On Mon, Oct 20, 2014 at 11:42:13AM +0800, Wenyou Yang wrote:
 Signed-off-by: Wenyou Yang wenyou.y...@atmel.com

Acked by: Ludovic Desroches ludovic.desroc...@atmel.com

 ---
  drivers/i2c/busses/i2c-at91.c |   30 ++
  1 file changed, 30 insertions(+)
 
 diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
 index 03b9f48..8f408f8 100644
 --- a/drivers/i2c/busses/i2c-at91.c
 +++ b/drivers/i2c/busses/i2c-at91.c
 @@ -845,6 +845,35 @@ static int at91_twi_remove(struct platform_device *pdev)
  }
  
  #ifdef CONFIG_PM
 +#ifdef CONFIG_PM_SLEEP
 +static int at91_twi_suspend(struct device *dev)
 +{
 + struct at91_twi_dev *twi_dev = dev_get_drvdata(dev);
 +
 + if (!pm_runtime_suspended(dev))
 + clk_disable_unprepare(twi_dev-clk);
 +
 + return 0;
 +}
 +
 +static int at91_twi_resume(struct device *dev)
 +{
 + struct at91_twi_dev *twi_dev = dev_get_drvdata(dev);
 + int ret;
 +
 + if (!pm_runtime_suspended(dev)) {
 + ret = clk_prepare_enable(twi_dev-clk);
 + if (ret)
 + return ret;
 + }
 +
 + pm_runtime_mark_last_busy(dev);
 + pm_request_autosuspend(dev);
 +
 + return 0;
 +}
 +#endif
 +
  #ifdef CONFIG_PM_RUNTIME
  static int at91_twi_runtime_suspend(struct device *dev)
  {
 @@ -864,6 +893,7 @@ static int at91_twi_runtime_resume(struct device *dev)
  #endif
  
  static const struct dev_pm_ops at91_twi_pm = {
 + SET_SYSTEM_SLEEP_PM_OPS(at91_twi_suspend, at91_twi_resume)
   SET_RUNTIME_PM_OPS(at91_twi_runtime_suspend,
   at91_twi_runtime_resume, NULL)
  };
 -- 
 1.7.9.5
 
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] i2c/at91: adopt pinctrl support

[Ludovic Desroches]

On Mon, Oct 20, 2014 at 11:42:14AM +0800, Wenyou Yang wrote:
 Amend the i2c at91 pin controller to optionally take a pin control
 handle and set the state of the pins to:
 
 - default on boot, resume and before performing an transfer
 - sleep on suspend()
 
 This should make it possible to optimize energy usage for the pins
 both for the suspend/resume cycle
 
 Signed-off-by: Wenyou Yang wenyou.y...@atmel.com

Acked by: Ludovic Desroches ludovic.desroc...@atmel.com

 ---
  drivers/i2c/busses/i2c-at91.c |   12 +++-
  1 file changed, 11 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
 index 8f408f8..ed2c382 100644
 --- a/drivers/i2c/busses/i2c-at91.c
 +++ b/drivers/i2c/busses/i2c-at91.c
 @@ -32,6 +32,7 @@
  #include linux/slab.h
  #include linux/platform_data/dma-atmel.h
  #include linux/pm_runtime.h
 +#include linux/pinctrl/consumer.h
  
  #define DEFAULT_TWI_CLK_HZ   10  /* max 400 Kbits/s */
  #define AT91_I2C_TIMEOUT msecs_to_jiffies(100)   /* transfer timeout */
 @@ -748,6 +749,8 @@ static int at91_twi_probe(struct platform_device *pdev)
   u32 phy_addr;
   u32 bus_clk_rate;
  
 + pinctrl_pm_select_default_state(pdev-dev);
 +
   dev = devm_kzalloc(pdev-dev, sizeof(*dev), GFP_KERNEL);
   if (!dev)
   return -ENOMEM;
 @@ -850,8 +853,10 @@ static int at91_twi_suspend(struct device *dev)
  {
   struct at91_twi_dev *twi_dev = dev_get_drvdata(dev);
  
 - if (!pm_runtime_suspended(dev))
 + if (!pm_runtime_suspended(dev)) {
   clk_disable_unprepare(twi_dev-clk);
 + pinctrl_pm_select_sleep_state(dev);
 + }
  
   return 0;
  }
 @@ -862,6 +867,7 @@ static int at91_twi_resume(struct device *dev)
   int ret;
  
   if (!pm_runtime_suspended(dev)) {
 + pinctrl_pm_select_default_state(dev);
   ret = clk_prepare_enable(twi_dev-clk);
   if (ret)
   return ret;
 @@ -881,6 +887,8 @@ static int at91_twi_runtime_suspend(struct device *dev)
  
   clk_disable_unprepare(twi_dev-clk);
  
 + pinctrl_pm_select_sleep_state(dev);
 +
   return 0;
  }
  
 @@ -888,6 +896,8 @@ static int at91_twi_runtime_resume(struct device *dev)
  {
   struct at91_twi_dev *twi_dev = dev_get_drvdata(dev);
  
 + pinctrl_pm_select_default_state(dev);
 +
   return clk_prepare_enable(twi_dev-clk);
  }
  #endif
 -- 
 1.7.9.5
 
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] i2c/at91: add support for system PM

[Ludovic Desroches]

Adding Kevin in the CC list since he had some comments about the PM
runtime support for the SPI driver.

On Mon, Oct 20, 2014 at 02:42:42PM +0200, Ludovic Desroches wrote:
 On Mon, Oct 20, 2014 at 11:42:13AM +0800, Wenyou Yang wrote:
  Signed-off-by: Wenyou Yang wenyou.y...@atmel.com
 
 Acked by: Ludovic Desroches ludovic.desroc...@atmel.com
 
  ---
   drivers/i2c/busses/i2c-at91.c |   30 ++
   1 file changed, 30 insertions(+)
  
  diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
  index 03b9f48..8f408f8 100644
  --- a/drivers/i2c/busses/i2c-at91.c
  +++ b/drivers/i2c/busses/i2c-at91.c
  @@ -845,6 +845,35 @@ static int at91_twi_remove(struct platform_device 
  *pdev)
   }
   
   #ifdef CONFIG_PM
  +#ifdef CONFIG_PM_SLEEP
  +static int at91_twi_suspend(struct device *dev)
  +{
  +   struct at91_twi_dev *twi_dev = dev_get_drvdata(dev);
  +
  +   if (!pm_runtime_suspended(dev))
  +   clk_disable_unprepare(twi_dev-clk);
  +
  +   return 0;
  +}
  +
  +static int at91_twi_resume(struct device *dev)
  +{
  +   struct at91_twi_dev *twi_dev = dev_get_drvdata(dev);
  +   int ret;
  +
  +   if (!pm_runtime_suspended(dev)) {
  +   ret = clk_prepare_enable(twi_dev-clk);
  +   if (ret)
  +   return ret;
  +   }
  +
  +   pm_runtime_mark_last_busy(dev);
  +   pm_request_autosuspend(dev);
  +
  +   return 0;
  +}
  +#endif
  +
   #ifdef CONFIG_PM_RUNTIME
   static int at91_twi_runtime_suspend(struct device *dev)
   {
  @@ -864,6 +893,7 @@ static int at91_twi_runtime_resume(struct device *dev)
   #endif
   
   static const struct dev_pm_ops at91_twi_pm = {
  +   SET_SYSTEM_SLEEP_PM_OPS(at91_twi_suspend, at91_twi_resume)
  SET_RUNTIME_PM_OPS(at91_twi_runtime_suspend,
  at91_twi_runtime_resume, NULL)
   };
  -- 
  1.7.9.5
  
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] i2c/at91: add support for runtime PM

[Ludovic Desroches]

Adding Kevin in the CC list since he had some comments about the PM
runtime support for the SPI driver.

On Mon, Oct 20, 2014 at 02:39:14PM +0200, Ludovic Desroches wrote:
 Hi Wenyou,
 
 On Mon, Oct 20, 2014 at 11:42:12AM +0800, Wenyou Yang wrote:
  Drivers should put the device into low power states proactively whenever the
  device is not in use. Thus implement support for runtime PM and use the
  autosuspend feature to make sure that we can still perform well in case we 
  see
  lots of i2c traffic within short period of time.
  
  Signed-off-by: Wenyou Yang wenyou.y...@atmel.com
  ---
   drivers/i2c/busses/i2c-at91.c |   48 
  -
   1 file changed, 38 insertions(+), 10 deletions(-)
  
  diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
  index 917d545..03b9f48 100644
  --- a/drivers/i2c/busses/i2c-at91.c
  +++ b/drivers/i2c/busses/i2c-at91.c
  @@ -31,10 +31,12 @@
   #include linux/platform_device.h
   #include linux/slab.h
   #include linux/platform_data/dma-atmel.h
  +#include linux/pm_runtime.h
   
   #define DEFAULT_TWI_CLK_HZ 10  /* max 400 Kbits/s */
   #define AT91_I2C_TIMEOUT   msecs_to_jiffies(100)   /* transfer timeout */
   #define AT91_I2C_DMA_THRESHOLD 8   /* enable DMA 
  if transfer size is bigger than this threshold */
  +#define AUTOSUSPEND_TIMEOUT2000
   
   /* AT91 TWI register definitions */
   #defineAT91_TWI_CR 0x  /* Control Register */
  @@ -475,12 +477,16 @@ error:
   static int at91_twi_xfer(struct i2c_adapter *adap, struct i2c_msg *msg, 
  int num)
   {
  struct at91_twi_dev *dev = i2c_get_adapdata(adap);
  -   int ret;
  +   int ret = 0;
 
 Not necessary.
 
  unsigned int_addr_flag = 0;
  struct i2c_msg *m_start = msg;
   
  dev_dbg(adap-dev, at91_xfer: processing %d messages:\n, num);
   
  +   ret = pm_runtime_get_sync(dev-dev);
  +   if (ret  0)
  +   goto out;
  +
  /*
   * The hardware can handle at most two messages concatenated by a
   * repeated start via it's internal address feature.
  @@ -488,18 +494,21 @@ static int at91_twi_xfer(struct i2c_adapter *adap, 
  struct i2c_msg *msg, int num)
  if (num  2) {
  dev_err(dev-dev,
  cannot handle more than two concatenated messages.\n);
  -   return 0;
  +   ret = 0;
  +   goto out;
  } else if (num == 2) {
  int internal_address = 0;
  int i;
   
  if (msg-flags  I2C_M_RD) {
  dev_err(dev-dev, first transfer must be write.\n);
  -   return -EINVAL;
  +   ret = -EINVAL;
  +   goto out;
  }
  if (msg-len  3) {
  dev_err(dev-dev, first message size must be = 3.\n);
  -   return -EINVAL;
  +   ret = -EINVAL;
  +   goto out;
  }
   
  /* 1st msg is put into the internal address, start with 2nd */
  @@ -523,7 +532,13 @@ static int at91_twi_xfer(struct i2c_adapter *adap, 
  struct i2c_msg *msg, int num)
   
  ret = at91_do_twi_transfer(dev);
   
  -   return (ret  0) ? ret : num;
  +   if (ret == 0)
 
 I don't figure out why you've changed this condition.
 
  +   ret = num;
  +out:
  +   pm_runtime_mark_last_busy(dev-dev);
  +   pm_runtime_put_autosuspend(dev-dev);
  +
  +   return ret;
   }
   
   static u32 at91_twi_func(struct i2c_adapter *adapter)
  @@ -795,11 +810,20 @@ static int at91_twi_probe(struct platform_device 
  *pdev)
  dev-adapter.timeout = AT91_I2C_TIMEOUT;
  dev-adapter.dev.of_node = pdev-dev.of_node;
   
  +   pm_runtime_set_autosuspend_delay(dev-dev, AUTOSUSPEND_TIMEOUT);
  +   pm_runtime_use_autosuspend(dev-dev);
  +   pm_runtime_set_active(dev-dev);
  +   pm_runtime_enable(dev-dev);
  +
  rc = i2c_add_numbered_adapter(dev-adapter);
  if (rc) {
  dev_err(dev-dev, Adapter %s registration failed\n,
  dev-adapter.name);
  clk_disable_unprepare(dev-clk);
  +
  +   pm_runtime_disable(dev-dev);
  +   pm_runtime_set_suspended(dev-dev);
  +
  return rc;
  }
   
  @@ -814,16 +838,19 @@ static int at91_twi_remove(struct platform_device 
  *pdev)
  i2c_del_adapter(dev-adapter);
  clk_disable_unprepare(dev-clk);
   
  +   pm_runtime_disable(dev-dev);
  +   pm_runtime_set_suspended(dev-dev);
  +
  return 0;
   }
   
   #ifdef CONFIG_PM
  -
  +#ifdef CONFIG_PM_RUNTIME
   static int at91_twi_runtime_suspend(struct device *dev)
   {
  struct at91_twi_dev *twi_dev = dev_get_drvdata(dev);
   
  -   clk_disable(twi_dev-clk);
  +   clk_disable_unprepare(twi_dev-clk);
   
  return 0;
   }
  @@ -832,12 +859,13 @@ static int at91_twi_runtime_resume(struct device *dev)
   {
  struct at91_twi_dev *twi_dev = dev_get_drvdata(dev);
   
  -   return

Re: [PATCH] i2c:at91: Fix a race condition during signal handling in at91_do_twi_xfer.

[Ludovic Desroches]

On Tue, Aug 26, 2014 at 09:13:24PM +0200, Simon Lindgren wrote:
 There is a race condition in at91_do_twi_xfer when signals arrive.
 If a signal is recieved while waiting for a transfer to complete
 wait_for_completion_interruptible_timeout() will return -ERESTARTSYS.
 This is not handled correctly resulting in interrupts still being
 enabled and a transfer being in flight when we return.
 
 Symptoms include a range of oopses and bus lockups. Oopses can happen
 when the transfer completes because the interrupt handler will corrupt
 the stack. If a new transfer is started before the interrupt fires
 the controller will start a new transfer in the middle of the old one,
 resulting in confused slaves and a locked bus.
 
 To avoid this, use wait_for_completion_io_timeout instead so that we
 don't have to deal with gracefully shutting down the transfer and
 disabling the interrupts.
 
 Signed-off-by: Simon Lindgren si...@aqwary.com
Acked-by: Ludovic Desroches ludovic.desroc...@atmel.com

Thanks for this fix.

 ---
  drivers/i2c/busses/i2c-at91.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
 index 79a6899..ec299ae 100644
 --- a/drivers/i2c/busses/i2c-at91.c
 +++ b/drivers/i2c/busses/i2c-at91.c
 @@ -421,8 +421,8 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
   }
   }
  
 - ret = wait_for_completion_interruptible_timeout(dev-cmd_complete,
 - dev-adapter.timeout);
 + ret = wait_for_completion_io_timeout(dev-cmd_complete,
 +  dev-adapter.timeout);
   if (ret == 0) {
   dev_err(dev-dev, controller timed out\n);
   at91_init_twi_bus(dev);
 -- 
 1.7.9.5
 
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c:at91: add bound checking on smbus block length bytes

[Ludovic Desroches]

Hi Mark,

On Mon, Aug 04, 2014 at 12:29:56PM -0400, Mark Roszko wrote:
 Hi Ludovic,
 
 Have you had a chance to look? This causes kernel panic in at91 systems in
 noisy environments and/or unreliable smbus slaves if left unfixed.

Thanks for the reminder, I had totally missed this patch.

Seems good, no objection to include this patch.

Only one comment: s/recieved/received otherwise

Acked-By: Ludovic Desroches ludovic.desroc...@atmel.com

Sorry for the delay.

Ludovic
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] drivers/i2c/busses: use correct type for dma_map/unmap

[Ludovic Desroches]

On Mon, Jul 21, 2014 at 11:42:03AM +0200, Wolfram Sang wrote:
 dma_{un}map_* uses 'enum dma_data_direction' not 'enum 
 dma_transfer_direction'.
 
 Signed-off-by: Wolfram Sang w...@the-dreams.de
Acked-by: Ludovic Desroches ludovic.desroc...@atmel.com

Thanks Wolfram.

 ---
  drivers/i2c/busses/i2c-at91.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
 index e95f9ba96790..83c989382be9 100644
 --- a/drivers/i2c/busses/i2c-at91.c
 +++ b/drivers/i2c/busses/i2c-at91.c
 @@ -210,7 +210,7 @@ static void at91_twi_write_data_dma_callback(void *data)
   struct at91_twi_dev *dev = (struct at91_twi_dev *)data;
  
   dma_unmap_single(dev-dev, sg_dma_address(dev-dma.sg),
 -  dev-buf_len, DMA_MEM_TO_DEV);
 +  dev-buf_len, DMA_TO_DEVICE);
  
   at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_STOP);
  }
 @@ -289,7 +289,7 @@ static void at91_twi_read_data_dma_callback(void *data)
   struct at91_twi_dev *dev = (struct at91_twi_dev *)data;
  
   dma_unmap_single(dev-dev, sg_dma_address(dev-dma.sg),
 -  dev-buf_len, DMA_DEV_TO_MEM);
 +  dev-buf_len, DMA_FROM_DEVICE);
  
   /* The last two bytes have to be read without using dma */
   dev-buf += dev-buf_len - 2;
 -- 
 2.0.0
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-i2c in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/16] i2c: i2c-at91: Drop class based scanning to improve bootup time

[Ludovic Desroches]

On Thu, Jul 10, 2014 at 01:46:22PM +0200, Wolfram Sang wrote:
 This driver has been flagged to drop class based instantiation. The removal
 improves boot-up time and is unneeded for embedded controllers. Users have 
 been
 warned to switch for some time now, so we can actually do the removal. Keep 
 the
 DEPRECATED flag, so the core can inform users that the behaviour finally
 changed now. After another transition period, this flag can go, too.
 
 Signed-off-by: Wolfram Sang w...@the-dreams.de
Acked-by: Ludovic Desroches ludovic.desroc...@atmel.com

Thanks

 ---
  drivers/i2c/busses/i2c-at91.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
 index e95f9ba96790..7033e9166d1f 100644
 --- a/drivers/i2c/busses/i2c-at91.c
 +++ b/drivers/i2c/busses/i2c-at91.c
 @@ -768,7 +768,7 @@ static int at91_twi_probe(struct platform_device *pdev)
   snprintf(dev-adapter.name, sizeof(dev-adapter.name), AT91);
   i2c_set_adapdata(dev-adapter, dev);
   dev-adapter.owner = THIS_MODULE;
 - dev-adapter.class = I2C_CLASS_HWMON | I2C_CLASS_DEPRECATED;
 + dev-adapter.class = I2C_CLASS_DEPRECATED;
   dev-adapter.algo = at91_twi_algorithm;
   dev-adapter.dev.parent = dev-dev;
   dev-adapter.nr = pdev-id;
 -- 
 2.0.0
 
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] i2c:at91: Add device tree property to set clock-frequency

[Ludovic Desroches]

On Mon, Feb 24, 2014 at 08:50:45PM -0500, Marek Roszko wrote:
 This adds the ability to set clock-frequency in the device tree for the i2c 
 bus following
 the naming of other i2c bus implementations. If the property is not set, the 
 clock
 frequency will default to the previously used define of 100KHz.
 
 Signed-off-by: Marek Roszko mark.ros...@gmail.com

Acked-by: Ludovic Desroches ludovic.desroc...@atmel.com

Thanks

 ---
 v2:
   -fixed return code usage and check to not compare agaisnt less than zero
 
 ---
  Documentation/devicetree/bindings/i2c/i2c-at91.txt |2 ++
  drivers/i2c/busses/i2c-at91.c  |   10 --
  2 files changed, 10 insertions(+), 2 deletions(-)
 
 diff --git a/Documentation/devicetree/bindings/i2c/i2c-at91.txt 
 b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
 index 4fade84..388f0a2 100644
 --- a/Documentation/devicetree/bindings/i2c/i2c-at91.txt
 +++ b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
 @@ -12,6 +12,7 @@ Required properties :
  - clocks: phandles to input clocks.
  
  Optional properties:
 +- clock-frequency: Desired I2C bus frequency in Hz, otherwise defaults to 
 10
  - Child nodes conforming to i2c bus binding
  
  Examples :
 @@ -23,6 +24,7 @@ i2c0: i2c@fff84000 {
   #address-cells = 1;
   #size-cells = 0;
   clocks = twi0_clk;
 + clock-frequency = 40;
  
   24c512@50 {
   compatible = 24c512;
 diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
 index 843d012..a407050 100644
 --- a/drivers/i2c/busses/i2c-at91.c
 +++ b/drivers/i2c/busses/i2c-at91.c
 @@ -32,7 +32,7 @@
  #include linux/slab.h
  #include linux/platform_data/dma-atmel.h
  
 -#define TWI_CLK_HZ   10  /* max 400 Kbits/s */
 +#define DEFAULT_TWI_CLK_HZ   10  /* max 400 Kbits/s */
  #define AT91_I2C_TIMEOUT msecs_to_jiffies(100)   /* transfer timeout */
  #define AT91_I2C_DMA_THRESHOLD   8   /* enable DMA 
 if transfer size is bigger than this threshold */
  
 @@ -711,6 +711,7 @@ static int at91_twi_probe(struct platform_device *pdev)
   struct resource *mem;
   int rc;
   u32 phy_addr;
 + int bus_clk_rate;
  
   dev = devm_kzalloc(pdev-dev, sizeof(*dev), GFP_KERNEL);
   if (!dev)
 @@ -756,7 +757,12 @@ static int at91_twi_probe(struct platform_device *pdev)
   dev-use_dma = true;
   }
  
 - at91_calc_twi_clock(dev, TWI_CLK_HZ);
 + rc = of_property_read_u32(dev-dev-of_node, clock-frequency,
 + bus_clk_rate);
 + if (rc)
 + bus_clk_rate = DEFAULT_TWI_CLK_HZ;
 +
 + at91_calc_twi_clock(dev, bus_clk_rate);
   at91_init_twi_bus(dev);
  
   snprintf(dev-adapter.name, sizeof(dev-adapter.name), AT91);
 -- 
 1.7.10.4
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-i2c in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c:at91: Add device tree property to set clock-frequency

[Ludovic Desroches]

Thanks for removing this hard coding bus frequency.

On Sun, Feb 23, 2014 at 03:19:00AM -0500, Marek Roszko wrote:
 This adds the ability to set clock-frequency in the device tree for the i2c 
 bus following
 the naming of other i2c bus implementations. If the property is not set, the 
 clock
 frequency will default to the previously used define of 100KHz.
 
 Signed-off-by: Marek Roszko mark.ros...@gmail.com
Acked-by: Ludovic Desroches ludovic.desroc...@atmel.com

 ---
  Documentation/devicetree/bindings/i2c/i2c-at91.txt |2 ++
  drivers/i2c/busses/i2c-at91.c  |   10 --
  2 files changed, 10 insertions(+), 2 deletions(-)
 
 diff --git a/Documentation/devicetree/bindings/i2c/i2c-at91.txt 
 b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
 index 4fade84..388f0a2 100644
 --- a/Documentation/devicetree/bindings/i2c/i2c-at91.txt
 +++ b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
 @@ -12,6 +12,7 @@ Required properties :
  - clocks: phandles to input clocks.
  
  Optional properties:
 +- clock-frequency: Desired I2C bus frequency in Hz, otherwise defaults to 
 10
  - Child nodes conforming to i2c bus binding
  
  Examples :
 @@ -23,6 +24,7 @@ i2c0: i2c@fff84000 {
   #address-cells = 1;
   #size-cells = 0;
   clocks = twi0_clk;
 + clock-frequency = 40;
  
   24c512@50 {
   compatible = 24c512;
 diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
 index 843d012..8e0871e 100644
 --- a/drivers/i2c/busses/i2c-at91.c
 +++ b/drivers/i2c/busses/i2c-at91.c
 @@ -32,7 +32,7 @@
  #include linux/slab.h
  #include linux/platform_data/dma-atmel.h
  
 -#define TWI_CLK_HZ   10  /* max 400 Kbits/s */
 +#define DEFAULT_TWI_CLK_HZ   10  /* max 400 Kbits/s */
  #define AT91_I2C_TIMEOUT msecs_to_jiffies(100)   /* transfer timeout */
  #define AT91_I2C_DMA_THRESHOLD   8   /* enable DMA 
 if transfer size is bigger than this threshold */
  
 @@ -711,6 +711,7 @@ static int at91_twi_probe(struct platform_device *pdev)
   struct resource *mem;
   int rc;
   u32 phy_addr;
 + int bus_clk_rate;
  
   dev = devm_kzalloc(pdev-dev, sizeof(*dev), GFP_KERNEL);
   if (!dev)
 @@ -756,7 +757,12 @@ static int at91_twi_probe(struct platform_device *pdev)
   dev-use_dma = true;
   }
  
 - at91_calc_twi_clock(dev, TWI_CLK_HZ);
 + ret = of_property_read_u32(dev-dev-of_node, clock-frequency,
 + bus_clk_rate);
 + if (ret  0)
 + bus_clk_rate = DEFAULT_TWI_CLK_HZ;
 +
 + at91_calc_twi_clock(dev, bus_clk_rate);
   at91_init_twi_bus(dev);
  
   snprintf(dev-adapter.name, sizeof(dev-adapter.name), AT91);
 -- 
 1.7.10.4
 
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 04/17] i2c: i2c-at91: deprecate class based instantiation

[Ludovic Desroches]

On Mon, Feb 10, 2014 at 11:03:58AM +0100, Wolfram Sang wrote:
 Warn users that class based instantiation is going away soon in favour
 of more robust probing and faster bootup times.
 
 Signed-off-by: Wolfram Sang w...@the-dreams.de
 Cc: Ludovic Desroches ludovic.desroc...@atmel.com

Acked-by: Ludovic Desroches ludovic.desroc...@atmel.com

Thanks

 ---
 
  drivers/i2c/busses/i2c-at91.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
 index 8edba9d..4babcdb 100644
 --- a/drivers/i2c/busses/i2c-at91.c
 +++ b/drivers/i2c/busses/i2c-at91.c
 @@ -759,7 +759,7 @@ static int at91_twi_probe(struct platform_device *pdev)
   snprintf(dev-adapter.name, sizeof(dev-adapter.name), AT91);
   i2c_set_adapdata(dev-adapter, dev);
   dev-adapter.owner = THIS_MODULE;
 - dev-adapter.class = I2C_CLASS_HWMON;
 + dev-adapter.class = I2C_CLASS_HWMON | I2C_CLASS_DEPRECATED;
   dev-adapter.algo = at91_twi_algorithm;
   dev-adapter.dev.parent = dev-dev;
   dev-adapter.nr = pdev-id;
 -- 
 1.8.5.1
 
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: at91: document clock properties

[Ludovic Desroches]

On Tue, Dec 17, 2013 at 04:54:04PM +0100, Boris BREZILLON wrote:
 Document the clock properties required by the at91 i2c bus driver.
 
 Signed-off-by: Boris BREZILLON b.brezil...@overkiz.com
Acked-by: Ludovic Desroches ludovic.desroc...@atmel.com

 ---
  Documentation/devicetree/bindings/i2c/i2c-at91.txt |2 ++
  1 file changed, 2 insertions(+)
 
 diff --git a/Documentation/devicetree/bindings/i2c/i2c-at91.txt 
 b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
 index b689a0d..4fade84 100644
 --- a/Documentation/devicetree/bindings/i2c/i2c-at91.txt
 +++ b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
 @@ -9,6 +9,7 @@ Required properties :
  - interrupts: interrupt number to the cpu.
  - #address-cells = 1;
  - #size-cells = 0;
 +- clocks: phandles to input clocks.
  
  Optional properties:
  - Child nodes conforming to i2c bus binding
 @@ -21,6 +22,7 @@ i2c0: i2c@fff84000 {
   interrupts = 12 4 6;
   #address-cells = 1;
   #size-cells = 0;
 + clocks = twi0_clk;
  
   24c512@50 {
   compatible = 24c512;
 -- 
 1.7.9.5
 
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html