[U-Boot] [PATCH V6 2/5] mv_i2c: use structure to replace the direclty define
Lei Wen
adrian.wenl at gmail.com
Sun Apr 3 13:30:47 CEST 2011
Hi Prafulla,
On Sat, Apr 2, 2011 at 2:29 AM, Prafulla Wadaskar <prafulla at marvell.com> wrote:
>
>
>> -----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.
Yep.
>
>> + 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.
Ok, I would do this change for next post.
>
>> +
>> /*
>> * 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?
You may notice original comment above this function is timeout if (no
match within 10 ms).
So the 10ms is 1000*10us, not the 10000*10us. In this patch I correct
this to match its comments.
>
>>
>> - 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.
Yep.
>
>> + 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
>> --
>
Best regards,
Lei
More information about the U-Boot
mailing list