[PATCH v2] lib: utils/i2c: update i2c interface
Xiang W
wxjstz at 126.com
Thu Dec 30 22:27:01 PST 2021
在 2021-12-30星期四的 22:25 +0800，Xiang W写道：
> 1. Remove i2c register related operations
> 2. Simplify the low-level interface
> 3. Add 10bit address support
> 4. Add combination operation
> 5. Update fdt_i2c_sifive.c
> 6. Update sifive_fu740.c
>
> Signed-off-by: Xiang W <wxjstz at 126.com>
I have performed 10 reboot on my unmatched, and it succeeded five times
and failed five times. Are we missing some timing issues?
Regards,
Xiang W
> ---
> Changes since v1:
>
> * Simplify fdt_i2c_sifive.c
> * Add struct i2c_msg, modify the interface
>
> include/sbi_utils/i2c/i2c.h | 65 ++++++---------
> lib/utils/i2c/fdt_i2c_sifive.c | 136 +++++++------------------------
> -
> lib/utils/i2c/i2c.c | 70 +++++++++++-----
> platform/generic/sifive_fu740.c | 52 ++++++++++--
> 4 files changed, 148 insertions(+), 175 deletions(-)
>
> diff --git a/include/sbi_utils/i2c/i2c.h b/include/sbi_utils/i2c/i2c.h
> index 5a70364..307fa55 100644
> --- a/include/sbi_utils/i2c/i2c.h
> +++ b/include/sbi_utils/i2c/i2c.h
> @@ -13,6 +13,21 @@
> #include <sbi/sbi_types.h>
> #include <sbi/sbi_list.h>
>
> +struct i2c_msg {
> + uint16_t addr;
> + uint16_t flags;
> +#define I2C_M_RD 0x0001
> +#define I2C_M_TEN 0x0010
> +#define I2C_M_RECV_LEN 0x0400 /* Fixup: unclear meaning,
> currently unused */
> +#define I2C_M_NO_RD_ACK 0x0800 /* Fixup: unclear
> meaning, currently unused */
> +#define I2C_M_IGNORE_NAK 0x1000 /* Fixup: unclear meaning,
> currently unused */
> +#define I2C_M_REV_DIR_ADDR 0x2000 /* Fixup: unclear meaning,
> currently unused */
> +#define I2C_M_NOSTART 0x4000
> +#define I2C_M_STOP 0x8000
> + uint16_t len;
> + uint8_t *buf;
> +};
> +
> /** Representation of a I2C adapter */
> struct i2c_adapter {
> /** Pointer to I2C driver owning this I2C adapter */
> @@ -21,21 +36,11 @@ struct i2c_adapter {
> /** Unique ID of the I2C adapter assigned by the driver */
> int id;
>
> - /**
> - * Send buffer to given address, register
> - *
> - * @return 0 on success and negative error code on failure
> - */
> - int (*write)(struct i2c_adapter *ia, uint8_t addr, uint8_t
> reg,
> - uint8_t *buffer, int len);
> + int (*write_byte)(struct i2c_adapter *ia, uint8_t byte,
> + bool send_start, bool send_stop);
>
> - /**
> - * Read buffer from given address, register
> - *
> - * @return 0 on success and negative error code on failure
> - */
> - int (*read)(struct i2c_adapter *ia, uint8_t addr, uint8_t reg,
> - uint8_t *buffer, int len);
> + int (*read_byte)(struct i2c_adapter *ia, uint8_t *byte,
> + bool nack, bool send_stop);
>
> /** List */
> struct sbi_dlist node;
> @@ -55,31 +60,11 @@ int i2c_adapter_add(struct i2c_adapter *ia);
> /** Un-register I2C adapter */
> void i2c_adapter_remove(struct i2c_adapter *ia);
>
> -/** Send to device on I2C adapter bus */
> -int i2c_adapter_write(struct i2c_adapter *ia, uint8_t addr, uint8_t
> reg,
> - uint8_t *buffer, int len);
> -
> -/** Read from device on I2C adapter bus */
> -int i2c_adapter_read(struct i2c_adapter *ia, uint8_t addr, uint8_t
> reg,
> - uint8_t *buffer, int len);
> -
> -static inline int i2c_adapter_reg_write(struct i2c_adapter *ia,
> uint8_t addr,
> - uint8_t reg, uint8_t val)
> -{
> - return i2c_adapter_write(ia, addr, reg, &val, 1);
> -}
> -
> -static inline int i2c_adapter_reg_read(struct i2c_adapter *ia,
> uint8_t addr,
> - uint8_t reg, uint8_t *val)
> -{
> - uint8_t buf;
> - int ret = i2c_adapter_read(ia, addr, reg, &buf, 1);
> -
> - if (ret)
> - return ret;
> -
> - *val = buf;
> - return 0;
> -}
> +/**
> + * once message transmission
> + * It can be a complete read or write
> + * can also be used in combination to form a combined transmission
> + */
> +int i2c_adapter_trans(struct i2c_adapter *ia, struct i2c_msg *msg);
>
> #endif
> diff --git a/lib/utils/i2c/fdt_i2c_sifive.c
> b/lib/utils/i2c/fdt_i2c_sifive.c
> index 871241a..47d85f3 100644
> --- a/lib/utils/i2c/fdt_i2c_sifive.c
> +++ b/lib/utils/i2c/fdt_i2c_sifive.c
> @@ -101,139 +101,59 @@ static int sifive_i2c_adapter_poll(struct
> sifive_i2c_adapter *adap,
> #define sifive_i2c_adapter_poll_busy(adap) \
> sifive_i2c_adapter_poll(adap, SIFIVE_I2C_STATUS_BUSY)
>
> -static int sifive_i2c_adapter_start(struct sifive_i2c_adapter *adap,
> - uint8_t addr, uint8_t bit)
> -{
> - uint8_t val = (addr << 1) | bit;
> -
> - sifive_i2c_setreg(adap, SIFIVE_I2C_TXR, val);
> - val = SIFIVE_I2C_CMD_STA | SIFIVE_I2C_CMD_WR |
> SIFIVE_I2C_CMD_IACK;
> - sifive_i2c_setreg(adap, SIFIVE_I2C_CR, val);
> -
> - return sifive_i2c_adapter_poll_tip(adap);
> -}
> -
> -static int sifive_i2c_adapter_write(struct i2c_adapter *ia, uint8_t
> addr,
> - uint8_t reg, uint8_t *buffer, int
> len)
> +static int sifive_i2c_adapter_write_byte(struct i2c_adapter *ia,
> + uint8_t byte, bool send_start, bool
> send_stop)
> {
> struct sifive_i2c_adapter *adap =
> container_of(ia, struct sifive_i2c_adapter, adapter);
> - int rc = sifive_i2c_adapter_start(adap, addr,
> SIFIVE_I2C_WRITE_BIT);
> -
> - if (rc)
> - return rc;
> -
> - rc = sifive_i2c_adapter_rxack(adap);
> - if (rc)
> - return rc;
> -
> - /* set register address */
> - sifive_i2c_setreg(adap, SIFIVE_I2C_TXR, reg);
> - sifive_i2c_setreg(adap, SIFIVE_I2C_CR, SIFIVE_I2C_CMD_WR |
> - SIFIVE_I2C_CMD_IACK);
> + int rc;
> + uint8_t val;
> + sifive_i2c_setreg(adap, SIFIVE_I2C_TXR, byte);
> + val = SIFIVE_I2C_CMD_WR | SIFIVE_I2C_CMD_IACK;
> + if (send_start)
> + val |= SIFIVE_I2C_CMD_STA;
> + sifive_i2c_setreg(adap, SIFIVE_I2C_CR, val);
> rc = sifive_i2c_adapter_poll_tip(adap);
> if (rc)
> return rc;
> -
> rc = sifive_i2c_adapter_rxack(adap);
> if (rc)
> return rc;
>
> - /* set value */
> - while (len) {
> - sifive_i2c_setreg(adap, SIFIVE_I2C_TXR, *buffer);
> - sifive_i2c_setreg(adap, SIFIVE_I2C_CR,
> SIFIVE_I2C_CMD_WR |
> -
> SIFIVE_I2C_CMD_IACK);
> -
> - rc = sifive_i2c_adapter_poll_tip(adap);
> - if (rc)
> - return rc;
> -
> - rc = sifive_i2c_adapter_rxack(adap);
> + if (send_stop) {
> + sifive_i2c_setreg(adap, SIFIVE_I2C_CR,
> + SIFIVE_I2C_CMD_STO | SIFIVE_I2C_CMD_IACK);
> + rc = sifive_i2c_adapter_poll_busy(adap);
> if (rc)
> return rc;
> -
> - buffer++;
> - len--;
> + sifive_i2c_setreg(adap, SIFIVE_I2C_CR,
> SIFIVE_I2C_CMD_IACK);
> }
> -
> - sifive_i2c_setreg(adap, SIFIVE_I2C_CR, SIFIVE_I2C_CMD_STO |
> - SIFIVE_I2C_CMD_IACK);
> -
> - /* poll BUSY instead of ACK*/
> - rc = sifive_i2c_adapter_poll_busy(adap);
> - if (rc)
> - return rc;
> -
> - sifive_i2c_setreg(adap, SIFIVE_I2C_CR, SIFIVE_I2C_CMD_IACK);
> -
> return 0;
> }
>
> -static int sifive_i2c_adapter_read(struct i2c_adapter *ia, uint8_t
> addr,
> - uint8_t reg, uint8_t *buffer, int
> len)
> +static int sifive_i2c_adapter_read_byte(struct i2c_adapter *ia,
> + uint8_t *byte, bool nack, bool
> send_stop)
> {
> struct sifive_i2c_adapter *adap =
> container_of(ia, struct sifive_i2c_adapter, adapter);
> int rc;
> -
> - rc = sifive_i2c_adapter_start(adap, addr,
> SIFIVE_I2C_WRITE_BIT);
> - if (rc)
> - return rc;
> -
> - rc = sifive_i2c_adapter_rxack(adap);
> - if (rc)
> - return rc;
> -
> - sifive_i2c_setreg(adap, SIFIVE_I2C_TXR, reg);
> - sifive_i2c_setreg(adap, SIFIVE_I2C_CR, SIFIVE_I2C_CMD_WR |
> - SIFIVE_I2C_CMD_IACK);
> -
> + uint8_t val = SIFIVE_I2C_CMD_RD | SIFIVE_I2C_CMD_IACK;
> + if (nack)
> + val |= SIFIVE_I2C_CMD_ACK;
> + sifive_i2c_setreg(adap, SIFIVE_I2C_CR, val);
> rc = sifive_i2c_adapter_poll_tip(adap);
> if (rc)
> return rc;
> + *byte = sifive_i2c_getreg(adap, SIFIVE_I2C_RXR);
>
> - rc = sifive_i2c_adapter_rxack(adap);
> - if (rc)
> - return rc;
> -
> - /* setting addr with high 0 bit */
> - rc = sifive_i2c_adapter_start(adap, addr,
> SIFIVE_I2C_READ_BIT);
> - if (rc)
> - return rc;
> -
> - rc = sifive_i2c_adapter_rxack(adap);
> - if (rc)
> - return rc;
> -
> - while (len) {
> - if (len == 1)
> - sifive_i2c_setreg(adap, SIFIVE_I2C_CR,
> - SIFIVE_I2C_CMD_ACK |
> - SIFIVE_I2C_CMD_RD |
> - SIFIVE_I2C_CMD_IACK);
> - else
> - sifive_i2c_setreg(adap, SIFIVE_I2C_CR,
> - SIFIVE_I2C_CMD_RD |
> - SIFIVE_I2C_CMD_IACK);
> -
> - rc = sifive_i2c_adapter_poll_tip(adap);
> + if (send_stop) {
> + sifive_i2c_setreg(adap, SIFIVE_I2C_CR,
> + SIFIVE_I2C_CMD_STO | SIFIVE_I2C_CMD_IACK);
> + rc = sifive_i2c_adapter_poll_busy(adap);
> if (rc)
> return rc;
> -
> - *buffer = sifive_i2c_getreg(adap, SIFIVE_I2C_RXR);
> - buffer++;
> - len--;
> + sifive_i2c_setreg(adap, SIFIVE_I2C_CR,
> SIFIVE_I2C_CMD_IACK);
> }
> -
> - sifive_i2c_setreg(adap, SIFIVE_I2C_CR, SIFIVE_I2C_CMD_STO |
> - SIFIVE_I2C_CMD_IACK);
> - rc = sifive_i2c_adapter_poll_busy(adap);
> - if (rc)
> - return rc;
> -
> - sifive_i2c_setreg(adap, SIFIVE_I2C_CR, SIFIVE_I2C_CMD_IACK);
> -
> return 0;
> }
>
> @@ -256,8 +176,8 @@ static int sifive_i2c_init(void *fdt, int nodeoff,
> adapter->addr = addr;
> adapter->adapter.driver = &fdt_i2c_adapter_sifive;
> adapter->adapter.id = nodeoff;
> - adapter->adapter.write = sifive_i2c_adapter_write;
> - adapter->adapter.read = sifive_i2c_adapter_read;
> + adapter->adapter.write_byte = sifive_i2c_adapter_write_byte;
> + adapter->adapter.read_byte = sifive_i2c_adapter_read_byte;
> rc = i2c_adapter_add(&adapter->adapter);
> if (rc)
> return rc;
> diff --git a/lib/utils/i2c/i2c.c b/lib/utils/i2c/i2c.c
> index 6be4e24..841756e 100644
> --- a/lib/utils/i2c/i2c.c
> +++ b/lib/utils/i2c/i2c.c
> @@ -12,6 +12,7 @@
> */
>
> #include <sbi/sbi_error.h>
> +#include <sbi/sbi_string.h>
> #include <sbi_utils/i2c/i2c.h>
>
> static SBI_LIST_HEAD(i2c_adapters_list);
> @@ -51,25 +52,56 @@ void i2c_adapter_remove(struct i2c_adapter *ia)
> sbi_list_del(&(ia->node));
> }
>
> -int i2c_adapter_write(struct i2c_adapter *ia, uint8_t addr, uint8_t
> reg,
> - uint8_t *buffer, int len)
> +int i2c_adapter_trans(struct i2c_adapter *ia, struct i2c_msg *msg)
> {
> - if (!ia)
> - return SBI_EINVAL;
> - if (!ia->write)
> - return SBI_ENOSYS;
> -
> - return ia->write(ia, addr, reg, buffer, len);
> -}
> -
> -
> -int i2c_adapter_read(struct i2c_adapter *ia, uint8_t addr, uint8_t
> reg,
> - uint8_t *buffer, int len)
> -{
> - if (!ia)
> + int ret;
> + uint8_t rw = msg->flags & I2C_M_RD;
> + uint8_t *buf = msg->buf;
> + uint16_t len = msg->len;
> + bool ten = msg->flags & I2C_M_TEN;
> + bool send_start = !(msg->flags & I2C_M_NOSTART);
> + bool send_stop = msg->flags & I2C_M_STOP;
> +
> + if(len == 0)
> return SBI_EINVAL;
> - if (!ia->read)
> - return SBI_ENOSYS;
>
> - return ia->read(ia, addr, reg, buffer, len);
> -}
> + if (ten) {
> + uint8_t addr_h = ((msg->addr >> 8) & 3) | 0x7c;
> + uint8_t addr_l = msg->addr & 0xff;
> + if (send_start) {
> + ret = ia->write_byte(ia, (addr_h << 1) | 0,
> true, false);
> + if (ret)
> + return ret;
> + ret = ia->write_byte(ia, addr_l, false,
> false);
> + if (ret)
> + return ret;
> + if (rw) {
> + ret = ia->write_byte(ia, (addr_h << 1)
> | 1, true, false);
> + if (ret)
> + return ret;
> + }
> + } else {
> + ret = ia->write_byte(ia, (addr_h << 1) | rw,
> false, false);
> + if (ret)
> + return ret;
> + }
> + } else {
> + uint8_t addr = msg->addr & 0x7f;
> + ret = ia->write_byte(ia, (addr << 1) | rw, send_start,
> false);
> + if (ret)
> + return ret;
> + }
> + while (len > 0) {
> + len--;
> + bool S = (len == 0) && !send_stop; /* repeated START
> */
> + bool P = (len == 0) && send_stop; /* STOP */
> + if (rw)
> + ret = ia->read_byte(ia, buf, S, P);
> + else
> + ret = ia->write_byte(ia, *buf, S, P);
> + if(ret)
> + return ret;
> + buf++;
> + }
> + return 0;
> +}
> \ No newline at end of file
> diff --git a/platform/generic/sifive_fu740.c
> b/platform/generic/sifive_fu740.c
> index 333b3fb..de82041 100644
> --- a/platform/generic/sifive_fu740.c
> +++ b/platform/generic/sifive_fu740.c
> @@ -34,6 +34,42 @@
>
> #define PMIC_CHIP_ID_DA9063 0x61
>
> +static inline int da9063_read_reg(struct i2c_adapter *adap, uint8_t
> addr,
> + uint8_t reg, uint8_t *val)
> +{
> + struct i2c_msg msg1 = {
> + .addr = addr,
> + .flags = 0,
> + .len = 1,
> + .buf = ®
> + };
> + struct i2c_msg msg2 = {
> + .addr = addr,
> + .flags = I2C_M_RD | I2C_M_NOSTART | I2C_M_STOP,
> + .len = 1,
> + .buf = val
> + };
> + int ret = i2c_adapter_trans(adap, &msg1);
> + if (ret)
> + return ret;
> + return i2c_adapter_trans(adap, &msg2);
> +}
> +
> +static inline int da9063_write_reg(struct i2c_adapter *adap, uint8_t
> addr,
> + uint8_t reg, uint8_t val)
> +{
> + uint8_t buff[2];
> + buff[0] = reg;
> + buff[1] = val;
> + struct i2c_msg msg = {
> + .addr = addr,
> + .flags = I2C_M_STOP,
> + .len = sizeof(buff),
> + .buf = buff
> + };
> + return i2c_adapter_trans(adap, &msg);
> +}
> +
> static struct {
> struct i2c_adapter *adapter;
> uint32_t reg;
> @@ -55,13 +91,13 @@ static int da9063_system_reset_check(u32 type, u32
> reason)
> static inline int da9063_sanity_check(struct i2c_adapter *adap,
> uint32_t reg)
> {
> uint8_t val;
> - int rc = i2c_adapter_reg_write(adap, reg, DA9063_REG_PAGE_CON,
> 0x02);
> + int rc = da9063_write_reg(adap, reg, DA9063_REG_PAGE_CON,
> 0x02);
>
> if (rc)
> return rc;
>
> /* check set page*/
> - rc = i2c_adapter_reg_read(adap, reg, 0x0, &val);
> + rc = da9063_read_reg(adap, reg, 0x0, &val);
> if (rc)
> return rc;
>
> @@ -69,7 +105,7 @@ static inline int da9063_sanity_check(struct
> i2c_adapter *adap, uint32_t reg)
> return SBI_ENODEV;
>
> /* read and check device id */
> - rc = i2c_adapter_reg_read(adap, reg, DA9063_REG_DEVICE_ID,
> &val);
> + rc = da9063_read_reg(adap, reg, DA9063_REG_DEVICE_ID, &val);
> if (rc)
> return rc;
>
> @@ -81,32 +117,32 @@ static inline int da9063_sanity_check(struct
> i2c_adapter *adap, uint32_t reg)
>
> static inline int da9063_shutdown(struct i2c_adapter *adap, uint32_t
> reg)
> {
> - int rc = i2c_adapter_reg_write(adap, da9063.reg,
> + int rc = da9063_write_reg(adap, da9063.reg,
> DA9063_REG_PAGE_CON, 0x00);
>
> if (rc)
> return rc;
>
> - return i2c_adapter_reg_write(adap, da9063.reg,
> + return da9063_write_reg(adap, da9063.reg,
> DA9063_REG_CONTROL_F,
> DA9063_CONTROL_F_SHUTDOWN);
> }
>
> static inline int da9063_reset(struct i2c_adapter *adap, uint32_t
> reg)
> {
> - int rc = i2c_adapter_reg_write(adap, da9063.reg,
> + int rc = da9063_write_reg(adap, da9063.reg,
> DA9063_REG_PAGE_CON, 0x00);
>
> if (rc)
> return rc;
>
> - rc = i2c_adapter_reg_write(adap, da9063.reg,
> + rc = da9063_write_reg(adap, da9063.reg,
> DA9063_REG_CONTROL_F,
> DA9063_CONTROL_F_WAKEUP);
> if (rc)
> return rc;
>
> - return i2c_adapter_reg_write(adap, da9063.reg,
> + return da9063_write_reg(adap, da9063.reg,
> DA9063_REG_CONTROL_A,
> DA9063_CONTROL_A_M_POWER1_EN |
> DA9063_CONTROL_A_M_POWER_EN |
> --
> 2.30.2
>
>
More information about the opensbi
mailing list