Re: [U-Boot] [PATCH 10/55] exynos: i2c: Tidy up the driver model code

Hello Simon,

Am 03.07.2015 um 02:15 schrieb Simon Glass:

The existing driver model implementation uses the old non-driver-model code to operate, but has become impossibly tangled as a result. The actual algorithm is quite simple.

Also the normal-speed and high-speed buses are quite different and it doesn't seem that useful to put them in the same driver.

Finally, there is a bug which breaks communication with the Maxim sound codec and may cause problems with other device.

Rewrite the driver model code for normal-speed operation so that it is easier to understand, and fix the bug. Add a TODO to split the drivers.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/i2c/s3c24x0_i2c.c | 139 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 103 insertions(+), 36 deletions(-)

Reviewed-by: Heiko Schocher hs@denx.de

bye, Heiko

diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c index 4bbd012..c11a6be 100644 --- a/drivers/i2c/s3c24x0_i2c.c +++ b/drivers/i2c/s3c24x0_i2c.c @@ -1284,62 +1284,106 @@ U_BOOT_I2C_ADAP_COMPLETE(s3c0, s3c24x0_i2c_init, s3c24x0_i2c_probe, #endif /* CONFIG_SYS_I2C */

#ifdef CONFIG_DM_I2C -static int i2c_write_data(struct s3c24x0_i2c_bus *i2c_bus, uchar chip,

• 	  uchar *buffer, int len, bool end_with_repeated_start)
  

+static int exynos_hs_i2c_xfer(struct udevice *dev, struct i2c_msg *msg,

• 	      int nmsgs)
  

  {
• struct s3c24x0_i2c_bus *i2c_bus = dev_get_priv(dev);
• struct exynos5_hsi2c *hsregs = i2c_bus->hsregs; int ret;

• if (i2c_bus->is_highspeed) {

• ret = hsi2c_write(i2c_bus->hsregs, chip, 0, 0,
  

• 		  buffer, len, true);
  

• if (ret)
  

• for (; nmsgs > 0; nmsgs--, msg++) {

• if (msg->flags & I2C_M_RD) {
  

• 	ret = hsi2c_read(hsregs, msg->addr, 0, 0, msg->buf,
  

• 			 msg->len);
  

• } else {
  

• 	ret = hsi2c_write(hsregs, msg->addr, 0, 0, msg->buf,
  

• 			  msg->len, true);
  

• }
  

• if (ret) {
  exynos5_i2c_reset(i2c_bus);
  

• } else {

• ret = i2c_transfer(i2c_bus->regs, I2C_WRITE,
  

• 		   chip << 1, 0, 0, buffer, len);
  

• 	return -EREMOTEIO;
  

• }
  

  }

• return ret != I2C_OK;

• return 0; }

-static int i2c_read_data(struct s3c24x0_i2c_bus *i2c_bus, uchar chip,

• 	 uchar *buffer, int len)
  

+static int s3c24x0_do_msg(struct s3c24x0_i2c_bus *i2c_bus, struct i2c_msg *msg,

• 	  int seq)
  

  {

• int ret;

• struct s3c24x0_i2c *i2c = i2c_bus->regs;
• bool is_read = msg->flags & I2C_M_RD;
• uint status;
• uint addr;
• int ret, i;

• if (i2c_bus->is_highspeed) {

• ret = hsi2c_read(i2c_bus->hsregs, chip, 0, 0, buffer, len);
  

• if (ret)
  

• 	exynos5_i2c_reset(i2c_bus);
  

• if (!seq)

• setbits_le32(&i2c->iiccon, I2CCON_ACKGEN);
  

• /* Get the slave chip address going */
• addr = msg->addr << 1;
• writel(addr, &i2c->iicds);
• status = I2C_TXRX_ENA | I2C_START_STOP;
• if (is_read)

• status |= I2C_MODE_MR;
  

• else

• status |= I2C_MODE_MT;
  

• writel(status, &i2c->iicstat);
• if (seq)

• read_write_byte(i2c);
  

• /* Wait for chip address to transmit */
• ret = WaitForXfer(i2c);
• if (ret)

• goto err;
  

• if (is_read) {

• for (i = 0; !ret && i < msg->len; i++) {
  

• 	/* disable ACK for final READ */
  

• 	if (i == msg->len - 1)
  

• 		clrbits_le32(&i2c->iiccon, I2CCON_ACKGEN);
  

• 	read_write_byte(i2c);
  

• 	ret = WaitForXfer(i2c);
  

• 	msg->buf[i] = readl(&i2c->iicds);
  

• }
  

• if (ret == I2C_NACK)
  

• 	ret = I2C_OK; /* Normal terminated read */
  

  } else {

• ret = i2c_transfer(i2c_bus->regs, I2C_READ,
  

• 		   chip << 1, 0, 0, buffer, len);
  

• for (i = 0; !ret && i < msg->len; i++) {
  

• 	writel(msg->buf[i], &i2c->iicds);
  

• 	read_write_byte(i2c);
  

• 	ret = WaitForXfer(i2c);
  

• }
  

  }

• return ret != I2C_OK;

+err:

• return ret; }

  static int s3c24x0_i2c_xfer(struct udevice *dev, struct i2c_msg *msg, int nmsgs) { struct s3c24x0_i2c_bus *i2c_bus = dev_get_priv(dev);

• int ret;

• struct s3c24x0_i2c *i2c = i2c_bus->regs;
• ulong start_time;
• int ret, i;

• for (; nmsgs > 0; nmsgs--, msg++) {

• bool next_is_read = nmsgs > 1 && (msg[1].flags & I2C_M_RD);
  

• if (msg->flags & I2C_M_RD) {
  

• 	ret = i2c_read_data(i2c_bus, msg->addr, msg->buf,
  

• 			    msg->len);
  

• } else {
  

• 	ret = i2c_write_data(i2c_bus, msg->addr, msg->buf,
  

• 			     msg->len, next_is_read);
  

• start_time = get_timer(0);
• while (readl(&i2c->iicstat) & I2CSTAT_BSY) {

• if (get_timer(start_time) > I2C_TIMEOUT_MS) {
  

• 	debug("Timeout\n");
  

• 	return -ETIMEDOUT;
  

  }

• if (ret)
  

• 	return -EREMOTEIO;
  

  }

• return 0;

• for (ret = 0, i = 0; !ret && i < nmsgs; i++)

• ret = s3c24x0_do_msg(i2c_bus, &msg[i], i);
  

• /* Send STOP */

• writel(I2C_MODE_MR | I2C_TXRX_ENA, &i2c->iicstat);

• read_write_byte(i2c);

• return ret ? -EREMOTEIO : 0; }

  static int s3c_i2c_ofdata_to_platdata(struct udevice *dev)

@@ -1364,8 +1408,7 @@ static int s3c_i2c_ofdata_to_platdata(struct udevice *dev) i2c_bus->id = pinmux_decode_periph_id(blob, node);

i2c_bus->clock_frequency = fdtdec_get_int(blob, node,

• 				"clock-frequency",
  

• 				CONFIG_SYS_I2C_S3C24X0_SPEED);
  

• 				  "clock-frequency", 100000);
  

  i2c_bus->node = node; i2c_bus->bus_num = dev->seq;

@@ -1384,7 +1427,6 @@ static const struct dm_i2c_ops s3c_i2c_ops = {

static const struct udevice_id s3c_i2c_ids[] = { { .compatible = "samsung,s3c2440-i2c", .data = EXYNOS_I2C_STD },

• { .compatible = "samsung,exynos5-hsi2c", .data = EXYNOS_I2C_HS }, { } };

@@ -1397,4 +1439,29 @@ U_BOOT_DRIVER(i2c_s3c) = { .priv_auto_alloc_size = sizeof(struct s3c24x0_i2c_bus), .ops = &s3c_i2c_ops, };

+/*

• • TODO(sjg@chromium.org): Move this to a separate file when everything uses
• • driver model
• */

+static const struct dm_i2c_ops exynos_hs_i2c_ops = {

• .xfer = exynos_hs_i2c_xfer,
• .probe_chip = s3c24x0_i2c_probe,
• .set_bus_speed = s3c24x0_i2c_set_bus_speed,

+};

+static const struct udevice_id exynos_hs_i2c_ids[] = {

• { .compatible = "samsung,exynos5-hsi2c", .data = EXYNOS_I2C_HS },
• { }

+};

+U_BOOT_DRIVER(hs_i2c) = {

• .name = "i2c_s3c_hs",
• .id = UCLASS_I2C,
• .of_match = exynos_hs_i2c_ids,
• .ofdata_to_platdata = s3c_i2c_ofdata_to_platdata,
• .per_child_auto_alloc_size = sizeof(struct dm_i2c_chip),
• .priv_auto_alloc_size = sizeof(struct s3c24x0_i2c_bus),
• .ops = &exynos_hs_i2c_ops,

+}; #endif /* CONFIG_DM_I2C */