public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Heiko Schocher <hs@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 10/55] exynos: i2c: Tidy up the driver model code
Date: Mon, 06 Jul 2015 07:40:12 +0200	[thread overview]
Message-ID: <559A14BC.60905@denx.de> (raw)
In-Reply-To: <1435882592-487-11-git-send-email-sjg@chromium.org>

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 at 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 */
>

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

  reply	other threads:[~2015-07-06  5:40 UTC|newest]

Thread overview: 139+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-03  0:15 [U-Boot] [PATCH 00/55] dm: exynos: Driver model improvements leading to spring support Simon Glass
2015-07-03  0:15 ` [U-Boot] [PATCH 01/55] dm: core: Support finding a device by phandle Simon Glass
2015-07-27 23:28   ` Simon Glass
2015-07-03  0:15 ` [U-Boot] [PATCH 02/55] dm: i2c: Add a message debug function Simon Glass
2015-07-03  6:52   ` Heiko Schocher denx
2015-07-27 23:28     ` Simon Glass
2015-07-03  0:15 ` [U-Boot] [PATCH 03/55] dm: i2c: Correct comment nits in dm_i2c_reg_read/write() Simon Glass
2015-07-03  6:53   ` Heiko Schocher denx
2015-07-27 23:28     ` Simon Glass
2015-07-03  0:15 ` [U-Boot] [PATCH 04/55] dm: i2c: Move definitions to the top of the header file Simon Glass
2015-07-03  6:55   ` Heiko Schocher denx
2015-07-27 23:28     ` Simon Glass
2015-07-03  0:15 ` [U-Boot] [PATCH 05/55] dm: i2c: Add a function to transfer messages Simon Glass
2015-07-03  6:58   ` Heiko Schocher denx
2015-07-27 23:28     ` Simon Glass
2015-07-03  0:15 ` [U-Boot] [PATCH 06/55] dm: i2c: Add support for multiplexed I2C buses Simon Glass
2015-07-05  6:10   ` Heiko Schocher
2015-07-06 16:38     ` Simon Glass
2015-07-03  0:15 ` [U-Boot] [PATCH 07/55] exynos: i2c: Correct bug in pinmux selection Simon Glass
2015-07-05  6:15   ` Heiko Schocher
2015-07-06 15:27     ` Simon Glass
2015-07-07  1:36       ` Minkyu Kang
2015-07-08  2:38         ` Simon Glass
2015-07-10  1:43           ` Minkyu Kang
2015-07-10  1:46             ` Simon Glass
2015-07-10  8:56               ` Przemyslaw Marczak
2015-07-10 12:14               ` Minkyu Kang
2015-07-07  6:51   ` Przemyslaw Marczak
2015-07-03  0:15 ` [U-Boot] [PATCH 08/55] i2c: Add a mux for GPIO-based I2C bus arbitration Simon Glass
2015-07-05  6:43   ` Heiko Schocher
2015-07-06 16:38     ` Simon Glass
2015-07-03  0:15 ` [U-Boot] [PATCH 09/55] exynos: i2c: Fix code style with ReadWriteByte() Simon Glass
2015-07-05  6:45   ` Heiko Schocher
2015-07-27 23:29     ` Simon Glass
2015-07-03  0:15 ` [U-Boot] [PATCH 10/55] exynos: i2c: Tidy up the driver model code Simon Glass
2015-07-06  5:40   ` Heiko Schocher [this message]
2015-07-27 23:28     ` Simon Glass
2015-07-03  0:15 ` [U-Boot] [PATCH 11/55] exynos: dts: Sync up I2C ports with the kernel Simon Glass
2015-07-10 11:51   ` Przemyslaw Marczak
2015-07-27 23:29     ` Simon Glass
2015-07-03  0:15 ` [U-Boot] [PATCH 12/55] exynos: dts: Add PMIC and regulator definitions Simon Glass
2015-07-10 11:51   ` Przemyslaw Marczak
2015-07-27 23:29     ` Simon Glass
2015-07-03  0:15 ` [U-Boot] [PATCH 13/55] exynos: dts: Support EC tunnel and main TPS65090 regulator Simon Glass
2015-07-27 23:29   ` Simon Glass
2015-07-03  0:15 ` [U-Boot] [PATCH 14/55] dm: cros_ec: Convert the I2C tunnel code to use driver model Simon Glass
2015-07-03  0:15 ` [U-Boot] [PATCH 15/55] cros_ec: Support the LDO access method used by spring Simon Glass
2015-07-03  0:15 ` [U-Boot] [PATCH 16/55] exynos: serial: Refactor init code for debug UART Simon Glass
2015-07-27 23:29   ` Simon Glass
2015-07-03  0:15 ` [U-Boot] [PATCH 17/55] exynos: Add debug UART support for Samsung S5P serial Simon Glass
2015-07-27 23:29   ` Simon Glass
2015-07-03  0:15 ` [U-Boot] [PATCH 18/55] exynos: Enable the debug UART in SPL Simon Glass
2015-07-27 23:30   ` Simon Glass
2015-07-03  0:15 ` [U-Boot] [PATCH 19/55] dm: gpio: Add support for setting a GPIO's pull direction Simon Glass
2015-07-05  4:55   ` Masahiro Yamada
2015-07-06 16:39     ` Simon Glass
2015-07-06 17:20       ` Masahiro Yamada
2015-07-06 17:33         ` Simon Glass
2015-07-03  0:15 ` [U-Boot] [PATCH 20/55] dm: exynos: gpio: Support pull-up/down in GPIOs Simon Glass
2015-07-03  0:15 ` [U-Boot] [PATCH 21/55] dm: power: Add a new driver for the TPS65090 PMIC Simon Glass
2015-07-10 11:52   ` Przemyslaw Marczak
2015-07-27 23:30     ` Simon Glass
2015-07-03  0:15 ` [U-Boot] [PATCH 22/55] dm: power: Add support for TPS65090 FETs Simon Glass
2015-07-10 11:52   ` Przemyslaw Marczak
2015-07-27 23:30     ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 23/55] dm: power: Add support for the S5M8767 PMIC Simon Glass
2015-07-10 11:52   ` Przemyslaw Marczak
2015-07-27 23:30     ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 24/55] dm: power: Add support for S5M8767 regulators Simon Glass
2015-07-10 11:52   ` Przemyslaw Marczak
2015-07-27 23:30     ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 25/55] dm: pmic: max77686: Correct a few nits Simon Glass
2015-07-10 11:53   ` Przemyslaw Marczak
2015-07-27 23:30     ` Simon Glass
2015-07-28  6:19       ` Przemyslaw Marczak
2015-08-03 14:02         ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 26/55] dm: pmic: Correct the pmic_reg_write() implementation Simon Glass
2015-07-10 11:53   ` Przemyslaw Marczak
2015-07-27 23:30     ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 27/55] dm: power: max77686: Correct BUCK register access Simon Glass
2015-07-10 11:53   ` Przemyslaw Marczak
2015-07-27 23:30     ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 28/55] dm: pmic: max77686: Support all BUCK regulators Simon Glass
2015-07-10 11:53   ` Przemyslaw Marczak
2015-07-30  2:05     ` Simon Glass
2015-07-30  8:22       ` Przemyslaw Marczak
2015-08-03 14:05         ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 29/55] dm: power: Don't return an error when regulators are not autoset Simon Glass
2015-07-10 11:54   ` Przemyslaw Marczak
2015-07-27 23:30     ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 30/55] dm: pmic: Display the regulator limits on error Simon Glass
2015-07-10 11:54   ` Przemyslaw Marczak
2015-07-27 23:30     ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 31/55] dm: video: Add support for video bridges Simon Glass
2015-07-27 23:30   ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 32/55] dm: video: Add support for the Parade PS8622/625 bridge Simon Glass
2015-07-27 23:30   ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 33/55] dm: video: Add support for the NXP PTN3460 bridge Simon Glass
2015-07-27 23:30   ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 34/55] exynos: spi: Convert the timeout to debug() Simon Glass
2015-07-27 23:30   ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 35/55] exynos: Correct return value in exynos_mmc_init() Simon Glass
2015-07-27 23:31   ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 36/55] exynos: Add support for the DisplayPort hotplug detect Simon Glass
2015-07-27 23:31   ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 37/55] exynos: video: Correct debug output Simon Glass
2015-07-27 23:31   ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 38/55] exynos: Tidy up CPU frequency display Simon Glass
2015-07-27 23:31   ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 39/55] dm: gpio: Check a GPIO is valid before using it Simon Glass
2015-07-27 23:31   ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 40/55] dts: exynos: snow: Add memory layout description Simon Glass
2015-07-27 23:31   ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 41/55] dts: exynos: pit: Add a new node for the parade video bridge driver Simon Glass
2015-07-27 23:31   ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 42/55] dts: exynos: snow: Add a new node for the NXP " Simon Glass
2015-07-27 23:31   ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 43/55] exynos: dts: Drop the old TPS65090 I2C node Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 44/55] exynos: Add common board code for exynos5 boards that use device tree Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 45/55] exynos: Enable new features for exynos5 boards Simon Glass
2015-07-10 11:55   ` Przemyslaw Marczak
2015-07-28 16:29     ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 46/55] exynos: Remove unneeded device tree control #ifdefs Simon Glass
2015-07-27 23:31   ` Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 47/55] exynos: config: Move common options to the common headers and tidy up Simon Glass
2015-07-10 11:56   ` Przemyslaw Marczak
2015-07-03  0:16 ` [U-Boot] [PATCH 48/55] exynos: Drop old exynos5420-specific board code Simon Glass
2015-07-10 11:56   ` Przemyslaw Marczak
2015-07-03  0:16 ` [U-Boot] [PATCH 49/55] exynos: Drop old exynos5250-specific " Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 50/55] power: Remove old TPS65090 drivers Simon Glass
2015-07-10 11:56   ` Przemyslaw Marczak
2015-07-03  0:16 ` [U-Boot] [PATCH 51/55] cros_ec: Remove the old tunnel code Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 52/55] video: Remove the old parade driver Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 53/55] dts: Drop unused compatible ID for the NXP video bridge Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 54/55] exynos: video: Remove non-device-tree code Simon Glass
2015-07-03  0:16 ` [U-Boot] [PATCH 55/55] exynos: Add support for spring Simon Glass
2015-07-20 14:19 ` [U-Boot] [PATCH 00/55] dm: exynos: Driver model improvements leading to spring support Simon Glass
2015-07-22  4:52   ` Minkyu Kang
2015-07-23  2:08     ` Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=559A14BC.60905@denx.de \
    --to=hs@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox