linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v11 0/4] i2c: aspeed: added driver for Aspeed I2C
@ 2017-06-20 21:15 Brendan Higgins
  2017-06-20 21:15 ` [PATCH v11 1/4] MAINTAINERS: add entry for Aspeed I2C driver Brendan Higgins
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Brendan Higgins @ 2017-06-20 21:15 UTC (permalink / raw)
  To: wsa, robh+dt, mark.rutland, gregkh, davem, mchehab, joel,
	martin.petersen, benh
  Cc: linux-i2c, devicetree, linux-kernel, openbmc

Addressed comments from:
  - Wolfram in: http://www.spinics.net/lists/devicetree/msg182142.html
    and http://www.spinics.net/lists/devicetree/msg182144.html

Changes since previous update:
  - MAINTAINERS: Added MAINTAINERS entry for Aspeed I2C driver and friends
  - I2C driver: Added support for COMPILE_TEST
  - I2C driver: Fixed i2c fault codes in interrupt handler
  - I2C driver: Use i2c_{get|set}_adapdata instead of adapter->algo_data
  - I2C driver: Fixed some other stylistic issues

As the Aspeed I2C interrupt controller changes were already accepted by the
irqchip maintainers, this patchset no longer contains them.

As before, tested on Aspeed 2500 evaluation board and a real platform with an
Aspeed 2520.

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

* [PATCH v11 1/4] MAINTAINERS: add entry for Aspeed I2C driver
  2017-06-20 21:15 [PATCH v11 0/4] i2c: aspeed: added driver for Aspeed I2C Brendan Higgins
@ 2017-06-20 21:15 ` Brendan Higgins
  2017-06-23 18:40   ` Wolfram Sang
  2017-06-20 21:15 ` [PATCH v11 2/4] i2c: aspeed: added documentation " Brendan Higgins
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 17+ messages in thread
From: Brendan Higgins @ 2017-06-20 21:15 UTC (permalink / raw)
  To: wsa, robh+dt, mark.rutland, gregkh, davem, mchehab, joel,
	martin.petersen, benh
  Cc: linux-i2c, devicetree, linux-kernel, openbmc, Brendan Higgins

Added myself as maintainer of the Aspeed I2C driver and the associated
I2C interrupt controller and added Joel Stanley and Benjamin
Herrenschmidt as reviewers.

Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
Added in v11:
  - Added MAINTAINERS entry for Aspeed I2C driver and friends
---
 MAINTAINERS | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 053c3bdd1fe5..2bb541549cc1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1123,6 +1123,18 @@ F:	arch/arm/mach-aspeed/
 F:	arch/arm/boot/dts/aspeed-*
 F:	drivers/*/*aspeed*
 
+ARM/ASPEED I2C DRIVER
+M:	Brendan Higgins <brendanhiggins@google.com>
+R:	Benjamin Herrenschmidt <benh@kernel.crashing.org>
+R:	Joel Stanley <joel@jms.id.au>
+L:	linux-i2c@vger.kernel.org
+L:	openbmc@lists.ozlabs.org
+S:	Maintained
+F:	drivers/irqchip/irq-aspeed-i2c-ic.c
+F:	drivers/i2c/busses/i2c-aspeed.c
+F:	Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2400-i2c-ic.txt
+F:	Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
+
 ARM/ATMEL AT91RM9200, AT91SAM9 AND SAMA5 SOC SUPPORT
 M:	Nicolas Ferre <nicolas.ferre@microchip.com>
 M:	Alexandre Belloni <alexandre.belloni@free-electrons.com>
-- 
2.13.1.611.g7e3b11ae1-goog

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

* [PATCH v11 2/4] i2c: aspeed: added documentation for Aspeed I2C driver
  2017-06-20 21:15 [PATCH v11 0/4] i2c: aspeed: added driver for Aspeed I2C Brendan Higgins
  2017-06-20 21:15 ` [PATCH v11 1/4] MAINTAINERS: add entry for Aspeed I2C driver Brendan Higgins
@ 2017-06-20 21:15 ` Brendan Higgins
  2017-06-23 18:41   ` Wolfram Sang
  2017-06-20 21:15 ` [PATCH v11 3/4] i2c: aspeed: added driver for Aspeed I2C Brendan Higgins
  2017-06-20 21:15 ` [PATCH v11 4/4] i2c: aspeed: added slave support for Aspeed I2C driver Brendan Higgins
  3 siblings, 1 reply; 17+ messages in thread
From: Brendan Higgins @ 2017-06-20 21:15 UTC (permalink / raw)
  To: wsa, robh+dt, mark.rutland, gregkh, davem, mchehab, joel,
	martin.petersen, benh
  Cc: linux-i2c, devicetree, linux-kernel, openbmc, Brendan Higgins

Added device tree binding documentation for Aspeed I2C busses.

Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Acked-by: Rob Herring <robh@kernel.org>
---
hanges for v2:
  - None
Changes for v3:
  - Removed reference to "bus" device tree param
Changes for v4:
  - None
Changes for v5:
  - None
Changes for v6:
  - Replaced the controller property with and interrupt controller, leaving only
    the busses in the I2C documentation.
Changes for v7:
  - Changed clock-frequency to bus-frequency in device tree
Changes for v8:
  - Added multi-master property to device tree
Changes for v9:
  - None
Changes for v10:
  - None
Changes for v11:
  - None
---
 .../devicetree/bindings/i2c/i2c-aspeed.txt         | 48 ++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt

diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
new file mode 100644
index 000000000000..bd6480b19535
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
@@ -0,0 +1,48 @@
+Device tree configuration for the I2C busses on the AST24XX and AST25XX SoCs.
+
+Required Properties:
+- #address-cells	: should be 1
+- #size-cells		: should be 0
+- reg			: address offset and range of bus
+- compatible		: should be "aspeed,ast2400-i2c-bus"
+			  or "aspeed,ast2500-i2c-bus"
+- clocks		: root clock of bus, should reference the APB
+			  clock
+- interrupts		: interrupt number
+- interrupt-parent	: interrupt controller for bus, should reference a
+			  aspeed,ast2400-i2c-ic or aspeed,ast2500-i2c-ic
+			  interrupt controller
+
+Optional Properties:
+- bus-frequency	: frequency of the bus clock in Hz defaults to 100 kHz when not
+		  specified
+- multi-master	: states that there is another master active on this bus.
+
+Example:
+
+i2c {
+	compatible = "simple-bus";
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges = <0 0x1e78a000 0x1000>;
+
+	i2c_ic: interrupt-controller@0 {
+		#interrupt-cells = <1>;
+		compatible = "aspeed,ast2400-i2c-ic";
+		reg = <0x0 0x40>;
+		interrupts = <12>;
+		interrupt-controller;
+	};
+
+	i2c0: i2c-bus@40 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		#interrupt-cells = <1>;
+		reg = <0x40 0x40>;
+		compatible = "aspeed,ast2400-i2c-bus";
+		clocks = <&clk_apb>;
+		bus-frequency = <100000>;
+		interrupts = <0>;
+		interrupt-parent = <&i2c_ic>;
+	};
+};
-- 
2.13.1.611.g7e3b11ae1-goog

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

* [PATCH v11 3/4] i2c: aspeed: added driver for Aspeed I2C
  2017-06-20 21:15 [PATCH v11 0/4] i2c: aspeed: added driver for Aspeed I2C Brendan Higgins
  2017-06-20 21:15 ` [PATCH v11 1/4] MAINTAINERS: add entry for Aspeed I2C driver Brendan Higgins
  2017-06-20 21:15 ` [PATCH v11 2/4] i2c: aspeed: added documentation " Brendan Higgins
@ 2017-06-20 21:15 ` Brendan Higgins
  2017-06-23 18:43   ` Wolfram Sang
  2017-06-20 21:15 ` [PATCH v11 4/4] i2c: aspeed: added slave support for Aspeed I2C driver Brendan Higgins
  3 siblings, 1 reply; 17+ messages in thread
From: Brendan Higgins @ 2017-06-20 21:15 UTC (permalink / raw)
  To: wsa, robh+dt, mark.rutland, gregkh, davem, mchehab, joel,
	martin.petersen, benh
  Cc: linux-i2c, devicetree, linux-kernel, openbmc, Brendan Higgins

Added initial master support for Aspeed I2C controller. Supports
fourteen busses present in AST24XX and AST25XX BMC SoCs by Aspeed.

Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
hanges for v2:
  - Added single module_init (multiple was breaking some builds).
Changes for v3:
  - Removed "bus" device tree param; now extracted from bus address offset
Changes for v4:
  - I2C adapter number is now generated dynamically unless specified in alias.
Changes for v5:
  - Removed irq_chip used to multiplex IRQ and replaced it with dummy_irq_chip
    along with some other IRQ cleanup.
  - Addressed comments from Cedric, and Vladimir, mostly stylistic things and
    using devm managed resources.
  - Increased max clock frequency before the bus is put in HighSpeed mode, as
    per Kachalov's comment.
Changes for v6:
  - No longer arbitrarily restrict bus to be slave xor master.
  - Pulled out "struct aspeed_i2c_controller" as a interrupt controller.
  - Pulled out slave support into its own commit.
  - Rewrote code that sets clock divider register because the original version
    set it incorrectly.
  - Rewrote the aspeed_i2c_master_irq handler because the old method of
    completing a completion in between restarts was too slow causing devices to
    misbehave.
  - Added support for I2C_M_RECV_LEN which I had incorrectly said was supported
    before.
  - Addressed other comments from Vladimir.
Changes for v7:
  - Changed clock-frequency to bus-frequency
  - Made some fixes to clock divider code
  - Added hardware reset function
  - Marked functions that need to be called with the lock held as "unlocked"
  - Did a bunch of clean up
Changes for v8:
  - ACK IRQ status bits before doing anything else
  - Added multi-master device tree property
  - Do not send STOP commands after interrupt errors
  - Fix SMBUS_QUICK emulation handling
  - Removed highspeed clock code (I will do it in a later patch set)
  - Use the platform_device name for the adapter name
  - Reset for all failed recoveries
  - Removed the "__" prefix from all of the non-thread safe functions
Changes for v9:
  - No longer debug log after every address NACK
  - Clear errors after reset
  - No longer access clock while holding lock
  - Ack all interrupts durring bus reset and initialization
Changes for v10:
  - Added master_xfer_result field to struct aspeed_i2c_bus
Changes for v11:
  - Added support for COMPILE_TEST
  - Fixed i2c fault codes in interrupt handler
  - Use i2c_{get|set}_adapdata instead of adapter->algo_data
  - Fixed some other stylistic issues
---
 drivers/i2c/busses/Kconfig      |  10 +
 drivers/i2c/busses/Makefile     |   1 +
 drivers/i2c/busses/i2c-aspeed.c | 690 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 701 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-aspeed.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 144cbadc7c72..6cb893a0911b 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -326,6 +326,16 @@ config I2C_POWERMAC
 
 comment "I2C system bus drivers (mostly embedded / system-on-chip)"
 
+config I2C_ASPEED
+	tristate "Aspeed I2C Controller"
+	depends on ARCH_ASPEED || COMPILE_TEST
+	help
+	  If you say yes to this option, support will be included for the
+	  Aspeed I2C controller.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-aspeed.
+
 config I2C_AT91
 	tristate "Atmel AT91 I2C Two-Wire interface (TWI)"
 	depends on ARCH_AT91
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 30b60855fbcd..e84604b9bf3b 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_I2C_HYDRA)		+= i2c-hydra.o
 obj-$(CONFIG_I2C_POWERMAC)	+= i2c-powermac.o
 
 # Embedded system I2C/SMBus host controller drivers
+obj-$(CONFIG_I2C_ASPEED)	+= i2c-aspeed.o
 obj-$(CONFIG_I2C_AT91)		+= i2c-at91.o
 obj-$(CONFIG_I2C_AU1550)	+= i2c-au1550.o
 obj-$(CONFIG_I2C_AXXIA)		+= i2c-axxia.o
diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
new file mode 100644
index 000000000000..55f241f2cfcc
--- /dev/null
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -0,0 +1,690 @@
+/*
+ *  Aspeed 24XX/25XX I2C Controller.
+ *
+ *  Copyright (C) 2012-2017 ASPEED Technology Inc.
+ *  Copyright 2017 IBM Corporation
+ *  Copyright 2017 Google, Inc.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+/* I2C Register */
+#define ASPEED_I2C_FUN_CTRL_REG				0x00
+#define ASPEED_I2C_AC_TIMING_REG1			0x04
+#define ASPEED_I2C_AC_TIMING_REG2			0x08
+#define ASPEED_I2C_INTR_CTRL_REG			0x0c
+#define ASPEED_I2C_INTR_STS_REG				0x10
+#define ASPEED_I2C_CMD_REG				0x14
+#define ASPEED_I2C_DEV_ADDR_REG				0x18
+#define ASPEED_I2C_BYTE_BUF_REG				0x20
+
+/* Global Register Definition */
+/* 0x00 : I2C Interrupt Status Register  */
+/* 0x08 : I2C Interrupt Target Assignment  */
+
+/* Device Register Definition */
+/* 0x00 : I2CD Function Control Register  */
+#define ASPEED_I2CD_MULTI_MASTER_DIS			BIT(15)
+#define ASPEED_I2CD_SDA_DRIVE_1T_EN			BIT(8)
+#define ASPEED_I2CD_M_SDA_DRIVE_1T_EN			BIT(7)
+#define ASPEED_I2CD_M_HIGH_SPEED_EN			BIT(6)
+#define ASPEED_I2CD_MASTER_EN				BIT(0)
+
+/* 0x04 : I2CD Clock and AC Timing Control Register #1 */
+#define ASPEED_I2CD_TIME_SCL_HIGH_SHIFT			16
+#define ASPEED_I2CD_TIME_SCL_HIGH_MASK			GENMASK(19, 16)
+#define ASPEED_I2CD_TIME_SCL_LOW_SHIFT			12
+#define ASPEED_I2CD_TIME_SCL_LOW_MASK			GENMASK(15, 12)
+#define ASPEED_I2CD_TIME_BASE_DIVISOR_MASK		GENMASK(3, 0)
+#define ASPEED_I2CD_TIME_SCL_REG_MAX			GENMASK(3, 0)
+/* 0x08 : I2CD Clock and AC Timing Control Register #2 */
+#define ASPEED_NO_TIMEOUT_CTRL				0
+
+/* 0x0c : I2CD Interrupt Control Register &
+ * 0x10 : I2CD Interrupt Status Register
+ *
+ * These share bit definitions, so use the same values for the enable &
+ * status bits.
+ */
+#define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT			BIT(14)
+#define ASPEED_I2CD_INTR_BUS_RECOVER_DONE		BIT(13)
+#define ASPEED_I2CD_INTR_SCL_TIMEOUT			BIT(6)
+#define ASPEED_I2CD_INTR_ABNORMAL			BIT(5)
+#define ASPEED_I2CD_INTR_NORMAL_STOP			BIT(4)
+#define ASPEED_I2CD_INTR_ARBIT_LOSS			BIT(3)
+#define ASPEED_I2CD_INTR_RX_DONE			BIT(2)
+#define ASPEED_I2CD_INTR_TX_NAK				BIT(1)
+#define ASPEED_I2CD_INTR_TX_ACK				BIT(0)
+#define ASPEED_I2CD_INTR_ALL						       \
+		(ASPEED_I2CD_INTR_SDA_DL_TIMEOUT |			       \
+		 ASPEED_I2CD_INTR_BUS_RECOVER_DONE |			       \
+		 ASPEED_I2CD_INTR_SCL_TIMEOUT |				       \
+		 ASPEED_I2CD_INTR_ABNORMAL |				       \
+		 ASPEED_I2CD_INTR_NORMAL_STOP |				       \
+		 ASPEED_I2CD_INTR_ARBIT_LOSS |				       \
+		 ASPEED_I2CD_INTR_RX_DONE |				       \
+		 ASPEED_I2CD_INTR_TX_NAK |				       \
+		 ASPEED_I2CD_INTR_TX_ACK)
+
+/* 0x14 : I2CD Command/Status Register   */
+#define ASPEED_I2CD_SCL_LINE_STS			BIT(18)
+#define ASPEED_I2CD_SDA_LINE_STS			BIT(17)
+#define ASPEED_I2CD_BUS_BUSY_STS			BIT(16)
+#define ASPEED_I2CD_BUS_RECOVER_CMD			BIT(11)
+
+/* Command Bit */
+#define ASPEED_I2CD_M_STOP_CMD				BIT(5)
+#define ASPEED_I2CD_M_S_RX_CMD_LAST			BIT(4)
+#define ASPEED_I2CD_M_RX_CMD				BIT(3)
+#define ASPEED_I2CD_S_TX_CMD				BIT(2)
+#define ASPEED_I2CD_M_TX_CMD				BIT(1)
+#define ASPEED_I2CD_M_START_CMD				BIT(0)
+
+enum aspeed_i2c_master_state {
+	ASPEED_I2C_MASTER_START,
+	ASPEED_I2C_MASTER_TX_FIRST,
+	ASPEED_I2C_MASTER_TX,
+	ASPEED_I2C_MASTER_RX_FIRST,
+	ASPEED_I2C_MASTER_RX,
+	ASPEED_I2C_MASTER_STOP,
+	ASPEED_I2C_MASTER_INACTIVE,
+};
+
+struct aspeed_i2c_bus {
+	struct i2c_adapter		adap;
+	struct device			*dev;
+	void __iomem			*base;
+	/* Synchronizes I/O mem access to base. */
+	spinlock_t			lock;
+	struct completion		cmd_complete;
+	unsigned long			parent_clk_frequency;
+	u32				bus_frequency;
+	/* Transaction state. */
+	enum aspeed_i2c_master_state	master_state;
+	struct i2c_msg			*msgs;
+	size_t				buf_index;
+	size_t				msgs_index;
+	size_t				msgs_count;
+	bool				send_stop;
+	int				cmd_err;
+	/* Protected only by i2c_lock_bus */
+	int				master_xfer_result;
+};
+
+static int aspeed_i2c_reset(struct aspeed_i2c_bus *bus);
+
+static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
+{
+	unsigned long time_left, flags;
+	int ret = 0;
+	u32 command;
+
+	spin_lock_irqsave(&bus->lock, flags);
+	command = readl(bus->base + ASPEED_I2C_CMD_REG);
+
+	if (command & ASPEED_I2CD_SDA_LINE_STS) {
+		/* Bus is idle: no recovery needed. */
+		if (command & ASPEED_I2CD_SCL_LINE_STS)
+			goto out;
+		dev_dbg(bus->dev, "SCL hung (state %x), attempting recovery\n",
+			command);
+
+		reinit_completion(&bus->cmd_complete);
+		writel(ASPEED_I2CD_M_STOP_CMD, bus->base + ASPEED_I2C_CMD_REG);
+		spin_unlock_irqrestore(&bus->lock, flags);
+
+		time_left = wait_for_completion_timeout(
+				&bus->cmd_complete, bus->adap.timeout);
+
+		spin_lock_irqsave(&bus->lock, flags);
+		if (time_left == 0)
+			goto reset_out;
+		else if (bus->cmd_err)
+			goto reset_out;
+		/* Recovery failed. */
+		else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
+			   ASPEED_I2CD_SCL_LINE_STS))
+			goto reset_out;
+	/* Bus error. */
+	} else {
+		dev_dbg(bus->dev, "SDA hung (state %x), attempting recovery\n",
+			command);
+
+		reinit_completion(&bus->cmd_complete);
+		/* Writes 1 to 8 SCL clock cycles until SDA is released. */
+		writel(ASPEED_I2CD_BUS_RECOVER_CMD,
+		       bus->base + ASPEED_I2C_CMD_REG);
+		spin_unlock_irqrestore(&bus->lock, flags);
+
+		time_left = wait_for_completion_timeout(
+				&bus->cmd_complete, bus->adap.timeout);
+
+		spin_lock_irqsave(&bus->lock, flags);
+		if (time_left == 0)
+			goto reset_out;
+		else if (bus->cmd_err)
+			goto reset_out;
+		/* Recovery failed. */
+		else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
+			   ASPEED_I2CD_SDA_LINE_STS))
+			goto reset_out;
+	}
+
+out:
+	spin_unlock_irqrestore(&bus->lock, flags);
+
+	return ret;
+
+reset_out:
+	spin_unlock_irqrestore(&bus->lock, flags);
+
+	return aspeed_i2c_reset(bus);
+}
+
+/* precondition: bus.lock has been acquired. */
+static void aspeed_i2c_do_start(struct aspeed_i2c_bus *bus)
+{
+	u32 command = ASPEED_I2CD_M_START_CMD | ASPEED_I2CD_M_TX_CMD;
+	struct i2c_msg *msg = &bus->msgs[bus->msgs_index];
+	u8 slave_addr = msg->addr << 1;
+
+	bus->master_state = ASPEED_I2C_MASTER_START;
+	bus->buf_index = 0;
+
+	if (msg->flags & I2C_M_RD) {
+		slave_addr |= 1;
+		command |= ASPEED_I2CD_M_RX_CMD;
+		/* Need to let the hardware know to NACK after RX. */
+		if (msg->len == 1 && !(msg->flags & I2C_M_RECV_LEN))
+			command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
+	}
+
+	writel(slave_addr, bus->base + ASPEED_I2C_BYTE_BUF_REG);
+	writel(command, bus->base + ASPEED_I2C_CMD_REG);
+}
+
+/* precondition: bus.lock has been acquired. */
+static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus)
+{
+	bus->master_state = ASPEED_I2C_MASTER_STOP;
+	writel(ASPEED_I2CD_M_STOP_CMD, bus->base + ASPEED_I2C_CMD_REG);
+}
+
+/* precondition: bus.lock has been acquired. */
+static void aspeed_i2c_next_msg_or_stop(struct aspeed_i2c_bus *bus)
+{
+	if (bus->msgs_index + 1 < bus->msgs_count) {
+		bus->msgs_index++;
+		aspeed_i2c_do_start(bus);
+	} else {
+		aspeed_i2c_do_stop(bus);
+	}
+}
+
+static int aspeed_i2c_is_irq_error(u32 irq_status)
+{
+	if (irq_status & ASPEED_I2CD_INTR_ARBIT_LOSS)
+		return -EAGAIN;
+	if (irq_status & (ASPEED_I2CD_INTR_SDA_DL_TIMEOUT |
+			  ASPEED_I2CD_INTR_SCL_TIMEOUT))
+		return -EBUSY;
+	if (irq_status & (ASPEED_I2CD_INTR_ABNORMAL))
+		return -EPROTO;
+
+	return 0;
+}
+
+static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
+{
+	u32 irq_status, status_ack = 0, command = 0;
+	struct i2c_msg *msg;
+	u8 recv_byte;
+	int ret;
+
+	spin_lock(&bus->lock);
+	irq_status = readl(bus->base + ASPEED_I2C_INTR_STS_REG);
+	/* Ack all interrupt bits. */
+	writel(irq_status, bus->base + ASPEED_I2C_INTR_STS_REG);
+
+	if (irq_status & ASPEED_I2CD_INTR_BUS_RECOVER_DONE) {
+		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
+		status_ack |= ASPEED_I2CD_INTR_BUS_RECOVER_DONE;
+		goto out_complete;
+	}
+
+	/*
+	 * We encountered an interrupt that reports an error: the hardware
+	 * should clear the command queue effectively taking us back to the
+	 * INACTIVE state.
+	 */
+	ret = aspeed_i2c_is_irq_error(irq_status);
+	if (ret < 0) {
+		dev_dbg(bus->dev, "received error interrupt: 0x%08x",
+			irq_status);
+		bus->cmd_err = ret;
+		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
+		goto out_complete;
+	}
+
+	/* We are in an invalid state; reset bus to a known state. */
+	if (!bus->msgs && bus->master_state != ASPEED_I2C_MASTER_STOP) {
+		dev_err(bus->dev, "bus in unknown state");
+		bus->cmd_err = -EIO;
+		aspeed_i2c_do_stop(bus);
+		goto out_no_complete;
+	}
+	msg = &bus->msgs[bus->msgs_index];
+
+	/*
+	 * START is a special case because we still have to handle a subsequent
+	 * TX or RX immediately after we handle it, so we handle it here and
+	 * then update the state and handle the new state below.
+	 */
+	if (bus->master_state == ASPEED_I2C_MASTER_START) {
+		if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
+			pr_devel("no slave present at %02x", msg->addr);
+			status_ack |= ASPEED_I2CD_INTR_TX_NAK;
+			bus->cmd_err = -ENXIO;
+			aspeed_i2c_do_stop(bus);
+			goto out_no_complete;
+		}
+		status_ack |= ASPEED_I2CD_INTR_TX_ACK;
+		if (msg->len == 0) { /* SMBUS_QUICK */
+			aspeed_i2c_do_stop(bus);
+			goto out_no_complete;
+		}
+		if (msg->flags & I2C_M_RD)
+			bus->master_state = ASPEED_I2C_MASTER_RX_FIRST;
+		else
+			bus->master_state = ASPEED_I2C_MASTER_TX_FIRST;
+	}
+
+	switch (bus->master_state) {
+	case ASPEED_I2C_MASTER_TX:
+		if (unlikely(irq_status & ASPEED_I2CD_INTR_TX_NAK)) {
+			dev_dbg(bus->dev, "slave NACKed TX");
+			status_ack |= ASPEED_I2CD_INTR_TX_NAK;
+			goto error_and_stop;
+		} else if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
+			dev_err(bus->dev, "slave failed to ACK TX");
+			goto error_and_stop;
+		}
+		status_ack |= ASPEED_I2CD_INTR_TX_ACK;
+		/* fallthrough intended */
+	case ASPEED_I2C_MASTER_TX_FIRST:
+		if (bus->buf_index < msg->len) {
+			bus->master_state = ASPEED_I2C_MASTER_TX;
+			writel(msg->buf[bus->buf_index++],
+			       bus->base + ASPEED_I2C_BYTE_BUF_REG);
+			writel(ASPEED_I2CD_M_TX_CMD,
+			       bus->base + ASPEED_I2C_CMD_REG);
+		} else {
+			aspeed_i2c_next_msg_or_stop(bus);
+		}
+		goto out_no_complete;
+	case ASPEED_I2C_MASTER_RX_FIRST:
+		/* RX may not have completed yet (only address cycle) */
+		if (!(irq_status & ASPEED_I2CD_INTR_RX_DONE))
+			goto out_no_complete;
+		/* fallthrough intended */
+	case ASPEED_I2C_MASTER_RX:
+		if (unlikely(!(irq_status & ASPEED_I2CD_INTR_RX_DONE))) {
+			dev_err(bus->dev, "master failed to RX");
+			goto error_and_stop;
+		}
+		status_ack |= ASPEED_I2CD_INTR_RX_DONE;
+
+		recv_byte = readl(bus->base + ASPEED_I2C_BYTE_BUF_REG) >> 8;
+		msg->buf[bus->buf_index++] = recv_byte;
+
+		if (msg->flags & I2C_M_RECV_LEN) {
+			if (unlikely(recv_byte > I2C_SMBUS_BLOCK_MAX)) {
+				bus->cmd_err = -EPROTO;
+				aspeed_i2c_do_stop(bus);
+				goto out_no_complete;
+			}
+			msg->len = recv_byte +
+					((msg->flags & I2C_CLIENT_PEC) ? 2 : 1);
+			msg->flags &= ~I2C_M_RECV_LEN;
+		}
+
+		if (bus->buf_index < msg->len) {
+			bus->master_state = ASPEED_I2C_MASTER_RX;
+			command = ASPEED_I2CD_M_RX_CMD;
+			if (bus->buf_index + 1 == msg->len)
+				command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
+			writel(command, bus->base + ASPEED_I2C_CMD_REG);
+		} else {
+			aspeed_i2c_next_msg_or_stop(bus);
+		}
+		goto out_no_complete;
+	case ASPEED_I2C_MASTER_STOP:
+		if (unlikely(!(irq_status & ASPEED_I2CD_INTR_NORMAL_STOP))) {
+			dev_err(bus->dev, "master failed to STOP");
+			bus->cmd_err = -EIO;
+			/* Do not STOP as we have already tried. */
+		} else {
+			status_ack |= ASPEED_I2CD_INTR_NORMAL_STOP;
+		}
+
+		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
+		goto out_complete;
+	case ASPEED_I2C_MASTER_INACTIVE:
+		dev_err(bus->dev,
+			"master received interrupt 0x%08x, but is inactive",
+			irq_status);
+		bus->cmd_err = -EIO;
+		/* Do not STOP as we should be inactive. */
+		goto out_complete;
+	default:
+		WARN(1, "unknown master state\n");
+		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
+		bus->cmd_err = -EINVAL;
+		goto out_complete;
+	}
+error_and_stop:
+	bus->cmd_err = -EIO;
+	aspeed_i2c_do_stop(bus);
+	goto out_no_complete;
+out_complete:
+	bus->msgs = NULL;
+	if (bus->cmd_err)
+		bus->master_xfer_result = bus->cmd_err;
+	else
+		bus->master_xfer_result = bus->msgs_index + 1;
+	complete(&bus->cmd_complete);
+out_no_complete:
+	if (irq_status != status_ack)
+		dev_err(bus->dev,
+			"irq handled != irq. expected 0x%08x, but was 0x%08x\n",
+			irq_status, status_ack);
+	spin_unlock(&bus->lock);
+	return !!irq_status;
+}
+
+static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
+{
+	struct aspeed_i2c_bus *bus = dev_id;
+
+	return aspeed_i2c_master_irq(bus) ? IRQ_HANDLED : IRQ_NONE;
+}
+
+static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
+				  struct i2c_msg *msgs, int num)
+{
+	struct aspeed_i2c_bus *bus = i2c_get_adapdata(adap);
+	unsigned long time_left, flags;
+	int ret = 0;
+
+	spin_lock_irqsave(&bus->lock, flags);
+	bus->cmd_err = 0;
+
+	/* If bus is busy, attempt recovery. We assume a single master
+	 * environment.
+	 */
+	if (readl(bus->base + ASPEED_I2C_CMD_REG) & ASPEED_I2CD_BUS_BUSY_STS) {
+		spin_unlock_irqrestore(&bus->lock, flags);
+		ret = aspeed_i2c_recover_bus(bus);
+		if (ret)
+			return ret;
+		spin_lock_irqsave(&bus->lock, flags);
+	}
+
+	bus->cmd_err = 0;
+	bus->msgs = msgs;
+	bus->msgs_index = 0;
+	bus->msgs_count = num;
+
+	reinit_completion(&bus->cmd_complete);
+	aspeed_i2c_do_start(bus);
+	spin_unlock_irqrestore(&bus->lock, flags);
+
+	time_left = wait_for_completion_timeout(&bus->cmd_complete,
+						bus->adap.timeout);
+
+	if (time_left == 0)
+		return -ETIMEDOUT;
+	else
+		return bus->master_xfer_result;
+}
+
+static u32 aspeed_i2c_functionality(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
+}
+
+static const struct i2c_algorithm aspeed_i2c_algo = {
+	.master_xfer	= aspeed_i2c_master_xfer,
+	.functionality	= aspeed_i2c_functionality,
+};
+
+static u32 aspeed_i2c_get_clk_reg_val(u32 divisor)
+{
+	u32 base_clk, clk_high, clk_low, tmp;
+
+	/*
+	 * The actual clock frequency of SCL is:
+	 *	SCL_freq = APB_freq / (base_freq * (SCL_high + SCL_low))
+	 *		 = APB_freq / divisor
+	 * where base_freq is a programmable clock divider; its value is
+	 *	base_freq = 1 << base_clk
+	 * SCL_high is the number of base_freq clock cycles that SCL stays high
+	 * and SCL_low is the number of base_freq clock cycles that SCL stays
+	 * low for a period of SCL.
+	 * The actual register has a minimum SCL_high and SCL_low minimum of 1;
+	 * thus, they start counting at zero. So
+	 *	SCL_high = clk_high + 1
+	 *	SCL_low	 = clk_low + 1
+	 * Thus,
+	 *	SCL_freq = APB_freq /
+	 *		((1 << base_clk) * (clk_high + 1 + clk_low + 1))
+	 * The documentation recommends clk_high >= 8 and clk_low >= 7 when
+	 * possible; this last constraint gives us the following solution:
+	 */
+	base_clk = divisor > 33 ? ilog2((divisor - 1) / 32) + 1 : 0;
+	tmp = divisor / (1 << base_clk);
+	clk_high = tmp / 2 + tmp % 2;
+	clk_low = tmp - clk_high;
+
+	clk_high -= 1;
+	clk_low -= 1;
+
+	return ((clk_high << ASPEED_I2CD_TIME_SCL_HIGH_SHIFT)
+		& ASPEED_I2CD_TIME_SCL_HIGH_MASK)
+			| ((clk_low << ASPEED_I2CD_TIME_SCL_LOW_SHIFT)
+			   & ASPEED_I2CD_TIME_SCL_LOW_MASK)
+			| (base_clk & ASPEED_I2CD_TIME_BASE_DIVISOR_MASK);
+}
+
+/* precondition: bus.lock has been acquired. */
+static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus)
+{
+	u32 divisor, clk_reg_val;
+
+	divisor = bus->parent_clk_frequency / bus->bus_frequency;
+	clk_reg_val = aspeed_i2c_get_clk_reg_val(divisor);
+	writel(clk_reg_val, bus->base + ASPEED_I2C_AC_TIMING_REG1);
+	writel(ASPEED_NO_TIMEOUT_CTRL, bus->base + ASPEED_I2C_AC_TIMING_REG2);
+
+	return 0;
+}
+
+/* precondition: bus.lock has been acquired. */
+static int aspeed_i2c_init(struct aspeed_i2c_bus *bus,
+			     struct platform_device *pdev)
+{
+	u32 fun_ctrl_reg = ASPEED_I2CD_MASTER_EN;
+	int ret;
+
+	/* Disable everything. */
+	writel(0, bus->base + ASPEED_I2C_FUN_CTRL_REG);
+
+	ret = aspeed_i2c_init_clk(bus);
+	if (ret < 0)
+		return ret;
+
+	if (!of_property_read_bool(pdev->dev.of_node, "multi-master"))
+		fun_ctrl_reg |= ASPEED_I2CD_MULTI_MASTER_DIS;
+
+	/* Enable Master Mode */
+	writel(readl(bus->base + ASPEED_I2C_FUN_CTRL_REG) | fun_ctrl_reg,
+	       bus->base + ASPEED_I2C_FUN_CTRL_REG);
+
+	/* Set interrupt generation of I2C controller */
+	writel(ASPEED_I2CD_INTR_ALL, bus->base + ASPEED_I2C_INTR_CTRL_REG);
+
+	return 0;
+}
+
+static int aspeed_i2c_reset(struct aspeed_i2c_bus *bus)
+{
+	struct platform_device *pdev = to_platform_device(bus->dev);
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&bus->lock, flags);
+
+	/* Disable and ack all interrupts. */
+	writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
+	writel(0xffffffff, bus->base + ASPEED_I2C_INTR_STS_REG);
+
+	ret = aspeed_i2c_init(bus, pdev);
+
+	spin_unlock_irqrestore(&bus->lock, flags);
+
+	return ret;
+}
+
+static int aspeed_i2c_probe_bus(struct platform_device *pdev)
+{
+	struct aspeed_i2c_bus *bus;
+	struct clk *parent_clk;
+	struct resource *res;
+	int irq, ret;
+
+	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
+	if (!bus)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	bus->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(bus->base))
+		return PTR_ERR(bus->base);
+
+	parent_clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(parent_clk))
+		return PTR_ERR(parent_clk);
+	bus->parent_clk_frequency = clk_get_rate(parent_clk);
+	/* We just need the clock rate, we don't actually use the clk object. */
+	devm_clk_put(&pdev->dev, parent_clk);
+
+	ret = of_property_read_u32(pdev->dev.of_node,
+				   "bus-frequency", &bus->bus_frequency);
+	if (ret < 0) {
+		dev_err(&pdev->dev,
+			"Could not read bus-frequency property\n");
+		bus->bus_frequency = 100000;
+	}
+
+	/* Initialize the I2C adapter */
+	spin_lock_init(&bus->lock);
+	init_completion(&bus->cmd_complete);
+	bus->adap.owner = THIS_MODULE;
+	bus->adap.retries = 0;
+	bus->adap.timeout = 5 * HZ;
+	bus->adap.algo = &aspeed_i2c_algo;
+	bus->adap.dev.parent = &pdev->dev;
+	bus->adap.dev.of_node = pdev->dev.of_node;
+	strlcpy(bus->adap.name, pdev->name, sizeof(bus->adap.name));
+	i2c_set_adapdata(&bus->adap, bus);
+
+	bus->dev = &pdev->dev;
+
+	/* Clean up any left over interrupt state. */
+	writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
+	writel(0xffffffff, bus->base + ASPEED_I2C_INTR_STS_REG);
+	/*
+	 * bus.lock does not need to be held because the interrupt handler has
+	 * not been enabled yet.
+	 */
+	ret = aspeed_i2c_init(bus, pdev);
+	if (ret < 0)
+		return ret;
+
+	irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+	ret = devm_request_irq(&pdev->dev, irq, aspeed_i2c_bus_irq,
+			       0, dev_name(&pdev->dev), bus);
+	if (ret < 0)
+		return ret;
+
+	ret = i2c_add_adapter(&bus->adap);
+	if (ret < 0)
+		return ret;
+
+	platform_set_drvdata(pdev, bus);
+
+	dev_info(bus->dev, "i2c bus %d registered, irq %d\n",
+		 bus->adap.nr, irq);
+
+	return 0;
+}
+
+static int aspeed_i2c_remove_bus(struct platform_device *pdev)
+{
+	struct aspeed_i2c_bus *bus = platform_get_drvdata(pdev);
+	unsigned long flags;
+
+	spin_lock_irqsave(&bus->lock, flags);
+
+	/* Disable everything. */
+	writel(0, bus->base + ASPEED_I2C_FUN_CTRL_REG);
+	writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
+
+	spin_unlock_irqrestore(&bus->lock, flags);
+
+	i2c_del_adapter(&bus->adap);
+
+	return 0;
+}
+
+static const struct of_device_id aspeed_i2c_bus_of_table[] = {
+	{ .compatible = "aspeed,ast2400-i2c-bus", },
+	{ .compatible = "aspeed,ast2500-i2c-bus", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, aspeed_i2c_bus_of_table);
+
+static struct platform_driver aspeed_i2c_bus_driver = {
+	.probe		= aspeed_i2c_probe_bus,
+	.remove		= aspeed_i2c_remove_bus,
+	.driver		= {
+		.name		= "aspeed-i2c-bus",
+		.of_match_table	= aspeed_i2c_bus_of_table,
+	},
+};
+module_platform_driver(aspeed_i2c_bus_driver);
+
+MODULE_AUTHOR("Brendan Higgins <brendanhiggins@google.com>");
+MODULE_DESCRIPTION("Aspeed I2C Bus Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.13.1.611.g7e3b11ae1-goog

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

* [PATCH v11 4/4] i2c: aspeed: added slave support for Aspeed I2C driver
  2017-06-20 21:15 [PATCH v11 0/4] i2c: aspeed: added driver for Aspeed I2C Brendan Higgins
                   ` (2 preceding siblings ...)
  2017-06-20 21:15 ` [PATCH v11 3/4] i2c: aspeed: added driver for Aspeed I2C Brendan Higgins
@ 2017-06-20 21:15 ` Brendan Higgins
  2017-06-23 18:41   ` Wolfram Sang
  3 siblings, 1 reply; 17+ messages in thread
From: Brendan Higgins @ 2017-06-20 21:15 UTC (permalink / raw)
  To: wsa, robh+dt, mark.rutland, gregkh, davem, mchehab, joel,
	martin.petersen, benh
  Cc: linux-i2c, devicetree, linux-kernel, openbmc, Brendan Higgins

Added slave support for Aspeed I2C controller. Supports fourteen busses
present in AST24XX and AST25XX BMC SoCs by Aspeed.

Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
Added in v6:
  - Pulled slave support out of initial driver commit into its own commit.
  - No longer arbitrarily restrict bus to be slave xor master.
Changes for v7:
  - Added hardware reset function
  - Marked functions that need to be called with the lock held as "unlocked"
  - Did some cleanup
Changes for v8:
  - None
Changes for v9:
  - None
Changes for v10:
  - None
Changes for v11:
  - Use i2c_{get|set}_adapdata instead of adapter->algo_data
---
 drivers/i2c/busses/i2c-aspeed.c | 201 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 201 insertions(+)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 55f241f2cfcc..f19348328a71 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -49,6 +49,7 @@
 #define ASPEED_I2CD_SDA_DRIVE_1T_EN			BIT(8)
 #define ASPEED_I2CD_M_SDA_DRIVE_1T_EN			BIT(7)
 #define ASPEED_I2CD_M_HIGH_SPEED_EN			BIT(6)
+#define ASPEED_I2CD_SLAVE_EN				BIT(1)
 #define ASPEED_I2CD_MASTER_EN				BIT(0)
 
 /* 0x04 : I2CD Clock and AC Timing Control Register #1 */
@@ -69,6 +70,7 @@
  */
 #define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT			BIT(14)
 #define ASPEED_I2CD_INTR_BUS_RECOVER_DONE		BIT(13)
+#define ASPEED_I2CD_INTR_SLAVE_MATCH			BIT(7)
 #define ASPEED_I2CD_INTR_SCL_TIMEOUT			BIT(6)
 #define ASPEED_I2CD_INTR_ABNORMAL			BIT(5)
 #define ASPEED_I2CD_INTR_NORMAL_STOP			BIT(4)
@@ -101,6 +103,9 @@
 #define ASPEED_I2CD_M_TX_CMD				BIT(1)
 #define ASPEED_I2CD_M_START_CMD				BIT(0)
 
+/* 0x18 : I2CD Slave Device Address Register   */
+#define ASPEED_I2CD_DEV_ADDR_MASK			GENMASK(6, 0)
+
 enum aspeed_i2c_master_state {
 	ASPEED_I2C_MASTER_START,
 	ASPEED_I2C_MASTER_TX_FIRST,
@@ -111,6 +116,15 @@ enum aspeed_i2c_master_state {
 	ASPEED_I2C_MASTER_INACTIVE,
 };
 
+enum aspeed_i2c_slave_state {
+	ASPEED_I2C_SLAVE_START,
+	ASPEED_I2C_SLAVE_READ_REQUESTED,
+	ASPEED_I2C_SLAVE_READ_PROCESSED,
+	ASPEED_I2C_SLAVE_WRITE_REQUESTED,
+	ASPEED_I2C_SLAVE_WRITE_RECEIVED,
+	ASPEED_I2C_SLAVE_STOP,
+};
+
 struct aspeed_i2c_bus {
 	struct i2c_adapter		adap;
 	struct device			*dev;
@@ -130,6 +144,10 @@ struct aspeed_i2c_bus {
 	int				cmd_err;
 	/* Protected only by i2c_lock_bus */
 	int				master_xfer_result;
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	struct i2c_client		*slave;
+	enum aspeed_i2c_slave_state	slave_state;
+#endif /* CONFIG_I2C_SLAVE */
 };
 
 static int aspeed_i2c_reset(struct aspeed_i2c_bus *bus);
@@ -202,6 +220,110 @@ static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
 	return aspeed_i2c_reset(bus);
 }
 
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus)
+{
+	u32 command, irq_status, status_ack = 0;
+	struct i2c_client *slave = bus->slave;
+	bool irq_handled = true;
+	u8 value;
+
+	spin_lock(&bus->lock);
+	if (!slave) {
+		irq_handled = false;
+		goto out;
+	}
+
+	command = readl(bus->base + ASPEED_I2C_CMD_REG);
+	irq_status = readl(bus->base + ASPEED_I2C_INTR_STS_REG);
+
+	/* Slave was requested, restart state machine. */
+	if (irq_status & ASPEED_I2CD_INTR_SLAVE_MATCH) {
+		status_ack |= ASPEED_I2CD_INTR_SLAVE_MATCH;
+		bus->slave_state = ASPEED_I2C_SLAVE_START;
+	}
+
+	/* Slave is not currently active, irq was for someone else. */
+	if (bus->slave_state == ASPEED_I2C_SLAVE_STOP) {
+		irq_handled = false;
+		goto out;
+	}
+
+	dev_dbg(bus->dev, "slave irq status 0x%08x, cmd 0x%08x\n",
+		irq_status, command);
+
+	/* Slave was sent something. */
+	if (irq_status & ASPEED_I2CD_INTR_RX_DONE) {
+		value = readl(bus->base + ASPEED_I2C_BYTE_BUF_REG) >> 8;
+		/* Handle address frame. */
+		if (bus->slave_state == ASPEED_I2C_SLAVE_START) {
+			if (value & 0x1)
+				bus->slave_state =
+						ASPEED_I2C_SLAVE_READ_REQUESTED;
+			else
+				bus->slave_state =
+						ASPEED_I2C_SLAVE_WRITE_REQUESTED;
+		}
+		status_ack |= ASPEED_I2CD_INTR_RX_DONE;
+	}
+
+	/* Slave was asked to stop. */
+	if (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) {
+		status_ack |= ASPEED_I2CD_INTR_NORMAL_STOP;
+		bus->slave_state = ASPEED_I2C_SLAVE_STOP;
+	}
+	if (irq_status & ASPEED_I2CD_INTR_TX_NAK) {
+		status_ack |= ASPEED_I2CD_INTR_TX_NAK;
+		bus->slave_state = ASPEED_I2C_SLAVE_STOP;
+	}
+
+	switch (bus->slave_state) {
+	case ASPEED_I2C_SLAVE_READ_REQUESTED:
+		if (irq_status & ASPEED_I2CD_INTR_TX_ACK)
+			dev_err(bus->dev, "Unexpected ACK on read request.\n");
+		bus->slave_state = ASPEED_I2C_SLAVE_READ_PROCESSED;
+
+		i2c_slave_event(slave, I2C_SLAVE_READ_REQUESTED, &value);
+		writel(value, bus->base + ASPEED_I2C_BYTE_BUF_REG);
+		writel(ASPEED_I2CD_S_TX_CMD, bus->base + ASPEED_I2C_CMD_REG);
+		break;
+	case ASPEED_I2C_SLAVE_READ_PROCESSED:
+		status_ack |= ASPEED_I2CD_INTR_TX_ACK;
+		if (!(irq_status & ASPEED_I2CD_INTR_TX_ACK))
+			dev_err(bus->dev,
+				"Expected ACK after processed read.\n");
+		i2c_slave_event(slave, I2C_SLAVE_READ_PROCESSED, &value);
+		writel(value, bus->base + ASPEED_I2C_BYTE_BUF_REG);
+		writel(ASPEED_I2CD_S_TX_CMD, bus->base + ASPEED_I2C_CMD_REG);
+		break;
+	case ASPEED_I2C_SLAVE_WRITE_REQUESTED:
+		bus->slave_state = ASPEED_I2C_SLAVE_WRITE_RECEIVED;
+		i2c_slave_event(slave, I2C_SLAVE_WRITE_REQUESTED, &value);
+		break;
+	case ASPEED_I2C_SLAVE_WRITE_RECEIVED:
+		i2c_slave_event(slave, I2C_SLAVE_WRITE_RECEIVED, &value);
+		break;
+	case ASPEED_I2C_SLAVE_STOP:
+		i2c_slave_event(slave, I2C_SLAVE_STOP, &value);
+		break;
+	default:
+		dev_err(bus->dev, "unhandled slave_state: %d\n",
+			bus->slave_state);
+		break;
+	}
+
+	if (status_ack != irq_status)
+		dev_err(bus->dev,
+			"irq handled != irq. expected %x, but was %x\n",
+			irq_status, status_ack);
+	writel(status_ack, bus->base + ASPEED_I2C_INTR_STS_REG);
+
+out:
+	spin_unlock(&bus->lock);
+	return irq_handled;
+}
+#endif /* CONFIG_I2C_SLAVE */
+
 /* precondition: bus.lock has been acquired. */
 static void aspeed_i2c_do_start(struct aspeed_i2c_bus *bus)
 {
@@ -427,6 +549,13 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
 {
 	struct aspeed_i2c_bus *bus = dev_id;
 
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	if (aspeed_i2c_slave_irq(bus)) {
+		dev_dbg(bus->dev, "irq handled by slave.\n");
+		return IRQ_HANDLED;
+	}
+#endif /* CONFIG_I2C_SLAVE */
+
 	return aspeed_i2c_master_irq(bus) ? IRQ_HANDLED : IRQ_NONE;
 }
 
@@ -474,9 +603,75 @@ static u32 aspeed_i2c_functionality(struct i2c_adapter *adap)
 	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
 }
 
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+/* precondition: bus.lock has been acquired. */
+static void __aspeed_i2c_reg_slave(struct aspeed_i2c_bus *bus, u16 slave_addr)
+{
+	u32 addr_reg_val, func_ctrl_reg_val;
+
+	/* Set slave addr. */
+	addr_reg_val = readl(bus->base + ASPEED_I2C_DEV_ADDR_REG);
+	addr_reg_val &= ~ASPEED_I2CD_DEV_ADDR_MASK;
+	addr_reg_val |= slave_addr & ASPEED_I2CD_DEV_ADDR_MASK;
+	writel(addr_reg_val, bus->base + ASPEED_I2C_DEV_ADDR_REG);
+
+	/* Turn on slave mode. */
+	func_ctrl_reg_val = readl(bus->base + ASPEED_I2C_FUN_CTRL_REG);
+	func_ctrl_reg_val |= ASPEED_I2CD_SLAVE_EN;
+	writel(func_ctrl_reg_val, bus->base + ASPEED_I2C_FUN_CTRL_REG);
+}
+
+static int aspeed_i2c_reg_slave(struct i2c_client *client)
+{
+	struct aspeed_i2c_bus *bus = i2c_get_adapdata(client->adapter);
+	unsigned long flags;
+
+	spin_lock_irqsave(&bus->lock, flags);
+	if (bus->slave) {
+		spin_unlock_irqrestore(&bus->lock, flags);
+		return -EINVAL;
+	}
+
+	__aspeed_i2c_reg_slave(bus, client->addr);
+
+	bus->slave = client;
+	bus->slave_state = ASPEED_I2C_SLAVE_STOP;
+	spin_unlock_irqrestore(&bus->lock, flags);
+
+	return 0;
+}
+
+static int aspeed_i2c_unreg_slave(struct i2c_client *client)
+{
+	struct aspeed_i2c_bus *bus = i2c_get_adapdata(client->adapter);
+	u32 func_ctrl_reg_val;
+	unsigned long flags;
+
+	spin_lock_irqsave(&bus->lock, flags);
+	if (!bus->slave) {
+		spin_unlock_irqrestore(&bus->lock, flags);
+		return -EINVAL;
+	}
+
+	/* Turn off slave mode. */
+	func_ctrl_reg_val = readl(bus->base + ASPEED_I2C_FUN_CTRL_REG);
+	func_ctrl_reg_val &= ~ASPEED_I2CD_SLAVE_EN;
+	writel(func_ctrl_reg_val, bus->base + ASPEED_I2C_FUN_CTRL_REG);
+
+	bus->slave = NULL;
+	spin_unlock_irqrestore(&bus->lock, flags);
+
+	return 0;
+}
+#endif /* CONFIG_I2C_SLAVE */
+
 static const struct i2c_algorithm aspeed_i2c_algo = {
 	.master_xfer	= aspeed_i2c_master_xfer,
 	.functionality	= aspeed_i2c_functionality,
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	.reg_slave	= aspeed_i2c_reg_slave,
+	.unreg_slave	= aspeed_i2c_unreg_slave,
+#endif /* CONFIG_I2C_SLAVE */
 };
 
 static u32 aspeed_i2c_get_clk_reg_val(u32 divisor)
@@ -551,6 +746,12 @@ static int aspeed_i2c_init(struct aspeed_i2c_bus *bus,
 	writel(readl(bus->base + ASPEED_I2C_FUN_CTRL_REG) | fun_ctrl_reg,
 	       bus->base + ASPEED_I2C_FUN_CTRL_REG);
 
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	/* If slave has already been registered, re-enable it. */
+	if (bus->slave)
+		__aspeed_i2c_reg_slave(bus, bus->slave->addr);
+#endif /* CONFIG_I2C_SLAVE */
+
 	/* Set interrupt generation of I2C controller */
 	writel(ASPEED_I2CD_INTR_ALL, bus->base + ASPEED_I2C_INTR_CTRL_REG);
 
-- 
2.13.1.611.g7e3b11ae1-goog

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

* Re: [PATCH v11 1/4] MAINTAINERS: add entry for Aspeed I2C driver
  2017-06-20 21:15 ` [PATCH v11 1/4] MAINTAINERS: add entry for Aspeed I2C driver Brendan Higgins
@ 2017-06-23 18:40   ` Wolfram Sang
  0 siblings, 0 replies; 17+ messages in thread
From: Wolfram Sang @ 2017-06-23 18:40 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: robh+dt, mark.rutland, gregkh, davem, mchehab, joel,
	martin.petersen, benh, linux-i2c, devicetree, linux-kernel,
	openbmc

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

On Tue, Jun 20, 2017 at 02:15:13PM -0700, Brendan Higgins wrote:
> Added myself as maintainer of the Aspeed I2C driver and the associated
> I2C interrupt controller and added Joel Stanley and Benjamin
> Herrenschmidt as reviewers.
> 
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>

Applied to for-next, thanks!


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

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

* Re: [PATCH v11 2/4] i2c: aspeed: added documentation for Aspeed I2C driver
  2017-06-20 21:15 ` [PATCH v11 2/4] i2c: aspeed: added documentation " Brendan Higgins
@ 2017-06-23 18:41   ` Wolfram Sang
  0 siblings, 0 replies; 17+ messages in thread
From: Wolfram Sang @ 2017-06-23 18:41 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: robh+dt, mark.rutland, gregkh, davem, mchehab, joel,
	martin.petersen, benh, linux-i2c, devicetree, linux-kernel,
	openbmc

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

On Tue, Jun 20, 2017 at 02:15:14PM -0700, Brendan Higgins wrote:
> Added device tree binding documentation for Aspeed I2C busses.
> 
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> Acked-by: Rob Herring <robh@kernel.org>

Applied to for-next, thanks!


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

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

* Re: [PATCH v11 4/4] i2c: aspeed: added slave support for Aspeed I2C driver
  2017-06-20 21:15 ` [PATCH v11 4/4] i2c: aspeed: added slave support for Aspeed I2C driver Brendan Higgins
@ 2017-06-23 18:41   ` Wolfram Sang
  0 siblings, 0 replies; 17+ messages in thread
From: Wolfram Sang @ 2017-06-23 18:41 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: robh+dt, mark.rutland, gregkh, davem, mchehab, joel,
	martin.petersen, benh, linux-i2c, devicetree, linux-kernel,
	openbmc

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

On Tue, Jun 20, 2017 at 02:15:16PM -0700, Brendan Higgins wrote:
> Added slave support for Aspeed I2C controller. Supports fourteen busses
> present in AST24XX and AST25XX BMC SoCs by Aspeed.
> 
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>

Applied to for-next, thanks!


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

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

* Re: [PATCH v11 3/4] i2c: aspeed: added driver for Aspeed I2C
  2017-06-20 21:15 ` [PATCH v11 3/4] i2c: aspeed: added driver for Aspeed I2C Brendan Higgins
@ 2017-06-23 18:43   ` Wolfram Sang
  2017-06-26  6:18     ` Joel Stanley
  2017-06-27  8:28     ` Brendan Higgins
  0 siblings, 2 replies; 17+ messages in thread
From: Wolfram Sang @ 2017-06-23 18:43 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: robh+dt, mark.rutland, gregkh, davem, mchehab, joel,
	martin.petersen, benh, linux-i2c, devicetree, linux-kernel,
	openbmc

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

On Tue, Jun 20, 2017 at 02:15:15PM -0700, Brendan Higgins wrote:
> Added initial master support for Aspeed I2C controller. Supports
> fourteen busses present in AST24XX and AST25XX BMC SoCs by Aspeed.
> 
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>

Applied to for-next, thanks for all the hard work!

One question however which can be solved incrementally if needed:

> +	if (command & ASPEED_I2CD_SDA_LINE_STS) {
> +		/* Bus is idle: no recovery needed. */
> +		if (command & ASPEED_I2CD_SCL_LINE_STS)
> +			goto out;
> +		dev_dbg(bus->dev, "SCL hung (state %x), attempting recovery\n",
> +			command);
> +
> +		reinit_completion(&bus->cmd_complete);
> +		writel(ASPEED_I2CD_M_STOP_CMD, bus->base + ASPEED_I2C_CMD_REG);

If SCL is stuck low, how do you want to send a STOP?


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

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

* Re: [PATCH v11 3/4] i2c: aspeed: added driver for Aspeed I2C
  2017-06-23 18:43   ` Wolfram Sang
@ 2017-06-26  6:18     ` Joel Stanley
  2017-06-27  8:36       ` Brendan Higgins
  2017-06-27  8:28     ` Brendan Higgins
  1 sibling, 1 reply; 17+ messages in thread
From: Joel Stanley @ 2017-06-26  6:18 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Brendan Higgins, Rob Herring, Mark Rutland, Greg KH,
	David S . Miller, mchehab, martin.petersen,
	Benjamin Herrenschmidt, linux-i2c, devicetree,
	Linux Kernel Mailing List, OpenBMC Maillist, Rick Altherr

On Sat, Jun 24, 2017 at 4:13 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
> On Tue, Jun 20, 2017 at 02:15:15PM -0700, Brendan Higgins wrote:
>> Added initial master support for Aspeed I2C controller. Supports
>> fourteen busses present in AST24XX and AST25XX BMC SoCs by Aspeed.
>>
>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>
> Applied to for-next, thanks for all the hard work!

Congratulations Brendan. It's great to see this driver in the tree.

Cheers,

Joel

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

* Re: [PATCH v11 3/4] i2c: aspeed: added driver for Aspeed I2C
  2017-06-23 18:43   ` Wolfram Sang
  2017-06-26  6:18     ` Joel Stanley
@ 2017-06-27  8:28     ` Brendan Higgins
  2017-06-27  9:00       ` Wolfram Sang
  1 sibling, 1 reply; 17+ messages in thread
From: Brendan Higgins @ 2017-06-27  8:28 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Rob Herring, Mark Rutland, gregkh, davem, mchehab, Joel Stanley,
	martin.petersen, Benjamin Herrenschmidt, linux-i2c, devicetree,
	Linux Kernel Mailing List, OpenBMC Maillist

On Fri, Jun 23, 2017 at 11:43 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
> On Tue, Jun 20, 2017 at 02:15:15PM -0700, Brendan Higgins wrote:
>> Added initial master support for Aspeed I2C controller. Supports
>> fourteen busses present in AST24XX and AST25XX BMC SoCs by Aspeed.
>>
>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>
> Applied to for-next, thanks for all the hard work!

Thanks for the patience!

>
> One question however which can be solved incrementally if needed:
>
>> +     if (command & ASPEED_I2CD_SDA_LINE_STS) {
>> +             /* Bus is idle: no recovery needed. */
>> +             if (command & ASPEED_I2CD_SCL_LINE_STS)
>> +                     goto out;
>> +             dev_dbg(bus->dev, "SCL hung (state %x), attempting recovery\n",
>> +                     command);
>> +
>> +             reinit_completion(&bus->cmd_complete);
>> +             writel(ASPEED_I2CD_M_STOP_CMD, bus->base + ASPEED_I2C_CMD_REG);
>
> If SCL is stuck low, how do you want to send a STOP?
>

Fair point. I should probably drop that in the future and just do a
reset, and even then, doing a
reset is probably just wishful thinking. If a slave is holding down
SCL, we are pretty screwed.

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

* Re: [PATCH v11 3/4] i2c: aspeed: added driver for Aspeed I2C
  2017-06-26  6:18     ` Joel Stanley
@ 2017-06-27  8:36       ` Brendan Higgins
  0 siblings, 0 replies; 17+ messages in thread
From: Brendan Higgins @ 2017-06-27  8:36 UTC (permalink / raw)
  To: Joel Stanley
  Cc: Wolfram Sang, Rob Herring, Mark Rutland, Greg KH,
	David S . Miller, mchehab, martin.petersen,
	Benjamin Herrenschmidt, linux-i2c, devicetree,
	Linux Kernel Mailing List, OpenBMC Maillist, Rick Altherr

On Sun, Jun 25, 2017 at 11:18 PM, Joel Stanley <joel@jms.id.au> wrote:
> On Sat, Jun 24, 2017 at 4:13 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> On Tue, Jun 20, 2017 at 02:15:15PM -0700, Brendan Higgins wrote:
>>> Added initial master support for Aspeed I2C controller. Supports
>>> fourteen busses present in AST24XX and AST25XX BMC SoCs by Aspeed.
>>>
>>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>>
>> Applied to for-next, thanks for all the hard work!
>
> Congratulations Brendan. It's great to see this driver in the tree.
>
> Cheers,
>
> Joel

Thanks! And thank you to everyone who helped! I am not the only person who
put in a lot of hard work.

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

* Re: [PATCH v11 3/4] i2c: aspeed: added driver for Aspeed I2C
  2017-06-27  8:28     ` Brendan Higgins
@ 2017-06-27  9:00       ` Wolfram Sang
  2017-06-27 23:47         ` Brendan Higgins
  0 siblings, 1 reply; 17+ messages in thread
From: Wolfram Sang @ 2017-06-27  9:00 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: Rob Herring, Mark Rutland, gregkh, davem, mchehab, Joel Stanley,
	martin.petersen, Benjamin Herrenschmidt, linux-i2c, devicetree,
	Linux Kernel Mailing List, OpenBMC Maillist

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


> > If SCL is stuck low, how do you want to send a STOP?
> >
> 
> Fair point. I should probably drop that in the future and just do a
> reset, and even then, doing a
> reset is probably just wishful thinking. If a slave is holding down
> SCL, we are pretty screwed.

Yes, you'd need to reset the client device. And that has to be done in
the client driver. I wonder if i2c_transfer and friends should return a
specific error value in that case... need to think about it.


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

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

* Re: [PATCH v11 3/4] i2c: aspeed: added driver for Aspeed I2C
  2017-06-27  9:00       ` Wolfram Sang
@ 2017-06-27 23:47         ` Brendan Higgins
  2017-06-28  7:45           ` Wolfram Sang
  0 siblings, 1 reply; 17+ messages in thread
From: Brendan Higgins @ 2017-06-27 23:47 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Rob Herring, Mark Rutland, Greg KH, David S . Miller, mchehab,
	Joel Stanley, martin.petersen, Benjamin Herrenschmidt, linux-i2c,
	devicetree, Linux Kernel Mailing List, OpenBMC Maillist

On Tue, Jun 27, 2017 at 2:00 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>> > If SCL is stuck low, how do you want to send a STOP?
>> >
>>
>> Fair point. I should probably drop that in the future and just do a
>> reset, and even then, doing a
>> reset is probably just wishful thinking. If a slave is holding down
>> SCL, we are pretty screwed.
>
> Yes, you'd need to reset the client device. And that has to be done in
> the client driver. I wonder if i2c_transfer and friends should return a
> specific error value in that case... need to think about it.
>

Well, a good first step would be to make my recovery code conform to
the struct i2c_bus_recovery_info. Is i2c_generic_scl_recovery supposed
to be part of the user interface, or is it just intended to help put the
main recovery function together?

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

* Re: [PATCH v11 3/4] i2c: aspeed: added driver for Aspeed I2C
  2017-06-27 23:47         ` Brendan Higgins
@ 2017-06-28  7:45           ` Wolfram Sang
  2017-07-13 22:44             ` Brendan Higgins
  0 siblings, 1 reply; 17+ messages in thread
From: Wolfram Sang @ 2017-06-28  7:45 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: Rob Herring, Mark Rutland, Greg KH, David S . Miller, mchehab,
	Joel Stanley, martin.petersen, Benjamin Herrenschmidt, linux-i2c,
	devicetree, Linux Kernel Mailing List, OpenBMC Maillist

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


> the struct i2c_bus_recovery_info. Is i2c_generic_scl_recovery supposed
> to be part of the user interface, or is it just intended to help put the
> main recovery function together?

Sorry, I don't understand the question. What do you mean?


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

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

* Re: [PATCH v11 3/4] i2c: aspeed: added driver for Aspeed I2C
  2017-06-28  7:45           ` Wolfram Sang
@ 2017-07-13 22:44             ` Brendan Higgins
  2017-07-14  7:18               ` Wolfram Sang
  0 siblings, 1 reply; 17+ messages in thread
From: Brendan Higgins @ 2017-07-13 22:44 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Rob Herring, Mark Rutland, Greg KH, David S . Miller, mchehab,
	Joel Stanley, martin.petersen, Benjamin Herrenschmidt, linux-i2c,
	devicetree, Linux Kernel Mailing List, OpenBMC Maillist

Sorry, went on vacation and then forgot about our conversion.

>> the struct i2c_bus_recovery_info. Is i2c_generic_scl_recovery supposed
>> to be part of the user interface, or is it just intended to help put the
>> main recovery function together?
>
> Sorry, I don't understand the question. What do you mean?
>

What I meant is that it looks like the only use of it is putting
together a default
recovery function, but I was wondering if it is fair to use it on its own.
Basically what I was asking is whether I could use i2c_generic_scl_recovery
in the case where SCL is hung.

I think I have a pretty good idea of what to do, I should probably just put
together an RFC patch.

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

* Re: [PATCH v11 3/4] i2c: aspeed: added driver for Aspeed I2C
  2017-07-13 22:44             ` Brendan Higgins
@ 2017-07-14  7:18               ` Wolfram Sang
  0 siblings, 0 replies; 17+ messages in thread
From: Wolfram Sang @ 2017-07-14  7:18 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: Rob Herring, Mark Rutland, Greg KH, David S . Miller, mchehab,
	Joel Stanley, martin.petersen, Benjamin Herrenschmidt, linux-i2c,
	devicetree, Linux Kernel Mailing List, OpenBMC Maillist

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


> Basically what I was asking is whether I could use i2c_generic_scl_recovery
> in the case where SCL is hung.

The name is a bit misleading, I am afraid. Recovery can only be used when
SDA is stuck low. And to fix this it *uses* SCL toggling to get out of
it. And 'generic_scl' means 'gimme some SCL to control and I will toggle
it'. Compared to 'gpio_recovery' which will do all the GPIO handling for
you.

When SCL is hung, you can only reset the device which forces SCL low.

> I think I have a pretty good idea of what to do, I should probably just put
> together an RFC patch.

Sure.


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

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

end of thread, other threads:[~2017-07-14  7:18 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-20 21:15 [PATCH v11 0/4] i2c: aspeed: added driver for Aspeed I2C Brendan Higgins
2017-06-20 21:15 ` [PATCH v11 1/4] MAINTAINERS: add entry for Aspeed I2C driver Brendan Higgins
2017-06-23 18:40   ` Wolfram Sang
2017-06-20 21:15 ` [PATCH v11 2/4] i2c: aspeed: added documentation " Brendan Higgins
2017-06-23 18:41   ` Wolfram Sang
2017-06-20 21:15 ` [PATCH v11 3/4] i2c: aspeed: added driver for Aspeed I2C Brendan Higgins
2017-06-23 18:43   ` Wolfram Sang
2017-06-26  6:18     ` Joel Stanley
2017-06-27  8:36       ` Brendan Higgins
2017-06-27  8:28     ` Brendan Higgins
2017-06-27  9:00       ` Wolfram Sang
2017-06-27 23:47         ` Brendan Higgins
2017-06-28  7:45           ` Wolfram Sang
2017-07-13 22:44             ` Brendan Higgins
2017-07-14  7:18               ` Wolfram Sang
2017-06-20 21:15 ` [PATCH v11 4/4] i2c: aspeed: added slave support for Aspeed I2C driver Brendan Higgins
2017-06-23 18:41   ` Wolfram Sang

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).