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

[Heiko Schocher <hs@denx.de>]

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