[U-Boot] [PATCH V6 2/5] mv_i2c: use structure to replace the direclty define

Prafulla Wadaskar prafulla at marvell.com
Fri Apr 1 20:29:17 CEST 2011



> -----Original Message-----
> From: Lei Wen [mailto:leiwen at marvell.com]
> Sent: Thursday, March 31, 2011 2:07 PM
> To: Heiko Schocher; Prafulla Wadaskar; Wolfgang Denk; u-
> boot at lists.denx.de; Marek Vasut; Ashish Karkare; Prabhanjan Sarnaik; Yu
> Tang; adrian.wenl at gmail.com
> Subject: [PATCH V6 2/5] mv_i2c: use structure to replace the direclty
> define
> 
> Add i2c_clk_enable in the cpu specific code, since previous platform it,
> while new platform don't need. In the pantheon and armada100 platform,
> this function is defined as NULL one.
> 
> Signed-off-by: Lei Wen <leiwen at marvell.com>
> ---
> Changelog:
> V2:
> NO CHANGE
> 
> V3:
> clean code sytle issue
> 
> V4:
> V5:
> V6:
> NO CHANGE
> 
>  arch/arm/cpu/pxa/cpu.c                   |   11 +++
>  arch/arm/include/asm/arch-pxa/pxa-regs.h |   56 -------------
>  board/innokom/innokom.c                  |    9 +--
>  drivers/i2c/mv_i2c.c                     |  131 ++++++++++++++---------
> -------
>  drivers/i2c/mv_i2c.h                     |   83 +++++++++++++++++++
>  include/configs/innokom.h                |    1 +
>  include/configs/xm250.h                  |    1 +
>  7 files changed, 159 insertions(+), 133 deletions(-)
>  create mode 100644 drivers/i2c/mv_i2c.h
> 
> diff --git a/arch/arm/cpu/pxa/cpu.c b/arch/arm/cpu/pxa/cpu.c
> index 7d49cbb..24b59e7 100644
> --- a/arch/arm/cpu/pxa/cpu.c
> +++ b/arch/arm/cpu/pxa/cpu.c
> @@ -318,3 +318,14 @@ int arch_cpu_init(void)
>  	pxa_clock_setup();
>  	return 0;
>  }
> +
> +void i2c_clk_enable(void)
> +{
> +#ifdef CONFIG_CPU_MONAHANS
> +	/* | CKENB_1_PWM1 | CKENB_0_PWM0); */

This comment line looks like part of code, Can be rephrased in better way.
 
> +	writel(readl(CKENB) | (CKENB_4_I2C), CKENB);
> +#else /* CONFIG_CPU_MONAHANS */
> +	/* set the global I2C clock on */
> +	writel(readl(CKEN) | CKEN14_I2C, CKEN);
> +#endif
> +}

...snip...

> @@ -81,27 +54,37 @@ struct i2c_msg {
>  	u8 data;
>  };
> 
> +struct pxa_i2c {
> +	u32 ibmr;
> +	u32 pad0;
> +	u32 idbr;
> +	u32 pad1;
> +	u32 icr;
> +	u32 pad2;
> +	u32 isr;
> +	u32 pad3;
> +	u32 isar;
> +};
> +
> +static struct pxa_i2c *base = (struct pxa_i2c *)CONFIG_PXA_I2C_REG;

I think to sync with mc_i2c change at least the macro CONFIG_PXA_I2C_REG
Need to be renamed as CONFIG_MV_I2C_REG, because the same will be referred by other SoC code.
 
> +
>  /*
>   * i2c_pxa_reset: - reset the host controller
>   *
>   */

...snip...
> @@ -114,13 +97,15 @@ static void i2c_reset(void)
>  static int i2c_isr_set_cleared(unsigned long set_mask,
>  			       unsigned long cleared_mask)
>  {
> -	int timeout = 10000;
> +	int timeout = 1000, isr;

Is this done purposely? Or reducing timeout value from 10000 to 1000 has some meaning?

> 
> -	while (((ISR & set_mask) != set_mask) || ((ISR & cleared_mask) !=
> 0)) {
> +	do {
> +		isr = readl(&base->isr);
>  		udelay(10);
>  		if (timeout-- < 0)
>  			return 0;
> -	}
> +	} while (((isr & set_mask) != set_mask)
> +		|| ((isr & cleared_mask) != 0));
> 
>  	return 1;
>  }
> @@ -153,26 +138,26 @@ int i2c_transfer(struct i2c_msg *msg)
>  			goto transfer_error_bus_busy;
> 
>  		/* start transmission */
> -		writel(readl(ICR) & ~ICR_START, ICR);
> -		writel(readl(ICR) & ~ICR_STOP, ICR);
> -		writel(msg->data, IDBR);
> +		writel(readl(&base->icr) & ~ICR_START, &base->icr);
> +		writel(readl(&base->icr) & ~ICR_STOP, &base->icr);
> +		writel(msg->data, &base->idbr);
>  		if (msg->condition == I2C_COND_START)
> -			writel(readl(ICR) | ICR_START, ICR);
> +			writel(readl(&base->icr) | ICR_START, &base->icr);
>  		if (msg->condition == I2C_COND_STOP)
> -			writel(readl(ICR) | ICR_STOP, ICR);
> +			writel(readl(&base->icr) | ICR_STOP, &base->icr);
>  		if (msg->acknack == I2C_ACKNAK_SENDNAK)
> -			writel(readl(ICR) | ICR_ACKNAK, ICR);
> +			writel(readl(&base->icr) | ICR_ACKNAK, &base->icr);
>  		if (msg->acknack == I2C_ACKNAK_SENDACK)
> -			writel(readl(ICR) & ~ICR_ACKNAK, ICR);
> -		writel(readl(ICR) & ~ICR_ALDIE, ICR);
> -		writel(readl(ICR) | ICR_TB, ICR);
> +			writel(readl(&base->icr) & ~ICR_ACKNAK, &base->icr);
> +		writel(readl(&base->icr) & ~ICR_ALDIE, &base->icr);
> +		writel(readl(&base->icr) | ICR_TB, &base->icr);
> 
>  		/* transmit register empty? */
>  		if (!i2c_isr_set_cleared(ISR_ITE, 0))
>  			goto transfer_error_transmit_timeout;
> 
>  		/* clear 'transmit empty' state */
> -		writel(readl(ISR) | ISR_ITE, ISR);
> +		writel(readl(&base->isr) | ISR_ITE, &base->isr);
> 
>  		/* wait for ACK from slave */
>  		if (msg->acknack == I2C_ACKNAK_WAITACK)
> @@ -187,28 +172,27 @@ int i2c_transfer(struct i2c_msg *msg)
>  			goto transfer_error_bus_busy;
> 
>  		/* start receive */
> -		writel(readl(ICR) & ~ICR_START, ICR);
> -		writel(readl(ICR) & ~ICR_STOP, ICR);
> +		writel(readl(&base->icr) & ~ICR_START, &base->icr);
> +		writel(readl(&base->icr) & ~ICR_STOP, &base->icr);
>  		if (msg->condition == I2C_COND_START)
> -			writel(readl(ICR) | ICR_START, ICR);
> +			writel(readl(&base->icr) | ICR_START, &base->icr);
>  		if (msg->condition == I2C_COND_STOP)
> -			writel(readl(ICR) | ICR_STOP, ICR);
> +			writel(readl(&base->icr) | ICR_STOP, &base->icr);
>  		if (msg->acknack == I2C_ACKNAK_SENDNAK)
> -			writel(readl(ICR) | ICR_ACKNAK, ICR);
> +			writel(readl(&base->icr) | ICR_ACKNAK, &base->icr);
>  		if (msg->acknack == I2C_ACKNAK_SENDACK)
> -			writel(readl(ICR) & ~ICR_ACKNAK, ICR);
> -		writel(readl(ICR) & ~ICR_ALDIE, ICR);
> -		writel(readl(ICR) | ICR_TB, ICR);
> +			writel(readl(&base->icr) & ~ICR_ACKNAK, &base->icr);
> +		writel(readl(&base->icr) & ~ICR_ALDIE, &base->icr);
> +		writel(readl(&base->icr) | ICR_TB, &base->icr);
> 
>  		/* receive register full? */
>  		if (!i2c_isr_set_cleared(ISR_IRF, 0))
>  			goto transfer_error_receive_timeout;
> 
> -		msg->data = readl(IDBR);
> +		msg->data = readl(&base->idbr);
> 
>  		/* clear 'receive empty' state */
> -		writel(readl(ISR) | ISR_IRF, ISR);
> -
> +		writel(readl(&base->isr) | ISR_IRF, &base->isr);
>  		break;
>  	default:
>  		goto transfer_error_illegal_param;
> @@ -252,10 +236,19 @@ i2c_transfer_finish:
>  void i2c_init(int speed, int slaveaddr)
>  {
>  #ifdef CONFIG_SYS_I2C_INIT_BOARD
> +	u32 icr;
>  	/* call board specific i2c bus reset routine before accessing the
> */
>  	/* environment, which might be in a chip on that bus. For details
> */
>  	/* about this problem see doc/I2C_Edge_Conditions.
> */
> +
> +	/* disable I2C controller first, otherwhise it thinks we want to
> */
> +	/* talk to the slave port...
> */

Again, commenting style can be improved for overall block of comments.

> +	icr = readl(&base->icr);
> +	writel(readl(&base->icr) & ~(ICR_SCLE | ICR_IUE), &base->icr);
> +
>  	i2c_init_board();
> +
> +	writel(icr, &base->icr);
>  #endif
>  }
> 
> diff --git a/drivers/i2c/mv_i2c.h b/drivers/i2c/mv_i2c.h
> new file mode 100644
> index 0000000..41af0d9
> --- /dev/null
> +++ b/drivers/i2c/mv_i2c.h
> @@ -0,0 +1,83 @@
> +/*
> + * (C) Copyright 2011
> + * Marvell Inc, <www.marvell.com>
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#ifndef _MV_I2C_H_
> +#define _MV_I2C_H_
> +extern void i2c_clk_enable(void);
> +
> +/* Shall the current transfer have a start/stop condition? */
> +#define I2C_COND_NORMAL		0
> +#define I2C_COND_START		1
> +#define I2C_COND_STOP		2
> +
> +/* Shall the current transfer be ack/nacked or being waited for it? */
> +#define I2C_ACKNAK_WAITACK	1
> +#define I2C_ACKNAK_SENDACK	2
> +#define I2C_ACKNAK_SENDNAK	4
> +
> +/* Specify who shall transfer the data (master or slave) */
> +#define I2C_READ		0
> +#define I2C_WRITE		1
> +
> +#if (CONFIG_SYS_I2C_SPEED == 400000)
> +#define I2C_ICR_INIT	(ICR_FM | ICR_BEIE | ICR_IRFIE | ICR_ITEIE |
> ICR_GCD \
> +		| ICR_SCLE)
> +#else
> +#define I2C_ICR_INIT	(ICR_BEIE | ICR_IRFIE | ICR_ITEIE | ICR_GCD |
> ICR_SCLE)
> +#endif
> +
> +#define I2C_ISR_INIT		0x7FF
> +/* ----- Control register bits ----------------------------------------
> */
> +
> +#define ICR_START	0x1		/* start bit */
> +#define ICR_STOP	0x2		/* stop bit */
> +#define ICR_ACKNAK	0x4		/* send ACK(0) or NAK(1) */
> +#define ICR_TB		0x8		/* transfer byte bit */
> +#define ICR_MA		0x10		/* master abort */
> +#define ICR_SCLE	0x20		/* master clock enable, mona SCLEA */
> +#define ICR_IUE		0x40		/* unit enable */
> +#define ICR_GCD		0x80		/* general call disable */
> +#define ICR_ITEIE	0x100		/* enable tx interrupts */
> +#define ICR_IRFIE	0x200		/* enable rx interrupts, mona: DRFIE
> */
> +#define ICR_BEIE	0x400		/* enable bus error ints */
> +#define ICR_SSDIE	0x800		/* slave STOP detected int enable */
> +#define ICR_ALDIE	0x1000		/* enable arbitration interrupt */
> +#define ICR_SADIE	0x2000		/* slave address detected int enable
> */
> +#define ICR_UR		0x4000		/* unit reset */
> +#define ICR_FM		0x8000		/* Fast Mode */
> +
> +/* ----- Status register bits -----------------------------------------
> */
> +
> +#define ISR_RWM		0x1		/* read/write mode */
> +#define ISR_ACKNAK	0x2		/* ack/nak status */
> +#define ISR_UB		0x4		/* unit busy */
> +#define ISR_IBB		0x8		/* bus busy */
> +#define ISR_SSD		0x10		/* slave stop detected */
> +#define ISR_ALD		0x20		/* arbitration loss detected */
> +#define ISR_ITE		0x40		/* tx buffer empty */
> +#define ISR_IRF		0x80		/* rx buffer full */
> +#define ISR_GCAD	0x100		/* general call address detected */
> +#define ISR_SAD		0x200		/* slave address detected */
> +#define ISR_BED		0x400		/* bus error no ACK/NAK */
> +
> +#endif
> diff --git a/include/configs/innokom.h b/include/configs/innokom.h
> index 0ea73c9..1ddee03 100644
> --- a/include/configs/innokom.h
> +++ b/include/configs/innokom.h
> @@ -141,6 +141,7 @@
>   * I2C bus
>   */
>  #define CONFIG_I2C_MV			1
> +#define CONFIG_PXA_I2C_REG		0x40301680

This should be CONFIG_MV_I2C_REG

>  #define CONFIG_HARD_I2C			1
>  #define CONFIG_SYS_I2C_SPEED			50000
>  #define CONFIG_SYS_I2C_SLAVE			0xfe
> diff --git a/include/configs/xm250.h b/include/configs/xm250.h
> index b4b940a..682d1ed 100644
> --- a/include/configs/xm250.h
> +++ b/include/configs/xm250.h
> @@ -62,6 +62,7 @@
>   * I2C bus
>   */
>  #define CONFIG_I2C_MV			1
> +#define CONFIG_PXA_I2C_REG		0x40301680

Ditto

>  #define CONFIG_HARD_I2C			1
>  #define CONFIG_SYS_I2C_SPEED			50000
>  #define CONFIG_SYS_I2C_SLAVE			0xfe
> --

Regards..
Prafulla . .

> 1.7.0.4



More information about the U-Boot mailing list