2015-12-03 09:53:37

by Ludovic Desroches

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

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


2015-12-03 09:53:59

by Ludovic Desroches

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

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 <[email protected]>
---
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",
+ &twd_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), 1000000);
+ 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 = false,
+ .has_hold_field = true,
};

static struct at91_twi_pdata sama5d2_config = {
@@ -861,6 +904,7 @@ static struct at91_twi_pdata sama5d2_config = {
.clk_offset = 4,
.has_unre_flag = true,
.has_alt_cmd = true,
+ .has_hold_field = true,
};

static const struct of_device_id atmel_twi_dt_ids[] = {
@@ -883,6 +927,9 @@ static const struct of_device_id atmel_twi_dt_ids[] = {
.compatible = "atmel,at91sam9x5-i2c",
.data = &at91sam9x5_config,
}, {
+ .compatible = "atmel,sama5d4-i2c",
+ .data = &sama5d4_config,
+ }, {
.compatible = "atmel,sama5d2-i2c",
.data = &sama5d2_config,
}, {
--
2.5.0

2015-12-03 09:54:07

by Ludovic Desroches

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

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 <[email protected]>
---
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",
+ &twd_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), 1000000);
+ 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 = false,
+ .has_hold_field = true,
};

static struct at91_twi_pdata sama5d2_config = {
@@ -861,6 +904,7 @@ static struct at91_twi_pdata sama5d2_config = {
.clk_offset = 4,
.has_unre_flag = true,
.has_alt_cmd = true,
+ .has_hold_field = true,
};

static const struct of_device_id atmel_twi_dt_ids[] = {
@@ -883,6 +927,9 @@ static const struct of_device_id atmel_twi_dt_ids[] = {
.compatible = "atmel,at91sam9x5-i2c",
.data = &at91sam9x5_config,
}, {
+ .compatible = "atmel,sama5d4-i2c",
+ .data = &sama5d4_config,
+ }, {
.compatible = "atmel,sama5d2-i2c",
.data = &sama5d2_config,
}, {
--
2.5.0

2015-12-03 09:54:25

by Ludovic Desroches

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

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

Signed-off-by: Ludovic Desroches <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
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 = <&flx0>;
atmel,fifo-size = <16>;
+ i2c-sda-hold-time-ns = <336>;

wm8731: wm8731@1a {
compatible = "wm8731";
--
2.5.0

2015-12-03 09:54:29

by Ludovic Desroches

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

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 <[email protected]>
---
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 = <&dma1
@@ -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 = <&dma1
@@ -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 = <&dma1
--
2.5.0

2015-12-03 09:54:50

by Ludovic Desroches

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

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 <[email protected]>
---
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 = <&pinctrl_i2c0_default>;
+ i2c-sda-hold-time-ns = <350>;
status = "okay";

pmic: act8865@5b {
--
2.5.0

2015-12-03 10:08:28

by Ludovic Desroches

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

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 <[email protected]>
> ---
> 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",
> + &twd_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), 1000000);
> + 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 = false,
> + .has_hold_field = true,
> };
>
> static struct at91_twi_pdata sama5d2_config = {
> @@ -861,6 +904,7 @@ static struct at91_twi_pdata sama5d2_config = {
> .clk_offset = 4,
> .has_unre_flag = true,
> .has_alt_cmd = true,
> + .has_hold_field = true,
> };
>
> static const struct of_device_id atmel_twi_dt_ids[] = {
> @@ -883,6 +927,9 @@ static const struct of_device_id atmel_twi_dt_ids[] = {
> .compatible = "atmel,at91sam9x5-i2c",
> .data = &at91sam9x5_config,
> }, {
> + .compatible = "atmel,sama5d4-i2c",
> + .data = &sama5d4_config,
> + }, {
> .compatible = "atmel,sama5d2-i2c",
> .data = &sama5d2_config,
> }, {
> --
> 2.5.0
>

2015-12-03 10:30:23

by Nicolas Ferre

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

Le 03/12/2015 10:53, Ludovic Desroches a ?crit :
> 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 <[email protected]>

Acked-by: Nicolas Ferre <[email protected]>


> ---
> 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",
> + &twd_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), 1000000);
> + 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 = false,
> + .has_hold_field = true,
> };
>
> static struct at91_twi_pdata sama5d2_config = {
> @@ -861,6 +904,7 @@ static struct at91_twi_pdata sama5d2_config = {
> .clk_offset = 4,
> .has_unre_flag = true,
> .has_alt_cmd = true,
> + .has_hold_field = true,
> };
>
> static const struct of_device_id atmel_twi_dt_ids[] = {
> @@ -883,6 +927,9 @@ static const struct of_device_id atmel_twi_dt_ids[] = {
> .compatible = "atmel,at91sam9x5-i2c",
> .data = &at91sam9x5_config,
> }, {
> + .compatible = "atmel,sama5d4-i2c",
> + .data = &sama5d4_config,
> + }, {
> .compatible = "atmel,sama5d2-i2c",
> .data = &sama5d2_config,
> }, {
>


--
Nicolas Ferre

2015-12-03 10:31:45

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] i2c: at91: update bindings documention

Le 03/12/2015 10:53, Ludovic Desroches a ?crit :
> The i2c-sda-hold-time-ns property is supported from atmel,sama5d4-i2c.
>
> Signed-off-by: Ludovic Desroches <[email protected]>
> Acked-by: Rob Herring <[email protected]>

Acked-by: Nicolas Ferre <[email protected]>

> ---
> 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 = <&flx0>;
> atmel,fifo-size = <16>;
> + i2c-sda-hold-time-ns = <336>;
>
> wm8731: wm8731@1a {
> compatible = "wm8731";
>


--
Nicolas Ferre

2015-12-03 10:34:37

by Nicolas Ferre

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

Le 03/12/2015 10:53, Ludovic Desroches a ?crit :
> 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 <[email protected]>

Wolfram, we'll take this one with us in the at91 branches that will go
into arm-soc. It'll be queued in at91-4.5-dt branch soon.

Acked-by: Nicolas Ferre <[email protected]>


> ---
> 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 = <&dma1
> @@ -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 = <&dma1
> @@ -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 = <&dma1
>


--
Nicolas Ferre

2015-12-03 10:33:53

by Nicolas Ferre

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

Le 03/12/2015 10:53, Ludovic Desroches a ?crit :
> 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 <[email protected]>

Same here, we'll take this one with at91 material in at91-4.5-dt branch
soon.

Acked-by: Nicolas Ferre <[email protected]>

Thanks, bye.

> ---
> 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 = <&pinctrl_i2c0_default>;
> + i2c-sda-hold-time-ns = <350>;
> status = "okay";
>
> pmic: act8865@5b {
>


--
Nicolas Ferre

2015-12-03 20:45:41

by Wolfram Sang

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

On Thu, Dec 03, 2015 at 11:33:24AM +0100, Nicolas Ferre wrote:
> Le 03/12/2015 10:53, Ludovic Desroches a écrit :
> > 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 <[email protected]>
>
> Wolfram, we'll take this one with us in the at91 branches that will go
> into arm-soc. It'll be queued in at91-4.5-dt branch soon.
>
> Acked-by: Nicolas Ferre <[email protected]>

I squashed patch 1+2 now and applied them to for-next, thanks!


Attachments:
(No filename) (577.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments