* [PATCH v5 0/2] add support for Socionext SynQuacer I2C controller
@ 2018-03-02 8:31 Ard Biesheuvel
2018-03-02 8:31 ` [PATCH v5 1/2] dt-bindings: i2c: add binding for Socionext SynQuacer I2C Ard Biesheuvel
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Ard Biesheuvel @ 2018-03-02 8:31 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, linux-i2c, devicetree
Cc: linux-kernel, linux-arm-kernel, jassisinghbrar, andy.shevchenko,
Ard Biesheuvel
Add a binding and a driver for the I2C IP found in the Socionext SynQuacer
SoC, which is essentially a rebranded version of the Fujitsu F_I2C controller.
I think this is good to go now. Wolfram?
v5:
- add Rob's ack to #1
- drop unnecessary 'platform_set_drvdata(pdev, NULL)' in remove path (#2)
v4:
- clarify binding that only a single interrupt specifier is expected (#1)
- check return value of clk_prepare_enable() on probe path (#2)
- add Andy's R-b to patch #2
v3:
- incorporate more of Andy's review comments (#2), especially regarding the
bus speed and clock source handling for ACPI
- patch #1 unchanged.
v2:
- incorporate Andy's review comments (#2)
- patch #1 unchanged.
Ard Biesheuvel (2):
dt-bindings: i2c: add binding for Socionext SynQuacer I2C
i2c: add support for Socionext SynQuacer I2C controller
Documentation/devicetree/bindings/i2c/i2c-synquacer.txt | 29 +
drivers/i2c/busses/Kconfig | 10 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-synquacer.c | 791 ++++++++++++++++++++
4 files changed, 831 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-synquacer.txt
create mode 100644 drivers/i2c/busses/i2c-synquacer.c
--
2.11.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v5 1/2] dt-bindings: i2c: add binding for Socionext SynQuacer I2C
2018-03-02 8:31 [PATCH v5 0/2] add support for Socionext SynQuacer I2C controller Ard Biesheuvel
@ 2018-03-02 8:31 ` Ard Biesheuvel
2018-03-02 8:31 ` [PATCH v5 2/2] i2c: add support for Socionext SynQuacer I2C controller Ard Biesheuvel
2018-03-10 15:01 ` [PATCH v5 0/2] " Ard Biesheuvel
2 siblings, 0 replies; 9+ messages in thread
From: Ard Biesheuvel @ 2018-03-02 8:31 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, linux-i2c, devicetree
Cc: linux-kernel, linux-arm-kernel, jassisinghbrar, andy.shevchenko,
Ard Biesheuvel
Add a binding for the I2C controller that can be found in the Socionext
SynQuacer SoC.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/i2c/i2c-synquacer.txt | 29 ++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-synquacer.txt b/Documentation/devicetree/bindings/i2c/i2c-synquacer.txt
new file mode 100644
index 000000000000..72f4a2f0fedc
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-synquacer.txt
@@ -0,0 +1,29 @@
+Socionext SynQuacer I2C
+
+Required properties:
+- compatible : Must be "socionext,synquacer-i2c"
+- reg : Offset and length of the register set for the device
+- interrupts : A single interrupt specifier
+- #address-cells : Must be <1>;
+- #size-cells : Must be <0>;
+- clock-names : Must contain "pclk".
+- clocks : Must contain an entry for each name in clock-names.
+ (See the common clock bindings.)
+
+Optional properties:
+- clock-frequency : Desired I2C bus clock frequency in Hz. As only Normal and
+ Fast modes are supported, possible values are 100000 and
+ 400000.
+
+Example :
+
+ i2c@51210000 {
+ compatible = "socionext,synquacer-i2c";
+ reg = <0x51210000 0x1000>;
+ interrupts = <GIC_SPI 165 IRQ_TYPE_LEVEL_HIGH>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ clock-names = "pclk";
+ clocks = <&clk_i2c>;
+ clock-frequency = <400000>;
+ };
--
2.11.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v5 2/2] i2c: add support for Socionext SynQuacer I2C controller
2018-03-02 8:31 [PATCH v5 0/2] add support for Socionext SynQuacer I2C controller Ard Biesheuvel
2018-03-02 8:31 ` [PATCH v5 1/2] dt-bindings: i2c: add binding for Socionext SynQuacer I2C Ard Biesheuvel
@ 2018-03-02 8:31 ` Ard Biesheuvel
2018-03-24 23:19 ` Wolfram Sang
2018-03-10 15:01 ` [PATCH v5 0/2] " Ard Biesheuvel
2 siblings, 1 reply; 9+ messages in thread
From: Ard Biesheuvel @ 2018-03-02 8:31 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, linux-i2c, devicetree
Cc: linux-kernel, linux-arm-kernel, jassisinghbrar, andy.shevchenko,
Ard Biesheuvel
This is a cleaned up version of the I2C controller driver for
the Fujitsu F_I2C IP, which was never supported upstream, and
has now been incorporated into the Socionext SynQuacer SoC.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
drivers/i2c/busses/Kconfig | 10 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-synquacer.c | 791 ++++++++++++++++++++
3 files changed, 802 insertions(+)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index e2954fb86d65..56903952d364 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -997,6 +997,16 @@ config I2C_SUN6I_P2WI
This interface is used to connect to specific PMIC devices (like the
AXP221).
+config I2C_SYNQUACER
+ tristate "Socionext SynQuacer I2C controller"
+ depends on ARCH_SYNQUACER || COMPILE_TEST
+ help
+ Say Y here to include support for the I2C controller used in some
+ Fujitsu and Socionext SoCs.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-synquacer.
+
config I2C_TEGRA
tristate "NVIDIA Tegra internal I2C controller"
depends on ARCH_TEGRA
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 2ce8576540a2..40ab7bfc3458 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -98,6 +98,7 @@ obj-$(CONFIG_I2C_STM32F4) += i2c-stm32f4.o
obj-$(CONFIG_I2C_STM32F7) += i2c-stm32f7.o
obj-$(CONFIG_I2C_STU300) += i2c-stu300.o
obj-$(CONFIG_I2C_SUN6I_P2WI) += i2c-sun6i-p2wi.o
+obj-$(CONFIG_I2C_SYNQUACER) += i2c-synquacer.o
obj-$(CONFIG_I2C_TEGRA) += i2c-tegra.o
obj-$(CONFIG_I2C_TEGRA_BPMP) += i2c-tegra-bpmp.o
obj-$(CONFIG_I2C_UNIPHIER) += i2c-uniphier.o
diff --git a/drivers/i2c/busses/i2c-synquacer.c b/drivers/i2c/busses/i2c-synquacer.c
new file mode 100644
index 000000000000..22096f06f3cf
--- /dev/null
+++ b/drivers/i2c/busses/i2c-synquacer.c
@@ -0,0 +1,791 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2012 FUJITSU SEMICONDUCTOR LIMITED
+ */
+
+#include <linux/acpi.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.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/platform_device.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#define WAIT_PCLK(n, rate) \
+ ndelay(DIV_ROUND_UP(DIV_ROUND_UP(1000000000, rate), n) + 10)
+
+/* I2C register address definitions */
+#define SYNQUACER_I2C_REG_BSR (0x00 << 2) // Bus Status
+#define SYNQUACER_I2C_REG_BCR (0x01 << 2) // Bus Control
+#define SYNQUACER_I2C_REG_CCR (0x02 << 2) // Clock Control
+#define SYNQUACER_I2C_REG_ADR (0x03 << 2) // Address
+#define SYNQUACER_I2C_REG_DAR (0x04 << 2) // Data
+#define SYNQUACER_I2C_REG_CSR (0x05 << 2) // Expansion CS
+#define SYNQUACER_I2C_REG_FSR (0x06 << 2) // Bus Clock Freq
+#define SYNQUACER_I2C_REG_BC2R (0x07 << 2) // Bus Control 2
+
+/* I2C register bit definitions */
+#define SYNQUACER_I2C_BSR_FBT BIT(0) // First Byte Transfer
+#define SYNQUACER_I2C_BSR_GCA BIT(1) // General Call Address
+#define SYNQUACER_I2C_BSR_AAS BIT(2) // Address as Slave
+#define SYNQUACER_I2C_BSR_TRX BIT(3) // Transfer/Receive
+#define SYNQUACER_I2C_BSR_LRB BIT(4) // Last Received Bit
+#define SYNQUACER_I2C_BSR_AL BIT(5) // Arbitration Lost
+#define SYNQUACER_I2C_BSR_RSC BIT(6) // Repeated Start Cond.
+#define SYNQUACER_I2C_BSR_BB BIT(7) // Bus Busy
+
+#define SYNQUACER_I2C_BCR_INT BIT(0) // Interrupt
+#define SYNQUACER_I2C_BCR_INTE BIT(1) // Interrupt Enable
+#define SYNQUACER_I2C_BCR_GCAA BIT(2) // Gen. Call Access Ack.
+#define SYNQUACER_I2C_BCR_ACK BIT(3) // Acknowledge
+#define SYNQUACER_I2C_BCR_MSS BIT(4) // Master Slave Select
+#define SYNQUACER_I2C_BCR_SCC BIT(5) // Start Condition Cont.
+#define SYNQUACER_I2C_BCR_BEIE BIT(6) // Bus Error Int Enable
+#define SYNQUACER_I2C_BCR_BER BIT(7) // Bus Error
+
+#define SYNQUACER_I2C_CCR_CS_MASK (0x1f) // CCR Clock Period Sel.
+#define SYNQUACER_I2C_CCR_EN BIT(5) // Enable
+#define SYNQUACER_I2C_CCR_FM BIT(6) // Speed Mode Select
+
+#define SYNQUACER_I2C_CSR_CS_MASK (0x3f) // CSR Clock Period Sel.
+
+#define SYNQUACER_I2C_BC2R_SCLL BIT(0) // SCL Low Drive
+#define SYNQUACER_I2C_BC2R_SDAL BIT(1) // SDA Low Drive
+#define SYNQUACER_I2C_BC2R_SCLS BIT(4) // SCL Status
+#define SYNQUACER_I2C_BC2R_SDAS BIT(5) // SDA Status
+
+/* PCLK frequency */
+#define SYNQUACER_I2C_BUS_CLK_FR(rate) (((rate) / 20000000) + 1)
+
+/* STANDARD MODE frequency */
+#define SYNQUACER_I2C_CLK_MASTER_STD(rate) \
+ DIV_ROUND_UP(DIV_ROUND_UP((rate), 100000) - 2, 2)
+/* FAST MODE frequency */
+#define SYNQUACER_I2C_CLK_MASTER_FAST(rate) \
+ DIV_ROUND_UP((DIV_ROUND_UP((rate), 400000) - 2) * 2, 3)
+
+/* (clkrate <= 18000000) */
+/* calculate the value of CS bits in CCR register on standard mode */
+#define SYNQUACER_I2C_CCR_CS_STD_MAX_18M(rate) \
+ ((SYNQUACER_I2C_CLK_MASTER_STD(rate) - 65) \
+ & SYNQUACER_I2C_CCR_CS_MASK)
+
+/* calculate the value of CS bits in CSR register on standard mode */
+#define SYNQUACER_I2C_CSR_CS_STD_MAX_18M(rate) 0x00
+
+/* calculate the value of CS bits in CCR register on fast mode */
+#define SYNQUACER_I2C_CCR_CS_FAST_MAX_18M(rate) \
+ ((SYNQUACER_I2C_CLK_MASTER_FAST(rate) - 1) \
+ & SYNQUACER_I2C_CCR_CS_MASK)
+
+/* calculate the value of CS bits in CSR register on fast mode */
+#define SYNQUACER_I2C_CSR_CS_FAST_MAX_18M(rate) 0x00
+
+/* (clkrate > 18000000) */
+/* calculate the value of CS bits in CCR register on standard mode */
+#define SYNQUACER_I2C_CCR_CS_STD_MIN_18M(rate) \
+ ((SYNQUACER_I2C_CLK_MASTER_STD(rate) - 1) \
+ & SYNQUACER_I2C_CCR_CS_MASK)
+
+/* calculate the value of CS bits in CSR register on standard mode */
+#define SYNQUACER_I2C_CSR_CS_STD_MIN_18M(rate) \
+ (((SYNQUACER_I2C_CLK_MASTER_STD(rate) - 1) >> 5) \
+ & SYNQUACER_I2C_CSR_CS_MASK)
+
+/* calculate the value of CS bits in CCR register on fast mode */
+#define SYNQUACER_I2C_CCR_CS_FAST_MIN_18M(rate) \
+ ((SYNQUACER_I2C_CLK_MASTER_FAST(rate) - 1) \
+ & SYNQUACER_I2C_CCR_CS_MASK)
+
+/* calculate the value of CS bits in CSR register on fast mode */
+#define SYNQUACER_I2C_CSR_CS_FAST_MIN_18M(rate) \
+ (((SYNQUACER_I2C_CLK_MASTER_FAST(rate) - 1) >> 5) \
+ & SYNQUACER_I2C_CSR_CS_MASK)
+
+/* min I2C clock frequency 14M */
+#define SYNQUACER_I2C_MIN_CLK_RATE (14 * 1000000)
+/* max I2C clock frequency 200M */
+#define SYNQUACER_I2C_MAX_CLK_RATE (200 * 1000000)
+/* I2C clock frequency 18M */
+#define SYNQUACER_I2C_CLK_RATE_18M (18 * 1000000)
+
+#define SYNQUACER_I2C_SPEED_FM 400 // Fast Mode
+#define SYNQUACER_I2C_SPEED_SM 100 // Standard Mode
+
+enum i2c_state {
+ STATE_IDLE,
+ STATE_START,
+ STATE_READ,
+ STATE_WRITE
+};
+
+struct synquacer_i2c {
+ struct completion completion;
+
+ struct i2c_msg *msg;
+ u32 msg_num;
+ u32 msg_idx;
+ u32 msg_ptr;
+
+ u32 irq;
+ struct device *dev;
+ void __iomem *base;
+ struct clk *pclk;
+ u32 pclkrate;
+ u32 speed_khz;
+ u32 timeout_ms;
+ enum i2c_state state;
+ struct i2c_adapter adapter;
+
+ bool is_suspended;
+};
+
+static inline int is_lastmsg(struct synquacer_i2c *i2c)
+{
+ return i2c->msg_idx >= (i2c->msg_num - 1);
+}
+
+static inline int is_msglast(struct synquacer_i2c *i2c)
+{
+ return i2c->msg_ptr == (i2c->msg->len - 1);
+}
+
+static inline int is_msgend(struct synquacer_i2c *i2c)
+{
+ return i2c->msg_ptr >= i2c->msg->len;
+}
+
+static inline unsigned long calc_timeout_ms(struct synquacer_i2c *i2c,
+ struct i2c_msg *msgs,
+ int num)
+{
+ unsigned long bit_count = 0;
+ int i;
+
+ for (i = 0; i < num; i++, msgs++)
+ bit_count += msgs->len;
+
+ return DIV_ROUND_UP((bit_count * 9 + num * 10) * 3, 200) + 10;
+}
+
+static void synquacer_i2c_stop(struct synquacer_i2c *i2c, int ret)
+{
+ /*
+ * clear IRQ (INT=0, BER=0)
+ * set Stop Condition (MSS=0)
+ * Interrupt Disable
+ */
+ writeb(0, i2c->base + SYNQUACER_I2C_REG_BCR);
+
+ i2c->state = STATE_IDLE;
+
+ i2c->msg_ptr = 0;
+ i2c->msg = NULL;
+ i2c->msg_idx++;
+ i2c->msg_num = 0;
+ if (ret)
+ i2c->msg_idx = ret;
+
+ complete(&i2c->completion);
+}
+
+static void synquacer_i2c_hw_init(struct synquacer_i2c *i2c)
+{
+ unsigned char ccr_cs, csr_cs;
+ u32 rt = i2c->pclkrate;
+
+ /* Set own Address */
+ writeb(0, i2c->base + SYNQUACER_I2C_REG_ADR);
+
+ /* Set PCLK frequency */
+ writeb(SYNQUACER_I2C_BUS_CLK_FR(i2c->pclkrate),
+ i2c->base + SYNQUACER_I2C_REG_FSR);
+
+ switch (i2c->speed_khz) {
+ case SYNQUACER_I2C_SPEED_FM:
+ if (i2c->pclkrate <= SYNQUACER_I2C_CLK_RATE_18M) {
+ ccr_cs = SYNQUACER_I2C_CCR_CS_FAST_MAX_18M(rt);
+ csr_cs = SYNQUACER_I2C_CSR_CS_FAST_MAX_18M(rt);
+ } else {
+ ccr_cs = SYNQUACER_I2C_CCR_CS_FAST_MIN_18M(rt);
+ csr_cs = SYNQUACER_I2C_CSR_CS_FAST_MIN_18M(rt);
+ }
+
+ /* Set Clock and enable, Set fast mode */
+ writeb(ccr_cs | SYNQUACER_I2C_CCR_FM |
+ SYNQUACER_I2C_CCR_EN,
+ i2c->base + SYNQUACER_I2C_REG_CCR);
+ writeb(csr_cs, i2c->base + SYNQUACER_I2C_REG_CSR);
+ break;
+ case SYNQUACER_I2C_SPEED_SM:
+ if (i2c->pclkrate <= SYNQUACER_I2C_CLK_RATE_18M) {
+ ccr_cs = SYNQUACER_I2C_CCR_CS_STD_MAX_18M(rt);
+ csr_cs = SYNQUACER_I2C_CSR_CS_STD_MAX_18M(rt);
+ } else {
+ ccr_cs = SYNQUACER_I2C_CCR_CS_STD_MIN_18M(rt);
+ csr_cs = SYNQUACER_I2C_CSR_CS_STD_MIN_18M(rt);
+ }
+
+ /* Set Clock and enable, Set standard mode */
+ writeb(ccr_cs | SYNQUACER_I2C_CCR_EN,
+ i2c->base + SYNQUACER_I2C_REG_CCR);
+ writeb(csr_cs, i2c->base + SYNQUACER_I2C_REG_CSR);
+ break;
+ default:
+ WARN_ON(1);
+ }
+
+ /* clear IRQ (INT=0, BER=0), Interrupt Disable */
+ writeb(0, i2c->base + SYNQUACER_I2C_REG_BCR);
+ writeb(0, i2c->base + SYNQUACER_I2C_REG_BC2R);
+}
+
+static void synquacer_i2c_hw_reset(struct synquacer_i2c *i2c)
+{
+ /* Disable clock */
+ writeb(0, i2c->base + SYNQUACER_I2C_REG_CCR);
+ writeb(0, i2c->base + SYNQUACER_I2C_REG_CSR);
+
+ WAIT_PCLK(100, i2c->pclkrate);
+
+ synquacer_i2c_hw_init(i2c);
+}
+
+static int synquacer_i2c_master_start(struct synquacer_i2c *i2c,
+ struct i2c_msg *pmsg)
+{
+ unsigned char bsr, bcr;
+
+ if (pmsg->flags & I2C_M_RD)
+ writeb((pmsg->addr << 1) | 1,
+ i2c->base + SYNQUACER_I2C_REG_DAR);
+ else
+ writeb(pmsg->addr << 1,
+ i2c->base + SYNQUACER_I2C_REG_DAR);
+
+ dev_dbg(i2c->dev, "slave:0x%02x\n", pmsg->addr);
+
+ /* Generate Start Condition */
+ bsr = readb(i2c->base + SYNQUACER_I2C_REG_BSR);
+ bcr = readb(i2c->base + SYNQUACER_I2C_REG_BCR);
+ dev_dbg(i2c->dev, "bsr:0x%02x, bcr:0x%02x\n", bsr, bcr);
+
+ if ((bsr & SYNQUACER_I2C_BSR_BB) &&
+ !(bcr & SYNQUACER_I2C_BCR_MSS)) {
+ dev_dbg(i2c->dev, "bus is busy");
+ return -EBUSY;
+ }
+
+ if (bsr & SYNQUACER_I2C_BSR_BB) { /* Bus is busy */
+ dev_dbg(i2c->dev, "Continuous Start");
+ writeb(bcr | SYNQUACER_I2C_BCR_SCC,
+ i2c->base + SYNQUACER_I2C_REG_BCR);
+ } else {
+ if (bcr & SYNQUACER_I2C_BCR_MSS) {
+ dev_dbg(i2c->dev, "not in master mode");
+ return -EAGAIN;
+ }
+ dev_dbg(i2c->dev, "Start Condition");
+ /* Start Condition + Enable Interrupts */
+ writeb(bcr | SYNQUACER_I2C_BCR_MSS |
+ SYNQUACER_I2C_BCR_INTE | SYNQUACER_I2C_BCR_BEIE,
+ i2c->base + SYNQUACER_I2C_REG_BCR);
+ }
+
+ WAIT_PCLK(10, i2c->pclkrate);
+
+ /* get BSR & BCR registers */
+ bsr = readb(i2c->base + SYNQUACER_I2C_REG_BSR);
+ bcr = readb(i2c->base + SYNQUACER_I2C_REG_BCR);
+ dev_dbg(i2c->dev, "bsr:0x%02x, bcr:0x%02x\n", bsr, bcr);
+
+ if ((bsr & SYNQUACER_I2C_BSR_AL) ||
+ !(bcr & SYNQUACER_I2C_BCR_MSS)) {
+ dev_dbg(i2c->dev, "arbitration lost\n");
+ return -EAGAIN;
+ }
+
+ return 0;
+}
+
+static int synquacer_i2c_master_recover(struct synquacer_i2c *i2c)
+{
+ unsigned int count;
+ unsigned char bc2r;
+
+ /* Disable interrupts */
+ writeb(0, i2c->base + SYNQUACER_I2C_REG_BCR);
+
+ /* monitor SDA, SCL */
+ bc2r = readb(i2c->base + SYNQUACER_I2C_REG_BC2R);
+ dev_dbg(i2c->dev, "bc2r:0x%02x\n", bc2r);
+
+ count = 100;
+ do {
+ WAIT_PCLK(20, i2c->pclkrate);
+ bc2r = readb(i2c->base + SYNQUACER_I2C_REG_BC2R);
+
+ /* another master is running */
+ if ((bc2r & SYNQUACER_I2C_BC2R_SDAS) ||
+ !(bc2r & SYNQUACER_I2C_BC2R_SCLS)) {
+ dev_dbg(i2c->dev, "another master is running?\n");
+ return -EAGAIN;
+ }
+ } while (--count);
+
+ /* Force to make one clock pulse */
+ count = 10;
+ do {
+ /* SCL = L->H */
+ writeb(SYNQUACER_I2C_BC2R_SCLL,
+ i2c->base + SYNQUACER_I2C_REG_BC2R);
+ WAIT_PCLK(20, i2c->pclkrate);
+ writeb(0, i2c->base + SYNQUACER_I2C_REG_BC2R);
+
+ WAIT_PCLK(10, i2c->pclkrate);
+
+ bc2r = readb(i2c->base + SYNQUACER_I2C_REG_BC2R);
+
+ WAIT_PCLK(5, i2c->pclkrate);
+
+ if (bc2r & SYNQUACER_I2C_BC2R_SDAS)
+ break;
+ WAIT_PCLK(10, i2c->pclkrate);
+ } while (--count);
+
+ if (!count) {
+ dev_err(i2c->dev, "bc2r: 0x%x\n", bc2r);
+ return -EIO;
+ }
+
+ /* force to make bus-error phase */
+ /* SDA = L */
+ writeb(SYNQUACER_I2C_BC2R_SDAL,
+ i2c->base + SYNQUACER_I2C_REG_BC2R);
+ WAIT_PCLK(10, i2c->pclkrate);
+ /* SDA = H */
+ writeb(0, i2c->base + SYNQUACER_I2C_REG_BC2R);
+ WAIT_PCLK(10, i2c->pclkrate);
+
+ /* Both SDA & SDL should be H */
+ bc2r = readb(i2c->base + SYNQUACER_I2C_REG_BC2R);
+ if ((bc2r & (SYNQUACER_I2C_BC2R_SDAS | SYNQUACER_I2C_BC2R_SCLS)) !=
+ (SYNQUACER_I2C_BC2R_SDAS | SYNQUACER_I2C_BC2R_SCLS)) {
+ dev_err(i2c->dev, "bc2r: 0x%x\n", bc2r);
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int synquacer_i2c_doxfer(struct synquacer_i2c *i2c,
+ struct i2c_msg *msgs, int num)
+{
+ unsigned char bsr;
+ unsigned long timeout, bb_timeout;
+ int ret;
+
+ if (i2c->is_suspended)
+ return -EBUSY;
+
+ synquacer_i2c_hw_init(i2c);
+ bsr = readb(i2c->base + SYNQUACER_I2C_REG_BSR);
+ if (bsr & SYNQUACER_I2C_BSR_BB) {
+ dev_err(i2c->dev, "cannot get bus (bus busy)\n");
+ return -EBUSY;
+ }
+
+ init_completion(&i2c->completion);
+
+ i2c->msg = msgs;
+ i2c->msg_num = num;
+ i2c->msg_ptr = 0;
+ i2c->msg_idx = 0;
+ i2c->state = STATE_START;
+
+ ret = synquacer_i2c_master_start(i2c, i2c->msg);
+ if (ret < 0) {
+ dev_dbg(i2c->dev, "Address failed: (%d)\n", ret);
+ return ret;
+ }
+
+ timeout = wait_for_completion_timeout(&i2c->completion,
+ msecs_to_jiffies(i2c->timeout_ms));
+ if (timeout <= 0) {
+ dev_dbg(i2c->dev, "timeout\n");
+ return -EAGAIN;
+ }
+
+ ret = i2c->msg_idx;
+ if (ret != num) {
+ dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret);
+ return -EAGAIN;
+ }
+
+ /* ensure the stop has been through the bus */
+ bb_timeout = jiffies + HZ;
+ do {
+ bsr = readb(i2c->base + SYNQUACER_I2C_REG_BSR);
+ if (!(bsr & SYNQUACER_I2C_BSR_BB))
+ return 0;
+ } while (time_before(jiffies, bb_timeout));
+
+ return -ETIMEDOUT;
+}
+
+static irqreturn_t synquacer_i2c_isr(int irq, void *dev_id)
+{
+ struct synquacer_i2c *i2c = dev_id;
+
+ unsigned char byte;
+ unsigned char bsr, bcr;
+ int ret;
+
+ bcr = readb(i2c->base + SYNQUACER_I2C_REG_BCR);
+ bsr = readb(i2c->base + SYNQUACER_I2C_REG_BSR);
+ dev_dbg(i2c->dev, "bsr:0x%02x, bcr:0x%02x\n", bsr, bcr);
+
+ if (bcr & SYNQUACER_I2C_BCR_BER) {
+ dev_err(i2c->dev, "bus error\n");
+ synquacer_i2c_stop(i2c, -EAGAIN);
+ goto out;
+ }
+ if ((bsr & SYNQUACER_I2C_BSR_AL) ||
+ !(bcr & SYNQUACER_I2C_BCR_MSS)) {
+ dev_dbg(i2c->dev, "arbitration lost\n");
+ synquacer_i2c_stop(i2c, -EAGAIN);
+ goto out;
+ }
+
+ switch (i2c->state) {
+
+ case STATE_START:
+ if (bsr & SYNQUACER_I2C_BSR_LRB) {
+ dev_dbg(i2c->dev, "ack was not received\n");
+ synquacer_i2c_stop(i2c, -EAGAIN);
+ goto out;
+ }
+
+ if (i2c->msg->flags & I2C_M_RD)
+ i2c->state = STATE_READ;
+ else
+ i2c->state = STATE_WRITE;
+
+ if (is_lastmsg(i2c) && i2c->msg->len == 0) {
+ synquacer_i2c_stop(i2c, 0);
+ goto out;
+ }
+
+ if (i2c->state == STATE_READ)
+ goto prepare_read;
+
+ /* fallthru */
+
+ case STATE_WRITE:
+ if (bsr & SYNQUACER_I2C_BSR_LRB) {
+ dev_dbg(i2c->dev, "WRITE: No Ack\n");
+ synquacer_i2c_stop(i2c, -EAGAIN);
+ goto out;
+ }
+
+ if (!is_msgend(i2c)) {
+ writeb(i2c->msg->buf[i2c->msg_ptr++],
+ i2c->base + SYNQUACER_I2C_REG_DAR);
+
+ /* clear IRQ, and continue */
+ writeb(SYNQUACER_I2C_BCR_BEIE |
+ SYNQUACER_I2C_BCR_MSS |
+ SYNQUACER_I2C_BCR_INTE,
+ i2c->base + SYNQUACER_I2C_REG_BCR);
+ break;
+ }
+ if (is_lastmsg(i2c)) {
+ synquacer_i2c_stop(i2c, 0);
+ break;
+ }
+ dev_dbg(i2c->dev, "WRITE: Next Message\n");
+
+ i2c->msg_ptr = 0;
+ i2c->msg_idx++;
+ i2c->msg++;
+
+ /* send the new start */
+ ret = synquacer_i2c_master_start(i2c, i2c->msg);
+ if (ret < 0) {
+ dev_dbg(i2c->dev, "restart error (%d)\n", ret);
+ synquacer_i2c_stop(i2c, -EAGAIN);
+ break;
+ }
+ i2c->state = STATE_START;
+ break;
+
+ case STATE_READ:
+ byte = readb(i2c->base + SYNQUACER_I2C_REG_DAR);
+ if (!(bsr & SYNQUACER_I2C_BSR_FBT)) /* data */
+ i2c->msg->buf[i2c->msg_ptr++] = byte;
+ else /* address */
+ dev_dbg(i2c->dev, "address:0x%02x. ignore it.\n", byte);
+
+prepare_read:
+ if (is_msglast(i2c)) {
+ writeb(SYNQUACER_I2C_BCR_MSS |
+ SYNQUACER_I2C_BCR_BEIE |
+ SYNQUACER_I2C_BCR_INTE,
+ i2c->base + SYNQUACER_I2C_REG_BCR);
+ break;
+ }
+ if (!is_msgend(i2c)) {
+ writeb(SYNQUACER_I2C_BCR_MSS |
+ SYNQUACER_I2C_BCR_BEIE |
+ SYNQUACER_I2C_BCR_INTE |
+ SYNQUACER_I2C_BCR_ACK,
+ i2c->base + SYNQUACER_I2C_REG_BCR);
+ break;
+ }
+ if (is_lastmsg(i2c)) {
+ /* last message, send stop and complete */
+ dev_dbg(i2c->dev, "READ: Send Stop\n");
+ synquacer_i2c_stop(i2c, 0);
+ break;
+ }
+ dev_dbg(i2c->dev, "READ: Next Transfer\n");
+
+ i2c->msg_ptr = 0;
+ i2c->msg_idx++;
+ i2c->msg++;
+
+ ret = synquacer_i2c_master_start(i2c, i2c->msg);
+ if (ret < 0) {
+ dev_dbg(i2c->dev, "restart error (%d)\n", ret);
+ synquacer_i2c_stop(i2c, -EAGAIN);
+ } else {
+ i2c->state = STATE_START;
+ }
+ break;
+ default:
+ dev_err(i2c->dev, "called in err STATE (%d)\n", i2c->state);
+ break;
+ }
+
+out:
+ WAIT_PCLK(10, i2c->pclkrate);
+ return IRQ_HANDLED;
+}
+
+static int synquacer_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
+ int num)
+{
+ struct synquacer_i2c *i2c;
+ int retry;
+ int ret;
+
+ if (!msgs)
+ return -EINVAL;
+ if (num <= 0)
+ return -EINVAL;
+
+ i2c = i2c_get_adapdata(adap);
+ i2c->timeout_ms = calc_timeout_ms(i2c, msgs, num);
+
+ dev_dbg(i2c->dev, "calculated timeout %d ms\n", i2c->timeout_ms);
+
+ for (retry = 0; retry < adap->retries; retry++) {
+
+ ret = synquacer_i2c_doxfer(i2c, msgs, num);
+ if (ret != -EAGAIN)
+ return ret;
+
+ dev_dbg(i2c->dev, "Retrying transmission (%d)\n", retry);
+
+ synquacer_i2c_master_recover(i2c);
+ synquacer_i2c_hw_reset(i2c);
+ }
+ return -EIO;
+}
+
+static u32 synquacer_i2c_functionality(struct i2c_adapter *adap)
+{
+ return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static const struct i2c_algorithm synquacer_i2c_algo = {
+ .master_xfer = synquacer_i2c_xfer,
+ .functionality = synquacer_i2c_functionality,
+};
+
+static struct i2c_adapter synquacer_i2c_ops = {
+ .owner = THIS_MODULE,
+ .name = "synquacer_i2c-adapter",
+ .algo = &synquacer_i2c_algo,
+ .retries = 5,
+};
+
+static int synquacer_i2c_probe(struct platform_device *pdev)
+{
+ struct synquacer_i2c *i2c;
+ struct resource *r;
+ u32 bus_speed;
+ int ret;
+
+ i2c = devm_kzalloc(&pdev->dev, sizeof(*i2c), GFP_KERNEL);
+ if (!i2c)
+ return -ENOMEM;
+
+ bus_speed = i2c_acpi_find_bus_speed(&pdev->dev);
+ if (!bus_speed)
+ device_property_read_u32(&pdev->dev, "clock-frequency",
+ &bus_speed);
+
+ device_property_read_u32(&pdev->dev, "socionext,pclk-rate",
+ &i2c->pclkrate);
+
+ i2c->pclk = devm_clk_get(&pdev->dev, "pclk");
+ if (IS_ERR(i2c->pclk) && PTR_ERR(i2c->pclk) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ if (!IS_ERR_OR_NULL(i2c->pclk)) {
+ dev_dbg(&pdev->dev, "clock source %p\n", i2c->pclk);
+
+ ret = clk_prepare_enable(i2c->pclk);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to enable clock (%d)\n",
+ ret);
+ return ret;
+ }
+ i2c->pclkrate = clk_get_rate(i2c->pclk);
+ }
+
+ if (i2c->pclkrate < SYNQUACER_I2C_MIN_CLK_RATE ||
+ i2c->pclkrate > SYNQUACER_I2C_MAX_CLK_RATE) {
+ dev_err(&pdev->dev, "PCLK missing or out of range (%d)\n",
+ i2c->pclkrate);
+ return -EINVAL;
+ }
+
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ i2c->base = devm_ioremap_resource(&pdev->dev, r);
+ if (IS_ERR(i2c->base))
+ return PTR_ERR(i2c->base);
+
+ i2c->irq = platform_get_irq(pdev, 0);
+ if (i2c->irq < 0) {
+ dev_err(&pdev->dev, "no IRQ resource found\n");
+ return -ENODEV;
+ }
+
+ ret = devm_request_irq(&pdev->dev, i2c->irq, synquacer_i2c_isr,
+ 0, dev_name(&pdev->dev), i2c);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "cannot claim IRQ %d\n", i2c->irq);
+ return ret;
+ }
+
+ i2c->state = STATE_IDLE;
+ i2c->dev = &pdev->dev;
+ i2c->adapter = synquacer_i2c_ops;
+ i2c_set_adapdata(&i2c->adapter, i2c);
+ i2c->adapter.dev.parent = &pdev->dev;
+ i2c->adapter.nr = pdev->id;
+
+ if (bus_speed < 400000)
+ i2c->speed_khz = SYNQUACER_I2C_SPEED_SM;
+ else
+ i2c->speed_khz = SYNQUACER_I2C_SPEED_FM;
+
+ synquacer_i2c_hw_init(i2c);
+
+ ret = i2c_add_numbered_adapter(&i2c->adapter);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to add bus to i2c core\n");
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, i2c);
+
+ dev_info(&pdev->dev, "%s: synquacer_i2c adapter\n",
+ dev_name(&i2c->adapter.dev));
+
+ return 0;
+}
+
+static int synquacer_i2c_remove(struct platform_device *pdev)
+{
+ struct synquacer_i2c *i2c = platform_get_drvdata(pdev);
+
+ i2c_del_adapter(&i2c->adapter);
+ if (!IS_ERR(i2c->pclk))
+ clk_disable_unprepare(i2c->pclk);
+
+ return 0;
+};
+
+static int __maybe_unused synquacer_i2c_suspend(struct device *dev)
+{
+ struct synquacer_i2c *i2c = dev_get_drvdata(dev);
+
+ i2c_lock_adapter(&i2c->adapter);
+ i2c->is_suspended = true;
+ i2c_unlock_adapter(&i2c->adapter);
+
+ if (!IS_ERR(i2c->pclk))
+ clk_disable_unprepare(i2c->pclk);
+
+ return 0;
+}
+
+static int __maybe_unused synquacer_i2c_resume(struct device *dev)
+{
+ struct synquacer_i2c *i2c = dev_get_drvdata(dev);
+ int ret = 0;
+
+ i2c_lock_adapter(&i2c->adapter);
+
+ if (!IS_ERR(i2c->pclk))
+ ret = clk_prepare_enable(i2c->pclk);
+ if (!ret)
+ i2c->is_suspended = false;
+
+ i2c_unlock_adapter(&i2c->adapter);
+
+ return ret;
+}
+
+static SIMPLE_DEV_PM_OPS(synquacer_i2c_pm, synquacer_i2c_suspend,
+ synquacer_i2c_resume);
+
+static const struct of_device_id synquacer_i2c_dt_ids[] = {
+ { .compatible = "socionext,synquacer-i2c" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, synquacer_i2c_dt_ids);
+
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id synquacer_i2c_acpi_ids[] = {
+ { "SCX0003" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(acpi, synquacer_i2c_acpi_ids);
+#endif
+
+static struct platform_driver synquacer_i2c_driver = {
+ .probe = synquacer_i2c_probe,
+ .remove = synquacer_i2c_remove,
+ .driver = {
+ .name = "synquacer_i2c",
+ .of_match_table = of_match_ptr(synquacer_i2c_dt_ids),
+ .acpi_match_table = ACPI_PTR(synquacer_i2c_acpi_ids),
+ .pm = &synquacer_i2c_pm,
+ },
+};
+module_platform_driver(synquacer_i2c_driver);
+
+MODULE_AUTHOR("Fujitsu Semiconductor Ltd");
+MODULE_DESCRIPTION("Socionext SynQuacer I2C Driver");
+MODULE_LICENSE("GPL v2");
--
2.11.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v5 0/2] add support for Socionext SynQuacer I2C controller
2018-03-02 8:31 [PATCH v5 0/2] add support for Socionext SynQuacer I2C controller Ard Biesheuvel
2018-03-02 8:31 ` [PATCH v5 1/2] dt-bindings: i2c: add binding for Socionext SynQuacer I2C Ard Biesheuvel
2018-03-02 8:31 ` [PATCH v5 2/2] i2c: add support for Socionext SynQuacer I2C controller Ard Biesheuvel
@ 2018-03-10 15:01 ` Ard Biesheuvel
2018-03-16 16:19 ` Ard Biesheuvel
2 siblings, 1 reply; 9+ messages in thread
From: Ard Biesheuvel @ 2018-03-10 15:01 UTC (permalink / raw)
To: Wolfram Sang, Rob Herring, Mark Rutland, linux-i2c, Devicetree List
Cc: Linux Kernel Mailing List, linux-arm-kernel, Jassi Brar,
Andy Shevchenko, Ard Biesheuvel
On 2 March 2018 at 08:31, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> Add a binding and a driver for the I2C IP found in the Socionext SynQuacer
> SoC, which is essentially a rebranded version of the Fujitsu F_I2C controller.
>
> I think this is good to go now. Wolfram?
>
Ping?
> v5:
> - add Rob's ack to #1
> - drop unnecessary 'platform_set_drvdata(pdev, NULL)' in remove path (#2)
>
> v4:
> - clarify binding that only a single interrupt specifier is expected (#1)
> - check return value of clk_prepare_enable() on probe path (#2)
> - add Andy's R-b to patch #2
>
> v3:
> - incorporate more of Andy's review comments (#2), especially regarding the
> bus speed and clock source handling for ACPI
> - patch #1 unchanged.
>
> v2:
> - incorporate Andy's review comments (#2)
> - patch #1 unchanged.
>
> Ard Biesheuvel (2):
> dt-bindings: i2c: add binding for Socionext SynQuacer I2C
> i2c: add support for Socionext SynQuacer I2C controller
>
> Documentation/devicetree/bindings/i2c/i2c-synquacer.txt | 29 +
> drivers/i2c/busses/Kconfig | 10 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-synquacer.c | 791 ++++++++++++++++++++
> 4 files changed, 831 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/i2c/i2c-synquacer.txt
> create mode 100644 drivers/i2c/busses/i2c-synquacer.c
>
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 0/2] add support for Socionext SynQuacer I2C controller
2018-03-10 15:01 ` [PATCH v5 0/2] " Ard Biesheuvel
@ 2018-03-16 16:19 ` Ard Biesheuvel
2018-03-21 1:48 ` Ard Biesheuvel
0 siblings, 1 reply; 9+ messages in thread
From: Ard Biesheuvel @ 2018-03-16 16:19 UTC (permalink / raw)
To: Wolfram Sang, Rob Herring, Mark Rutland, linux-i2c, Devicetree List
Cc: Linux Kernel Mailing List, linux-arm-kernel, Jassi Brar,
Andy Shevchenko, Ard Biesheuvel
On 10 March 2018 at 15:01, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 2 March 2018 at 08:31, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> Add a binding and a driver for the I2C IP found in the Socionext SynQuacer
>> SoC, which is essentially a rebranded version of the Fujitsu F_I2C controller.
>>
>> I think this is good to go now. Wolfram?
>>
>
> Ping?
>
Ping?
>> v5:
>> - add Rob's ack to #1
>> - drop unnecessary 'platform_set_drvdata(pdev, NULL)' in remove path (#2)
>>
>> v4:
>> - clarify binding that only a single interrupt specifier is expected (#1)
>> - check return value of clk_prepare_enable() on probe path (#2)
>> - add Andy's R-b to patch #2
>>
>> v3:
>> - incorporate more of Andy's review comments (#2), especially regarding the
>> bus speed and clock source handling for ACPI
>> - patch #1 unchanged.
>>
>> v2:
>> - incorporate Andy's review comments (#2)
>> - patch #1 unchanged.
>>
>> Ard Biesheuvel (2):
>> dt-bindings: i2c: add binding for Socionext SynQuacer I2C
>> i2c: add support for Socionext SynQuacer I2C controller
>>
>> Documentation/devicetree/bindings/i2c/i2c-synquacer.txt | 29 +
>> drivers/i2c/busses/Kconfig | 10 +
>> drivers/i2c/busses/Makefile | 1 +
>> drivers/i2c/busses/i2c-synquacer.c | 791 ++++++++++++++++++++
>> 4 files changed, 831 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/i2c/i2c-synquacer.txt
>> create mode 100644 drivers/i2c/busses/i2c-synquacer.c
>>
>> --
>> 2.11.0
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 0/2] add support for Socionext SynQuacer I2C controller
2018-03-16 16:19 ` Ard Biesheuvel
@ 2018-03-21 1:48 ` Ard Biesheuvel
0 siblings, 0 replies; 9+ messages in thread
From: Ard Biesheuvel @ 2018-03-21 1:48 UTC (permalink / raw)
To: Wolfram Sang, Rob Herring, Mark Rutland, linux-i2c, Devicetree List
Cc: Linux Kernel Mailing List, linux-arm-kernel, Jassi Brar,
Andy Shevchenko, Ard Biesheuvel
On 17 March 2018 at 00:19, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 10 March 2018 at 15:01, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> On 2 March 2018 at 08:31, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>> Add a binding and a driver for the I2C IP found in the Socionext SynQuacer
>>> SoC, which is essentially a rebranded version of the Fujitsu F_I2C controller.
>>>
>>> I think this is good to go now. Wolfram?
>>>
>>
>> Ping?
>>
>
> Ping?
>
Hello Wolfram,
Do you intend to queue this for v4.17? Or are there any issues that
need to be addressed first?
Thanks,
Ard.
>>> v5:
>>> - add Rob's ack to #1
>>> - drop unnecessary 'platform_set_drvdata(pdev, NULL)' in remove path (#2)
>>>
>>> v4:
>>> - clarify binding that only a single interrupt specifier is expected (#1)
>>> - check return value of clk_prepare_enable() on probe path (#2)
>>> - add Andy's R-b to patch #2
>>>
>>> v3:
>>> - incorporate more of Andy's review comments (#2), especially regarding the
>>> bus speed and clock source handling for ACPI
>>> - patch #1 unchanged.
>>>
>>> v2:
>>> - incorporate Andy's review comments (#2)
>>> - patch #1 unchanged.
>>>
>>> Ard Biesheuvel (2):
>>> dt-bindings: i2c: add binding for Socionext SynQuacer I2C
>>> i2c: add support for Socionext SynQuacer I2C controller
>>>
>>> Documentation/devicetree/bindings/i2c/i2c-synquacer.txt | 29 +
>>> drivers/i2c/busses/Kconfig | 10 +
>>> drivers/i2c/busses/Makefile | 1 +
>>> drivers/i2c/busses/i2c-synquacer.c | 791 ++++++++++++++++++++
>>> 4 files changed, 831 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/i2c/i2c-synquacer.txt
>>> create mode 100644 drivers/i2c/busses/i2c-synquacer.c
>>>
>>> --
>>> 2.11.0
>>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 2/2] i2c: add support for Socionext SynQuacer I2C controller
2018-03-02 8:31 ` [PATCH v5 2/2] i2c: add support for Socionext SynQuacer I2C controller Ard Biesheuvel
@ 2018-03-24 23:19 ` Wolfram Sang
2018-03-24 23:25 ` Wolfram Sang
2018-03-25 10:30 ` Ard Biesheuvel
0 siblings, 2 replies; 9+ messages in thread
From: Wolfram Sang @ 2018-03-24 23:19 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: robh+dt, mark.rutland, linux-i2c, devicetree, linux-kernel,
linux-arm-kernel, jassisinghbrar, andy.shevchenko
[-- Attachment #1: Type: text/plain, Size: 2670 bytes --]
Hi Ard,
> +static int synquacer_i2c_master_start(struct synquacer_i2c *i2c,
> + struct i2c_msg *pmsg)
> +{
> + unsigned char bsr, bcr;
> +
> + if (pmsg->flags & I2C_M_RD)
> + writeb((pmsg->addr << 1) | 1,
> + i2c->base + SYNQUACER_I2C_REG_DAR);
> + else
> + writeb(pmsg->addr << 1,
> + i2c->base + SYNQUACER_I2C_REG_DAR);
writeb(i2c_8bit_addr_from_msg(pmsg), i2c->base + SYNQUACER_I2C_REG_DAR);
?
> +static int synquacer_i2c_master_recover(struct synquacer_i2c *i2c)
> +{
This is the bus recovery mechanism with toggling SCL pulses, right?
That should be implemented using a 'struct i2c_bus_recovery_info' and
the core helpers.
> +static int synquacer_i2c_doxfer(struct synquacer_i2c *i2c,
> + struct i2c_msg *msgs, int num)
> +{
> + unsigned char bsr;
> + unsigned long timeout, bb_timeout;
> + int ret;
> +
> + if (i2c->is_suspended)
> + return -EBUSY;
> +
> + synquacer_i2c_hw_init(i2c);
> + bsr = readb(i2c->base + SYNQUACER_I2C_REG_BSR);
> + if (bsr & SYNQUACER_I2C_BSR_BB) {
> + dev_err(i2c->dev, "cannot get bus (bus busy)\n");
> + return -EBUSY;
> + }
> +
> + init_completion(&i2c->completion);
reinit_completion? And init_completion() in probe()?
> + /* ensure the stop has been through the bus */
> + bb_timeout = jiffies + HZ;
> + do {
> + bsr = readb(i2c->base + SYNQUACER_I2C_REG_BSR);
> + if (!(bsr & SYNQUACER_I2C_BSR_BB))
> + return 0;
> + } while (time_before(jiffies, bb_timeout));
Busy looping for one second? And won't the bus_free detection at the
beginning of a transfer do?
> +static int synquacer_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> + int num)
> +{
> + struct synquacer_i2c *i2c;
> + int retry;
> + int ret;
> +
> + if (!msgs)
> + return -EINVAL;
> + if (num <= 0)
> + return -EINVAL;
Hmm, this should be done by the core. I am surprised it doesn't do that yet :/
> +
> + i2c = i2c_get_adapdata(adap);
> + i2c->timeout_ms = calc_timeout_ms(i2c, msgs, num);
> +
> + dev_dbg(i2c->dev, "calculated timeout %d ms\n", i2c->timeout_ms);
> +
> + for (retry = 0; retry < adap->retries; retry++) {
> +
> + ret = synquacer_i2c_doxfer(i2c, msgs, num);
> + if (ret != -EAGAIN)
> + return ret;
> +
> + dev_dbg(i2c->dev, "Retrying transmission (%d)\n", retry);
> +
> + synquacer_i2c_master_recover(i2c);
Recovery is only when SDA is stuck low, held by a client. That is
nothing you should do just on any error.
If you want the driver in v4.17, I'd suggest to drop
synquacer_i2c_master_recover() now and add it incrementally later, using
the existing recovery infrastructure. The rest is only minor stuff and
needs not much further discussion IMO.
Regards,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 2/2] i2c: add support for Socionext SynQuacer I2C controller
2018-03-24 23:19 ` Wolfram Sang
@ 2018-03-24 23:25 ` Wolfram Sang
2018-03-25 10:30 ` Ard Biesheuvel
1 sibling, 0 replies; 9+ messages in thread
From: Wolfram Sang @ 2018-03-24 23:25 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: robh+dt, mark.rutland, linux-i2c, devicetree, linux-kernel,
linux-arm-kernel, jassisinghbrar, andy.shevchenko
[-- Attachment #1: Type: text/plain, Size: 445 bytes --]
My code checkers mention this:
CPPCHECK
drivers/i2c/busses/i2c-synquacer.c:422: style: Checking if unsigned variable 'timeout' is less than zero.
CC drivers/i2c/busses/i2c-synquacer.o
drivers/i2c/busses/i2c-synquacer.c: In function ‘synquacer_i2c_probe’:
drivers/i2c/busses/i2c-synquacer.c:678:15: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
if (i2c->irq < 0) {
^
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 2/2] i2c: add support for Socionext SynQuacer I2C controller
2018-03-24 23:19 ` Wolfram Sang
2018-03-24 23:25 ` Wolfram Sang
@ 2018-03-25 10:30 ` Ard Biesheuvel
1 sibling, 0 replies; 9+ messages in thread
From: Ard Biesheuvel @ 2018-03-25 10:30 UTC (permalink / raw)
To: Wolfram Sang
Cc: Rob Herring, Mark Rutland, linux-i2c, Devicetree List,
Linux Kernel Mailing List, linux-arm-kernel, Jassi Brar,
Andy Shevchenko
On 24 March 2018 at 23:19, Wolfram Sang <wsa@the-dreams.de> wrote:
> Hi Ard,
>
>> +static int synquacer_i2c_master_start(struct synquacer_i2c *i2c,
>> + struct i2c_msg *pmsg)
>> +{
>> + unsigned char bsr, bcr;
>> +
>> + if (pmsg->flags & I2C_M_RD)
>> + writeb((pmsg->addr << 1) | 1,
>> + i2c->base + SYNQUACER_I2C_REG_DAR);
>> + else
>> + writeb(pmsg->addr << 1,
>> + i2c->base + SYNQUACER_I2C_REG_DAR);
>
> writeb(i2c_8bit_addr_from_msg(pmsg), i2c->base + SYNQUACER_I2C_REG_DAR);
>
> ?
>
>> +static int synquacer_i2c_master_recover(struct synquacer_i2c *i2c)
>> +{
>
> This is the bus recovery mechanism with toggling SCL pulses, right?
> That should be implemented using a 'struct i2c_bus_recovery_info' and
> the core helpers.
>
>> +static int synquacer_i2c_doxfer(struct synquacer_i2c *i2c,
>> + struct i2c_msg *msgs, int num)
>> +{
>> + unsigned char bsr;
>> + unsigned long timeout, bb_timeout;
>> + int ret;
>> +
>> + if (i2c->is_suspended)
>> + return -EBUSY;
>> +
>> + synquacer_i2c_hw_init(i2c);
>> + bsr = readb(i2c->base + SYNQUACER_I2C_REG_BSR);
>> + if (bsr & SYNQUACER_I2C_BSR_BB) {
>> + dev_err(i2c->dev, "cannot get bus (bus busy)\n");
>> + return -EBUSY;
>> + }
>> +
>> + init_completion(&i2c->completion);
>
> reinit_completion? And init_completion() in probe()?
>
>> + /* ensure the stop has been through the bus */
>> + bb_timeout = jiffies + HZ;
>> + do {
>> + bsr = readb(i2c->base + SYNQUACER_I2C_REG_BSR);
>> + if (!(bsr & SYNQUACER_I2C_BSR_BB))
>> + return 0;
>> + } while (time_before(jiffies, bb_timeout));
>
> Busy looping for one second? And won't the bus_free detection at the
> beginning of a transfer do?
>
>> +static int synquacer_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
>> + int num)
>> +{
>> + struct synquacer_i2c *i2c;
>> + int retry;
>> + int ret;
>> +
>> + if (!msgs)
>> + return -EINVAL;
>> + if (num <= 0)
>> + return -EINVAL;
>
> Hmm, this should be done by the core. I am surprised it doesn't do that yet :/
>
>> +
>> + i2c = i2c_get_adapdata(adap);
>> + i2c->timeout_ms = calc_timeout_ms(i2c, msgs, num);
>> +
>> + dev_dbg(i2c->dev, "calculated timeout %d ms\n", i2c->timeout_ms);
>> +
>> + for (retry = 0; retry < adap->retries; retry++) {
>> +
>> + ret = synquacer_i2c_doxfer(i2c, msgs, num);
>> + if (ret != -EAGAIN)
>> + return ret;
>> +
>> + dev_dbg(i2c->dev, "Retrying transmission (%d)\n", retry);
>> +
>> + synquacer_i2c_master_recover(i2c);
>
> Recovery is only when SDA is stuck low, held by a client. That is
> nothing you should do just on any error.
>
> If you want the driver in v4.17, I'd suggest to drop
> synquacer_i2c_master_recover() now and add it incrementally later, using
> the existing recovery infrastructure. The rest is only minor stuff and
> needs not much further discussion IMO.
>
Thanks Wolfram.
I have fixed all these up and am about to send out the v6.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-03-25 10:30 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-02 8:31 [PATCH v5 0/2] add support for Socionext SynQuacer I2C controller Ard Biesheuvel
2018-03-02 8:31 ` [PATCH v5 1/2] dt-bindings: i2c: add binding for Socionext SynQuacer I2C Ard Biesheuvel
2018-03-02 8:31 ` [PATCH v5 2/2] i2c: add support for Socionext SynQuacer I2C controller Ard Biesheuvel
2018-03-24 23:19 ` Wolfram Sang
2018-03-24 23:25 ` Wolfram Sang
2018-03-25 10:30 ` Ard Biesheuvel
2018-03-10 15:01 ` [PATCH v5 0/2] " Ard Biesheuvel
2018-03-16 16:19 ` Ard Biesheuvel
2018-03-21 1:48 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).