* [PATCH v10 0/5] i2c: aspeed: added driver for Aspeed I2C
@ 2017-06-03 1:29 Brendan Higgins
2017-06-03 1:29 ` [PATCH v10 1/5] irqchip/aspeed-i2c-ic: binding docs for Aspeed I2C Interrupt Controller Brendan Higgins
` (4 more replies)
0 siblings, 5 replies; 27+ messages in thread
From: Brendan Higgins @ 2017-06-03 1:29 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, tglx, jason, marc.zyngier, joel, vz,
mouse, clg, benh, ryan_chen
Cc: linux-i2c, devicetree, linux-kernel, openbmc
Addressed comments from:
- Marc in: http://www.spinics.net/lists/devicetree/msg179432.html
- Ben in: http://www.spinics.net/lists/devicetree/msg179491.html
Changes since previous update:
- I2C irqchip: Unallocate resources on init failure
- I2C driver: Added master_xfer_result field to struct aspeed_i2c_bus
As before, tested on Aspeed 2500 evaluation board and a real platform with an
Aspeed 2520.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v10 1/5] irqchip/aspeed-i2c-ic: binding docs for Aspeed I2C Interrupt Controller
2017-06-03 1:29 [PATCH v10 0/5] i2c: aspeed: added driver for Aspeed I2C Brendan Higgins
@ 2017-06-03 1:29 ` Brendan Higgins
2017-06-05 6:25 ` Joel Stanley
2017-06-07 11:31 ` Marc Zyngier
2017-06-03 1:29 ` [PATCH v10 2/5] irqchip/aspeed-i2c-ic: Add I2C IRQ controller for Aspeed Brendan Higgins
` (3 subsequent siblings)
4 siblings, 2 replies; 27+ messages in thread
From: Brendan Higgins @ 2017-06-03 1:29 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, tglx, jason, marc.zyngier, joel, vz,
mouse, clg, benh, ryan_chen
Cc: linux-i2c, devicetree, linux-kernel, openbmc, Brendan Higgins
Added device tree binding documentation for Aspeed I2C Interrupt
Controller.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Acked-by: Rob Herring <robh@kernel.org>
---
Added in v6:
- Pulled "aspeed_i2c_controller" out into a interrupt controller since that is
what it actually does.
Changes for v7:
- None
Changes for v8:
- None
Changes for v9:
- None
Changes for v10:
- None
---
.../interrupt-controller/aspeed,ast2400-i2c-ic.txt | 25 ++++++++++++++++++++++
1 file changed, 25 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2400-i2c-ic.txt
diff --git a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2400-i2c-ic.txt b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2400-i2c-ic.txt
new file mode 100644
index 000000000000..033cc82e5684
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2400-i2c-ic.txt
@@ -0,0 +1,25 @@
+Device tree configuration for the I2C Interrupt Controller on the AST24XX and
+AST25XX SoCs.
+
+Required Properties:
+- #address-cells : should be 1
+- #size-cells : should be 1
+- #interrupt-cells : should be 1
+- compatible : should be "aspeed,ast2400-i2c-ic"
+ or "aspeed,ast2500-i2c-ic"
+- reg : address start and range of controller
+- interrupts : interrupt number
+- interrupt-controller : denotes that the controller receives and fires
+ new interrupts for child busses
+
+Example:
+
+i2c_ic: interrupt-controller@0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ #interrupt-cells = <1>;
+ compatible = "aspeed,ast2400-i2c-ic";
+ reg = <0x0 0x40>;
+ interrupts = <12>;
+ interrupt-controller;
+};
--
2.13.0.506.g27d5fe0cd-goog
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v10 2/5] irqchip/aspeed-i2c-ic: Add I2C IRQ controller for Aspeed
2017-06-03 1:29 [PATCH v10 0/5] i2c: aspeed: added driver for Aspeed I2C Brendan Higgins
2017-06-03 1:29 ` [PATCH v10 1/5] irqchip/aspeed-i2c-ic: binding docs for Aspeed I2C Interrupt Controller Brendan Higgins
@ 2017-06-03 1:29 ` Brendan Higgins
2017-06-05 4:56 ` Andrew Jeffery
2017-06-03 1:29 ` [PATCH v10 3/5] i2c: aspeed: added documentation for Aspeed I2C driver Brendan Higgins
` (2 subsequent siblings)
4 siblings, 1 reply; 27+ messages in thread
From: Brendan Higgins @ 2017-06-03 1:29 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, tglx, jason, marc.zyngier, joel, vz,
mouse, clg, benh, ryan_chen
Cc: linux-i2c, devicetree, linux-kernel, openbmc, Brendan Higgins
The Aspeed 24XX/25XX chips share a single hardware interrupt across 14
separate I2C busses. This adds a dummy irqchip which maps the single
hardware interrupt to software interrupts for each of the busses.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
Added in v6:
- Pulled "aspeed_i2c_controller" out into a interrupt controller since that is
what it actually does.
Changes for v7:
- Renamed irq domain for consistency
Changes for v8:
- None
Changes for v9:
- None
Changes for v10:
- Unallocate resources on init failure
- Changed formatting of a function call
---
drivers/irqchip/Makefile | 2 +-
drivers/irqchip/irq-aspeed-i2c-ic.c | 115 ++++++++++++++++++++++++++++++++++++
2 files changed, 116 insertions(+), 1 deletion(-)
create mode 100644 drivers/irqchip/irq-aspeed-i2c-ic.c
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index b64c59b838a0..e067f9839b33 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -73,6 +73,6 @@ obj-$(CONFIG_MVEBU_ODMI) += irq-mvebu-odmi.o
obj-$(CONFIG_MVEBU_PIC) += irq-mvebu-pic.o
obj-$(CONFIG_LS_SCFG_MSI) += irq-ls-scfg-msi.o
obj-$(CONFIG_EZNPS_GIC) += irq-eznps.o
-obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-vic.o
+obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-vic.o irq-aspeed-i2c-ic.o
obj-$(CONFIG_STM32_EXTI) += irq-stm32-exti.o
obj-$(CONFIG_QCOM_IRQ_COMBINER) += qcom-irq-combiner.o
diff --git a/drivers/irqchip/irq-aspeed-i2c-ic.c b/drivers/irqchip/irq-aspeed-i2c-ic.c
new file mode 100644
index 000000000000..815b88dd18f2
--- /dev/null
+++ b/drivers/irqchip/irq-aspeed-i2c-ic.c
@@ -0,0 +1,115 @@
+/*
+ * Aspeed 24XX/25XX I2C Interrupt 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/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/io.h>
+
+
+#define ASPEED_I2C_IC_NUM_BUS 14
+
+struct aspeed_i2c_ic {
+ void __iomem *base;
+ int parent_irq;
+ struct irq_domain *irq_domain;
+};
+
+/*
+ * The aspeed chip provides a single hardware interrupt for all of the I2C
+ * busses, so we use a dummy interrupt chip to translate this single interrupt
+ * into multiple interrupts, each associated with a single I2C bus.
+ */
+static void aspeed_i2c_ic_irq_handler(struct irq_desc *desc)
+{
+ struct aspeed_i2c_ic *i2c_ic = irq_desc_get_handler_data(desc);
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ unsigned long bit, status;
+ unsigned int bus_irq;
+
+ chained_irq_enter(chip, desc);
+ status = readl(i2c_ic->base);
+ for_each_set_bit(bit, &status, ASPEED_I2C_IC_NUM_BUS) {
+ bus_irq = irq_find_mapping(i2c_ic->irq_domain, bit);
+ generic_handle_irq(bus_irq);
+ }
+ chained_irq_exit(chip, desc);
+}
+
+/*
+ * Set simple handler and mark IRQ as valid. Nothing interesting to do here
+ * since we are using a dummy interrupt chip.
+ */
+static int aspeed_i2c_ic_map_irq_domain(struct irq_domain *domain,
+ unsigned int irq, irq_hw_number_t hwirq)
+{
+ irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq);
+ irq_set_chip_data(irq, domain->host_data);
+
+ return 0;
+}
+
+static const struct irq_domain_ops aspeed_i2c_ic_irq_domain_ops = {
+ .map = aspeed_i2c_ic_map_irq_domain,
+};
+
+static int __init aspeed_i2c_ic_of_init(struct device_node *node,
+ struct device_node *parent)
+{
+ struct aspeed_i2c_ic *i2c_ic;
+ int ret = 0;
+
+ i2c_ic = kzalloc(sizeof(*i2c_ic), GFP_KERNEL);
+ if (!i2c_ic)
+ return -ENOMEM;
+
+ i2c_ic->base = of_iomap(node, 0);
+ if (IS_ERR(i2c_ic->base)) {
+ ret = PTR_ERR(i2c_ic->base);
+ goto err_free_ic;
+ }
+
+ i2c_ic->parent_irq = irq_of_parse_and_map(node, 0);
+ if (i2c_ic->parent_irq < 0) {
+ ret = i2c_ic->parent_irq;
+ goto err_iounmap;
+ }
+
+ i2c_ic->irq_domain = irq_domain_add_linear(node, ASPEED_I2C_IC_NUM_BUS,
+ &aspeed_i2c_ic_irq_domain_ops,
+ NULL);
+ if (!i2c_ic->irq_domain) {
+ ret = -ENOMEM;
+ goto err_iounmap;
+ }
+
+ i2c_ic->irq_domain->name = "aspeed-i2c-domain";
+
+ irq_set_chained_handler_and_data(i2c_ic->parent_irq,
+ aspeed_i2c_ic_irq_handler, i2c_ic);
+
+ pr_info("i2c controller registered, irq %d\n", i2c_ic->parent_irq);
+
+ return 0;
+
+err_iounmap:
+ iounmap(i2c_ic->base);
+err_free_ic:
+ kfree(i2c_ic);
+ return ret;
+}
+
+IRQCHIP_DECLARE(ast2400_i2c_ic, "aspeed,ast2400-i2c-ic", aspeed_i2c_ic_of_init);
+IRQCHIP_DECLARE(ast2500_i2c_ic, "aspeed,ast2500-i2c-ic", aspeed_i2c_ic_of_init);
--
2.13.0.506.g27d5fe0cd-goog
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v10 3/5] i2c: aspeed: added documentation for Aspeed I2C driver
2017-06-03 1:29 [PATCH v10 0/5] i2c: aspeed: added driver for Aspeed I2C Brendan Higgins
2017-06-03 1:29 ` [PATCH v10 1/5] irqchip/aspeed-i2c-ic: binding docs for Aspeed I2C Interrupt Controller Brendan Higgins
2017-06-03 1:29 ` [PATCH v10 2/5] irqchip/aspeed-i2c-ic: Add I2C IRQ controller for Aspeed Brendan Higgins
@ 2017-06-03 1:29 ` Brendan Higgins
2017-06-05 4:40 ` Joel Stanley
2017-06-03 1:29 ` [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C Brendan Higgins
2017-06-03 1:29 ` [PATCH v10 5/5] i2c: aspeed: added slave support for Aspeed I2C driver Brendan Higgins
4 siblings, 1 reply; 27+ messages in thread
From: Brendan Higgins @ 2017-06-03 1:29 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, tglx, jason, marc.zyngier, joel, vz,
mouse, clg, benh, ryan_chen
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>
---
Changes 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
---
.../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.0.506.g27d5fe0cd-goog
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C
2017-06-03 1:29 [PATCH v10 0/5] i2c: aspeed: added driver for Aspeed I2C Brendan Higgins
` (2 preceding siblings ...)
2017-06-03 1:29 ` [PATCH v10 3/5] i2c: aspeed: added documentation for Aspeed I2C driver Brendan Higgins
@ 2017-06-03 1:29 ` Brendan Higgins
2017-06-05 4:40 ` Joel Stanley
` (2 more replies)
2017-06-03 1:29 ` [PATCH v10 5/5] i2c: aspeed: added slave support for Aspeed I2C driver Brendan Higgins
4 siblings, 3 replies; 27+ messages in thread
From: Brendan Higgins @ 2017-06-03 1:29 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, tglx, jason, marc.zyngier, joel, vz,
mouse, clg, benh, ryan_chen
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>
---
Changes 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
---
drivers/i2c/busses/Kconfig | 10 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-aspeed.c | 681 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 692 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-aspeed.c
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 144cbadc7c72..280f84a0d7d1 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
+ 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..a04021e3b3ab
--- /dev/null
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -0,0 +1,681 @@
+/*
+ * 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_ERROR \
+ (ASPEED_I2CD_INTR_ARBIT_LOSS | \
+ ASPEED_I2CD_INTR_ABNORMAL | \
+ ASPEED_I2CD_INTR_SCL_TIMEOUT | \
+ ASPEED_I2CD_INTR_SDA_DL_TIMEOUT)
+#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;
+ int irq;
+ 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, "bus 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, "bus hung (state %x), attempting recovery\n",
+ command);
+
+ reinit_completion(&bus->cmd_complete);
+ 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 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;
+
+ 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.
+ */
+ if (irq_status & ASPEED_I2CD_INTR_ERROR) {
+ dev_dbg(bus->dev, "received error interrupt: 0x%08x",
+ irq_status);
+ bus->cmd_err = -EIO;
+ 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;
+ goto error_and_stop;
+ }
+ 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 = -EIO;
+ 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;
+
+ if (aspeed_i2c_master_irq(bus))
+ return IRQ_HANDLED;
+ else
+ return IRQ_NONE;
+}
+
+static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
+ struct i2c_msg *msgs, int num)
+{
+ struct aspeed_i2c_bus *bus = adap->algo_data;
+ 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 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.algo_data = bus;
+ 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));
+
+ 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;
+
+ bus->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+ ret = devm_request_irq(&pdev->dev, bus->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, bus->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.0.506.g27d5fe0cd-goog
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v10 5/5] i2c: aspeed: added slave support for Aspeed I2C driver
2017-06-03 1:29 [PATCH v10 0/5] i2c: aspeed: added driver for Aspeed I2C Brendan Higgins
` (3 preceding siblings ...)
2017-06-03 1:29 ` [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C Brendan Higgins
@ 2017-06-03 1:29 ` Brendan Higgins
2017-06-07 6:57 ` Joel Stanley
2017-06-19 14:14 ` Wolfram Sang
4 siblings, 2 replies; 27+ messages in thread
From: Brendan Higgins @ 2017-06-03 1:29 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, tglx, jason, marc.zyngier, joel, vz,
mouse, clg, benh, ryan_chen
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
---
drivers/i2c/busses/i2c-aspeed.c | 202 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 202 insertions(+)
diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index a04021e3b3ab..6516addf5bb8 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)
@@ -106,6 +108,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,
@@ -116,6 +121,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;
@@ -136,6 +150,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);
@@ -207,6 +225,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)
{
@@ -415,6 +537,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 */
+
if (aspeed_i2c_master_irq(bus))
return IRQ_HANDLED;
else
@@ -465,9 +594,76 @@ 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;
+ unsigned long flags;
+
+ bus = client->adapter->algo_data;
+ 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 = client->adapter->algo_data;
+ 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)
@@ -542,6 +738,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.0.506.g27d5fe0cd-goog
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C
@ 2017-06-05 4:40 ` Joel Stanley
0 siblings, 0 replies; 27+ messages in thread
From: Joel Stanley @ 2017-06-05 4:40 UTC (permalink / raw)
To: Brendan Higgins
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Thomas Gleixner,
Jason Cooper, Marc Zyngier, vz, mouse, Cédric Le Goater,
Benjamin Herrenschmidt, Ryan Chen, linux-i2c, devicetree,
Linux Kernel Mailing List, OpenBMC Maillist
On Sat, Jun 3, 2017 at 10:59 AM, Brendan Higgins
<brendanhiggins@google.com> 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>
Reviewed-by: Joel Stanley <joel@jms.id.au>
I have also tested this on an ast2500evb.
Cheers,
Joel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C
@ 2017-06-05 4:40 ` Joel Stanley
0 siblings, 0 replies; 27+ messages in thread
From: Joel Stanley @ 2017-06-05 4:40 UTC (permalink / raw)
To: Brendan Higgins
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Thomas Gleixner,
Jason Cooper, Marc Zyngier, vz-ChpfBGZJDbMAvxtiuMwx3w,
mouse-Pma6HLj0uuo, Cédric Le Goater, Benjamin Herrenschmidt,
Ryan Chen, linux-i2c-u79uwXL29TY76Z2rM5mHXA, devicetree,
Linux Kernel Mailing List, OpenBMC Maillist
On Sat, Jun 3, 2017 at 10:59 AM, Brendan Higgins
<brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> 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-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
I have also tested this on an ast2500evb.
Cheers,
Joel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v10 3/5] i2c: aspeed: added documentation for Aspeed I2C driver
2017-06-03 1:29 ` [PATCH v10 3/5] i2c: aspeed: added documentation for Aspeed I2C driver Brendan Higgins
@ 2017-06-05 4:40 ` Joel Stanley
0 siblings, 0 replies; 27+ messages in thread
From: Joel Stanley @ 2017-06-05 4:40 UTC (permalink / raw)
To: Brendan Higgins
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Thomas Gleixner,
Jason Cooper, Marc Zyngier, vz, mouse, Cédric Le Goater,
Benjamin Herrenschmidt, Ryan Chen, linux-i2c, devicetree,
Linux Kernel Mailing List, OpenBMC Maillist
On Sat, Jun 3, 2017 at 10:59 AM, Brendan Higgins
<brendanhiggins@google.com> 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>
Acked-by: Joel Stanley <joel@jms.id.au>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v10 2/5] irqchip/aspeed-i2c-ic: Add I2C IRQ controller for Aspeed
2017-06-03 1:29 ` [PATCH v10 2/5] irqchip/aspeed-i2c-ic: Add I2C IRQ controller for Aspeed Brendan Higgins
@ 2017-06-05 4:56 ` Andrew Jeffery
0 siblings, 0 replies; 27+ messages in thread
From: Andrew Jeffery @ 2017-06-05 4:56 UTC (permalink / raw)
To: Brendan Higgins, wsa, robh+dt, mark.rutland, tglx, jason,
marc.zyngier, joel, vz, mouse, clg, benh, ryan_chen
Cc: devicetree, openbmc, linux-i2c, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 5618 bytes --]
On Fri, 2017-06-02 at 18:29 -0700, Brendan Higgins wrote:
> The Aspeed 24XX/25XX chips share a single hardware interrupt across 14
> separate I2C busses. This adds a dummy irqchip which maps the single
> hardware interrupt to software interrupts for each of the busses.
>
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
I exercised the patch on an AST2500-based machine with good results.
Tested-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> Added in v6:
> - Pulled "aspeed_i2c_controller" out into a interrupt controller since that is
> what it actually does.
> Changes for v7:
> - Renamed irq domain for consistency
> Changes for v8:
> - None
> Changes for v9:
> - None
> Changes for v10:
> - Unallocate resources on init failure
> - Changed formatting of a function call
> ---
> drivers/irqchip/Makefile | 2 +-
> drivers/irqchip/irq-aspeed-i2c-ic.c | 115 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 116 insertions(+), 1 deletion(-)
> create mode 100644 drivers/irqchip/irq-aspeed-i2c-ic.c
>
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index b64c59b838a0..e067f9839b33 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> > @@ -73,6 +73,6 @@ obj-$(CONFIG_MVEBU_ODMI) += irq-mvebu-odmi.o
> > obj-$(CONFIG_MVEBU_PIC) += irq-mvebu-pic.o
> > obj-$(CONFIG_LS_SCFG_MSI) += irq-ls-scfg-msi.o
> > obj-$(CONFIG_EZNPS_GIC) += irq-eznps.o
> > -obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-vic.o
> > +obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-vic.o irq-aspeed-i2c-ic.o
> > obj-$(CONFIG_STM32_EXTI) += irq-stm32-exti.o
> > obj-$(CONFIG_QCOM_IRQ_COMBINER) += qcom-irq-combiner.o
> diff --git a/drivers/irqchip/irq-aspeed-i2c-ic.c b/drivers/irqchip/irq-aspeed-i2c-ic.c
> new file mode 100644
> index 000000000000..815b88dd18f2
> --- /dev/null
> +++ b/drivers/irqchip/irq-aspeed-i2c-ic.c
> @@ -0,0 +1,115 @@
> +/*
> + * Aspeed 24XX/25XX I2C Interrupt 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/irq.h>
> +#include <linux/irqchip.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/io.h>
> +
> +
> +#define ASPEED_I2C_IC_NUM_BUS 14
> +
> +struct aspeed_i2c_ic {
> > > + void __iomem *base;
> > > + int parent_irq;
> > > + struct irq_domain *irq_domain;
> +};
> +
> +/*
> + * The aspeed chip provides a single hardware interrupt for all of the I2C
> + * busses, so we use a dummy interrupt chip to translate this single interrupt
> + * into multiple interrupts, each associated with a single I2C bus.
> + */
> +static void aspeed_i2c_ic_irq_handler(struct irq_desc *desc)
> +{
> > + struct aspeed_i2c_ic *i2c_ic = irq_desc_get_handler_data(desc);
> > + struct irq_chip *chip = irq_desc_get_chip(desc);
> > + unsigned long bit, status;
> > + unsigned int bus_irq;
> +
> > + chained_irq_enter(chip, desc);
> > + status = readl(i2c_ic->base);
> > + for_each_set_bit(bit, &status, ASPEED_I2C_IC_NUM_BUS) {
> > + bus_irq = irq_find_mapping(i2c_ic->irq_domain, bit);
> > + generic_handle_irq(bus_irq);
> > + }
> > + chained_irq_exit(chip, desc);
> +}
> +
> +/*
> + * Set simple handler and mark IRQ as valid. Nothing interesting to do here
> + * since we are using a dummy interrupt chip.
> + */
> +static int aspeed_i2c_ic_map_irq_domain(struct irq_domain *domain,
> > + unsigned int irq, irq_hw_number_t hwirq)
> +{
> > + irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq);
> > + irq_set_chip_data(irq, domain->host_data);
> +
> > + return 0;
> +}
> +
> +static const struct irq_domain_ops aspeed_i2c_ic_irq_domain_ops = {
> > + .map = aspeed_i2c_ic_map_irq_domain,
> +};
> +
> +static int __init aspeed_i2c_ic_of_init(struct device_node *node,
> > + struct device_node *parent)
> +{
> > + struct aspeed_i2c_ic *i2c_ic;
> > + int ret = 0;
> +
> > + i2c_ic = kzalloc(sizeof(*i2c_ic), GFP_KERNEL);
> > + if (!i2c_ic)
> > + return -ENOMEM;
> +
> > + i2c_ic->base = of_iomap(node, 0);
> > + if (IS_ERR(i2c_ic->base)) {
> > + ret = PTR_ERR(i2c_ic->base);
> > + goto err_free_ic;
> > + }
> +
> > + i2c_ic->parent_irq = irq_of_parse_and_map(node, 0);
> > + if (i2c_ic->parent_irq < 0) {
> > + ret = i2c_ic->parent_irq;
> > + goto err_iounmap;
> > + }
> +
> > + i2c_ic->irq_domain = irq_domain_add_linear(node, ASPEED_I2C_IC_NUM_BUS,
> > + &aspeed_i2c_ic_irq_domain_ops,
> > + NULL);
> > + if (!i2c_ic->irq_domain) {
> > + ret = -ENOMEM;
> > + goto err_iounmap;
> > + }
> +
> > + i2c_ic->irq_domain->name = "aspeed-i2c-domain";
> +
> > + irq_set_chained_handler_and_data(i2c_ic->parent_irq,
> > + aspeed_i2c_ic_irq_handler, i2c_ic);
> +
> > + pr_info("i2c controller registered, irq %d\n", i2c_ic->parent_irq);
> +
> > + return 0;
> +
> +err_iounmap:
> > + iounmap(i2c_ic->base);
> +err_free_ic:
> > + kfree(i2c_ic);
> > + return ret;
> +}
> +
> +IRQCHIP_DECLARE(ast2400_i2c_ic, "aspeed,ast2400-i2c-ic", aspeed_i2c_ic_of_init);
> +IRQCHIP_DECLARE(ast2500_i2c_ic, "aspeed,ast2500-i2c-ic", aspeed_i2c_ic_of_init);
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v10 2/5] irqchip/aspeed-i2c-ic: Add I2C IRQ controller for Aspeed
@ 2017-06-05 4:56 ` Andrew Jeffery
0 siblings, 0 replies; 27+ messages in thread
From: Andrew Jeffery @ 2017-06-05 4:56 UTC (permalink / raw)
To: Brendan Higgins, wsa, robh+dt, mark.rutland, tglx, jason,
marc.zyngier, joel, vz, mouse, clg, benh, ryan_chen
Cc: devicetree, openbmc, linux-i2c, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 5618 bytes --]
On Fri, 2017-06-02 at 18:29 -0700, Brendan Higgins wrote:
> The Aspeed 24XX/25XX chips share a single hardware interrupt across 14
> separate I2C busses. This adds a dummy irqchip which maps the single
> hardware interrupt to software interrupts for each of the busses.
>
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
I exercised the patch on an AST2500-based machine with good results.
Tested-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> Added in v6:
> - Pulled "aspeed_i2c_controller" out into a interrupt controller since that is
> what it actually does.
> Changes for v7:
> - Renamed irq domain for consistency
> Changes for v8:
> - None
> Changes for v9:
> - None
> Changes for v10:
> - Unallocate resources on init failure
> - Changed formatting of a function call
> ---
> drivers/irqchip/Makefile | 2 +-
> drivers/irqchip/irq-aspeed-i2c-ic.c | 115 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 116 insertions(+), 1 deletion(-)
> create mode 100644 drivers/irqchip/irq-aspeed-i2c-ic.c
>
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index b64c59b838a0..e067f9839b33 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> > @@ -73,6 +73,6 @@ obj-$(CONFIG_MVEBU_ODMI) += irq-mvebu-odmi.o
> > obj-$(CONFIG_MVEBU_PIC) += irq-mvebu-pic.o
> > obj-$(CONFIG_LS_SCFG_MSI) += irq-ls-scfg-msi.o
> > obj-$(CONFIG_EZNPS_GIC) += irq-eznps.o
> > -obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-vic.o
> > +obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-vic.o irq-aspeed-i2c-ic.o
> > obj-$(CONFIG_STM32_EXTI) += irq-stm32-exti.o
> > obj-$(CONFIG_QCOM_IRQ_COMBINER) += qcom-irq-combiner.o
> diff --git a/drivers/irqchip/irq-aspeed-i2c-ic.c b/drivers/irqchip/irq-aspeed-i2c-ic.c
> new file mode 100644
> index 000000000000..815b88dd18f2
> --- /dev/null
> +++ b/drivers/irqchip/irq-aspeed-i2c-ic.c
> @@ -0,0 +1,115 @@
> +/*
> + * Aspeed 24XX/25XX I2C Interrupt 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/irq.h>
> +#include <linux/irqchip.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/io.h>
> +
> +
> +#define ASPEED_I2C_IC_NUM_BUS 14
> +
> +struct aspeed_i2c_ic {
> > > + void __iomem *base;
> > > + int parent_irq;
> > > + struct irq_domain *irq_domain;
> +};
> +
> +/*
> + * The aspeed chip provides a single hardware interrupt for all of the I2C
> + * busses, so we use a dummy interrupt chip to translate this single interrupt
> + * into multiple interrupts, each associated with a single I2C bus.
> + */
> +static void aspeed_i2c_ic_irq_handler(struct irq_desc *desc)
> +{
> > + struct aspeed_i2c_ic *i2c_ic = irq_desc_get_handler_data(desc);
> > + struct irq_chip *chip = irq_desc_get_chip(desc);
> > + unsigned long bit, status;
> > + unsigned int bus_irq;
> +
> > + chained_irq_enter(chip, desc);
> > + status = readl(i2c_ic->base);
> > + for_each_set_bit(bit, &status, ASPEED_I2C_IC_NUM_BUS) {
> > + bus_irq = irq_find_mapping(i2c_ic->irq_domain, bit);
> > + generic_handle_irq(bus_irq);
> > + }
> > + chained_irq_exit(chip, desc);
> +}
> +
> +/*
> + * Set simple handler and mark IRQ as valid. Nothing interesting to do here
> + * since we are using a dummy interrupt chip.
> + */
> +static int aspeed_i2c_ic_map_irq_domain(struct irq_domain *domain,
> > + unsigned int irq, irq_hw_number_t hwirq)
> +{
> > + irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq);
> > + irq_set_chip_data(irq, domain->host_data);
> +
> > + return 0;
> +}
> +
> +static const struct irq_domain_ops aspeed_i2c_ic_irq_domain_ops = {
> > + .map = aspeed_i2c_ic_map_irq_domain,
> +};
> +
> +static int __init aspeed_i2c_ic_of_init(struct device_node *node,
> > + struct device_node *parent)
> +{
> > + struct aspeed_i2c_ic *i2c_ic;
> > + int ret = 0;
> +
> > + i2c_ic = kzalloc(sizeof(*i2c_ic), GFP_KERNEL);
> > + if (!i2c_ic)
> > + return -ENOMEM;
> +
> > + i2c_ic->base = of_iomap(node, 0);
> > + if (IS_ERR(i2c_ic->base)) {
> > + ret = PTR_ERR(i2c_ic->base);
> > + goto err_free_ic;
> > + }
> +
> > + i2c_ic->parent_irq = irq_of_parse_and_map(node, 0);
> > + if (i2c_ic->parent_irq < 0) {
> > + ret = i2c_ic->parent_irq;
> > + goto err_iounmap;
> > + }
> +
> > + i2c_ic->irq_domain = irq_domain_add_linear(node, ASPEED_I2C_IC_NUM_BUS,
> > + &aspeed_i2c_ic_irq_domain_ops,
> > + NULL);
> > + if (!i2c_ic->irq_domain) {
> > + ret = -ENOMEM;
> > + goto err_iounmap;
> > + }
> +
> > + i2c_ic->irq_domain->name = "aspeed-i2c-domain";
> +
> > + irq_set_chained_handler_and_data(i2c_ic->parent_irq,
> > + aspeed_i2c_ic_irq_handler, i2c_ic);
> +
> > + pr_info("i2c controller registered, irq %d\n", i2c_ic->parent_irq);
> +
> > + return 0;
> +
> +err_iounmap:
> > + iounmap(i2c_ic->base);
> +err_free_ic:
> > + kfree(i2c_ic);
> > + return ret;
> +}
> +
> +IRQCHIP_DECLARE(ast2400_i2c_ic, "aspeed,ast2400-i2c-ic", aspeed_i2c_ic_of_init);
> +IRQCHIP_DECLARE(ast2500_i2c_ic, "aspeed,ast2500-i2c-ic", aspeed_i2c_ic_of_init);
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C
2017-06-03 1:29 ` [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C Brendan Higgins
@ 2017-06-05 5:02 ` Andrew Jeffery
2017-06-05 5:02 ` Andrew Jeffery
2017-06-19 14:13 ` Wolfram Sang
2 siblings, 0 replies; 27+ messages in thread
From: Andrew Jeffery @ 2017-06-05 5:02 UTC (permalink / raw)
To: Brendan Higgins, wsa, robh+dt, mark.rutland, tglx, jason,
marc.zyngier, joel, vz, mouse, clg, benh, ryan_chen
Cc: devicetree, openbmc, linux-i2c, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 27716 bytes --]
On Fri, 2017-06-02 at 18:29 -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>
I exercised this patch on an AST2500-based machine. The results were
good, and the implementation copes fine with a workload that causes an
oops in our (older, unrelated) out-of-tree bus driver.
Tested-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> Changes 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
> ---
> drivers/i2c/busses/Kconfig | 10 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-aspeed.c | 681
> ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 692 insertions(+)
> create mode 100644 drivers/i2c/busses/i2c-aspeed.c
>
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 144cbadc7c72..280f84a0d7d1 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
> + 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..a04021e3b3ab
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -0,0 +1,681 @@
> +/*
> + * 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 GENMAS
> K(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(1
> 4)
> +#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_ERROR
> \
> + (ASPEED_I2CD_INTR_ARBIT_LOSS |
> \
> + ASPEED_I2CD_INTR_ABNORMAL |
> \
> + ASPEED_I2CD_INTR_SCL_TIMEOUT |
> \
> + ASPEED_I2CD_INTR_SDA_DL_TIMEOUT)
> +#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;
> + int irq;
> + 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, "bus 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, "bus hung (state %x), attempting
> recovery\n",
> + command);
> +
> + reinit_completion(&bus->cmd_complete);
> + 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 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;
> +
> + 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.
> + */
> + if (irq_status & ASPEED_I2CD_INTR_ERROR) {
> + dev_dbg(bus->dev, "received error interrupt:
> 0x%08x",
> + irq_status);
> + bus->cmd_err = -EIO;
> + 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;
> + goto error_and_stop;
> + }
> + 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 = -EIO;
> + 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;
> +
> + if (aspeed_i2c_master_irq(bus))
> + return IRQ_HANDLED;
> + else
> + return IRQ_NONE;
> +}
> +
> +static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
> + struct i2c_msg *msgs, int num)
> +{
> + struct aspeed_i2c_bus *bus = adap->algo_data;
> + 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 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.algo_data = bus;
> + 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));
> +
> + 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;
> +
> + bus->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> + ret = devm_request_irq(&pdev->dev, bus->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, bus->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");
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C
@ 2017-06-05 5:02 ` Andrew Jeffery
0 siblings, 0 replies; 27+ messages in thread
From: Andrew Jeffery @ 2017-06-05 5:02 UTC (permalink / raw)
To: Brendan Higgins, wsa, robh+dt, mark.rutland, tglx, jason,
marc.zyngier, joel, vz, mouse, clg, benh, ryan_chen
Cc: devicetree, openbmc, linux-i2c, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 27716 bytes --]
On Fri, 2017-06-02 at 18:29 -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>
I exercised this patch on an AST2500-based machine. The results were
good, and the implementation copes fine with a workload that causes an
oops in our (older, unrelated) out-of-tree bus driver.
Tested-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> Changes 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
> ---
> drivers/i2c/busses/Kconfig | 10 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-aspeed.c | 681
> ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 692 insertions(+)
> create mode 100644 drivers/i2c/busses/i2c-aspeed.c
>
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 144cbadc7c72..280f84a0d7d1 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
> + 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..a04021e3b3ab
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -0,0 +1,681 @@
> +/*
> + * 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 GENMAS
> K(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(1
> 4)
> +#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_ERROR
> \
> + (ASPEED_I2CD_INTR_ARBIT_LOSS |
> \
> + ASPEED_I2CD_INTR_ABNORMAL |
> \
> + ASPEED_I2CD_INTR_SCL_TIMEOUT |
> \
> + ASPEED_I2CD_INTR_SDA_DL_TIMEOUT)
> +#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;
> + int irq;
> + 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, "bus 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, "bus hung (state %x), attempting
> recovery\n",
> + command);
> +
> + reinit_completion(&bus->cmd_complete);
> + 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 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;
> +
> + 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.
> + */
> + if (irq_status & ASPEED_I2CD_INTR_ERROR) {
> + dev_dbg(bus->dev, "received error interrupt:
> 0x%08x",
> + irq_status);
> + bus->cmd_err = -EIO;
> + 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;
> + goto error_and_stop;
> + }
> + 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 = -EIO;
> + 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;
> +
> + if (aspeed_i2c_master_irq(bus))
> + return IRQ_HANDLED;
> + else
> + return IRQ_NONE;
> +}
> +
> +static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
> + struct i2c_msg *msgs, int num)
> +{
> + struct aspeed_i2c_bus *bus = adap->algo_data;
> + 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 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.algo_data = bus;
> + 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));
> +
> + 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;
> +
> + bus->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> + ret = devm_request_irq(&pdev->dev, bus->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, bus->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");
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v10 1/5] irqchip/aspeed-i2c-ic: binding docs for Aspeed I2C Interrupt Controller
2017-06-03 1:29 ` [PATCH v10 1/5] irqchip/aspeed-i2c-ic: binding docs for Aspeed I2C Interrupt Controller Brendan Higgins
@ 2017-06-05 6:25 ` Joel Stanley
2017-06-07 11:31 ` Marc Zyngier
1 sibling, 0 replies; 27+ messages in thread
From: Joel Stanley @ 2017-06-05 6:25 UTC (permalink / raw)
To: Brendan Higgins
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Thomas Gleixner,
Jason Cooper, Marc Zyngier, vz, mouse, Cédric Le Goater,
Benjamin Herrenschmidt, Ryan Chen, linux-i2c, devicetree,
Linux Kernel Mailing List, OpenBMC Maillist
On Sat, Jun 3, 2017 at 10:59 AM, Brendan Higgins
<brendanhiggins@google.com> wrote:
> Added device tree binding documentation for Aspeed I2C Interrupt
> Controller.
>
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Joel Stanley <joel@jms.id.au>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v10 5/5] i2c: aspeed: added slave support for Aspeed I2C driver
2017-06-03 1:29 ` [PATCH v10 5/5] i2c: aspeed: added slave support for Aspeed I2C driver Brendan Higgins
@ 2017-06-07 6:57 ` Joel Stanley
2017-06-19 14:14 ` Wolfram Sang
1 sibling, 0 replies; 27+ messages in thread
From: Joel Stanley @ 2017-06-07 6:57 UTC (permalink / raw)
To: Brendan Higgins
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Thomas Gleixner,
Jason Cooper, Marc Zyngier, vz, mouse, Cédric Le Goater,
Benjamin Herrenschmidt, Ryan Chen, linux-i2c, devicetree,
Linux Kernel Mailing List, OpenBMC Maillist
On Sat, Jun 3, 2017 at 10:59 AM, Brendan Higgins
<brendanhiggins@google.com> 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>
I wired up the first and second buses on the ast2500evb, and performed
the following test:
echo slave-24c02 0x1064 > /sys/bus/i2c/devices/i2c-1/new_device
echo 24c02 0x64 > /sys/bus/i2c/devices/i2c-0/new_device
echo "How about a nice game of chess? 0xDEADBEEF" >
/sys/bus/i2c/devices/1-1064/slave-eeprom
hexdump -C /sys/devices/platform/ahb/ahb:apb/ahb:apb:i2c@1e78a000/1e78a040.i2c-bus/i2c-0/0-0064/eeprom
00000000 48 6f 77 20 61 62 6f 75 74 20 61 20 6e 69 63 65 |How about a nice|
00000010 20 67 61 6d 65 20 6f 66 20 63 68 65 73 73 3f 20 | game of chess? |
00000020 30 78 44 45 41 44 42 45 45 46 0a 00 00 00 00 00 |0xDEADBEEF......|
00000030 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
*
00000100
I also tested the other direction:
# echo "hack the planet" >
/sys/devices/platform/ahb/ahb:apb/ahb:apb:i2c@1e78a000/1e78a040.i2c-bus/i2c-0/0-0064/eeprom
# hexdump -C /sys/bus/i2c/devices/1-1064/slave-eeprom
00000000 68 61 63 6b 20 74 68 65 20 70 6c 61 6e 65 74 0a |hack the planet.|
00000010 20 67 61 6d 65 20 6f 66 20 63 68 65 73 73 3f 20 | game of chess? |
00000020 30 78 44 45 41 44 42 45 45 46 0a 00 00 00 00 00 |0xDEADBEEF......|
00000030 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
*
00000100
# md5sum /sys/devices/platform/ahb/ahb:apb/ahb:apb:i2c@1e78a000/1e78a040.i2c-bus
/i2c-0/0-0064/eeprom
0ae09da1dcbeb410cd311579fb6bd3b4
/sys/devices/platform/ahb/ahb:apb/ahb:apb:i2c@1e78a000/1e78a040.i2c-bus/i2c-0/0-0064/eeprom
# md5sum /sys/bus/i2c/devices/1-1064/slave-eeprom
0ae09da1dcbeb410cd311579fb6bd3b4 /sys/bus/i2c/devices/1-1064/slave-eeprom
Tested-by: Joel Stanley <joel@jms.id.au>
Cheers,
Joel
> ---
> 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
> ---
> drivers/i2c/busses/i2c-aspeed.c | 202 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 202 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> index a04021e3b3ab..6516addf5bb8 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)
> @@ -106,6 +108,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,
> @@ -116,6 +121,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;
> @@ -136,6 +150,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);
> @@ -207,6 +225,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)
> {
> @@ -415,6 +537,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 */
> +
> if (aspeed_i2c_master_irq(bus))
> return IRQ_HANDLED;
> else
> @@ -465,9 +594,76 @@ 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;
> + unsigned long flags;
> +
> + bus = client->adapter->algo_data;
> + 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 = client->adapter->algo_data;
> + 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)
> @@ -542,6 +738,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.0.506.g27d5fe0cd-goog
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v10 1/5] irqchip/aspeed-i2c-ic: binding docs for Aspeed I2C Interrupt Controller
2017-06-03 1:29 ` [PATCH v10 1/5] irqchip/aspeed-i2c-ic: binding docs for Aspeed I2C Interrupt Controller Brendan Higgins
2017-06-05 6:25 ` Joel Stanley
@ 2017-06-07 11:31 ` Marc Zyngier
1 sibling, 0 replies; 27+ messages in thread
From: Marc Zyngier @ 2017-06-07 11:31 UTC (permalink / raw)
To: Brendan Higgins, wsa, robh+dt, mark.rutland, tglx, jason, joel,
vz, mouse, clg, benh, ryan_chen
Cc: linux-i2c, devicetree, linux-kernel, openbmc
On 03/06/17 02:29, Brendan Higgins wrote:
> Added device tree binding documentation for Aspeed I2C Interrupt
> Controller.
>
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> Acked-by: Rob Herring <robh@kernel.org>
I've queued this (and the next patch) for 4.13.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C
@ 2017-06-15 23:42 ` Brendan Higgins
0 siblings, 0 replies; 27+ messages in thread
From: Brendan Higgins @ 2017-06-15 23:42 UTC (permalink / raw)
To: Wolfram Sang
Cc: Rob Herring, Mark Rutland, Thomas Gleixner, Jason Cooper,
Marc Zyngier, Joel Stanley, Vladimir Zapolskiy, Kachalov Anton,
Cédric Le Goater, Benjamin Herrenschmidt, Ryan Chen,
devicetree, OpenBMC Maillist, linux-i2c,
Linux Kernel Mailing List, Andrew Jeffery
On Sun, Jun 4, 2017 at 10:02 PM, Andrew Jeffery <andrew@aj.id.au> wrote:
> On Fri, 2017-06-02 at 18:29 -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>
>
> I exercised this patch on an AST2500-based machine. The results were
> good, and the implementation copes fine with a workload that causes an
> oops in our (older, unrelated) out-of-tree bus driver.
>
> Tested-by: Andrew Jeffery <andrew@aj.id.au>
Wolfram, could you take a look at these patches? Ben, Joel and others
seem to be happy with it and Marc said he would take the irqchip
stuff.
Cheers
>
>> ---
>> Changes 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
>> ---
>> drivers/i2c/busses/Kconfig | 10 +
>> drivers/i2c/busses/Makefile | 1 +
>> drivers/i2c/busses/i2c-aspeed.c | 681
>> ++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 692 insertions(+)
>> create mode 100644 drivers/i2c/busses/i2c-aspeed.c
>>
>> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
>> index 144cbadc7c72..280f84a0d7d1 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
>> + 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..a04021e3b3ab
>> --- /dev/null
>> +++ b/drivers/i2c/busses/i2c-aspeed.c
>> @@ -0,0 +1,681 @@
>> +/*
>> + * 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 GENMAS
>> K(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(1
>> 4)
>> +#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_ERROR
>> \
>> + (ASPEED_I2CD_INTR_ARBIT_LOSS |
>> \
>> + ASPEED_I2CD_INTR_ABNORMAL |
>> \
>> + ASPEED_I2CD_INTR_SCL_TIMEOUT |
>> \
>> + ASPEED_I2CD_INTR_SDA_DL_TIMEOUT)
>> +#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;
>> + int irq;
>> + 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, "bus 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, "bus hung (state %x), attempting
>> recovery\n",
>> + command);
>> +
>> + reinit_completion(&bus->cmd_complete);
>> + 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 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;
>> +
>> + 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.
>> + */
>> + if (irq_status & ASPEED_I2CD_INTR_ERROR) {
>> + dev_dbg(bus->dev, "received error interrupt:
>> 0x%08x",
>> + irq_status);
>> + bus->cmd_err = -EIO;
>> + 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;
>> + goto error_and_stop;
>> + }
>> + 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 = -EIO;
>> + 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;
>> +
>> + if (aspeed_i2c_master_irq(bus))
>> + return IRQ_HANDLED;
>> + else
>> + return IRQ_NONE;
>> +}
>> +
>> +static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
>> + struct i2c_msg *msgs, int num)
>> +{
>> + struct aspeed_i2c_bus *bus = adap->algo_data;
>> + 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 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.algo_data = bus;
>> + 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));
>> +
>> + 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;
>> +
>> + bus->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
>> + ret = devm_request_irq(&pdev->dev, bus->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, bus->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");
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C
@ 2017-06-15 23:42 ` Brendan Higgins
0 siblings, 0 replies; 27+ messages in thread
From: Brendan Higgins @ 2017-06-15 23:42 UTC (permalink / raw)
To: Wolfram Sang
Cc: Rob Herring, Mark Rutland, Thomas Gleixner, Jason Cooper,
Marc Zyngier, Joel Stanley, Vladimir Zapolskiy, Kachalov Anton,
Cédric Le Goater, Benjamin Herrenschmidt, Ryan Chen,
devicetree-u79uwXL29TY76Z2rM5mHXA, OpenBMC Maillist,
linux-i2c-u79uwXL29TY76Z2rM5mHXA, Linux Kernel Mailing List,
Andrew Jeffery
On Sun, Jun 4, 2017 at 10:02 PM, Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org> wrote:
> On Fri, 2017-06-02 at 18:29 -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-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>
> I exercised this patch on an AST2500-based machine. The results were
> good, and the implementation copes fine with a workload that causes an
> oops in our (older, unrelated) out-of-tree bus driver.
>
> Tested-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>
Wolfram, could you take a look at these patches? Ben, Joel and others
seem to be happy with it and Marc said he would take the irqchip
stuff.
Cheers
>
>> ---
>> Changes 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
>> ---
>> drivers/i2c/busses/Kconfig | 10 +
>> drivers/i2c/busses/Makefile | 1 +
>> drivers/i2c/busses/i2c-aspeed.c | 681
>> ++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 692 insertions(+)
>> create mode 100644 drivers/i2c/busses/i2c-aspeed.c
>>
>> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
>> index 144cbadc7c72..280f84a0d7d1 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
>> + 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..a04021e3b3ab
>> --- /dev/null
>> +++ b/drivers/i2c/busses/i2c-aspeed.c
>> @@ -0,0 +1,681 @@
>> +/*
>> + * 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 GENMAS
>> K(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(1
>> 4)
>> +#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_ERROR
>> \
>> + (ASPEED_I2CD_INTR_ARBIT_LOSS |
>> \
>> + ASPEED_I2CD_INTR_ABNORMAL |
>> \
>> + ASPEED_I2CD_INTR_SCL_TIMEOUT |
>> \
>> + ASPEED_I2CD_INTR_SDA_DL_TIMEOUT)
>> +#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;
>> + int irq;
>> + 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, "bus 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, "bus hung (state %x), attempting
>> recovery\n",
>> + command);
>> +
>> + reinit_completion(&bus->cmd_complete);
>> + 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 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;
>> +
>> + 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.
>> + */
>> + if (irq_status & ASPEED_I2CD_INTR_ERROR) {
>> + dev_dbg(bus->dev, "received error interrupt:
>> 0x%08x",
>> + irq_status);
>> + bus->cmd_err = -EIO;
>> + 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;
>> + goto error_and_stop;
>> + }
>> + 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 = -EIO;
>> + 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;
>> +
>> + if (aspeed_i2c_master_irq(bus))
>> + return IRQ_HANDLED;
>> + else
>> + return IRQ_NONE;
>> +}
>> +
>> +static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
>> + struct i2c_msg *msgs, int num)
>> +{
>> + struct aspeed_i2c_bus *bus = adap->algo_data;
>> + 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 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.algo_data = bus;
>> + 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));
>> +
>> + 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;
>> +
>> + bus->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
>> + ret = devm_request_irq(&pdev->dev, bus->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, bus->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-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>");
>> +MODULE_DESCRIPTION("Aspeed I2C Bus Driver");
>> +MODULE_LICENSE("GPL v2");
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C
@ 2017-06-19 14:13 ` Wolfram Sang
0 siblings, 0 replies; 27+ messages in thread
From: Wolfram Sang @ 2017-06-19 14:13 UTC (permalink / raw)
To: Brendan Higgins
Cc: robh+dt, mark.rutland, tglx, jason, marc.zyngier, joel, vz,
mouse, clg, benh, ryan_chen, linux-i2c, devicetree, linux-kernel,
openbmc
[-- Attachment #1: Type: text/plain, Size: 5152 bytes --]
Hi Brendan,
here is my review. Only minor stuff, no real show-stopper.
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 144cbadc7c72..280f84a0d7d1 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
> +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;
> + int irq;
'irq' not really needed, I'd think.
> + 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_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, "bus 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, "bus hung (state %x), attempting recovery\n",
> + command);
Same dbg message as in the condition? Move it out of the 'if'?
> +
> + reinit_completion(&bus->cmd_complete);
> + writel(ASPEED_I2CD_BUS_RECOVER_CMD,
> + bus->base + ASPEED_I2C_CMD_REG);
Out of interest: What does the RECOVER_CMD do?
> + 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);
> +}
...
> + 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 = -EIO;
> + 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;
> +}
You return in the interrupt handler -EIO always in case of errors. Can
you check Documentation/i2c/fault-codes and see if you can follow those
guidelines? Especially ENXIO on NACK and EAGAIN...
> +
> +static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
> +{
> + struct aspeed_i2c_bus *bus = dev_id;
> +
> + if (aspeed_i2c_master_irq(bus))
> + return IRQ_HANDLED;
> + else
> + return IRQ_NONE;
Ternary operator? Your choice, though...
And one more question: Why do you use adapter->algo_data instead of
i2c_{get|set}_adapdata?
Kind regards,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C
@ 2017-06-19 14:13 ` Wolfram Sang
0 siblings, 0 replies; 27+ messages in thread
From: Wolfram Sang @ 2017-06-19 14:13 UTC (permalink / raw)
To: Brendan Higgins
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
tglx-hfZtesqFncYOwBW4kG4KsQ, jason-NLaQJdtUoK4Be96aLqz0jA,
marc.zyngier-5wv7dgnIgG8, joel-U3u1mxZcP9KHXe+LvDLADg,
vz-ChpfBGZJDbMAvxtiuMwx3w, mouse-Pma6HLj0uuo, clg-Bxea+6Xhats,
benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r,
ryan_chen-SAlXDmAnmOAqDJ6do+/SaQ,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ
[-- Attachment #1: Type: text/plain, Size: 5152 bytes --]
Hi Brendan,
here is my review. Only minor stuff, no real show-stopper.
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 144cbadc7c72..280f84a0d7d1 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
> +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;
> + int irq;
'irq' not really needed, I'd think.
> + 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_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, "bus 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, "bus hung (state %x), attempting recovery\n",
> + command);
Same dbg message as in the condition? Move it out of the 'if'?
> +
> + reinit_completion(&bus->cmd_complete);
> + writel(ASPEED_I2CD_BUS_RECOVER_CMD,
> + bus->base + ASPEED_I2C_CMD_REG);
Out of interest: What does the RECOVER_CMD do?
> + 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);
> +}
...
> + 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 = -EIO;
> + 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;
> +}
You return in the interrupt handler -EIO always in case of errors. Can
you check Documentation/i2c/fault-codes and see if you can follow those
guidelines? Especially ENXIO on NACK and EAGAIN...
> +
> +static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
> +{
> + struct aspeed_i2c_bus *bus = dev_id;
> +
> + if (aspeed_i2c_master_irq(bus))
> + return IRQ_HANDLED;
> + else
> + return IRQ_NONE;
Ternary operator? Your choice, though...
And one more question: Why do you use adapter->algo_data instead of
i2c_{get|set}_adapdata?
Kind regards,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v10 5/5] i2c: aspeed: added slave support for Aspeed I2C driver
2017-06-03 1:29 ` [PATCH v10 5/5] i2c: aspeed: added slave support for Aspeed I2C driver Brendan Higgins
2017-06-07 6:57 ` Joel Stanley
@ 2017-06-19 14:14 ` Wolfram Sang
1 sibling, 0 replies; 27+ messages in thread
From: Wolfram Sang @ 2017-06-19 14:14 UTC (permalink / raw)
To: Brendan Higgins
Cc: robh+dt, mark.rutland, tglx, jason, marc.zyngier, joel, vz,
mouse, clg, benh, ryan_chen, linux-i2c, devicetree, linux-kernel,
openbmc
[-- Attachment #1: Type: text/plain, Size: 290 bytes --]
On Fri, Jun 02, 2017 at 06:29:52PM -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>
Nice, looks good to me!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C
2017-06-19 14:13 ` Wolfram Sang
(?)
@ 2017-06-19 14:21 ` Wolfram Sang
2017-06-20 0:58 ` Brendan Higgins
-1 siblings, 1 reply; 27+ messages in thread
From: Wolfram Sang @ 2017-06-19 14:21 UTC (permalink / raw)
To: Brendan Higgins
Cc: robh+dt, mark.rutland, tglx, jason, marc.zyngier, joel, vz,
mouse, clg, benh, ryan_chen, linux-i2c, devicetree, linux-kernel,
openbmc
[-- Attachment #1: Type: text/plain, Size: 42 bytes --]
And any chance for a MAINTAINERS entry?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C
2017-06-19 14:21 ` Wolfram Sang
@ 2017-06-20 0:58 ` Brendan Higgins
0 siblings, 0 replies; 27+ messages in thread
From: Brendan Higgins @ 2017-06-20 0:58 UTC (permalink / raw)
To: Wolfram Sang
Cc: Rob Herring, Mark Rutland, Thomas Gleixner, Jason Cooper,
Marc Zyngier, Joel Stanley, Vladimir Zapolskiy, Kachalov Anton,
Cédric Le Goater, Benjamin Herrenschmidt, Ryan Chen,
linux-i2c, devicetree, Linux Kernel Mailing List,
OpenBMC Maillist
On Mon, Jun 19, 2017 at 7:21 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>
> And any chance for a MAINTAINERS entry?
>
We already have one:
http://elixir.free-electrons.com/linux/v4.11.5/source/MAINTAINERS#L1090
However, if you would like me to own this driver, I could add an entry
for myself.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C
@ 2017-06-20 0:58 ` Brendan Higgins
0 siblings, 0 replies; 27+ messages in thread
From: Brendan Higgins @ 2017-06-20 0:58 UTC (permalink / raw)
To: Wolfram Sang
Cc: Rob Herring, Mark Rutland, Thomas Gleixner, Jason Cooper,
Marc Zyngier, Joel Stanley, Vladimir Zapolskiy, Kachalov Anton,
Cédric Le Goater, Benjamin Herrenschmidt, Ryan Chen,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Kernel Mailing List,
OpenBMC Maillist
On Mon, Jun 19, 2017 at 7:21 AM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
>
> And any chance for a MAINTAINERS entry?
>
We already have one:
http://elixir.free-electrons.com/linux/v4.11.5/source/MAINTAINERS#L1090
However, if you would like me to own this driver, I could add an entry
for myself.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C
2017-06-19 14:13 ` Wolfram Sang
@ 2017-06-20 4:02 ` Brendan Higgins
-1 siblings, 0 replies; 27+ messages in thread
From: Brendan Higgins @ 2017-06-20 4:02 UTC (permalink / raw)
To: Wolfram Sang
Cc: Rob Herring, Mark Rutland, Thomas Gleixner, Jason Cooper,
Marc Zyngier, Joel Stanley, Vladimir Zapolskiy, Kachalov Anton,
Cédric Le Goater, Benjamin Herrenschmidt, Ryan Chen,
linux-i2c, devicetree, Linux Kernel Mailing List,
OpenBMC Maillist
I thought all of the comments made sense and will be addressed in my
next revision,
except the following:
>> +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, "bus 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, "bus hung (state %x), attempting recovery\n",
>> + command);
>
> Same dbg message as in the condition? Move it out of the 'if'?
Message is the same; that's true; however, I only want to print this
if I am actually doing
the recovery. There is a case in the first condition where we actually
don't attempt recovery.
(See both SDA and SCL high). Nevertheless, now that I am looking at
this. I think it might
make sense to make the dbg statements different since I can explain
what type of recovery
I am attempting.
>
>> +
>> + reinit_completion(&bus->cmd_complete);
>> + writel(ASPEED_I2CD_BUS_RECOVER_CMD,
>> + bus->base + ASPEED_I2C_CMD_REG);
>
> Out of interest: What does the RECOVER_CMD do?
According to the documentation, it attempts to create 1 to 8 SCL
cycles to force any stuck
device to let go of the SDA line. It then puts the hardware in a sane
state once it detects
that the bus has been recovered. I will put a comment to this effect.
>
>> + 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);
>> +}
Cheers!
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C
@ 2017-06-20 4:02 ` Brendan Higgins
0 siblings, 0 replies; 27+ messages in thread
From: Brendan Higgins @ 2017-06-20 4:02 UTC (permalink / raw)
To: Wolfram Sang
Cc: Rob Herring, Mark Rutland, Thomas Gleixner, Jason Cooper,
Marc Zyngier, Joel Stanley, Vladimir Zapolskiy, Kachalov Anton,
Cédric Le Goater, Benjamin Herrenschmidt, Ryan Chen,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Kernel Mailing List,
OpenBMC Maillist
I thought all of the comments made sense and will be addressed in my
next revision,
except the following:
>> +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, "bus 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, "bus hung (state %x), attempting recovery\n",
>> + command);
>
> Same dbg message as in the condition? Move it out of the 'if'?
Message is the same; that's true; however, I only want to print this
if I am actually doing
the recovery. There is a case in the first condition where we actually
don't attempt recovery.
(See both SDA and SCL high). Nevertheless, now that I am looking at
this. I think it might
make sense to make the dbg statements different since I can explain
what type of recovery
I am attempting.
>
>> +
>> + reinit_completion(&bus->cmd_complete);
>> + writel(ASPEED_I2CD_BUS_RECOVER_CMD,
>> + bus->base + ASPEED_I2C_CMD_REG);
>
> Out of interest: What does the RECOVER_CMD do?
According to the documentation, it attempts to create 1 to 8 SCL
cycles to force any stuck
device to let go of the SDA line. It then puts the hardware in a sane
state once it detects
that the bus has been recovered. I will put a comment to this effect.
>
>> + 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);
>> +}
Cheers!
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C
2017-06-20 0:58 ` Brendan Higgins
(?)
@ 2017-06-20 7:49 ` Wolfram Sang
-1 siblings, 0 replies; 27+ messages in thread
From: Wolfram Sang @ 2017-06-20 7:49 UTC (permalink / raw)
To: Brendan Higgins
Cc: Rob Herring, Mark Rutland, Thomas Gleixner, Jason Cooper,
Marc Zyngier, Joel Stanley, Vladimir Zapolskiy, Kachalov Anton,
Cédric Le Goater, Benjamin Herrenschmidt, Ryan Chen,
linux-i2c, devicetree, Linux Kernel Mailing List,
OpenBMC Maillist
[-- Attachment #1: Type: text/plain, Size: 653 bytes --]
On Mon, Jun 19, 2017 at 05:58:19PM -0700, Brendan Higgins wrote:
> On Mon, Jun 19, 2017 at 7:21 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
> >
> > And any chance for a MAINTAINERS entry?
> >
>
> We already have one:
> http://elixir.free-electrons.com/linux/v4.11.5/source/MAINTAINERS#L1090
>
> However, if you would like me to own this driver, I could add an entry
> for myself.
As far as get_maintainers.pl prints out someone responsibe, I am very
happy. The above line won't work for I2C (and maybe some other
subsystems), though, because we are one directory level deeper
(drivers/i2c/busses/*aspeed*). That might want fixing.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2017-06-20 7:49 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-03 1:29 [PATCH v10 0/5] i2c: aspeed: added driver for Aspeed I2C Brendan Higgins
2017-06-03 1:29 ` [PATCH v10 1/5] irqchip/aspeed-i2c-ic: binding docs for Aspeed I2C Interrupt Controller Brendan Higgins
2017-06-05 6:25 ` Joel Stanley
2017-06-07 11:31 ` Marc Zyngier
2017-06-03 1:29 ` [PATCH v10 2/5] irqchip/aspeed-i2c-ic: Add I2C IRQ controller for Aspeed Brendan Higgins
2017-06-05 4:56 ` Andrew Jeffery
2017-06-05 4:56 ` Andrew Jeffery
2017-06-03 1:29 ` [PATCH v10 3/5] i2c: aspeed: added documentation for Aspeed I2C driver Brendan Higgins
2017-06-05 4:40 ` Joel Stanley
2017-06-03 1:29 ` [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C Brendan Higgins
2017-06-05 4:40 ` Joel Stanley
2017-06-05 4:40 ` Joel Stanley
2017-06-05 5:02 ` Andrew Jeffery
2017-06-05 5:02 ` Andrew Jeffery
2017-06-15 23:42 ` Brendan Higgins
2017-06-15 23:42 ` Brendan Higgins
2017-06-19 14:13 ` Wolfram Sang
2017-06-19 14:13 ` Wolfram Sang
2017-06-19 14:21 ` Wolfram Sang
2017-06-20 0:58 ` Brendan Higgins
2017-06-20 0:58 ` Brendan Higgins
2017-06-20 7:49 ` Wolfram Sang
2017-06-20 4:02 ` Brendan Higgins
2017-06-20 4:02 ` Brendan Higgins
2017-06-03 1:29 ` [PATCH v10 5/5] i2c: aspeed: added slave support for Aspeed I2C driver Brendan Higgins
2017-06-07 6:57 ` Joel Stanley
2017-06-19 14:14 ` Wolfram Sang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.