* [PATCH v9 1/6] i2c: designware: Cleaning and comment style fixes.
2017-05-08 10:37 [PATCH v9 0/6] i2c: designware: add I2C SLAVE support Luis Oliveira
@ 2017-05-08 10:37 ` Luis Oliveira
2017-05-08 10:37 ` [PATCH v9 2/6] i2c: designware: refactoring of the i2c-designware Luis Oliveira
` (4 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Luis Oliveira @ 2017-05-08 10:37 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, jarkko.nikula, andriy.shevchenko,
mika.westerberg, linux-i2c, devicetree, linux-kernel
Cc: Luis.Oliveira, Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
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 <lolivei@synopsys.com>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v8-v9
- no changes
drivers/i2c/busses/i2c-designware-core.c | 76 ++++++++++++++---------------
drivers/i2c/busses/i2c-designware-core.h | 2 +-
drivers/i2c/busses/i2c-designware-platdrv.c | 43 ++++++++--------
3 files changed, 61 insertions(+), 60 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index c453717b753b..c0e9cf374e2b 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,
+ ret = devm_request_irq(dev->dev, dev->irq, i2c_dw_isr, irq_flags,
dev_name(dev->dev), dev);
- if (r) {
+ 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 5a4eb6b6bd92..972001c01bbf 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)
@@ -205,11 +206,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)
@@ -272,12 +273,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;
@@ -323,11 +324,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)
@@ -335,7 +336,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)
@@ -419,7 +420,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.11.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v9 2/6] i2c: designware: refactoring of the i2c-designware
2017-05-08 10:37 [PATCH v9 0/6] i2c: designware: add I2C SLAVE support Luis Oliveira
2017-05-08 10:37 ` [PATCH v9 1/6] i2c: designware: Cleaning and comment style fixes Luis Oliveira
@ 2017-05-08 10:37 ` Luis Oliveira
2017-05-08 10:37 ` [PATCH v9 3/6] i2c: designware: MASTER mode as separated driver Luis Oliveira
` (3 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Luis Oliveira @ 2017-05-08 10:37 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, jarkko.nikula, andriy.shevchenko,
mika.westerberg, linux-i2c, devicetree, linux-kernel
Cc: Luis.Oliveira, Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
- Factor out all _master() part of code from i2c-designware-core
and i2c-designware-platdrv to separate functions.
- Standardize all code related with MASTER mode.
- I have to take off DW_IC_INTR_TX_EMPTY from DW_IC_INTR_DEFAULT_MASK
because it is master specific.
The purpose of this is to prepare the controller to have is I2C MASTER
flow in a separate driver. To do this first all the
functions/definitions related to the MASTER flow were identified.
Signed-off-by: Luis Oliveira <lolivei@synopsys.com>
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
---
v8-v9
- no changes
drivers/i2c/busses/i2c-designware-core.c | 56 +++++++++++++++++------------
drivers/i2c/busses/i2c-designware-platdrv.c | 31 +++++++++-------
2 files changed, 52 insertions(+), 35 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index c0e9cf374e2b..ce5d11102070 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -87,10 +87,10 @@
#define DW_IC_INTR_GEN_CALL 0x800
#define DW_IC_INTR_DEFAULT_MASK (DW_IC_INTR_RX_FULL | \
- DW_IC_INTR_TX_EMPTY | \
DW_IC_INTR_TX_ABRT | \
DW_IC_INTR_STOP_DET)
-
+#define DW_IC_INTR_MASTER_MASK (DW_IC_INTR_DEFAULT_MASK | \
+ DW_IC_INTR_TX_EMPTY)
#define DW_IC_STATUS_ACTIVITY 0x1
#define DW_IC_SDA_HOLD_RX_SHIFT 16
@@ -202,6 +202,16 @@ static void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset)
}
}
+static void i2c_dw_configure_fifo_master(struct dw_i2c_dev *dev)
+{
+ /* Configure Tx/Rx FIFO threshold levels */
+ dw_writel(dev, dev->tx_fifo_depth / 2, DW_IC_TX_TL);
+ dw_writel(dev, 0, DW_IC_RX_TL);
+
+ /* Configure the I2C master */
+ dw_writel(dev, dev->master_cfg, DW_IC_CON);
+}
+
static u32
i2c_dw_scl_hcnt(u32 ic_clk, u32 tSYMBOL, u32 tf, int cond, int offset)
{
@@ -440,13 +450,7 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
"Hardware too old to adjust SDA hold time.\n");
}
- /* Configure Tx/Rx FIFO threshold levels */
- dw_writel(dev, dev->tx_fifo_depth / 2, DW_IC_TX_TL);
- dw_writel(dev, 0, DW_IC_RX_TL);
-
- /* Configure the I2C master */
- dw_writel(dev, dev->master_cfg , DW_IC_CON);
-
+ i2c_dw_configure_fifo_master(dev);
i2c_dw_release_lock(dev);
return 0;
@@ -511,7 +515,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
/* Clear and enable interrupts */
dw_readl(dev, DW_IC_CLR_INTR);
- dw_writel(dev, DW_IC_INTR_DEFAULT_MASK, DW_IC_INTR_MASK);
+ dw_writel(dev, DW_IC_INTR_MASTER_MASK, DW_IC_INTR_MASK);
}
/*
@@ -531,7 +535,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
u8 *buf = dev->tx_buf;
bool need_restart = false;
- intr_mask = DW_IC_INTR_DEFAULT_MASK;
+ intr_mask = DW_IC_INTR_MASTER_MASK;
for (; dev->msg_write_idx < dev->msgs_num; dev->msg_write_idx++) {
u32 flags = msgs[dev->msg_write_idx].flags;
@@ -881,22 +885,14 @@ static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
}
/*
- * Interrupt service routine. This gets called whenever an I2C interrupt
+ * Interrupt service routine. This gets called whenever an I2C master interrupt
* occurs.
*/
-static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
+static int i2c_dw_irq_handler_master(struct dw_i2c_dev *dev)
{
- struct dw_i2c_dev *dev = dev_id;
- u32 stat, enabled;
-
- enabled = dw_readl(dev, DW_IC_ENABLE);
- stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
- dev_dbg(dev->dev, "%s: enabled=%#x stat=%#x\n", __func__, enabled, stat);
- if (!enabled || !(stat & ~DW_IC_INTR_ACTIVITY))
- return IRQ_NONE;
+ u32 stat;
stat = i2c_dw_read_clear_intrbits(dev);
-
if (stat & DW_IC_INTR_TX_ABRT) {
dev->cmd_err |= DW_IC_ERR_TX_ABRT;
dev->status = STATUS_IDLE;
@@ -931,6 +927,22 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
dw_writel(dev, stat, DW_IC_INTR_MASK);
}
+ return 0;
+}
+
+static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
+{
+ struct dw_i2c_dev *dev = dev_id;
+ u32 stat, enabled;
+
+ enabled = dw_readl(dev, DW_IC_ENABLE);
+ stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
+ dev_dbg(dev->dev, "enabled=%#x stat=%#x\n", enabled, stat);
+ if (!enabled || !(stat & ~DW_IC_INTR_ACTIVITY))
+ return IRQ_NONE;
+
+ i2c_dw_irq_handler_master(dev);
+
return IRQ_HANDLED;
}
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 972001c01bbf..06205bd39d7e 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -168,6 +168,23 @@ static inline int dw_i2c_acpi_configure(struct platform_device *pdev)
}
#endif
+static void i2c_dw_configure_master(struct dw_i2c_dev *dev)
+{
+ dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
+ DW_IC_CON_RESTART_EN;
+
+ switch (dev->clk_freq) {
+ case 100000:
+ dev->master_cfg |= DW_IC_CON_SPEED_STD;
+ break;
+ case 3400000:
+ dev->master_cfg |= DW_IC_CON_SPEED_HIGH;
+ break;
+ default:
+ dev->master_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))
@@ -283,19 +300,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
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;
-
- switch (dev->clk_freq) {
- case 100000:
- dev->master_cfg |= DW_IC_CON_SPEED_STD;
- break;
- case 3400000:
- dev->master_cfg |= DW_IC_CON_SPEED_HIGH;
- break;
- default:
- dev->master_cfg |= DW_IC_CON_SPEED_FAST;
- }
+ i2c_dw_configure_master(dev);
dev->clk = devm_clk_get(&pdev->dev, NULL);
if (!i2c_dw_plat_prepare_clk(dev, true)) {
--
2.11.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v9 3/6] i2c: designware: MASTER mode as separated driver
2017-05-08 10:37 [PATCH v9 0/6] i2c: designware: add I2C SLAVE support Luis Oliveira
2017-05-08 10:37 ` [PATCH v9 1/6] i2c: designware: Cleaning and comment style fixes Luis Oliveira
2017-05-08 10:37 ` [PATCH v9 2/6] i2c: designware: refactoring of the i2c-designware Luis Oliveira
@ 2017-05-08 10:37 ` Luis Oliveira
2017-05-08 10:37 ` [PATCH v9 4/6] i2c: designware: introducing I2C_SLAVE definitions Luis Oliveira
` (2 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Luis Oliveira @ 2017-05-08 10:37 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, jarkko.nikula, andriy.shevchenko,
mika.westerberg, linux-i2c, devicetree, linux-kernel
Cc: Luis.Oliveira, Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
- The functions related to I2C master mode of operation were transformed
in a single driver.
- Common definitions were moved to i2c-designware-core.h
- The i2c-designware-core is now only a library file, the functions
associated are in a source file called i2c-designware-common and
are used by both i2c-designware-master and i2c-designware-slave.
- To decrease noise in namespace common i2c_dw_*() functions are
now using ops to keep them private.
- Designware PCI driver had to be changed to match the previous ops
functions implementation.
Almost all of the "core" source is now part of the "master" source. The
difference is the functions used by both modes and they are in the
"common" source file.
Signed-off-by: Luis Oliveira <lolivei@synopsys.com>
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
---
v8-v9
- no changes
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-designware-common.c | 274 +++++++++++++++
drivers/i2c/busses/i2c-designware-core.h | 145 +++++++-
...c-designware-core.c => i2c-designware-master.c} | 367 +--------------------
drivers/i2c/busses/i2c-designware-pcidrv.c | 8 +-
drivers/i2c/busses/i2c-designware-platdrv.c | 6 +-
6 files changed, 432 insertions(+), 369 deletions(-)
create mode 100644 drivers/i2c/busses/i2c-designware-common.c
rename drivers/i2c/busses/{i2c-designware-core.c => i2c-designware-master.c} (65%)
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 30b60855fbcd..c89a4314c563 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_I2C_CBUS_GPIO) += i2c-cbus-gpio.o
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
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
new file mode 100644
index 000000000000..98b64e4f5977
--- /dev/null
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -0,0 +1,274 @@
+/*
+ * Synopsys DesignWare I2C adapter driver.
+ *
+ * Based on the TI DAVINCI I2C adapter driver.
+ *
+ * Copyright (C) 2006 Texas Instruments.
+ * Copyright (C) 2007 MontaVista Software Inc.
+ * Copyright (C) 2009 Provigent Ltd.
+ *
+ * ----------------------------------------------------------------------------
+ *
+ * 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/export.h>
+#include <linux/errno.h>
+#include <linux/err.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 char *abort_sources[] = {
+ [ABRT_7B_ADDR_NOACK] =
+ "slave address not acknowledged (7bit mode)",
+ [ABRT_10ADDR1_NOACK] =
+ "first address byte not acknowledged (10bit mode)",
+ [ABRT_10ADDR2_NOACK] =
+ "second address byte not acknowledged (10bit mode)",
+ [ABRT_TXDATA_NOACK] =
+ "data not acknowledged",
+ [ABRT_GCALL_NOACK] =
+ "no acknowledgement for a general call",
+ [ABRT_GCALL_READ] =
+ "read after general call",
+ [ABRT_SBYTE_ACKDET] =
+ "start byte acknowledged",
+ [ABRT_SBYTE_NORSTRT] =
+ "trying to send start byte when restart is disabled",
+ [ABRT_10B_RD_NORSTRT] =
+ "trying to read when restart is disabled (10bit mode)",
+ [ABRT_MASTER_DIS] =
+ "trying to use disabled adapter",
+ [ARB_LOST] =
+ "lost arbitration",
+};
+
+u32 dw_readl(struct dw_i2c_dev *dev, int offset)
+{
+ u32 value;
+
+ if (dev->flags & ACCESS_16BIT)
+ value = readw_relaxed(dev->base + offset) |
+ (readw_relaxed(dev->base + offset + 2) << 16);
+ else
+ value = readl_relaxed(dev->base + offset);
+
+ if (dev->flags & ACCESS_SWAP)
+ return swab32(value);
+ else
+ return value;
+}
+
+void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset)
+{
+ if (dev->flags & ACCESS_SWAP)
+ b = swab32(b);
+
+ if (dev->flags & ACCESS_16BIT) {
+ writew_relaxed((u16)b, dev->base + offset);
+ writew_relaxed((u16)(b >> 16), dev->base + offset + 2);
+ } else {
+ writel_relaxed(b, dev->base + offset);
+ }
+}
+
+u32 i2c_dw_scl_hcnt(u32 ic_clk, u32 tSYMBOL, u32 tf, int cond, int offset)
+{
+ /*
+ * DesignWare I2C core doesn't seem to have solid strategy to meet
+ * the tHD;STA timing spec. Configuring _HCNT based on tHIGH spec
+ * will result in violation of the tHD;STA spec.
+ */
+ if (cond)
+ /*
+ * Conditional expression:
+ *
+ * IC_[FS]S_SCL_HCNT + (1+4+3) >= IC_CLK * tHIGH
+ *
+ * This is based on the DW manuals, and represents an ideal
+ * configuration. The resulting I2C bus speed will be
+ * faster than any of the others.
+ *
+ * If your hardware is free from tHD;STA issue, try this one.
+ */
+ return (ic_clk * tSYMBOL + 500000) / 1000000 - 8 + offset;
+ else
+ /*
+ * Conditional expression:
+ *
+ * IC_[FS]S_SCL_HCNT + 3 >= IC_CLK * (tHD;STA + tf)
+ *
+ * This is just experimental rule; the tHD;STA period turned
+ * out to be proportinal to (_HCNT + 3). With this setting,
+ * we could meet both tHIGH and tHD;STA timing specs.
+ *
+ * If unsure, you'd better to take this alternative.
+ *
+ * The reason why we need to take into account "tf" here,
+ * is the same as described in i2c_dw_scl_lcnt().
+ */
+ return (ic_clk * (tSYMBOL + tf) + 500000) / 1000000
+ - 3 + offset;
+}
+
+u32 i2c_dw_scl_lcnt(u32 ic_clk, u32 tLOW, u32 tf, int offset)
+{
+ /*
+ * Conditional expression:
+ *
+ * IC_[FS]S_SCL_LCNT + 1 >= IC_CLK * (tLOW + tf)
+ *
+ * DW I2C core starts counting the SCL CNTs for the LOW period
+ * of the SCL clock (tLOW) as soon as it pulls the SCL line.
+ * In order to meet the tLOW timing spec, we need to take into
+ * account the fall time of SCL signal (tf). Default tf value
+ * should be 0.3 us, for safety.
+ */
+ return ((ic_clk * (tLOW + tf) + 500000) / 1000000) - 1 + offset;
+}
+
+void __i2c_dw_enable(struct dw_i2c_dev *dev, bool enable)
+{
+ dw_writel(dev, enable, DW_IC_ENABLE);
+}
+
+void __i2c_dw_enable_and_wait(struct dw_i2c_dev *dev, bool enable)
+{
+ int timeout = 100;
+
+ do {
+ __i2c_dw_enable(dev, enable);
+ if ((dw_readl(dev, DW_IC_ENABLE_STATUS) & 1) == enable)
+ return;
+
+ /*
+ * Wait 10 times the signaling period of the highest I2C
+ * transfer supported by the driver (for 400KHz this is
+ * 25us) as described in the DesignWare I2C databook.
+ */
+ usleep_range(25, 250);
+ } while (timeout--);
+
+ dev_warn(dev->dev, "timeout in %sabling adapter\n",
+ enable ? "en" : "dis");
+}
+
+unsigned long i2c_dw_clk_rate(struct dw_i2c_dev *dev)
+{
+ /*
+ * Clock is not necessary if we got LCNT/HCNT values directly from
+ * the platform code.
+ */
+ if (WARN_ON_ONCE(!dev->get_clk_rate_khz))
+ return 0;
+ return dev->get_clk_rate_khz(dev);
+}
+
+int i2c_dw_acquire_lock(struct dw_i2c_dev *dev)
+{
+ int ret;
+
+ if (!dev->acquire_lock)
+ return 0;
+
+ ret = dev->acquire_lock(dev);
+ if (!ret)
+ return 0;
+
+ dev_err(dev->dev, "couldn't acquire bus ownership\n");
+
+ return ret;
+}
+
+void i2c_dw_release_lock(struct dw_i2c_dev *dev)
+{
+ if (dev->release_lock)
+ dev->release_lock(dev);
+}
+
+/*
+ * Waiting for bus not busy
+ */
+int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev)
+{
+ int timeout = TIMEOUT;
+
+ while (dw_readl(dev, DW_IC_STATUS) & DW_IC_STATUS_ACTIVITY) {
+ if (timeout <= 0) {
+ dev_warn(dev->dev, "timeout waiting for bus ready\n");
+ return -ETIMEDOUT;
+ }
+ timeout--;
+ usleep_range(1000, 1100);
+ }
+
+ return 0;
+}
+
+int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
+{
+ unsigned long abort_source = dev->abort_source;
+ int i;
+
+ if (abort_source & DW_IC_TX_ABRT_NOACK) {
+ for_each_set_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
+ dev_dbg(dev->dev,
+ "%s: %s\n", __func__, abort_sources[i]);
+ return -EREMOTEIO;
+ }
+
+ for_each_set_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
+ dev_err(dev->dev, "%s: %s\n", __func__, abort_sources[i]);
+
+ if (abort_source & DW_IC_TX_ARB_LOST)
+ return -EAGAIN;
+ else if (abort_source & DW_IC_TX_ABRT_GCALL_READ)
+ return -EINVAL; /* wrong msgs[] data */
+ else
+ return -EIO;
+}
+
+u32 i2c_dw_func(struct i2c_adapter *adap)
+{
+ struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
+
+ return dev->functionality;
+}
+void i2c_dw_disable(struct dw_i2c_dev *dev)
+{
+ /* Disable controller */
+ __i2c_dw_enable_and_wait(dev, false);
+
+ /* Disable all interupts */
+ dw_writel(dev, 0, DW_IC_INTR_MASK);
+ dw_readl(dev, DW_IC_CLR_INTR);
+}
+
+void i2c_dw_disable_int(struct dw_i2c_dev *dev)
+{
+ dw_writel(dev, 0, DW_IC_INTR_MASK);
+}
+
+u32 i2c_dw_read_comp_param(struct dw_i2c_dev *dev)
+{
+ return dw_readl(dev, DW_IC_COMP_PARAM_1);
+}
+EXPORT_SYMBOL_GPL(i2c_dw_read_comp_param);
+
+MODULE_DESCRIPTION("Synopsys DesignWare I2C bus adapter core");
+MODULE_LICENSE("GPL");
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index fe9b66898a72..81b46c8529fa 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -41,6 +41,124 @@
#define DW_IC_CON_RESTART_EN 0x20
#define DW_IC_CON_SLAVE_DISABLE 0x40
+/*
+ * Registers offset
+ */
+#define DW_IC_CON 0x0
+#define DW_IC_TAR 0x4
+#define DW_IC_DATA_CMD 0x10
+#define DW_IC_SS_SCL_HCNT 0x14
+#define DW_IC_SS_SCL_LCNT 0x18
+#define DW_IC_FS_SCL_HCNT 0x1c
+#define DW_IC_FS_SCL_LCNT 0x20
+#define DW_IC_HS_SCL_HCNT 0x24
+#define DW_IC_HS_SCL_LCNT 0x28
+#define DW_IC_INTR_STAT 0x2c
+#define DW_IC_INTR_MASK 0x30
+#define DW_IC_RAW_INTR_STAT 0x34
+#define DW_IC_RX_TL 0x38
+#define DW_IC_TX_TL 0x3c
+#define DW_IC_CLR_INTR 0x40
+#define DW_IC_CLR_RX_UNDER 0x44
+#define DW_IC_CLR_RX_OVER 0x48
+#define DW_IC_CLR_TX_OVER 0x4c
+#define DW_IC_CLR_RD_REQ 0x50
+#define DW_IC_CLR_TX_ABRT 0x54
+#define DW_IC_CLR_RX_DONE 0x58
+#define DW_IC_CLR_ACTIVITY 0x5c
+#define DW_IC_CLR_STOP_DET 0x60
+#define DW_IC_CLR_START_DET 0x64
+#define DW_IC_CLR_GEN_CALL 0x68
+#define DW_IC_ENABLE 0x6c
+#define DW_IC_STATUS 0x70
+#define DW_IC_TXFLR 0x74
+#define DW_IC_RXFLR 0x78
+#define DW_IC_SDA_HOLD 0x7c
+#define DW_IC_TX_ABRT_SOURCE 0x80
+#define DW_IC_ENABLE_STATUS 0x9c
+#define DW_IC_COMP_PARAM_1 0xf4
+#define DW_IC_COMP_VERSION 0xf8
+#define DW_IC_SDA_HOLD_MIN_VERS 0x3131312A
+#define DW_IC_COMP_TYPE 0xfc
+#define DW_IC_COMP_TYPE_VALUE 0x44570140
+
+#define DW_IC_INTR_RX_UNDER 0x001
+#define DW_IC_INTR_RX_OVER 0x002
+#define DW_IC_INTR_RX_FULL 0x004
+#define DW_IC_INTR_TX_OVER 0x008
+#define DW_IC_INTR_TX_EMPTY 0x010
+#define DW_IC_INTR_RD_REQ 0x020
+#define DW_IC_INTR_TX_ABRT 0x040
+#define DW_IC_INTR_RX_DONE 0x080
+#define DW_IC_INTR_ACTIVITY 0x100
+#define DW_IC_INTR_STOP_DET 0x200
+#define DW_IC_INTR_START_DET 0x400
+#define DW_IC_INTR_GEN_CALL 0x800
+
+#define DW_IC_INTR_DEFAULT_MASK (DW_IC_INTR_RX_FULL | \
+ DW_IC_INTR_TX_ABRT | \
+ DW_IC_INTR_STOP_DET)
+#define DW_IC_INTR_MASTER_MASK (DW_IC_INTR_DEFAULT_MASK | \
+ DW_IC_INTR_TX_EMPTY)
+#define DW_IC_STATUS_ACTIVITY 0x1
+#define DW_IC_STATUS_TFE BIT(2)
+#define DW_IC_STATUS_MASTER_ACTIVITY BIT(5)
+
+#define DW_IC_SDA_HOLD_RX_SHIFT 16
+#define DW_IC_SDA_HOLD_RX_MASK GENMASK(23, DW_IC_SDA_HOLD_RX_SHIFT)
+
+#define DW_IC_ERR_TX_ABRT 0x1
+
+#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)
+
+/*
+ * Status codes
+ */
+#define STATUS_IDLE 0x0
+#define STATUS_WRITE_IN_PROGRESS 0x1
+#define STATUS_READ_IN_PROGRESS 0x2
+
+#define TIMEOUT 20 /* ms */
+
+/*
+ * 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
+ */
+#define ABRT_7B_ADDR_NOACK 0
+#define ABRT_10ADDR1_NOACK 1
+#define ABRT_10ADDR2_NOACK 2
+#define ABRT_TXDATA_NOACK 3
+#define ABRT_GCALL_NOACK 4
+#define ABRT_GCALL_READ 5
+#define ABRT_SBYTE_ACKDET 7
+#define ABRT_SBYTE_NORSTRT 9
+#define ABRT_10B_RD_NORSTRT 10
+#define ABRT_MASTER_DIS 11
+#define ARB_LOST 12
+
+#define DW_IC_TX_ABRT_7B_ADDR_NOACK (1UL << ABRT_7B_ADDR_NOACK)
+#define DW_IC_TX_ABRT_10ADDR1_NOACK (1UL << ABRT_10ADDR1_NOACK)
+#define DW_IC_TX_ABRT_10ADDR2_NOACK (1UL << ABRT_10ADDR2_NOACK)
+#define DW_IC_TX_ABRT_TXDATA_NOACK (1UL << ABRT_TXDATA_NOACK)
+#define DW_IC_TX_ABRT_GCALL_NOACK (1UL << ABRT_GCALL_NOACK)
+#define DW_IC_TX_ABRT_GCALL_READ (1UL << ABRT_GCALL_READ)
+#define DW_IC_TX_ABRT_SBYTE_ACKDET (1UL << ABRT_SBYTE_ACKDET)
+#define DW_IC_TX_ABRT_SBYTE_NORSTRT (1UL << ABRT_SBYTE_NORSTRT)
+#define DW_IC_TX_ABRT_10B_RD_NORSTRT (1UL << ABRT_10B_RD_NORSTRT)
+#define DW_IC_TX_ABRT_MASTER_DIS (1UL << ABRT_MASTER_DIS)
+#define DW_IC_TX_ARB_LOST (1UL << ARB_LOST)
+
+#define DW_IC_TX_ABRT_NOACK (DW_IC_TX_ABRT_7B_ADDR_NOACK | \
+ DW_IC_TX_ABRT_10ADDR1_NOACK | \
+ DW_IC_TX_ABRT_10ADDR2_NOACK | \
+ DW_IC_TX_ABRT_TXDATA_NOACK | \
+ DW_IC_TX_ABRT_GCALL_NOACK)
+
/**
* struct dw_i2c_dev - private i2c-designware data
@@ -80,7 +198,10 @@
* @acquire_lock: function to acquire a hardware lock on the bus
* @release_lock: function to release a hardware lock on the bus
* @pm_disabled: true if power-management should be disabled for this i2c-bus
- *
+ * @disable: function to disable the controller
+ * @disable_int: function to disable all interrupts
+ * @init: function to initialize the I2C hardware
+ *
* 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.
* Leave them to be %0 if not used.
@@ -129,6 +250,9 @@ struct dw_i2c_dev {
int (*acquire_lock)(struct dw_i2c_dev *dev);
void (*release_lock)(struct dw_i2c_dev *dev);
bool pm_disabled;
+ void (*disable)(struct dw_i2c_dev *dev);
+ void (*disable_int)(struct dw_i2c_dev *dev);
+ int (*init)(struct dw_i2c_dev *dev);
};
#define ACCESS_SWAP 0x00000001
@@ -137,9 +261,22 @@ struct dw_i2c_dev {
#define MODEL_CHERRYTRAIL 0x00000100
-extern int i2c_dw_init(struct dw_i2c_dev *dev);
-extern void i2c_dw_disable(struct dw_i2c_dev *dev);
-extern void i2c_dw_disable_int(struct dw_i2c_dev *dev);
+u32 dw_readl(struct dw_i2c_dev *dev, int offset);
+void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset);
+u32 i2c_dw_scl_hcnt(u32 ic_clk, u32 tSYMBOL, u32 tf, int cond, int offset);
+u32 i2c_dw_scl_lcnt(u32 ic_clk, u32 tLOW, u32 tf, int offset);
+void __i2c_dw_enable(struct dw_i2c_dev *dev, bool enable);
+void __i2c_dw_enable_and_wait(struct dw_i2c_dev *dev, bool enable);
+unsigned long i2c_dw_clk_rate(struct dw_i2c_dev *dev);
+int i2c_dw_acquire_lock(struct dw_i2c_dev *dev);
+void i2c_dw_release_lock(struct dw_i2c_dev *dev);
+int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev);
+int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev);
+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);
+
extern u32 i2c_dw_read_comp_param(struct dw_i2c_dev *dev);
extern int i2c_dw_probe(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-master.c
similarity index 65%
rename from drivers/i2c/busses/i2c-designware-core.c
rename to drivers/i2c/busses/i2c-designware-master.c
index ce5d11102070..54d0d5954a6e 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -22,9 +22,9 @@
*
*/
#include <linux/delay.h>
-#include <linux/export.h>
-#include <linux/errno.h>
#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/export.h>
#include <linux/i2c.h>
#include <linux/interrupt.h>
#include <linux/io.h>
@@ -32,175 +32,6 @@
#include <linux/pm_runtime.h>
#include "i2c-designware-core.h"
-/*
- * Registers offset
- */
-#define DW_IC_CON 0x0
-#define DW_IC_TAR 0x4
-#define DW_IC_DATA_CMD 0x10
-#define DW_IC_SS_SCL_HCNT 0x14
-#define DW_IC_SS_SCL_LCNT 0x18
-#define DW_IC_FS_SCL_HCNT 0x1c
-#define DW_IC_FS_SCL_LCNT 0x20
-#define DW_IC_HS_SCL_HCNT 0x24
-#define DW_IC_HS_SCL_LCNT 0x28
-#define DW_IC_INTR_STAT 0x2c
-#define DW_IC_INTR_MASK 0x30
-#define DW_IC_RAW_INTR_STAT 0x34
-#define DW_IC_RX_TL 0x38
-#define DW_IC_TX_TL 0x3c
-#define DW_IC_CLR_INTR 0x40
-#define DW_IC_CLR_RX_UNDER 0x44
-#define DW_IC_CLR_RX_OVER 0x48
-#define DW_IC_CLR_TX_OVER 0x4c
-#define DW_IC_CLR_RD_REQ 0x50
-#define DW_IC_CLR_TX_ABRT 0x54
-#define DW_IC_CLR_RX_DONE 0x58
-#define DW_IC_CLR_ACTIVITY 0x5c
-#define DW_IC_CLR_STOP_DET 0x60
-#define DW_IC_CLR_START_DET 0x64
-#define DW_IC_CLR_GEN_CALL 0x68
-#define DW_IC_ENABLE 0x6c
-#define DW_IC_STATUS 0x70
-#define DW_IC_TXFLR 0x74
-#define DW_IC_RXFLR 0x78
-#define DW_IC_SDA_HOLD 0x7c
-#define DW_IC_TX_ABRT_SOURCE 0x80
-#define DW_IC_ENABLE_STATUS 0x9c
-#define DW_IC_COMP_PARAM_1 0xf4
-#define DW_IC_COMP_VERSION 0xf8
-#define DW_IC_SDA_HOLD_MIN_VERS 0x3131312A
-#define DW_IC_COMP_TYPE 0xfc
-#define DW_IC_COMP_TYPE_VALUE 0x44570140
-
-#define DW_IC_INTR_RX_UNDER 0x001
-#define DW_IC_INTR_RX_OVER 0x002
-#define DW_IC_INTR_RX_FULL 0x004
-#define DW_IC_INTR_TX_OVER 0x008
-#define DW_IC_INTR_TX_EMPTY 0x010
-#define DW_IC_INTR_RD_REQ 0x020
-#define DW_IC_INTR_TX_ABRT 0x040
-#define DW_IC_INTR_RX_DONE 0x080
-#define DW_IC_INTR_ACTIVITY 0x100
-#define DW_IC_INTR_STOP_DET 0x200
-#define DW_IC_INTR_START_DET 0x400
-#define DW_IC_INTR_GEN_CALL 0x800
-
-#define DW_IC_INTR_DEFAULT_MASK (DW_IC_INTR_RX_FULL | \
- DW_IC_INTR_TX_ABRT | \
- DW_IC_INTR_STOP_DET)
-#define DW_IC_INTR_MASTER_MASK (DW_IC_INTR_DEFAULT_MASK | \
- DW_IC_INTR_TX_EMPTY)
-#define DW_IC_STATUS_ACTIVITY 0x1
-
-#define DW_IC_SDA_HOLD_RX_SHIFT 16
-#define DW_IC_SDA_HOLD_RX_MASK GENMASK(23, DW_IC_SDA_HOLD_RX_SHIFT)
-
-#define DW_IC_ERR_TX_ABRT 0x1
-
-#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)
-
-/*
- * status codes
- */
-#define STATUS_IDLE 0x0
-#define STATUS_WRITE_IN_PROGRESS 0x1
-#define STATUS_READ_IN_PROGRESS 0x2
-
-#define TIMEOUT 20 /* ms */
-
-/*
- * 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.
- */
-#define ABRT_7B_ADDR_NOACK 0
-#define ABRT_10ADDR1_NOACK 1
-#define ABRT_10ADDR2_NOACK 2
-#define ABRT_TXDATA_NOACK 3
-#define ABRT_GCALL_NOACK 4
-#define ABRT_GCALL_READ 5
-#define ABRT_SBYTE_ACKDET 7
-#define ABRT_SBYTE_NORSTRT 9
-#define ABRT_10B_RD_NORSTRT 10
-#define ABRT_MASTER_DIS 11
-#define ARB_LOST 12
-
-#define DW_IC_TX_ABRT_7B_ADDR_NOACK (1UL << ABRT_7B_ADDR_NOACK)
-#define DW_IC_TX_ABRT_10ADDR1_NOACK (1UL << ABRT_10ADDR1_NOACK)
-#define DW_IC_TX_ABRT_10ADDR2_NOACK (1UL << ABRT_10ADDR2_NOACK)
-#define DW_IC_TX_ABRT_TXDATA_NOACK (1UL << ABRT_TXDATA_NOACK)
-#define DW_IC_TX_ABRT_GCALL_NOACK (1UL << ABRT_GCALL_NOACK)
-#define DW_IC_TX_ABRT_GCALL_READ (1UL << ABRT_GCALL_READ)
-#define DW_IC_TX_ABRT_SBYTE_ACKDET (1UL << ABRT_SBYTE_ACKDET)
-#define DW_IC_TX_ABRT_SBYTE_NORSTRT (1UL << ABRT_SBYTE_NORSTRT)
-#define DW_IC_TX_ABRT_10B_RD_NORSTRT (1UL << ABRT_10B_RD_NORSTRT)
-#define DW_IC_TX_ABRT_MASTER_DIS (1UL << ABRT_MASTER_DIS)
-#define DW_IC_TX_ARB_LOST (1UL << ARB_LOST)
-
-#define DW_IC_TX_ABRT_NOACK (DW_IC_TX_ABRT_7B_ADDR_NOACK | \
- DW_IC_TX_ABRT_10ADDR1_NOACK | \
- DW_IC_TX_ABRT_10ADDR2_NOACK | \
- DW_IC_TX_ABRT_TXDATA_NOACK | \
- DW_IC_TX_ABRT_GCALL_NOACK)
-
-static char *abort_sources[] = {
- [ABRT_7B_ADDR_NOACK] =
- "slave address not acknowledged (7bit mode)",
- [ABRT_10ADDR1_NOACK] =
- "first address byte not acknowledged (10bit mode)",
- [ABRT_10ADDR2_NOACK] =
- "second address byte not acknowledged (10bit mode)",
- [ABRT_TXDATA_NOACK] =
- "data not acknowledged",
- [ABRT_GCALL_NOACK] =
- "no acknowledgement for a general call",
- [ABRT_GCALL_READ] =
- "read after general call",
- [ABRT_SBYTE_ACKDET] =
- "start byte acknowledged",
- [ABRT_SBYTE_NORSTRT] =
- "trying to send start byte when restart is disabled",
- [ABRT_10B_RD_NORSTRT] =
- "trying to read when restart is disabled (10bit mode)",
- [ABRT_MASTER_DIS] =
- "trying to use disabled adapter",
- [ARB_LOST] =
- "lost arbitration",
-};
-
-static u32 dw_readl(struct dw_i2c_dev *dev, int offset)
-{
- u32 value;
-
- if (dev->flags & ACCESS_16BIT)
- value = readw_relaxed(dev->base + offset) |
- (readw_relaxed(dev->base + offset + 2) << 16);
- else
- value = readl_relaxed(dev->base + offset);
-
- if (dev->flags & ACCESS_SWAP)
- return swab32(value);
- else
- return value;
-}
-
-static void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset)
-{
- if (dev->flags & ACCESS_SWAP)
- b = swab32(b);
-
- if (dev->flags & ACCESS_16BIT) {
- writew_relaxed((u16)b, dev->base + offset);
- writew_relaxed((u16)(b >> 16), dev->base + offset + 2);
- } else {
- writel_relaxed(b, dev->base + offset);
- }
-}
static void i2c_dw_configure_fifo_master(struct dw_i2c_dev *dev)
{
@@ -212,121 +43,6 @@ static void i2c_dw_configure_fifo_master(struct dw_i2c_dev *dev)
dw_writel(dev, dev->master_cfg, DW_IC_CON);
}
-static u32
-i2c_dw_scl_hcnt(u32 ic_clk, u32 tSYMBOL, u32 tf, int cond, int offset)
-{
- /*
- * DesignWare I2C core doesn't seem to have solid strategy to meet
- * the tHD;STA timing spec. Configuring _HCNT based on tHIGH spec
- * will result in violation of the tHD;STA spec.
- */
- if (cond)
- /*
- * Conditional expression:
- *
- * IC_[FS]S_SCL_HCNT + (1+4+3) >= IC_CLK * tHIGH
- *
- * This is based on the DW manuals, and represents an ideal
- * configuration. The resulting I2C bus speed will be
- * faster than any of the others.
- *
- * If your hardware is free from tHD;STA issue, try this one.
- */
- return (ic_clk * tSYMBOL + 500000) / 1000000 - 8 + offset;
- else
- /*
- * Conditional expression:
- *
- * IC_[FS]S_SCL_HCNT + 3 >= IC_CLK * (tHD;STA + tf)
- *
- * This is just experimental rule; the tHD;STA period turned
- * out to be proportinal to (_HCNT + 3). With this setting,
- * we could meet both tHIGH and tHD;STA timing specs.
- *
- * If unsure, you'd better to take this alternative.
- *
- * The reason why we need to take into account "tf" here,
- * is the same as described in i2c_dw_scl_lcnt().
- */
- return (ic_clk * (tSYMBOL + tf) + 500000) / 1000000
- - 3 + offset;
-}
-
-static u32 i2c_dw_scl_lcnt(u32 ic_clk, u32 tLOW, u32 tf, int offset)
-{
- /*
- * Conditional expression:
- *
- * IC_[FS]S_SCL_LCNT + 1 >= IC_CLK * (tLOW + tf)
- *
- * DW I2C core starts counting the SCL CNTs for the LOW period
- * of the SCL clock (tLOW) as soon as it pulls the SCL line.
- * In order to meet the tLOW timing spec, we need to take into
- * account the fall time of SCL signal (tf). Default tf value
- * should be 0.3 us, for safety.
- */
- return ((ic_clk * (tLOW + tf) + 500000) / 1000000) - 1 + offset;
-}
-
-static void __i2c_dw_enable(struct dw_i2c_dev *dev, bool enable)
-{
- dw_writel(dev, enable, DW_IC_ENABLE);
-}
-
-static void __i2c_dw_enable_and_wait(struct dw_i2c_dev *dev, bool enable)
-{
- int timeout = 100;
-
- do {
- __i2c_dw_enable(dev, enable);
- if ((dw_readl(dev, DW_IC_ENABLE_STATUS) & 1) == enable)
- return;
-
- /*
- * Wait 10 times the signaling period of the highest I2C
- * transfer supported by the driver (for 400KHz this is
- * 25us) as described in the DesignWare I2C databook.
- */
- usleep_range(25, 250);
- } while (timeout--);
-
- dev_warn(dev->dev, "timeout in %sabling adapter\n",
- enable ? "en" : "dis");
-}
-
-static unsigned long i2c_dw_clk_rate(struct dw_i2c_dev *dev)
-{
- /*
- * Clock is not necessary if we got LCNT/HCNT values directly from
- * the platform code.
- */
- if (WARN_ON_ONCE(!dev->get_clk_rate_khz))
- return 0;
- return dev->get_clk_rate_khz(dev);
-}
-
-static int i2c_dw_acquire_lock(struct dw_i2c_dev *dev)
-{
- int ret;
-
- if (!dev->acquire_lock)
- return 0;
-
- ret = dev->acquire_lock(dev);
- if (!ret)
- return 0;
-
- dev_err(dev->dev, "couldn't acquire bus ownership\n");
-
- return ret;
-}
-
-static void i2c_dw_release_lock(struct dw_i2c_dev *dev)
-{
- if (dev->release_lock)
- dev->release_lock(dev);
-}
-
/**
* i2c_dw_init() - Initialize the designware I2C master hardware
* @dev: device private data
@@ -457,25 +173,6 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
}
EXPORT_SYMBOL_GPL(i2c_dw_init);
-/*
- * Waiting for bus not busy
- */
-static int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev)
-{
- int timeout = TIMEOUT;
-
- while (dw_readl(dev, DW_IC_STATUS) & DW_IC_STATUS_ACTIVITY) {
- if (timeout <= 0) {
- dev_warn(dev->dev, "timeout waiting for bus ready\n");
- return -ETIMEDOUT;
- }
- timeout--;
- usleep_range(1000, 1100);
- }
-
- return 0;
-}
-
static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
{
struct i2c_msg *msgs = dev->msgs;
@@ -708,29 +405,6 @@ i2c_dw_read(struct dw_i2c_dev *dev)
}
}
-static int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
-{
- unsigned long abort_source = dev->abort_source;
- int i;
-
- if (abort_source & DW_IC_TX_ABRT_NOACK) {
- for_each_set_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
- dev_dbg(dev->dev,
- "%s: %s\n", __func__, abort_sources[i]);
- return -EREMOTEIO;
- }
-
- for_each_set_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
- dev_err(dev->dev, "%s: %s\n", __func__, abort_sources[i]);
-
- if (abort_source & DW_IC_TX_ARB_LOST)
- return -EAGAIN;
- else if (abort_source & DW_IC_TX_ABRT_GCALL_READ)
- return -EINVAL; /* wrong msgs[] data */
- else
- return -EIO;
-}
-
/*
* Prepare controller for a transaction and call i2c_dw_xfer_msg.
*/
@@ -818,12 +492,6 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
return ret;
}
-static u32 i2c_dw_func(struct i2c_adapter *adap)
-{
- struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
- return dev->functionality;
-}
-
static const struct i2c_algorithm i2c_dw_algo = {
.master_xfer = i2c_dw_xfer,
.functionality = i2c_dw_func,
@@ -946,29 +614,6 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
return IRQ_HANDLED;
}
-void i2c_dw_disable(struct dw_i2c_dev *dev)
-{
- /* Disable controller */
- __i2c_dw_enable_and_wait(dev, false);
-
- /* Disable all interupts */
- dw_writel(dev, 0, DW_IC_INTR_MASK);
- dw_readl(dev, DW_IC_CLR_INTR);
-}
-EXPORT_SYMBOL_GPL(i2c_dw_disable);
-
-void i2c_dw_disable_int(struct dw_i2c_dev *dev)
-{
- dw_writel(dev, 0, DW_IC_INTR_MASK);
-}
-EXPORT_SYMBOL_GPL(i2c_dw_disable_int);
-
-u32 i2c_dw_read_comp_param(struct dw_i2c_dev *dev)
-{
- return dw_readl(dev, DW_IC_COMP_PARAM_1);
-}
-EXPORT_SYMBOL_GPL(i2c_dw_read_comp_param);
-
int i2c_dw_probe(struct dw_i2c_dev *dev)
{
struct i2c_adapter *adap = &dev->adapter;
@@ -977,7 +622,11 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
init_completion(&dev->cmd_complete);
- ret = i2c_dw_init(dev);
+ dev->init = i2c_dw_init;
+ dev->disable = i2c_dw_disable;
+ dev->disable_int = i2c_dw_disable_int;
+
+ ret = dev->init(dev);
if (ret)
return ret;
@@ -1020,5 +669,5 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
}
EXPORT_SYMBOL_GPL(i2c_dw_probe);
-MODULE_DESCRIPTION("Synopsys DesignWare I2C bus adapter core");
+MODULE_DESCRIPTION("Synopsys DesignWare I2C bus master adapter");
MODULE_LICENSE("GPL");
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index ed485b69b449..b582a5f2fb94 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -187,16 +187,18 @@ static struct dw_pci_controller dw_pci_controllers[] = {
static int i2c_dw_pci_suspend(struct device *dev)
{
struct pci_dev *pdev = to_pci_dev(dev);
+ struct dw_i2c_dev *i_dev = pci_get_drvdata(pdev);
+ i_dev->disable(i_dev);
- i2c_dw_disable(pci_get_drvdata(pdev));
return 0;
}
static int i2c_dw_pci_resume(struct device *dev)
{
struct pci_dev *pdev = to_pci_dev(dev);
+ struct dw_i2c_dev *i_dev = pci_get_drvdata(pdev);
- return i2c_dw_init(pci_get_drvdata(pdev));
+ return i_dev->init(i_dev);
}
#endif
@@ -296,7 +298,7 @@ static void i2c_dw_pci_remove(struct pci_dev *pdev)
{
struct dw_i2c_dev *dev = pci_get_drvdata(pdev);
- i2c_dw_disable(dev);
+ dev->disable(dev);
pm_runtime_forbid(&pdev->dev);
pm_runtime_get_noresume(&pdev->dev);
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 06205bd39d7e..ab7a4314a91a 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -352,7 +352,7 @@ static int dw_i2c_plat_remove(struct platform_device *pdev)
i2c_del_adapter(&dev->adapter);
- i2c_dw_disable(dev);
+ dev->disable(dev);
pm_runtime_dont_use_autosuspend(&pdev->dev);
pm_runtime_put_sync(&pdev->dev);
@@ -396,7 +396,7 @@ static int dw_i2c_plat_suspend(struct device *dev)
struct platform_device *pdev = to_platform_device(dev);
struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
- i2c_dw_disable(i_dev);
+ i_dev->disable(i_dev);
i2c_dw_plat_prepare_clk(i_dev, false);
return 0;
@@ -408,7 +408,7 @@ static int dw_i2c_plat_resume(struct device *dev)
struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
i2c_dw_plat_prepare_clk(i_dev, true);
- i2c_dw_init(i_dev);
+ i_dev->init(i_dev);
return 0;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v9 4/6] i2c: designware: introducing I2C_SLAVE definitions
2017-05-08 10:37 [PATCH v9 0/6] i2c: designware: add I2C SLAVE support Luis Oliveira
` (2 preceding siblings ...)
2017-05-08 10:37 ` [PATCH v9 3/6] i2c: designware: MASTER mode as separated driver Luis Oliveira
@ 2017-05-08 10:37 ` Luis Oliveira
2017-05-08 10:37 ` [PATCH v9 5/6] i2c: designware: add SLAVE mode functions Luis Oliveira
2017-05-08 10:37 ` [PATCH v9 6/6] i2c: designware: enable SLAVE in platform module Luis Oliveira
5 siblings, 0 replies; 16+ messages in thread
From: Luis Oliveira @ 2017-05-08 10:37 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, jarkko.nikula, andriy.shevchenko,
mika.westerberg, linux-i2c, devicetree, linux-kernel
Cc: Luis.Oliveira, Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
- Definitions were added to core library
- A example was added to designware-core.txt Documentation that shows
how the slave can be setup using DTS
SLAVE related definitions were added to the core of the controller.
Signed-off-by: Luis Oliveira <lolivei@synopsys.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
---
v8-v9
- no changes
.../devicetree/bindings/i2c/i2c-designware.txt | 16 ++++++++++-
drivers/i2c/busses/i2c-designware-core.h | 33 ++++++++++++++++++++--
2 files changed, 46 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-designware.txt b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
index fee26dc3e858..fbb0a6d8b964 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-designware.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
@@ -20,7 +20,7 @@ Optional properties :
- i2c-sda-falling-time-ns : should contain the SDA falling time in nanoseconds.
This value which is by default 300ns is used to compute the tHIGH period.
-Example :
+Examples :
i2c@f0000 {
#address-cells = <1>;
@@ -43,3 +43,17 @@ Example :
i2c-sda-falling-time-ns = <300>;
i2c-scl-falling-time-ns = <300>;
};
+
+ i2c@1120000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x2000 0x100>;
+ clock-frequency = <400000>;
+ clocks = <&i2cclk>;
+ interrupts = <0>;
+
+ eeprom@64 {
+ compatible = "linux,slave-24c02";
+ reg = <0x40000064>;
+ };
+ };
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 81b46c8529fa..dc20ef079fc1 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -1,5 +1,5 @@
/*
- * Synopsys DesignWare I2C adapter driver (master only).
+ * Synopsys DesignWare I2C adapter driver.
*
* Based on the TI DAVINCI I2C adapter driver.
*
@@ -37,15 +37,20 @@
#define DW_IC_CON_SPEED_FAST 0x4
#define DW_IC_CON_SPEED_HIGH 0x6
#define DW_IC_CON_SPEED_MASK 0x6
+#define DW_IC_CON_10BITADDR_SLAVE 0x8
#define DW_IC_CON_10BITADDR_MASTER 0x10
#define DW_IC_CON_RESTART_EN 0x20
#define DW_IC_CON_SLAVE_DISABLE 0x40
+#define DW_IC_CON_STOP_DET_IFADDRESSED 0x80
+#define DW_IC_CON_TX_EMPTY_CTRL 0x100
+#define DW_IC_CON_RX_FIFO_FULL_HLD_CTRL 0x200
/*
* Registers offset
*/
#define DW_IC_CON 0x0
#define DW_IC_TAR 0x4
+#define DW_IC_SAR 0x8
#define DW_IC_DATA_CMD 0x10
#define DW_IC_SS_SCL_HCNT 0x14
#define DW_IC_SS_SCL_LCNT 0x18
@@ -76,6 +81,7 @@
#define DW_IC_SDA_HOLD 0x7c
#define DW_IC_TX_ABRT_SOURCE 0x80
#define DW_IC_ENABLE_STATUS 0x9c
+#define DW_IC_CLR_RESTART_DET 0xa8
#define DW_IC_COMP_PARAM_1 0xf4
#define DW_IC_COMP_VERSION 0xf8
#define DW_IC_SDA_HOLD_MIN_VERS 0x3131312A
@@ -94,15 +100,22 @@
#define DW_IC_INTR_STOP_DET 0x200
#define DW_IC_INTR_START_DET 0x400
#define DW_IC_INTR_GEN_CALL 0x800
+#define DW_IC_INTR_RESTART_DET 0x1000
#define DW_IC_INTR_DEFAULT_MASK (DW_IC_INTR_RX_FULL | \
DW_IC_INTR_TX_ABRT | \
DW_IC_INTR_STOP_DET)
#define DW_IC_INTR_MASTER_MASK (DW_IC_INTR_DEFAULT_MASK | \
DW_IC_INTR_TX_EMPTY)
+#define DW_IC_INTR_SLAVE_MASK (DW_IC_INTR_DEFAULT_MASK | \
+ DW_IC_INTR_RX_DONE | \
+ DW_IC_INTR_RX_UNDER | \
+ DW_IC_INTR_RD_REQ)
+
#define DW_IC_STATUS_ACTIVITY 0x1
#define DW_IC_STATUS_TFE BIT(2)
#define DW_IC_STATUS_MASTER_ACTIVITY BIT(5)
+#define DW_IC_STATUS_SLAVE_ACTIVITY BIT(6)
#define DW_IC_SDA_HOLD_RX_SHIFT 16
#define DW_IC_SDA_HOLD_RX_MASK GENMASK(23, DW_IC_SDA_HOLD_RX_SHIFT)
@@ -115,7 +128,7 @@
#define DW_IC_COMP_PARAM_1_SPEED_MODE_MASK GENMASK(3, 2)
/*
- * Status codes
+ * status codes
*/
#define STATUS_IDLE 0x0
#define STATUS_WRITE_IN_PROGRESS 0x1
@@ -124,6 +137,12 @@
#define TIMEOUT 20 /* ms */
/*
+ * operation modes
+ */
+#define DW_IC_MASTER 0
+#define DW_IC_SLAVE 1
+
+/*
* Hardware abort codes from the DW_IC_TX_ABRT_SOURCE register
*
* Only expected abort codes are listed here
@@ -140,6 +159,9 @@
#define ABRT_10B_RD_NORSTRT 10
#define ABRT_MASTER_DIS 11
#define ARB_LOST 12
+#define ABRT_SLAVE_FLUSH_TXFIFO 13
+#define ABRT_SLAVE_ARBLOST 14
+#define ABRT_SLAVE_RD_INTX 15
#define DW_IC_TX_ABRT_7B_ADDR_NOACK (1UL << ABRT_7B_ADDR_NOACK)
#define DW_IC_TX_ABRT_10ADDR1_NOACK (1UL << ABRT_10ADDR1_NOACK)
@@ -152,6 +174,9 @@
#define DW_IC_TX_ABRT_10B_RD_NORSTRT (1UL << ABRT_10B_RD_NORSTRT)
#define DW_IC_TX_ABRT_MASTER_DIS (1UL << ABRT_MASTER_DIS)
#define DW_IC_TX_ARB_LOST (1UL << ARB_LOST)
+#define DW_IC_RX_ABRT_SLAVE_RD_INTX (1UL << ABRT_SLAVE_RD_INTX)
+#define DW_IC_RX_ABRT_SLAVE_ARBLOST (1UL << ABRT_SLAVE_ARBLOST)
+#define DW_IC_RX_ABRT_SLAVE_FLUSH_TXFIFO (1UL << ABRT_SLAVE_FLUSH_TXFIFO)
#define DW_IC_TX_ABRT_NOACK (DW_IC_TX_ABRT_7B_ADDR_NOACK | \
DW_IC_TX_ABRT_10ADDR1_NOACK | \
@@ -166,6 +191,7 @@
* @base: IO registers pointer
* @cmd_complete: tx completion indicator
* @clk: input reference clock
+ * @slave: represent an I2C slave device
* @cmd_err: run time hadware error code
* @msgs: points to an array of messages currently being transferred
* @msgs_num: the number of elements in msgs
@@ -182,6 +208,7 @@
* @abort_source: copy of the TX_ABRT_SOURCE register
* @irq: interrupt number for the i2c master
* @adapter: i2c subsystem adapter node
+ * @slave_cfg: configuration for the slave device
* @tx_fifo_depth: depth of the hardware tx fifo
* @rx_fifo_depth: depth of the hardware rx fifo
* @rx_outstanding: current master-rx elements in tx fifo
@@ -212,6 +239,7 @@ struct dw_i2c_dev {
struct completion cmd_complete;
struct clk *clk;
struct reset_control *rst;
+ struct i2c_client *slave;
u32 (*get_clk_rate_khz) (struct dw_i2c_dev *dev);
struct dw_pci_controller *controller;
int cmd_err;
@@ -231,6 +259,7 @@ struct dw_i2c_dev {
struct i2c_adapter adapter;
u32 functionality;
u32 master_cfg;
+ u32 slave_cfg;
unsigned int tx_fifo_depth;
unsigned int rx_fifo_depth;
int rx_outstanding;
--
2.11.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v9 5/6] i2c: designware: add SLAVE mode functions
2017-05-08 10:37 [PATCH v9 0/6] i2c: designware: add I2C SLAVE support Luis Oliveira
` (3 preceding siblings ...)
2017-05-08 10:37 ` [PATCH v9 4/6] i2c: designware: introducing I2C_SLAVE definitions Luis Oliveira
@ 2017-05-08 10:37 ` Luis Oliveira
2017-05-08 16:53 ` Andy Shevchenko
2017-05-08 10:37 ` [PATCH v9 6/6] i2c: designware: enable SLAVE in platform module Luis Oliveira
5 siblings, 1 reply; 16+ messages in thread
From: Luis Oliveira @ 2017-05-08 10:37 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, jarkko.nikula, andriy.shevchenko,
mika.westerberg, linux-i2c, devicetree, linux-kernel
Cc: Luis.Oliveira, Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
- 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 <lolivei@synopsys.com>
---
v8-v9
- removed I2C_SLAVE_EEPROM selection
drivers/i2c/busses/Kconfig | 13 +-
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-designware-common.c | 6 +
drivers/i2c/busses/i2c-designware-core.h | 2 +
drivers/i2c/busses/i2c-designware-slave.c | 403 +++++++++++++++++++++++++++++
5 files changed, 424 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..88e5062015f8 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -474,11 +474,22 @@ config I2C_DESIGNWARE_PLATFORM
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..678cf6f4a552 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -41,6 +41,7 @@ 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
+i2c-designware-core-$(CONFIG_I2C_DESIGNWARE_SLAVE) += i2c-designware-slave.o
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 98b64e4f5977..fcd9d6bb8897 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 dc20ef079fc1..8c6fc8384b1e 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..3ed4d4161e00
--- /dev/null
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -0,0 +1,403 @@
+/*
+ * 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 functions configures and enables the I2C.
+ * 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 SLAVE_ACTV=%#x : RAW_INTR_STAT=%#x : INTR_STAT=%#x\n",
+ enabled, slave_activity, raw_stat, stat);
+
+ if (stat & DW_IC_INTR_RESTART_DET)
+ dw_readl(dev, DW_IC_CLR_RESTART_DET);
+ if (stat & DW_IC_INTR_START_DET)
+ dw_readl(dev, DW_IC_CLR_START_DET);
+ if (stat & DW_IC_INTR_ACTIVITY)
+ dw_readl(dev, DW_IC_CLR_ACTIVITY);
+ if (stat & DW_IC_INTR_RX_OVER)
+ dw_readl(dev, DW_IC_CLR_RX_OVER);
+ if ((stat & DW_IC_INTR_RX_FULL) && (stat & DW_IC_INTR_STOP_DET))
+ i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED, &val);
+
+ if (slave_activity) {
+ if (stat & DW_IC_INTR_RD_REQ) {
+ 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 <lolivei@synopsys.com>");
+MODULE_DESCRIPTION("Synopsys DesignWare I2C bus slave adapter");
+MODULE_LICENSE("GPL v2");
--
2.11.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v9 5/6] i2c: designware: add SLAVE mode functions
@ 2017-05-08 16:53 ` Andy Shevchenko
0 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2017-05-08 16:53 UTC (permalink / raw)
To: Luis Oliveira, wsa, robh+dt, mark.rutland, jarkko.nikula,
mika.westerberg, linux-i2c, devicetree, linux-kernel
Cc: Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
On Mon, 2017-05-08 at 11:37 +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.
My comments below.
Overall entire series looks much much better than first version.
> @@ -41,6 +41,7 @@ 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
> +i2c-designware-core-$(CONFIG_I2C_DESIGNWARE_SLAVE) += i2c-designware-
> slave.o
What is your intention here?
AFAIUC it should be like
ifneq ($(CONFIG_I2C_DESIGNWARE_SLAVE),n)
i2c-designware-core-objs += i2c-designware-slave.o
endif
> +/**
> + * i2c_dw_init_slave() - Initialize the designware i2c slave hardware
> + * @dev: device private data
> + *
> + * This functions configures and enables the I2C.
functions -> function
I2C -> 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)) {
GENMASK(15, 0)
> + /* 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;
> + }
> +/*
> + * 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 SLAVE_ACTV=%#x : RAW_INTR_STAT=%#x :
> INTR_STAT=%#x\n",
SLAVE_ACTV -> STATUS_SLAVE_ACTIVITY
> + enabled, slave_activity, raw_stat, stat);
> +
> + if (slave_activity) {
> + if (stat & DW_IC_INTR_RD_REQ) {
Looking into next condition (stat & DW_IC_INTR_RX_DONE) I would
exchange these lines
> + 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);
> +
I would remove this line.
> + if (ret > 0)
> + complete(&dev->cmd_complete);
> +
> + return IRQ_RETVAL(ret);
> +}
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v9 5/6] i2c: designware: add SLAVE mode functions
@ 2017-05-08 16:53 ` Andy Shevchenko
0 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2017-05-08 16:53 UTC (permalink / raw)
To: Luis Oliveira, wsa-z923LK4zBo2bacvFa/9K2g,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
jarkko.nikula-VuQAYsv1563Yd54FQh9/CA,
mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Ramiro.Oliveira-HKixBCOQz3hWk0Htik3J/w,
Joao.Pinto-HKixBCOQz3hWk0Htik3J/w,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
On Mon, 2017-05-08 at 11:37 +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.
My comments below.
Overall entire series looks much much better than first version.
> @@ -41,6 +41,7 @@ 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
> +i2c-designware-core-$(CONFIG_I2C_DESIGNWARE_SLAVE) += i2c-designware-
> slave.o
What is your intention here?
AFAIUC it should be like
ifneq ($(CONFIG_I2C_DESIGNWARE_SLAVE),n)
i2c-designware-core-objs += i2c-designware-slave.o
endif
> +/**
> + * i2c_dw_init_slave() - Initialize the designware i2c slave hardware
> + * @dev: device private data
> + *
> + * This functions configures and enables the I2C.
functions -> function
I2C -> 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)) {
GENMASK(15, 0)
> + /* 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;
> + }
> +/*
> + * 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 SLAVE_ACTV=%#x : RAW_INTR_STAT=%#x :
> INTR_STAT=%#x\n",
SLAVE_ACTV -> STATUS_SLAVE_ACTIVITY
> + enabled, slave_activity, raw_stat, stat);
> +
> + if (slave_activity) {
> + if (stat & DW_IC_INTR_RD_REQ) {
Looking into next condition (stat & DW_IC_INTR_RX_DONE) I would
exchange these lines
> + 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);
> +
I would remove this line.
> + if (ret > 0)
> + complete(&dev->cmd_complete);
> +
> + return IRQ_RETVAL(ret);
> +}
--
Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v9 5/6] i2c: designware: add SLAVE mode functions
@ 2017-05-11 13:20 ` Jarkko Nikula
0 siblings, 0 replies; 16+ messages in thread
From: Jarkko Nikula @ 2017-05-11 13:20 UTC (permalink / raw)
To: Andy Shevchenko, Luis Oliveira, wsa, robh+dt, mark.rutland,
mika.westerberg, linux-i2c, devicetree, linux-kernel
Cc: Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
On 05/08/2017 07:53 PM, Andy Shevchenko wrote:
>> + } else if (reg == (DW_IC_COMP_TYPE_VALUE & 0x0000ffff)) {
>
> GENMASK(15, 0)
>
I don't think this increase readability compared to 0x0000ffff here.
--
Jarkko
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v9 5/6] i2c: designware: add SLAVE mode functions
@ 2017-05-11 13:20 ` Jarkko Nikula
0 siblings, 0 replies; 16+ messages in thread
From: Jarkko Nikula @ 2017-05-11 13:20 UTC (permalink / raw)
To: Andy Shevchenko, Luis Oliveira, wsa-z923LK4zBo2bacvFa/9K2g,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Ramiro.Oliveira-HKixBCOQz3hWk0Htik3J/w,
Joao.Pinto-HKixBCOQz3hWk0Htik3J/w,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
On 05/08/2017 07:53 PM, Andy Shevchenko wrote:
>> + } else if (reg == (DW_IC_COMP_TYPE_VALUE & 0x0000ffff)) {
>
> GENMASK(15, 0)
>
I don't think this increase readability compared to 0x0000ffff here.
--
Jarkko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v9 5/6] i2c: designware: add SLAVE mode functions
2017-05-11 13:20 ` Jarkko Nikula
(?)
@ 2017-05-11 13:32 ` Andy Shevchenko
-1 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2017-05-11 13:32 UTC (permalink / raw)
To: Jarkko Nikula, Luis Oliveira, wsa, robh+dt, mark.rutland,
mika.westerberg, linux-i2c, devicetree, linux-kernel
Cc: Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
On Thu, 2017-05-11 at 16:20 +0300, Jarkko Nikula wrote:
> On 05/08/2017 07:53 PM, Andy Shevchenko wrote:
> > > + } else if (reg == (DW_IC_COMP_TYPE_VALUE & 0x0000ffff)) {
> >
> > GENMASK(15, 0)
> >
>
> I don't think this increase readability compared to 0x0000ffff here.
If it's a defined constant it might do, otherwise I'm fine with explicit
value.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v9 5/6] i2c: designware: add SLAVE mode functions
2017-05-08 16:53 ` Andy Shevchenko
@ 2017-05-22 14:32 ` Luis Oliveira
-1 siblings, 0 replies; 16+ messages in thread
From: Luis Oliveira @ 2017-05-22 14:32 UTC (permalink / raw)
To: Andy Shevchenko, Luis Oliveira, wsa, robh+dt, mark.rutland,
jarkko.nikula, mika.westerberg, linux-i2c, devicetree,
linux-kernel
Cc: Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
On 08-May-17 17:53, Andy Shevchenko wrote:
> On Mon, 2017-05-08 at 11:37 +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.
>
> My comments below.
> Overall entire series looks much much better than first version.
>
>> @@ -41,6 +41,7 @@ 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
>
>> +i2c-designware-core-$(CONFIG_I2C_DESIGNWARE_SLAVE) += i2c-designware-
>> slave.o
>
> What is your intention here?
>
> AFAIUC it should be like
>
> ifneq ($(CONFIG_I2C_DESIGNWARE_SLAVE),n)
> i2c-designware-core-objs += i2c-designware-slave.o
> endif
>
Ok, I will change it in the next patchset.
>> +/**
>> + * i2c_dw_init_slave() - Initialize the designware i2c slave hardware
>> + * @dev: device private data
>> + *
>> + * This functions configures and enables the I2C.
>
> functions -> function
> I2C -> I2C in slave mode
>
Yes.
>> + * 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)) {
>
> GENMASK(15, 0)
>
I think I will leave it as it is. As Jarkko said it has more readability this way.
>> + /* 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;
>> + }
>
>> +/*
>> + * 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 SLAVE_ACTV=%#x : RAW_INTR_STAT=%#x :
>> INTR_STAT=%#x\n",
>
> SLAVE_ACTV -> STATUS_SLAVE_ACTIVITY
Ok.
>
>> + enabled, slave_activity, raw_stat, stat);
>> +
>
>> + if (slave_activity) {
>> + if (stat & DW_IC_INTR_RD_REQ) {
>
> Looking into next condition (stat & DW_IC_INTR_RX_DONE) I would
> exchange these lines
>
By order? I don't think I understood your point. Please elaborate a little more.
>> + 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);
>
>> +
>
> I would remove this line.
Ok. Thank you.
>
>> + if (ret > 0)
>> + complete(&dev->cmd_complete);
>> +
>> + return IRQ_RETVAL(ret);
>> +}
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v9 5/6] i2c: designware: add SLAVE mode functions
@ 2017-05-22 14:32 ` Luis Oliveira
0 siblings, 0 replies; 16+ messages in thread
From: Luis Oliveira @ 2017-05-22 14:32 UTC (permalink / raw)
To: Andy Shevchenko, Luis Oliveira, wsa, robh+dt, mark.rutland,
jarkko.nikula, mika.westerberg, linux-i2c, devicetree,
linux-kernel
Cc: Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
On 08-May-17 17:53, Andy Shevchenko wrote:
> On Mon, 2017-05-08 at 11:37 +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.
>
> My comments below.
> Overall entire series looks much much better than first version.
>
>> @@ -41,6 +41,7 @@ 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
>
>> +i2c-designware-core-$(CONFIG_I2C_DESIGNWARE_SLAVE) += i2c-designware-
>> slave.o
>
> What is your intention here?
>
> AFAIUC it should be like
>
> ifneq ($(CONFIG_I2C_DESIGNWARE_SLAVE),n)
> i2c-designware-core-objs += i2c-designware-slave.o
> endif
>
Ok, I will change it in the next patchset.
>> +/**
>> + * i2c_dw_init_slave() - Initialize the designware i2c slave hardware
>> + * @dev: device private data
>> + *
>> + * This functions configures and enables the I2C.
>
> functions -> function
> I2C -> I2C in slave mode
>
Yes.
>> + * 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)) {
>
> GENMASK(15, 0)
>
I think I will leave it as it is. As Jarkko said it has more readability this way.
>> + /* 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;
>> + }
>
>> +/*
>> + * 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 SLAVE_ACTV=%#x : RAW_INTR_STAT=%#x :
>> INTR_STAT=%#x\n",
>
> SLAVE_ACTV -> STATUS_SLAVE_ACTIVITY
Ok.
>
>> + enabled, slave_activity, raw_stat, stat);
>> +
>
>> + if (slave_activity) {
>> + if (stat & DW_IC_INTR_RD_REQ) {
>
> Looking into next condition (stat & DW_IC_INTR_RX_DONE) I would
> exchange these lines
>
By order? I don't think I understood your point. Please elaborate a little more.
>> + 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);
>
>> +
>
> I would remove this line.
Ok. Thank you.
>
>> + if (ret > 0)
>> + complete(&dev->cmd_complete);
>> +
>> + return IRQ_RETVAL(ret);
>> +}
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v9 5/6] i2c: designware: add SLAVE mode functions
2017-05-22 14:32 ` Luis Oliveira
(?)
@ 2017-05-22 16:37 ` Andy Shevchenko
-1 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2017-05-22 16:37 UTC (permalink / raw)
To: Luis Oliveira
Cc: Andy Shevchenko, Wolfram Sang, Rob Herring, Mark Rutland,
Jarkko Nikula, Mika Westerberg, linux-i2c, devicetree,
linux-kernel, Ramiro Oliveira, Joao Pinto, CARLOS.PALMINHA
On Mon, May 22, 2017 at 5:32 PM, Luis Oliveira
<Luis.Oliveira@synopsys.com> wrote:
> On 08-May-17 17:53, Andy Shevchenko wrote:
>> On Mon, 2017-05-08 at 11:37 +0100, Luis Oliveira wrote:
>>> + enabled, slave_activity, raw_stat, stat);
>>> +
>>
>>> + if (slave_activity) {
>>> + if (stat & DW_IC_INTR_RD_REQ) {
>>
>> Looking into next condition (stat & DW_IC_INTR_RX_DONE) I would
>> exchange these lines
>>
>
> By order? I don't think I understood your point. Please elaborate a little more.
if (stat & DW_IC_INTR_RD_REQ) {
if (slave_activity) {
And perhaps refactor function a bit.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v9 6/6] i2c: designware: enable SLAVE in platform module
2017-05-08 10:37 [PATCH v9 0/6] i2c: designware: add I2C SLAVE support Luis Oliveira
` (4 preceding siblings ...)
2017-05-08 10:37 ` [PATCH v9 5/6] i2c: designware: add SLAVE mode functions Luis Oliveira
@ 2017-05-08 10:37 ` Luis Oliveira
2017-05-08 16:40 ` Andy Shevchenko
5 siblings, 1 reply; 16+ messages in thread
From: Luis Oliveira @ 2017-05-08 10:37 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, jarkko.nikula, andriy.shevchenko,
mika.westerberg, linux-i2c, devicetree, linux-kernel
Cc: Luis.Oliveira, Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
- Slave mode selected in platform module if the support is detected in
the DT.
Signed-off-by: Luis Oliveira <lolivei@synopsys.com>
---
v8-v9
- fixed kbuild error regarding I2C_DESIGNWARE_SLAVE auto selection to
correctly compile the module
drivers/i2c/busses/Kconfig | 2 ++
drivers/i2c/busses/i2c-designware-core.h | 2 ++
drivers/i2c/busses/i2c-designware-platdrv.c | 41 +++++++++++++++++++++++++----
3 files changed, 40 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 88e5062015f8..9d4d9901be01 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -471,6 +471,7 @@ 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
@@ -483,6 +484,7 @@ config I2C_DESIGNWARE_SLAVE
bool "Synopsys DesignWare Slave"
select I2C_SLAVE
depends on I2C_DESIGNWARE_PLATFORM
+ default y
help
If you say yes to this option, support will be included for the
Synopsys DesignWare I2C slave adapter.
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 8c6fc8384b1e..944af016eac0 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 ab7a4314a91a..a2b2c8084692 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.
*
@@ -170,9 +170,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;
@@ -185,6 +189,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))
@@ -298,9 +324,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)) {
@@ -329,7 +356,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.11.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v9 6/6] i2c: designware: enable SLAVE in platform module
2017-05-08 10:37 ` [PATCH v9 6/6] i2c: designware: enable SLAVE in platform module Luis Oliveira
@ 2017-05-08 16:40 ` Andy Shevchenko
0 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2017-05-08 16:40 UTC (permalink / raw)
To: Luis Oliveira, wsa, robh+dt, mark.rutland, jarkko.nikula,
mika.westerberg, linux-i2c, devicetree, linux-kernel
Cc: Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
On Mon, 2017-05-08 at 11:37 +0100, Luis Oliveira wrote:
> - Slave mode selected in platform module if the support is detected in
> the DT.
> 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
> @@ -483,6 +484,7 @@ config I2C_DESIGNWARE_SLAVE
> bool "Synopsys DesignWare Slave"
> select I2C_SLAVE
> depends on I2C_DESIGNWARE_PLATFORM
> + default y
Can you elaborate why this line is needed?
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 16+ messages in thread