All of lore.kernel.org
 help / color / mirror / Atom feed
From: Heiko Schocher <hs@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] I2C:Zynq: Adapt this driver to the new model
Date: Mon, 14 Oct 2013 07:59:42 +0200	[thread overview]
Message-ID: <525B884E.3060405@denx.de> (raw)
In-Reply-To: <EFBFAB0BC299D345A30F813A3C85DA87012539FB@EDPR-EX01.logicpd.com>

Hello Michael,

Am 26.09.2013 17:43, schrieb Michael Burr:
> Signed-off-by: Michael Burr<michael.burr@logicpd.com>
> Cc: Heiko Schocher<hs@denx.de>
> Cc: Michal Simek<monstr@monstr.eu>
> ---
> === Note: this patch depends on the previous patch titled
> === "[PATCH] I2C: Zynq: Support for 0-length register address"
> === submitted 24 Sep. 2013.
>
> Tested on Xilinx ZC702 eval board:
> Select various I2C chips using TI PCA9548 bus multiplexer.
> Write and read registers with addresses of length 0 and 1.
>
>   drivers/i2c/zynq_i2c.c |  108 ++++++++++++++++++++++++++++++------------------
>   1 file changed, 67 insertions(+), 41 deletions(-)

Could you please change the config define for this driver from
CONFIG_ZYNQ_I2C to CONFIG_SYS_I2C_ZYNQ ?

Also, adapt please all boards, which use this driver.

and add a entry in README.

Thanks!

> diff --git a/drivers/i2c/zynq_i2c.c b/drivers/i2c/zynq_i2c.c
> index 9cbd3e4..7972a59 100644
> --- a/drivers/i2c/zynq_i2c.c
> +++ b/drivers/i2c/zynq_i2c.c
> @@ -7,6 +7,8 @@
>    *
>    * Copyright (c) 2012-2013 Xilinx, Michal Simek
>    *
> + * Copyright (c) 2013 Logic PD, Michael Burr
> + *
>    * SPDX-License-Identifier:	GPL-2.0+
>    */
>
> @@ -64,18 +66,28 @@ struct zynq_i2c_registers {
>   #define ZYNQ_I2C_FIFO_DEPTH		16
>   #define ZYNQ_I2C_TRANSFERT_SIZE_MAX	255 /* Controller transfer limit */
>
> -#if defined(CONFIG_ZYNQ_I2C0)
> -# define ZYNQ_I2C_BASE	ZYNQ_I2C_BASEADDR0
> -#else
> -# define ZYNQ_I2C_BASE	ZYNQ_I2C_BASEADDR1
> -#endif
> -
> -static struct zynq_i2c_registers *zynq_i2c =
> -	(struct zynq_i2c_registers *)ZYNQ_I2C_BASE;
> +static struct zynq_i2c_registers *i2c_select(struct i2c_adapter *adap)
> +{
> +	return adap->hwadapnr ?
> +	       /* Zynq PS I2C1 */
> +	       (struct zynq_i2c_registers *)ZYNQ_I2C_BASEADDR1 :
> +	       /* Zynq PS I2C0 */
> +	       (struct zynq_i2c_registers *)ZYNQ_I2C_BASEADDR0;
> +}
>
>   /* I2C init called by cmd_i2c when doing 'i2c reset'. */
> -void i2c_init(int requested_speed, int slaveadd)
> +static void zynq_i2c_init(struct i2c_adapter *adap, int speed, int slaveaddr)
>   {
> +	struct zynq_i2c_registers *zynq_i2c = i2c_select(adap);
> +
> +	if (speed != 100000)
> +		debug("Warning: requested speed not supported.\n");
> +	if (slaveaddr)
> +		debug("Warning: slave mode not supported.\n");
> +
> +	/* The following _assumes_ clock rate cpu_1x = 111 MHz */
> +	/* This could use improvement! Also see 'i2c_set_bus_speed' below */
> +
>   	/* 111MHz / ( (3 * 17) * 22 ) = ~100KHz */
>   	writel((16<<  ZYNQ_I2C_CONTROL_DIV_B_SHIFT) |
>   		(2<<  ZYNQ_I2C_CONTROL_DIV_A_SHIFT),&zynq_i2c->control);
> @@ -86,7 +98,7 @@ void i2c_init(int requested_speed, int slaveadd)
>   }
>
>   #ifdef DEBUG
> -static void zynq_i2c_debug_status(void)
> +static void zynq_i2c_debug_status(struct zynq_i2c_registers *zynq_i2c)
>   {
>   	int int_status;
>   	int status;
> @@ -128,7 +140,7 @@ static void zynq_i2c_debug_status(void)
>   #endif
>
>   /* Wait for an interrupt */
> -static u32 zynq_i2c_wait(u32 mask)
> +static u32 zynq_i2c_wait(struct zynq_i2c_registers *zynq_i2c, u32 mask)
>   {
>   	int timeout, int_status;
>
> @@ -139,7 +151,7 @@ static u32 zynq_i2c_wait(u32 mask)
>   			break;
>   	}
>   #ifdef DEBUG
> -	zynq_i2c_debug_status();
> +	zynq_i2c_debug_status(zynq_i2c);
>   #endif
>   	/* Clear interrupt status flags */
>   	writel(int_status&  mask,&zynq_i2c->interrupt_status);
> @@ -151,17 +163,19 @@ static u32 zynq_i2c_wait(u32 mask)
>    * I2C probe called by cmd_i2c when doing 'i2c probe'.
>    * Begin read, nak data byte, end.
>    */
> -int i2c_probe(u8 dev)
> +static int zynq_i2c_probe(struct i2c_adapter *adap, uint8_t chip)
>   {
> +	struct zynq_i2c_registers *zynq_i2c = i2c_select(adap);
> +
>   	/* Attempt to read a byte */
>   	setbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_CLR_FIFO |
>   		ZYNQ_I2C_CONTROL_RW);
>   	clrbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_HOLD);
>   	writel(0xFF,&zynq_i2c->interrupt_status);
> -	writel(dev,&zynq_i2c->address);
> +	writel(chip,&zynq_i2c->address);
>   	writel(1,&zynq_i2c->transfer_size);
>
> -	return (zynq_i2c_wait(ZYNQ_I2C_INTERRUPT_COMP |
> +	return (zynq_i2c_wait(zynq_i2c, ZYNQ_I2C_INTERRUPT_COMP |
>   		ZYNQ_I2C_INTERRUPT_NACK)&
>   		ZYNQ_I2C_INTERRUPT_COMP) ? 0 : -ETIMEDOUT;
>   }
> @@ -170,14 +184,18 @@ int i2c_probe(u8 dev)
>    * I2C read called by cmd_i2c when doing 'i2c read' and by cmd_eeprom.c
>    * Begin write, send address byte(s), begin read, receive data bytes, end.
>    */
> -int i2c_read(u8 dev, uint addr, int alen, u8 *data, int length)
> +static int zynq_i2c_read(struct i2c_adapter *adap, uint8_t chip, uint addr,
> +			 int alen, uint8_t *buffer, int len)
>   {
> +	struct zynq_i2c_registers *zynq_i2c;
>   	u32 status;
>   	u32 i = 0;
> -	u8 *cur_data = data;
> +	u8 *cur_data = buffer;
> +
> +	zynq_i2c = i2c_select(adap);
>
>   	/* Check the hardware can handle the requested bytes */
> -	if ((length<  0) || (length>  ZYNQ_I2C_TRANSFERT_SIZE_MAX))
> +	if ((len<  0) || (len>  ZYNQ_I2C_TRANSFERT_SIZE_MAX))
>   		return -EINVAL;
>
>   	/* Write the register address */
> @@ -191,12 +209,12 @@ int i2c_read(u8 dev, uint addr, int alen, u8 *data, int length)
>   	writel(0xFF,&zynq_i2c->interrupt_status);
>   	if (alen) {
>   		clrbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_RW);
> -		writel(dev,&zynq_i2c->address);
> +		writel(chip,&zynq_i2c->address);
>   		while (alen--)
>   			writel(addr>>  (8*alen),&zynq_i2c->data);
>
>   		/* Wait for the address to be sent */
> -		if (!zynq_i2c_wait(ZYNQ_I2C_INTERRUPT_COMP)) {
> +		if (!zynq_i2c_wait(zynq_i2c, ZYNQ_I2C_INTERRUPT_COMP)) {
>   			/* Release the bus */
>   			clrbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_HOLD);
>   			return -ETIMEDOUT;
> @@ -207,12 +225,12 @@ int i2c_read(u8 dev, uint addr, int alen, u8 *data, int length)
>   	setbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_CLR_FIFO |
>   		ZYNQ_I2C_CONTROL_RW);
>   	/* Start reading data */
> -	writel(dev,&zynq_i2c->address);
> -	writel(length,&zynq_i2c->transfer_size);
> +	writel(len,&zynq_i2c->transfer_size);
> +	writel(chip,&zynq_i2c->address);
>
>   	/* Wait for data */
>   	do {
> -		status = zynq_i2c_wait(ZYNQ_I2C_INTERRUPT_COMP |
> +		status = zynq_i2c_wait(zynq_i2c, ZYNQ_I2C_INTERRUPT_COMP |
>   			ZYNQ_I2C_INTERRUPT_DATA);
>   		if (!status) {
>   			/* Release the bus */
> @@ -220,15 +238,15 @@ int i2c_read(u8 dev, uint addr, int alen, u8 *data, int length)
>   			return -ETIMEDOUT;
>   		}
>   		debug("Read %d bytes\n",
> -		      length - readl(&zynq_i2c->transfer_size));
> -		for (; i<  length - readl(&zynq_i2c->transfer_size); i++)
> +		      len - readl(&zynq_i2c->transfer_size));
> +		for (; i<  len - readl(&zynq_i2c->transfer_size); i++)
>   			*(cur_data++) = readl(&zynq_i2c->data);
>   	} while (readl(&zynq_i2c->transfer_size) != 0);
>   	/* All done... release the bus */
>   	clrbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_HOLD);
>
>   #ifdef DEBUG
> -	zynq_i2c_debug_status();
> +	zynq_i2c_debug_status(zynq_i2c);
>   #endif
>   	return 0;
>   }
> @@ -237,31 +255,35 @@ int i2c_read(u8 dev, uint addr, int alen, u8 *data, int length)
>    * I2C write called by cmd_i2c when doing 'i2c write' and by cmd_eeprom.c
>    * Begin write, send address byte(s), send data bytes, end.
>    */
> -int i2c_write(u8 dev, uint addr, int alen, u8 *data, int length)
> +static int zynq_i2c_write(struct i2c_adapter *adap, uint8_t chip, uint addr,
> +			  int alen, uint8_t *buffer, int len)
>   {
> -	u8 *cur_data = data;
> +	struct zynq_i2c_registers *zynq_i2c;
> +	u8 *cur_data = buffer;
> +
> +	zynq_i2c = i2c_select(adap);
>
>   	/* Write the register address */
>   	setbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_CLR_FIFO |
>   		ZYNQ_I2C_CONTROL_HOLD);
>   	clrbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_RW);
>   	writel(0xFF,&zynq_i2c->interrupt_status);
> -	writel(dev,&zynq_i2c->address);
> +	writel(chip,&zynq_i2c->address);
>   	if (alen) {
>   		while (alen--)
>   			writel(addr>>  (8*alen),&zynq_i2c->data);
>   		/* Start the tranfer */
> -		if (!zynq_i2c_wait(ZYNQ_I2C_INTERRUPT_COMP)) {
> +		if (!zynq_i2c_wait(zynq_i2c, ZYNQ_I2C_INTERRUPT_COMP)) {
>   			/* Release the bus */
>   			clrbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_HOLD);
>   			return -ETIMEDOUT;
>   		}
>   		debug("Device acked address\n");
>   	}
> -	while (length--) {
> +	while (len--) {
>   		writel(*(cur_data++),&zynq_i2c->data);
>   		if (readl(&zynq_i2c->transfer_size) == ZYNQ_I2C_FIFO_DEPTH) {
> -			if (!zynq_i2c_wait(ZYNQ_I2C_INTERRUPT_COMP)) {
> +			if (!zynq_i2c_wait(zynq_i2c, ZYNQ_I2C_INTERRUPT_COMP)) {
>   				/* Release the bus */
>   				clrbits_le32(&zynq_i2c->control,
>   					     ZYNQ_I2C_CONTROL_HOLD);
> @@ -273,21 +295,25 @@ int i2c_write(u8 dev, uint addr, int alen, u8 *data, int length)
>   	/* All done... release the bus */
>   	clrbits_le32(&zynq_i2c->control, ZYNQ_I2C_CONTROL_HOLD);
>   	/* Wait for the address and data to be sent */
> -	if (!zynq_i2c_wait(ZYNQ_I2C_INTERRUPT_COMP))
> +	if (!zynq_i2c_wait(zynq_i2c, ZYNQ_I2C_INTERRUPT_COMP))
>   		return -ETIMEDOUT;
>   	return 0;
>   }
>
> -int i2c_set_bus_num(unsigned int bus)
> +static uint zynq_i2c_set_bus_speed(struct i2c_adapter *adap, uint speed)
>   {
> -	/* Only support bus 0 */
> -	if (bus>  0)
> +	/* struct zynq_i2c_registers *zynq_i2c = i2c_select(adap); */
> +
> +	/* Bus-speed selection is not implemented */
> +	if (speed != 100000)
>   		return -1;
> -	return 0;
> -}
>
> -unsigned int i2c_get_bus_num(void)
> -{
> -	/* Only support bus 0 */
>   	return 0;
>   }
> +
> +U_BOOT_I2C_ADAP_COMPLETE(ZYNQ_I2C0, zynq_i2c_init, zynq_i2c_probe,
> +			 zynq_i2c_read, zynq_i2c_write,
> +			 zynq_i2c_set_bus_speed, 100000, 0, 0)
> +U_BOOT_I2C_ADAP_COMPLETE(ZYNQ_I2C1, zynq_i2c_init, zynq_i2c_probe,
> +			 zynq_i2c_read, zynq_i2c_write,
> +			 zynq_i2c_set_bus_speed, 100000, 0, 1)

Do we need here a define for speed and slave settings?

Thanks for your work!

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

  reply	other threads:[~2013-10-14  5:59 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-26 15:43 [U-Boot] [PATCH] I2C:Zynq: Adapt this driver to the new model Michael Burr
2013-10-14  5:59 ` Heiko Schocher [this message]
2013-10-14 16:25   ` Michael Burr
2013-10-15  5:13     ` Heiko Schocher
2013-10-15  5:24       ` Michal Simek
2013-10-15  5:56         ` Heiko Schocher
  -- strict thread matches above, loose matches on Subject: below --
2013-10-14 16:25 Michael Burr
2013-10-15  5:59 ` Heiko Schocher
2013-10-15 12:05   ` Jagan Teki
2013-10-15 17:19     ` Michael Burr

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=525B884E.3060405@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.