[PATCH v2 0/9] i2c: at91: filters support for at91 SoCs

[PATCH v2 0/9] i2c: at91: filters support for at91 SoCs

[Eugen.Hristev]

From: Eugen Hristev <eugen.hristev@microchip.com>

This series adds the support for i2c filters: analog and digital filter

digital filter is available for sama5d4
an advanced digital filter that allows threshold configuration is available
for sama5d2 and sam9x60
analog filtering is available for sama5d2 and sam9x60

This series also adds platform data for sam9x60 to the driver

In this series version, the enablement of the filters is via Device tree,
as requested by Alexandre Belloni. The PCB/board specific noise is hardware
dependent.

The platform data properties keep the same pattern : set for each SoC
true/false value as it is done for previous properties.

Changes in v2:
- added device tree bindings and support for enable-ana-filt and
enable-dig-filt
- added the new properties to the DT for sama5d4_xplained/sama5d2_xplained

Eugen Hristev (9):
  dt-bindings: i2c: at91: add new compatible
  i2c: at91: add new platform support for sam9x60
  dt-bindings: i2c: at91: add binding for enable-dig-filtr
  i2c: at91: add support for digital filtering
  i2c: at91: add support for advanced digital filtering
  dt-bindings: i2c: at91: add binding for enable-ana-filt
  i2c: at91: add support for analog filtering
  ARM: dts: at91: sama5d2_xplained: add analogic and digital filter for
    i2c
  ARM: dts: at91: sama5d4_xplained: add analogic filter for i2c

 Documentation/devicetree/bindings/i2c/i2c-at91.txt | 11 ++++++-
 arch/arm/boot/dts/at91-sama5d2_xplained.dts        |  4 +++
 arch/arm/boot/dts/at91-sama5d4_xplained.dts        |  1 +
 drivers/i2c/busses/i2c-at91-core.c                 | 38 ++++++++++++++++++++++
 drivers/i2c/busses/i2c-at91-master.c               | 24 ++++++++++++++
 drivers/i2c/busses/i2c-at91.h                      | 12 +++++++
 6 files changed, 89 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH v2 1/9] dt-bindings: i2c: at91: add new compatible

[Eugen.Hristev]

From: Eugen Hristev <eugen.hristev@microchip.com>

Add compatible for new Microchip SoC, sam9x60

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 Documentation/devicetree/bindings/i2c/i2c-at91.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-at91.txt b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
index b7cec17..2210f43 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-at91.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
@@ -3,7 +3,8 @@ 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", "atmel,sama5d4-i2c" or "atmel,sama5d2-i2c"
+     "atmel,at91sam9x5-i2c", "atmel,sama5d4-i2c", "atmel,sama5d2-i2c" or
+     "microchip,sam9x60-i2c"
 - reg: physical base address of the controller and length of memory mapped
      region.
 - interrupts: interrupt number to the cpu.
-- 
2.7.4

[PATCH v2 2/9] i2c: at91: add new platform support for sam9x60

[Eugen.Hristev]

From: Eugen Hristev <eugen.hristev@microchip.com>

Add new platform data support for the sam9x60 SoC

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 drivers/i2c/busses/i2c-at91-core.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/i2c/busses/i2c-at91-core.c b/drivers/i2c/busses/i2c-at91-core.c
index 8d55cdd..a663a7a 100644
--- a/drivers/i2c/busses/i2c-at91-core.c
+++ b/drivers/i2c/busses/i2c-at91-core.c
@@ -148,6 +148,14 @@ static struct at91_twi_pdata sama5d2_config = {
 	.has_hold_field = true,
 };
 
+static struct at91_twi_pdata sam9x60_config = {
+	.clk_max_div = 7,
+	.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[] = {
 	{
 		.compatible = "atmel,at91rm9200-i2c",
@@ -174,6 +182,9 @@ static const struct of_device_id atmel_twi_dt_ids[] = {
 		.compatible = "atmel,sama5d2-i2c",
 		.data = &sama5d2_config,
 	}, {
+		.compatible = "microchip,sam9x60-i2c",
+		.data = &sam9x60_config,
+	}, {
 		/* sentinel */
 	}
 };
-- 
2.7.4

[PATCH v2 3/9] dt-bindings: i2c: at91: add binding for enable-dig-filtr

[Eugen.Hristev]

From: Eugen Hristev <eugen.hristev@microchip.com>

Add binding specificatoin for digital filter inside the i2c controller

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.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 2210f43..8268595 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-at91.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
@@ -20,6 +20,9 @@ Optional properties:
   capable I2C controllers.
 - i2c-sda-hold-time-ns: TWD hold time, only available for "atmel,sama5d4-i2c"
   and "atmel,sama5d2-i2c".
+- enable-dig-filt: Enable the built-in digital filter on the i2c lines,
+  specifically required depending on the hardware PCB/board and if the
+  version of the controller includes it.
 - Child nodes conforming to i2c bus binding
 
 Examples :
@@ -56,6 +59,7 @@ i2c0: i2c@f8034600 {
 	clocks = <&flx0>;
 	atmel,fifo-size = <16>;
 	i2c-sda-hold-time-ns = <336>;
+	enable-dig-filt;
 
 	wm8731: wm8731@1a {
 		compatible = "wm8731";
-- 
2.7.4

Re: [PATCH v2 3/9] dt-bindings: i2c: at91: add binding for enable-dig-filtr

[Peter Rosin]

On 2019-06-25 10:04, Eugen.Hristev@microchip.com wrote:
> From: Eugen Hristev <eugen.hristev@microchip.com>
> 
> Add binding specificatoin for digital filter inside the i2c controller
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.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 2210f43..8268595 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-at91.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
> @@ -20,6 +20,9 @@ Optional properties:
>    capable I2C controllers.
>  - i2c-sda-hold-time-ns: TWD hold time, only available for "atmel,sama5d4-i2c"
>    and "atmel,sama5d2-i2c".
> +- enable-dig-filt: Enable the built-in digital filter on the i2c lines,
> +  specifically required depending on the hardware PCB/board and if the
> +  version of the controller includes it.
>  - Child nodes conforming to i2c bus binding
>  
>  Examples :
> @@ -56,6 +59,7 @@ i2c0: i2c@f8034600 {
>  	clocks = <&flx0>;
>  	atmel,fifo-size = <16>;
>  	i2c-sda-hold-time-ns = <336>;
> +	enable-dig-filt;

This spelling does not match the spelling in the subject. But please see
my comment on patch 6/9...

Cheers,
Peter

>  
>  	wm8731: wm8731@1a {
>  		compatible = "wm8731";
> 

[PATCH v2 4/9] i2c: at91: add support for digital filtering

[Eugen.Hristev]

From: Eugen Hristev <eugen.hristev@microchip.com>

Add new platform data support for digital filtering for i2c.
The sama5d4, sama5d2 and sam9x60 support this feature.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 drivers/i2c/busses/i2c-at91-core.c   | 9 +++++++++
 drivers/i2c/busses/i2c-at91-master.c | 9 +++++++++
 drivers/i2c/busses/i2c-at91.h        | 5 +++++
 3 files changed, 23 insertions(+)

diff --git a/drivers/i2c/busses/i2c-at91-core.c b/drivers/i2c/busses/i2c-at91-core.c
index a663a7a..62610af 100644
--- a/drivers/i2c/busses/i2c-at91-core.c
+++ b/drivers/i2c/busses/i2c-at91-core.c
@@ -68,6 +68,7 @@ static struct at91_twi_pdata at91rm9200_config = {
 	.has_unre_flag = true,
 	.has_alt_cmd = false,
 	.has_hold_field = false,
+	.has_dig_filtr = false,
 };
 
 static struct at91_twi_pdata at91sam9261_config = {
@@ -76,6 +77,7 @@ static struct at91_twi_pdata at91sam9261_config = {
 	.has_unre_flag = false,
 	.has_alt_cmd = false,
 	.has_hold_field = false,
+	.has_dig_filtr = false,
 };
 
 static struct at91_twi_pdata at91sam9260_config = {
@@ -84,6 +86,7 @@ static struct at91_twi_pdata at91sam9260_config = {
 	.has_unre_flag = false,
 	.has_alt_cmd = false,
 	.has_hold_field = false,
+	.has_dig_filtr = false,
 };
 
 static struct at91_twi_pdata at91sam9g20_config = {
@@ -92,6 +95,7 @@ static struct at91_twi_pdata at91sam9g20_config = {
 	.has_unre_flag = false,
 	.has_alt_cmd = false,
 	.has_hold_field = false,
+	.has_dig_filtr = false,
 };
 
 static struct at91_twi_pdata at91sam9g10_config = {
@@ -100,6 +104,7 @@ static struct at91_twi_pdata at91sam9g10_config = {
 	.has_unre_flag = false,
 	.has_alt_cmd = false,
 	.has_hold_field = false,
+	.has_dig_filtr = false,
 };
 
 static const struct platform_device_id at91_twi_devtypes[] = {
@@ -130,6 +135,7 @@ static struct at91_twi_pdata at91sam9x5_config = {
 	.has_unre_flag = false,
 	.has_alt_cmd = false,
 	.has_hold_field = false,
+	.has_dig_filtr = false,
 };
 
 static struct at91_twi_pdata sama5d4_config = {
@@ -138,6 +144,7 @@ static struct at91_twi_pdata sama5d4_config = {
 	.has_unre_flag = false,
 	.has_alt_cmd = false,
 	.has_hold_field = true,
+	.has_dig_filtr = true,
 };
 
 static struct at91_twi_pdata sama5d2_config = {
@@ -146,6 +153,7 @@ static struct at91_twi_pdata sama5d2_config = {
 	.has_unre_flag = true,
 	.has_alt_cmd = true,
 	.has_hold_field = true,
+	.has_dig_filtr = true,
 };
 
 static struct at91_twi_pdata sam9x60_config = {
@@ -154,6 +162,7 @@ static struct at91_twi_pdata sam9x60_config = {
 	.has_unre_flag = true,
 	.has_alt_cmd = true,
 	.has_hold_field = true,
+	.has_dig_filtr = true,
 };
 
 static const struct of_device_id atmel_twi_dt_ids[] = {
diff --git a/drivers/i2c/busses/i2c-at91-master.c b/drivers/i2c/busses/i2c-at91-master.c
index e87232f..7afce62 100644
--- a/drivers/i2c/busses/i2c-at91-master.c
+++ b/drivers/i2c/busses/i2c-at91-master.c
@@ -31,12 +31,18 @@
 
 void at91_init_twi_bus_master(struct at91_twi_dev *dev)
 {
+	struct at91_twi_pdata *pdata = dev->pdata;
+
 	/* 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);
+
+	/* enable digital filter */
+	if (pdata->has_dig_filtr && dev->enable_dig_filt)
+		at91_twi_write(dev, AT91_TWI_FILTR, AT91_TWI_FILTR_FILT);
 }
 
 /*
@@ -792,6 +798,9 @@ int at91_twi_probe_master(struct platform_device *pdev,
 		dev_info(dev->dev, "Using FIFO (%u data)\n", dev->fifo_size);
 	}
 
+	dev->enable_dig_filt = of_property_read_bool(pdev->dev.of_node,
+						     "enable-dig-filt");
+
 	at91_calc_twi_clock(dev);
 
 	dev->adapter.algo = &at91_twi_algorithm;
diff --git a/drivers/i2c/busses/i2c-at91.h b/drivers/i2c/busses/i2c-at91.h
index 499b506..c75447e 100644
--- a/drivers/i2c/busses/i2c-at91.h
+++ b/drivers/i2c/busses/i2c-at91.h
@@ -84,6 +84,9 @@
 #define	AT91_TWI_ACR_DATAL(len)	((len) & 0xff)
 #define	AT91_TWI_ACR_DIR	BIT(8)
 
+#define AT91_TWI_FILTR		0x0044
+#define AT91_TWI_FILTR_FILT	BIT(0)
+
 #define	AT91_TWI_FMR		0x0050	/* FIFO Mode Register */
 #define	AT91_TWI_FMR_TXRDYM(mode)	(((mode) & 0x3) << 0)
 #define	AT91_TWI_FMR_TXRDYM_MASK	(0x3 << 0)
@@ -108,6 +111,7 @@ struct at91_twi_pdata {
 	bool has_unre_flag;
 	bool has_alt_cmd;
 	bool has_hold_field;
+	bool has_dig_filtr;
 	struct at_dma_slave dma_slave;
 };
 
@@ -145,6 +149,7 @@ struct at91_twi_dev {
 	unsigned smr;
 	struct i2c_client *slave;
 #endif
+	bool enable_dig_filt;
 };
 
 unsigned at91_twi_read(struct at91_twi_dev *dev, unsigned reg);
-- 
2.7.4

[PATCH v2 5/9] i2c: at91: add support for advanced digital filtering

[Eugen.Hristev]

From: Eugen Hristev <eugen.hristev@microchip.com>

Add new platform data support for advanced digital filtering for i2c.
The sama5d2 and sam9x60 support this feature.
This digital filter allows the user to configure the maximum
width of the spikes that can be filtered.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 drivers/i2c/busses/i2c-at91-core.c   | 9 +++++++++
 drivers/i2c/busses/i2c-at91-master.c | 6 ++++++
 drivers/i2c/busses/i2c-at91.h        | 3 +++
 3 files changed, 18 insertions(+)

diff --git a/drivers/i2c/busses/i2c-at91-core.c b/drivers/i2c/busses/i2c-at91-core.c
index 62610af..3bbe37c 100644
--- a/drivers/i2c/busses/i2c-at91-core.c
+++ b/drivers/i2c/busses/i2c-at91-core.c
@@ -69,6 +69,7 @@ static struct at91_twi_pdata at91rm9200_config = {
 	.has_alt_cmd = false,
 	.has_hold_field = false,
 	.has_dig_filtr = false,
+	.has_adv_dig_filtr = false,
 };
 
 static struct at91_twi_pdata at91sam9261_config = {
@@ -78,6 +79,7 @@ static struct at91_twi_pdata at91sam9261_config = {
 	.has_alt_cmd = false,
 	.has_hold_field = false,
 	.has_dig_filtr = false,
+	.has_adv_dig_filtr = false,
 };
 
 static struct at91_twi_pdata at91sam9260_config = {
@@ -87,6 +89,7 @@ static struct at91_twi_pdata at91sam9260_config = {
 	.has_alt_cmd = false,
 	.has_hold_field = false,
 	.has_dig_filtr = false,
+	.has_adv_dig_filtr = false,
 };
 
 static struct at91_twi_pdata at91sam9g20_config = {
@@ -96,6 +99,7 @@ static struct at91_twi_pdata at91sam9g20_config = {
 	.has_alt_cmd = false,
 	.has_hold_field = false,
 	.has_dig_filtr = false,
+	.has_adv_dig_filtr = false,
 };
 
 static struct at91_twi_pdata at91sam9g10_config = {
@@ -105,6 +109,7 @@ static struct at91_twi_pdata at91sam9g10_config = {
 	.has_alt_cmd = false,
 	.has_hold_field = false,
 	.has_dig_filtr = false,
+	.has_adv_dig_filtr = false,
 };
 
 static const struct platform_device_id at91_twi_devtypes[] = {
@@ -136,6 +141,7 @@ static struct at91_twi_pdata at91sam9x5_config = {
 	.has_alt_cmd = false,
 	.has_hold_field = false,
 	.has_dig_filtr = false,
+	.has_adv_dig_filtr = false,
 };
 
 static struct at91_twi_pdata sama5d4_config = {
@@ -145,6 +151,7 @@ static struct at91_twi_pdata sama5d4_config = {
 	.has_alt_cmd = false,
 	.has_hold_field = true,
 	.has_dig_filtr = true,
+	.has_adv_dig_filtr = false,
 };
 
 static struct at91_twi_pdata sama5d2_config = {
@@ -154,6 +161,7 @@ static struct at91_twi_pdata sama5d2_config = {
 	.has_alt_cmd = true,
 	.has_hold_field = true,
 	.has_dig_filtr = true,
+	.has_adv_dig_filtr = true,
 };
 
 static struct at91_twi_pdata sam9x60_config = {
@@ -163,6 +171,7 @@ static struct at91_twi_pdata sam9x60_config = {
 	.has_alt_cmd = true,
 	.has_hold_field = true,
 	.has_dig_filtr = true,
+	.has_adv_dig_filtr = true,
 };
 
 static const struct of_device_id atmel_twi_dt_ids[] = {
diff --git a/drivers/i2c/busses/i2c-at91-master.c b/drivers/i2c/busses/i2c-at91-master.c
index 7afce62..d82601b 100644
--- a/drivers/i2c/busses/i2c-at91-master.c
+++ b/drivers/i2c/busses/i2c-at91-master.c
@@ -43,6 +43,12 @@ void at91_init_twi_bus_master(struct at91_twi_dev *dev)
 	/* enable digital filter */
 	if (pdata->has_dig_filtr && dev->enable_dig_filt)
 		at91_twi_write(dev, AT91_TWI_FILTR, AT91_TWI_FILTR_FILT);
+
+	/* enable advanced digital filter */
+	if (pdata->has_adv_dig_filtr && dev->enable_dig_filt)
+		at91_twi_write(dev, AT91_TWI_FILTR, AT91_TWI_FILTR_FILT |
+			       (AT91_TWI_FILTR_THRES(7) &
+				AT91_TWI_FILTR_THRES_MASK));
 }
 
 /*
diff --git a/drivers/i2c/busses/i2c-at91.h b/drivers/i2c/busses/i2c-at91.h
index c75447e..7b8c371 100644
--- a/drivers/i2c/busses/i2c-at91.h
+++ b/drivers/i2c/busses/i2c-at91.h
@@ -86,6 +86,8 @@
 
 #define AT91_TWI_FILTR		0x0044
 #define AT91_TWI_FILTR_FILT	BIT(0)
+#define AT91_TWI_FILTR_THRES(v)		((v) << 8)
+#define AT91_TWI_FILTR_THRES_MASK	GENMASK(10, 8)
 
 #define	AT91_TWI_FMR		0x0050	/* FIFO Mode Register */
 #define	AT91_TWI_FMR_TXRDYM(mode)	(((mode) & 0x3) << 0)
@@ -112,6 +114,7 @@ struct at91_twi_pdata {
 	bool has_alt_cmd;
 	bool has_hold_field;
 	bool has_dig_filtr;
+	bool has_adv_dig_filtr;
 	struct at_dma_slave dma_slave;
 };
 
-- 
2.7.4

[PATCH v2 6/9] dt-bindings: i2c: at91: add binding for enable-ana-filt

[Eugen.Hristev]

From: Eugen Hristev <eugen.hristev@microchip.com>

Add binding specification for analogic filter inside the i2c controller

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.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 8268595..20d334c 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-at91.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
@@ -23,6 +23,9 @@ Optional properties:
 - enable-dig-filt: Enable the built-in digital filter on the i2c lines,
   specifically required depending on the hardware PCB/board and if the
   version of the controller includes it.
+- enable-ana-filt: Enable the built-in analogic filter on the i2c lines,
+  specifically required depending on the hardware PCB/board and if the
+  version of the controller includes it.
 - Child nodes conforming to i2c bus binding
 
 Examples :
@@ -60,6 +63,7 @@ i2c0: i2c@f8034600 {
 	atmel,fifo-size = <16>;
 	i2c-sda-hold-time-ns = <336>;
 	enable-dig-filt;
+	enable-ana-filt;
 
 	wm8731: wm8731@1a {
 		compatible = "wm8731";
-- 
2.7.4

Re: [PATCH v2 6/9] dt-bindings: i2c: at91: add binding for enable-ana-filt

[Peter Rosin]

On 2019-06-25 10:05, Eugen.Hristev@microchip.com wrote:
> From: Eugen Hristev <eugen.hristev@microchip.com>
> 
> Add binding specification for analogic filter inside the i2c controller

s/analogic/the analog/

> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.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 8268595..20d334c 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-at91.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
> @@ -23,6 +23,9 @@ Optional properties:
>  - enable-dig-filt: Enable the built-in digital filter on the i2c lines,
>    specifically required depending on the hardware PCB/board and if the
>    version of the controller includes it.
> +- enable-ana-filt: Enable the built-in analogic filter on the i2c lines,
> +  specifically required depending on the hardware PCB/board and if the
> +  version of the controller includes it.
>  - Child nodes conforming to i2c bus binding
>  
>  Examples :
> @@ -60,6 +63,7 @@ i2c0: i2c@f8034600 {
>  	atmel,fifo-size = <16>;
>  	i2c-sda-hold-time-ns = <336>;
>  	enable-dig-filt;
> +	enable-ana-filt;

Perhaps

	microchip,digital-filter;
	microchip,analog-filter;

?

Cheers,
Peter

>  
>  	wm8731: wm8731@1a {
>  		compatible = "wm8731";
> 

Re: [PATCH v2 6/9] dt-bindings: i2c: at91: add binding for enable-ana-filt

[Eugen.Hristev]

On 25.06.2019 11:55, Peter Rosin wrote:

> 
> On 2019-06-25 10:05, Eugen.Hristev@microchip.com wrote:
>> From: Eugen Hristev <eugen.hristev@microchip.com>
>>
>> Add binding specification for analogic filter inside the i2c controller
> 
> s/analogic/the analog/
> 
>> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.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 8268595..20d334c 100644
>> --- a/Documentation/devicetree/bindings/i2c/i2c-at91.txt
>> +++ b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
>> @@ -23,6 +23,9 @@ Optional properties:
>>   - enable-dig-filt: Enable the built-in digital filter on the i2c lines,
>>     specifically required depending on the hardware PCB/board and if the
>>     version of the controller includes it.
>> +- enable-ana-filt: Enable the built-in analogic filter on the i2c lines,
>> +  specifically required depending on the hardware PCB/board and if the
>> +  version of the controller includes it.
>>   - Child nodes conforming to i2c bus binding
>>   
>>   Examples :
>> @@ -60,6 +63,7 @@ i2c0: i2c@f8034600 {
>>   	atmel,fifo-size = <16>;
>>   	i2c-sda-hold-time-ns = <336>;
>>   	enable-dig-filt;
>> +	enable-ana-filt;
> 
> Perhaps
> 
> 	microchip,digital-filter;
> 	microchip,analog-filter;
> 
> ?

Hi Peter,

Thanks for reviewing. The name of the property does not matter much to 
me, and we have properties prefixed with vendor, and some are not.

@Alexandre Belloni: which name you think it's best ?

Eugen

> 
> Cheers,
> Peter
> 
>>   
>>   	wm8731: wm8731@1a {
>>   		compatible = "wm8731";
>>
> 

Re: [PATCH v2 6/9] dt-bindings: i2c: at91: add binding for enable-ana-filt

[Alexandre Belloni]

On 25/06/2019 09:14:13+0000, Eugen.Hristev@microchip.com wrote:
> > Perhaps
> > 
> > 	microchip,digital-filter;
> > 	microchip,analog-filter;
> > 
> > ?
> 
> Hi Peter,
> 
> Thanks for reviewing. The name of the property does not matter much to 
> me, and we have properties prefixed with vendor, and some are not.
> 
> @Alexandre Belloni: which name you think it's best ?
> 

I'm not sure, it depends on whether Wolfram thinks it is generic enough
to be used without a vendor prefix.


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

I2C filtering (was Re: [PATCH v2 6/9] dt-bindings: i2c: at91: add binding for enable-ana-filt)

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1098 bytes --]

On Tue, Jun 25, 2019 at 11:31:56AM +0200, Alexandre Belloni wrote:
> On 25/06/2019 09:14:13+0000, Eugen.Hristev@microchip.com wrote:
> > > Perhaps
> > > 
> > > 	microchip,digital-filter;
> > > 	microchip,analog-filter;
> > > 
> > > ?
> > 
> > Hi Peter,
> > 
> > Thanks for reviewing. The name of the property does not matter much to 
> > me, and we have properties prefixed with vendor, and some are not.
> > 
> > @Alexandre Belloni: which name you think it's best ?
> > 
> 
> I'm not sure, it depends on whether Wolfram thinks it is generic enough
> to be used without a vendor prefix.

I could imagine that we design a generic property for filters. The ones
above make me wonder, though, because they are bool. I'd think you can
configure the filters in some way, too?

I never used such filtering, so I am unaware of the parameters needed /
suitable. Quick grepping through I2C master drivers reveals that
i2c-stm32f7.c also handles filters, but only with default values. Maybe
DT configuration would be benefitial to that driver, too?

Adding some people to CC.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: I2C filtering (was Re: [PATCH v2 6/9] dt-bindings: i2c: at91: add binding for enable-ana-filt)

[Alexandre Belloni]

On 25/06/2019 11:55:33+0200, Wolfram Sang wrote:
> On Tue, Jun 25, 2019 at 11:31:56AM +0200, Alexandre Belloni wrote:
> > On 25/06/2019 09:14:13+0000, Eugen.Hristev@microchip.com wrote:
> > > > Perhaps
> > > > 
> > > > 	microchip,digital-filter;
> > > > 	microchip,analog-filter;
> > > > 
> > > > ?
> > > 
> > > Hi Peter,
> > > 
> > > Thanks for reviewing. The name of the property does not matter much to 
> > > me, and we have properties prefixed with vendor, and some are not.
> > > 
> > > @Alexandre Belloni: which name you think it's best ?
> > > 
> > 
> > I'm not sure, it depends on whether Wolfram thinks it is generic enough
> > to be used without a vendor prefix.
> 
> I could imagine that we design a generic property for filters. The ones
> above make me wonder, though, because they are bool. I'd think you can
> configure the filters in some way, too?
> 

Apart from enabling the filter there is indeed one configuration
setting, the maximum pulse width of spikes to be suppressed by the input
filter.

> I never used such filtering, so I am unaware of the parameters needed /
> suitable. Quick grepping through I2C master drivers reveals that
> i2c-stm32f7.c also handles filters, but only with default values. Maybe
> DT configuration would be benefitial to that driver, too?
> 
> Adding some people to CC.
> 



-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: I2C filtering (was Re: [PATCH v2 6/9] dt-bindings: i2c: at91: add binding for enable-ana-filt)

[Eugen.Hristev]

On 27.06.2019 16:22, Alexandre Belloni wrote:
> External E-Mail
> 
> 
> On 25/06/2019 11:55:33+0200, Wolfram Sang wrote:
>> On Tue, Jun 25, 2019 at 11:31:56AM +0200, Alexandre Belloni wrote:
>>> On 25/06/2019 09:14:13+0000, Eugen.Hristev@microchip.com wrote:
>>>>> Perhaps
>>>>>
>>>>> 	microchip,digital-filter;
>>>>> 	microchip,analog-filter;
>>>>>
>>>>> ?
>>>>
>>>> Hi Peter,
>>>>
>>>> Thanks for reviewing. The name of the property does not matter much to
>>>> me, and we have properties prefixed with vendor, and some are not.
>>>>
>>>> @Alexandre Belloni: which name you think it's best ?
>>>>
>>>
>>> I'm not sure, it depends on whether Wolfram thinks it is generic enough
>>> to be used without a vendor prefix.
>>
>> I could imagine that we design a generic property for filters. The ones
>> above make me wonder, though, because they are bool. I'd think you can
>> configure the filters in some way, too?
>>
> 
> Apart from enabling the filter there is indeed one configuration
> setting, the maximum pulse width of spikes to be suppressed by the input
> filter.

Hello,

This is a number 0 to 7 (3 bits) that represents the width of the spike 
in periph clock cycles.
I am looking to see what is PADFCFG , as it's related to the PAD analog 
filter configuration. It may be unused by the filter.

Eugen

> 
>> I never used such filtering, so I am unaware of the parameters needed /
>> suitable. Quick grepping through I2C master drivers reveals that
>> i2c-stm32f7.c also handles filters, but only with default values. Maybe
>> DT configuration would be benefitial to that driver, too?
>>
>> Adding some people to CC.
>>
> 
> 
> 

Re: I2C filtering (was Re: [PATCH v2 6/9] dt-bindings: i2c: at91: add binding for enable-ana-filt)

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 586 bytes --]


> > Apart from enabling the filter there is indeed one configuration
> > setting, the maximum pulse width of spikes to be suppressed by the input
> > filter.

Yup, this is what I anticipated.

> This is a number 0 to 7 (3 bits) that represents the width of the spike 
> in periph clock cycles.

For a generic binding, we would need some time-value as a parameter and
convert it to clock cycles in the driver then, I'd think.

> I am looking to see what is PADFCFG , as it's related to the PAD analog 
> filter configuration. It may be unused by the filter.

Thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: I2C filtering (was Re: [PATCH v2 6/9] dt-bindings: i2c: at91: add binding for enable-ana-filt)

[Alexandre Belloni]

On 27/06/2019 15:34:40+0200, Wolfram Sang wrote:
> 
> > > Apart from enabling the filter there is indeed one configuration
> > > setting, the maximum pulse width of spikes to be suppressed by the input
> > > filter.
> 
> Yup, this is what I anticipated.
> 
> > This is a number 0 to 7 (3 bits) that represents the width of the spike 
> > in periph clock cycles.
> 
> For a generic binding, we would need some time-value as a parameter and
> convert it to clock cycles in the driver then, I'd think.
> 

Yes, that is what I was going to suggest.


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH v2 7/9] i2c: at91: add support for analog filtering

[Eugen.Hristev]

From: Eugen Hristev <eugen.hristev@microchip.com>

Add support for analog filtering for i2c lines.
The sama5d2 and sam9x60 support this feature.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 drivers/i2c/busses/i2c-at91-core.c   |  9 +++++++++
 drivers/i2c/busses/i2c-at91-master.c | 17 +++++++++++++----
 drivers/i2c/busses/i2c-at91.h        |  4 ++++
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-at91-core.c b/drivers/i2c/busses/i2c-at91-core.c
index 3bbe37c..d2840ba 100644
--- a/drivers/i2c/busses/i2c-at91-core.c
+++ b/drivers/i2c/busses/i2c-at91-core.c
@@ -70,6 +70,7 @@ static struct at91_twi_pdata at91rm9200_config = {
 	.has_hold_field = false,
 	.has_dig_filtr = false,
 	.has_adv_dig_filtr = false,
+	.has_ana_filtr = false,
 };
 
 static struct at91_twi_pdata at91sam9261_config = {
@@ -80,6 +81,7 @@ static struct at91_twi_pdata at91sam9261_config = {
 	.has_hold_field = false,
 	.has_dig_filtr = false,
 	.has_adv_dig_filtr = false,
+	.has_ana_filtr = false,
 };
 
 static struct at91_twi_pdata at91sam9260_config = {
@@ -90,6 +92,7 @@ static struct at91_twi_pdata at91sam9260_config = {
 	.has_hold_field = false,
 	.has_dig_filtr = false,
 	.has_adv_dig_filtr = false,
+	.has_ana_filtr = false,
 };
 
 static struct at91_twi_pdata at91sam9g20_config = {
@@ -100,6 +103,7 @@ static struct at91_twi_pdata at91sam9g20_config = {
 	.has_hold_field = false,
 	.has_dig_filtr = false,
 	.has_adv_dig_filtr = false,
+	.has_ana_filtr = false,
 };
 
 static struct at91_twi_pdata at91sam9g10_config = {
@@ -110,6 +114,7 @@ static struct at91_twi_pdata at91sam9g10_config = {
 	.has_hold_field = false,
 	.has_dig_filtr = false,
 	.has_adv_dig_filtr = false,
+	.has_ana_filtr = false,
 };
 
 static const struct platform_device_id at91_twi_devtypes[] = {
@@ -142,6 +147,7 @@ static struct at91_twi_pdata at91sam9x5_config = {
 	.has_hold_field = false,
 	.has_dig_filtr = false,
 	.has_adv_dig_filtr = false,
+	.has_ana_filtr = false,
 };
 
 static struct at91_twi_pdata sama5d4_config = {
@@ -152,6 +158,7 @@ static struct at91_twi_pdata sama5d4_config = {
 	.has_hold_field = true,
 	.has_dig_filtr = true,
 	.has_adv_dig_filtr = false,
+	.has_ana_filtr = false,
 };
 
 static struct at91_twi_pdata sama5d2_config = {
@@ -162,6 +169,7 @@ static struct at91_twi_pdata sama5d2_config = {
 	.has_hold_field = true,
 	.has_dig_filtr = true,
 	.has_adv_dig_filtr = true,
+	.has_ana_filtr = true,
 };
 
 static struct at91_twi_pdata sam9x60_config = {
@@ -172,6 +180,7 @@ static struct at91_twi_pdata sam9x60_config = {
 	.has_hold_field = true,
 	.has_dig_filtr = true,
 	.has_adv_dig_filtr = true,
+	.has_ana_filtr = true,
 };
 
 static const struct of_device_id atmel_twi_dt_ids[] = {
diff --git a/drivers/i2c/busses/i2c-at91-master.c b/drivers/i2c/busses/i2c-at91-master.c
index d82601b..8fe3a2c 100644
--- a/drivers/i2c/busses/i2c-at91-master.c
+++ b/drivers/i2c/busses/i2c-at91-master.c
@@ -32,6 +32,7 @@
 void at91_init_twi_bus_master(struct at91_twi_dev *dev)
 {
 	struct at91_twi_pdata *pdata = dev->pdata;
+	u32 filtr = 0;
 
 	/* FIFO should be enabled immediately after the software reset */
 	if (dev->fifo_size)
@@ -42,13 +43,19 @@ void at91_init_twi_bus_master(struct at91_twi_dev *dev)
 
 	/* enable digital filter */
 	if (pdata->has_dig_filtr && dev->enable_dig_filt)
-		at91_twi_write(dev, AT91_TWI_FILTR, AT91_TWI_FILTR_FILT);
+		filtr |= AT91_TWI_FILTR_FILT;
 
 	/* enable advanced digital filter */
 	if (pdata->has_adv_dig_filtr && dev->enable_dig_filt)
-		at91_twi_write(dev, AT91_TWI_FILTR, AT91_TWI_FILTR_FILT |
-			       (AT91_TWI_FILTR_THRES(7) &
-				AT91_TWI_FILTR_THRES_MASK));
+		filtr |= AT91_TWI_FILTR_FILT |
+			 (AT91_TWI_FILTR_THRES(7) & AT91_TWI_FILTR_THRES_MASK);
+
+	/* enable analog filter */
+	if (pdata->has_ana_filtr && dev->enable_ana_filt)
+		filtr |= AT91_TWI_FILTR_PADFEN | AT91_TWI_FILTR_PADFCFG;
+
+	if (filtr)
+		at91_twi_write(dev, AT91_TWI_FILTR, filtr);
 }
 
 /*
@@ -807,6 +814,8 @@ int at91_twi_probe_master(struct platform_device *pdev,
 	dev->enable_dig_filt = of_property_read_bool(pdev->dev.of_node,
 						     "enable-dig-filt");
 
+	dev->enable_ana_filt = of_property_read_bool(pdev->dev.of_node,
+						     "enable-ana-filt");
 	at91_calc_twi_clock(dev);
 
 	dev->adapter.algo = &at91_twi_algorithm;
diff --git a/drivers/i2c/busses/i2c-at91.h b/drivers/i2c/busses/i2c-at91.h
index 7b8c371..4ca9c191 100644
--- a/drivers/i2c/busses/i2c-at91.h
+++ b/drivers/i2c/busses/i2c-at91.h
@@ -86,6 +86,8 @@
 
 #define AT91_TWI_FILTR		0x0044
 #define AT91_TWI_FILTR_FILT	BIT(0)
+#define AT91_TWI_FILTR_PADFEN	BIT(1)
+#define AT91_TWI_FILTR_PADFCFG	BIT(2)
 #define AT91_TWI_FILTR_THRES(v)		((v) << 8)
 #define AT91_TWI_FILTR_THRES_MASK	GENMASK(10, 8)
 
@@ -115,6 +117,7 @@ struct at91_twi_pdata {
 	bool has_hold_field;
 	bool has_dig_filtr;
 	bool has_adv_dig_filtr;
+	bool has_ana_filtr;
 	struct at_dma_slave dma_slave;
 };
 
@@ -153,6 +156,7 @@ struct at91_twi_dev {
 	struct i2c_client *slave;
 #endif
 	bool enable_dig_filt;
+	bool enable_ana_filt;
 };
 
 unsigned at91_twi_read(struct at91_twi_dev *dev, unsigned reg);
-- 
2.7.4

[PATCH v2 8/9] ARM: dts: at91: sama5d2_xplained: add analogic and digital filter for i2c

[Eugen.Hristev]

From: Eugen Hristev <eugen.hristev@microchip.com>

Add property for analogic and digital filter for i2c1 and i2c2 nodes
for sama5d2_xplained

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 arch/arm/boot/dts/at91-sama5d2_xplained.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/at91-sama5d2_xplained.dts b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
index 808e399..bde4a711b 100644
--- a/arch/arm/boot/dts/at91-sama5d2_xplained.dts
+++ b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
@@ -334,6 +334,8 @@
 					pinctrl-names = "default";
 					pinctrl-0 = <&pinctrl_flx4_default>;
 					atmel,fifo-size = <16>;
+					enable-ana-filt;
+					enable-dig-filt;
 					status = "okay";
 				};
 			};
@@ -342,6 +344,8 @@
 				dmas = <0>, <0>;
 				pinctrl-names = "default";
 				pinctrl-0 = <&pinctrl_i2c1_default>;
+				enable-ana-filt;
+				enable-dig-filt;
 				status = "okay";
 
 				at24@54 {
-- 
2.7.4

[PATCH v2 9/9] ARM: dts: at91: sama5d4_xplained: add analogic filter for i2c

[Eugen.Hristev]

From: Eugen Hristev <eugen.hristev@microchip.com>

Add property for digital filter for i2c0 node sama5d4_xplained

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 arch/arm/boot/dts/at91-sama5d4_xplained.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/at91-sama5d4_xplained.dts b/arch/arm/boot/dts/at91-sama5d4_xplained.dts
index fdfc37d..06068dc 100644
--- a/arch/arm/boot/dts/at91-sama5d4_xplained.dts
+++ b/arch/arm/boot/dts/at91-sama5d4_xplained.dts
@@ -49,6 +49,7 @@
 			};
 
 			i2c0: i2c@f8014000 {
+				enable-ana-filt;
 				status = "okay";
 			};
 
-- 
2.7.4

Re: [PATCH v2 9/9] ARM: dts: at91: sama5d4_xplained: add analogic filter for i2c

[Peter Rosin]

On 2019-06-25 10:05, Eugen.Hristev@microchip.com wrote:
> From: Eugen Hristev <eugen.hristev@microchip.com>
> 
> Add property for digital filter for i2c0 node sama5d4_xplained

This does not match the below hunk.

Cheers,
Peter

> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> ---
>  arch/arm/boot/dts/at91-sama5d4_xplained.dts | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/at91-sama5d4_xplained.dts b/arch/arm/boot/dts/at91-sama5d4_xplained.dts
> index fdfc37d..06068dc 100644
> --- a/arch/arm/boot/dts/at91-sama5d4_xplained.dts
> +++ b/arch/arm/boot/dts/at91-sama5d4_xplained.dts
> @@ -49,6 +49,7 @@
>  			};
>  
>  			i2c0: i2c@f8014000 {
> +				enable-ana-filt;
>  				status = "okay";
>  			};
>  
>