linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] i2c: at91: slave mode support
@ 2018-07-16  9:40 Ludovic Desroches
  2018-07-16  9:40 ` [PATCH v3 1/3] i2c: at91: segregate master mode specific code from probe and init func Ludovic Desroches
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Ludovic Desroches @ 2018-07-16  9:40 UTC (permalink / raw)
  To: linux-arm-kernel

[Ludovic Desroches: see 'Changes in v3' section]

Based on the discussion we had on the i2c-linux list [1], I wrote a patch for
AT91 hardware and tried to fulfill the Linux I2C slave interface description
[2] as good as possible. This enables aforementioned hardware to act as an I2C
slave that can be accessed by a remote I2C master.

I have tested this patchset successfully on an ATSAMA5D27.

                                 ^  3.3V   ^  3.3V
    +-----------------------+    |         |         +-----------------------+
    | Slave: ATSAMA5D27     |   +-+       +-+        | Master: ATSAMA5D35    |
    | with i2c-slave-eeprom |   | | 100k  | | 100k   | with i2cset           |
    +-------------------+-+-+   +-+       +-+        +-+-+-------------------+
                        | |      |         |           | |
                        | +------+---------|---(SDA)---+ |
                        +------------------+---(SCL)-----+

    Schematic: Connection of slave and master with 100kOhm pullup resistors.

On the master the following BASH script has been used to stress the slave.

	root at emblinux:~# cat ./stress.sh
	#!/bin/bash
	I=0
	while true
	do
		if i2cset -y -r 1 0x64 0 $I w | grep mismatch
		then
			echo "$(date): Error in transmission ${I}"
		fi
		((I++))
		if [ $I -eq 65536 ]
		then
			I=0
			echo "$(date): Sent 65536 transmissions"
		fi
	done

After running the script for some time I had the following output. To me this
looks promising :)

	root at emblinux:~# ./stress.sh
	Thu Nov  9 13:58:45 CTE 2017: Sent 65536 transmissions
	Thu Nov  9 14:35:20 CTE 2017: Sent 65536 transmissions
	Thu Nov  9 15:12:11 CTE 2017: Sent 65536 transmissions
	Thu Nov  9 15:49:04 CTE 2017: Sent 65536 transmissions
	Thu Nov  9 16:26:00 CTE 2017: Sent 65536 transmissions
	Thu Nov  9 17:03:07 UTC 2017: Sent 65536 transmissions
	Thu Nov  9 17:40:15 UTC 2017: Sent 65536 transmissions

	If you have some hardware with an at91-i2c interface included at hand, I really
	would appreciate if you can run the test script on your hardware and test this
	driver.

Best regards
  Juergen


[Ludovic Desroches]
Changes in v3:
 - rebase (cherry-pick was enough)
 - fix checkpatch errors
 - tests:
   - hangs with a SAMA5D4 (master and slave on different busses) after about
   100 transfers. It's the firs time I do this test.
   - some mismatches with a SAMA5D4 as slave and a SAMA5D2 as master
   I don't know if it's a regression. I don't remember having seen this
   behavior previously.
   I think it's worth taking those patches even if there are some possible
   bugs. It'll allow to get more people using it and so to consolidate the
   slave mode support.

Changes in v2:
 - Implemented all suggestions made by Ludovic. (Thank you!)
 - Reworked the IRQ handler completely. Have a look in patch 3 fort further
   details.

[1] https://marc.info/?t=150824004800001&r=1&w=1
[2] https://www.kernel.org/doc/Documentation/i2c/slave-interface

Juergen Fitschen (3):
  i2c: at91: segregate master mode specific code from probe and init
    func
  i2c: at91: split driver into core and master file
  i2c: at91: added slave mode support

 MAINTAINERS                                        |   3 +-
 drivers/i2c/busses/Makefile                        |   4 +
 drivers/i2c/busses/i2c-at91-core.c                 | 380 +++++++++++++++++
 .../i2c/busses/{i2c-at91.c => i2c-at91-master.c}   | 453 +--------------------
 drivers/i2c/busses/i2c-at91-slave.c                | 147 +++++++
 drivers/i2c/busses/i2c-at91.h                      | 179 ++++++++
 6 files changed, 719 insertions(+), 447 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-at91-core.c
 rename drivers/i2c/busses/{i2c-at91.c => i2c-at91-master.c} (67%)
 create mode 100644 drivers/i2c/busses/i2c-at91-slave.c
 create mode 100644 drivers/i2c/busses/i2c-at91.h

-- 
2.12.2

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 1/3] i2c: at91: segregate master mode specific code from probe and init func
  2018-07-16  9:40 [PATCH v3 0/3] i2c: at91: slave mode support Ludovic Desroches
@ 2018-07-16  9:40 ` Ludovic Desroches
  2018-07-16  9:40 ` [PATCH v3 2/3] i2c: at91: split driver into core and master file Ludovic Desroches
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Ludovic Desroches @ 2018-07-16  9:40 UTC (permalink / raw)
  To: linux-arm-kernel

From: Juergen Fitschen <me@jue.yt>

In order to implement slave mode support for the at91 hardware we have to
segregate all master mode specific function parts from the general parts.
The upcoming slave mode patch will call its sepcific probe resp. init
function instead of the master mode functions after the shared general
code has been executed.

This concept has been influenced by the i2c-designware driver.

Signed-off-by: Juergen Fitschen <me@jue.yt>
Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
---
 drivers/i2c/busses/i2c-at91.c | 90 ++++++++++++++++++++++++++-----------------
 1 file changed, 55 insertions(+), 35 deletions(-)

diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index 3f3e8b3bf5ff..bc74184462c6 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -174,10 +174,8 @@ static void at91_twi_irq_restore(struct at91_twi_dev *dev)
 	at91_twi_write(dev, AT91_TWI_IER, dev->imr);
 }
 
-static void at91_init_twi_bus(struct at91_twi_dev *dev)
+static void at91_init_twi_bus_master(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);
@@ -186,6 +184,14 @@ static void at91_init_twi_bus(struct at91_twi_dev *dev)
 	at91_twi_write(dev, AT91_TWI_CWGR, dev->twi_cwgr_reg);
 }
 
+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);
+
+	at91_init_twi_bus_master(dev);
+}
+
 /*
  * Calculate symmetric clock as stated in datasheet:
  * twi_clk = F_MAIN / (2 * (cdiv * (1 << ckdiv) + offset))
@@ -1046,18 +1052,56 @@ static struct at91_twi_pdata *at91_twi_get_driver_data(
 	return (struct at91_twi_pdata *) platform_get_device_id(pdev)->driver_data;
 }
 
+static int at91_twi_probe_master(struct platform_device *pdev,
+				 u32 phy_addr, struct at91_twi_dev *dev)
+{
+	int rc;
+	u32 bus_clk_rate;
+
+	init_completion(&dev->cmd_complete);
+
+	rc = devm_request_irq(&pdev->dev, dev->irq, atmel_twi_interrupt, 0,
+			      dev_name(dev->dev), dev);
+	if (rc) {
+		dev_err(dev->dev, "Cannot get irq %d: %d\n", dev->irq, rc);
+		return rc;
+	}
+
+	if (dev->dev->of_node) {
+		rc = at91_twi_configure_dma(dev, phy_addr);
+		if (rc == -EPROBE_DEFER)
+			return rc;
+	}
+
+	if (!of_property_read_u32(pdev->dev.of_node, "atmel,fifo-size",
+				  &dev->fifo_size)) {
+		dev_info(dev->dev, "Using FIFO (%u data)\n", dev->fifo_size);
+	}
+
+	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);
+
+	dev->adapter.algo = &at91_twi_algorithm;
+	dev->adapter.quirks = &at91_twi_quirks;
+
+	return 0;
+}
+
 static int at91_twi_probe(struct platform_device *pdev)
 {
 	struct at91_twi_dev *dev;
 	struct resource *mem;
 	int rc;
 	u32 phy_addr;
-	u32 bus_clk_rate;
 
 	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
 	if (!dev)
 		return -ENOMEM;
-	init_completion(&dev->cmd_complete);
+
 	dev->dev = &pdev->dev;
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1077,13 +1121,6 @@ static int at91_twi_probe(struct platform_device *pdev)
 	if (dev->irq < 0)
 		return dev->irq;
 
-	rc = devm_request_irq(&pdev->dev, dev->irq, atmel_twi_interrupt, 0,
-			 dev_name(dev->dev), dev);
-	if (rc) {
-		dev_err(dev->dev, "Cannot get irq %d: %d\n", dev->irq, rc);
-		return rc;
-	}
-
 	platform_set_drvdata(pdev, dev);
 
 	dev->clk = devm_clk_get(dev->dev, NULL);
@@ -1095,38 +1132,21 @@ static int at91_twi_probe(struct platform_device *pdev)
 	if (rc)
 		return rc;
 
-	if (dev->dev->of_node) {
-		rc = at91_twi_configure_dma(dev, phy_addr);
-		if (rc == -EPROBE_DEFER) {
-			clk_disable_unprepare(dev->clk);
-			return rc;
-		}
-	}
-
-	if (!of_property_read_u32(pdev->dev.of_node, "atmel,fifo-size",
-				  &dev->fifo_size)) {
-		dev_info(dev->dev, "Using FIFO (%u data)\n", dev->fifo_size);
-	}
-
-	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");
 	i2c_set_adapdata(&dev->adapter, dev);
 	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;
 	dev->adapter.dev.of_node = pdev->dev.of_node;
 
+	rc = at91_twi_probe_master(pdev, phy_addr, dev);
+	if (rc)
+		return rc;
+
+	at91_init_twi_bus(dev);
+
 	pm_runtime_set_autosuspend_delay(dev->dev, AUTOSUSPEND_TIMEOUT);
 	pm_runtime_use_autosuspend(dev->dev);
 	pm_runtime_set_active(dev->dev);
-- 
2.12.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 2/3] i2c: at91: split driver into core and master file
  2018-07-16  9:40 [PATCH v3 0/3] i2c: at91: slave mode support Ludovic Desroches
  2018-07-16  9:40 ` [PATCH v3 1/3] i2c: at91: segregate master mode specific code from probe and init func Ludovic Desroches
@ 2018-07-16  9:40 ` Ludovic Desroches
  2018-07-16  9:40 ` [PATCH v3 3/3] i2c: at91: added slave mode support Ludovic Desroches
  2018-07-20 22:41 ` [PATCH v3 0/3] i2c: at91: " Wolfram Sang
  3 siblings, 0 replies; 8+ messages in thread
From: Ludovic Desroches @ 2018-07-16  9:40 UTC (permalink / raw)
  To: linux-arm-kernel

From: Juergen Fitschen <me@jue.yt>

The single file i2c-at91.c has been split into core code (i2c-at91-core.c)
and master mode specific code (i2c-at91-master.c). This should enhance
maintainability and reduce ifdeffery for slave mode related code.

The code itself hasn't been touched. Shared functions only had to be made
non-static. Furthermore, includes have been cleaned up.

Signed-off-by: Juergen Fitschen <me@jue.yt>
[ludovic.desroches at microchip.com: fix checkpatch errors]
Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
---
 MAINTAINERS                                        |   3 +-
 drivers/i2c/busses/Makefile                        |   1 +
 drivers/i2c/busses/i2c-at91-core.c                 | 373 ++++++++++++++++
 .../i2c/busses/{i2c-at91.c => i2c-at91-master.c}   | 467 +--------------------
 drivers/i2c/busses/i2c-at91.h                      | 151 +++++++
 5 files changed, 531 insertions(+), 464 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-at91-core.c
 rename drivers/i2c/busses/{i2c-at91.c => i2c-at91-master.c} (67%)
 create mode 100644 drivers/i2c/busses/i2c-at91.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 96e98e206b0d..14a87595b482 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2426,7 +2426,8 @@ ATMEL I2C DRIVER
 M:	Ludovic Desroches <ludovic.desroches@microchip.com>
 L:	linux-i2c at vger.kernel.org
 S:	Supported
-F:	drivers/i2c/busses/i2c-at91.c
+F:	drivers/i2c/busses/i2c-at91.h
+F:	drivers/i2c/busses/i2c-at91-*.c
 
 ATMEL ISI DRIVER
 M:	Ludovic Desroches <ludovic.desroches@microchip.com>
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 5a869144a0c5..378daa860ed4 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_I2C_POWERMAC)	+= i2c-powermac.o
 obj-$(CONFIG_I2C_ALTERA)	+= i2c-altera.o
 obj-$(CONFIG_I2C_ASPEED)	+= i2c-aspeed.o
 obj-$(CONFIG_I2C_AT91)		+= i2c-at91.o
+i2c-at91-objs			:= i2c-at91-core.o i2c-at91-master.o
 obj-$(CONFIG_I2C_AU1550)	+= i2c-au1550.o
 obj-$(CONFIG_I2C_AXXIA)		+= i2c-axxia.o
 obj-$(CONFIG_I2C_BCM2835)	+= i2c-bcm2835.o
diff --git a/drivers/i2c/busses/i2c-at91-core.c b/drivers/i2c/busses/i2c-at91-core.c
new file mode 100644
index 000000000000..5d9c6c81e6ab
--- /dev/null
+++ b/drivers/i2c/busses/i2c-at91-core.c
@@ -0,0 +1,373 @@
+/*
+ *  i2c Support for Atmel's AT91 Two-Wire Interface (TWI)
+ *
+ *  Copyright (C) 2011 Weinmann Medical GmbH
+ *  Author: Nikolaus Voss <n.voss@weinmann.de>
+ *
+ *  Evolved from original work by:
+ *  Copyright (C) 2004 Rick Bronson
+ *  Converted to 2.6 by Andrew Victor <andrew@sanpeople.com>
+ *
+ *  Borrowed heavily from original work by:
+ *  Copyright (C) 2000 Philip Edelbrock <phil@stimpy.netroedge.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/pinctrl/consumer.h>
+
+#include "i2c-at91.h"
+
+unsigned at91_twi_read(struct at91_twi_dev *dev, unsigned reg)
+{
+	return readl_relaxed(dev->base + reg);
+}
+
+void at91_twi_write(struct at91_twi_dev *dev, unsigned reg, unsigned val)
+{
+	writel_relaxed(val, dev->base + reg);
+}
+
+void at91_disable_twi_interrupts(struct at91_twi_dev *dev)
+{
+	at91_twi_write(dev, AT91_TWI_IDR, AT91_TWI_INT_MASK);
+}
+
+void at91_twi_irq_save(struct at91_twi_dev *dev)
+{
+	dev->imr = at91_twi_read(dev, AT91_TWI_IMR) & AT91_TWI_INT_MASK;
+	at91_disable_twi_interrupts(dev);
+}
+
+void at91_twi_irq_restore(struct at91_twi_dev *dev)
+{
+	at91_twi_write(dev, AT91_TWI_IER, dev->imr);
+}
+
+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);
+
+	at91_init_twi_bus_master(dev);
+}
+
+static struct at91_twi_pdata at91rm9200_config = {
+	.clk_max_div = 5,
+	.clk_offset = 3,
+	.has_unre_flag = true,
+	.has_alt_cmd = false,
+	.has_hold_field = false,
+};
+
+static struct at91_twi_pdata at91sam9261_config = {
+	.clk_max_div = 5,
+	.clk_offset = 4,
+	.has_unre_flag = false,
+	.has_alt_cmd = false,
+	.has_hold_field = false,
+};
+
+static struct at91_twi_pdata at91sam9260_config = {
+	.clk_max_div = 7,
+	.clk_offset = 4,
+	.has_unre_flag = false,
+	.has_alt_cmd = false,
+	.has_hold_field = false,
+};
+
+static struct at91_twi_pdata at91sam9g20_config = {
+	.clk_max_div = 7,
+	.clk_offset = 4,
+	.has_unre_flag = false,
+	.has_alt_cmd = false,
+	.has_hold_field = false,
+};
+
+static struct at91_twi_pdata at91sam9g10_config = {
+	.clk_max_div = 7,
+	.clk_offset = 4,
+	.has_unre_flag = false,
+	.has_alt_cmd = false,
+	.has_hold_field = false,
+};
+
+static const struct platform_device_id at91_twi_devtypes[] = {
+	{
+		.name = "i2c-at91rm9200",
+		.driver_data = (unsigned long) &at91rm9200_config,
+	}, {
+		.name = "i2c-at91sam9261",
+		.driver_data = (unsigned long) &at91sam9261_config,
+	}, {
+		.name = "i2c-at91sam9260",
+		.driver_data = (unsigned long) &at91sam9260_config,
+	}, {
+		.name = "i2c-at91sam9g20",
+		.driver_data = (unsigned long) &at91sam9g20_config,
+	}, {
+		.name = "i2c-at91sam9g10",
+		.driver_data = (unsigned long) &at91sam9g10_config,
+	}, {
+		/* sentinel */
+	}
+};
+
+#if defined(CONFIG_OF)
+static struct at91_twi_pdata at91sam9x5_config = {
+	.clk_max_div = 7,
+	.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 = {
+	.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",
+		.data = &at91rm9200_config,
+	}, {
+		.compatible = "atmel,at91sam9260-i2c",
+		.data = &at91sam9260_config,
+	}, {
+		.compatible = "atmel,at91sam9261-i2c",
+		.data = &at91sam9261_config,
+	}, {
+		.compatible = "atmel,at91sam9g20-i2c",
+		.data = &at91sam9g20_config,
+	}, {
+		.compatible = "atmel,at91sam9g10-i2c",
+		.data = &at91sam9g10_config,
+	}, {
+		.compatible = "atmel,at91sam9x5-i2c",
+		.data = &at91sam9x5_config,
+	}, {
+		.compatible = "atmel,sama5d4-i2c",
+		.data = &sama5d4_config,
+	}, {
+		.compatible = "atmel,sama5d2-i2c",
+		.data = &sama5d2_config,
+	}, {
+		/* sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(of, atmel_twi_dt_ids);
+#endif
+
+static struct at91_twi_pdata *at91_twi_get_driver_data(
+					struct platform_device *pdev)
+{
+	if (pdev->dev.of_node) {
+		const struct of_device_id *match;
+		match = of_match_node(atmel_twi_dt_ids, pdev->dev.of_node);
+		if (!match)
+			return NULL;
+		return (struct at91_twi_pdata *)match->data;
+	}
+	return (struct at91_twi_pdata *) platform_get_device_id(pdev)->driver_data;
+}
+
+static int at91_twi_probe(struct platform_device *pdev)
+{
+	struct at91_twi_dev *dev;
+	struct resource *mem;
+	int rc;
+	u32 phy_addr;
+
+	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
+	if (!dev)
+		return -ENOMEM;
+
+	dev->dev = &pdev->dev;
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!mem)
+		return -ENODEV;
+	phy_addr = mem->start;
+
+	dev->pdata = at91_twi_get_driver_data(pdev);
+	if (!dev->pdata)
+		return -ENODEV;
+
+	dev->base = devm_ioremap_resource(&pdev->dev, mem);
+	if (IS_ERR(dev->base))
+		return PTR_ERR(dev->base);
+
+	dev->irq = platform_get_irq(pdev, 0);
+	if (dev->irq < 0)
+		return dev->irq;
+
+	platform_set_drvdata(pdev, dev);
+
+	dev->clk = devm_clk_get(dev->dev, NULL);
+	if (IS_ERR(dev->clk)) {
+		dev_err(dev->dev, "no clock defined\n");
+		return -ENODEV;
+	}
+	clk_prepare_enable(dev->clk);
+
+	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_DEPRECATED;
+	dev->adapter.dev.parent = dev->dev;
+	dev->adapter.nr = pdev->id;
+	dev->adapter.timeout = AT91_I2C_TIMEOUT;
+	dev->adapter.dev.of_node = pdev->dev.of_node;
+
+	rc = at91_twi_probe_master(pdev, phy_addr, dev);
+	if (rc)
+		return rc;
+
+	at91_init_twi_bus(dev);
+
+	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) {
+		clk_disable_unprepare(dev->clk);
+
+		pm_runtime_disable(dev->dev);
+		pm_runtime_set_suspended(dev->dev);
+
+		return rc;
+	}
+
+	dev_info(dev->dev, "AT91 i2c bus driver (hw version: %#x).\n",
+		 at91_twi_read(dev, AT91_TWI_VER));
+	return 0;
+}
+
+static int at91_twi_remove(struct platform_device *pdev)
+{
+	struct at91_twi_dev *dev = platform_get_drvdata(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
+
+static int at91_twi_runtime_suspend(struct device *dev)
+{
+	struct at91_twi_dev *twi_dev = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(twi_dev->clk);
+
+	pinctrl_pm_select_sleep_state(dev);
+
+	return 0;
+}
+
+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);
+}
+
+static int at91_twi_suspend_noirq(struct device *dev)
+{
+	if (!pm_runtime_status_suspended(dev))
+		at91_twi_runtime_suspend(dev);
+
+	return 0;
+}
+
+static int at91_twi_resume_noirq(struct device *dev)
+{
+	struct at91_twi_dev *twi_dev = dev_get_drvdata(dev);
+	int ret;
+
+	if (!pm_runtime_status_suspended(dev)) {
+		ret = at91_twi_runtime_resume(dev);
+		if (ret)
+			return ret;
+	}
+
+	pm_runtime_mark_last_busy(dev);
+	pm_request_autosuspend(dev);
+
+	at91_init_twi_bus(twi_dev);
+
+	return 0;
+}
+
+static const struct dev_pm_ops at91_twi_pm = {
+	.suspend_noirq	= at91_twi_suspend_noirq,
+	.resume_noirq	= at91_twi_resume_noirq,
+	.runtime_suspend	= at91_twi_runtime_suspend,
+	.runtime_resume		= at91_twi_runtime_resume,
+};
+
+#define at91_twi_pm_ops (&at91_twi_pm)
+#else
+#define at91_twi_pm_ops NULL
+#endif
+
+static struct platform_driver at91_twi_driver = {
+	.probe		= at91_twi_probe,
+	.remove		= at91_twi_remove,
+	.id_table	= at91_twi_devtypes,
+	.driver		= {
+		.name	= "at91_i2c",
+		.of_match_table = of_match_ptr(atmel_twi_dt_ids),
+		.pm	= at91_twi_pm_ops,
+	},
+};
+
+static int __init at91_twi_init(void)
+{
+	return platform_driver_register(&at91_twi_driver);
+}
+
+static void __exit at91_twi_exit(void)
+{
+	platform_driver_unregister(&at91_twi_driver);
+}
+
+subsys_initcall(at91_twi_init);
+module_exit(at91_twi_exit);
+
+MODULE_AUTHOR("Nikolaus Voss <n.voss@weinmann.de>");
+MODULE_DESCRIPTION("I2C (TWI) driver for Atmel AT91");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:at91_i2c");
diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91-master.c
similarity index 67%
rename from drivers/i2c/busses/i2c-at91.c
rename to drivers/i2c/busses/i2c-at91-master.c
index bc74184462c6..4ef335f95506 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91-master.c
@@ -25,156 +25,15 @@
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
-#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
-#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		100000		/* 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
-#define AT91_I2C_MAX_ALT_CMD_DATA_SIZE	256
-
-/* AT91 TWI register definitions */
-#define	AT91_TWI_CR		0x0000	/* Control Register */
-#define	AT91_TWI_START		BIT(0)	/* Send a Start Condition */
-#define	AT91_TWI_STOP		BIT(1)	/* Send a Stop Condition */
-#define	AT91_TWI_MSEN		BIT(2)	/* Master Transfer Enable */
-#define	AT91_TWI_MSDIS		BIT(3)	/* Master Transfer Disable */
-#define	AT91_TWI_SVEN		BIT(4)	/* Slave Transfer Enable */
-#define	AT91_TWI_SVDIS		BIT(5)	/* Slave Transfer Disable */
-#define	AT91_TWI_QUICK		BIT(6)	/* SMBus quick command */
-#define	AT91_TWI_SWRST		BIT(7)	/* Software Reset */
-#define	AT91_TWI_ACMEN		BIT(16) /* Alternative Command Mode Enable */
-#define	AT91_TWI_ACMDIS		BIT(17) /* Alternative Command Mode Disable */
-#define	AT91_TWI_THRCLR		BIT(24) /* Transmit Holding Register Clear */
-#define	AT91_TWI_RHRCLR		BIT(25) /* Receive Holding Register Clear */
-#define	AT91_TWI_LOCKCLR	BIT(26) /* Lock Clear */
-#define	AT91_TWI_FIFOEN		BIT(28) /* FIFO Enable */
-#define	AT91_TWI_FIFODIS	BIT(29) /* FIFO Disable */
-
-#define	AT91_TWI_MMR		0x0004	/* Master Mode Register */
-#define	AT91_TWI_IADRSZ_1	0x0100	/* Internal Device Address Size */
-#define	AT91_TWI_MREAD		BIT(12)	/* Master Read Direction */
-
-#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 */
-#define	AT91_TWI_RXRDY		BIT(1)	/* Receive Holding Register Ready */
-#define	AT91_TWI_TXRDY		BIT(2)	/* Transmit Holding Register Ready */
-#define	AT91_TWI_OVRE		BIT(6)	/* Overrun Error */
-#define	AT91_TWI_UNRE		BIT(7)	/* Underrun Error */
-#define	AT91_TWI_NACK		BIT(8)	/* Not Acknowledged */
-#define	AT91_TWI_LOCK		BIT(23) /* TWI Lock due to Frame Errors */
-
-#define	AT91_TWI_INT_MASK \
-	(AT91_TWI_TXCOMP | AT91_TWI_RXRDY | AT91_TWI_TXRDY | AT91_TWI_NACK)
-
-#define	AT91_TWI_IER		0x0024	/* Interrupt Enable Register */
-#define	AT91_TWI_IDR		0x0028	/* Interrupt Disable Register */
-#define	AT91_TWI_IMR		0x002c	/* Interrupt Mask Register */
-#define	AT91_TWI_RHR		0x0030	/* Receive Holding Register */
-#define	AT91_TWI_THR		0x0034	/* Transmit Holding Register */
-
-#define	AT91_TWI_ACR		0x0040	/* Alternative Command Register */
-#define	AT91_TWI_ACR_DATAL(len)	((len) & 0xff)
-#define	AT91_TWI_ACR_DIR	BIT(8)
-
-#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)
-#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_FLR		0x0054	/* FIFO Level Register */
-
-#define	AT91_TWI_FSR		0x0060	/* 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_VER		0x00fc	/* Version Register */
-
-struct at91_twi_pdata {
-	unsigned clk_max_div;
-	unsigned clk_offset;
-	bool has_unre_flag;
-	bool has_alt_cmd;
-	bool has_hold_field;
-	struct at_dma_slave dma_slave;
-};
-
-struct at91_twi_dma {
-	struct dma_chan *chan_rx;
-	struct dma_chan *chan_tx;
-	struct scatterlist sg[2];
-	struct dma_async_tx_descriptor *data_desc;
-	enum dma_data_direction direction;
-	bool buf_mapped;
-	bool xfer_in_progress;
-};
-
-struct at91_twi_dev {
-	struct device *dev;
-	void __iomem *base;
-	struct completion cmd_complete;
-	struct clk *clk;
-	u8 *buf;
-	size_t buf_len;
-	struct i2c_msg *msg;
-	int irq;
-	unsigned imr;
-	unsigned transfer_status;
-	struct i2c_adapter adapter;
-	unsigned twi_cwgr_reg;
-	struct at91_twi_pdata *pdata;
-	bool use_dma;
-	bool use_alt_cmd;
-	bool recv_len_abort;
-	u32 fifo_size;
-	struct at91_twi_dma dma;
-};
-
-static unsigned at91_twi_read(struct at91_twi_dev *dev, unsigned reg)
-{
-	return readl_relaxed(dev->base + reg);
-}
-
-static void at91_twi_write(struct at91_twi_dev *dev, unsigned reg, unsigned val)
-{
-	writel_relaxed(val, dev->base + reg);
-}
-
-static void at91_disable_twi_interrupts(struct at91_twi_dev *dev)
-{
-	at91_twi_write(dev, AT91_TWI_IDR, AT91_TWI_INT_MASK);
-}
-
-static void at91_twi_irq_save(struct at91_twi_dev *dev)
-{
-	dev->imr = at91_twi_read(dev, AT91_TWI_IMR) & AT91_TWI_INT_MASK;
-	at91_disable_twi_interrupts(dev);
-}
 
-static void at91_twi_irq_restore(struct at91_twi_dev *dev)
-{
-	at91_twi_write(dev, AT91_TWI_IER, dev->imr);
-}
+#include "i2c-at91.h"
 
-static void at91_init_twi_bus_master(struct at91_twi_dev *dev)
+void at91_init_twi_bus_master(struct at91_twi_dev *dev)
 {
 	/* FIFO should be enabled immediately after the software reset */
 	if (dev->fifo_size)
@@ -184,14 +43,6 @@ static void at91_init_twi_bus_master(struct at91_twi_dev *dev)
 	at91_twi_write(dev, AT91_TWI_CWGR, dev->twi_cwgr_reg);
 }
 
-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);
-
-	at91_init_twi_bus_master(dev);
-}
-
 /*
  * Calculate symmetric clock as stated in datasheet:
  * twi_clk = F_MAIN / (2 * (cdiv * (1 << ckdiv) + offset))
@@ -839,124 +690,6 @@ static const struct i2c_algorithm at91_twi_algorithm = {
 	.functionality	= at91_twi_func,
 };
 
-static struct at91_twi_pdata at91rm9200_config = {
-	.clk_max_div = 5,
-	.clk_offset = 3,
-	.has_unre_flag = true,
-	.has_alt_cmd = false,
-	.has_hold_field = false,
-};
-
-static struct at91_twi_pdata at91sam9261_config = {
-	.clk_max_div = 5,
-	.clk_offset = 4,
-	.has_unre_flag = false,
-	.has_alt_cmd = false,
-	.has_hold_field = false,
-};
-
-static struct at91_twi_pdata at91sam9260_config = {
-	.clk_max_div = 7,
-	.clk_offset = 4,
-	.has_unre_flag = false,
-	.has_alt_cmd = false,
-	.has_hold_field = false,
-};
-
-static struct at91_twi_pdata at91sam9g20_config = {
-	.clk_max_div = 7,
-	.clk_offset = 4,
-	.has_unre_flag = false,
-	.has_alt_cmd = false,
-	.has_hold_field = false,
-};
-
-static struct at91_twi_pdata at91sam9g10_config = {
-	.clk_max_div = 7,
-	.clk_offset = 4,
-	.has_unre_flag = false,
-	.has_alt_cmd = false,
-	.has_hold_field = false,
-};
-
-static const struct platform_device_id at91_twi_devtypes[] = {
-	{
-		.name = "i2c-at91rm9200",
-		.driver_data = (unsigned long) &at91rm9200_config,
-	}, {
-		.name = "i2c-at91sam9261",
-		.driver_data = (unsigned long) &at91sam9261_config,
-	}, {
-		.name = "i2c-at91sam9260",
-		.driver_data = (unsigned long) &at91sam9260_config,
-	}, {
-		.name = "i2c-at91sam9g20",
-		.driver_data = (unsigned long) &at91sam9g20_config,
-	}, {
-		.name = "i2c-at91sam9g10",
-		.driver_data = (unsigned long) &at91sam9g10_config,
-	}, {
-		/* sentinel */
-	}
-};
-
-#if defined(CONFIG_OF)
-static struct at91_twi_pdata at91sam9x5_config = {
-	.clk_max_div = 7,
-	.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 = {
-	.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",
-		.data = &at91rm9200_config,
-	} , {
-		.compatible = "atmel,at91sam9260-i2c",
-		.data = &at91sam9260_config,
-	} , {
-		.compatible = "atmel,at91sam9261-i2c",
-		.data = &at91sam9261_config,
-	} , {
-		.compatible = "atmel,at91sam9g20-i2c",
-		.data = &at91sam9g20_config,
-	} , {
-		.compatible = "atmel,at91sam9g10-i2c",
-		.data = &at91sam9g10_config,
-	}, {
-		.compatible = "atmel,at91sam9x5-i2c",
-		.data = &at91sam9x5_config,
-	}, {
-		.compatible = "atmel,sama5d4-i2c",
-		.data = &sama5d4_config,
-	}, {
-		.compatible = "atmel,sama5d2-i2c",
-		.data = &sama5d2_config,
-	}, {
-		/* sentinel */
-	}
-};
-MODULE_DEVICE_TABLE(of, atmel_twi_dt_ids);
-#endif
-
 static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr)
 {
 	int ret = 0;
@@ -1039,21 +772,8 @@ static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr)
 	return ret;
 }
 
-static struct at91_twi_pdata *at91_twi_get_driver_data(
-					struct platform_device *pdev)
-{
-	if (pdev->dev.of_node) {
-		const struct of_device_id *match;
-		match = of_match_node(atmel_twi_dt_ids, pdev->dev.of_node);
-		if (!match)
-			return NULL;
-		return (struct at91_twi_pdata *)match->data;
-	}
-	return (struct at91_twi_pdata *) platform_get_device_id(pdev)->driver_data;
-}
-
-static int at91_twi_probe_master(struct platform_device *pdev,
-				 u32 phy_addr, struct at91_twi_dev *dev)
+int at91_twi_probe_master(struct platform_device *pdev,
+			  u32 phy_addr, struct at91_twi_dev *dev)
 {
 	int rc;
 	u32 bus_clk_rate;
@@ -1090,182 +810,3 @@ static int at91_twi_probe_master(struct platform_device *pdev,
 
 	return 0;
 }
-
-static int at91_twi_probe(struct platform_device *pdev)
-{
-	struct at91_twi_dev *dev;
-	struct resource *mem;
-	int rc;
-	u32 phy_addr;
-
-	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
-	if (!dev)
-		return -ENOMEM;
-
-	dev->dev = &pdev->dev;
-
-	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!mem)
-		return -ENODEV;
-	phy_addr = mem->start;
-
-	dev->pdata = at91_twi_get_driver_data(pdev);
-	if (!dev->pdata)
-		return -ENODEV;
-
-	dev->base = devm_ioremap_resource(&pdev->dev, mem);
-	if (IS_ERR(dev->base))
-		return PTR_ERR(dev->base);
-
-	dev->irq = platform_get_irq(pdev, 0);
-	if (dev->irq < 0)
-		return dev->irq;
-
-	platform_set_drvdata(pdev, dev);
-
-	dev->clk = devm_clk_get(dev->dev, NULL);
-	if (IS_ERR(dev->clk)) {
-		dev_err(dev->dev, "no clock defined\n");
-		return -ENODEV;
-	}
-	rc = clk_prepare_enable(dev->clk);
-	if (rc)
-		return rc;
-
-	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_DEPRECATED;
-	dev->adapter.dev.parent = dev->dev;
-	dev->adapter.nr = pdev->id;
-	dev->adapter.timeout = AT91_I2C_TIMEOUT;
-	dev->adapter.dev.of_node = pdev->dev.of_node;
-
-	rc = at91_twi_probe_master(pdev, phy_addr, dev);
-	if (rc)
-		return rc;
-
-	at91_init_twi_bus(dev);
-
-	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) {
-		clk_disable_unprepare(dev->clk);
-
-		pm_runtime_disable(dev->dev);
-		pm_runtime_set_suspended(dev->dev);
-
-		return rc;
-	}
-
-	dev_info(dev->dev, "AT91 i2c bus driver (hw version: %#x).\n",
-		 at91_twi_read(dev, AT91_TWI_VER));
-	return 0;
-}
-
-static int at91_twi_remove(struct platform_device *pdev)
-{
-	struct at91_twi_dev *dev = platform_get_drvdata(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
-
-static int at91_twi_runtime_suspend(struct device *dev)
-{
-	struct at91_twi_dev *twi_dev = dev_get_drvdata(dev);
-
-	clk_disable_unprepare(twi_dev->clk);
-
-	pinctrl_pm_select_sleep_state(dev);
-
-	return 0;
-}
-
-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);
-}
-
-static int at91_twi_suspend_noirq(struct device *dev)
-{
-	if (!pm_runtime_status_suspended(dev))
-		at91_twi_runtime_suspend(dev);
-
-	return 0;
-}
-
-static int at91_twi_resume_noirq(struct device *dev)
-{
-	struct at91_twi_dev *twi_dev = dev_get_drvdata(dev);
-	int ret;
-
-	if (!pm_runtime_status_suspended(dev)) {
-		ret = at91_twi_runtime_resume(dev);
-		if (ret)
-			return ret;
-	}
-
-	pm_runtime_mark_last_busy(dev);
-	pm_request_autosuspend(dev);
-
-	at91_init_twi_bus(twi_dev);
-
-	return 0;
-}
-
-static const struct dev_pm_ops at91_twi_pm = {
-	.suspend_noirq	= at91_twi_suspend_noirq,
-	.resume_noirq	= at91_twi_resume_noirq,
-	.runtime_suspend	= at91_twi_runtime_suspend,
-	.runtime_resume		= at91_twi_runtime_resume,
-};
-
-#define at91_twi_pm_ops (&at91_twi_pm)
-#else
-#define at91_twi_pm_ops NULL
-#endif
-
-static struct platform_driver at91_twi_driver = {
-	.probe		= at91_twi_probe,
-	.remove		= at91_twi_remove,
-	.id_table	= at91_twi_devtypes,
-	.driver		= {
-		.name	= "at91_i2c",
-		.of_match_table = of_match_ptr(atmel_twi_dt_ids),
-		.pm	= at91_twi_pm_ops,
-	},
-};
-
-static int __init at91_twi_init(void)
-{
-	return platform_driver_register(&at91_twi_driver);
-}
-
-static void __exit at91_twi_exit(void)
-{
-	platform_driver_unregister(&at91_twi_driver);
-}
-
-subsys_initcall(at91_twi_init);
-module_exit(at91_twi_exit);
-
-MODULE_AUTHOR("Nikolaus Voss <n.voss@weinmann.de>");
-MODULE_DESCRIPTION("I2C (TWI) driver for Atmel AT91");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:at91_i2c");
diff --git a/drivers/i2c/busses/i2c-at91.h b/drivers/i2c/busses/i2c-at91.h
new file mode 100644
index 000000000000..555b1674ddbe
--- /dev/null
+++ b/drivers/i2c/busses/i2c-at91.h
@@ -0,0 +1,151 @@
+/*
+ *  i2c Support for Atmel's AT91 Two-Wire Interface (TWI)
+ *
+ *  Copyright (C) 2011 Weinmann Medical GmbH
+ *  Author: Nikolaus Voss <n.voss@weinmann.de>
+ *
+ *  Evolved from original work by:
+ *  Copyright (C) 2004 Rick Bronson
+ *  Converted to 2.6 by Andrew Victor <andrew@sanpeople.com>
+ *
+ *  Borrowed heavily from original work by:
+ *  Copyright (C) 2000 Philip Edelbrock <phil@stimpy.netroedge.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ */
+
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/i2c.h>
+#include <linux/platform_data/dma-atmel.h>
+#include <linux/platform_device.h>
+
+#define DEFAULT_TWI_CLK_HZ		100000		/* 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
+#define AT91_I2C_MAX_ALT_CMD_DATA_SIZE	256
+
+/* AT91 TWI register definitions */
+#define	AT91_TWI_CR		0x0000	/* Control Register */
+#define	AT91_TWI_START		BIT(0)	/* Send a Start Condition */
+#define	AT91_TWI_STOP		BIT(1)	/* Send a Stop Condition */
+#define	AT91_TWI_MSEN		BIT(2)	/* Master Transfer Enable */
+#define	AT91_TWI_MSDIS		BIT(3)	/* Master Transfer Disable */
+#define	AT91_TWI_SVEN		BIT(4)	/* Slave Transfer Enable */
+#define	AT91_TWI_SVDIS		BIT(5)	/* Slave Transfer Disable */
+#define	AT91_TWI_QUICK		BIT(6)	/* SMBus quick command */
+#define	AT91_TWI_SWRST		BIT(7)	/* Software Reset */
+#define	AT91_TWI_ACMEN		BIT(16) /* Alternative Command Mode Enable */
+#define	AT91_TWI_ACMDIS		BIT(17) /* Alternative Command Mode Disable */
+#define	AT91_TWI_THRCLR		BIT(24) /* Transmit Holding Register Clear */
+#define	AT91_TWI_RHRCLR		BIT(25) /* Receive Holding Register Clear */
+#define	AT91_TWI_LOCKCLR	BIT(26) /* Lock Clear */
+#define	AT91_TWI_FIFOEN		BIT(28) /* FIFO Enable */
+#define	AT91_TWI_FIFODIS	BIT(29) /* FIFO Disable */
+
+#define	AT91_TWI_MMR		0x0004	/* Master Mode Register */
+#define	AT91_TWI_IADRSZ_1	0x0100	/* Internal Device Address Size */
+#define	AT91_TWI_MREAD		BIT(12)	/* Master Read Direction */
+
+#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 */
+#define	AT91_TWI_RXRDY		BIT(1)	/* Receive Holding Register Ready */
+#define	AT91_TWI_TXRDY		BIT(2)	/* Transmit Holding Register Ready */
+#define	AT91_TWI_OVRE		BIT(6)	/* Overrun Error */
+#define	AT91_TWI_UNRE		BIT(7)	/* Underrun Error */
+#define	AT91_TWI_NACK		BIT(8)	/* Not Acknowledged */
+#define	AT91_TWI_LOCK		BIT(23) /* TWI Lock due to Frame Errors */
+
+#define	AT91_TWI_INT_MASK \
+	(AT91_TWI_TXCOMP | AT91_TWI_RXRDY | AT91_TWI_TXRDY | AT91_TWI_NACK)
+
+#define	AT91_TWI_IER		0x0024	/* Interrupt Enable Register */
+#define	AT91_TWI_IDR		0x0028	/* Interrupt Disable Register */
+#define	AT91_TWI_IMR		0x002c	/* Interrupt Mask Register */
+#define	AT91_TWI_RHR		0x0030	/* Receive Holding Register */
+#define	AT91_TWI_THR		0x0034	/* Transmit Holding Register */
+
+#define	AT91_TWI_ACR		0x0040	/* Alternative Command Register */
+#define	AT91_TWI_ACR_DATAL(len)	((len) & 0xff)
+#define	AT91_TWI_ACR_DIR	BIT(8)
+
+#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)
+#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_FLR		0x0054	/* FIFO Level Register */
+
+#define	AT91_TWI_FSR		0x0060	/* 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_VER		0x00fc	/* Version Register */
+
+struct at91_twi_pdata {
+	unsigned clk_max_div;
+	unsigned clk_offset;
+	bool has_unre_flag;
+	bool has_alt_cmd;
+	bool has_hold_field;
+	struct at_dma_slave dma_slave;
+};
+
+struct at91_twi_dma {
+	struct dma_chan *chan_rx;
+	struct dma_chan *chan_tx;
+	struct scatterlist sg[2];
+	struct dma_async_tx_descriptor *data_desc;
+	enum dma_data_direction direction;
+	bool buf_mapped;
+	bool xfer_in_progress;
+};
+
+struct at91_twi_dev {
+	struct device *dev;
+	void __iomem *base;
+	struct completion cmd_complete;
+	struct clk *clk;
+	u8 *buf;
+	size_t buf_len;
+	struct i2c_msg *msg;
+	int irq;
+	unsigned imr;
+	unsigned transfer_status;
+	struct i2c_adapter adapter;
+	unsigned twi_cwgr_reg;
+	struct at91_twi_pdata *pdata;
+	bool use_dma;
+	bool use_alt_cmd;
+	bool recv_len_abort;
+	u32 fifo_size;
+	struct at91_twi_dma dma;
+};
+
+unsigned at91_twi_read(struct at91_twi_dev *dev, unsigned reg);
+void at91_twi_write(struct at91_twi_dev *dev, unsigned reg, unsigned val);
+void at91_disable_twi_interrupts(struct at91_twi_dev *dev);
+void at91_twi_irq_save(struct at91_twi_dev *dev);
+void at91_twi_irq_restore(struct at91_twi_dev *dev);
+void at91_init_twi_bus(struct at91_twi_dev *dev);
+
+void at91_init_twi_bus_master(struct at91_twi_dev *dev);
+int at91_twi_probe_master(struct platform_device *pdev, u32 phy_addr,
+			  struct at91_twi_dev *dev);
-- 
2.12.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 3/3] i2c: at91: added slave mode support
  2018-07-16  9:40 [PATCH v3 0/3] i2c: at91: slave mode support Ludovic Desroches
  2018-07-16  9:40 ` [PATCH v3 1/3] i2c: at91: segregate master mode specific code from probe and init func Ludovic Desroches
  2018-07-16  9:40 ` [PATCH v3 2/3] i2c: at91: split driver into core and master file Ludovic Desroches
@ 2018-07-16  9:40 ` Ludovic Desroches
  2018-07-20 22:41 ` [PATCH v3 0/3] i2c: at91: " Wolfram Sang
  3 siblings, 0 replies; 8+ messages in thread
From: Ludovic Desroches @ 2018-07-16  9:40 UTC (permalink / raw)
  To: linux-arm-kernel

From: Juergen Fitschen <jfi@ssv-embedded.de>

Slave mode driver is based on the concept of i2c-designware driver.

Signed-off-by: Juergen Fitschen <me@jue.yt>
Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
---
 drivers/i2c/busses/Makefile         |   3 +
 drivers/i2c/busses/i2c-at91-core.c  |  13 +++-
 drivers/i2c/busses/i2c-at91-slave.c | 147 ++++++++++++++++++++++++++++++++++++
 drivers/i2c/busses/i2c-at91.h       |  30 +++++++-
 4 files changed, 189 insertions(+), 4 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-at91-slave.c

diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 378daa860ed4..1ab8b51a7657 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -35,6 +35,9 @@ obj-$(CONFIG_I2C_ALTERA)	+= i2c-altera.o
 obj-$(CONFIG_I2C_ASPEED)	+= i2c-aspeed.o
 obj-$(CONFIG_I2C_AT91)		+= i2c-at91.o
 i2c-at91-objs			:= i2c-at91-core.o i2c-at91-master.o
+ifeq ($(CONFIG_I2C_SLAVE),y)
+	i2c-at91-objs		+= i2c-at91-slave.o
+endif
 obj-$(CONFIG_I2C_AU1550)	+= i2c-au1550.o
 obj-$(CONFIG_I2C_AXXIA)		+= i2c-axxia.o
 obj-$(CONFIG_I2C_BCM2835)	+= i2c-bcm2835.o
diff --git a/drivers/i2c/busses/i2c-at91-core.c b/drivers/i2c/busses/i2c-at91-core.c
index 5d9c6c81e6ab..c74283fa567f 100644
--- a/drivers/i2c/busses/i2c-at91-core.c
+++ b/drivers/i2c/busses/i2c-at91-core.c
@@ -60,8 +60,10 @@ 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);
-
-	at91_init_twi_bus_master(dev);
+	if (dev->slave_detected)
+		at91_init_twi_bus_slave(dev);
+	else
+		at91_init_twi_bus_master(dev);
 }
 
 static struct at91_twi_pdata at91rm9200_config = {
@@ -243,7 +245,12 @@ static int at91_twi_probe(struct platform_device *pdev)
 	dev->adapter.timeout = AT91_I2C_TIMEOUT;
 	dev->adapter.dev.of_node = pdev->dev.of_node;
 
-	rc = at91_twi_probe_master(pdev, phy_addr, dev);
+	dev->slave_detected = i2c_detect_slave_mode(&pdev->dev);
+
+	if (dev->slave_detected)
+		rc = at91_twi_probe_slave(pdev, phy_addr, dev);
+	else
+		rc = at91_twi_probe_master(pdev, phy_addr, dev);
 	if (rc)
 		return rc;
 
diff --git a/drivers/i2c/busses/i2c-at91-slave.c b/drivers/i2c/busses/i2c-at91-slave.c
new file mode 100644
index 000000000000..4b4808e0c8f7
--- /dev/null
+++ b/drivers/i2c/busses/i2c-at91-slave.c
@@ -0,0 +1,147 @@
+/*
+ *  i2c slave support for Atmel's AT91 Two-Wire Interface (TWI)
+ *
+ *  Copyright (C) 2017 Juergen Fitschen <me@jue.yt>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/pm_runtime.h>
+
+#include "i2c-at91.h"
+
+static irqreturn_t atmel_twi_interrupt_slave(int irq, void *dev_id)
+{
+	struct at91_twi_dev *dev = dev_id;
+	const unsigned status = at91_twi_read(dev, AT91_TWI_SR);
+	const unsigned irqstatus = status & at91_twi_read(dev, AT91_TWI_IMR);
+	u8 value;
+
+	if (!irqstatus)
+		return IRQ_NONE;
+
+	/* slave address has been detected on I2C bus */
+	if (irqstatus & AT91_TWI_SVACC) {
+		if (status & AT91_TWI_SVREAD) {
+			i2c_slave_event(dev->slave,
+					I2C_SLAVE_READ_REQUESTED, &value);
+			writeb_relaxed(value, dev->base + AT91_TWI_THR);
+			at91_twi_write(dev, AT91_TWI_IER,
+				       AT91_TWI_TXRDY | AT91_TWI_EOSACC);
+		} else {
+			i2c_slave_event(dev->slave,
+					I2C_SLAVE_WRITE_REQUESTED, &value);
+			at91_twi_write(dev, AT91_TWI_IER,
+				       AT91_TWI_RXRDY | AT91_TWI_EOSACC);
+		}
+		at91_twi_write(dev, AT91_TWI_IDR, AT91_TWI_SVACC);
+	}
+
+	/* byte transmitted to remote master */
+	if (irqstatus & AT91_TWI_TXRDY) {
+		i2c_slave_event(dev->slave, I2C_SLAVE_READ_PROCESSED, &value);
+		writeb_relaxed(value, dev->base + AT91_TWI_THR);
+	}
+
+	/* byte received from remote master */
+	if (irqstatus & AT91_TWI_RXRDY) {
+		value = readb_relaxed(dev->base + AT91_TWI_RHR);
+		i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED, &value);
+	}
+
+	/* master sent stop */
+	if (irqstatus & AT91_TWI_EOSACC) {
+		at91_twi_write(dev, AT91_TWI_IDR,
+			       AT91_TWI_TXRDY | AT91_TWI_RXRDY | AT91_TWI_EOSACC);
+		at91_twi_write(dev, AT91_TWI_IER, AT91_TWI_SVACC);
+		i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &value);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int at91_reg_slave(struct i2c_client *slave)
+{
+	struct at91_twi_dev *dev = i2c_get_adapdata(slave->adapter);
+
+	if (dev->slave)
+		return -EBUSY;
+
+	if (slave->flags & I2C_CLIENT_TEN)
+		return -EAFNOSUPPORT;
+
+	/* Make sure twi_clk doesn't get turned off! */
+	pm_runtime_get_sync(dev->dev);
+
+	dev->slave = slave;
+	dev->smr = AT91_TWI_SMR_SADR(slave->addr);
+
+	at91_init_twi_bus(dev);
+	at91_twi_write(dev, AT91_TWI_IER, AT91_TWI_SVACC);
+
+	dev_info(dev->dev, "entered slave mode (ADR=%d)\n", slave->addr);
+
+	return 0;
+}
+
+static int at91_unreg_slave(struct i2c_client *slave)
+{
+	struct at91_twi_dev *dev = i2c_get_adapdata(slave->adapter);
+
+	WARN_ON(!dev->slave);
+
+	dev_info(dev->dev, "leaving slave mode\n");
+
+	dev->slave = NULL;
+	dev->smr = 0;
+
+	at91_init_twi_bus(dev);
+
+	pm_runtime_put(dev->dev);
+
+	return 0;
+}
+
+static u32 at91_twi_func(struct i2c_adapter *adapter)
+{
+	return I2C_FUNC_SLAVE | I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL
+		| I2C_FUNC_SMBUS_READ_BLOCK_DATA;
+}
+
+static const struct i2c_algorithm at91_twi_algorithm_slave = {
+	.reg_slave	= at91_reg_slave,
+	.unreg_slave	= at91_unreg_slave,
+	.functionality	= at91_twi_func,
+};
+
+int at91_twi_probe_slave(struct platform_device *pdev,
+			 u32 phy_addr, struct at91_twi_dev *dev)
+{
+	int rc;
+
+	rc = devm_request_irq(&pdev->dev, dev->irq, atmel_twi_interrupt_slave,
+			      0, dev_name(dev->dev), dev);
+	if (rc) {
+		dev_err(dev->dev, "Cannot get irq %d: %d\n", dev->irq, rc);
+		return rc;
+	}
+
+	dev->adapter.algo = &at91_twi_algorithm_slave;
+
+	return 0;
+}
+
+void at91_init_twi_bus_slave(struct at91_twi_dev *dev)
+{
+	at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_MSDIS);
+	if (dev->slave_detected && dev->smr) {
+		at91_twi_write(dev, AT91_TWI_SMR, dev->smr);
+		at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_SVEN);
+	}
+}
diff --git a/drivers/i2c/busses/i2c-at91.h b/drivers/i2c/busses/i2c-at91.h
index 555b1674ddbe..bb502c192697 100644
--- a/drivers/i2c/busses/i2c-at91.h
+++ b/drivers/i2c/busses/i2c-at91.h
@@ -53,6 +53,10 @@
 #define	AT91_TWI_IADRSZ_1	0x0100	/* Internal Device Address Size */
 #define	AT91_TWI_MREAD		BIT(12)	/* Master Read Direction */
 
+#define	AT91_TWI_SMR		0x0008	/* Slave Mode Register */
+#define	AT91_TWI_SMR_SADR_MAX	0x007f
+#define	AT91_TWI_SMR_SADR(x)	(((x) & AT91_TWI_SMR_SADR_MAX) << 16)
+
 #define	AT91_TWI_IADR		0x000c	/* Internal Address Register */
 
 #define	AT91_TWI_CWGR		0x0010	/* Clock Waveform Generator Reg */
@@ -63,13 +67,17 @@
 #define	AT91_TWI_TXCOMP		BIT(0)	/* Transmission Complete */
 #define	AT91_TWI_RXRDY		BIT(1)	/* Receive Holding Register Ready */
 #define	AT91_TWI_TXRDY		BIT(2)	/* Transmit Holding Register Ready */
+#define	AT91_TWI_SVREAD		BIT(3)	/* Slave Read */
+#define	AT91_TWI_SVACC		BIT(4)	/* Slave Access */
 #define	AT91_TWI_OVRE		BIT(6)	/* Overrun Error */
 #define	AT91_TWI_UNRE		BIT(7)	/* Underrun Error */
 #define	AT91_TWI_NACK		BIT(8)	/* Not Acknowledged */
+#define	AT91_TWI_EOSACC		BIT(11)	/* End Of Slave Access */
 #define	AT91_TWI_LOCK		BIT(23) /* TWI Lock due to Frame Errors */
 
 #define	AT91_TWI_INT_MASK \
-	(AT91_TWI_TXCOMP | AT91_TWI_RXRDY | AT91_TWI_TXRDY | AT91_TWI_NACK)
+	(AT91_TWI_TXCOMP | AT91_TWI_RXRDY | AT91_TWI_TXRDY | AT91_TWI_NACK \
+	| AT91_TWI_SVACC | AT91_TWI_EOSACC)
 
 #define	AT91_TWI_IER		0x0024	/* Interrupt Enable Register */
 #define	AT91_TWI_IDR		0x0028	/* Interrupt Disable Register */
@@ -137,6 +145,11 @@ struct at91_twi_dev {
 	bool recv_len_abort;
 	u32 fifo_size;
 	struct at91_twi_dma dma;
+	bool slave_detected;
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	unsigned smr;
+	struct i2c_client *slave;
+#endif
 };
 
 unsigned at91_twi_read(struct at91_twi_dev *dev, unsigned reg);
@@ -149,3 +162,18 @@ void at91_init_twi_bus(struct at91_twi_dev *dev);
 void at91_init_twi_bus_master(struct at91_twi_dev *dev);
 int at91_twi_probe_master(struct platform_device *pdev, u32 phy_addr,
 			  struct at91_twi_dev *dev);
+
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+void at91_init_twi_bus_slave(struct at91_twi_dev *dev);
+int at91_twi_probe_slave(struct platform_device *pdev, u32 phy_addr,
+			 struct at91_twi_dev *dev);
+
+#else
+static inline void at91_init_twi_bus_slave(struct at91_twi_dev *dev) {}
+static inline int at91_twi_probe_slave(struct platform_device *pdev,
+				       u32 phy_addr, struct at91_twi_dev *dev)
+{
+	return -EINVAL;
+}
+
+#endif
-- 
2.12.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 0/3] i2c: at91: slave mode support
  2018-07-16  9:40 [PATCH v3 0/3] i2c: at91: slave mode support Ludovic Desroches
                   ` (2 preceding siblings ...)
  2018-07-16  9:40 ` [PATCH v3 3/3] i2c: at91: added slave mode support Ludovic Desroches
@ 2018-07-20 22:41 ` Wolfram Sang
  2018-07-30  7:14   ` Ludovic Desroches
  3 siblings, 1 reply; 8+ messages in thread
From: Wolfram Sang @ 2018-07-20 22:41 UTC (permalink / raw)
  To: linux-arm-kernel

> [Ludovic Desroches]
> Changes in v3:
>  - rebase (cherry-pick was enough)

Thanks for the rebase. I wonder, though, I recall to had more
complicated issues. However...

>  - fix checkpatch errors
>  - tests:
>    - hangs with a SAMA5D4 (master and slave on different busses) after about
>    100 transfers. It's the firs time I do this test.
>    - some mismatches with a SAMA5D4 as slave and a SAMA5D2 as master
>    I don't know if it's a regression. I don't remember having seen this
>    behavior previously.
>    I think it's worth taking those patches even if there are some possible
>    bugs. It'll allow to get more people using it and so to consolidate the
>    slave mode support.

I really want to see those patches go upstream, too. But I am also not a
big fan of delivering the user something with known issues. Especially
not when they affect the main feature to be added. My rationale here is
that someone who is able to fix the issues remaining will also be able
to pick up and apply patches.

Maybe, maybe if it was to be enabled by a special
I2C_AT91_SLAVE_EXPERIMANTEL symbol with lots of explanations. I need to
think twice about that, though.

Speaking of Kconfig, I think this series needs to place a

	select I2C_SLAVE

somewhere.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180721/c34789bd/attachment.sig>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 0/3] i2c: at91: slave mode support
  2018-07-20 22:41 ` [PATCH v3 0/3] i2c: at91: " Wolfram Sang
@ 2018-07-30  7:14   ` Ludovic Desroches
  2018-12-11 19:28     ` Wolfram Sang
  0 siblings, 1 reply; 8+ messages in thread
From: Ludovic Desroches @ 2018-07-30  7:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Jul 21, 2018 at 12:41:41AM +0200, Wolfram Sang wrote:
> > [Ludovic Desroches]
> > Changes in v3:
> >  - rebase (cherry-pick was enough)
> 
> Thanks for the rebase. I wonder, though, I recall to had more
> complicated issues. However...
> 
> >  - fix checkpatch errors
> >  - tests:
> >    - hangs with a SAMA5D4 (master and slave on different busses) after about
> >    100 transfers. It's the firs time I do this test.
> >    - some mismatches with a SAMA5D4 as slave and a SAMA5D2 as master
> >    I don't know if it's a regression. I don't remember having seen this
> >    behavior previously.
> >    I think it's worth taking those patches even if there are some possible
> >    bugs. It'll allow to get more people using it and so to consolidate the
> >    slave mode support.
> 
> I really want to see those patches go upstream, too. But I am also not a
> big fan of delivering the user something with known issues. Especially
> not when they affect the main feature to be added. My rationale here is
> that someone who is able to fix the issues remaining will also be able
> to pick up and apply patches.
> 
> Maybe, maybe if it was to be enabled by a special
> I2C_AT91_SLAVE_EXPERIMANTEL symbol with lots of explanations. I need to
> think twice about that, though.
> 

I understand your point. The experimental mentionning could be a good
trade-off. Let me know once you make up your mind.

> Speaking of Kconfig, I think this series needs to place a
> 
> 	select I2C_SLAVE
> 
> somewhere.
> 

Ok I'll update it if we go further with this set of patches.

Regards

Ludovic

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/3] i2c: at91: slave mode support
  2018-07-30  7:14   ` Ludovic Desroches
@ 2018-12-11 19:28     ` Wolfram Sang
  2018-12-21 16:16       ` Ludovic Desroches
  0 siblings, 1 reply; 8+ messages in thread
From: Wolfram Sang @ 2018-12-11 19:28 UTC (permalink / raw)
  To: linux-i2c, linux-arm-kernel, me, nicolas.ferre,
	alexandre.belloni, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 1836 bytes --]

Hi Ludovic,

> > >  - fix checkpatch errors
> > >  - tests:
> > >    - hangs with a SAMA5D4 (master and slave on different busses) after about
> > >    100 transfers. It's the firs time I do this test.
> > >    - some mismatches with a SAMA5D4 as slave and a SAMA5D2 as master
> > >    I don't know if it's a regression. I don't remember having seen this
> > >    behavior previously.
> > >    I think it's worth taking those patches even if there are some possible
> > >    bugs. It'll allow to get more people using it and so to consolidate the
> > >    slave mode support.
> > 
> > I really want to see those patches go upstream, too. But I am also not a
> > big fan of delivering the user something with known issues. Especially
> > not when they affect the main feature to be added. My rationale here is
> > that someone who is able to fix the issues remaining will also be able
> > to pick up and apply patches.
> > 
> > Maybe, maybe if it was to be enabled by a special
> > I2C_AT91_SLAVE_EXPERIMANTEL symbol with lots of explanations. I need to
> > think twice about that, though.
> > 
> 
> I understand your point. The experimental mentionning could be a good
> trade-off. Let me know once you make up your mind.
> 
> > Speaking of Kconfig, I think this series needs to place a
> > 
> > 	select I2C_SLAVE
> > 
> > somewhere.
> > 
> 
> Ok I'll update it if we go further with this set of patches.

Ok, I give in. If you:

* add 'select I2C_SLAVE'
* make slave support selectable by I2C_AT91_SLAVE_STAGING or
  _EXPERIMENTAL or something alike (default n)
* and add to the help text of that symbol the above known issues
  and 'not for production' and 'help wanted' and where to get more
  info and all that

then I'll apply this series soonish. Promised!

Thanks,

   Wolfram


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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/3] i2c: at91: slave mode support
  2018-12-11 19:28     ` Wolfram Sang
@ 2018-12-21 16:16       ` Ludovic Desroches
  0 siblings, 0 replies; 8+ messages in thread
From: Ludovic Desroches @ 2018-12-21 16:16 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: alexandre.belloni, linux-kernel, me, linux-i2c, linux-arm-kernel

Hi Wolfram,

On Tue, Dec 11, 2018 at 08:28:05PM +0100, Wolfram Sang wrote:
> Hi Ludovic,
> 
> > > >  - fix checkpatch errors
> > > >  - tests:
> > > >    - hangs with a SAMA5D4 (master and slave on different busses) after about
> > > >    100 transfers. It's the firs time I do this test.
> > > >    - some mismatches with a SAMA5D4 as slave and a SAMA5D2 as master
> > > >    I don't know if it's a regression. I don't remember having seen this
> > > >    behavior previously.
> > > >    I think it's worth taking those patches even if there are some possible
> > > >    bugs. It'll allow to get more people using it and so to consolidate the
> > > >    slave mode support.
> > > 
> > > I really want to see those patches go upstream, too. But I am also not a
> > > big fan of delivering the user something with known issues. Especially
> > > not when they affect the main feature to be added. My rationale here is
> > > that someone who is able to fix the issues remaining will also be able
> > > to pick up and apply patches.
> > > 
> > > Maybe, maybe if it was to be enabled by a special
> > > I2C_AT91_SLAVE_EXPERIMANTEL symbol with lots of explanations. I need to
> > > think twice about that, though.
> > > 
> > 
> > I understand your point. The experimental mentionning could be a good
> > trade-off. Let me know once you make up your mind.
> > 
> > > Speaking of Kconfig, I think this series needs to place a
> > > 
> > > 	select I2C_SLAVE
> > > 
> > > somewhere.
> > > 
> > 
> > Ok I'll update it if we go further with this set of patches.
> 
> Ok, I give in. If you:
> 
> * add 'select I2C_SLAVE'
> * make slave support selectable by I2C_AT91_SLAVE_STAGING or
>   _EXPERIMENTAL or something alike (default n)
> * and add to the help text of that symbol the above known issues
>   and 'not for production' and 'help wanted' and where to get more
>   info and all that
> 
> then I'll apply this series soonish. Promised!

Ok I will handle these requests and resend it.

Thanks

Ludovic

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-12-21 16:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-16  9:40 [PATCH v3 0/3] i2c: at91: slave mode support Ludovic Desroches
2018-07-16  9:40 ` [PATCH v3 1/3] i2c: at91: segregate master mode specific code from probe and init func Ludovic Desroches
2018-07-16  9:40 ` [PATCH v3 2/3] i2c: at91: split driver into core and master file Ludovic Desroches
2018-07-16  9:40 ` [PATCH v3 3/3] i2c: at91: added slave mode support Ludovic Desroches
2018-07-20 22:41 ` [PATCH v3 0/3] i2c: at91: " Wolfram Sang
2018-07-30  7:14   ` Ludovic Desroches
2018-12-11 19:28     ` Wolfram Sang
2018-12-21 16:16       ` Ludovic Desroches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).