* [PATCHv2 0/1] Add ASPEED AST2600 I2C new controller driver @ 2023-01-11 6:47 ryan_chen 2023-01-11 6:47 ` [PATCH 1/1] i2c:aspeed:support ast2600 i2c new register mode driver ryan_chen 0 siblings, 1 reply; 7+ messages in thread From: ryan_chen @ 2023-01-11 6:47 UTC (permalink / raw) To: Heiko Schocher, Ryan Chen, Aspeed BMC SW team, u-boot This series add AST2600 i2c new register set driver. The i2c new register set have new clock divider option for more flexiable generation. v2: add speed support desciption in Kconfig. modify include header ordering. separate include register define header. modify wording reserver to reserved. remove defined string AST2600. modify signle-line comment style. remove extra (). modify local regs for register ctrl. ryan_chen (1): i2c:aspeed:support ast2600 i2c new register mode driver MAINTAINERS | 6 + drivers/i2c/Kconfig | 10 + drivers/i2c/Makefile | 1 + drivers/i2c/ast2600_i2c.c | 375 ++++++++++++++++++++++++++++++++++++++ drivers/i2c/ast2600_i2c.h | 120 ++++++++++++ 5 files changed, 512 insertions(+) create mode 100644 drivers/i2c/ast2600_i2c.c create mode 100644 drivers/i2c/ast2600_i2c.h -- 2.34.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/1] i2c:aspeed:support ast2600 i2c new register mode driver 2023-01-11 6:47 [PATCHv2 0/1] Add ASPEED AST2600 I2C new controller driver ryan_chen @ 2023-01-11 6:47 ` ryan_chen 2023-01-11 21:08 ` Simon Glass 0 siblings, 1 reply; 7+ messages in thread From: ryan_chen @ 2023-01-11 6:47 UTC (permalink / raw) To: Heiko Schocher, Ryan Chen, Aspeed BMC SW team, u-boot Add i2c new register mode driver to support AST2600 i2c new register mode. AST2600 i2c controller have legacy and new register mode. The new register mode have global register support 4 base clock for scl clock selection, and new clock divider mode. Signed-off-by: ryan_chen <ryan_chen@aspeedtech.com> --- MAINTAINERS | 6 + drivers/i2c/Kconfig | 10 + drivers/i2c/Makefile | 1 + drivers/i2c/ast2600_i2c.c | 375 ++++++++++++++++++++++++++++++++++++++ drivers/i2c/ast2600_i2c.h | 120 ++++++++++++ 5 files changed, 512 insertions(+) create mode 100644 drivers/i2c/ast2600_i2c.c create mode 100644 drivers/i2c/ast2600_i2c.h diff --git a/MAINTAINERS b/MAINTAINERS index 3fc4cd0f12..1cf54f0b4e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -769,6 +769,12 @@ S: Maintained F: drivers/pci/pcie_phytium.c F: arch/arm/dts/phytium-durian.dts +ASPEED AST2600 I2C DRIVER +M: Ryan Chen <ryan_chen@aspeedtech.com> +R: Aspeed BMC SW team <BMC-SW@aspeedtech.com> +S: Maintained +F: drivers/i2c/ast2600_i2c.c + ASPEED FMC SPI DRIVER M: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> M: Cédric Le Goater <clg@kaod.org> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig index 08b6c7bdcc..77e2a1c4c0 100644 --- a/drivers/i2c/Kconfig +++ b/drivers/i2c/Kconfig @@ -221,6 +221,16 @@ config SYS_I2C_DW controller is used in various SoCs, e.g. the ST SPEAr, Altera SoCFPGA, Synopsys ARC700 and some Intel x86 SoCs. +config SYS_I2C_AST2600 + bool "AST2600 I2C Controller" + depends on DM_I2C && ARCH_ASPEED + help + Say yes here to select AST2600 I2C Host Controller. The driver + support AST2600 I2C new mode register. This I2C controller supports: + _Standard-mode (up to 100 kHz) + _Fast-mode (up to 400 kHz) + _Fast-mode Plus (up to 1 MHz) + config SYS_I2C_ASPEED bool "Aspeed I2C Controller" depends on DM_I2C && ARCH_ASPEED diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile index 920aafb91c..89db2d8e37 100644 --- a/drivers/i2c/Makefile +++ b/drivers/i2c/Makefile @@ -12,6 +12,7 @@ obj-$(CONFIG_$(SPL_)I2C_CROS_EC_LDO) += cros_ec_ldo.o obj-$(CONFIG_$(SPL_)SYS_I2C_LEGACY) += i2c_core.o obj-$(CONFIG_SYS_I2C_ASPEED) += ast_i2c.o +obj-$(CONFIG_SYS_I2C_AST2600) += ast2600_i2c.o obj-$(CONFIG_SYS_I2C_AT91) += at91_i2c.o obj-$(CONFIG_SYS_I2C_CADENCE) += i2c-cdns.o obj-$(CONFIG_SYS_I2C_CA) += i2c-cortina.o diff --git a/drivers/i2c/ast2600_i2c.c b/drivers/i2c/ast2600_i2c.c new file mode 100644 index 0000000000..0fad30b5a2 --- /dev/null +++ b/drivers/i2c/ast2600_i2c.c @@ -0,0 +1,375 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright ASPEED Technology Inc. + */ +#include <common.h> +#include <clk.h> +#include <dm.h> +#include <errno.h> +#include <fdtdec.h> +#include <i2c.h> +#include <log.h> +#include <regmap.h> +#include <reset.h> +#include <asm/io.h> +#include <linux/iopoll.h> +#include "ast2600_i2c.h" + +/* Device private data */ +struct ast2600_i2c_priv { + struct clk clk; + struct ast2600_i2c_regs *regs; + void __iomem *global; +}; + +static int ast2600_i2c_read_data(struct ast2600_i2c_priv *priv, u8 chip_addr, + u8 *buffer, size_t len, bool send_stop) +{ + int rx_cnt, ret = 0; + u32 cmd, isr; + + for (rx_cnt = 0; rx_cnt < len; rx_cnt++, buffer++) { + cmd = I2CM_PKT_EN | I2CM_PKT_ADDR(chip_addr) | + I2CM_RX_CMD; + if (!rx_cnt) + cmd |= I2CM_START_CMD; + + if ((len - 1) == rx_cnt) + cmd |= I2CM_RX_CMD_LAST; + + if (send_stop && ((len - 1) == rx_cnt)) + cmd |= I2CM_STOP_CMD; + + writel(cmd, &priv->regs->cmd_sts); + + ret = readl_poll_timeout(&priv->regs->isr, isr, + isr & I2CM_PKT_DONE, + I2C_TIMEOUT_US); + if (ret) + return -ETIMEDOUT; + + *buffer = + I2CC_GET_RX_BUFF(readl(&priv->regs->trx_buff)); + + writel(I2CM_PKT_DONE, &priv->regs->isr); + + if (isr & I2CM_TX_NAK) + return -EREMOTEIO; + } + + return 0; +} + +static int ast2600_i2c_write_data(struct ast2600_i2c_priv *priv, u8 chip_addr, + u8 *buffer, size_t len, bool send_stop) +{ + int tx_cnt, ret = 0; + u32 cmd, isr; + + if (!len) { + cmd = I2CM_PKT_EN | I2CM_PKT_ADDR(chip_addr) | + I2CM_START_CMD; + writel(cmd, &priv->regs->cmd_sts); + ret = readl_poll_timeout(&priv->regs->isr, isr, + isr & I2CM_PKT_DONE, + I2C_TIMEOUT_US); + if (ret) + return -ETIMEDOUT; + + writel(I2CM_PKT_DONE, &priv->regs->isr); + + if (isr & I2CM_TX_NAK) + return -EREMOTEIO; + } + + for (tx_cnt = 0; tx_cnt < len; tx_cnt++, buffer++) { + cmd = I2CM_PKT_EN | I2CM_PKT_ADDR(chip_addr); + cmd |= I2CM_TX_CMD; + + if (!tx_cnt) + cmd |= I2CM_START_CMD; + + if (send_stop && ((len - 1) == tx_cnt)) + cmd |= I2CM_STOP_CMD; + + writel(*buffer, &priv->regs->trx_buff); + writel(cmd, &priv->regs->cmd_sts); + ret = readl_poll_timeout(&priv->regs->isr, isr, + isr & I2CM_PKT_DONE, + I2C_TIMEOUT_US); + if (ret) + return -ETIMEDOUT; + + writel(I2CM_PKT_DONE, &priv->regs->isr); + + if (isr & I2CM_TX_NAK) + return -EREMOTEIO; + } + + return 0; +} + +static int ast2600_i2c_deblock(struct udevice *dev) +{ + struct ast2600_i2c_priv *priv = dev_get_priv(dev); + u32 csr = readl(&priv->regs->cmd_sts); + u32 isr; + int ret; + + /* reinit */ + writel(0, &priv->regs->fun_ctrl); + /* Enable Master Mode. Assuming single-master */ + writel(I2CC_BUS_AUTO_RELEASE | I2CC_MASTER_EN | + I2CC_MULTI_MASTER_DIS, + &priv->regs->fun_ctrl); + + csr = readl(&priv->regs->cmd_sts); + + if (!(csr & I2CC_SDA_LINE_STS) && + (csr & I2CC_SCL_LINE_STS)) { + debug("Bus stuck (%x), attempting recovery\n", csr); + writel(I2CM_RECOVER_CMD_EN, &priv->regs->cmd_sts); + ret = readl_poll_timeout(&priv->regs->isr, isr, + isr & (I2CM_BUS_RECOVER_FAIL | + I2CM_BUS_RECOVER), + I2C_TIMEOUT_US); + writel(~0, &priv->regs->isr); + if (ret) + return -EREMOTEIO; + } + + return 0; +} + +static int ast2600_i2c_xfer(struct udevice *dev, struct i2c_msg *msg, int nmsgs) +{ + struct ast2600_i2c_priv *priv = dev_get_priv(dev); + int ret; + + if (readl(&priv->regs->trx_buff) & I2CC_BUS_BUSY_STS) + return -EREMOTEIO; + + for (; nmsgs > 0; nmsgs--, msg++) { + if (msg->flags & I2C_M_RD) { + debug("i2c_read: chip=0x%x, len=0x%x, flags=0x%x\n", + msg->addr, msg->len, msg->flags); + ret = ast2600_i2c_read_data(priv, msg->addr, msg->buf, + msg->len, (nmsgs == 1)); + } else { + debug("i2c_write: chip=0x%x, len=0x%x, flags=0x%x\n", + msg->addr, msg->len, msg->flags); + ret = ast2600_i2c_write_data(priv, msg->addr, msg->buf, + msg->len, (nmsgs == 1)); + } + if (ret) { + debug("%s: error (%d)\n", __func__, ret); + return -EREMOTEIO; + } + } + + return 0; +} + +static int ast2600_i2c_set_speed(struct udevice *dev, unsigned int speed) +{ + struct ast2600_i2c_priv *priv = dev_get_priv(dev); + unsigned long base_clk1, base_clk2, base_clk3, base_clk4; + int multiply = 10; + int baseclk_idx; + u32 clk_div_reg; + u32 apb_clk; + u32 scl_low; + u32 scl_high; + int divisor; + int inc = 0; + u32 data; + + debug("Setting speed for I2C%d to <%u>\n", dev->seq_, speed); + if (!speed) { + debug("No valid speed specified\n"); + return -EINVAL; + } + + apb_clk = clk_get_rate(&priv->clk); + + clk_div_reg = readl(priv->global + I2CG_CLK_DIV_CTRL); + + base_clk1 = (apb_clk * multiply) / (((GET_CLK1_DIV(clk_div_reg) + 2) * multiply) / 2); + base_clk2 = (apb_clk * multiply) / (((GET_CLK2_DIV(clk_div_reg) + 2) * multiply) / 2); + base_clk3 = (apb_clk * multiply) / (((GET_CLK3_DIV(clk_div_reg) + 2) * multiply) / 2); + base_clk4 = (apb_clk * multiply) / (((GET_CLK4_DIV(clk_div_reg) + 2) * multiply) / 2); + + if ((apb_clk / speed) <= 32) { + baseclk_idx = 0; + divisor = DIV_ROUND_UP(apb_clk, speed); + } else if ((base_clk1 / speed) <= 32) { + baseclk_idx = 1; + divisor = DIV_ROUND_UP(base_clk1, speed); + } else if ((base_clk2 / speed) <= 32) { + baseclk_idx = 2; + divisor = DIV_ROUND_UP(base_clk2, speed); + } else if ((base_clk3 / speed) <= 32) { + baseclk_idx = 3; + divisor = DIV_ROUND_UP(base_clk3, speed); + } else { + baseclk_idx = 4; + divisor = DIV_ROUND_UP(base_clk4, speed); + inc = 0; + while ((divisor + inc) > 32) { + inc |= divisor & 0x1; + divisor >>= 1; + baseclk_idx++; + } + divisor += inc; + } + divisor = min_t(int, divisor, 32); + baseclk_idx &= 0xf; + scl_low = ((divisor * 9) / 16) - 1; + scl_low = min_t(u32, scl_low, 0xf); + scl_high = (divisor - scl_low - 2) & 0xf; + /* Divisor : Base Clock : tCKHighMin : tCK High : tCK Low */ + data = ((scl_high - 1) << 20) | (scl_high << 16) | (scl_low << 12) | + baseclk_idx; + /* Set AC Timing */ + writel(data, &priv->regs->ac_timing); + + return 0; +} + +static int ast2600_i2c_probe(struct udevice *dev) +{ + struct ast2600_i2c_priv *priv = dev_get_priv(dev); + struct udevice *global_dev; + int ret; + + /* find global base address */ + ret = uclass_get_device_by_driver(UCLASS_MISC, + DM_DRIVER_GET(aspeed_i2c_global), + &global_dev); + if (ret) { + debug("i2c global not defined\n"); + return ret; + } + + priv->global = dev_read_addr_ptr(global_dev); + if (IS_ERR(priv->global)) { + debug("%s(): can't get global\n", __func__); + return PTR_ERR(priv->global); + } + + /* Reset device */ + writel(0, &priv->regs->fun_ctrl); + /* Enable Master Mode. Assuming single-master */ + writel(I2CC_BUS_AUTO_RELEASE | I2CC_MASTER_EN | + I2CC_MULTI_MASTER_DIS, + &priv->regs->fun_ctrl); + + writel(0, &priv->regs->ier); + /* Clear Interrupt */ + writel(~0, &priv->regs->isr); + + return 0; +} + +static int ast2600_i2c_of_to_plat(struct udevice *dev) +{ + struct ast2600_i2c_priv *priv = dev_get_priv(dev); + int ret; + + priv->regs = dev_read_addr_ptr(dev); + if (!priv->regs) + return -EINVAL; + + ret = clk_get_by_index(dev, 0, &priv->clk); + if (ret < 0) { + debug("%s: Can't get clock for %s: %d\n", __func__, dev->name, + ret); + return ret; + } + + return 0; +} + +static const struct dm_i2c_ops ast2600_i2c_ops = { + .xfer = ast2600_i2c_xfer, + .deblock = ast2600_i2c_deblock, + .set_bus_speed = ast2600_i2c_set_speed, +}; + +static const struct udevice_id ast2600_i2c_ids[] = { + { .compatible = "aspeed,ast2600-i2c" }, + {}, +}; + +U_BOOT_DRIVER(ast2600_i2c) = { + .name = "ast2600_i2c", + .id = UCLASS_I2C, + .of_match = ast2600_i2c_ids, + .probe = ast2600_i2c_probe, + .of_to_plat = ast2600_i2c_of_to_plat, + .priv_auto = sizeof(struct ast2600_i2c_priv), + .ops = &ast2600_i2c_ops, +}; + +struct ast2600_i2c_global_priv { + void __iomem *regs; + struct reset_ctl reset; +}; + +/* + * APB clk : 100Mhz + * div : scl : baseclk [APB/((div/2) + 1)] : tBuf [1/bclk * 16] + * I2CG10[31:24] base clk4 for i2c auto recovery timeout counter (0xC6) + * I2CG10[23:16] base clk3 for Standard-mode (100Khz) min tBuf 4.7us + * 0x3c : 100.8Khz : 3.225Mhz : 4.96us + * 0x3d : 99.2Khz : 3.174Mhz : 5.04us + * 0x3e : 97.65Khz : 3.125Mhz : 5.12us + * 0x40 : 97.75Khz : 3.03Mhz : 5.28us + * 0x41 : 99.5Khz : 2.98Mhz : 5.36us (default) + * I2CG10[15:8] base clk2 for Fast-mode (400Khz) min tBuf 1.3us + * 0x12 : 400Khz : 10Mhz : 1.6us + * I2CG10[7:0] base clk1 for Fast-mode Plus (1Mhz) min tBuf 0.5us + * 0x08 : 1Mhz : 20Mhz : 0.8us + */ + +static int aspeed_i2c_global_probe(struct udevice *dev) +{ + struct ast2600_i2c_global_priv *i2c_global = dev_get_priv(dev); + void __iomem *regs; + int ret = 0; + + i2c_global->regs = dev_read_addr_ptr(dev); + if (!i2c_global->regs) + return -EINVAL; + + debug("%s(dev=%p)\n", __func__, dev); + + regs = i2c_global->regs; + + ret = reset_get_by_index(dev, 0, &i2c_global->reset); + if (ret) { + printf("%s(): Failed to get reset signal\n", __func__); + return ret; + } + + reset_deassert(&i2c_global->reset); + + writel(GLOBAL_INIT, regs + I2CG_CTRL); + writel(I2CCG_DIV_CTRL, regs + I2CG_CLK_DIV_CTRL); + + return 0; +} + +static const struct udevice_id aspeed_i2c_global_ids[] = { + { .compatible = "aspeed,ast2600-i2c-global", }, + { } +}; + +U_BOOT_DRIVER(aspeed_i2c_global) = { + .name = "aspeed_i2c_global", + .id = UCLASS_MISC, + .of_match = aspeed_i2c_global_ids, + .probe = aspeed_i2c_global_probe, + .priv_auto = sizeof(struct ast2600_i2c_global_priv), +}; + diff --git a/drivers/i2c/ast2600_i2c.h b/drivers/i2c/ast2600_i2c.h new file mode 100644 index 0000000000..69699ba84c --- /dev/null +++ b/drivers/i2c/ast2600_i2c.h @@ -0,0 +1,120 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright ASPEED Technology Inc. + */ +#ifndef __AST2600_I2C_H_ +#define __AST2600_I2C_H_ + +struct ast2600_i2c_regs { + u32 fun_ctrl; + u32 ac_timing; + u32 trx_buff; + u32 icr; + u32 ier; + u32 isr; + u32 cmd_sts; +}; + +/* 0x00 : I2CC Master/Slave Function Control Register */ +#define I2CC_SLAVE_ADDR_RX_EN BIT(20) +#define I2CC_MASTER_RETRY_MASK GENMASK(19, 18) +#define I2CC_MASTER_RETRY(x) (((x) & GENMASK(1, 0)) << 18) +#define I2CC_BUS_AUTO_RELEASE BIT(17) +#define I2CC_M_SDA_LOCK_EN BIT(16) +#define I2CC_MULTI_MASTER_DIS BIT(15) +#define I2CC_M_SCL_DRIVE_EN BIT(14) +#define I2CC_MSB_STS BIT(9) +#define I2CC_SDA_DRIVE_1T_EN BIT(8) +#define I2CC_M_SDA_DRIVE_1T_EN BIT(7) +#define I2CC_M_HIGH_SPEED_EN BIT(6) +/* reserved 5 : 2 */ +#define I2CC_SLAVE_EN BIT(1) +#define I2CC_MASTER_EN BIT(0) + +/* 0x04 : I2CD Clock and AC Timing Control Register #1 */ +/* Base register value. These bits are always set by the driver. */ +#define I2CD_CACTC_BASE 0xfff00300 +#define I2CD_TCKHIGH_SHIFT 16 +#define I2CD_TCKLOW_SHIFT 12 +#define I2CD_THDDAT_SHIFT 10 +#define I2CD_TO_DIV_SHIFT 8 +#define I2CD_BASE_DIV_SHIFT 0 + +/* 0x08 : I2CC Master/Slave Transmit/Receive Byte Buffer Register */ +#define I2CC_TX_DIR_MASK GENMASK(31, 29) +#define I2CC_SDA_OE BIT(28) +#define I2CC_SDA_O BIT(27) +#define I2CC_SCL_OE BIT(26) +#define I2CC_SCL_O BIT(25) + +#define I2CC_SCL_LINE_STS BIT(18) +#define I2CC_SDA_LINE_STS BIT(17) +#define I2CC_BUS_BUSY_STS BIT(16) +#define I2CC_GET_RX_BUFF(x) (((x) >> 8) & GENMASK(7, 0)) + +/* 0x10 : I2CM Master Interrupt Control Register */ +/* 0x14 : I2CM Master Interrupt Status Register */ +#define I2CM_PKT_TIMEOUT BIT(18) +#define I2CM_PKT_ERROR BIT(17) +#define I2CM_PKT_DONE BIT(16) + +#define I2CM_BUS_RECOVER_FAIL BIT(15) +#define I2CM_SDA_DL_TO BIT(14) +#define I2CM_BUS_RECOVER BIT(13) +#define I2CM_SMBUS_ALT BIT(12) + +#define I2CM_SCL_LOW_TO BIT(6) +#define I2CM_ABNORMAL BIT(5) +#define I2CM_NORMAL_STOP BIT(4) +#define I2CM_ARBIT_LOSS BIT(3) +#define I2CM_RX_DONE BIT(2) +#define I2CM_TX_NAK BIT(1) +#define I2CM_TX_ACK BIT(0) + +/* 0x18 : I2CM Master Command/Status Register */ +#define I2CM_PKT_ADDR(x) (((x) & GENMASK(6, 0)) << 24) +#define I2CM_PKT_EN BIT(16) +#define I2CM_SDA_OE_OUT_DIR BIT(15) +#define I2CM_SDA_O_OUT_DIR BIT(14) +#define I2CM_SCL_OE_OUT_DIR BIT(13) +#define I2CM_SCL_O_OUT_DIR BIT(12) +#define I2CM_RECOVER_CMD_EN BIT(11) + +#define I2CM_RX_DMA_EN BIT(9) +#define I2CM_TX_DMA_EN BIT(8) +/* Command Bit */ +#define I2CM_RX_BUFF_EN BIT(7) +#define I2CM_TX_BUFF_EN BIT(6) +#define I2CM_STOP_CMD BIT(5) +#define I2CM_RX_CMD_LAST BIT(4) +#define I2CM_RX_CMD BIT(3) + +#define I2CM_TX_CMD BIT(1) +#define I2CM_START_CMD BIT(0) + +#define I2C_TIMEOUT_US 100000 + +/* I2C Global Register */ +#define I2CG_ISR 0x00 +#define I2CG_SLAVE_ISR 0x04 +#define I2CG_OWNER 0x08 +#define I2CG_CTRL 0x0C +#define I2CG_CLK_DIV_CTRL 0x10 + +#define I2CG_SLAVE_PKT_NAK BIT(4) +#define I2CG_M_S_SEPARATE_INTR BIT(3) +#define I2CG_CTRL_NEW_REG BIT(2) +#define I2CG_CTRL_NEW_CLK_DIV BIT(1) + +#define GLOBAL_INIT \ + (I2CG_SLAVE_PKT_NAK | \ + I2CG_CTRL_NEW_REG | \ + I2CG_CTRL_NEW_CLK_DIV) +#define I2CCG_DIV_CTRL 0xc6411208 + +#define GET_CLK1_DIV(x) ((x) & 0xff) +#define GET_CLK2_DIV(x) (((x) >> 8) & 0xff) +#define GET_CLK3_DIV(x) (((x) >> 16) & 0xff) +#define GET_CLK4_DIV(x) (((x) >> 24) & 0xff) + +#endif /* __AST2600_I2C_H_ */ -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] i2c:aspeed:support ast2600 i2c new register mode driver 2023-01-11 6:47 ` [PATCH 1/1] i2c:aspeed:support ast2600 i2c new register mode driver ryan_chen @ 2023-01-11 21:08 ` Simon Glass 2023-01-15 1:21 ` Ryan Chen 0 siblings, 1 reply; 7+ messages in thread From: Simon Glass @ 2023-01-11 21:08 UTC (permalink / raw) To: ryan_chen; +Cc: Heiko Schocher, Aspeed BMC SW team, u-boot Hi Ryan, On Tue, 10 Jan 2023 at 23:48, ryan_chen <ryan_chen@aspeedtech.com> wrote: > > Add i2c new register mode driver to support AST2600 i2c > new register mode. AST2600 i2c controller have legacy and > new register mode. The new register mode have global register > support 4 base clock for scl clock selection, and new clock > divider mode. > > Signed-off-by: ryan_chen <ryan_chen@aspeedtech.com> > --- > MAINTAINERS | 6 + > drivers/i2c/Kconfig | 10 + > drivers/i2c/Makefile | 1 + > drivers/i2c/ast2600_i2c.c | 375 ++++++++++++++++++++++++++++++++++++++ > drivers/i2c/ast2600_i2c.h | 120 ++++++++++++ > 5 files changed, 512 insertions(+) > create mode 100644 drivers/i2c/ast2600_i2c.c > create mode 100644 drivers/i2c/ast2600_i2c.h Is this a new version? Regards, Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH 1/1] i2c:aspeed:support ast2600 i2c new register mode driver 2023-01-11 21:08 ` Simon Glass @ 2023-01-15 1:21 ` Ryan Chen 0 siblings, 0 replies; 7+ messages in thread From: Ryan Chen @ 2023-01-15 1:21 UTC (permalink / raw) To: Simon Glass; +Cc: Heiko Schocher, BMC-SW, u-boot@lists.denx.de Hello, I resend the v2 version here https://www.mail-archive.com/u-boot@lists.denx.de/msg460560.html Ryan Chen > -----Original Message----- > From: Simon Glass <sjg@chromium.org> > Sent: Thursday, January 12, 2023 5:08 AM > To: Ryan Chen <ryan_chen@aspeedtech.com> > Cc: Heiko Schocher <hs@denx.de>; BMC-SW <BMC-SW@aspeedtech.com>; > u-boot@lists.denx.de > Subject: Re: [PATCH 1/1] i2c:aspeed:support ast2600 i2c new register mode > driver > > Hi Ryan, > > On Tue, 10 Jan 2023 at 23:48, ryan_chen <ryan_chen@aspeedtech.com> > wrote: > > > > Add i2c new register mode driver to support AST2600 i2c new register > > mode. AST2600 i2c controller have legacy and new register mode. The > > new register mode have global register support 4 base clock for scl > > clock selection, and new clock divider mode. > > > > Signed-off-by: ryan_chen <ryan_chen@aspeedtech.com> > > --- > > MAINTAINERS | 6 + > > drivers/i2c/Kconfig | 10 + > > drivers/i2c/Makefile | 1 + > > drivers/i2c/ast2600_i2c.c | 375 > > ++++++++++++++++++++++++++++++++++++++ > > drivers/i2c/ast2600_i2c.h | 120 ++++++++++++ > > 5 files changed, 512 insertions(+) > > create mode 100644 drivers/i2c/ast2600_i2c.c create mode 100644 > > drivers/i2c/ast2600_i2c.h > > Is this a new version? > > Regards, > Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 0/1] Add ASPEED AST2600 I2C new controller driver @ 2023-01-09 8:29 ryan_chen 2023-01-09 8:29 ` [PATCH 1/1] i2c:aspeed:support ast2600 i2c new register mode driver ryan_chen 0 siblings, 1 reply; 7+ messages in thread From: ryan_chen @ 2023-01-09 8:29 UTC (permalink / raw) To: Heiko Schocher, Ryan Chen, Aspeed BMC SW team, u-boot This series add AST2600 i2c new register set driver. The i2c new register set have new clock divider option for more flexiable generation. ryan_chen (1): i2c:aspeed:support ast2600 i2c new register mode driver MAINTAINERS | 6 + drivers/i2c/Kconfig | 7 + drivers/i2c/Makefile | 1 + drivers/i2c/ast2600_i2c.c | 480 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 494 insertions(+) create mode 100644 drivers/i2c/ast2600_i2c.c -- 2.34.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/1] i2c:aspeed:support ast2600 i2c new register mode driver 2023-01-09 8:29 [PATCH 0/1] Add ASPEED AST2600 I2C new controller driver ryan_chen @ 2023-01-09 8:29 ` ryan_chen 2023-01-09 19:47 ` Simon Glass 0 siblings, 1 reply; 7+ messages in thread From: ryan_chen @ 2023-01-09 8:29 UTC (permalink / raw) To: Heiko Schocher, Ryan Chen, Aspeed BMC SW team, u-boot Add i2c new register mode driver to support AST2600 i2c new register mode. AST2600 i2c controller have legacy and new register mode. The new register mode have global register support 4 base clock for scl clock selection, and new clock divider mode. Signed-off-by: ryan_chen <ryan_chen@aspeedtech.com> --- MAINTAINERS | 6 + drivers/i2c/Kconfig | 7 + drivers/i2c/Makefile | 1 + drivers/i2c/ast2600_i2c.c | 480 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 494 insertions(+) create mode 100644 drivers/i2c/ast2600_i2c.c diff --git a/MAINTAINERS b/MAINTAINERS index 3fc4cd0f12..1cf54f0b4e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -769,6 +769,12 @@ S: Maintained F: drivers/pci/pcie_phytium.c F: arch/arm/dts/phytium-durian.dts +ASPEED AST2600 I2C DRIVER +M: Ryan Chen <ryan_chen@aspeedtech.com> +R: Aspeed BMC SW team <BMC-SW@aspeedtech.com> +S: Maintained +F: drivers/i2c/ast2600_i2c.c + ASPEED FMC SPI DRIVER M: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> M: Cédric Le Goater <clg@kaod.org> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig index 08b6c7bdcc..34f507fb3b 100644 --- a/drivers/i2c/Kconfig +++ b/drivers/i2c/Kconfig @@ -221,6 +221,13 @@ config SYS_I2C_DW controller is used in various SoCs, e.g. the ST SPEAr, Altera SoCFPGA, Synopsys ARC700 and some Intel x86 SoCs. +config SYS_I2C_AST2600 + bool "AST2600 I2C Controller" + depends on DM_I2C && ARCH_ASPEED + help + Say yes here to select AST2600 I2C Host Controller. The driver + support AST2600 I2C new mode register. + config SYS_I2C_ASPEED bool "Aspeed I2C Controller" depends on DM_I2C && ARCH_ASPEED diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile index 920aafb91c..89db2d8e37 100644 --- a/drivers/i2c/Makefile +++ b/drivers/i2c/Makefile @@ -12,6 +12,7 @@ obj-$(CONFIG_$(SPL_)I2C_CROS_EC_LDO) += cros_ec_ldo.o obj-$(CONFIG_$(SPL_)SYS_I2C_LEGACY) += i2c_core.o obj-$(CONFIG_SYS_I2C_ASPEED) += ast_i2c.o +obj-$(CONFIG_SYS_I2C_AST2600) += ast2600_i2c.o obj-$(CONFIG_SYS_I2C_AT91) += at91_i2c.o obj-$(CONFIG_SYS_I2C_CADENCE) += i2c-cdns.o obj-$(CONFIG_SYS_I2C_CA) += i2c-cortina.o diff --git a/drivers/i2c/ast2600_i2c.c b/drivers/i2c/ast2600_i2c.c new file mode 100644 index 0000000000..52aea460ac --- /dev/null +++ b/drivers/i2c/ast2600_i2c.c @@ -0,0 +1,480 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright ASPEED Technology Inc. + */ +#include <linux/iopoll.h> +#include <common.h> +#include <clk.h> +#include <dm.h> +#include <errno.h> +#include <fdtdec.h> +#include <i2c.h> +#include <asm/io.h> +#include <regmap.h> +#include <reset.h> + +struct ast2600_i2c_regs { + u32 fun_ctrl; + u32 ac_timing; + u32 trx_buff; + u32 icr; + u32 ier; + u32 isr; + u32 cmd_sts; +}; + +/* 0x00 : I2CC Master/Slave Function Control Register */ +#define AST2600_I2CC_SLAVE_ADDR_RX_EN BIT(20) +#define AST2600_I2CC_MASTER_RETRY_MASK GENMASK(19, 18) +#define AST2600_I2CC_MASTER_RETRY(x) (((x) & GENMASK(1, 0)) << 18) +#define AST2600_I2CC_BUS_AUTO_RELEASE BIT(17) +#define AST2600_I2CC_M_SDA_LOCK_EN BIT(16) +#define AST2600_I2CC_MULTI_MASTER_DIS BIT(15) +#define AST2600_I2CC_M_SCL_DRIVE_EN BIT(14) +#define AST2600_I2CC_MSB_STS BIT(9) +#define AST2600_I2CC_SDA_DRIVE_1T_EN BIT(8) +#define AST2600_I2CC_M_SDA_DRIVE_1T_EN BIT(7) +#define AST2600_I2CC_M_HIGH_SPEED_EN BIT(6) +/* reserver 5 : 2 */ +#define AST2600_I2CC_SLAVE_EN BIT(1) +#define AST2600_I2CC_MASTER_EN BIT(0) + +/* 0x04 : I2CD Clock and AC Timing Control Register #1 */ +/* Base register value. These bits are always set by the driver. */ +#define I2CD_CACTC_BASE 0xfff00300 +#define I2CD_TCKHIGH_SHIFT 16 +#define I2CD_TCKLOW_SHIFT 12 +#define I2CD_THDDAT_SHIFT 10 +#define I2CD_TO_DIV_SHIFT 8 +#define I2CD_BASE_DIV_SHIFT 0 + +/* 0x08 : I2CC Master/Slave Transmit/Receive Byte Buffer Register */ +#define AST2600_I2CC_TX_DIR_MASK GENMASK(31, 29) +#define AST2600_I2CC_SDA_OE BIT(28) +#define AST2600_I2CC_SDA_O BIT(27) +#define AST2600_I2CC_SCL_OE BIT(26) +#define AST2600_I2CC_SCL_O BIT(25) + +#define AST2600_I2CC_SCL_LINE_STS BIT(18) +#define AST2600_I2CC_SDA_LINE_STS BIT(17) +#define AST2600_I2CC_BUS_BUSY_STS BIT(16) +#define AST2600_I2CC_GET_RX_BUFF(x) (((x) >> 8) & GENMASK(7, 0)) + +/* 0x10 : I2CM Master Interrupt Control Register */ +/* 0x14 : I2CM Master Interrupt Status Register */ +#define AST2600_I2CM_PKT_TIMEOUT BIT(18) +#define AST2600_I2CM_PKT_ERROR BIT(17) +#define AST2600_I2CM_PKT_DONE BIT(16) + +#define AST2600_I2CM_BUS_RECOVER_FAIL BIT(15) +#define AST2600_I2CM_SDA_DL_TO BIT(14) +#define AST2600_I2CM_BUS_RECOVER BIT(13) +#define AST2600_I2CM_SMBUS_ALT BIT(12) + +#define AST2600_I2CM_SCL_LOW_TO BIT(6) +#define AST2600_I2CM_ABNORMAL BIT(5) +#define AST2600_I2CM_NORMAL_STOP BIT(4) +#define AST2600_I2CM_ARBIT_LOSS BIT(3) +#define AST2600_I2CM_RX_DONE BIT(2) +#define AST2600_I2CM_TX_NAK BIT(1) +#define AST2600_I2CM_TX_ACK BIT(0) + +/* 0x18 : I2CM Master Command/Status Register */ +#define AST2600_I2CM_PKT_ADDR(x) (((x) & GENMASK(6, 0)) << 24) +#define AST2600_I2CM_PKT_EN BIT(16) +#define AST2600_I2CM_SDA_OE_OUT_DIR BIT(15) +#define AST2600_I2CM_SDA_O_OUT_DIR BIT(14) +#define AST2600_I2CM_SCL_OE_OUT_DIR BIT(13) +#define AST2600_I2CM_SCL_O_OUT_DIR BIT(12) +#define AST2600_I2CM_RECOVER_CMD_EN BIT(11) + +#define AST2600_I2CM_RX_DMA_EN BIT(9) +#define AST2600_I2CM_TX_DMA_EN BIT(8) +/* Command Bit */ +#define AST2600_I2CM_RX_BUFF_EN BIT(7) +#define AST2600_I2CM_TX_BUFF_EN BIT(6) +#define AST2600_I2CM_STOP_CMD BIT(5) +#define AST2600_I2CM_RX_CMD_LAST BIT(4) +#define AST2600_I2CM_RX_CMD BIT(3) + +#define AST2600_I2CM_TX_CMD BIT(1) +#define AST2600_I2CM_START_CMD BIT(0) + +#define I2C_TIMEOUT_US 100000 +/* + * Device private data + */ +struct ast2600_i2c_priv { + struct clk clk; + struct ast2600_i2c_regs *regs; + void __iomem *global; +}; + +static int ast2600_i2c_read_data(struct ast2600_i2c_priv *priv, u8 chip_addr, + u8 *buffer, size_t len, bool send_stop) +{ + int rx_cnt, ret = 0; + u32 cmd, isr; + + for (rx_cnt = 0; rx_cnt < len; rx_cnt++, buffer++) { + cmd = AST2600_I2CM_PKT_EN | AST2600_I2CM_PKT_ADDR(chip_addr) | + AST2600_I2CM_RX_CMD; + if (!rx_cnt) + cmd |= AST2600_I2CM_START_CMD; + + if ((len - 1) == rx_cnt) + cmd |= AST2600_I2CM_RX_CMD_LAST; + + if (send_stop && ((len - 1) == rx_cnt)) + cmd |= AST2600_I2CM_STOP_CMD; + + writel(cmd, &priv->regs->cmd_sts); + + ret = readl_poll_timeout(&priv->regs->isr, isr, + isr & AST2600_I2CM_PKT_DONE, + I2C_TIMEOUT_US); + if (ret) + return -ETIMEDOUT; + + *buffer = + AST2600_I2CC_GET_RX_BUFF(readl(&priv->regs->trx_buff)); + + writel(AST2600_I2CM_PKT_DONE, &priv->regs->isr); + + if (isr & AST2600_I2CM_TX_NAK) + return -EREMOTEIO; + } + + return 0; +} + +static int ast2600_i2c_write_data(struct ast2600_i2c_priv *priv, u8 chip_addr, + u8 *buffer, size_t len, bool send_stop) +{ + int tx_cnt, ret = 0; + u32 cmd, isr; + + if (!len) { + cmd = AST2600_I2CM_PKT_EN | AST2600_I2CM_PKT_ADDR(chip_addr) | + AST2600_I2CM_START_CMD; + writel(cmd, &priv->regs->cmd_sts); + ret = readl_poll_timeout(&priv->regs->isr, isr, + isr & AST2600_I2CM_PKT_DONE, + I2C_TIMEOUT_US); + if (ret) + return -ETIMEDOUT; + + writel(AST2600_I2CM_PKT_DONE, &priv->regs->isr); + + if (isr & AST2600_I2CM_TX_NAK) + return -EREMOTEIO; + } + + for (tx_cnt = 0; tx_cnt < len; tx_cnt++, buffer++) { + cmd = AST2600_I2CM_PKT_EN | AST2600_I2CM_PKT_ADDR(chip_addr); + cmd |= AST2600_I2CM_TX_CMD; + + if (!tx_cnt) + cmd |= AST2600_I2CM_START_CMD; + + if (send_stop && ((len - 1) == tx_cnt)) + cmd |= AST2600_I2CM_STOP_CMD; + + writel(*buffer, &priv->regs->trx_buff); + writel(cmd, &priv->regs->cmd_sts); + ret = readl_poll_timeout(&priv->regs->isr, isr, + isr & AST2600_I2CM_PKT_DONE, + I2C_TIMEOUT_US); + if (ret) + return -ETIMEDOUT; + + writel(AST2600_I2CM_PKT_DONE, &priv->regs->isr); + + if (isr & AST2600_I2CM_TX_NAK) + return -EREMOTEIO; + } + + return 0; +} + +static int ast2600_i2c_deblock(struct udevice *dev) +{ + struct ast2600_i2c_priv *priv = dev_get_priv(dev); + u32 csr = readl(&priv->regs->cmd_sts); + u32 isr; + int ret; + + //reinit + writel(0, &priv->regs->fun_ctrl); + /* Enable Master Mode. Assuming single-master */ + writel(AST2600_I2CC_BUS_AUTO_RELEASE | AST2600_I2CC_MASTER_EN | + AST2600_I2CC_MULTI_MASTER_DIS, + &priv->regs->fun_ctrl); + + csr = readl(&priv->regs->cmd_sts); + + if (!(csr & AST2600_I2CC_SDA_LINE_STS) && + (csr & AST2600_I2CC_SCL_LINE_STS)) { + debug("Bus stuck (%x), attempting recovery\n", csr); + writel(AST2600_I2CM_RECOVER_CMD_EN, &priv->regs->cmd_sts); + ret = readl_poll_timeout(&priv->regs->isr, isr, + isr & (AST2600_I2CM_BUS_RECOVER_FAIL | + AST2600_I2CM_BUS_RECOVER), + I2C_TIMEOUT_US); + writel(~0, &priv->regs->isr); + if (ret) + return -EREMOTEIO; + } + + return 0; +} + +static int ast2600_i2c_xfer(struct udevice *dev, struct i2c_msg *msg, int nmsgs) +{ + struct ast2600_i2c_priv *priv = dev_get_priv(dev); + int ret; + + if (readl(&priv->regs->trx_buff) & AST2600_I2CC_BUS_BUSY_STS) + return -EREMOTEIO; + + for (; nmsgs > 0; nmsgs--, msg++) { + if (msg->flags & I2C_M_RD) { + debug("i2c_read: chip=0x%x, len=0x%x, flags=0x%x\n", + msg->addr, msg->len, msg->flags); + ret = ast2600_i2c_read_data(priv, msg->addr, msg->buf, + msg->len, (nmsgs == 1)); + } else { + debug("i2c_write: chip=0x%x, len=0x%x, flags=0x%x\n", + msg->addr, msg->len, msg->flags); + ret = ast2600_i2c_write_data(priv, msg->addr, msg->buf, + msg->len, (nmsgs == 1)); + } + if (ret) { + debug("%s: error (%d)\n", __func__, ret); + return -EREMOTEIO; + } + } + + return 0; +} + +static int ast2600_i2c_set_speed(struct udevice *dev, unsigned int speed) +{ + struct ast2600_i2c_priv *priv = dev_get_priv(dev); + unsigned long base_clk1, base_clk2, base_clk3, base_clk4; + int baseclk_idx; + u32 clk_div_reg; + u32 apb_clk; + u32 scl_low; + u32 scl_high; + int divisor; + int inc = 0; + u32 data; + + debug("Setting speed for I2C%d to <%u>\n", dev->seq_, speed); + if (!speed) { + debug("No valid speed specified\n"); + return -EINVAL; + } + + apb_clk = clk_get_rate(&priv->clk); + + clk_div_reg = readl(priv->global + 0x10); + base_clk1 = (apb_clk * 10) / ((((clk_div_reg & 0xff) + 2) * 10) / 2); + base_clk2 = + (apb_clk * 10) / (((((clk_div_reg >> 8) & 0xff) + 2) * 10) / 2); + base_clk3 = (apb_clk * 10) / + (((((clk_div_reg >> 16) & 0xff) + 2) * 10) / 2); + base_clk4 = (apb_clk * 10) / + (((((clk_div_reg >> 24) & 0xff) + 2) * 10) / 2); + + if ((apb_clk / speed) <= 32) { + baseclk_idx = 0; + divisor = DIV_ROUND_UP(apb_clk, speed); + } else if ((base_clk1 / speed) <= 32) { + baseclk_idx = 1; + divisor = DIV_ROUND_UP(base_clk1, speed); + } else if ((base_clk2 / speed) <= 32) { + baseclk_idx = 2; + divisor = DIV_ROUND_UP(base_clk2, speed); + } else if ((base_clk3 / speed) <= 32) { + baseclk_idx = 3; + divisor = DIV_ROUND_UP(base_clk3, speed); + } else { + baseclk_idx = 4; + divisor = DIV_ROUND_UP(base_clk4, speed); + inc = 0; + while ((divisor + inc) > 32) { + inc |= divisor & 0x1; + divisor >>= 1; + baseclk_idx++; + } + divisor += inc; + } + divisor = min_t(int, divisor, 32); + baseclk_idx &= 0xf; + scl_low = ((divisor * 9) / 16) - 1; + scl_low = min_t(u32, scl_low, 0xf); + scl_high = (divisor - scl_low - 2) & 0xf; + /* Divisor : Base Clock : tCKHighMin : tCK High : tCK Low */ + data = ((scl_high - 1) << 20) | (scl_high << 16) | (scl_low << 12) | + (baseclk_idx); + /* Set AC Timing */ + writel(data, &priv->regs->ac_timing); + + return 0; +} + +static int ast2600_i2c_probe(struct udevice *dev) +{ + struct ast2600_i2c_priv *priv = dev_get_priv(dev); + struct udevice *misc_dev; + int ret; + + /* find global base address */ + ret = uclass_get_device_by_driver(UCLASS_MISC, + DM_DRIVER_GET(aspeed_i2c_global), + &misc_dev); + if (ret) { + debug("i2c global not defined\n"); + return ret; + } + + priv->global = devfdt_get_addr_ptr(misc_dev); + if (IS_ERR(priv->global)) { + debug("%s(): can't get global\n", __func__); + return PTR_ERR(priv->global); + } + + /* Reset device */ + writel(0, &priv->regs->fun_ctrl); + /* Enable Master Mode. Assuming single-master */ + writel(AST2600_I2CC_BUS_AUTO_RELEASE | AST2600_I2CC_MASTER_EN | + AST2600_I2CC_MULTI_MASTER_DIS, + &priv->regs->fun_ctrl); + + writel(0, &priv->regs->ier); + /* Clear Interrupt */ + writel(~0, &priv->regs->isr); + + return 0; +} + +static int ast2600_i2c_of_to_plat(struct udevice *dev) +{ + struct ast2600_i2c_priv *priv = dev_get_priv(dev); + int ret; + + priv->regs = dev_read_addr_ptr(dev); + if (!(priv->regs)) + return -EINVAL; + + ret = clk_get_by_index(dev, 0, &priv->clk); + if (ret < 0) { + debug("%s: Can't get clock for %s: %d\n", __func__, dev->name, + ret); + return ret; + } + + return 0; +} + +static const struct dm_i2c_ops ast2600_i2c_ops = { + .xfer = ast2600_i2c_xfer, + .deblock = ast2600_i2c_deblock, + .set_bus_speed = ast2600_i2c_set_speed, +}; + +static const struct udevice_id ast2600_i2c_ids[] = { + { .compatible = "aspeed,ast2600-i2c" }, + {}, +}; + +U_BOOT_DRIVER(ast2600_i2c) = { + .name = "ast2600_i2c", + .id = UCLASS_I2C, + .of_match = ast2600_i2c_ids, + .probe = ast2600_i2c_probe, + .of_to_plat = ast2600_i2c_of_to_plat, + .priv_auto = sizeof(struct ast2600_i2c_priv), + .ops = &ast2600_i2c_ops, +}; + +#define AST2600_I2CG_ISR 0x00 +#define AST2600_I2CG_SLAVE_ISR 0x04 +#define AST2600_I2CG_OWNER 0x08 +#define AST2600_I2CG_CTRL 0x0C +#define AST2600_I2CG_CLK_DIV_CTRL 0x10 + +#define AST2600_I2CG_SLAVE_PKT_NAK BIT(4) +#define AST2600_I2CG_M_S_SEPARATE_INTR BIT(3) +#define AST2600_I2CG_CTRL_NEW_REG BIT(2) +#define AST2600_I2CG_CTRL_NEW_CLK_DIV BIT(1) + +#define AST2600_GLOBAL_INIT \ + (AST2600_I2CG_SLAVE_PKT_NAK | \ + AST2600_I2CG_CTRL_NEW_REG | \ + AST2600_I2CG_CTRL_NEW_CLK_DIV) +#define I2CCG_DIV_CTRL 0xC6411208 + +struct ast2600_i2c_global_priv { + void __iomem *regs; + struct reset_ctl reset; +}; + +/* + * APB clk : 100Mhz + * div : scl : baseclk [APB/((div/2) + 1)] : tBuf [1/bclk * 16] + * I2CG10[31:24] base clk4 for i2c auto recovery timeout counter (0xC6) + * I2CG10[23:16] base clk3 for Standard-mode (100Khz) min tBuf 4.7us + * 0x3c : 100.8Khz : 3.225Mhz : 4.96us + * 0x3d : 99.2Khz : 3.174Mhz : 5.04us + * 0x3e : 97.65Khz : 3.125Mhz : 5.12us + * 0x40 : 97.75Khz : 3.03Mhz : 5.28us + * 0x41 : 99.5Khz : 2.98Mhz : 5.36us (default) + * I2CG10[15:8] base clk2 for Fast-mode (400Khz) min tBuf 1.3us + * 0x12 : 400Khz : 10Mhz : 1.6us + * I2CG10[7:0] base clk1 for Fast-mode Plus (1Mhz) min tBuf 0.5us + * 0x08 : 1Mhz : 20Mhz : 0.8us + */ + +static int aspeed_i2c_global_probe(struct udevice *dev) +{ + struct ast2600_i2c_global_priv *i2c_global = dev_get_priv(dev); + int ret = 0; + + i2c_global->regs = dev_read_addr_ptr(dev); + if (!(i2c_global->regs)) + return -EINVAL; + + debug("%s(dev=%p)\n", __func__, dev); + + ret = reset_get_by_index(dev, 0, &i2c_global->reset); + if (ret) { + printf("%s(): Failed to get reset signal\n", __func__); + return ret; + } + + reset_deassert(&i2c_global->reset); + + writel(AST2600_GLOBAL_INIT, i2c_global->regs + + AST2600_I2CG_CTRL); + writel(I2CCG_DIV_CTRL, i2c_global->regs + + AST2600_I2CG_CLK_DIV_CTRL); + + return 0; +} + +static const struct udevice_id aspeed_i2c_global_ids[] = { + { .compatible = "aspeed,ast2600-i2c-global", }, + { } +}; + +U_BOOT_DRIVER(aspeed_i2c_global) = { + .name = "aspeed_i2c_global", + .id = UCLASS_MISC, + .of_match = aspeed_i2c_global_ids, + .probe = aspeed_i2c_global_probe, + .priv_auto = sizeof(struct ast2600_i2c_global_priv), +}; + -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] i2c:aspeed:support ast2600 i2c new register mode driver 2023-01-09 8:29 ` [PATCH 1/1] i2c:aspeed:support ast2600 i2c new register mode driver ryan_chen @ 2023-01-09 19:47 ` Simon Glass 2023-01-10 2:41 ` Ryan Chen 0 siblings, 1 reply; 7+ messages in thread From: Simon Glass @ 2023-01-09 19:47 UTC (permalink / raw) To: ryan_chen; +Cc: Heiko Schocher, Aspeed BMC SW team, u-boot Hi Ryan_chen, On Mon, 9 Jan 2023 at 01:30, ryan_chen <ryan_chen@aspeedtech.com> wrote: > > Add i2c new register mode driver to support AST2600 i2c > new register mode. AST2600 i2c controller have legacy and > new register mode. The new register mode have global register > support 4 base clock for scl clock selection, and new clock > divider mode. > > Signed-off-by: ryan_chen <ryan_chen@aspeedtech.com> > --- > MAINTAINERS | 6 + > drivers/i2c/Kconfig | 7 + > drivers/i2c/Makefile | 1 + > drivers/i2c/ast2600_i2c.c | 480 ++++++++++++++++++++++++++++++++++++++ > 4 files changed, 494 insertions(+) > create mode 100644 drivers/i2c/ast2600_i2c.c This generally looks OK, but I have quite a few minor comments, and one major one: could/should this driver be an update to the existing one, instead? That is the short of thing that really should be in your commit message. > > diff --git a/MAINTAINERS b/MAINTAINERS > index 3fc4cd0f12..1cf54f0b4e 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -769,6 +769,12 @@ S: Maintained > F: drivers/pci/pcie_phytium.c > F: arch/arm/dts/phytium-durian.dts > > +ASPEED AST2600 I2C DRIVER > +M: Ryan Chen <ryan_chen@aspeedtech.com> > +R: Aspeed BMC SW team <BMC-SW@aspeedtech.com> > +S: Maintained > +F: drivers/i2c/ast2600_i2c.c > + > ASPEED FMC SPI DRIVER > M: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> > M: Cédric Le Goater <clg@kaod.org> > diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig > index 08b6c7bdcc..34f507fb3b 100644 > --- a/drivers/i2c/Kconfig > +++ b/drivers/i2c/Kconfig > @@ -221,6 +221,13 @@ config SYS_I2C_DW > controller is used in various SoCs, e.g. the ST SPEAr, Altera > SoCFPGA, Synopsys ARC700 and some Intel x86 SoCs. > > +config SYS_I2C_AST2600 > + bool "AST2600 I2C Controller" > + depends on DM_I2C && ARCH_ASPEED > + help > + Say yes here to select AST2600 I2C Host Controller. The driver > + support AST2600 I2C new mode register. What speeds does it support? Please add at least 3 lines of info. A link to the datasheet would help too, either here or in doc/ > + > config SYS_I2C_ASPEED > bool "Aspeed I2C Controller" > depends on DM_I2C && ARCH_ASPEED > diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile > index 920aafb91c..89db2d8e37 100644 > --- a/drivers/i2c/Makefile > +++ b/drivers/i2c/Makefile > @@ -12,6 +12,7 @@ obj-$(CONFIG_$(SPL_)I2C_CROS_EC_LDO) += cros_ec_ldo.o > > obj-$(CONFIG_$(SPL_)SYS_I2C_LEGACY) += i2c_core.o > obj-$(CONFIG_SYS_I2C_ASPEED) += ast_i2c.o > +obj-$(CONFIG_SYS_I2C_AST2600) += ast2600_i2c.o > obj-$(CONFIG_SYS_I2C_AT91) += at91_i2c.o > obj-$(CONFIG_SYS_I2C_CADENCE) += i2c-cdns.o > obj-$(CONFIG_SYS_I2C_CA) += i2c-cortina.o > diff --git a/drivers/i2c/ast2600_i2c.c b/drivers/i2c/ast2600_i2c.c > new file mode 100644 > index 0000000000..52aea460ac > --- /dev/null > +++ b/drivers/i2c/ast2600_i2c.c > @@ -0,0 +1,480 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright ASPEED Technology Inc. > + */ > +#include <linux/iopoll.h> The ordering is off here. Please see https://u-boot.readthedocs.io/en/latest/develop/codingstyle.html#include-files > +#include <common.h> > +#include <clk.h> > +#include <dm.h> > +#include <errno.h> > +#include <fdtdec.h> > +#include <i2c.h> > +#include <asm/io.h> > +#include <regmap.h> > +#include <reset.h> > + > +struct ast2600_i2c_regs { > + u32 fun_ctrl; > + u32 ac_timing; > + u32 trx_buff; > + u32 icr; > + u32 ier; > + u32 isr; > + u32 cmd_sts; > +}; > + > +/* 0x00 : I2CC Master/Slave Function Control Register */ > +#define AST2600_I2CC_SLAVE_ADDR_RX_EN BIT(20) Perhaps this should go in the .h file like the other drivers? > +#define AST2600_I2CC_MASTER_RETRY_MASK GENMASK(19, 18) > +#define AST2600_I2CC_MASTER_RETRY(x) (((x) & GENMASK(1, 0)) << 18) Can you drop unused things? > +#define AST2600_I2CC_BUS_AUTO_RELEASE BIT(17) > +#define AST2600_I2CC_M_SDA_LOCK_EN BIT(16) > +#define AST2600_I2CC_MULTI_MASTER_DIS BIT(15) > +#define AST2600_I2CC_M_SCL_DRIVE_EN BIT(14) > +#define AST2600_I2CC_MSB_STS BIT(9) > +#define AST2600_I2CC_SDA_DRIVE_1T_EN BIT(8) > +#define AST2600_I2CC_M_SDA_DRIVE_1T_EN BIT(7) > +#define AST2600_I2CC_M_HIGH_SPEED_EN BIT(6) > +/* reserver 5 : 2 */ reserved? > +#define AST2600_I2CC_SLAVE_EN BIT(1) > +#define AST2600_I2CC_MASTER_EN BIT(0) > + > +/* 0x04 : I2CD Clock and AC Timing Control Register #1 */ > +/* Base register value. These bits are always set by the driver. */ > +#define I2CD_CACTC_BASE 0xfff00300 > +#define I2CD_TCKHIGH_SHIFT 16 > +#define I2CD_TCKLOW_SHIFT 12 > +#define I2CD_THDDAT_SHIFT 10 > +#define I2CD_TO_DIV_SHIFT 8 > +#define I2CD_BASE_DIV_SHIFT 0 > + > +/* 0x08 : I2CC Master/Slave Transmit/Receive Byte Buffer Register */ > +#define AST2600_I2CC_TX_DIR_MASK GENMASK(31, 29) > +#define AST2600_I2CC_SDA_OE BIT(28) > +#define AST2600_I2CC_SDA_O BIT(27) > +#define AST2600_I2CC_SCL_OE BIT(26) > +#define AST2600_I2CC_SCL_O BIT(25) > + > +#define AST2600_I2CC_SCL_LINE_STS BIT(18) > +#define AST2600_I2CC_SDA_LINE_STS BIT(17) > +#define AST2600_I2CC_BUS_BUSY_STS BIT(16) > +#define AST2600_I2CC_GET_RX_BUFF(x) (((x) >> 8) & GENMASK(7, 0)) Can you drop the AST2600 prefix here? It is too long and we know it is this driver. > + > +/* 0x10 : I2CM Master Interrupt Control Register */ > +/* 0x14 : I2CM Master Interrupt Status Register */ > +#define AST2600_I2CM_PKT_TIMEOUT BIT(18) > +#define AST2600_I2CM_PKT_ERROR BIT(17) > +#define AST2600_I2CM_PKT_DONE BIT(16) > + > +#define AST2600_I2CM_BUS_RECOVER_FAIL BIT(15) > +#define AST2600_I2CM_SDA_DL_TO BIT(14) > +#define AST2600_I2CM_BUS_RECOVER BIT(13) > +#define AST2600_I2CM_SMBUS_ALT BIT(12) > + > +#define AST2600_I2CM_SCL_LOW_TO BIT(6) > +#define AST2600_I2CM_ABNORMAL BIT(5) > +#define AST2600_I2CM_NORMAL_STOP BIT(4) > +#define AST2600_I2CM_ARBIT_LOSS BIT(3) > +#define AST2600_I2CM_RX_DONE BIT(2) > +#define AST2600_I2CM_TX_NAK BIT(1) > +#define AST2600_I2CM_TX_ACK BIT(0) > + > +/* 0x18 : I2CM Master Command/Status Register */ > +#define AST2600_I2CM_PKT_ADDR(x) (((x) & GENMASK(6, 0)) << 24) > +#define AST2600_I2CM_PKT_EN BIT(16) > +#define AST2600_I2CM_SDA_OE_OUT_DIR BIT(15) > +#define AST2600_I2CM_SDA_O_OUT_DIR BIT(14) > +#define AST2600_I2CM_SCL_OE_OUT_DIR BIT(13) > +#define AST2600_I2CM_SCL_O_OUT_DIR BIT(12) > +#define AST2600_I2CM_RECOVER_CMD_EN BIT(11) > + > +#define AST2600_I2CM_RX_DMA_EN BIT(9) > +#define AST2600_I2CM_TX_DMA_EN BIT(8) > +/* Command Bit */ > +#define AST2600_I2CM_RX_BUFF_EN BIT(7) > +#define AST2600_I2CM_TX_BUFF_EN BIT(6) > +#define AST2600_I2CM_STOP_CMD BIT(5) > +#define AST2600_I2CM_RX_CMD_LAST BIT(4) > +#define AST2600_I2CM_RX_CMD BIT(3) > + > +#define AST2600_I2CM_TX_CMD BIT(1) > +#define AST2600_I2CM_START_CMD BIT(0) > + > +#define I2C_TIMEOUT_US 100000 > +/* > + * Device private data > + */ single-line comment style is /* Device-private data */ > +struct ast2600_i2c_priv { > + struct clk clk; > + struct ast2600_i2c_regs *regs; > + void __iomem *global; > +}; > + > +static int ast2600_i2c_read_data(struct ast2600_i2c_priv *priv, u8 chip_addr, > + u8 *buffer, size_t len, bool send_stop) Please add a full comment for this one > +{ > + int rx_cnt, ret = 0; > + u32 cmd, isr; > + > + for (rx_cnt = 0; rx_cnt < len; rx_cnt++, buffer++) { > + cmd = AST2600_I2CM_PKT_EN | AST2600_I2CM_PKT_ADDR(chip_addr) | > + AST2600_I2CM_RX_CMD; > + if (!rx_cnt) > + cmd |= AST2600_I2CM_START_CMD; > + > + if ((len - 1) == rx_cnt) > + cmd |= AST2600_I2CM_RX_CMD_LAST; > + > + if (send_stop && ((len - 1) == rx_cnt)) > + cmd |= AST2600_I2CM_STOP_CMD; > + > + writel(cmd, &priv->regs->cmd_sts); > + > + ret = readl_poll_timeout(&priv->regs->isr, isr, > + isr & AST2600_I2CM_PKT_DONE, > + I2C_TIMEOUT_US); > + if (ret) > + return -ETIMEDOUT; > + > + *buffer = > + AST2600_I2CC_GET_RX_BUFF(readl(&priv->regs->trx_buff)); ^^ too long > + > + writel(AST2600_I2CM_PKT_DONE, &priv->regs->isr); > + > + if (isr & AST2600_I2CM_TX_NAK) > + return -EREMOTEIO; > + } > + > + return 0; > +} > + > +static int ast2600_i2c_write_data(struct ast2600_i2c_priv *priv, u8 chip_addr, > + u8 *buffer, size_t len, bool send_stop) and this > +{ > + int tx_cnt, ret = 0; > + u32 cmd, isr; > + > + if (!len) { > + cmd = AST2600_I2CM_PKT_EN | AST2600_I2CM_PKT_ADDR(chip_addr) | > + AST2600_I2CM_START_CMD; > + writel(cmd, &priv->regs->cmd_sts); > + ret = readl_poll_timeout(&priv->regs->isr, isr, > + isr & AST2600_I2CM_PKT_DONE, > + I2C_TIMEOUT_US); > + if (ret) > + return -ETIMEDOUT; > + > + writel(AST2600_I2CM_PKT_DONE, &priv->regs->isr); > + > + if (isr & AST2600_I2CM_TX_NAK) > + return -EREMOTEIO; > + } > + > + for (tx_cnt = 0; tx_cnt < len; tx_cnt++, buffer++) { > + cmd = AST2600_I2CM_PKT_EN | AST2600_I2CM_PKT_ADDR(chip_addr); > + cmd |= AST2600_I2CM_TX_CMD; > + > + if (!tx_cnt) > + cmd |= AST2600_I2CM_START_CMD; > + > + if (send_stop && ((len - 1) == tx_cnt)) > + cmd |= AST2600_I2CM_STOP_CMD; > + > + writel(*buffer, &priv->regs->trx_buff); > + writel(cmd, &priv->regs->cmd_sts); > + ret = readl_poll_timeout(&priv->regs->isr, isr, > + isr & AST2600_I2CM_PKT_DONE, > + I2C_TIMEOUT_US); > + if (ret) > + return -ETIMEDOUT; > + > + writel(AST2600_I2CM_PKT_DONE, &priv->regs->isr); > + > + if (isr & AST2600_I2CM_TX_NAK) > + return -EREMOTEIO; > + } > + > + return 0; > +} > + > +static int ast2600_i2c_deblock(struct udevice *dev) > +{ > + struct ast2600_i2c_priv *priv = dev_get_priv(dev); > + u32 csr = readl(&priv->regs->cmd_sts); > + u32 isr; > + int ret; > + > + //reinit /* reinit */ > + writel(0, &priv->regs->fun_ctrl); > + /* Enable Master Mode. Assuming single-master */ > + writel(AST2600_I2CC_BUS_AUTO_RELEASE | AST2600_I2CC_MASTER_EN | > + AST2600_I2CC_MULTI_MASTER_DIS, > + &priv->regs->fun_ctrl); > + [..] > +static int ast2600_i2c_set_speed(struct udevice *dev, unsigned int speed) > +{ > + struct ast2600_i2c_priv *priv = dev_get_priv(dev); > + unsigned long base_clk1, base_clk2, base_clk3, base_clk4; > + int baseclk_idx; > + u32 clk_div_reg; > + u32 apb_clk; > + u32 scl_low; > + u32 scl_high; > + int divisor; > + int inc = 0; > + u32 data; > + > + debug("Setting speed for I2C%d to <%u>\n", dev->seq_, speed); > + if (!speed) { > + debug("No valid speed specified\n"); > + return -EINVAL; > + } > + > + apb_clk = clk_get_rate(&priv->clk); > + > + clk_div_reg = readl(priv->global + 0x10); > + base_clk1 = (apb_clk * 10) / ((((clk_div_reg & 0xff) + 2) * 10) / 2); Can apb_clk * 10 go in a local var to reduce duplication? > + base_clk2 = > + (apb_clk * 10) / (((((clk_div_reg >> 8) & 0xff) + 2) * 10) / 2); > + base_clk3 = (apb_clk * 10) / > + (((((clk_div_reg >> 16) & 0xff) + 2) * 10) / 2); > + base_clk4 = (apb_clk * 10) / > + (((((clk_div_reg >> 24) & 0xff) + 2) * 10) / 2); Normally we like to define these fields, like: #define CLK1_DIV_SHIFT 0 #define CLK1_DIV_MASK (0xff >> CLK1_DIV_SHIFT) #define CLK2_DIV_SHIFT 8 #define CLK2_DIV_MASK (0xff >> CLK2_DIV_SHIFT) etc. > + > + if ((apb_clk / speed) <= 32) { > + baseclk_idx = 0; > + divisor = DIV_ROUND_UP(apb_clk, speed); > + } else if ((base_clk1 / speed) <= 32) { > + baseclk_idx = 1; > + divisor = DIV_ROUND_UP(base_clk1, speed); > + } else if ((base_clk2 / speed) <= 32) { > + baseclk_idx = 2; > + divisor = DIV_ROUND_UP(base_clk2, speed); > + } else if ((base_clk3 / speed) <= 32) { > + baseclk_idx = 3; > + divisor = DIV_ROUND_UP(base_clk3, speed); > + } else { > + baseclk_idx = 4; > + divisor = DIV_ROUND_UP(base_clk4, speed); > + inc = 0; > + while ((divisor + inc) > 32) { > + inc |= divisor & 0x1; > + divisor >>= 1; > + baseclk_idx++; > + } > + divisor += inc; > + } > + divisor = min_t(int, divisor, 32); > + baseclk_idx &= 0xf; What happens if baseclk_idx is >= 16 ? > + scl_low = ((divisor * 9) / 16) - 1; > + scl_low = min_t(u32, scl_low, 0xf); > + scl_high = (divisor - scl_low - 2) & 0xf; > + /* Divisor : Base Clock : tCKHighMin : tCK High : tCK Low */ > + data = ((scl_high - 1) << 20) | (scl_high << 16) | (scl_low << 12) | > + (baseclk_idx); drop extra () around baseclk_idx > + /* Set AC Timing */ > + writel(data, &priv->regs->ac_timing); > + > + return 0; > +} > + > +static int ast2600_i2c_probe(struct udevice *dev) > +{ > + struct ast2600_i2c_priv *priv = dev_get_priv(dev); > + struct udevice *misc_dev; > + int ret; > + > + /* find global base address */ > + ret = uclass_get_device_by_driver(UCLASS_MISC, > + DM_DRIVER_GET(aspeed_i2c_global), > + &misc_dev); But isn't this the parent device of this one? Just use device_get_parent(dev) Also misc_dev is not a great name. How about glob_dev ? > + if (ret) { > + debug("i2c global not defined\n"); > + return ret; > + } > + > + priv->global = devfdt_get_addr_ptr(misc_dev); dev_read_addr() > + if (IS_ERR(priv->global)) { > + debug("%s(): can't get global\n", __func__); > + return PTR_ERR(priv->global); > + } > + > + /* Reset device */ > + writel(0, &priv->regs->fun_ctrl); When you have functions with a lot of priv->regs things it makes sense to add a local 'regs' variable. > + /* Enable Master Mode. Assuming single-master */ > + writel(AST2600_I2CC_BUS_AUTO_RELEASE | AST2600_I2CC_MASTER_EN | > + AST2600_I2CC_MULTI_MASTER_DIS, > + &priv->regs->fun_ctrl); > + > + writel(0, &priv->regs->ier); > + /* Clear Interrupt */ > + writel(~0, &priv->regs->isr); > + > + return 0; > +} > + > +static int ast2600_i2c_of_to_plat(struct udevice *dev) > +{ > + struct ast2600_i2c_priv *priv = dev_get_priv(dev); > + int ret; > + > + priv->regs = dev_read_addr_ptr(dev); > + if (!(priv->regs)) Drop extra () > + return -EINVAL; > + > + ret = clk_get_by_index(dev, 0, &priv->clk); > + if (ret < 0) { > + debug("%s: Can't get clock for %s: %d\n", __func__, dev->name, > + ret); > + return ret; > + } > + > + return 0; > +} > + > +static const struct dm_i2c_ops ast2600_i2c_ops = { > + .xfer = ast2600_i2c_xfer, > + .deblock = ast2600_i2c_deblock, > + .set_bus_speed = ast2600_i2c_set_speed, > +}; > + > +static const struct udevice_id ast2600_i2c_ids[] = { > + { .compatible = "aspeed,ast2600-i2c" }, > + {}, > +}; > + > +U_BOOT_DRIVER(ast2600_i2c) = { > + .name = "ast2600_i2c", > + .id = UCLASS_I2C, > + .of_match = ast2600_i2c_ids, > + .probe = ast2600_i2c_probe, > + .of_to_plat = ast2600_i2c_of_to_plat, > + .priv_auto = sizeof(struct ast2600_i2c_priv), > + .ops = &ast2600_i2c_ops, > +}; > + > +#define AST2600_I2CG_ISR 0x00 > +#define AST2600_I2CG_SLAVE_ISR 0x04 > +#define AST2600_I2CG_OWNER 0x08 > +#define AST2600_I2CG_CTRL 0x0C > +#define AST2600_I2CG_CLK_DIV_CTRL 0x10 > + > +#define AST2600_I2CG_SLAVE_PKT_NAK BIT(4) > +#define AST2600_I2CG_M_S_SEPARATE_INTR BIT(3) > +#define AST2600_I2CG_CTRL_NEW_REG BIT(2) > +#define AST2600_I2CG_CTRL_NEW_CLK_DIV BIT(1) > + > +#define AST2600_GLOBAL_INIT \ > + (AST2600_I2CG_SLAVE_PKT_NAK | \ > + AST2600_I2CG_CTRL_NEW_REG | \ > + AST2600_I2CG_CTRL_NEW_CLK_DIV) > +#define I2CCG_DIV_CTRL 0xC6411208 Please use lower-case hex consistently Hmm these should really go in the header file too? > + > +struct ast2600_i2c_global_priv { > + void __iomem *regs; > + struct reset_ctl reset; > +}; > + > +/* > + * APB clk : 100Mhz > + * div : scl : baseclk [APB/((div/2) + 1)] : tBuf [1/bclk * 16] > + * I2CG10[31:24] base clk4 for i2c auto recovery timeout counter (0xC6) > + * I2CG10[23:16] base clk3 for Standard-mode (100Khz) min tBuf 4.7us > + * 0x3c : 100.8Khz : 3.225Mhz : 4.96us > + * 0x3d : 99.2Khz : 3.174Mhz : 5.04us > + * 0x3e : 97.65Khz : 3.125Mhz : 5.12us > + * 0x40 : 97.75Khz : 3.03Mhz : 5.28us > + * 0x41 : 99.5Khz : 2.98Mhz : 5.36us (default) > + * I2CG10[15:8] base clk2 for Fast-mode (400Khz) min tBuf 1.3us > + * 0x12 : 400Khz : 10Mhz : 1.6us > + * I2CG10[7:0] base clk1 for Fast-mode Plus (1Mhz) min tBuf 0.5us > + * 0x08 : 1Mhz : 20Mhz : 0.8us > + */ > + > +static int aspeed_i2c_global_probe(struct udevice *dev) > +{ > + struct ast2600_i2c_global_priv *i2c_global = dev_get_priv(dev); > + int ret = 0; > + > + i2c_global->regs = dev_read_addr_ptr(dev); > + if (!(i2c_global->regs)) again too many (). Please fix globally > + return -EINVAL; > + > + debug("%s(dev=%p)\n", __func__, dev); > + > + ret = reset_get_by_index(dev, 0, &i2c_global->reset); > + if (ret) { > + printf("%s(): Failed to get reset signal\n", __func__); log_err() if you want an error. Try to avoid using __func__ - use log...() instead > + return ret; > + } > + > + reset_deassert(&i2c_global->reset); Can this fail? > + > + writel(AST2600_GLOBAL_INIT, i2c_global->regs + > + AST2600_I2CG_CTRL); > + writel(I2CCG_DIV_CTRL, i2c_global->regs + > + AST2600_I2CG_CLK_DIV_CTRL); > + > + return 0; > +} > + > +static const struct udevice_id aspeed_i2c_global_ids[] = { > + { .compatible = "aspeed,ast2600-i2c-global", }, > + { } > +}; > + > +U_BOOT_DRIVER(aspeed_i2c_global) = { > + .name = "aspeed_i2c_global", > + .id = UCLASS_MISC, > + .of_match = aspeed_i2c_global_ids, > + .probe = aspeed_i2c_global_probe, > + .priv_auto = sizeof(struct ast2600_i2c_global_priv), > +}; > + > -- > 2.34.1 > Regards, Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH 1/1] i2c:aspeed:support ast2600 i2c new register mode driver 2023-01-09 19:47 ` Simon Glass @ 2023-01-10 2:41 ` Ryan Chen 0 siblings, 0 replies; 7+ messages in thread From: Ryan Chen @ 2023-01-10 2:41 UTC (permalink / raw) To: Simon Glass; +Cc: Heiko Schocher, BMC-SW, u-boot@lists.denx.de Hello Simon, Thank your feedback. > -----Original Message----- > From: Simon Glass <sjg@chromium.org> > Sent: Tuesday, January 10, 2023 3:47 AM > To: Ryan Chen <ryan_chen@aspeedtech.com> > Cc: Heiko Schocher <hs@denx.de>; BMC-SW <BMC-SW@aspeedtech.com>; > u-boot@lists.denx.de > Subject: Re: [PATCH 1/1] i2c:aspeed:support ast2600 i2c new register mode > driver > > Hi Ryan_chen, > > On Mon, 9 Jan 2023 at 01:30, ryan_chen <ryan_chen@aspeedtech.com> > wrote: > > > > Add i2c new register mode driver to support AST2600 i2c new register > > mode. AST2600 i2c controller have legacy and new register mode. The > > new register mode have global register support 4 base clock for scl > > clock selection, and new clock divider mode. > > > > Signed-off-by: ryan_chen <ryan_chen@aspeedtech.com> > > --- > > MAINTAINERS | 6 + > > drivers/i2c/Kconfig | 7 + > > drivers/i2c/Makefile | 1 + > > drivers/i2c/ast2600_i2c.c | 480 > > ++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 494 insertions(+) > > create mode 100644 drivers/i2c/ast2600_i2c.c > > This generally looks OK, but I have quite a few minor comments, and one > major one: could/should this driver be an update to the existing one, instead? > That is the short of thing that really should be in your commit message. > This driver is now driver not be an update to the exiting one. > > > > diff --git a/MAINTAINERS b/MAINTAINERS index 3fc4cd0f12..1cf54f0b4e > > 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -769,6 +769,12 @@ S: Maintained > > F: drivers/pci/pcie_phytium.c > > F: arch/arm/dts/phytium-durian.dts > > > > +ASPEED AST2600 I2C DRIVER > > +M: Ryan Chen <ryan_chen@aspeedtech.com> > > +R: Aspeed BMC SW team <BMC-SW@aspeedtech.com> > > +S: Maintained > > +F: drivers/i2c/ast2600_i2c.c > > + > > ASPEED FMC SPI DRIVER > > M: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com> > > M: Cédric Le Goater <clg@kaod.org> > > diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig index > > 08b6c7bdcc..34f507fb3b 100644 > > --- a/drivers/i2c/Kconfig > > +++ b/drivers/i2c/Kconfig > > @@ -221,6 +221,13 @@ config SYS_I2C_DW > > controller is used in various SoCs, e.g. the ST SPEAr, Altera > > SoCFPGA, Synopsys ARC700 and some Intel x86 SoCs. > > > > +config SYS_I2C_AST2600 > > + bool "AST2600 I2C Controller" > > + depends on DM_I2C && ARCH_ASPEED > > + help > > + Say yes here to select AST2600 I2C Host Controller. The driver > > + support AST2600 I2C new mode register. > > What speeds does it support? Will modify by following. +config SYS_I2C_AST2600 + bool "AST2600 I2C Controller" + depends on DM_I2C && ARCH_ASPEED + help + Say yes here to select AST2600 I2C Host Controller. The driver + support AST2600 I2C new mode register. This I2C controller supports: + -Standard-mode (up to 100 kHz) + -Fast-mode (up to 400 kHz) + -Fast-mode Plus (up to 1 MHz) >Please add at least 3 lines of info. Sorry, what do you mean about 3 lines of info? The i2c have two lines, SDA/SCL only. > > A link to the datasheet would help too, either here or in doc/ > > > + > > config SYS_I2C_ASPEED > > bool "Aspeed I2C Controller" > > depends on DM_I2C && ARCH_ASPEED diff --git > > a/drivers/i2c/Makefile b/drivers/i2c/Makefile index > > 920aafb91c..89db2d8e37 100644 > > --- a/drivers/i2c/Makefile > > +++ b/drivers/i2c/Makefile > > @@ -12,6 +12,7 @@ obj-$(CONFIG_$(SPL_)I2C_CROS_EC_LDO) += > > cros_ec_ldo.o > > > > obj-$(CONFIG_$(SPL_)SYS_I2C_LEGACY) += i2c_core.o > > obj-$(CONFIG_SYS_I2C_ASPEED) += ast_i2c.o > > +obj-$(CONFIG_SYS_I2C_AST2600) += ast2600_i2c.o > > obj-$(CONFIG_SYS_I2C_AT91) += at91_i2c.o > > obj-$(CONFIG_SYS_I2C_CADENCE) += i2c-cdns.o > > obj-$(CONFIG_SYS_I2C_CA) += i2c-cortina.o diff --git > > a/drivers/i2c/ast2600_i2c.c b/drivers/i2c/ast2600_i2c.c new file mode > > 100644 index 0000000000..52aea460ac > > --- /dev/null > > +++ b/drivers/i2c/ast2600_i2c.c > > @@ -0,0 +1,480 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright ASPEED Technology Inc. > > + */ > > +#include <linux/iopoll.h> > > The ordering is off here. Please see > > https://u-boot.readthedocs.io/en/latest/develop/codingstyle.html#include-files Will update > > > +#include <common.h> > > +#include <clk.h> > > +#include <dm.h> > > +#include <errno.h> > > +#include <fdtdec.h> > > +#include <i2c.h> > > +#include <asm/io.h> > > +#include <regmap.h> > > +#include <reset.h> > > + > > +struct ast2600_i2c_regs { > > + u32 fun_ctrl; > > + u32 ac_timing; > > + u32 trx_buff; > > + u32 icr; > > + u32 ier; > > + u32 isr; > > + u32 cmd_sts; > > +}; > > + > > +/* 0x00 : I2CC Master/Slave Function Control Register */ #define > > +AST2600_I2CC_SLAVE_ADDR_RX_EN BIT(20) > > Perhaps this should go in the .h file like the other drivers? Yes, will go for ast2600_i2c.h file > > > +#define AST2600_I2CC_MASTER_RETRY_MASK GENMASK(19, 18) > > +#define AST2600_I2CC_MASTER_RETRY(x) (((x) & GENMASK(1, 0)) << > 18) > > Can you drop unused things? Those are register definition, could I keep this for avoid future used? > > > +#define AST2600_I2CC_BUS_AUTO_RELEASE BIT(17) > > +#define AST2600_I2CC_M_SDA_LOCK_EN BIT(16) > > +#define AST2600_I2CC_MULTI_MASTER_DIS BIT(15) > > +#define AST2600_I2CC_M_SCL_DRIVE_EN BIT(14) > > +#define AST2600_I2CC_MSB_STS BIT(9) > > +#define AST2600_I2CC_SDA_DRIVE_1T_EN BIT(8) > > +#define AST2600_I2CC_M_SDA_DRIVE_1T_EN BIT(7) > > +#define AST2600_I2CC_M_HIGH_SPEED_EN BIT(6) > > +/* reserver 5 : 2 */ > > reserved? will update /* reserved 5 : 2 */ > > > +#define AST2600_I2CC_SLAVE_EN BIT(1) > > +#define AST2600_I2CC_MASTER_EN BIT(0) > > + > > +/* 0x04 : I2CD Clock and AC Timing Control Register #1 */ > > +/* Base register value. These bits are always set by the driver. */ > > +#define I2CD_CACTC_BASE 0xfff00300 > > +#define I2CD_TCKHIGH_SHIFT 16 > > +#define I2CD_TCKLOW_SHIFT 12 > > +#define I2CD_THDDAT_SHIFT 10 > > +#define I2CD_TO_DIV_SHIFT 8 > > +#define I2CD_BASE_DIV_SHIFT 0 > > + > > +/* 0x08 : I2CC Master/Slave Transmit/Receive Byte Buffer Register */ > > +#define AST2600_I2CC_TX_DIR_MASK GENMASK(31, 29) > > +#define AST2600_I2CC_SDA_OE BIT(28) > > +#define AST2600_I2CC_SDA_O BIT(27) > > +#define AST2600_I2CC_SCL_OE BIT(26) > > +#define AST2600_I2CC_SCL_O BIT(25) > > + > > +#define AST2600_I2CC_SCL_LINE_STS BIT(18) > > +#define AST2600_I2CC_SDA_LINE_STS BIT(17) > > +#define AST2600_I2CC_BUS_BUSY_STS BIT(16) > > +#define AST2600_I2CC_GET_RX_BUFF(x) (((x) >> 8) & > GENMASK(7, 0)) > > Can you drop the AST2600 prefix here? It is too long and we know it is this > driver. Sorry, Do you mean direct drop string "AST2600"? Ex: AST2600_I2CC_SCL_LINE_STS => I2CC_SCL_LINE_STS > > > + > > +/* 0x10 : I2CM Master Interrupt Control Register */ > > +/* 0x14 : I2CM Master Interrupt Status Register */ > > +#define AST2600_I2CM_PKT_TIMEOUT BIT(18) > > +#define AST2600_I2CM_PKT_ERROR BIT(17) > > +#define AST2600_I2CM_PKT_DONE BIT(16) > > + > > +#define AST2600_I2CM_BUS_RECOVER_FAIL BIT(15) > > +#define AST2600_I2CM_SDA_DL_TO BIT(14) > > +#define AST2600_I2CM_BUS_RECOVER BIT(13) > > +#define AST2600_I2CM_SMBUS_ALT BIT(12) > > + > > +#define AST2600_I2CM_SCL_LOW_TO BIT(6) > > +#define AST2600_I2CM_ABNORMAL BIT(5) > > +#define AST2600_I2CM_NORMAL_STOP BIT(4) > > +#define AST2600_I2CM_ARBIT_LOSS BIT(3) > > +#define AST2600_I2CM_RX_DONE BIT(2) > > +#define AST2600_I2CM_TX_NAK BIT(1) > > +#define AST2600_I2CM_TX_ACK BIT(0) > > + > > +/* 0x18 : I2CM Master Command/Status Register */ > > +#define AST2600_I2CM_PKT_ADDR(x) (((x) & GENMASK(6, > 0)) << 24) > > +#define AST2600_I2CM_PKT_EN BIT(16) > > +#define AST2600_I2CM_SDA_OE_OUT_DIR BIT(15) > > +#define AST2600_I2CM_SDA_O_OUT_DIR BIT(14) > > +#define AST2600_I2CM_SCL_OE_OUT_DIR BIT(13) > > +#define AST2600_I2CM_SCL_O_OUT_DIR BIT(12) > > +#define AST2600_I2CM_RECOVER_CMD_EN BIT(11) > > + > > +#define AST2600_I2CM_RX_DMA_EN BIT(9) > > +#define AST2600_I2CM_TX_DMA_EN BIT(8) > > +/* Command Bit */ > > +#define AST2600_I2CM_RX_BUFF_EN BIT(7) > > +#define AST2600_I2CM_TX_BUFF_EN BIT(6) > > +#define AST2600_I2CM_STOP_CMD BIT(5) > > +#define AST2600_I2CM_RX_CMD_LAST BIT(4) > > +#define AST2600_I2CM_RX_CMD BIT(3) > > + > > +#define AST2600_I2CM_TX_CMD BIT(1) > > +#define AST2600_I2CM_START_CMD BIT(0) > > + > > +#define I2C_TIMEOUT_US 100000 > > +/* > > + * Device private data > > + */ > > single-line comment style is > > /* Device-private data */ > Will update > > +struct ast2600_i2c_priv { > > + struct clk clk; > > + struct ast2600_i2c_regs *regs; > > + void __iomem *global; > > +}; > > + > > +static int ast2600_i2c_read_data(struct ast2600_i2c_priv *priv, u8 > chip_addr, > > + u8 *buffer, size_t len, bool > > +send_stop) > > Please add a full comment for this one Sorry, what comment for this function, it is i2c read/write function like others driver implement. I don't see any comment in others driver implement. Could you help point me what driver reference? > > > +{ > > + int rx_cnt, ret = 0; > > + u32 cmd, isr; > > + > > + for (rx_cnt = 0; rx_cnt < len; rx_cnt++, buffer++) { > > + cmd = AST2600_I2CM_PKT_EN | > AST2600_I2CM_PKT_ADDR(chip_addr) | > > + AST2600_I2CM_RX_CMD; > > + if (!rx_cnt) > > + cmd |= AST2600_I2CM_START_CMD; > > + > > + if ((len - 1) == rx_cnt) > > + cmd |= AST2600_I2CM_RX_CMD_LAST; > > + > > + if (send_stop && ((len - 1) == rx_cnt)) > > + cmd |= AST2600_I2CM_STOP_CMD; > > + > > + writel(cmd, &priv->regs->cmd_sts); > > + > > + ret = readl_poll_timeout(&priv->regs->isr, isr, > > + isr & > AST2600_I2CM_PKT_DONE, > > + I2C_TIMEOUT_US); > > + if (ret) > > + return -ETIMEDOUT; > > + > > + *buffer = > > + > > + AST2600_I2CC_GET_RX_BUFF(readl(&priv->regs->trx_buff)); > > ^^ too long > tmp = readl(&priv->regs->trx_buff) AST2600_I2CC_GET_RX_BUFF(tmp); Is better ? > > + > > + writel(AST2600_I2CM_PKT_DONE, &priv->regs->isr); > > + > > + if (isr & AST2600_I2CM_TX_NAK) > > + return -EREMOTEIO; > > + } > > + > > + return 0; > > +} > > + > > +static int ast2600_i2c_write_data(struct ast2600_i2c_priv *priv, u8 > chip_addr, > > + u8 *buffer, size_t len, bool > > +send_stop) > > and this > > > +{ > > + int tx_cnt, ret = 0; > > + u32 cmd, isr; > > + > > + if (!len) { > > + cmd = AST2600_I2CM_PKT_EN | > AST2600_I2CM_PKT_ADDR(chip_addr) | > > + AST2600_I2CM_START_CMD; > > + writel(cmd, &priv->regs->cmd_sts); > > + ret = readl_poll_timeout(&priv->regs->isr, isr, > > + isr & > AST2600_I2CM_PKT_DONE, > > + I2C_TIMEOUT_US); > > + if (ret) > > + return -ETIMEDOUT; > > + > > + writel(AST2600_I2CM_PKT_DONE, &priv->regs->isr); > > + > > + if (isr & AST2600_I2CM_TX_NAK) > > + return -EREMOTEIO; > > + } > > + > > + for (tx_cnt = 0; tx_cnt < len; tx_cnt++, buffer++) { > > + cmd = AST2600_I2CM_PKT_EN | > AST2600_I2CM_PKT_ADDR(chip_addr); > > + cmd |= AST2600_I2CM_TX_CMD; > > + > > + if (!tx_cnt) > > + cmd |= AST2600_I2CM_START_CMD; > > + > > + if (send_stop && ((len - 1) == tx_cnt)) > > + cmd |= AST2600_I2CM_STOP_CMD; > > + > > + writel(*buffer, &priv->regs->trx_buff); > > + writel(cmd, &priv->regs->cmd_sts); > > + ret = readl_poll_timeout(&priv->regs->isr, isr, > > + isr & > AST2600_I2CM_PKT_DONE, > > + I2C_TIMEOUT_US); > > + if (ret) > > + return -ETIMEDOUT; > > + > > + writel(AST2600_I2CM_PKT_DONE, &priv->regs->isr); > > + > > + if (isr & AST2600_I2CM_TX_NAK) > > + return -EREMOTEIO; > > + } > > + > > + return 0; > > +} > > + > > +static int ast2600_i2c_deblock(struct udevice *dev) { > > + struct ast2600_i2c_priv *priv = dev_get_priv(dev); > > + u32 csr = readl(&priv->regs->cmd_sts); > > + u32 isr; > > + int ret; > > + > > + //reinit > > /* reinit */ Will update > > > + writel(0, &priv->regs->fun_ctrl); > > + /* Enable Master Mode. Assuming single-master */ > > + writel(AST2600_I2CC_BUS_AUTO_RELEASE | > AST2600_I2CC_MASTER_EN | > > + AST2600_I2CC_MULTI_MASTER_DIS, > > + &priv->regs->fun_ctrl); > > + > > [..] > > > +static int ast2600_i2c_set_speed(struct udevice *dev, unsigned int > > +speed) { > > + struct ast2600_i2c_priv *priv = dev_get_priv(dev); > > + unsigned long base_clk1, base_clk2, base_clk3, base_clk4; > > + int baseclk_idx; > > + u32 clk_div_reg; > > + u32 apb_clk; > > + u32 scl_low; > > + u32 scl_high; > > + int divisor; > > + int inc = 0; > > + u32 data; > > + > > + debug("Setting speed for I2C%d to <%u>\n", dev->seq_, speed); > > + if (!speed) { > > + debug("No valid speed specified\n"); > > + return -EINVAL; > > + } > > + > > + apb_clk = clk_get_rate(&priv->clk); > > + > > + clk_div_reg = readl(priv->global + 0x10); > > + base_clk1 = (apb_clk * 10) / ((((clk_div_reg & 0xff) + 2) * > > + 10) / 2); > > Can apb_clk * 10 go in a local var to reduce duplication? Yes, will update > > > + base_clk2 = > > + (apb_clk * 10) / (((((clk_div_reg >> 8) & 0xff) + 2) * 10) / > 2); > > + base_clk3 = (apb_clk * 10) / > > + (((((clk_div_reg >> 16) & 0xff) + 2) * 10) / 2); > > + base_clk4 = (apb_clk * 10) / > > + (((((clk_div_reg >> 24) & 0xff) + 2) * 10) / 2); > > Normally we like to define these fields, like: > > #define CLK1_DIV_SHIFT 0 > #define CLK1_DIV_MASK (0xff >> CLK1_DIV_SHIFT) #define CLK2_DIV_SHIFT 8 > #define CLK2_DIV_MASK (0xff >> CLK2_DIV_SHIFT) > Thanks, will MASK define for usage. > etc. > > > + > > + if ((apb_clk / speed) <= 32) { > > + baseclk_idx = 0; > > + divisor = DIV_ROUND_UP(apb_clk, speed); > > + } else if ((base_clk1 / speed) <= 32) { > > + baseclk_idx = 1; > > + divisor = DIV_ROUND_UP(base_clk1, speed); > > + } else if ((base_clk2 / speed) <= 32) { > > + baseclk_idx = 2; > > + divisor = DIV_ROUND_UP(base_clk2, speed); > > + } else if ((base_clk3 / speed) <= 32) { > > + baseclk_idx = 3; > > + divisor = DIV_ROUND_UP(base_clk3, speed); > > + } else { > > + baseclk_idx = 4; > > + divisor = DIV_ROUND_UP(base_clk4, speed); > > + inc = 0; > > + while ((divisor + inc) > 32) { > > + inc |= divisor & 0x1; > > + divisor >>= 1; > > + baseclk_idx++; > > + } > > + divisor += inc; > > + } > > + divisor = min_t(int, divisor, 32); > > + baseclk_idx &= 0xf; > > What happens if baseclk_idx is >= 16 ? In previous if else case. baseclk_idx only have five cases. baseclk_idx =0, 1, 2, 3, 4, 5, no others cases. Even >= 16 > > > + scl_low = ((divisor * 9) / 16) - 1; > > + scl_low = min_t(u32, scl_low, 0xf); > > + scl_high = (divisor - scl_low - 2) & 0xf; > > + /* Divisor : Base Clock : tCKHighMin : tCK High : tCK Low */ > > + data = ((scl_high - 1) << 20) | (scl_high << 16) | (scl_low << 12) | > > + (baseclk_idx); > > drop extra () around baseclk_idx Will remove () > > > + /* Set AC Timing */ > > + writel(data, &priv->regs->ac_timing); > > + > > + return 0; > > +} > > + > > +static int ast2600_i2c_probe(struct udevice *dev) { > > + struct ast2600_i2c_priv *priv = dev_get_priv(dev); > > + struct udevice *misc_dev; > > + int ret; > > + > > + /* find global base address */ > > + ret = uclass_get_device_by_driver(UCLASS_MISC, > > + > DM_DRIVER_GET(aspeed_i2c_global), > > + &misc_dev); > > But isn't this the parent device of this one? Just use device_get_parent(dev) > Yes, use parent device will be better. > Also misc_dev is not a great name. How about glob_dev ? Great, this naming is better. global_dev > > > + if (ret) { > > + debug("i2c global not defined\n"); > > + return ret; > > + } > > + > > + priv->global = devfdt_get_addr_ptr(misc_dev); > > dev_read_addr() Will update > > > + if (IS_ERR(priv->global)) { > > + debug("%s(): can't get global\n", __func__); > > + return PTR_ERR(priv->global); > > + } > > + > > + /* Reset device */ > > + writel(0, &priv->regs->fun_ctrl); > > When you have functions with a lot of priv->regs things it makes sense to add a > local 'regs' variable. Yes, will update with local regs = &priv->regs > > > + /* Enable Master Mode. Assuming single-master */ > > + writel(AST2600_I2CC_BUS_AUTO_RELEASE | > AST2600_I2CC_MASTER_EN | > > + AST2600_I2CC_MULTI_MASTER_DIS, > > + &priv->regs->fun_ctrl); > > + > > + writel(0, &priv->regs->ier); > > + /* Clear Interrupt */ > > + writel(~0, &priv->regs->isr); > > + > > + return 0; > > +} > > + > > +static int ast2600_i2c_of_to_plat(struct udevice *dev) { > > + struct ast2600_i2c_priv *priv = dev_get_priv(dev); > > + int ret; > > + > > + priv->regs = dev_read_addr_ptr(dev); > > + if (!(priv->regs)) > > Drop extra () Will update > > > + return -EINVAL; > > + > > + ret = clk_get_by_index(dev, 0, &priv->clk); > > + if (ret < 0) { > > + debug("%s: Can't get clock for %s: %d\n", __func__, > dev->name, > > + ret); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static const struct dm_i2c_ops ast2600_i2c_ops = { > > + .xfer = ast2600_i2c_xfer, > > + .deblock = ast2600_i2c_deblock, > > + .set_bus_speed = ast2600_i2c_set_speed, }; > > + > > +static const struct udevice_id ast2600_i2c_ids[] = { > > + { .compatible = "aspeed,ast2600-i2c" }, > > + {}, > > +}; > > + > > +U_BOOT_DRIVER(ast2600_i2c) = { > > + .name = "ast2600_i2c", > > + .id = UCLASS_I2C, > > + .of_match = ast2600_i2c_ids, > > + .probe = ast2600_i2c_probe, > > + .of_to_plat = ast2600_i2c_of_to_plat, > > + .priv_auto = sizeof(struct ast2600_i2c_priv), > > + .ops = &ast2600_i2c_ops, > > +}; > > + > > +#define AST2600_I2CG_ISR 0x00 > > +#define AST2600_I2CG_SLAVE_ISR 0x04 > > +#define AST2600_I2CG_OWNER 0x08 > > +#define AST2600_I2CG_CTRL 0x0C > > +#define AST2600_I2CG_CLK_DIV_CTRL 0x10 > > + > > +#define AST2600_I2CG_SLAVE_PKT_NAK BIT(4) > > +#define AST2600_I2CG_M_S_SEPARATE_INTR BIT(3) > > +#define AST2600_I2CG_CTRL_NEW_REG BIT(2) > > +#define AST2600_I2CG_CTRL_NEW_CLK_DIV BIT(1) > > + > > +#define AST2600_GLOBAL_INIT > \ > > + (AST2600_I2CG_SLAVE_PKT_NAK | \ > > + AST2600_I2CG_CTRL_NEW_REG | > \ > > + AST2600_I2CG_CTRL_NEW_CLK_DIV) #define > > +I2CCG_DIV_CTRL 0xC6411208 > > Please use lower-case hex consistently > > Hmm these should really go in the header file too? Will update and add "ast2600-i2c.h" > > > + > > +struct ast2600_i2c_global_priv { > > + void __iomem *regs; > > + struct reset_ctl reset; > > +}; > > + > > +/* > > + * APB clk : 100Mhz > > + * div : scl : baseclk [APB/((div/2) + 1)] : tBuf [1/bclk * 16] > > + * I2CG10[31:24] base clk4 for i2c auto recovery timeout counter > > +(0xC6) > > + * I2CG10[23:16] base clk3 for Standard-mode (100Khz) min tBuf 4.7us > > + * 0x3c : 100.8Khz : 3.225Mhz : 4.96us > > + * 0x3d : 99.2Khz : 3.174Mhz : 5.04us > > + * 0x3e : 97.65Khz : 3.125Mhz : 5.12us > > + * 0x40 : 97.75Khz : 3.03Mhz : 5.28us > > + * 0x41 : 99.5Khz : 2.98Mhz : 5.36us (default) > > + * I2CG10[15:8] base clk2 for Fast-mode (400Khz) min tBuf 1.3us > > + * 0x12 : 400Khz : 10Mhz : 1.6us > > + * I2CG10[7:0] base clk1 for Fast-mode Plus (1Mhz) min tBuf 0.5us > > + * 0x08 : 1Mhz : 20Mhz : 0.8us > > + */ > > + > > +static int aspeed_i2c_global_probe(struct udevice *dev) { > > + struct ast2600_i2c_global_priv *i2c_global = dev_get_priv(dev); > > + int ret = 0; > > + > > + i2c_global->regs = dev_read_addr_ptr(dev); > > + if (!(i2c_global->regs)) > > again too many (). Please fix globally Will update if (!i2c_global->regs) > > > + return -EINVAL; > > + > > + debug("%s(dev=%p)\n", __func__, dev); > > + > > + ret = reset_get_by_index(dev, 0, &i2c_global->reset); > > + if (ret) { > > + printf("%s(): Failed to get reset signal\n", > > + __func__); > > log_err() if you want an error. Try to avoid using __func__ - use > log...() instead > Will update to log_err > > + return ret; > > + } > > + > > + reset_deassert(&i2c_global->reset); > > Can this fail? Will add fail check > > > + > > + writel(AST2600_GLOBAL_INIT, i2c_global->regs + > > + AST2600_I2CG_CTRL); > > + writel(I2CCG_DIV_CTRL, i2c_global->regs + > > + AST2600_I2CG_CLK_DIV_CTRL); > > + > > + return 0; > > +} > > + > > +static const struct udevice_id aspeed_i2c_global_ids[] = { > > + { .compatible = "aspeed,ast2600-i2c-global", }, > > + { } > > +}; > > + > > +U_BOOT_DRIVER(aspeed_i2c_global) = { > > + .name = "aspeed_i2c_global", > > + .id = UCLASS_MISC, > > + .of_match = aspeed_i2c_global_ids, > > + .probe = aspeed_i2c_global_probe, > > + .priv_auto = sizeof(struct ast2600_i2c_global_priv), }; > > + > > -- > > 2.34.1 > > > > Regards, > Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-01-15 1:21 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-01-11 6:47 [PATCHv2 0/1] Add ASPEED AST2600 I2C new controller driver ryan_chen 2023-01-11 6:47 ` [PATCH 1/1] i2c:aspeed:support ast2600 i2c new register mode driver ryan_chen 2023-01-11 21:08 ` Simon Glass 2023-01-15 1:21 ` Ryan Chen -- strict thread matches above, loose matches on Subject: below -- 2023-01-09 8:29 [PATCH 0/1] Add ASPEED AST2600 I2C new controller driver ryan_chen 2023-01-09 8:29 ` [PATCH 1/1] i2c:aspeed:support ast2600 i2c new register mode driver ryan_chen 2023-01-09 19:47 ` Simon Glass 2023-01-10 2:41 ` Ryan Chen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox