diff mbox

[U-Boot] I2C:Zynq: Adapt this driver to the new model

Message ID EFBFAB0BC299D345A30F813A3C85DA87012539FB@EDPR-EX01.logicpd.com
State Changes Requested
Delegated to: Heiko Schocher
Headers show

Commit Message

Michael Burr Sept. 26, 2013, 3:43 p.m. UTC
Signed-off-by: Michael Burr <michael.burr@logicpd.com>
Cc: Heiko Schocher <hs@denx.de>
Cc: Michal Simek <monstr@monstr.eu>
---

Comments

Heiko Schocher Oct. 14, 2013, 5:59 a.m. UTC | #1
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
Michael Burr Oct. 14, 2013, 4:25 p.m. UTC | #2
I am submitting a patch for 'include/zynq.h' as well as for the README file.

I might be able to add code for certain particular boards which use this driver -- however, I think it may be easier for everyone if we don't do that until mainline is synchronized with Michal's Xilinx repo. (That's because the Xilinx repo has config files for several individual boards, but the mainline repo doesn't yet have those files.)

Michael Burr  //  Software Engineer II

Logic PD
T // 612.436.7273
NOTICE: Important disclaimers and limitations apply to this email.
Please see this web page for our disclaimers and limitations:
http://logicpd.com/email-disclaimer/

-----Original Message-----
From: Heiko Schocher [mailto:hs@denx.de] 
Sent: Monday, October 14, 2013 1:00 AM
To: Michael Burr
Cc: u-boot@lists.denx.de; Michal Simek
Subject: Re: [PATCH] I2C:Zynq: Adapt this driver to the new model

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
Heiko Schocher Oct. 15, 2013, 5:13 a.m. UTC | #3
Hello Michael,

Am 14.10.2013 18:25, schrieb Michael Burr:
> I am submitting a patch for 'include/zynq.h' as well as for the README file.

and the drivers/i2c/Makefile ;-)
Thanks!

> I might be able to add code for certain particular boards which use this driver -- however, I think it may be easier for everyone if we don't do that until mainline is synchronized with Michal's Xilinx repo. (That's because the Xilinx repo has config files for several individual boards, but the mainline repo doesn't yet have those files.)

Ok, please sent this patches, if this is done ... or Michal pick up
this patches?

Maybe it is time to make a common include file for this boards, if all
use the same i2c (not only i2c) settings?

bye,
Heiko
Michal Simek Oct. 15, 2013, 5:24 a.m. UTC | #4
Hi Michael and Heiko,

On 10/15/2013 07:13 AM, Heiko Schocher wrote:
> Hello Michael,
> 
> Am 14.10.2013 18:25, schrieb Michael Burr:
>> I am submitting a patch for 'include/zynq.h' as well as for the README file.
> 
> and the drivers/i2c/Makefile ;-)
> Thanks!

Regarding this change. I have asked Siva and Jagan to retest this patch.
It was holiday/day off in India that's why I haven't got any reply yet.

> 
>> I might be able to add code for certain particular boards which use this driver -- however, I think it may be easier for everyone if we don't do that until mainline is synchronized with Michal's Xilinx repo. (That's because the Xilinx repo has config files for several individual boards, but the mainline repo doesn't yet have those files.)
> 
> Ok, please sent this patches, if this is done ... or Michal pick up
> this patches?
>
> Maybe it is time to make a common include file for this boards, if all
> use the same i2c (not only i2c) settings?

v2013.10 hasn't been tagged yet that's why I or Jagan haven't started with unification.
But for the next release I want to synchronize our configuration with mainline one.
It is mostly in include/configs/ folder. There should be a week after ELCE to look at it.

Thanks,
Michal
Heiko Schocher Oct. 15, 2013, 5:56 a.m. UTC | #5
Hello Michael,

Am 15.10.2013 07:24, schrieb Michal Simek:
> Hi Michael and Heiko,
>
> On 10/15/2013 07:13 AM, Heiko Schocher wrote:
>> Hello Michael,
>>
>> Am 14.10.2013 18:25, schrieb Michael Burr:
>>> I am submitting a patch for 'include/zynq.h' as well as for the README file.
>>
>> and the drivers/i2c/Makefile ;-)
>> Thanks!
>
> Regarding this change. I have asked Siva and Jagan to retest this patch.
> It was holiday/day off in India that's why I haven't got any reply yet.

Ah, ok! So you can pickup this patch after it is tested. If patch is
tested please resend it, so I can add my Acked-by to it.

>>> I might be able to add code for certain particular boards which use this driver -- however, I think it may be easier for everyone if we don't do that until mainline is synchronized with Michal's Xilinx repo. (That's because the Xilinx repo has config files for several individual boards, but the mainline repo doesn't yet have those files.)
>>
>> Ok, please sent this patches, if this is done ... or Michal pick up
>> this patches?
>>
>> Maybe it is time to make a common include file for this boards, if all
>> use the same i2c (not only i2c) settings?
>
> v2013.10 hasn't been tagged yet that's why I or Jagan haven't started with unification.
> But for the next release I want to synchronize our configuration with mainline one.
> It is mostly in include/configs/ folder. There should be a week after ELCE to look at it.

Great!

Thanks!

bye,
Heiko
diff mbox

Patch

=== 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(-)

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)