[PATCH v30 4/5] i2c: ast2600: Add controller driver for AST2600 new register set
Ryan Chen
ryan_chen at aspeedtech.com
Thu May 28 19:25:19 PDT 2026
Hello William,
> Subject: Re: [PATCH v30 4/5] i2c: ast2600: Add controller driver for AST2600
> new register set
>
.......
> > +
> > +static int ast2600_i2c_setup_buff_rx(u32 cmd, struct ast2600_i2c_bus
> > +*i2c_bus) {
> > + struct i2c_msg *msg = &i2c_bus->msgs[i2c_bus->msgs_index];
> > + int xfer_len = msg->len - i2c_bus->controller_xfer_cnt;
> > +
> > + cmd |= AST2600_I2CM_PKT_EN | AST2600_I2CM_RX_BUFF_EN |
> > +AST2600_I2CM_RX_CMD;
> > +
> > + if (cmd & AST2600_I2CM_START_CMD)
> > + cmd |= AST2600_I2CM_PKT_ADDR(msg->addr);
> > +
> > + if (msg->flags & I2C_M_RECV_LEN) {
> > + dev_dbg(i2c_bus->dev, "smbus read\n");
> > + xfer_len = 1;
> > + } else if (xfer_len > i2c_bus->buf_size) {
> > + xfer_len = i2c_bus->buf_size;
> > + } else if (i2c_bus->msgs_index + 1 == i2c_bus->msgs_count) {
> > + cmd |= CONTROLLER_TRIGGER_LAST_STOP;
> > + }
>
> You need to check `xfer_len > 0` somewhere before
> `AST2600_I2CC_SET_RX_BUF_LEN(xfer_len)`. Otherwise, a value of 0 will
> become -1 inside `AST2600_I2CC_SET_RX_BUF_LEN(x)` and cause a kernel
> crash when the device reads back extra memory.
>
> Consider userspace doing the following
> ```
> union i2c_smbus_data data = {};
> // data.block[0] (length) is 0 here
> struct i2c_smbus_ioctl_data args = {};
> args.read_write = I2C_SMBUS_READ;
> args.command = mycmd;
> args.size = I2C_SMBUS_I2C_BLOCK_DATA;
> args.data = &data;
> ioctl(fd, I2C_SMBUS, &args);
> ```
OH, thanks your example.
I will update with following.
if (xfer_len <= 0)
return -EINVAL;
> > + writel(AST2600_I2CC_SET_RX_BUF_LEN(xfer_len), i2c_bus->reg_base +
> > +AST2600_I2CC_BUFF_CTRL);
> > +
> > + writel(cmd, i2c_bus->reg_base + AST2600_I2CM_CMD_STS);
> > +
> > + return 0;
> > +}
> > +
> > +static int ast2600_i2c_do_start(struct ast2600_i2c_bus *i2c_bus) {
> > + struct i2c_msg *msg = &i2c_bus->msgs[i2c_bus->msgs_index];
> > +
> > + /* send start */
> > + dev_dbg(i2c_bus->dev, "[%d] %s %d byte%s %s 0x%02x\n",
> > + i2c_bus->msgs_index, str_read_write(msg->flags & I2C_M_RD),
> > + msg->len, str_plural(msg->len),
> > + msg->flags & I2C_M_RD ? "from" : "to", msg->addr);
> > +
> > + i2c_bus->controller_xfer_cnt = 0;
> > +
> > + if (msg->flags & I2C_M_RD)
> > + return ast2600_i2c_setup_buff_rx(AST2600_I2CM_START_CMD,
> i2c_bus);
> > +
> > + return ast2600_i2c_setup_buff_tx(AST2600_I2CM_START_CMD,
> i2c_bus); }
> > +
> > +static int ast2600_i2c_irq_err_to_errno(u32 irq_status) {
> > + if (irq_status & AST2600_I2CM_ARBIT_LOSS)
> > + return -EAGAIN;
> > + if (irq_status & (AST2600_I2CM_SDA_DL_TO |
> AST2600_I2CM_SCL_LOW_TO))
> > + return -ETIMEDOUT;
> > + if (irq_status & (AST2600_I2CM_ABNORMAL))
> > + return -EPROTO;
> > +
> > + return 0;
> > +}
> > +
> > +static void ast2600_i2c_controller_packet_irq(struct ast2600_i2c_bus
> > +*i2c_bus, u32 sts) {
> > + struct i2c_msg *msg = &i2c_bus->msgs[i2c_bus->msgs_index];
> > + int xfer_len;
> > + int i;
> > +
> > + sts &= ~AST2600_I2CM_PKT_DONE;
> > + writel(AST2600_I2CM_PKT_DONE, i2c_bus->reg_base +
> AST2600_I2CM_ISR);
> > + switch (sts) {
> > + case AST2600_I2CM_PKT_ERROR:
> > + i2c_bus->cmd_err = -EAGAIN;
> > + complete(&i2c_bus->cmd_complete);
> > + break;
> > + case AST2600_I2CM_PKT_ERROR | AST2600_I2CM_TX_NAK: /* a0 fix for
> issue */
> > + fallthrough;
> > + case AST2600_I2CM_PKT_ERROR | AST2600_I2CM_TX_NAK |
> AST2600_I2CM_NORMAL_STOP:
> > + i2c_bus->cmd_err = -ENXIO;
> > + complete(&i2c_bus->cmd_complete);
> > + break;
> > + case AST2600_I2CM_NORMAL_STOP:
> > + /* write 0 byte only have stop isr */
> > + i2c_bus->msgs_index++;
> > + if (i2c_bus->msgs_index < i2c_bus->msgs_count) {
> > + if (ast2600_i2c_do_start(i2c_bus)) {
> > + i2c_bus->cmd_err = -EBUSY;
> > + complete(&i2c_bus->cmd_complete);
> > + }
> > + } else {
> > + i2c_bus->cmd_err = i2c_bus->msgs_index;
> > + complete(&i2c_bus->cmd_complete);
> > + }
> > + break;
> > + case AST2600_I2CM_TX_ACK:
> > + case AST2600_I2CM_TX_ACK | AST2600_I2CM_NORMAL_STOP:
> > + xfer_len =
> AST2600_I2CC_GET_TX_BUF_LEN(readl(i2c_bus->reg_base +
> > + AST2600_I2CC_BUFF_CTRL));
> > + i2c_bus->controller_xfer_cnt += xfer_len;
> > +
> > + if (i2c_bus->controller_xfer_cnt == msg->len) {
> > + i2c_bus->msgs_index++;
> > + if (i2c_bus->msgs_index == i2c_bus->msgs_count) {
> > + i2c_bus->cmd_err = i2c_bus->msgs_index;
> > + complete(&i2c_bus->cmd_complete);
> > + } else {
> > + if (ast2600_i2c_do_start(i2c_bus)) {
> > + i2c_bus->cmd_err = -EBUSY;
> > + complete(&i2c_bus->cmd_complete);
> > + }
> > + }
> > + } else {
> > + ast2600_i2c_setup_buff_tx(0, i2c_bus);
> > + }
> > + break;
> > + case AST2600_I2CM_RX_DONE:
> > + case AST2600_I2CM_RX_DONE | AST2600_I2CM_NORMAL_STOP:
> > + xfer_len =
> AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
> > + AST2600_I2CC_BUFF_CTRL));
> > + for (i = 0; i < xfer_len; i++)
> > + msg->buf[i2c_bus->controller_xfer_cnt + i] =
> > + readb(i2c_bus->buf_base + i2c_bus->buf_size + i);
> > +
> > + if (msg->flags & I2C_M_RECV_LEN) {
> > + u8 recv_len =
> AST2600_I2CC_GET_RX_BUFF(readl(i2c_bus->reg_base
> > + + AST2600_I2CC_STS_AND_BUFF));
> > + msg->len = min_t(unsigned int, recv_len,
> I2C_SMBUS_BLOCK_MAX);
> > + msg->len += ((msg->flags & I2C_CLIENT_PEC) ? 2 : 1);
> > + msg->flags &= ~I2C_M_RECV_LEN;
> > + if (!recv_len)
> > + i2c_bus->controller_xfer_cnt = 0;
> > + else
> > + i2c_bus->controller_xfer_cnt = 1;
> > + } else {
> > + i2c_bus->controller_xfer_cnt += xfer_len;
> > + }
> > +
> > + if (i2c_bus->controller_xfer_cnt == msg->len) {
> > + i2c_bus->msgs_index++;
> > + if (i2c_bus->msgs_index == i2c_bus->msgs_count) {
> > + i2c_bus->cmd_err = i2c_bus->msgs_index;
> > + complete(&i2c_bus->cmd_complete);
> > + } else {
> > + if (ast2600_i2c_do_start(i2c_bus)) {
> > + i2c_bus->cmd_err = -EBUSY;
> > + complete(&i2c_bus->cmd_complete);
> > + }
> > + }
> > + } else {
> > + ast2600_i2c_setup_buff_rx(0, i2c_bus);
> > + }
> > + break;
> > + default:
> > + dev_dbg(i2c_bus->dev, "unhandled sts %x\n", sts);
> > + break;
> > + }
> > +}
> > +
> > +static int ast2600_i2c_controller_irq(struct ast2600_i2c_bus
> > +*i2c_bus) {
> > + u32 sts = readl(i2c_bus->reg_base + AST2600_I2CM_ISR);
> > + u32 ctrl;
> > +
> > + sts &= ~AST2600_I2CM_SMBUS_ALERT;
> > +
> > + if (sts & AST2600_I2CM_BUS_RECOVER_FAIL) {
> > + writel(AST2600_I2CM_BUS_RECOVER_FAIL, i2c_bus->reg_base +
> AST2600_I2CM_ISR);
> > + ctrl = readl(i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> > + writel(0, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> > + writel(ctrl, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> > + i2c_bus->cmd_err = -EPROTO;
> > + complete(&i2c_bus->cmd_complete);
> > + return 1;
> > + }
> > +
> > + if (sts & AST2600_I2CM_BUS_RECOVER) {
> > + writel(AST2600_I2CM_BUS_RECOVER, i2c_bus->reg_base +
> AST2600_I2CM_ISR);
> > + i2c_bus->cmd_err = 0;
> > + complete(&i2c_bus->cmd_complete);
> > + return 1;
> > + }
> > +
> > + i2c_bus->cmd_err = ast2600_i2c_irq_err_to_errno(sts);
> > + if (i2c_bus->cmd_err) {
> > + writel(AST2600_I2CM_PKT_DONE, i2c_bus->reg_base +
> AST2600_I2CM_ISR);
> > + complete(&i2c_bus->cmd_complete);
> > + return 1;
> > + }
> > +
> > + if (sts & AST2600_I2CM_PKT_DONE) {
> > + ast2600_i2c_controller_packet_irq(i2c_bus, sts);
> > + return 1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static irqreturn_t ast2600_i2c_bus_irq(int irq, void *dev_id) {
> > + struct ast2600_i2c_bus *i2c_bus = dev_id;
> > +
> > + return IRQ_RETVAL(ast2600_i2c_controller_irq(i2c_bus));
> > +}
> > +
> > +static int ast2600_i2c_controller_xfer(struct i2c_adapter *adap,
> > +struct i2c_msg *msgs, int num) {
> > + struct ast2600_i2c_bus *i2c_bus = i2c_get_adapdata(adap);
> > + unsigned long timeout;
> > + int ret;
> > +
> > + if (!i2c_bus->multi_master &&
> > + (readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF) &
> AST2600_I2CC_BUS_BUSY_STS)) {
> > + ret = ast2600_i2c_recover_bus(i2c_bus);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + i2c_bus->cmd_err = 0;
> > + i2c_bus->msgs = msgs;
> > + i2c_bus->msgs_index = 0;
> > + i2c_bus->msgs_count = num;
> > + reinit_completion(&i2c_bus->cmd_complete);
> > + ret = ast2600_i2c_do_start(i2c_bus);
> > + if (ret)
> > + goto controller_out;
> > + timeout = wait_for_completion_timeout(&i2c_bus->cmd_complete,
> i2c_bus->adap.timeout);
> > + if (timeout == 0) {
> > + u32 ctrl = readl(i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> > +
> > + dev_dbg(i2c_bus->dev, "timeout isr[%x], sts[%x]\n",
> > + readl(i2c_bus->reg_base + AST2600_I2CM_ISR),
> > + readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF));
> > +
> > + writel(0, i2c_bus->reg_base + AST2600_I2CM_IER);
> > + synchronize_irq(i2c_bus->irq);
> > + writel(readl(i2c_bus->reg_base + AST2600_I2CM_ISR),
> > + i2c_bus->reg_base + AST2600_I2CM_ISR);
> > +
> > + writel(ctrl & ~AST2600_I2CC_MASTER_EN, i2c_bus->reg_base +
> AST2600_I2CC_FUN_CTRL);
> > + writel(ctrl, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> > +
> > + writel(AST2600_I2CM_PKT_DONE | AST2600_I2CM_BUS_RECOVER,
> > + i2c_bus->reg_base + AST2600_I2CM_IER);
> > +
> > + /*
> > + * A slave holding SCL low can stall the transfer and trigger
> > + * a master timeout. In multi-master mode, attempt bus recovery
> > + * if the bus is still busy.
> > + */
> > + if (i2c_bus->multi_master &&
> > + (readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF) &
> > + AST2600_I2CC_BUS_BUSY_STS))
> > + ast2600_i2c_recover_bus(i2c_bus);
> > + ret = -ETIMEDOUT;
> > + } else {
> > + ret = i2c_bus->cmd_err;
> > + }
> > +
> > + dev_dbg(i2c_bus->dev, "bus%d-m: %d end\n", i2c_bus->adap.nr,
> > +i2c_bus->cmd_err);
> > +
> > +controller_out:
> > + return ret;
> > +}
> > +
> > +static int ast2600_i2c_init(struct ast2600_i2c_bus *i2c_bus) {
> > + u32 fun_ctrl = AST2600_I2CC_BUS_AUTO_RELEASE |
> > +AST2600_I2CC_MASTER_EN;
> > +
> > + /* I2C Reset */
> > + writel(0, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> > +
> > + if (!i2c_bus->multi_master)
> > + fun_ctrl |= AST2600_I2CC_MULTI_MASTER_DIS;
> > +
> > + /* Enable Controller Mode */
> > + writel(fun_ctrl, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> > + /* disable target address */
> > + writel(0, i2c_bus->reg_base + AST2600_I2CS_ADDR_CTRL);
> > +
> > + /* Set AC Timing */
> > + ast2600_i2c_ac_timing_config(i2c_bus);
> > +
> > + /* Clear Interrupt */
> > + writel(GENMASK(27, 0), i2c_bus->reg_base + AST2600_I2CM_ISR);
> > +
> > + return 0;
> > +}
> > +
> > +static u32 ast2600_i2c_functionality(struct i2c_adapter *adap) {
> > + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
> > +I2C_FUNC_SMBUS_BLOCK_DATA; }
> > +
> > +static const struct i2c_algorithm i2c_ast2600_algorithm = {
> > + .xfer = ast2600_i2c_controller_xfer,
> > + .functionality = ast2600_i2c_functionality, };
> > +
> > +static const struct i2c_adapter_quirks ast2600_i2c_quirks = {
> > + .flags = I2C_AQ_NO_ZERO_LEN_READ,
> > +};
> > +
> > +static int ast2600_i2c_probe(struct platform_device *pdev) {
> > + struct device *dev = &pdev->dev;
> > + struct ast2600_i2c_bus *i2c_bus;
> > + void __iomem *buf_base;
> > + struct reset_control *rst;
> > + struct resource *res;
> > + u32 global_ctrl;
> > + int ret;
> > +
> > + if (!device_property_present(dev, "aspeed,global-regs"))
> > + return -ENODEV;
> > +
> > + i2c_bus = devm_kzalloc(dev, sizeof(*i2c_bus), GFP_KERNEL);
> > + if (!i2c_bus)
> > + return -ENOMEM;
> > +
> > + i2c_bus->reg_base = devm_platform_ioremap_resource(pdev, 0);
> > + if (IS_ERR(i2c_bus->reg_base))
> > + return PTR_ERR(i2c_bus->reg_base);
> > +
> > + rst = devm_reset_control_get_shared_deasserted(dev, NULL);
> > + if (IS_ERR(rst))
> > + return dev_err_probe(dev, PTR_ERR(rst), "Missing reset ctrl\n");
> > +
> > + i2c_bus->global_regs =
> > + syscon_regmap_lookup_by_phandle(dev_of_node(dev),
> "aspeed,global-regs");
> > + if (IS_ERR(i2c_bus->global_regs))
> > + return PTR_ERR(i2c_bus->global_regs);
> > +
> > + regmap_read(i2c_bus->global_regs, AST2600_I2CG_CTRL, &global_ctrl);
> > + if ((global_ctrl & AST2600_GLOBAL_INIT) != AST2600_GLOBAL_INIT) {
> > + regmap_write(i2c_bus->global_regs, AST2600_I2CG_CTRL,
> AST2600_GLOBAL_INIT);
> > + regmap_write(i2c_bus->global_regs, AST2600_I2CG_CLK_DIV_CTRL,
> I2CCG_DIV_CTRL);
> > + }
> > +
> > + i2c_bus->dev = dev;
> > + i2c_bus->multi_master = device_property_read_bool(dev,
> > +"multi-master");
> > +
> > + buf_base = devm_platform_get_and_ioremap_resource(pdev, 1, &res);
> > + if (IS_ERR(buf_base))
> > + return dev_err_probe(dev, PTR_ERR(buf_base), "Missing buffer
> resource\n");
> > + i2c_bus->buf_base = buf_base;
> > + i2c_bus->buf_size = resource_size(res) / 2;
> > +
> > + /*
> > + * i2c timeout counter: use base clk4 1Mhz,
> > + * per unit: 1/(1000/1024) = 1024us
> > + */
> > + ret = device_property_read_u32(dev, "i2c-scl-clk-low-timeout-us",
> &i2c_bus->timeout);
> > + if (!ret) {
> > + i2c_bus->timeout = DIV_ROUND_UP(i2c_bus->timeout, 1024);
> > + if (i2c_bus->timeout > GENMASK(4, 0)) {
> > + dev_warn(dev,
> > + "i2c-scl-clk-low-timeout-us exceeds HW max (31 *
> 1024us), clamped\n");
> > + i2c_bus->timeout = GENMASK(4, 0);
> > + }
> > + }
> > +
> > + init_completion(&i2c_bus->cmd_complete);
> > +
> > + i2c_bus->irq = platform_get_irq(pdev, 0);
> > + if (i2c_bus->irq < 0)
> > + return i2c_bus->irq;
> > +
> > + platform_set_drvdata(pdev, i2c_bus);
> > +
> > + i2c_bus->clk = devm_clk_get(i2c_bus->dev, NULL);
> > + if (IS_ERR(i2c_bus->clk))
> > + return dev_err_probe(i2c_bus->dev, PTR_ERR(i2c_bus->clk), "Can't
> > +get clock\n");
> > +
> > + i2c_bus->apb_clk = clk_get_rate(i2c_bus->clk);
> > +
> > + i2c_parse_fw_timings(i2c_bus->dev, &i2c_bus->timing_info, true);
> > +
> > + /* Initialize the I2C adapter */
> > + i2c_bus->adap.owner = THIS_MODULE;
> > + i2c_bus->adap.algo = &i2c_ast2600_algorithm;
> > + i2c_bus->adap.quirks = &ast2600_i2c_quirks;
> > + i2c_bus->adap.retries = 0;
> > + i2c_bus->adap.dev.parent = i2c_bus->dev;
> > + device_set_node(&i2c_bus->adap.dev, dev_fwnode(dev));
> > + i2c_bus->adap.algo_data = i2c_bus;
> > + strscpy(i2c_bus->adap.name, pdev->name);
> > + i2c_set_adapdata(&i2c_bus->adap, i2c_bus);
> > +
> > + ret = ast2600_i2c_init(i2c_bus);
> > + if (ret < 0)
> > + return dev_err_probe(dev, ret, "Unable to initialize i2c %d\n",
> > +ret);
> > +
> > + ret = devm_request_irq(dev, i2c_bus->irq, ast2600_i2c_bus_irq, 0,
> > + dev_name(dev), i2c_bus);
> > + if (ret < 0) {
> > + ret = dev_err_probe(dev, ret, "Unable to request irq %d\n",
> > + i2c_bus->irq);
> > + goto err;
> > + }
> > +
> > + writel(AST2600_I2CM_PKT_DONE | AST2600_I2CM_BUS_RECOVER,
> > + i2c_bus->reg_base + AST2600_I2CM_IER);
> > +
> > + ret = i2c_add_adapter(&i2c_bus->adap);
> > + if (ret)
> > + goto err;
> > +
> > + return 0;
> > +
> > +err:
> > + writel(0, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> > + writel(0, i2c_bus->reg_base + AST2600_I2CM_IER);
> > + return ret;
> > +}
> > +
> > +static void ast2600_i2c_remove(struct platform_device *pdev) {
> > + struct ast2600_i2c_bus *i2c_bus = platform_get_drvdata(pdev);
> > +
> > + i2c_del_adapter(&i2c_bus->adap);
> > +
> > + /* Disable everything. */
> > + writel(0, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> > + writel(0, i2c_bus->reg_base + AST2600_I2CM_IER); }
> > +
> > +static const struct of_device_id ast2600_i2c_of_match[] = {
> > + { .compatible = "aspeed,ast2600-i2c-bus" },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, ast2600_i2c_of_match);
> > +
> > +static struct platform_driver ast2600_i2c_driver = {
> > + .probe = ast2600_i2c_probe,
> > + .remove = ast2600_i2c_remove,
> > + .driver = {
> > + .name = "ast2600-i2c",
> > + .of_match_table = ast2600_i2c_of_match,
> > + },
> > +};
> > +module_platform_driver(ast2600_i2c_driver);
> > +
> > +MODULE_AUTHOR("Ryan Chen <ryan_chen at aspeedtech.com>");
> > +MODULE_DESCRIPTION("ASPEED AST2600 I2C Controller Driver");
> > +MODULE_LICENSE("GPL");
> >
More information about the linux-arm-kernel
mailing list