Hello Jim, On 23.05.22 10:16, Jim Liu wrote: > Hi Heiko > > Thanks for your reply. > I will follow your suggestion to modify it for version 2. > and some explain and question as below: > > 1. while function will do the i2c recovery feature . > the function will break when toggle 27 times or SDA is high. >> + while (1) { >> + /* toggle SCL line */ >> + writeb(SMBCST_TGSCL, ®->cst); >> + >> + udelay(20); >> + val = readb(®->ctl3); >> + if (val & SMBCTL3_SDA_LVL) >> + break; >> + if (iter-- == 0) >> + break; >> + }
Ah, yes, seems I was blind... sorry. > 2. npcm845 is our new bmc and we are on-going to push it to linux. > This link is our push status. > https://patchwork.ozlabs.org/project/linux-i2c/patch/20220517101142.28421-10-warp...@gmail.com/ Ah, thanks! > may i keep this npcm845 compatible or remove it? If it gets accepted in linux, I am fine with it... ah I see: https://lore.kernel.org/lkml/20220517101142.28421-2-warp...@gmail.com/ you add there the new compatible entry and have already a Reviewed-by from Rob, so I think this is fine to accept it in u-boot too. bye, Heiko > On Thu, May 19, 2022 at 12:48 PM Heiko Schocher <h...@denx.de> wrote: >> >> Hello Jim, >> >> On 17.05.22 10:25, Jim Liu wrote: >>> Add Nuvoton BMC NPCM750 i2c driver >>> >>> Signed-off-by: Jim Liu <jjl...@nuvoton.com> >>> --- >>> drivers/i2c/Kconfig | 5 + >>> drivers/i2c/Makefile | 1 + >>> drivers/i2c/npcm-i2c.c | 632 +++++++++++++++++++++++++++++++++++++++++ >>> 3 files changed, 638 insertions(+) >>> create mode 100644 drivers/i2c/npcm-i2c.c >>> >>> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig >>> index d25c5736ef..7e113b289e 100644 >>> --- a/drivers/i2c/Kconfig >>> +++ b/drivers/i2c/Kconfig >>> @@ -447,6 +447,11 @@ config SYS_I2C_NEXELL >>> have several I2C ports and all are provided, controlled by the >>> device tree. >>> >>> +config SYS_I2C_NPCM >>> + bool "Nuvoton NPCM I2C driver" >>> + help >>> + Support for Nuvoton I2C controller driver. >>> + >>> config SYS_I2C_OCORES >>> bool "ocores I2C driver" >>> depends on DM_I2C >>> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile >>> index 9d41f379bb..7e046f809a 100644 >>> --- a/drivers/i2c/Makefile >>> +++ b/drivers/i2c/Makefile >>> @@ -33,6 +33,7 @@ obj-$(CONFIG_SYS_I2C_MV) += mv_i2c.o >>> obj-$(CONFIG_SYS_I2C_MVTWSI) += mvtwsi.o >>> obj-$(CONFIG_SYS_I2C_MXC) += mxc_i2c.o >>> obj-$(CONFIG_SYS_I2C_NEXELL) += nx_i2c.o >>> +obj-$(CONFIG_SYS_I2C_NPCM) += npcm_i2c.o >>> obj-$(CONFIG_SYS_I2C_OCORES) += ocores_i2c.o >>> obj-$(CONFIG_SYS_I2C_OCTEON) += octeon_i2c.o >>> obj-$(CONFIG_SYS_I2C_OMAP24XX) += omap24xx_i2c.o >>> diff --git a/drivers/i2c/npcm-i2c.c b/drivers/i2c/npcm-i2c.c >>> new file mode 100644 >>> index 0000000000..5a066a164b >>> --- /dev/null >>> +++ b/drivers/i2c/npcm-i2c.c >>> @@ -0,0 +1,632 @@ >>> +// SPDX-License-Identifier: GPL-2.0+ >>> +/* >>> + * Copyright (c) 2021 Nuvoton Technology Corp. >>> + */ >>> + >>> +#include <clk.h> >>> +#include <dm.h> >>> +#include <i2c.h> >>> +#include <asm/io.h> >>> +#include <linux/iopoll.h> >>> +#include <asm/arch/gcr.h> >>> + >>> +#define I2C_FREQ_100K 100000 >>> +#define NPCM_I2C_TIMEOUT_MS 10 >>> +#define NPCM7XX_I2CSEGCTL_INIT_VAL 0x0333F000 >>> +#define NPCM8XX_I2CSEGCTL_INIT_VAL 0x9333F000 >>> + >>> +/* SCLFRQ min/max field values */ >>> +#define SCLFRQ_MIN 10 >>> +#define SCLFRQ_MAX 511 >>> + >>> +/* SMBCTL1 */ >>> +#define SMBCTL1_START BIT(0) >>> +#define SMBCTL1_STOP BIT(1) >>> +#define SMBCTL1_INTEN BIT(2) >>> +#define SMBCTL1_ACK BIT(4) >>> +#define SMBCTL1_STASTRE BIT(7) >>> + >>> +/* SMBCTL2 */ >>> +#define SMBCTL2_ENABLE BIT(0) >>> + >>> +/* SMBCTL3 */ >>> +#define SMBCTL3_SCL_LVL BIT(7) >>> +#define SMBCTL3_SDA_LVL BIT(6) >>> + >>> +/* SMBCST */ >>> +#define SMBCST_BB BIT(1) >>> +#define SMBCST_TGSCL BIT(5) >>> + >>> +/* SMBST */ >>> +#define SMBST_XMIT BIT(0) >>> +#define SMBST_MASTER BIT(1) >>> +#define SMBST_STASTR BIT(3) >>> +#define SMBST_NEGACK BIT(4) >>> +#define SMBST_BER BIT(5) >>> +#define SMBST_SDAST BIT(6) >>> + >>> +/* SMBCST3 in bank0 */ >>> +#define SMBCST3_EO_BUSY BIT(7) >>> + >>> +/* SMBFIF_CTS in bank1 */ >>> +#define SMBFIF_CTS_CLR_FIFO BIT(6) >>> + >>> +#define SMBFIF_CTL_FIFO_EN BIT(4) >>> +#define SMBCTL3_BNK_SEL BIT(5) >>> + >>> +enum { >>> + I2C_ERR_NACK = 1, >>> + I2C_ERR_BER, >>> + I2C_ERR_TIMEOUT, >>> +}; >>> + >>> +struct smb_bank0_regs { >>> + u8 addr3; >>> + u8 addr7; >>> + u8 addr4; >>> + u8 addr8; >>> + u16 addr5; >>> + u16 addr6; >>> + u8 cst2; >>> + u8 cst3; >>> + u8 ctl4; >>> + u8 ctl5; >>> + u8 scllt; >>> + u8 fif_ctl; >>> + u8 sclht; >>> +}; >>> + >>> +struct smb_bank1_regs { >>> + u8 fif_cts; >>> + u8 fair_per; >>> + u16 txf_ctl; >>> + u32 t_out; >>> + u8 cst2; >>> + u8 cst3; >>> + u16 txf_sts; >>> + u16 rxf_sts; >>> + u8 rxf_ctl; >>> +}; >>> + >>> +struct npcm_i2c_regs { >>> + u16 sda; >>> + u16 st; >>> + u16 cst; >>> + u16 ctl1; >>> + u16 addr; >>> + u16 ctl2; >>> + u16 addr2; >>> + u16 ctl3; >>> + union { >>> + struct smb_bank0_regs bank0; >>> + struct smb_bank1_regs bank1; >>> + }; >>> + >>> +}; >>> + >>> +struct npcm_i2c_bus { >>> + struct npcm_i2c_regs *reg; >>> + int num; >>> + u32 apb_clk; >>> + u32 freq; >>> + bool started; >>> +}; >>> + >>> +static void npcm_dump_regs(struct npcm_i2c_bus *bus) >>> +{ >>> + struct npcm_i2c_regs *reg = bus->reg; >>> + >>> + printf("\n"); >>> + printf("SMBST=0x%x\n", readb(®->st)); >>> + printf("SMBCST=0x%x\n", readb(®->cst)); >>> + printf("SMBCTL1=0x%x\n", readb(®->ctl1)); >>> + printf("\n"); >>> +} >>> + >>> +static int npcm_i2c_check_sda(struct npcm_i2c_bus *bus) >>> +{ >>> + struct npcm_i2c_regs *reg = bus->reg; >>> + ulong start_time; >>> + int err = I2C_ERR_TIMEOUT; >>> + u8 val; >>> + >>> + start_time = get_timer(0); >>> + /* wait SDAST to be 1 */ >>> + while (get_timer(start_time) < NPCM_I2C_TIMEOUT_MS) { >>> + val = readb(®->st); >>> + if (val & SMBST_NEGACK) { >>> + err = I2C_ERR_NACK; >>> + break; >>> + } >>> + if (val & SMBST_BER) { >>> + err = I2C_ERR_BER; >>> + break; >>> + } >>> + if (val & SMBST_SDAST) { >>> + err = 0; >>> + break; >>> + } >>> + } >>> + >>> + if (err) >>> + printf("%s: err %d\n", __func__, err); >>> + >>> + return err; >>> +} >>> + >>> +static int npcm_i2c_send_start(struct npcm_i2c_bus *bus) >>> +{ >>> + struct npcm_i2c_regs *reg = bus->reg; >>> + ulong start_time; >>> + int err = I2C_ERR_TIMEOUT; >>> + >>> + /* Generate START condition */ >>> + writeb(readb(®->ctl1) | SMBCTL1_START, ®->ctl1); >>> + >>> + start_time = get_timer(0); >>> + while (get_timer(start_time) < NPCM_I2C_TIMEOUT_MS) { >>> + if (readb(®->st) & SMBST_BER) >>> + return I2C_ERR_BER; >>> + if (readb(®->st) & SMBST_MASTER) { >>> + err = 0; >>> + break; >>> + } >>> + } >>> + bus->started = true; >>> + >>> + return err; >>> +} >>> + >>> +static int npcm_i2c_send_stop(struct npcm_i2c_bus *bus, bool wait) >>> +{ >>> + struct npcm_i2c_regs *reg = bus->reg; >>> + ulong start_time; >>> + int err = I2C_ERR_TIMEOUT; >>> + >>> + writeb(readb(®->ctl1) | SMBCTL1_STOP, ®->ctl1); >>> + >>> + /* Clear NEGACK, STASTR and BER bits */ >>> + writeb(SMBST_STASTR | SMBST_NEGACK | SMBST_BER, ®->st); >>> + >>> + bus->started = false; >>> + >>> + if (!wait) >>> + return 0; >>> + >>> + start_time = get_timer(0); >>> + while (get_timer(start_time) < NPCM_I2C_TIMEOUT_MS) { >>> + if ((readb(®->ctl1) & SMBCTL1_STOP) == 0) { >>> + err = 0; >>> + break; >>> + } >>> + } >>> + if (err) { >>> + printf("%s: err %d\n", __func__, err); >>> + npcm_dump_regs(bus); >>> + } >>> + >>> + return err; >>> +} >>> + >>> +static void npcm_i2c_reset(struct npcm_i2c_bus *bus) >>> +{ >>> + struct npcm_i2c_regs *reg = bus->reg; >>> + >>> + printf("%s: module %d\n", __func__, bus->num); >> >> May better only debug() ? >> >>> + /* disable & enable SMB moudle */ >>> + writeb(readb(®->ctl2) & ~SMBCTL2_ENABLE, ®->ctl2); >>> + writeb(readb(®->ctl2) | SMBCTL2_ENABLE, ®->ctl2); >> >> You may want to use here clr/setbits_8 and friends? >> >> Please use them in this driver, thanks! >> >>> + >>> + /* clear BB and status */ >>> + writeb(SMBCST_BB, ®->cst); >>> + writeb(0xff, ®->st); >>> + >>> + /* select bank 1 */ >>> + writeb(readb(®->ctl3) | SMBCTL3_BNK_SEL, ®->ctl3); >>> + /* Clear all fifo bits */ >>> + writeb(SMBFIF_CTS_CLR_FIFO, ®->bank1.fif_cts); >>> + >>> + /* select bank 0 */ >>> + writeb(readb(®->ctl3) & ~SMBCTL3_BNK_SEL, ®->ctl3); >>> + /* clear EOB bit */ >>> + writeb(SMBCST3_EO_BUSY, ®->bank0.cst3); >>> + /* single byte mode */ >>> + writeb(readb(®->bank0.fif_ctl) & ~SMBFIF_CTL_FIFO_EN, >>> ®->bank0.fif_ctl); >>> + >>> + /* set POLL mode */ >>> + writeb(0, ®->ctl1); >>> +} >>> + >>> +static void npcm_i2c_recovery(struct npcm_i2c_bus *bus, u32 addr) >>> +{ >>> + u8 val; >>> + int iter = 27; >>> + struct npcm_i2c_regs *reg = bus->reg; >>> + int err; >>> + >>> + val = readb(®->ctl3); >>> + /* Skip recovery, bus not stucked */ >>> + if ((val & SMBCTL3_SCL_LVL) && (val & SMBCTL3_SDA_LVL)) >>> + return; >>> + >>> + printf("Performing I2C bus %d recovery...\n", bus->num); >>> + /* SCL/SDA are not releaed, perform recovery */ >>> + while (1) { >>> + /* toggle SCL line */ >>> + writeb(SMBCST_TGSCL, ®->cst); >>> + >>> + udelay(20); >>> + val = readb(®->ctl3); >>> + if (val & SMBCTL3_SDA_LVL) >>> + break; >>> + if (iter-- == 0) >>> + break; >>> + } >> >> Hmm... endless while(1) ? >> >>> + >>> + if (val & SMBCTL3_SDA_LVL) { >>> + writeb((u8)((addr << 1) & 0xff), ®->sda); >>> + err = npcm_i2c_send_start(bus); >>> + if (!err) { >>> + udelay(20); >>> + npcm_i2c_send_stop(bus, false); >>> + udelay(200); >>> + printf("I2C bus %d recovery completed\n", >>> + bus->num); >>> + } else { >>> + printf("%s: send START err %d\n", __func__, err); >>> + } >>> + } else { >>> + printf("Fail to recover I2C bus %d\n", bus->num); >>> + } >>> + npcm_i2c_reset(bus); >>> +} >>> + >>> +static int npcm_i2c_send_address(struct npcm_i2c_bus *bus, u8 addr, >>> + bool stall) >>> +{ >>> + struct npcm_i2c_regs *reg = bus->reg; >>> + ulong start_time; >>> + u8 val; >>> + >>> + /* Stall After Start Enable */ >>> + if (stall) >>> + writeb(readb(®->ctl1) | SMBCTL1_STASTRE, ®->ctl1); >>> + >>> + writeb(addr, ®->sda); >>> + if (stall) { >>> + start_time = get_timer(0); >>> + while (get_timer(start_time) < NPCM_I2C_TIMEOUT_MS) { >>> + if (readb(®->st) & SMBST_STASTR) >>> + break; >>> + >>> + if (readb(®->st) & SMBST_BER) { >>> + writeb(readb(®->ctl1) & ~SMBCTL1_STASTRE, >>> + ®->ctl1); >>> + return I2C_ERR_BER; >>> + } >>> + } >>> + } >>> + >>> + /* check ACK */ >>> + val = readb(®->st); >>> + if (val & SMBST_NEGACK) { >>> + debug("NACK on addr 0x%x\n", addr >> 1); >>> + /* After a Stop condition, writing 1 to NEGACK clears it */ >>> + return I2C_ERR_NACK; >>> + } >>> + if (val & SMBST_BER) >>> + return I2C_ERR_BER; >>> + >>> + return 0; >>> +} >>> + >>> +static int npcm_i2c_read_bytes(struct npcm_i2c_bus *bus, u8 *data, int len) >>> +{ >>> + struct npcm_i2c_regs *reg = bus->reg; >>> + u8 val; >>> + int i; >>> + int err = 0; >>> + >>> + if (len == 1) { >>> + /* bus should be stalled before receiving last byte */ >>> + writeb(readb(®->ctl1) | SMBCTL1_ACK, ®->ctl1); >>> + >>> + /* clear STASTRE if it is set */ >>> + if (readb(®->ctl1) & SMBCTL1_STASTRE) { >>> + writeb(SMBST_STASTR, ®->st); >>> + writeb(readb(®->ctl1) & ~SMBCTL1_STASTRE, >>> ®->ctl1); >>> + } >>> + npcm_i2c_check_sda(bus); >>> + npcm_i2c_send_stop(bus, false); >>> + *data = readb(®->sda); >>> + /* this must be done to generate STOP condition */ >>> + writeb(SMBST_NEGACK, ®->st); >>> + } else { >>> + for (i = 0; i < len; i++) { >>> + /* >>> + * When NEGACK bit is set to 1 after the transmission >>> of a byte, >>> + * SDAST is not set to 1. >>> + */ >>> + if (i != (len - 1)) { >>> + err = npcm_i2c_check_sda(bus); >>> + } else { >>> + err = readb_poll_timeout(®->ctl1, val, >>> + !(val & >>> SMBCTL1_ACK), 100000); >>> + if (err) { >>> + printf("wait nack timeout\n"); >>> + err = I2C_ERR_TIMEOUT; >>> + npcm_dump_regs(bus); >>> + } >>> + } >>> + if (err && err != I2C_ERR_TIMEOUT) >>> + break; >>> + if (i == (len - 2)) { >>> + /* set NACK before last byte */ >>> + writeb(readb(®->ctl1) | SMBCTL1_ACK, >>> ®->ctl1); >>> + } >>> + if (i == (len - 1)) { >>> + /* last byte, send STOP condition */ >>> + npcm_i2c_send_stop(bus, false); >>> + *data = readb(®->sda); >>> + writeb(SMBST_NEGACK, ®->st); >>> + break; >>> + } >>> + *data = readb(®->sda); >>> + data++; >>> + } >>> + } >>> + >>> + return err; >>> +} >>> + >>> +static int npcm_i2c_send_bytes(struct npcm_i2c_bus *bus, u8 *data, int len) >>> +{ >>> + struct npcm_i2c_regs *reg = bus->reg; >>> + u8 val; >>> + int i; >>> + int err = 0; >>> + >>> + val = readb(®->st); >>> + if (val & SMBST_NEGACK) >>> + return I2C_ERR_NACK; >>> + else if (val & SMBST_BER) >>> + return I2C_ERR_BER; >>> + >>> + /* clear STASTRE if it is set */ >>> + if (readb(®->ctl1) & SMBCTL1_STASTRE) >>> + writeb(readb(®->ctl1) & ~SMBCTL1_STASTRE, ®->ctl1); >>> + >>> + for (i = 0; i < len; i++) { >>> + err = npcm_i2c_check_sda(bus); >>> + if (err) >>> + break; >>> + writeb(*data, ®->sda); >>> + data++; >>> + } >>> + npcm_i2c_check_sda(bus); >>> + >>> + return err; >>> +} >>> + >>> +static int npcm_i2c_read(struct npcm_i2c_bus *bus, u32 addr, u8 *data, >>> + u32 len) >>> +{ >>> + struct npcm_i2c_regs *reg = bus->reg; >>> + int err; >>> + bool stall; >>> + >>> + if (len <= 0) >>> + return -EINVAL; >>> + >>> + /* send START condition */ >>> + err = npcm_i2c_send_start(bus); >>> + if (err) { >>> + debug("%s: send START err %d\n", __func__, err); >>> + return err; >>> + } >>> + >>> + stall = (len == 1) ? true : false; >>> + /* send address byte */ >>> + err = npcm_i2c_send_address(bus, (u8)(addr << 1) | 0x1, stall); >>> + >>> + if (!err && len) >>> + npcm_i2c_read_bytes(bus, data, len); >>> + >>> + if (err == I2C_ERR_NACK) { >>> + /* clear NACK */ >>> + writeb(SMBST_NEGACK, ®->st); >>> + } >>> + >>> + if (err) >>> + debug("%s: err %d\n", __func__, err); >>> + >>> + return err; >>> +} >>> + >>> +static int npcm_i2c_write(struct npcm_i2c_bus *bus, u32 addr, u8 *data, >>> + u32 len) >>> +{ >>> + struct npcm_i2c_regs *reg = bus->reg; >>> + int err; >>> + bool stall; >>> + >>> + /* send START condition */ >>> + err = npcm_i2c_send_start(bus); >>> + if (err) { >>> + debug("%s: send START err %d\n", __func__, err); >>> + return err; >>> + } >>> + >>> + stall = (len == 0) ? true : false; >>> + /* send address byte */ >>> + err = npcm_i2c_send_address(bus, (u8)(addr << 1), stall); >>> + >>> + if (!err && len) >>> + err = npcm_i2c_send_bytes(bus, data, len); >>> + >>> + /* clear STASTRE if it is set */ >>> + if (stall) >>> + writeb(readb(®->ctl1) & ~SMBCTL1_STASTRE, ®->ctl1); >>> + >>> + if (err) >>> + debug("%s: err %d\n", __func__, err); >>> + >>> + return err; >>> +} >>> + >>> +static int npcm_i2c_xfer(struct udevice *dev, >>> + struct i2c_msg *msg, int nmsgs) >>> +{ >>> + struct npcm_i2c_bus *bus = dev_get_priv(dev); >>> + struct npcm_i2c_regs *reg = bus->reg; >>> + int ret = 0, err = 0; >>> + >>> + if (nmsgs < 1 || nmsgs > 2) { >>> + printf("%s: commands not support\n", __func__); >>> + return -EREMOTEIO; >>> + } >>> + /* clear ST register */ >>> + writeb(0xFF, ®->st); >>> + >>> + for ( ; nmsgs > 0; nmsgs--, msg++) { >> >> above you say no commands supported, so this for loop is useless ..or? >> >>> + if (msg->flags & I2C_M_RD) >>> + err = npcm_i2c_read(bus, msg->addr, msg->buf, >>> + msg->len); >>> + else >>> + err = npcm_i2c_write(bus, msg->addr, msg->buf, >>> + msg->len); >>> + if (err) { >>> + debug("i2c_xfer: error %d\n", err); >>> + ret = -EREMOTEIO; >>> + break; >>> + } >>> + } >>> + >>> + if (bus->started) >>> + npcm_i2c_send_stop(bus, true); >>> + >>> + if (err) >>> + npcm_i2c_recovery(bus, msg->addr); >>> + >>> + return ret; >>> +} >>> + >>> +static int npcm_i2c_init_clk(struct npcm_i2c_bus *bus, u32 bus_freq) >>> +{ >>> + struct npcm_i2c_regs *reg = bus->reg; >>> + u32 freq = bus->apb_clk; >>> + u32 sclfrq; >>> + u8 hldt, val; >>> + >>> + if (bus_freq > I2C_FREQ_100K) { >>> + printf("Support standard mode only\n"); >>> + return -EINVAL; >>> + } >>> + >>> + /* SCLFRQ = T(SCL)/4/T(CLK) = FREQ(CLK)/4/FREQ(SCL) */ >>> + sclfrq = freq / (bus_freq * 4); >>> + if (sclfrq < SCLFRQ_MIN || sclfrq > SCLFRQ_MAX) >>> + return -EINVAL; >>> + >>> + if (freq >= 40000000) >>> + hldt = 17; >>> + else if (freq >= 12500000) >>> + hldt = 15; >>> + else >>> + hldt = 7; >>> + >>> + val = readb(®->ctl2) & 0x1; >>> + val |= (sclfrq & 0x7F) << 1; >>> + writeb(val, ®->ctl2); >>> + >>> + /* clear 400K_MODE bit */ >>> + val = readb(®->ctl3) & 0xc; >>> + val |= (sclfrq >> 7) & 0x3; >>> + writeb(val, ®->ctl3); >>> + >>> + writeb(hldt, ®->bank0.ctl4); >>> + >>> + return 0; >>> +} >>> + >>> +static int npcm_i2c_set_bus_speed(struct udevice *dev, >>> + unsigned int speed) >>> +{ >>> + struct npcm_i2c_bus *bus = dev_get_priv(dev); >>> + >>> + return npcm_i2c_init_clk(bus, speed); >>> +} >>> + >>> +static int npcm_i2c_probe(struct udevice *dev) >>> +{ >>> + struct npcm_i2c_bus *bus = dev_get_priv(dev); >>> + struct npcm_gcr *gcr = (struct npcm_gcr *)NPCM_GCR_BA; >>> + struct npcm_i2c_regs *reg; >>> + u32 i2csegctl_val = dev_get_driver_data(dev); >>> + struct clk clk; >>> + int ret; >>> + >>> + ret = clk_get_by_index(dev, 0, &clk); >>> + if (ret) { >>> + printf("%s: ret %d\n", __func__, ret); >>> + return ret; >>> + } >>> + bus->apb_clk = clk_get_rate(&clk); >>> + if (bus->apb_clk <= 0) { >>> + printf("%s: fail to get rate\n", __func__); >>> + return -EINVAL; >>> + } >>> + clk_free(&clk); >>> + >>> + bus->num = dev->seq_; >>> + bus->reg = dev_read_addr_ptr(dev); >>> + bus->freq = dev_read_u32_default(dev, "clock-frequency", 100000); >>> + bus->started = false; >>> + reg = bus->reg; >>> + >>> + if (npcm_i2c_init_clk(bus, bus->freq)) { >>> + printf("%s: init_clk failed\n", __func__); >>> + return -EINVAL; >>> + } >>> + >>> + /* set initial i2csegctl value */ >>> + writel(i2csegctl_val, &gcr->i2csegctl); >>> + >>> + /* enable SMB module */ >>> + writeb(readb(®->ctl2) | SMBCTL2_ENABLE, ®->ctl2); >>> + >>> + /* select register bank 0 */ >>> + writeb(readb(®->ctl3) & ~SMBCTL3_BNK_SEL, ®->ctl3); >>> + >>> + /* single byte mode */ >>> + writeb(readb(®->bank0.fif_ctl) & ~SMBFIF_CTL_FIFO_EN, >>> ®->bank0.fif_ctl); >>> + >>> + /* set POLL mode */ >>> + writeb(0, ®->ctl1); >>> + >>> + printf("I2C bus %d ready. speed=%d, base=0x%x, apb=%u\n", >>> + bus->num, bus->freq, (u32)(uintptr_t)bus->reg, bus->apb_clk); >> >> Please use debug here. >> >>> + >>> + return 0; >>> +} >>> + >>> +static const struct dm_i2c_ops nuvoton_i2c_ops = { >>> + .xfer = npcm_i2c_xfer, >>> + .set_bus_speed = npcm_i2c_set_bus_speed, >>> +}; >> >> As you have a deblock function, may you want to set >> .deblock here too? >> >>> + >>> +static const struct udevice_id nuvoton_i2c_of_match[] = { >>> + { .compatible = "nuvoton,npcm845-i2c", .data = >>> NPCM8XX_I2CSEGCTL_INIT_VAL}, >> >> I do not find this string in linux ... do you plan to push this >> to linux? >> >>> + { .compatible = "nuvoton,npcm750-i2c", .data = >>> NPCM7XX_I2CSEGCTL_INIT_VAL}, >>> + {} >>> +}; >> >> and please document the bindings in u-boot:/doc/device-tree-bindings/i2c >> >>> + >>> +U_BOOT_DRIVER(npcm_i2c_bus) = { >>> + .name = "npcm-i2c", >>> + .id = UCLASS_I2C, >>> + .of_match = nuvoton_i2c_of_match, >>> + .probe = npcm_i2c_probe, >>> + .priv_auto = sizeof(struct npcm_i2c_bus), >>> + .ops = &nuvoton_i2c_ops, >>> +}; >>> + >>> >> >> Thanks! >> >> bye, >> Heiko >> -- >> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk >> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany >> Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: h...@denx.de -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: h...@denx.de