* [PATCH v8 0/2] i2c: Add driver support Amlogic A9 I2C
@ 2026-11-10 8:19 Xianwei Zhao via B4 Relay
2026-11-10 8:19 ` [PATCH v8 1/2] dt-bindings: i2c: add Amlogic A9 I2C controller Xianwei Zhao via B4 Relay
2026-11-10 8:19 ` [PATCH v8 2/2] i2c: amlogic: Add Amlogic A9 I2C controller driver Xianwei Zhao via B4 Relay
0 siblings, 2 replies; 5+ messages in thread
From: Xianwei Zhao via B4 Relay @ 2026-11-10 8:19 UTC (permalink / raw)
To: Junyi Zhao, Andi Shyti, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Xianwei Zhao
Cc: Junyi Zhao, linux-i2c, linux-amlogic, devicetree, linux-kernel,
Xianwei Zhao, Conor Dooley
Add a Device Tree binding and driver for the I2C controller found on the
Amlogic A9 SoC.
Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
Changes in v8:
- Reserve complete(done) into lock, and adjust the code process in irq func.
- Link to v7: https://lore.kernel.org/r/20261104-a9-i2c-v7-0-b31f2b53faf4@amlogic.com
Changes in v7:
- Move complete(done) out of lock, and when state = idle not complete(done) in irq func.
- Link to v6: https://lore.kernel.org/r/20261104-a9-i2c-v6-0-baa6171f26f8@amlogic.com
Changes in v6:
- use xfer instead of master_xfer interface, and when div=8 remain valid.
- Optimized the function.
- Link to v5: https://lore.kernel.org/r/20261103-a9-i2c-v5-0-86f2dcd179d8@amlogic.com
Changes in v5:
- Mask out I2C_FUNC_SMBUS_QUICK and return I2C_FUNC_I2C | (I2C_FUNC_SMBUS_EMUL & ~I2C_FUNC_SMBUS_QUICK).
- Link to v4: https://lore.kernel.org/r/20261102-a9-i2c-v4-0-5a169215b8e9@amlogic.com
Changes in v4:
- Repocess xfer len equal 0 use I2C_AQ_NO_ZERO_LEN, and Optimized the function.
- Disable A9_RX_THRESH_READ/A9_TX_THRESH_WRITE enables of irq when xfer timeout.
- Use guard(spinlock_irqsave) instead for direct call spinlock in xfer timeout process.
- Link to v3: https://lore.kernel.org/r/20261029-a9-i2c-v3-0-7b30ccc00533@amlogic.com
Changes in v3:
- Based on the questions raised by the robot, the data flow was reorganized.
- Some processing was optimized according to Andi's and Markus's suggestions.
- Link to v2: https://lore.kernel.org/r/20261022-a9-i2c-v2-0-45e3cef175bf@amlogic.com
Changes in v2:
- dtbinding: make some minor modifications, suggestted by Conor and robot.
- driver: Make some adjustments, including macro definitions, format adjustments,
adding quirks, process optimization, and so on. According to Andi and robot's suggestion
- Give up MAINTAINERS file the separate submission, make it together with the driver.
- Link to v1: https://lore.kernel.org/r/20260924-a9-i2c-v1-0-b8ad9b46f4a8@amlogic.com
---
Junyi Zhao (1):
i2c: amlogic: Add Amlogic A9 I2C controller driver
Xianwei Zhao (1):
dt-bindings: i2c: add Amlogic A9 I2C controller
.../devicetree/bindings/i2c/amlogic,a9-i2c.yaml | 56 +++
MAINTAINERS | 9 +
drivers/i2c/busses/Kconfig | 8 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-amlogic-a9.c | 430 +++++++++++++++++++++
5 files changed, 504 insertions(+)
---
base-commit: 4ec2365c08de306f52650d8736aef5862baedb8d
change-id: 20260911-a9-i2c-24478e22e683
Best regards,
--
Xianwei Zhao <xianwei.zhao@amlogic.com>
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v8 1/2] dt-bindings: i2c: add Amlogic A9 I2C controller 2026-11-10 8:19 [PATCH v8 0/2] i2c: Add driver support Amlogic A9 I2C Xianwei Zhao via B4 Relay @ 2026-11-10 8:19 ` Xianwei Zhao via B4 Relay 2026-11-10 8:19 ` [PATCH v8 2/2] i2c: amlogic: Add Amlogic A9 I2C controller driver Xianwei Zhao via B4 Relay 1 sibling, 0 replies; 5+ messages in thread From: Xianwei Zhao via B4 Relay @ 2026-11-10 8:19 UTC (permalink / raw) To: Junyi Zhao, Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Xianwei Zhao Cc: Junyi Zhao, linux-i2c, linux-amlogic, devicetree, linux-kernel, Xianwei Zhao, Conor Dooley From: Xianwei Zhao <xianwei.zhao@amlogic.com> Add a Device Tree binding for the I2C controller found on the Amlogic A9 SoC. Acked-by: Conor Dooley <conor.dooley@microchip.com> Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com> --- .../devicetree/bindings/i2c/amlogic,a9-i2c.yaml | 56 ++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/Documentation/devicetree/bindings/i2c/amlogic,a9-i2c.yaml b/Documentation/devicetree/bindings/i2c/amlogic,a9-i2c.yaml new file mode 100644 index 000000000000..31ddc6d23732 --- /dev/null +++ b/Documentation/devicetree/bindings/i2c/amlogic,a9-i2c.yaml @@ -0,0 +1,56 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/i2c/amlogic,a9-i2c.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Amlogic A9 I2C Controller + +maintainers: + - Xianwei Zhao <xianwei.zhao@amlogic.com> + - Junyi Zhao <junyi.zhao@amlogic.com> + +allOf: + - $ref: /schemas/i2c/i2c-controller.yaml# + +properties: + compatible: + const: amlogic,a9-i2c + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + maxItems: 1 + + dmas: + items: + - description: DMA controller phandle and request line for RX + - description: DMA controller phandle and request line for TX + + dma-names: + items: + - const: rx + - const: tx + +required: + - compatible + - reg + - interrupts + - clocks + +unevaluatedProperties: false + +examples: + - | + i2c@3a4400 { + compatible = "amlogic,a9-i2c"; + reg = <0x3a4400 0x3c>; + interrupts = <0 164 1>; + clocks = <&clkc_periphs 11>; + #address-cells = <1>; + #size-cells = <0>; + }; -- 2.52.0 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v8 2/2] i2c: amlogic: Add Amlogic A9 I2C controller driver 2026-11-10 8:19 [PATCH v8 0/2] i2c: Add driver support Amlogic A9 I2C Xianwei Zhao via B4 Relay 2026-11-10 8:19 ` [PATCH v8 1/2] dt-bindings: i2c: add Amlogic A9 I2C controller Xianwei Zhao via B4 Relay @ 2026-11-10 8:19 ` Xianwei Zhao via B4 Relay 2026-11-10 8:41 ` sashiko-bot 1 sibling, 1 reply; 5+ messages in thread From: Xianwei Zhao via B4 Relay @ 2026-11-10 8:19 UTC (permalink / raw) To: Junyi Zhao, Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Xianwei Zhao Cc: Junyi Zhao, linux-i2c, linux-amlogic, devicetree, linux-kernel, Xianwei Zhao From: Junyi Zhao <junyi.zhao@amlogic.com> Add support for the new I2C controller used by Amlogic A9 SoCs. Add a dedicated Kconfig option and Makefile entry for the A9 I2C controller driver. And add Amlogic I2C controller entry to MAINTAINERS to clarify the maintainers. Signed-off-by: Junyi Zhao <junyi.zhao@amlogic.com> Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com> --- MAINTAINERS | 9 + drivers/i2c/busses/Kconfig | 8 + drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-amlogic-a9.c | 430 ++++++++++++++++++++++++++++++++++++ 4 files changed, 448 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index a2d702919ef3..a56bdfefabad 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1371,6 +1371,15 @@ F: Documentation/devicetree/bindings/perf/amlogic,g12-ddr-pmu.yaml F: drivers/perf/amlogic/ F: include/soc/amlogic/ +AMLOGIC I2C DRIVER +M: Junyi Zhao <junyi.zhao@amlogic.com> +M: Xianwei Zhao <xianwei.zhao@amlogic.com> +L: linux-i2c@vger.kernel.org +L: linux-amlogic@lists.infradead.org +S: Maintained +F: Documentation/devicetree/bindings/i2c/amlogic,a9-i2c.yaml +F: drivers/i2c/busses/i2c-amlogic-a9.c + AMLOGIC ISP DRIVER M: Keke Li <keke.li@amlogic.com> L: linux-media@vger.kernel.org diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 275aaa7f3583..50e031d97ed3 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -885,6 +885,14 @@ config I2C_MESON If you say yes to this option, support will be included for the I2C interface on the Amlogic Meson family of SoCs. +config I2C_AMLOGIC_A9 + tristate "Amlogic new I2C controller" + depends on ARCH_MESON || COMPILE_TEST + depends on COMMON_CLK + help + If you say yes to this option, support will be included for the + I2C interface on the new Amlogic family of SoCs. + config I2C_MICROCHIP_CORE tristate "Microchip FPGA I2C controller" depends on ARCH_MICROCHIP_POLARFIRE || COMPILE_TEST diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile index e5440de0fe14..343eec4b0889 100644 --- a/drivers/i2c/busses/Makefile +++ b/drivers/i2c/busses/Makefile @@ -82,6 +82,7 @@ obj-$(CONFIG_I2C_LPC2K) += i2c-lpc2k.o obj-$(CONFIG_I2C_LS2X) += i2c-ls2x.o obj-$(CONFIG_I2C_LS2X_V2) += i2c-ls2x-v2.o obj-$(CONFIG_I2C_MESON) += i2c-meson.o +obj-$(CONFIG_I2C_AMLOGIC_A9) += i2c-amlogic-a9.o obj-$(CONFIG_I2C_MICROCHIP_CORE) += i2c-microchip-corei2c.o obj-$(CONFIG_I2C_MPC) += i2c-mpc.o obj-$(CONFIG_I2C_MT65XX) += i2c-mt65xx.o diff --git a/drivers/i2c/busses/i2c-amlogic-a9.c b/drivers/i2c/busses/i2c-amlogic-a9.c new file mode 100644 index 000000000000..3e08a0333cee --- /dev/null +++ b/drivers/i2c/busses/i2c-amlogic-a9.c @@ -0,0 +1,430 @@ +// SPDX-License-Identifier: (GPL-2.0-only OR MIT) +/* + * Copyright (C) 2026 Amlogic, Inc. All rights reserved + */ + +#include <linux/clk.h> +#include <linux/completion.h> +#include <linux/i2c.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> + +/* Amlogic I2C register map */ +#define REG_CFG_RDY 0x00 +#define REG_CFG_I2C 0x04 +#define REG_CFG_START 0x08 +#define REG_CFG_BUS 0x0c +#define REG_TX_RD_ADDR 0x10 +#define REG_TX_WR_ADDR 0x14 +#define REG_RX_RD_ADDR 0x18 +#define REG_RX_WR_ADDR 0x1c +#define REG_CFG_TX 0x20 +#define REG_CFG_RX 0x24 +#define REG_CFG_IRQ_STATE 0x30 +#define REG_CFG_IRQ_ENABLE 0x34 +#define REG_SHAKE_BLK_CNT 0x38 + +/* CFG RDY fields */ +#define RDY_TEE_ONLY BIT(1) +#define RDY_IF BIT(0) + +/* CFG I2C fields */ +#define I2C_RX_THR GENMASK(23, 16) +#define I2C_TX_THR GENMASK(15, 8) + +/* CFG BUS fields */ +#define BUS_NO_STOP BIT(18) + +/* SCL = clk/b_ratio if b_ratio<=8, SCL = clk/8 */ +#define BUS_RATIO_MASK GENMASK(11, 0) + +/* CFG START fields */ +#define START_START BIT(31) +#define START_LEN GENMASK(23, 12) +#define START_LEN_SHIFT 12 +#define START_SLAVE_ADDR GENMASK(10, 1) +#define START_SADDR_SHIFT 1 +#define START_READ BIT(0) + +/* CFG TX_RX fields */ +#define TX_RX_DATA GENMASK(7, 0) + +/* CFG IRQ fields */ +#define IRQ_ALL_MASK GENMASK(15, 0) +#define A9_NCK_ERROR BIT(0) +#define A9_RX_THRESH_READ BIT(5) +#define A9_TX_THRESH_WRITE BIT(6) +#define A9_PHY_DONE BIT(7) +#define A9_TASK_DONE BIT(8) +#define A9_ALL_DONE BIT(9) + +#define A9_TRANS_DONE (A9_PHY_DONE | A9_NCK_ERROR) +#define A9_TRANS_ERROR (A9_NCK_ERROR) +#define A9_ENABLE_IRQ_BIT (A9_NCK_ERROR | A9_PHY_DONE) +#define THRESH_MODE (A9_RX_THRESH_READ | A9_TX_THRESH_WRITE) + +#define A9_I2C_FIFO_DEPTH 32 +#define A9_I2C_HALF_FIFO (A9_I2C_FIFO_DEPTH >> 1) + +#define A9_I2C_MAX_XFER_SIZE 0xfff + +#define I2C_TIMEOUT_MS 500 + +enum { + A9_STATE_IDLE, + A9_STATE_READ, + A9_STATE_WRITE, +}; + +enum fifo_fill_mode { + A9_FIFO_FILL_FULL, + A9_FIFO_FILL_HALF, +}; + +/** + * struct aml_i2c - Amlogic I2C device private data + * + * @adap: I2C adapter instance + * @dev: Pointer to device structure + * @regs: Base address of the device memory mapped registers + * @clk: Pointer to clock structure + * @msg: Pointer to the current I2C message + * @state: Current state in the driver state machine + * @pos: Current position in the send/receive buffer + * @error: Flag set when an error is received + * @lock: To avoid race conditions between irq handler and xfer code + * @done: Completion used to wait for transfer termination + * @clk_rate: Clock source rate of I2C module + * @frequency: Operating frequency of I2C bus clock + */ +struct aml_i2c { + struct i2c_adapter adap; + struct device *dev; + void __iomem *regs; + struct clk *clk; + + struct i2c_msg *msg; + int state; + int pos; + int error; + /* protect reg access */ + spinlock_t lock; + struct completion done; + unsigned long clk_rate; + unsigned int frequency; +}; + +static void aml_i2c_set_mask(struct aml_i2c *i2c, int reg, u32 mask, u32 val) +{ + u32 data; + + data = readl(i2c->regs + reg); + data &= ~mask; + data |= val & mask; + writel(data, i2c->regs + reg); +} + +/* + * SCL = clk/b_ratio. if b_ratio<=8, SCL = clk/8 + */ +static int aml_i2c_set_clk_div(struct aml_i2c *i2c) +{ + unsigned long clk_rate = i2c->clk_rate; + unsigned int div; + + div = DIV_ROUND_UP(clk_rate, i2c->frequency); + + /* clock divider has 12 bits */ + if (div >= (1 << 12)) { + dev_err(i2c->dev, "requested bus frequency too low\n"); + return -EINVAL; + } else if (div < 8) { + dev_err(i2c->dev, "requested bus frequency too high\n"); + return -EINVAL; + } + /* CFG_BUS reg:11 - 0 bits */ + aml_i2c_set_mask(i2c, REG_CFG_BUS, BUS_RATIO_MASK, div); + + return 0; +} + +static void reset_fifo_pos(struct aml_i2c *i2c) +{ + writel(0x00, i2c->regs + REG_TX_RD_ADDR); + writel(0x00, i2c->regs + REG_TX_WR_ADDR); + writel(0x00, i2c->regs + REG_RX_WR_ADDR); + writel(0x00, i2c->regs + REG_RX_RD_ADDR); +} + +static void clear_irq_status(struct aml_i2c *i2c, u32 irq_bits) +{ + writel(irq_bits, i2c->regs + REG_CFG_IRQ_STATE); +} + +static int aml_i2c_init(struct aml_i2c *i2c) +{ + clear_irq_status(i2c, IRQ_ALL_MASK); + aml_i2c_set_mask(i2c, REG_CFG_BUS, BUS_NO_STOP, BUS_NO_STOP); + aml_i2c_set_mask(i2c, REG_CFG_I2C, I2C_RX_THR | I2C_TX_THR, + (A9_I2C_FIFO_DEPTH << 16) | (A9_I2C_HALF_FIFO << 8)); + reset_fifo_pos(i2c); + + return aml_i2c_set_clk_div(i2c); +} + +static void aml_i2c_prepare_xfer(struct aml_i2c *i2c, enum fifo_fill_mode mode) +{ + bool write = !(i2c->msg->flags & I2C_M_RD); + int i, count; + + if (!write) + return; + + count = min(i2c->msg->len - i2c->pos, + mode == A9_FIFO_FILL_FULL ? A9_I2C_FIFO_DEPTH : A9_I2C_HALF_FIFO); + + for (i = 0; i < count; i++) + writel(i2c->msg->buf[i2c->pos++], i2c->regs + REG_CFG_TX); +} + +static irqreturn_t aml_i2c_irq(int irqno, void *dev_id) +{ + struct aml_i2c *i2c = dev_id; + u8 *buf = NULL; + u32 rd_len, i; + u32 rd_val; + u32 irq_state; + + spin_lock(&i2c->lock); + irq_state = readl(i2c->regs + REG_CFG_IRQ_STATE); + if (i2c->state == A9_STATE_IDLE) { + dev_warn(i2c->dev, "i2c irq triggered abnormally, irq_state:0x%x,irq_en:0x%x\n", + irq_state, readl(i2c->regs + REG_CFG_IRQ_ENABLE)); + goto irq_trans_out; + } + + if (irq_state & A9_TRANS_ERROR) { + i2c->error = -ENXIO; + goto complete_irq_out; + } + + switch (i2c->state) { + case A9_STATE_READ: + buf = i2c->msg->buf; + rd_len = min(i2c->msg->len - i2c->pos, A9_I2C_FIFO_DEPTH); + for (i = 0; i < rd_len; i++) { + rd_val = readl(i2c->regs + REG_CFG_RX); + buf[i2c->pos++] = rd_val & TX_RX_DATA; + } + /* Rx must clear thresh after read fifo */ + clear_irq_status(i2c, A9_RX_THRESH_READ); + break; + case A9_STATE_WRITE: + aml_i2c_prepare_xfer(i2c, A9_FIFO_FILL_HALF); + if (i2c->pos == i2c->msg->len) + aml_i2c_set_mask(i2c, REG_CFG_IRQ_ENABLE, A9_TX_THRESH_WRITE, 0); + /* Tx must clear thresh after write fifo */ + clear_irq_status(i2c, A9_TX_THRESH_WRITE); + break; + default: + break; + } + + if (!(irq_state & A9_TRANS_DONE) || + (i2c->state == A9_STATE_READ && i2c->pos < i2c->msg->len)) { + spin_unlock(&i2c->lock); + return IRQ_HANDLED; + } + +complete_irq_out: + i2c->state = A9_STATE_IDLE; + reset_fifo_pos(i2c); + aml_i2c_set_mask(i2c, REG_CFG_IRQ_ENABLE, A9_RX_THRESH_READ | A9_TX_THRESH_WRITE, 0); + complete(&i2c->done); +irq_trans_out: + clear_irq_status(i2c, IRQ_ALL_MASK); + spin_unlock(&i2c->lock); + + return IRQ_HANDLED; +} + +static void aml_i2c_do_start(struct aml_i2c *i2c, struct i2c_msg *msg) +{ + unsigned int reg_val; + unsigned int trans_len; + + trans_len = msg->len; + reg_val = (unsigned int)(((msg->addr & 0x7f) << START_SADDR_SHIFT) | + ((msg->flags & I2C_M_RD) ? START_READ : 0) | + ((trans_len & 0xfff) << START_LEN_SHIFT) | START_START); + aml_i2c_set_mask(i2c, REG_CFG_START, + START_START | START_SLAVE_ADDR | START_READ | START_LEN, reg_val); + clear_irq_status(i2c, IRQ_ALL_MASK); + /* enable thresh mode */ + if (trans_len > A9_I2C_FIFO_DEPTH) { + reg_val = (msg->flags & I2C_M_RD) ? A9_RX_THRESH_READ : A9_TX_THRESH_WRITE; + aml_i2c_set_mask(i2c, REG_CFG_IRQ_ENABLE, + A9_RX_THRESH_READ | A9_TX_THRESH_WRITE, reg_val); + } +} + +static int aml_i2c_xfer_msg(struct aml_i2c *i2c, struct i2c_msg *msg, bool last) +{ + unsigned long time_left, flags; + + i2c->msg = msg; + i2c->pos = 0; + i2c->error = 0; + + spin_lock_irqsave(&i2c->lock, flags); + i2c->state = (msg->flags & I2C_M_RD) ? A9_STATE_READ : A9_STATE_WRITE; + aml_i2c_prepare_xfer(i2c, A9_FIFO_FILL_FULL); + reinit_completion(&i2c->done); + + aml_i2c_set_mask(i2c, REG_CFG_BUS, BUS_NO_STOP, (last ? 0 : BUS_NO_STOP)); + + /* Start the transfer */ + aml_i2c_do_start(i2c, msg); + spin_unlock_irqrestore(&i2c->lock, flags); + + time_left = msecs_to_jiffies(I2C_TIMEOUT_MS); + time_left = wait_for_completion_timeout(&i2c->done, time_left); + + /* + * Protect access to i2c struct and registers from interrupt + * handlers triggered by a transfer terminated after the + * timeout period + */ + + if (!time_left) { + spin_lock_irqsave(&i2c->lock, flags); + i2c->state = A9_STATE_IDLE; + aml_i2c_set_mask(i2c, REG_CFG_IRQ_ENABLE, + A9_RX_THRESH_READ | A9_TX_THRESH_WRITE, 0); + clear_irq_status(i2c, IRQ_ALL_MASK); + reset_fifo_pos(i2c); + spin_unlock_irqrestore(&i2c->lock, flags); + return -ETIMEDOUT; + } + + return i2c->error; +} + +static int aml_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) +{ + struct aml_i2c *i2c = adap->algo_data; + int i, ret = 0; + + for (i = 0; i < num; i++) { + ret = aml_i2c_xfer_msg(i2c, msgs + i, i == num - 1); + if (ret) + return ret; + } + + return i; +} + +static u32 aml_i2c_func(struct i2c_adapter *adap) +{ + return I2C_FUNC_I2C | (I2C_FUNC_SMBUS_EMUL & ~I2C_FUNC_SMBUS_QUICK); +} + +static const struct i2c_adapter_quirks aml_i2c_quirks = { + .max_read_len = A9_I2C_MAX_XFER_SIZE, + .max_write_len = A9_I2C_MAX_XFER_SIZE, + .flags = I2C_AQ_NO_ZERO_LEN, +}; + +static const struct i2c_algorithm aml_i2c_algorithm = { + .xfer = aml_i2c_xfer, + .functionality = aml_i2c_func, +}; + +static int aml_i2c_probe(struct platform_device *pdev) +{ + struct device_node *np = pdev->dev.of_node; + struct aml_i2c *i2c; + struct i2c_timings timings; + int irq, ret; + + i2c = devm_kzalloc(&pdev->dev, sizeof(struct aml_i2c), GFP_KERNEL); + if (!i2c) + return -ENOMEM; + + i2c_parse_fw_timings(&pdev->dev, &timings, true); + i2c->frequency = timings.bus_freq_hz; + + i2c->dev = &pdev->dev; + platform_set_drvdata(pdev, i2c); + + spin_lock_init(&i2c->lock); + init_completion(&i2c->done); + + i2c->regs = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(i2c->regs)) + return PTR_ERR(i2c->regs); + + i2c->clk = devm_clk_get_enabled(&pdev->dev, NULL); + if (IS_ERR(i2c->clk)) + return dev_err_probe(&pdev->dev, PTR_ERR(i2c->clk), "can't get enable clock\n"); + + i2c->clk_rate = clk_get_rate(i2c->clk); + if (i2c->clk_rate == 0) + return dev_err_probe(&pdev->dev, -EINVAL, "failed to get clk rate\n"); + + ret = aml_i2c_init(i2c); + if (ret < 0) + return dev_err_probe(&pdev->dev, ret, "fail init i2c\n"); + + irq = platform_get_irq(pdev, 0); + if (irq < 0) + return dev_err_probe(&pdev->dev, irq, "can't find IRQ\n"); + + ret = devm_request_irq(&pdev->dev, irq, aml_i2c_irq, 0, dev_name(&pdev->dev), i2c); + if (ret < 0) + return dev_err_probe(&pdev->dev, ret, "can't request IRQ\n"); + /* Enable error & done irq */ + writel(A9_ENABLE_IRQ_BIT, i2c->regs + REG_CFG_IRQ_ENABLE); + strscpy(i2c->adap.name, "Amlogic I2C adapter", + sizeof(i2c->adap.name)); + i2c->adap.owner = THIS_MODULE; + i2c->adap.algo = &aml_i2c_algorithm; + i2c->adap.quirks = &aml_i2c_quirks; + i2c->adap.dev.parent = &pdev->dev; + i2c->adap.dev.of_node = np; + i2c->adap.algo_data = i2c; + + return i2c_add_adapter(&i2c->adap); +} + +static void aml_i2c_remove(struct platform_device *pdev) +{ + struct aml_i2c *i2c = platform_get_drvdata(pdev); + + i2c_del_adapter(&i2c->adap); +} + +static const struct of_device_id aml_i2c_match[] = { + { .compatible = "amlogic,a9-i2c",}, + { } +}; + +MODULE_DEVICE_TABLE(of, aml_i2c_match); + +static struct platform_driver aml_i2c_driver = { + .probe = aml_i2c_probe, + .remove = aml_i2c_remove, + .driver = { + .name = "amlogic-i2c", + .of_match_table = aml_i2c_match, + }, +}; + +module_platform_driver(aml_i2c_driver); + +MODULE_DESCRIPTION("Amlogic I2C Bus driver"); +MODULE_AUTHOR("Junyi Zhao <junyi.zhao@amlogic.com>"); +MODULE_LICENSE("GPL"); -- 2.52.0 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v8 2/2] i2c: amlogic: Add Amlogic A9 I2C controller driver 2026-11-10 8:19 ` [PATCH v8 2/2] i2c: amlogic: Add Amlogic A9 I2C controller driver Xianwei Zhao via B4 Relay @ 2026-11-10 8:41 ` sashiko-bot 2026-11-11 5:36 ` Xianwei Zhao 0 siblings, 1 reply; 5+ messages in thread From: sashiko-bot @ 2026-11-10 8:41 UTC (permalink / raw) To: Xianwei Zhao; +Cc: linux-amlogic, devicetree, robh, conor+dt, neil.armstrong Thank you for your contribution! Sashiko AI review found 2 potential issues to consider: - [Medium] i2c: amlogic-a9: transfer started before IRQ setup in aml_i2c_do_start() - [Medium] i2c: amlogic-a9: stale irq_state and edge IRQ miss in aml_i2c_irq() -- commit 75985e9b577bcde7a76a8a6f844a5a7564d2426a Author: Junyi Zhao <junyi.zhao@amlogic.com> i2c: amlogic: Add Amlogic A9 I2C controller driver This commit adds an I2C controller driver for Amlogic A9 SoCs along with its Kconfig, Makefile, and MAINTAINERS entries. > diff --git a/drivers/i2c/busses/i2c-amlogic-a9.c b/drivers/i2c/busses/i2c-amlogic-a9.c > new file mode 100644 > index 0000000000000..3e08a0333ceea > --- /dev/null > +++ b/drivers/i2c/busses/i2c-amlogic-a9.c [ ... ] > +static irqreturn_t aml_i2c_irq(int irqno, void *dev_id) > +{ > + struct aml_i2c *i2c = dev_id; > + u8 *buf = NULL; > + u32 rd_len, i; > + u32 rd_val; > + u32 irq_state; > + > + spin_lock(&i2c->lock); > + irq_state = readl(i2c->regs + REG_CFG_IRQ_STATE); > + if (i2c->state == A9_STATE_IDLE) { > + dev_warn(i2c->dev, "i2c irq triggered abnormally, irq_state:0x%x,irq_en:0x%x\n", > + irq_state, readl(i2c->regs + REG_CFG_IRQ_ENABLE)); > + goto irq_trans_out; > + } > + > + if (irq_state & A9_TRANS_ERROR) { > + i2c->error = -ENXIO; > + goto complete_irq_out; > + } > + > + switch (i2c->state) { > + case A9_STATE_READ: > + buf = i2c->msg->buf; > + rd_len = min(i2c->msg->len - i2c->pos, A9_I2C_FIFO_DEPTH); > + for (i = 0; i < rd_len; i++) { > + rd_val = readl(i2c->regs + REG_CFG_RX); > + buf[i2c->pos++] = rd_val & TX_RX_DATA; > + } > + /* Rx must clear thresh after read fifo */ > + clear_irq_status(i2c, A9_RX_THRESH_READ); > + break; > + case A9_STATE_WRITE: > + aml_i2c_prepare_xfer(i2c, A9_FIFO_FILL_HALF); > + if (i2c->pos == i2c->msg->len) > + aml_i2c_set_mask(i2c, REG_CFG_IRQ_ENABLE, A9_TX_THRESH_WRITE, 0); > + /* Tx must clear thresh after write fifo */ > + clear_irq_status(i2c, A9_TX_THRESH_WRITE); > + break; > + default: > + break; > + } > + > + if (!(irq_state & A9_TRANS_DONE) || > + (i2c->state == A9_STATE_READ && i2c->pos < i2c->msg->len)) { > + spin_unlock(&i2c->lock); > + return IRQ_HANDLED; > + } [Severity: Medium] Can multi-chunk reads (msg->len > A9_I2C_FIFO_DEPTH) miss the completion interrupt and time out in aml_i2c_xfer_msg()? Documentation/devicetree/bindings/i2c/amlogic,a9-i2c.yaml: interrupts = <0 164 1>; The interrupt is rising-edge-triggered (IRQ_TYPE_EDGE_RISING), so the controller's enabled interrupt output must drop to 0 and transition back to 1 to latch a new interrupt in the GIC. In aml_i2c_irq(), irq_state is sampled once before draining up to 32 bytes from REG_CFG_RX. On a 64-byte read, the 32nd byte of the second chunk asserts A9_RX_THRESH_READ before the final NACK and STOP finish on the bus. If A9_PHY_DONE asserts while the handler is reading REG_CFG_RX, prior to clear_irq_status(i2c, A9_RX_THRESH_READ), the interrupt output stays continuously high across the clear without generating a new rising edge, while aml_i2c_irq() checks the stale irq_state snapshot and returns without completing i2c->done. Similarly, if both A9_RX_THRESH_READ and A9_PHY_DONE are already set in irq_state while more than 32 bytes remain to be read, aml_i2c_irq() reads 32 bytes, clears only A9_RX_THRESH_READ, and returns early because i2c->pos < i2c->msg->len. Because A9_PHY_DONE remains asserted in REG_CFG_IRQ_STATE and fewer than 32 tail bytes remain in the FIFO, no new rising edge is generated and wait_for_completion_timeout() hangs for 500 ms before returning -ETIMEDOUT. > + > +complete_irq_out: > + i2c->state = A9_STATE_IDLE; > + reset_fifo_pos(i2c); > + aml_i2c_set_mask(i2c, REG_CFG_IRQ_ENABLE, A9_RX_THRESH_READ | A9_TX_THRESH_WRITE, 0); > + complete(&i2c->done); > +irq_trans_out: > + clear_irq_status(i2c, IRQ_ALL_MASK); > + spin_unlock(&i2c->lock); > + > + return IRQ_HANDLED; > +} > + > +static void aml_i2c_do_start(struct aml_i2c *i2c, struct i2c_msg *msg) > +{ > + unsigned int reg_val; > + unsigned int trans_len; > + > + trans_len = msg->len; > + reg_val = (unsigned int)(((msg->addr & 0x7f) << START_SADDR_SHIFT) | > + ((msg->flags & I2C_M_RD) ? START_READ : 0) | > + ((trans_len & 0xfff) << START_LEN_SHIFT) | START_START); > + aml_i2c_set_mask(i2c, REG_CFG_START, > + START_START | START_SLAVE_ADDR | START_READ | START_LEN, reg_val); > + clear_irq_status(i2c, IRQ_ALL_MASK); > + /* enable thresh mode */ > + if (trans_len > A9_I2C_FIFO_DEPTH) { > + reg_val = (msg->flags & I2C_M_RD) ? A9_RX_THRESH_READ : A9_TX_THRESH_WRITE; > + aml_i2c_set_mask(i2c, REG_CFG_IRQ_ENABLE, > + A9_RX_THRESH_READ | A9_TX_THRESH_WRITE, reg_val); > + } > +} [Severity: Medium] Does writing START_START to REG_CFG_START before clearing IRQ_ALL_MASK and enabling threshold interrupts race with hardware completion or NACK? If execution is delayed between writing REG_CFG_START and calling clear_irq_status(i2c, IRQ_ALL_MASK) (for example due to vCPU preemption or PREEMPT_RT spinlock preemption), an address NACK or short transfer can assert A9_NCK_ERROR or A9_PHY_DONE and then have its status bits wiped out by clear_irq_status(). When aml_i2c_irq() then runs, it reads irq_state == 0, misses A9_TRANS_ERROR or A9_TRANS_DONE, and returns without completing i2c->done, causing wait_for_completion_timeout() to time out. Clearing REG_CFG_IRQ_STATE and configuring REG_CFG_IRQ_ENABLE before asserting START_START avoids this window. -- Sashiko AI review · https://sashiko.dev/#/patchset/20261110-a9-i2c-v8-0-1e9d2682f86b@amlogic.com?part=2 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v8 2/2] i2c: amlogic: Add Amlogic A9 I2C controller driver 2026-11-10 8:41 ` sashiko-bot @ 2026-11-11 5:36 ` Xianwei Zhao 0 siblings, 0 replies; 5+ messages in thread From: Xianwei Zhao @ 2026-11-11 5:36 UTC (permalink / raw) To: sashiko-reviews; +Cc: linux-amlogic, devicetree, robh, conor+dt, neil.armstrong Hi, On 2026/11/10 16:41, sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 2 potential > issues to consider: > - [Medium] i2c: amlogic-a9: transfer started before IRQ setup in > aml_i2c_do_start() > - [Medium] i2c: amlogic-a9: stale irq_state and edge IRQ miss in > aml_i2c_irq() > -- > > commit 75985e9b577bcde7a76a8a6f844a5a7564d2426a > Author: Junyi Zhao<junyi.zhao@amlogic.com> > > i2c: amlogic: Add Amlogic A9 I2C controller driver > > This commit adds an I2C controller driver for Amlogic A9 SoCs along with its > Kconfig, Makefile, and MAINTAINERS entries. > >> diff --git a/drivers/i2c/busses/i2c-amlogic-a9.c b/drivers/i2c/busses/i2c-amlogic-a9.c >> new file mode 100644 >> index 0000000000000..3e08a0333ceea >> --- /dev/null >> +++ b/drivers/i2c/busses/i2c-amlogic-a9.c > [ ... ] > >> +static irqreturn_t aml_i2c_irq(int irqno, void *dev_id) >> +{ >> + struct aml_i2c *i2c = dev_id; >> + u8 *buf = NULL; >> + u32 rd_len, i; >> + u32 rd_val; >> + u32 irq_state; >> + >> + spin_lock(&i2c->lock); >> + irq_state = readl(i2c->regs + REG_CFG_IRQ_STATE); >> + if (i2c->state == A9_STATE_IDLE) { >> + dev_warn(i2c->dev, "i2c irq triggered abnormally, irq_state:0x%x,irq_en:0x%x\n", >> + irq_state, readl(i2c->regs + REG_CFG_IRQ_ENABLE)); >> + goto irq_trans_out; >> + } >> + >> + if (irq_state & A9_TRANS_ERROR) { >> + i2c->error = -ENXIO; >> + goto complete_irq_out; >> + } >> + >> + switch (i2c->state) { >> + case A9_STATE_READ: >> + buf = i2c->msg->buf; >> + rd_len = min(i2c->msg->len - i2c->pos, A9_I2C_FIFO_DEPTH); >> + for (i = 0; i < rd_len; i++) { >> + rd_val = readl(i2c->regs + REG_CFG_RX); >> + buf[i2c->pos++] = rd_val & TX_RX_DATA; >> + } >> + /* Rx must clear thresh after read fifo */ >> + clear_irq_status(i2c, A9_RX_THRESH_READ); >> + break; >> + case A9_STATE_WRITE: >> + aml_i2c_prepare_xfer(i2c, A9_FIFO_FILL_HALF); >> + if (i2c->pos == i2c->msg->len) >> + aml_i2c_set_mask(i2c, REG_CFG_IRQ_ENABLE, A9_TX_THRESH_WRITE, 0); >> + /* Tx must clear thresh after write fifo */ >> + clear_irq_status(i2c, A9_TX_THRESH_WRITE); >> + break; >> + default: >> + break; >> + } >> + >> + if (!(irq_state & A9_TRANS_DONE) || >> + (i2c->state == A9_STATE_READ && i2c->pos < i2c->msg->len)) { >> + spin_unlock(&i2c->lock); >> + return IRQ_HANDLED; >> + } > [Severity: Medium] > Can multi-chunk reads (msg->len > A9_I2C_FIFO_DEPTH) miss the completion > interrupt and time out in aml_i2c_xfer_msg()? > > Documentation/devicetree/bindings/i2c/amlogic,a9-i2c.yaml: > interrupts = <0 164 1>; > > The interrupt is rising-edge-triggered (IRQ_TYPE_EDGE_RISING), so the > controller's enabled interrupt output must drop to 0 and transition back to 1 > to latch a new interrupt in the GIC. > > In aml_i2c_irq(), irq_state is sampled once before draining up to 32 bytes > from REG_CFG_RX. On a 64-byte read, the 32nd byte of the second chunk asserts > A9_RX_THRESH_READ before the final NACK and STOP finish on the bus. If > A9_PHY_DONE asserts while the handler is reading REG_CFG_RX, prior to > clear_irq_status(i2c, A9_RX_THRESH_READ), the interrupt output stays > continuously high across the clear without generating a new rising edge, while > aml_i2c_irq() checks the stale irq_state snapshot and returns without > completing i2c->done. > > Similarly, if both A9_RX_THRESH_READ and A9_PHY_DONE are already set in > irq_state while more than 32 bytes remain to be read, aml_i2c_irq() reads 32 > bytes, clears only A9_RX_THRESH_READ, and returns early because i2c->pos < > i2c->msg->len. Because A9_PHY_DONE remains asserted in REG_CFG_IRQ_STATE and > fewer than 32 tail bytes remain in the FIFO, no new rising edge is generated > and wait_for_completion_timeout() hangs for 500 ms before returning > -ETIMEDOUT. > As described in this v2 No. According to the code logic, if the data has not been read completely, the condition for completing the operation is not met, so the interrupt status bit will not be cleared. Once the data transfer is complete, the interrupt will immediately be triggered again, allowing the handler to continue reading the data. Finally, once the read operation is complete and the condition is satisfied, the completion interrupt will be cleared. >> + >> +complete_irq_out: >> + i2c->state = A9_STATE_IDLE; >> + reset_fifo_pos(i2c); >> + aml_i2c_set_mask(i2c, REG_CFG_IRQ_ENABLE, A9_RX_THRESH_READ | A9_TX_THRESH_WRITE, 0); >> + complete(&i2c->done); >> +irq_trans_out: >> + clear_irq_status(i2c, IRQ_ALL_MASK); >> + spin_unlock(&i2c->lock); >> + >> + return IRQ_HANDLED; >> +} >> + >> +static void aml_i2c_do_start(struct aml_i2c *i2c, struct i2c_msg *msg) >> +{ >> + unsigned int reg_val; >> + unsigned int trans_len; >> + >> + trans_len = msg->len; >> + reg_val = (unsigned int)(((msg->addr & 0x7f) << START_SADDR_SHIFT) | >> + ((msg->flags & I2C_M_RD) ? START_READ : 0) | >> + ((trans_len & 0xfff) << START_LEN_SHIFT) | START_START); >> + aml_i2c_set_mask(i2c, REG_CFG_START, >> + START_START | START_SLAVE_ADDR | START_READ | START_LEN, reg_val); >> + clear_irq_status(i2c, IRQ_ALL_MASK); >> + /* enable thresh mode */ >> + if (trans_len > A9_I2C_FIFO_DEPTH) { >> + reg_val = (msg->flags & I2C_M_RD) ? A9_RX_THRESH_READ : A9_TX_THRESH_WRITE; >> + aml_i2c_set_mask(i2c, REG_CFG_IRQ_ENABLE, >> + A9_RX_THRESH_READ | A9_TX_THRESH_WRITE, reg_val); >> + } >> +} > [Severity: Medium] > Does writing START_START to REG_CFG_START before clearing IRQ_ALL_MASK and > enabling threshold interrupts race with hardware completion or NACK? > > If execution is delayed between writing REG_CFG_START and calling > clear_irq_status(i2c, IRQ_ALL_MASK) (for example due to vCPU preemption or > PREEMPT_RT spinlock preemption), an address NACK or short transfer can assert > A9_NCK_ERROR or A9_PHY_DONE and then have its status bits wiped out by > clear_irq_status(). When aml_i2c_irq() then runs, it reads irq_state == 0, > misses A9_TRANS_ERROR or A9_TRANS_DONE, and returns without completing > i2c->done, causing wait_for_completion_timeout() to time out. Clearing > REG_CFG_IRQ_STATE and configuring REG_CFG_IRQ_ENABLE before asserting > START_START avoids this window. As described in this v3, This is a hardware-related issue. Some status bits can only be cleared during startup. Changing the initialization sequence may cause data transmission issues. _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-11-11 5:37 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-11-10 8:19 [PATCH v8 0/2] i2c: Add driver support Amlogic A9 I2C Xianwei Zhao via B4 Relay 2026-11-10 8:19 ` [PATCH v8 1/2] dt-bindings: i2c: add Amlogic A9 I2C controller Xianwei Zhao via B4 Relay 2026-11-10 8:19 ` [PATCH v8 2/2] i2c: amlogic: Add Amlogic A9 I2C controller driver Xianwei Zhao via B4 Relay 2026-11-10 8:41 ` sashiko-bot 2026-11-11 5:36 ` Xianwei Zhao
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®