Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH v10 0/6] i2c: designware: add I2C SLAVE support

309 views
Skip to first unread message

Luis Oliveira

unread,
Jun 8, 2017, 1:40:10 PM6/8/17
to
The purpose of this patchset is to enable Linux to be a I2C slave by
enabling the slave functionality in the designware I2C controller and at
the same time it does some cleaning of the existing code.

The patch 1 is for cleaning and commentary fix.
The patch 2 refactors the original i2c-designware-core and extracts all
master functions to a i2c-designware-master source file as suggested by
Andy Shevchenko when I first submited the update. The patch 3 then
separates the MASTER flow from the common flow.
The patch 4 introduces the SLAVE necessary definitions to the
i2c-designware library and adds a how-to-use example to the designware.txt
in device tree bindings.
The patch 5 adds the necessary functions to give the ability to be a SLAVE
to the controller and for that changes also had to be made in the
Makefile and Kconfig.
The patch 6 enables the SLAVE mode to be detected by the platform module.

The functionality was tested using the hardware independent slave-eeprom
driver based on top of i2c/for-next. The tree I used can be found here:

https://git.kernel.org/cgit/linux/kernel/git/wsa/linux.git/log/?h=i2c/for-next

Luis Oliveira (6):
i2c: designware: Cleaning and comment style fixes.
i2c: designware: refactoring of the i2c-designware
i2c: designware: MASTER mode as separated driver
i2c: designware: introducing I2C_SLAVE definitions
i2c: designware: add SLAVE mode functions
i2c: designware: enable SLAVE in platform module

.../devicetree/bindings/i2c/i2c-designware.txt | 16 +-
drivers/i2c/busses/Kconfig | 14 +-
drivers/i2c/busses/Makefile | 4 +
drivers/i2c/busses/i2c-designware-common.c | 280 +++++++++++++
drivers/i2c/busses/i2c-designware-core.h | 182 +++++++-
...c-designware-core.c => i2c-designware-master.c} | 465 +++------------------
drivers/i2c/busses/i2c-designware-pcidrv.c | 8 +-
drivers/i2c/busses/i2c-designware-platdrv.c | 117 ++++--
drivers/i2c/busses/i2c-designware-slave.c | 396 ++++++++++++++++++
9 files changed, 1029 insertions(+), 453 deletions(-)
create mode 100644 drivers/i2c/busses/i2c-designware-common.c
rename drivers/i2c/busses/{i2c-designware-core.c => i2c-designware-master.c} (60%)
create mode 100644 drivers/i2c/busses/i2c-designware-slave.c

--
2.13.0

Luis Oliveira

unread,
Jun 14, 2017, 6:50:08 AM6/14/17
to
The purpose of this commit is to fix some comments and styling in the
existing code due to the need of reuse this code. What is being made
here is:

- Sorted the headers files
- Corrected some comments style (capital letters, lowcase i2c)
- Reverse tree in the variables declaration
- Add/remove empty lines and tabs where needed
- Fix of misspelled word "endianness" and "transferred"
- Replaced the return variable "r" with the more standard "ret"

The value of this, besides the rules of coding style, is because I
will use this code after and it will make my future patch a lot bigger and
complicated to review. The work here won't bring any additional work to
backported fixes because is just style and reordering.

Signed-off-by: Luis Oliveira <lol...@synopsys.com>
Acked-by: Jarkko Nikula <jarkko...@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.s...@linux.intel.com>
---
v10-v11
- two extra spaces added to align the opening bracket @peda

drivers/i2c/busses/i2c-designware-core.c | 78 ++++++++++++++---------------
drivers/i2c/busses/i2c-designware-core.h | 2 +-
drivers/i2c/busses/i2c-designware-platdrv.c | 43 ++++++++--------
3 files changed, 62 insertions(+), 61 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 3c41995634c2..d9cab6dc2812 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -21,17 +21,17 @@
* ----------------------------------------------------------------------------
*
*/
+#include <linux/delay.h>
#include <linux/export.h>
#include <linux/errno.h>
#include <linux/err.h>
#include <linux/i2c.h>
#include <linux/interrupt.h>
#include <linux/io.h>
-#include <linux/pm_runtime.h>
-#include <linux/delay.h>
#include <linux/module.h>
-#include "i2c-designware-core.h"
+#include <linux/pm_runtime.h>

+#include "i2c-designware-core.h"
/*
* Registers offset
*/
@@ -98,7 +98,7 @@

#define DW_IC_ERR_TX_ABRT 0x1

-#define DW_IC_TAR_10BITADDR_MASTER BIT(12)
+#define DW_IC_TAR_10BITADDR_MASTER BIT(12)

#define DW_IC_COMP_PARAM_1_SPEED_MODE_HIGH (BIT(2) | BIT(3))
#define DW_IC_COMP_PARAM_1_SPEED_MODE_MASK GENMASK(3, 2)
@@ -113,10 +113,10 @@
#define TIMEOUT 20 /* ms */

/*
- * hardware abort codes from the DW_IC_TX_ABRT_SOURCE register
+ * Hardware abort codes from the DW_IC_TX_ABRT_SOURCE register
*
- * only expected abort codes are listed here
- * refer to the datasheet for the full list
+ * Only expected abort codes are listed here,
+ * refer to the datasheet for the full list.
*/
#define ABRT_7B_ADDR_NOACK 0
#define ABRT_10ADDR1_NOACK 1
@@ -318,7 +318,7 @@ static void i2c_dw_release_lock(struct dw_i2c_dev *dev)
}

/**
- * i2c_dw_init() - initialize the designware i2c master hardware
+ * i2c_dw_init() - Initialize the designware I2C master hardware
* @dev: device private data
*
* This functions configures and enables the I2C master.
@@ -344,8 +344,8 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
/* Configure register access mode 16bit */
dev->flags |= ACCESS_16BIT;
} else if (reg != DW_IC_COMP_TYPE_VALUE) {
- dev_err(dev->dev, "Unknown Synopsys component type: "
- "0x%08x\n", reg);
+ dev_err(dev->dev,
+ "Unknown Synopsys component type: 0x%08x\n", reg);
i2c_dw_release_lock(dev);
return -ENODEV;
}
@@ -355,7 +355,7 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
/* Disable the adapter */
__i2c_dw_enable_and_wait(dev, false);

- /* set standard and fast speed deviders for high/low periods */
+ /* Set standard and fast speed deviders for high/low periods */

sda_falling_time = dev->sda_falling_time ?: 300; /* ns */
scl_falling_time = dev->scl_falling_time ?: 300; /* ns */
@@ -444,7 +444,7 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
dw_writel(dev, dev->tx_fifo_depth / 2, DW_IC_TX_TL);
dw_writel(dev, 0, DW_IC_RX_TL);

- /* configure the i2c master */
+ /* Configure the I2C master */
dw_writel(dev, dev->master_cfg , DW_IC_CON);

i2c_dw_release_lock(dev);
@@ -480,7 +480,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
/* Disable the adapter */
__i2c_dw_enable_and_wait(dev, false);

- /* if the slave address is ten bit address, enable 10BITADDR */
+ /* If the slave address is ten bit address, enable 10BITADDR */
ic_con = dw_readl(dev, DW_IC_CON);
if (msgs[dev->msg_write_idx].flags & I2C_M_TEN) {
ic_con |= DW_IC_CON_10BITADDR_MASTER;
@@ -503,7 +503,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
*/
dw_writel(dev, msgs[dev->msg_write_idx].addr | ic_tar, DW_IC_TAR);

- /* enforce disabled interrupts (due to HW issues) */
+ /* Enforce disabled interrupts (due to HW issues) */
i2c_dw_disable_int(dev);

/* Enable the adapter */
@@ -537,9 +537,9 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
u32 flags = msgs[dev->msg_write_idx].flags;

/*
- * if target address has changed, we need to
- * reprogram the target address in the i2c
- * adapter when we are done with this transfer
+ * If target address has changed, we need to
+ * reprogram the target address in the I2C
+ * adapter when we are done with this transfer.
*/
if (msgs[dev->msg_write_idx].addr != addr) {
dev_err(dev->dev,
@@ -599,7 +599,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)

if (msgs[dev->msg_write_idx].flags & I2C_M_RD) {

- /* avoid rx buffer overrun */
+ /* Avoid rx buffer overrun */
if (dev->rx_outstanding >= dev->rx_fifo_depth)
break;

@@ -728,7 +728,7 @@ static int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
}

/*
- * Prepare controller for a transaction and call i2c_dw_xfer_msg
+ * Prepare controller for a transaction and call i2c_dw_xfer_msg.
*/
static int
i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
@@ -759,10 +759,10 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
if (ret < 0)
goto done;

- /* start the transfers */
+ /* Start the transfers */
i2c_dw_xfer_init(dev);

- /* wait for tx to complete */
+ /* Wait for tx to complete */
if (!wait_for_completion_timeout(&dev->cmd_complete, adap->timeout)) {
dev_err(dev->dev, "controller timed out\n");
/* i2c_dw_init implicitly disables the adapter */
@@ -786,7 +786,7 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
goto done;
}

- /* no error */
+ /* No error */
if (likely(!dev->cmd_err && !dev->status)) {
ret = num;
goto done;
@@ -821,8 +821,8 @@ static u32 i2c_dw_func(struct i2c_adapter *adap)
}

static const struct i2c_algorithm i2c_dw_algo = {
- .master_xfer = i2c_dw_xfer,
- .functionality = i2c_dw_func,
+ .master_xfer = i2c_dw_xfer,
+ .functionality = i2c_dw_func,
};

static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
@@ -903,7 +903,7 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)

/*
* Anytime TX_ABRT is set, the contents of the tx/rx
- * buffers are flushed. Make sure to skip them.
+ * buffers are flushed. Make sure to skip them.
*/
dw_writel(dev, 0, DW_IC_INTR_MASK);
goto tx_aborted;
@@ -925,7 +925,7 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
if ((stat & (DW_IC_INTR_TX_ABRT | DW_IC_INTR_STOP_DET)) || dev->msg_err)
complete(&dev->cmd_complete);
else if (unlikely(dev->flags & ACCESS_INTR_MASK)) {
- /* workaround to trigger pending interrupt */
+ /* Workaround to trigger pending interrupt */
stat = dw_readl(dev, DW_IC_INTR_MASK);
i2c_dw_disable_int(dev);
dw_writel(dev, stat, DW_IC_INTR_MASK);
@@ -961,13 +961,13 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
{
struct i2c_adapter *adap = &dev->adapter;
unsigned long irq_flags;
- int r;
+ int ret;

init_completion(&dev->cmd_complete);

- r = i2c_dw_init(dev);
- if (r)
- return r;
+ ret = i2c_dw_init(dev);
+ if (ret)
+ return ret;

snprintf(adap->name, sizeof(adap->name),
"Synopsys DesignWare I2C adapter");
@@ -984,12 +984,12 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
}

i2c_dw_disable_int(dev);
- r = devm_request_irq(dev->dev, dev->irq, i2c_dw_isr, irq_flags,
- dev_name(dev->dev), dev);
- if (r) {
+ ret = devm_request_irq(dev->dev, dev->irq, i2c_dw_isr, irq_flags,
+ dev_name(dev->dev), dev);
+ if (ret) {
dev_err(dev->dev, "failure requesting irq %i: %d\n",
- dev->irq, r);
- return r;
+ dev->irq, ret);
+ return ret;
}

/*
@@ -999,12 +999,12 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
* registered I2C slaves that do I2C transfers in their probe.
*/
pm_runtime_get_noresume(dev->dev);
- r = i2c_add_numbered_adapter(adap);
- if (r)
- dev_err(dev->dev, "failure adding adapter: %d\n", r);
+ ret = i2c_add_numbered_adapter(adap);
+ if (ret)
+ dev_err(dev->dev, "failure adding adapter: %d\n", ret);
pm_runtime_put_noidle(dev->dev);

- return r;
+ return ret;
}
EXPORT_SYMBOL_GPL(i2c_dw_probe);

diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index a7cf429daf60..fe9b66898a72 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -49,7 +49,7 @@
* @cmd_complete: tx completion indicator
* @clk: input reference clock
* @cmd_err: run time hadware error code
- * @msgs: points to an array of messages currently being transfered
+ * @msgs: points to an array of messages currently being transferred
* @msgs_num: the number of elements in msgs
* @msg_write_idx: the element index of the current tx message in the msgs
* array
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index d1263b82d646..613dfb88307f 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -21,27 +21,28 @@
* ----------------------------------------------------------------------------
*
*/
-#include <linux/kernel.h>
-#include <linux/module.h>
+#include <linux/acpi.h>
+#include <linux/clk-provider.h>
+#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/dmi.h>
-#include <linux/i2c.h>
-#include <linux/clk.h>
-#include <linux/clk-provider.h>
-#include <linux/errno.h>
-#include <linux/sched.h>
#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
#include <linux/of.h>
+#include <linux/platform_data/i2c-designware.h>
#include <linux/platform_device.h>
#include <linux/pm.h>
#include <linux/pm_runtime.h>
#include <linux/property.h>
-#include <linux/io.h>
#include <linux/reset.h>
+#include <linux/sched.h>
#include <linux/slab.h>
-#include <linux/acpi.h>
-#include <linux/platform_data/i2c-designware.h>
+
#include "i2c-designware-core.h"

static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
@@ -209,11 +210,11 @@ static void dw_i2c_set_fifo_size(struct dw_i2c_dev *dev, int id)
static int dw_i2c_plat_probe(struct platform_device *pdev)
{
struct dw_i2c_platform_data *pdata = dev_get_platdata(&pdev->dev);
- struct dw_i2c_dev *dev;
struct i2c_adapter *adap;
- struct resource *mem;
- int irq, r;
+ struct dw_i2c_dev *dev;
u32 acpi_speed, ht = 0;
+ struct resource *mem;
+ int irq, ret;

irq = platform_get_irq(pdev, 0);
if (irq < 0)
@@ -276,12 +277,12 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
&& dev->clk_freq != 1000000 && dev->clk_freq != 3400000) {
dev_err(&pdev->dev,
"Only 100kHz, 400kHz, 1MHz and 3.4MHz supported");
- r = -EINVAL;
+ ret = -EINVAL;
goto exit_reset;
}

- r = i2c_dw_probe_lock_support(dev);
- if (r)
+ ret = i2c_dw_probe_lock_support(dev);
+ if (ret)
goto exit_reset;

dev->functionality = I2C_FUNC_10BIT_ADDR | DW_IC_DEFAULT_FUNCTIONALITY;
@@ -327,11 +328,11 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
pm_runtime_enable(&pdev->dev);
}

- r = i2c_dw_probe(dev);
- if (r)
+ ret = i2c_dw_probe(dev);
+ if (ret)
goto exit_probe;

- return r;
+ return ret;

exit_probe:
if (!dev->pm_disabled)
@@ -339,7 +340,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
exit_reset:
if (!IS_ERR_OR_NULL(dev->rst))
reset_control_assert(dev->rst);
- return r;
+ return ret;
}

static int dw_i2c_plat_remove(struct platform_device *pdev)
@@ -423,7 +424,7 @@ static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
#define DW_I2C_DEV_PMOPS NULL
#endif

-/* work with hotplug and coldplug */
+/* Work with hotplug and coldplug */
MODULE_ALIAS("platform:i2c_designware");

static struct platform_driver dw_i2c_driver = {
--
2.13.0

Luis Oliveira

unread,
Jun 14, 2017, 6:50:08 AM6/14/17
to
drivers/i2c/busses/i2c-designware-common.c | 281 +++++++++++++
drivers/i2c/busses/i2c-designware-core.h | 180 +++++++-
...c-designware-core.c => i2c-designware-master.c} | 467 +++------------------
drivers/i2c/busses/i2c-designware-pcidrv.c | 9 +-
drivers/i2c/busses/i2c-designware-platdrv.c | 117 ++++--
drivers/i2c/busses/i2c-designware-slave.c | 398 ++++++++++++++++++
9 files changed, 1033 insertions(+), 453 deletions(-)

Luis Oliveira

unread,
Jun 14, 2017, 6:50:11 AM6/14/17
to
- Changes in Kconfig to enable I2C_DESIGNWARE_SLAVE support
- Slave functions added to core library file
- Slave abort sources added to common source file
- New driver: i2c-designware-slave added
- Changes in the Makefile to compile the I2C_DESIGNWARE_SLAVE module
when supported by the architecture.

All the SLAVE flow is added but it is not enabled via platform
driver.

Signed-off-by: Luis Oliveira <lol...@synopsys.com>
---
v10-v11
- select I2C_DESIGNWARE_SLAVE had to be moved to this patch

drivers/i2c/busses/Kconfig | 14 +-
drivers/i2c/busses/Makefile | 3 +
drivers/i2c/busses/i2c-designware-common.c | 6 +
drivers/i2c/busses/i2c-designware-core.h | 2 +
drivers/i2c/busses/i2c-designware-slave.c | 398 +++++++++++++++++++++++++++++
5 files changed, 422 insertions(+), 1 deletion(-)
create mode 100644 drivers/i2c/busses/i2c-designware-slave.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 144cbadc7c72..f3f22085e227 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -471,14 +471,26 @@ config I2C_DESIGNWARE_CORE
config I2C_DESIGNWARE_PLATFORM
tristate "Synopsys DesignWare Platform"
select I2C_DESIGNWARE_CORE
+ select I2C_DESIGNWARE_SLAVE
depends on (ACPI && COMMON_CLK) || !ACPI
help
If you say yes to this option, support will be included for the
- Synopsys DesignWare I2C adapter. Only master mode is supported.
+ Synopsys DesignWare I2C adapter.

This driver can also be built as a module. If so, the module
will be called i2c-designware-platform.

+config I2C_DESIGNWARE_SLAVE
+ bool "Synopsys DesignWare Slave"
+ select I2C_SLAVE
+ depends on I2C_DESIGNWARE_PLATFORM
+ help
+ If you say yes to this option, support will be included for the
+ Synopsys DesignWare I2C slave adapter.
+
+ This is not a standalone module, this module compiles together with
+ i2c-designware-core.
+
config I2C_DESIGNWARE_PCI
tristate "Synopsys DesignWare PCI"
depends on PCI
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index c89a4314c563..142f1b3a96c2 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -41,6 +41,9 @@ obj-$(CONFIG_I2C_CPM) += i2c-cpm.o
obj-$(CONFIG_I2C_DAVINCI) += i2c-davinci.o
obj-$(CONFIG_I2C_DESIGNWARE_CORE) += i2c-designware-core.o
i2c-designware-core-objs := i2c-designware-common.o i2c-designware-master.o
+ifneq ($(CONFIG_I2C_DESIGNWARE_SLAVE),n)
+i2c-designware-core-objs += i2c-designware-slave.o
+endif
obj-$(CONFIG_I2C_DESIGNWARE_PLATFORM) += i2c-designware-platform.o
i2c-designware-platform-objs := i2c-designware-platdrv.o
i2c-designware-platform-$(CONFIG_I2C_DESIGNWARE_BAYTRAIL) += i2c-designware-baytrail.o
diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 0d0ccb73f0e6..d1a69372432f 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -56,6 +56,12 @@ static char *abort_sources[] = {
"trying to use disabled adapter",
[ARB_LOST] =
"lost arbitration",
+ [ABRT_SLAVE_FLUSH_TXFIFO] =
+ "read command so flush old data in the TX FIFO",
+ [ABRT_SLAVE_ARBLOST] =
+ "slave lost the bus while transmitting data to a remote master",
+ [ABRT_SLAVE_RD_INTX] =
+ "incorrect slave-transmitter mode configuration",
};

u32 dw_readl(struct dw_i2c_dev *dev, int offset)
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 76807eafda53..64a39a983410 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -305,9 +305,11 @@ u32 i2c_dw_func(struct i2c_adapter *adap);
void i2c_dw_disable(struct dw_i2c_dev *dev);
void i2c_dw_disable_int(struct dw_i2c_dev *dev);
int i2c_dw_init(struct dw_i2c_dev *dev);
+int i2c_dw_init_slave(struct dw_i2c_dev *dev);

extern u32 i2c_dw_read_comp_param(struct dw_i2c_dev *dev);
extern int i2c_dw_probe(struct dw_i2c_dev *dev);
+extern int i2c_dw_probe_slave(struct dw_i2c_dev *dev);

#if IS_ENABLED(CONFIG_I2C_DESIGNWARE_BAYTRAIL)
extern int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
new file mode 100644
index 000000000000..84be0b76170d
--- /dev/null
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -0,0 +1,398 @@
+/*
+ * Synopsys DesignWare I2C adapter driver (slave only).
+ *
+ * Based on the Synopsys DesignWare I2C adapter driver (master).
+ *
+ * Copyright (C) 2016 Synopsys Inc.
+ *
+ * ----------------------------------------------------------------------------
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ * ----------------------------------------------------------------------------
+ *
+ */
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+
+#include "i2c-designware-core.h"
+
+static void i2c_dw_configure_fifo_slave(struct dw_i2c_dev *dev)
+{
+ /* Configure Tx/Rx FIFO threshold levels. */
+ dw_writel(dev, 0, DW_IC_TX_TL);
+ dw_writel(dev, 0, DW_IC_RX_TL);
+
+ /* Configure the I2C slave. */
+ dw_writel(dev, dev->slave_cfg, DW_IC_CON);
+ dw_writel(dev, DW_IC_INTR_SLAVE_MASK, DW_IC_INTR_MASK);
+}
+
+/**
+ * i2c_dw_init_slave() - Initialize the designware i2c slave hardware
+ * @dev: device private data
+ *
+ * This function configures and enables the I2C in slave mode.
+ * This function is called during I2C init function, and in case of timeout at
+ * run time.
+ */
+int i2c_dw_init_slave(struct dw_i2c_dev *dev)
+{
+ u32 sda_falling_time, scl_falling_time;
+ u32 reg, comp_param1;
+ u32 hcnt, lcnt;
+ int ret;
+
+ ret = i2c_dw_acquire_lock(dev);
+ if (ret)
+ return ret;
+
+ reg = dw_readl(dev, DW_IC_COMP_TYPE);
+ if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) {
+ /* Configure register endianness access. */
+ dev->flags |= ACCESS_SWAP;
+ } else if (reg == (DW_IC_COMP_TYPE_VALUE & 0x0000ffff)) {
+ /* Configure register access mode 16bit. */
+ dev->flags |= ACCESS_16BIT;
+ } else if (reg != DW_IC_COMP_TYPE_VALUE) {
+ dev_err(dev->dev,
+ "Unknown Synopsys component type: 0x%08x\n", reg);
+ i2c_dw_release_lock(dev);
+ return -ENODEV;
+ }
+
+ comp_param1 = dw_readl(dev, DW_IC_COMP_PARAM_1);
+
+ /* Disable the adapter. */
+ __i2c_dw_enable_and_wait(dev, false);
+
+ /* Set standard and fast speed deviders for high/low periods. */
+ sda_falling_time = dev->sda_falling_time ?: 300; /* ns */
+ scl_falling_time = dev->scl_falling_time ?: 300; /* ns */
+
+ /* Set SCL timing parameters for standard-mode. */
+ if (dev->ss_hcnt && dev->ss_lcnt) {
+ hcnt = dev->ss_hcnt;
+ lcnt = dev->ss_lcnt;
+ } else {
+ hcnt = i2c_dw_scl_hcnt(i2c_dw_clk_rate(dev),
+ 4000, /* tHD;STA = tHIGH = 4.0 us */
+ sda_falling_time,
+ 0, /* 0: DW default, 1: Ideal */
+ 0); /* No offset */
+ lcnt = i2c_dw_scl_lcnt(i2c_dw_clk_rate(dev),
+ 4700, /* tLOW = 4.7 us */
+ scl_falling_time,
+ 0); /* No offset */
+ }
+ dw_writel(dev, hcnt, DW_IC_SS_SCL_HCNT);
+ dw_writel(dev, lcnt, DW_IC_SS_SCL_LCNT);
+ dev_dbg(dev->dev, "Standard-mode HCNT:LCNT = %d:%d\n", hcnt, lcnt);
+
+ /* Set SCL timing parameters for fast-mode or fast-mode plus. */
+ if ((dev->clk_freq == 1000000) && dev->fp_hcnt && dev->fp_lcnt) {
+ hcnt = dev->fp_hcnt;
+ lcnt = dev->fp_lcnt;
+ } else if (dev->fs_hcnt && dev->fs_lcnt) {
+ hcnt = dev->fs_hcnt;
+ lcnt = dev->fs_lcnt;
+ } else {
+ hcnt = i2c_dw_scl_hcnt(i2c_dw_clk_rate(dev),
+ 600, /* tHD;STA = tHIGH = 0.6 us */
+ sda_falling_time,
+ 0, /* 0: DW default, 1: Ideal */
+ 0); /* No offset */
+ lcnt = i2c_dw_scl_lcnt(i2c_dw_clk_rate(dev),
+ 1300, /* tLOW = 1.3 us */
+ scl_falling_time,
+ 0); /* No offset */
+ }
+ dw_writel(dev, hcnt, DW_IC_FS_SCL_HCNT);
+ dw_writel(dev, lcnt, DW_IC_FS_SCL_LCNT);
+ dev_dbg(dev->dev, "Fast-mode HCNT:LCNT = %d:%d\n", hcnt, lcnt);
+
+ if ((dev->slave_cfg & DW_IC_CON_SPEED_MASK) ==
+ DW_IC_CON_SPEED_HIGH) {
+ if ((comp_param1 & DW_IC_COMP_PARAM_1_SPEED_MODE_MASK)
+ != DW_IC_COMP_PARAM_1_SPEED_MODE_HIGH) {
+ dev_err(dev->dev, "High Speed not supported!\n");
+ dev->slave_cfg &= ~DW_IC_CON_SPEED_MASK;
+ dev->slave_cfg |= DW_IC_CON_SPEED_FAST;
+ } else if (dev->hs_hcnt && dev->hs_lcnt) {
+ hcnt = dev->hs_hcnt;
+ lcnt = dev->hs_lcnt;
+ dw_writel(dev, hcnt, DW_IC_HS_SCL_HCNT);
+ dw_writel(dev, lcnt, DW_IC_HS_SCL_LCNT);
+ dev_dbg(dev->dev, "HighSpeed-mode HCNT:LCNT = %d:%d\n",
+ hcnt, lcnt);
+ }
+ }
+
+ /* Configure SDA Hold Time if required. */
+ reg = dw_readl(dev, DW_IC_COMP_VERSION);
+ if (reg >= DW_IC_SDA_HOLD_MIN_VERS) {
+ if (!dev->sda_hold_time) {
+ /* Keep previous hold time setting if no one set it. */
+ dev->sda_hold_time = dw_readl(dev, DW_IC_SDA_HOLD);
+ }
+ /*
+ * Workaround for avoiding TX arbitration lost in case I2C
+ * slave pulls SDA down "too quickly" after falling egde of
+ * SCL by enabling non-zero SDA RX hold. Specification says it
+ * extends incoming SDA low to high transition while SCL is
+ * high but it apprears to help also above issue.
+ */
+ if (!(dev->sda_hold_time & DW_IC_SDA_HOLD_RX_MASK))
+ dev->sda_hold_time |= 1 << DW_IC_SDA_HOLD_RX_SHIFT;
+ dw_writel(dev, dev->sda_hold_time, DW_IC_SDA_HOLD);
+ } else {
+ dev_warn(dev->dev,
+ "Hardware too old to adjust SDA hold time.\n");
+ }
+
+ i2c_dw_configure_fifo_slave(dev);
+ i2c_dw_release_lock(dev);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(i2c_dw_init_slave);
+
+int i2c_dw_reg_slave(struct i2c_client *slave)
+{
+ struct dw_i2c_dev *dev = i2c_get_adapdata(slave->adapter);
+
+ if (dev->slave)
+ return -EBUSY;
+ if (slave->flags & I2C_CLIENT_TEN)
+ return -EAFNOSUPPORT;
+ /*
+ * Set slave address in the IC_SAR register,
+ * the address to which the DW_apb_i2c responds.
+ */
+ __i2c_dw_enable(dev, false);
+ dw_writel(dev, slave->addr, DW_IC_SAR);
+ dev->slave = slave;
+
+ __i2c_dw_enable(dev, true);
+
+ dev->cmd_err = 0;
+ dev->msg_write_idx = 0;
+ dev->msg_read_idx = 0;
+ dev->msg_err = 0;
+ dev->status = STATUS_IDLE;
+ dev->abort_source = 0;
+ dev->rx_outstanding = 0;
+
+ return 0;
+}
+
+static int i2c_dw_unreg_slave(struct i2c_client *slave)
+{
+ struct dw_i2c_dev *dev = i2c_get_adapdata(slave->adapter);
+
+ dev->disable_int(dev);
+ dev->disable(dev);
+ dev->slave = NULL;
+
+ return 0;
+}
+
+static u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
+{
+ u32 stat;
+
+ /*
+ * The IC_INTR_STAT register just indicates "enabled" interrupts.
+ * Ths unmasked raw version of interrupt status bits are available
+ * in the IC_RAW_INTR_STAT register.
+ *
+ * That is,
+ * stat = dw_readl(IC_INTR_STAT);
+ * equals to,
+ * stat = dw_readl(IC_RAW_INTR_STAT) & dw_readl(IC_INTR_MASK);
+ *
+ * The raw version might be useful for debugging purposes.
+ */
+ stat = dw_readl(dev, DW_IC_INTR_STAT);
+
+ /*
+ * Do not use the IC_CLR_INTR register to clear interrupts, or
+ * you'll miss some interrupts, triggered during the period from
+ * dw_readl(IC_INTR_STAT) to dw_readl(IC_CLR_INTR).
+ *
+ * Instead, use the separately-prepared IC_CLR_* registers.
+ */
+ if (stat & DW_IC_INTR_TX_ABRT)
+ dw_readl(dev, DW_IC_CLR_TX_ABRT);
+ if (stat & DW_IC_INTR_RX_UNDER)
+ dw_readl(dev, DW_IC_CLR_RX_UNDER);
+ if (stat & DW_IC_INTR_RX_OVER)
+ dw_readl(dev, DW_IC_CLR_RX_OVER);
+ if (stat & DW_IC_INTR_TX_OVER)
+ dw_readl(dev, DW_IC_CLR_TX_OVER);
+ if (stat & DW_IC_INTR_RX_DONE)
+ dw_readl(dev, DW_IC_CLR_RX_DONE);
+ if (stat & DW_IC_INTR_ACTIVITY)
+ dw_readl(dev, DW_IC_CLR_ACTIVITY);
+ if (stat & DW_IC_INTR_STOP_DET)
+ dw_readl(dev, DW_IC_CLR_STOP_DET);
+ if (stat & DW_IC_INTR_START_DET)
+ dw_readl(dev, DW_IC_CLR_START_DET);
+ if (stat & DW_IC_INTR_GEN_CALL)
+ dw_readl(dev, DW_IC_CLR_GEN_CALL);
+
+ return stat;
+}
+
+/*
+ * Interrupt service routine. This gets called whenever an I2C slave interrupt
+ * occurs.
+ */
+
+static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
+{
+ u32 raw_stat, stat, enabled;
+ u8 val, slave_activity;
+
+ stat = dw_readl(dev, DW_IC_INTR_STAT);
+ enabled = dw_readl(dev, DW_IC_ENABLE);
+ raw_stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
+ slave_activity = ((dw_readl(dev, DW_IC_STATUS) &
+ DW_IC_STATUS_SLAVE_ACTIVITY) >> 6);
+
+ if (!enabled || !(raw_stat & ~DW_IC_INTR_ACTIVITY))
+ return 0;
+
+ dev_dbg(dev->dev,
+ "%#x STAUTS SLAVE_ACTTVITY=%#x : RAW_INTR_STAT=%#x"
+ " : INTR_STAT=%#x\n",
+ enabled, slave_activity, raw_stat, stat);
+
+ if ((stat & DW_IC_INTR_RX_FULL) && (stat & DW_IC_INTR_STOP_DET))
+ i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED, &val);
+
+ if (stat & DW_IC_INTR_RD_REQ) {
+ if (slave_activity) {
+ if (stat & DW_IC_INTR_RX_FULL) {
+ val = dw_readl(dev, DW_IC_DATA_CMD);
+
+ if (!i2c_slave_event(dev->slave,
+ I2C_SLAVE_WRITE_RECEIVED,
+ &val)) {
+ dev_vdbg(dev->dev, "Byte %X acked!",
+ val);
+ }
+ dw_readl(dev, DW_IC_CLR_RD_REQ);
+ stat = i2c_dw_read_clear_intrbits_slave(dev);
+ } else {
+ dw_readl(dev, DW_IC_CLR_RD_REQ);
+ dw_readl(dev, DW_IC_CLR_RX_UNDER);
+ stat = i2c_dw_read_clear_intrbits_slave(dev);
+ }
+ if (!i2c_slave_event(dev->slave,
+ I2C_SLAVE_READ_REQUESTED,
+ &val))
+ dw_writel(dev, val, DW_IC_DATA_CMD);
+ }
+ }
+
+ if (stat & DW_IC_INTR_RX_DONE) {
+ if (!i2c_slave_event(dev->slave, I2C_SLAVE_READ_PROCESSED,
+ &val))
+ dw_readl(dev, DW_IC_CLR_RX_DONE);
+
+ i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
+ stat = i2c_dw_read_clear_intrbits_slave(dev);
+ return 1;
+ }
+
+ if (stat & DW_IC_INTR_RX_FULL) {
+ val = dw_readl(dev, DW_IC_DATA_CMD);
+ if (!i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
+ &val))
+ dev_vdbg(dev->dev, "Byte %X acked!", val);
+ } else {
+ i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
+ stat = i2c_dw_read_clear_intrbits_slave(dev);
+ }
+
+ //~ if (stat & DW_IC_INTR_TX_OVER)
+ //~ dw_readl(dev, DW_IC_CLR_TX_OVER);
+
+ return 1;
+}
+
+static irqreturn_t i2c_dw_isr_slave(int this_irq, void *dev_id)
+{
+ struct dw_i2c_dev *dev = dev_id;
+ int ret;
+
+ i2c_dw_read_clear_intrbits_slave(dev);
+ ret = i2c_dw_irq_handler_slave(dev);
+ if (ret > 0)
+ complete(&dev->cmd_complete);
+
+ return IRQ_RETVAL(ret);
+}
+
+static struct i2c_algorithm i2c_dw_algo = {
+ .functionality = i2c_dw_func,
+ .reg_slave = i2c_dw_reg_slave,
+ .unreg_slave = i2c_dw_unreg_slave,
+};
+
+int i2c_dw_probe_slave(struct dw_i2c_dev *dev)
+{
+ struct i2c_adapter *adap = &dev->adapter;
+ int ret;
+
+ init_completion(&dev->cmd_complete);
+
+ dev->init = i2c_dw_init_slave;
+ dev->disable = i2c_dw_disable;
+ dev->disable_int = i2c_dw_disable_int;
+
+ ret = dev->init(dev);
+ if (ret)
+ return ret;
+
+ snprintf(adap->name, sizeof(adap->name),
+ "Synopsys DesignWare I2C Slave adapter");
+ adap->retries = 3;
+ adap->algo = &i2c_dw_algo;
+ adap->dev.parent = dev->dev;
+ i2c_set_adapdata(adap, dev);
+
+ ret = devm_request_irq(dev->dev, dev->irq, i2c_dw_isr_slave,
+ IRQF_SHARED, dev_name(dev->dev), dev);
+ if (ret) {
+ dev_err(dev->dev, "failure requesting irq %i: %d\n",
+ dev->irq, ret);
+ return ret;
+ }
+
+ ret = i2c_add_numbered_adapter(adap);
+ if (ret)
+ dev_err(dev->dev, "failure adding adapter: %d\n", ret);
+ pm_runtime_put_noidle(dev->dev);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(i2c_dw_probe_slave);
+
+MODULE_AUTHOR("Luis Oliveira <lol...@synopsys.com>");
+MODULE_DESCRIPTION("Synopsys DesignWare I2C bus slave adapter");
+MODULE_LICENSE("GPL v2");
--
2.13.0

Andy Shevchenko

unread,
Jun 14, 2017, 8:30:05 AM6/14/17
to
On Wed, Jun 14, 2017 at 1:43 PM, Luis Oliveira
<Luis.O...@synopsys.com> wrote:
> - Changes in Kconfig to enable I2C_DESIGNWARE_SLAVE support
> - Slave functions added to core library file
> - Slave abort sources added to common source file
> - New driver: i2c-designware-slave added
> - Changes in the Makefile to compile the I2C_DESIGNWARE_SLAVE module
> when supported by the architecture.
>
> All the SLAVE flow is added but it is not enabled via platform
> driver.

> + //~ if (stat & DW_IC_INTR_TX_OVER)
> + //~ dw_readl(dev, DW_IC_CLR_TX_OVER);

If you are going to ignore reviewers' comments you will delay your
work to be applied.

Certainly you are about to miss v4.13.

--
With Best Regards,
Andy Shevchenko

Luis Oliveira

unread,
Jun 14, 2017, 9:10:05 AM6/14/17
to
On 14-Jun-17 13:20, Andy Shevchenko wrote:
> On Wed, Jun 14, 2017 at 1:43 PM, Luis Oliveira
> <Luis.O...@synopsys.com> wrote:
>> - Changes in Kconfig to enable I2C_DESIGNWARE_SLAVE support
>> - Slave functions added to core library file
>> - Slave abort sources added to common source file
>> - New driver: i2c-designware-slave added
>> - Changes in the Makefile to compile the I2C_DESIGNWARE_SLAVE module
>> when supported by the architecture.
>>
>> All the SLAVE flow is added but it is not enabled via platform
>> driver.
>
>> + //~ if (stat & DW_IC_INTR_TX_OVER)
>> + //~ dw_readl(dev, DW_IC_CLR_TX_OVER);
>
> If you are going to ignore reviewers' comments you will delay your
> work to be applied.
>
Im so sorry. I was doing some tests on the irq_handler and used the wrong branch
to create the patchset.
Believe me I've looked for mistakes like this but I've read the code so many
times I don't think I can see the obvious anymore.

Wolfram Sang

unread,
Jun 19, 2017, 12:00:06 PM6/19/17
to
Hiya,

> Believe me I've looked for mistakes like this but I've read the code so many
> times I don't think I can see the obvious anymore.

Yes, I know this. And you worked hard on this slave feature,
acknowledged.

Patches 1-4 look good to me from what I glimpsed. I largely trust here
the *much* appreciated review from Jarkko and Andy. Thanks a lot, guys!

I wonder if we haven't reached a state where Luis just could fix the
buildbot error (missing 'select I2C_SLAVE') and the thing pointed out by
Andy and we handle further small fixes incrementally during the v4.13
cycle? AFAICS there is no major show-stopper, or am I wrong?

Thanks guys,

Wolfram
signature.asc

Andy Shevchenko

unread,
Jun 19, 2017, 12:10:34 PM6/19/17
to
I would go with the following plan:
1. Push 1-4
2. Resend 5-6 with addressed pointed issues for one more (fast) round

Luis Oliveira

unread,
Jun 19, 2017, 12:20:14 PM6/19/17
to
For the Kbuild error I fixed it by changing

ifneq ($(CONFIG_I2C_DESIGNWARE_SLAVE),n)
i2c-designware-core-objs += i2c-designware-slave.o

to:

ifeq ($(CONFIG_I2C_DESIGNWARE_SLAVE),y)
i2c-designware-core-objs += i2c-designware-slave.o

I have test it with the .config provided and it works now.

Wolfram Sang

unread,
Jun 19, 2017, 12:30:05 PM6/19/17
to
On Wed, Jun 14, 2017 at 11:43:21AM +0100, Luis Oliveira wrote:
> The purpose of this commit is to fix some comments and styling in the
> existing code due to the need of reuse this code. What is being made
> here is:
>
> - Sorted the headers files
> - Corrected some comments style (capital letters, lowcase i2c)
> - Reverse tree in the variables declaration
> - Add/remove empty lines and tabs where needed
> - Fix of misspelled word "endianness" and "transferred"
> - Replaced the return variable "r" with the more standard "ret"
>
> The value of this, besides the rules of coding style, is because I
> will use this code after and it will make my future patch a lot bigger and
> complicated to review. The work here won't bring any additional work to
> backported fixes because is just style and reordering.
>
> Signed-off-by: Luis Oliveira <lol...@synopsys.com>
> Acked-by: Jarkko Nikula <jarkko...@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andriy.s...@linux.intel.com>

Applied to for-next, thanks!

signature.asc

Wolfram Sang

unread,
Jun 19, 2017, 12:30:07 PM6/19/17
to

> > I wonder if we haven't reached a state where Luis just could fix the
> > buildbot error (missing 'select I2C_SLAVE') and the thing pointed out by
> > Andy and we handle further small fixes incrementally during the v4.13
> > cycle? AFAICS there is no major show-stopper, or am I wrong?
>
> I would go with the following plan:
> 1. Push 1-4
> 2. Resend 5-6 with addressed pointed issues for one more (fast) round

Good, so I'll pick up 1-4 now. Thanks!

signature.asc

Jarkko Nikula

unread,
Jun 20, 2017, 4:20:12 AM6/20/17
to
Yep, that's easier for everyone.

--
Jarkko

Luis Oliveira

unread,
Jun 22, 2017, 6:20:05 AM6/22/17
to
- Changes in Kconfig to enable I2C_DESIGNWARE_SLAVE support
- Slave functions added to core library file
- Slave abort sources added to common source file
- New driver: i2c-designware-slave added
- Changes in the Makefile to compile the I2C_DESIGNWARE_SLAVE module
when supported by the architecture.

All the SLAVE flow is added but it is not enabled via platform
driver.

Signed-off-by: Luis Oliveira <lol...@synopsys.com>
---
V11-v12
- Changed ifdef condition in the Makefile fixed Kbuild error

drivers/i2c/busses/Kconfig | 14 +-
drivers/i2c/busses/Makefile | 3 +
drivers/i2c/busses/i2c-designware-common.c | 6 +
drivers/i2c/busses/i2c-designware-core.h | 2 +
drivers/i2c/busses/i2c-designware-slave.c | 395 +++++++++++++++++++++++++++++
5 files changed, 419 insertions(+), 1 deletion(-)
create mode 100644 drivers/i2c/busses/i2c-designware-slave.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index ed7106e48ba4..ed92bf560825 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -473,14 +473,26 @@ config I2C_DESIGNWARE_CORE
index c89a4314c563..d630a6cbafeb 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -41,6 +41,9 @@ obj-$(CONFIG_I2C_CPM) += i2c-cpm.o
obj-$(CONFIG_I2C_DAVINCI) += i2c-davinci.o
obj-$(CONFIG_I2C_DESIGNWARE_CORE) += i2c-designware-core.o
i2c-designware-core-objs := i2c-designware-common.o i2c-designware-master.o
+ifeq ($(CONFIG_I2C_DESIGNWARE_SLAVE),y)
index 000000000000..1f6a13f69d0c
--- /dev/null
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -0,0 +1,395 @@

Luis Oliveira

unread,
Jun 22, 2017, 6:20:05 AM6/22/17
to
This patches follows the merged patchset that refactors I2C Designware and
enables it to have I2C support.

The patch 1 adds the necessary functions to give the ability to be a SLAVE
to the controller and for that changes also had to be made in the
Makefile and Kconfig.
The patch 2 enables the SLAVE mode to be detected by the platform module.

The functionality was tested using the hardware independent slave-eeprom
driver based on top of i2c/for-next. The tree I used can be found here:

https://git.kernel.org/cgit/linux/kernel/git/wsa/linux.git/log/?h=i2c/for-next

Luis Oliveira (2):
i2c: designware: add SLAVE mode functions
i2c: designware: enable SLAVE in platform module

drivers/i2c/busses/Kconfig | 14 +-
drivers/i2c/busses/Makefile | 3 +
drivers/i2c/busses/i2c-designware-common.c | 6 +
drivers/i2c/busses/i2c-designware-core.h | 4 +
drivers/i2c/busses/i2c-designware-platdrv.c | 41 ++-
drivers/i2c/busses/i2c-designware-slave.c | 395 ++++++++++++++++++++++++++++
6 files changed, 457 insertions(+), 6 deletions(-)

Luis Oliveira

unread,
Jun 22, 2017, 6:20:06 AM6/22/17
to
- Slave mode selected in platform module if the support is detected in
the DT.

Signed-off-by: Luis Oliveira <lol...@synopsys.com>
Reviewed-by: Andy Shevchenko <andy.sh...@gmail.com>
---
V11-V12
- no changes

drivers/i2c/busses/i2c-designware-core.h | 2 ++
drivers/i2c/busses/i2c-designware-platdrv.c | 41 +++++++++++++++++++++++++----
2 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 64a39a983410..7bfbcc62f6c5 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -228,6 +228,7 @@
* @disable: function to disable the controller
* @disable_int: function to disable all interrupts
* @init: function to initialize the I2C hardware
+ * @mode: operation mode - DW_IC_MASTER or DW_IC_SLAVE
*
* HCNT and LCNT parameters can be used if the platform knows more accurate
* values than the one computed based only on the input clock frequency.
@@ -282,6 +283,7 @@ struct dw_i2c_dev {
void (*disable)(struct dw_i2c_dev *dev);
void (*disable_int)(struct dw_i2c_dev *dev);
int (*init)(struct dw_i2c_dev *dev);
+ int mode;
};

#define ACCESS_SWAP 0x00000001
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 1f38c807be5f..2ea6d0d25a01 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -1,5 +1,5 @@
/*
- * Synopsys DesignWare I2C adapter driver (master only).
+ * Synopsys DesignWare I2C adapter driver.
*
* Based on the TI DAVINCI I2C adapter driver.
*
@@ -174,9 +174,13 @@ static inline int dw_i2c_acpi_configure(struct platform_device *pdev)

static void i2c_dw_configure_master(struct dw_i2c_dev *dev)
{
+ dev->functionality = I2C_FUNC_10BIT_ADDR | DW_IC_DEFAULT_FUNCTIONALITY;
+
dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
DW_IC_CON_RESTART_EN;

+ dev->mode = DW_IC_MASTER;
+
switch (dev->clk_freq) {
case 100000:
dev->master_cfg |= DW_IC_CON_SPEED_STD;
@@ -189,6 +193,28 @@ static void i2c_dw_configure_master(struct dw_i2c_dev *dev)
}
}

+static void i2c_dw_configure_slave(struct dw_i2c_dev *dev)
+{
+ dev->functionality = I2C_FUNC_SLAVE | DW_IC_DEFAULT_FUNCTIONALITY;
+
+ dev->slave_cfg = DW_IC_CON_RX_FIFO_FULL_HLD_CTRL |
+ DW_IC_CON_RESTART_EN | DW_IC_CON_STOP_DET_IFADDRESSED |
+ DW_IC_CON_SPEED_FAST;
+
+ dev->mode = DW_IC_SLAVE;
+
+ switch (dev->clk_freq) {
+ case 100000:
+ dev->slave_cfg |= DW_IC_CON_SPEED_STD;
+ break;
+ case 3400000:
+ dev->slave_cfg |= DW_IC_CON_SPEED_HIGH;
+ break;
+ default:
+ dev->slave_cfg |= DW_IC_CON_SPEED_FAST;
+ }
+}
+
static int i2c_dw_plat_prepare_clk(struct dw_i2c_dev *i_dev, bool prepare)
{
if (IS_ERR(i_dev->clk))
@@ -302,9 +328,10 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
if (ret)
goto exit_reset;

- dev->functionality = I2C_FUNC_10BIT_ADDR | DW_IC_DEFAULT_FUNCTIONALITY;
-
- i2c_dw_configure_master(dev);
+ if (i2c_detect_slave_mode(&pdev->dev))
+ i2c_dw_configure_slave(dev);
+ else
+ i2c_dw_configure_master(dev);

dev->clk = devm_clk_get(&pdev->dev, NULL);
if (!i2c_dw_plat_prepare_clk(dev, true)) {
@@ -333,7 +360,11 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
pm_runtime_enable(&pdev->dev);
}

- ret = i2c_dw_probe(dev);
+ if (dev->mode == DW_IC_SLAVE)
+ ret = i2c_dw_probe_slave(dev);
+ else
+ ret = i2c_dw_probe(dev);
+
if (ret)
goto exit_probe;

--
2.13.0

Andy Shevchenko

unread,
Jun 27, 2017, 10:30:10 AM6/27/17
to
On Thu, 2017-06-22 at 11:17 +0100, Luis Oliveira wrote:
> - Changes in Kconfig to enable I2C_DESIGNWARE_SLAVE support
> - Slave functions added to core library file
> - Slave abort sources added to common source file
> - New driver: i2c-designware-slave added
> - Changes in the Makefile to compile the I2C_DESIGNWARE_SLAVE module
>   when supported by the architecture.
>
> All the SLAVE flow is added but it is not enabled via platform
> driver.

There are couple of nits, otherwise I'm okay with the patch.

Reviewed-by: Andy Shevchenko <andriy.s...@linux.intel.com>

> V11-v12
> - Changed ifdef condition in the Makefile fixed Kbuild error

+ rebased on top of latest i2c-next (drop applied patches effectively)

> +static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
> +{
>

> + dev_dbg(dev->dev,

> + "%#x STAUTS SLAVE_ACTTVITY=%#x : RAW_INTR_STAT=%#x"
> + " : INTR_STAT=%#x\n",

Don't split string literals.

> + enabled, slave_activity, raw_stat, stat);


> + dev_vdbg(dev->dev, "Byte %X
> acked!",
> +  val);

I'm not sure it's any helpful.

--
Andy Shevchenko <andriy.s...@linux.intel.com>
Intel Finland Oy

Jarkko Nikula

unread,
Jun 27, 2017, 10:30:15 AM6/27/17
to
On 06/22/2017 01:17 PM, Luis Oliveira wrote:
> - Changes in Kconfig to enable I2C_DESIGNWARE_SLAVE support
> - Slave functions added to core library file
> - Slave abort sources added to common source file
> - New driver: i2c-designware-slave added
> - Changes in the Makefile to compile the I2C_DESIGNWARE_SLAVE module
> when supported by the architecture.
>
> All the SLAVE flow is added but it is not enabled via platform
> driver.
>
> Signed-off-by: Luis Oliveira <lol...@synopsys.com>
> ---
> V11-v12
> - Changed ifdef condition in the Makefile fixed Kbuild error
>
> drivers/i2c/busses/Kconfig | 14 +-
> drivers/i2c/busses/Makefile | 3 +
> drivers/i2c/busses/i2c-designware-common.c | 6 +
> drivers/i2c/busses/i2c-designware-core.h | 2 +
> drivers/i2c/busses/i2c-designware-slave.c | 395 +++++++++++++++++++++++++++++
> 5 files changed, 419 insertions(+), 1 deletion(-)
> create mode 100644 drivers/i2c/busses/i2c-designware-slave.c
>
...
> +static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
> +{
> + u32 raw_stat, stat, enabled;
> + u8 val, slave_activity;
> +
> + stat = dw_readl(dev, DW_IC_INTR_STAT);
> + enabled = dw_readl(dev, DW_IC_ENABLE);
> + raw_stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
> + slave_activity = ((dw_readl(dev, DW_IC_STATUS) &
> + DW_IC_STATUS_SLAVE_ACTIVITY) >> 6);
> +
> + if (!enabled || !(raw_stat & ~DW_IC_INTR_ACTIVITY))
> + return 0;
> +
> + dev_dbg(dev->dev,
> + "%#x STAUTS SLAVE_ACTTVITY=%#x : RAW_INTR_STAT=%#x"
> + " : INTR_STAT=%#x\n",
> + enabled, slave_activity, raw_stat, stat);
> +

Minor notes:
- Strings that are printed should fit in single line for making easier
to grep them from sources. Maybe not all of those variables are not
necessary in this debug print (e.g. are we interested other than bit 0
in DW_IC_ENABLE and line above already returns if it reads zero).
- Typos "STAUTS SLAVE_ACTTVITY".

You may add my acked by if you want to send another version or I'm fine
with incremental patch also.

Acked-by: Jarkko Nikula <jarkko...@linux.intel.com>

Jarkko Nikula

unread,
Jun 27, 2017, 10:40:10 AM6/27/17
to
On 06/22/2017 01:17 PM, Luis Oliveira wrote:
> - Slave mode selected in platform module if the support is detected in
> the DT.
>
> Signed-off-by: Luis Oliveira <lol...@synopsys.com>
> Reviewed-by: Andy Shevchenko <andy.sh...@gmail.com>
> ---
> V11-V12
> - no changes
>
> drivers/i2c/busses/i2c-designware-core.h | 2 ++
> drivers/i2c/busses/i2c-designware-platdrv.c | 41 +++++++++++++++++++++++++----
> 2 files changed, 38 insertions(+), 5 deletions(-)
>
Acked-by: Jarkko Nikula <jarkko...@linux.intel.com>

Wolfram Sang

unread,
Jun 27, 2017, 5:40:07 PM6/27/17
to
On Thu, Jun 22, 2017 at 11:17:33AM +0100, Luis Oliveira wrote:
> - Slave mode selected in platform module if the support is detected in
> the DT.
>
> Signed-off-by: Luis Oliveira <lol...@synopsys.com>
> Reviewed-by: Andy Shevchenko <andy.sh...@gmail.com>

Applied to for-next, thanks!

signature.asc

Wolfram Sang

unread,
Jun 27, 2017, 5:40:07 PM6/27/17
to
On Thu, Jun 22, 2017 at 11:17:32AM +0100, Luis Oliveira wrote:
> - Changes in Kconfig to enable I2C_DESIGNWARE_SLAVE support
> - Slave functions added to core library file
> - Slave abort sources added to common source file
> - New driver: i2c-designware-slave added
> - Changes in the Makefile to compile the I2C_DESIGNWARE_SLAVE module
> when supported by the architecture.
>
> All the SLAVE flow is added but it is not enabled via platform
> driver.
>
> Signed-off-by: Luis Oliveira <lol...@synopsys.com>

I fixed this checkpatch warning for you:

WARNING: quoted string split across lines
#391: FILE: drivers/i2c/busses/i2c-designware-slave.c:281:
+ "%#x STAUTS SLAVE_ACTTVITY=%#x : RAW_INTR_STAT=%#x"
+ " : INTR_STAT=%#x\n",

Nice, you fooled two code checkers by having code in comments :) No
need to fix, of course:

SMATCH
drivers/i2c/busses/i2c-designware-slave.c:318 i2c_dw_irq_handler_slave warn: unused return: stat = i2c_dw_read_clear_intrbits_slave()
drivers/i2c/busses/i2c-designware-slave.c:329 i2c_dw_irq_handler_slave warn: unused return: stat = i2c_dw_read_clear_intrbits_slave()
CPPCHECK
drivers/i2c/busses/i2c-designware-slave.c:329: style: Variable 'stat' is assigned a value that is never used.

Build warning:

CC drivers/i2c/busses/i2c-designware-slave.o
drivers/i2c/busses/i2c-designware-slave.c:173:5: warning: no previous prototype for ‘i2c_dw_reg_slave’ [-Wmissing-prototypes]
int i2c_dw_reg_slave(struct i2c_client *slave)

I declared the function static to fix it.

Applied to for-next, thanks!

signature.asc
0 new messages