| Message ID | 1301295228-12752-4-git-send-email-leiwen@marvell.com |
|---|---|
| State | Superseded |
| Headers | show |
> -----Original Message----- > From: Lei Wen [mailto:leiwen@marvell.com] > Sent: Monday, March 28, 2011 12:24 PM > To: Heiko Schocher; Prafulla Wadaskar; Wolfgang Denk; u- > boot@lists.denx.de; Marek Vasut; Ashish Karkare; Prabhanjan Sarnaik; Yu > Tang; adrian.wenl@gmail.com > Subject: [PATCH V5 3/6] 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@marvell.com> > --- > Changelog: > V2: > NO CHANGE > > V3: > clean code sytle issue > > V4: > V5: > 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 > ...snip... > diff --git a/drivers/i2c/mv_i2c.c b/drivers/i2c/mv_i2c.c > index 09756a4..734148b 100644 > --- a/drivers/i2c/mv_i2c.c > +++ b/drivers/i2c/mv_i2c.c > @@ -8,6 +8,9 @@ > * (C) Copyright 2003 Pengutronix e.K. > * Robert Schwebel <r.schwebel@pengutronix.de> > * > + * (C) Copyright 2011 Marvell Inc. > + * Lei Wen <leiwen@marvell.com> > + * > * See file CREDITS for list of people who contributed to this > * project. > * > @@ -34,24 +37,8 @@ > #include <asm/io.h> > > #ifdef CONFIG_HARD_I2C > - > -/* > - * - CONFIG_SYS_I2C_SPEED > - * - I2C_PXA_SLAVE_ADDR > - */ > - > -#include <asm/arch/hardware.h> > -#include <asm/arch/pxa-regs.h> > #include <i2c.h> > - > -#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 > +#include "mv_i2c.h" > > #ifdef DEBUG_I2C > #define PRINTD(x) printf x > @@ -59,20 +46,6 @@ > #define PRINTD(x) > #endif > > -/* 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 > - > /* All transfers are described by this data structure */ > struct i2c_msg { > u8 condition; > @@ -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; > +}; (Optional to implement) It is better if you can push register structure definition to the SoC specific header file, so that if there are new SoC that has different register structures that can be addressed cleanly. > + > +static struct pxa_i2c *base = (struct pxa_i2c *)CONFIG_PXA_I2C_REG; > + > /* > * i2c_pxa_reset: - reset the host controller > * > */ > static void i2c_reset(void) > { > - writel(readl(ICR) & ~ICR_IUE, ICR); /* disable unit */ > - writel(readl(ICR) | ICR_UR, ICR); /* reset the unit */ > + andl(~ICR_IUE, &base->icr); /* disable unit */ > + orl(ICR_UR, &base->icr); /* reset the unit */ Apart from discussion going on for patch - [PATCH V5.1 1/6] io: add and* and or* operation api to set and clear bit The original code looks more readable to me. > udelay(100); > - writel(readl(ICR) & ~ICR_IUE, ICR); /* disable unit */ > -#ifdef CONFIG_CPU_MONAHANS > - /* | CKENB_1_PWM1 | CKENB_0_PWM0); */ > - writel(readl(CKENB) | (CKENB_4_I2C), CKENB); > -#else /* CONFIG_CPU_MONAHANS */ > - /* set the global I2C clock on */ > - writel(readl(CKEN) | CKEN14_I2C, CKEN); > -#endif > - writel(I2C_PXA_SLAVE_ADDR, ISAR); /* set our slave address */ > - writel(I2C_ICR_INIT, ICR); /* set control reg values */ > - writel(I2C_ISR_INIT, ISR); /* set clear interrupt bits */ > - writel(readl(ICR) | ICR_IUE, ICR); /* enable unit */ > + andl(~ICR_IUE, &base->icr); /* disable unit */ > + > + i2c_clk_enable(); > + > + writel(CONFIG_SYS_I2C_SLAVE, &base->isar);/* set our slave address > */ > + writel(I2C_ICR_INIT, &base->icr); /* set control reg values */ Why don't you do I2C_ICR_INIT | ICR_IUE here and avoid orl below? > + writel(I2C_ISR_INIT, &base->isr); /* set clear interrupt bits */ > + orl(ICR_IUE, &base->icr); /* enable unit */ > udelay(100); > } Regards.. Prafulla . .
Hi Prafulla, On Tue, Mar 29, 2011 at 9:27 PM, Prafulla Wadaskar <prafulla@marvell.com> wrote: > > >> -----Original Message----- >> From: Lei Wen [mailto:leiwen@marvell.com] >> Sent: Monday, March 28, 2011 12:24 PM >> To: Heiko Schocher; Prafulla Wadaskar; Wolfgang Denk; u- >> boot@lists.denx.de; Marek Vasut; Ashish Karkare; Prabhanjan Sarnaik; Yu >> Tang; adrian.wenl@gmail.com >> Subject: [PATCH V5 3/6] 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@marvell.com> >> --- >> Changelog: >> V2: >> NO CHANGE >> >> V3: >> clean code sytle issue >> >> V4: >> V5: >> 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 >> > ...snip... >> diff --git a/drivers/i2c/mv_i2c.c b/drivers/i2c/mv_i2c.c >> index 09756a4..734148b 100644 >> --- a/drivers/i2c/mv_i2c.c >> +++ b/drivers/i2c/mv_i2c.c >> @@ -8,6 +8,9 @@ >> * (C) Copyright 2003 Pengutronix e.K. >> * Robert Schwebel <r.schwebel@pengutronix.de> >> * >> + * (C) Copyright 2011 Marvell Inc. >> + * Lei Wen <leiwen@marvell.com> >> + * >> * See file CREDITS for list of people who contributed to this >> * project. >> * >> @@ -34,24 +37,8 @@ >> #include <asm/io.h> >> >> #ifdef CONFIG_HARD_I2C >> - >> -/* >> - * - CONFIG_SYS_I2C_SPEED >> - * - I2C_PXA_SLAVE_ADDR >> - */ >> - >> -#include <asm/arch/hardware.h> >> -#include <asm/arch/pxa-regs.h> >> #include <i2c.h> >> - >> -#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 >> +#include "mv_i2c.h" >> >> #ifdef DEBUG_I2C >> #define PRINTD(x) printf x >> @@ -59,20 +46,6 @@ >> #define PRINTD(x) >> #endif >> >> -/* 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 >> - >> /* All transfers are described by this data structure */ >> struct i2c_msg { >> u8 condition; >> @@ -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; >> +}; > > (Optional to implement) > It is better if you can push register structure definition to the SoC specific header file, so that if there are new SoC that has different register structures that can be addressed cleanly. For current there is no different register arrage for this structure, so I think it is ok to just keep it current state. For the adding register doc description, I have no objection. > >> + >> +static struct pxa_i2c *base = (struct pxa_i2c *)CONFIG_PXA_I2C_REG; >> + >> /* >> * i2c_pxa_reset: - reset the host controller >> * >> */ >> static void i2c_reset(void) >> { >> - writel(readl(ICR) & ~ICR_IUE, ICR); /* disable unit */ >> - writel(readl(ICR) | ICR_UR, ICR); /* reset the unit */ >> + andl(~ICR_IUE, &base->icr); /* disable unit */ >> + orl(ICR_UR, &base->icr); /* reset the unit */ > > Apart from discussion going on for patch - [PATCH V5.1 1/6] io: add and* and or* operation api to set and clear bit > > The original code looks more readable to me. > >> udelay(100); >> - writel(readl(ICR) & ~ICR_IUE, ICR); /* disable unit */ >> -#ifdef CONFIG_CPU_MONAHANS >> - /* | CKENB_1_PWM1 | CKENB_0_PWM0); */ >> - writel(readl(CKENB) | (CKENB_4_I2C), CKENB); >> -#else /* CONFIG_CPU_MONAHANS */ >> - /* set the global I2C clock on */ >> - writel(readl(CKEN) | CKEN14_I2C, CKEN); >> -#endif >> - writel(I2C_PXA_SLAVE_ADDR, ISAR); /* set our slave address */ >> - writel(I2C_ICR_INIT, ICR); /* set control reg values */ >> - writel(I2C_ISR_INIT, ISR); /* set clear interrupt bits */ >> - writel(readl(ICR) | ICR_IUE, ICR); /* enable unit */ >> + andl(~ICR_IUE, &base->icr); /* disable unit */ >> + >> + i2c_clk_enable(); >> + >> + writel(CONFIG_SYS_I2C_SLAVE, &base->isar);/* set our slave address >> */ >> + writel(I2C_ICR_INIT, &base->icr); /* set control reg values */ > > Why don't you do I2C_ICR_INIT | ICR_IUE here and avoid orl below? > This part of logic is the same which I brought from the pxa/i2c code. I cannot make sure if I do this change whether it would make harm to the original platform or not... So my suggestion is keep it like it is. Is that ok for you? Best regards, Lei
> -----Original Message----- > From: Lei Wen [mailto:adrian.wenl@gmail.com] > Sent: Wednesday, March 30, 2011 7:42 PM > To: Prafulla Wadaskar > Cc: Lei Wen; Heiko Schocher; Wolfgang Denk; u-boot@lists.denx.de; Marek > Vasut; Ashish Karkare; Prabhanjan Sarnaik; Yu Tang > Subject: Re: [PATCH V5 3/6] mv_i2c: use structure to replace the > direclty define > > Hi Prafulla, > > On Tue, Mar 29, 2011 at 9:27 PM, Prafulla Wadaskar > <prafulla@marvell.com> wrote: > > > > > >> -----Original Message----- > >> From: Lei Wen [mailto:leiwen@marvell.com] > >> Sent: Monday, March 28, 2011 12:24 PM > >> To: Heiko Schocher; Prafulla Wadaskar; Wolfgang Denk; u- > >> boot@lists.denx.de; Marek Vasut; Ashish Karkare; Prabhanjan Sarnaik; > Yu > >> Tang; adrian.wenl@gmail.com > >> Subject: [PATCH V5 3/6] 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@marvell.com> > >> --- > >> Changelog: > >> V2: > >> NO CHANGE > >> > >> V3: > >> clean code sytle issue > >> > >> V4: > >> V5: > >> 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 > >> > > ...snip... > >> diff --git a/drivers/i2c/mv_i2c.c b/drivers/i2c/mv_i2c.c > >> index 09756a4..734148b 100644 > >> --- a/drivers/i2c/mv_i2c.c > >> +++ b/drivers/i2c/mv_i2c.c > >> @@ -8,6 +8,9 @@ > >> * (C) Copyright 2003 Pengutronix e.K. > >> * Robert Schwebel <r.schwebel@pengutronix.de> > >> * > >> + * (C) Copyright 2011 Marvell Inc. > >> + * Lei Wen <leiwen@marvell.com> > >> + * > >> * See file CREDITS for list of people who contributed to this > >> * project. > >> * > >> @@ -34,24 +37,8 @@ > >> #include <asm/io.h> > >> > >> #ifdef CONFIG_HARD_I2C > >> - > >> -/* > >> - * - CONFIG_SYS_I2C_SPEED > >> - * - I2C_PXA_SLAVE_ADDR > >> - */ > >> - > >> -#include <asm/arch/hardware.h> > >> -#include <asm/arch/pxa-regs.h> > >> #include <i2c.h> > >> - > >> -#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 > >> +#include "mv_i2c.h" > >> > >> #ifdef DEBUG_I2C > >> #define PRINTD(x) printf x > >> @@ -59,20 +46,6 @@ > >> #define PRINTD(x) > >> #endif > >> > >> -/* 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 > >> - > >> /* All transfers are described by this data structure */ > >> struct i2c_msg { > >> u8 condition; > >> @@ -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; > >> +}; > > > > (Optional to implement) > > It is better if you can push register structure definition to the SoC > specific header file, so that if there are new SoC that has different > register structures that can be addressed cleanly. > > For current there is no different register arrage for this structure, > so I think it is > ok to just keep it current state. For the adding register doc > description, I have no > objection. That was just a suggestion, it up to you. > > > >> + > >> +static struct pxa_i2c *base = (struct pxa_i2c *)CONFIG_PXA_I2C_REG; > >> + > >> /* > >> * i2c_pxa_reset: - reset the host controller > >> * > >> */ > >> static void i2c_reset(void) > >> { > >> - writel(readl(ICR) & ~ICR_IUE, ICR); /* disable unit */ > >> - writel(readl(ICR) | ICR_UR, ICR); /* reset the unit */ > >> + andl(~ICR_IUE, &base->icr); /* disable unit */ > >> + orl(ICR_UR, &base->icr); /* reset the unit */ > > > > Apart from discussion going on for patch - [PATCH V5.1 1/6] io: add > and* and or* operation api to set and clear bit > > > > The original code looks more readable to me. > > > >> udelay(100); > >> - writel(readl(ICR) & ~ICR_IUE, ICR); /* disable unit */ > >> -#ifdef CONFIG_CPU_MONAHANS > >> - /* | CKENB_1_PWM1 | CKENB_0_PWM0); */ > >> - writel(readl(CKENB) | (CKENB_4_I2C), CKENB); > >> -#else /* CONFIG_CPU_MONAHANS */ > >> - /* set the global I2C clock on */ > >> - writel(readl(CKEN) | CKEN14_I2C, CKEN); > >> -#endif > >> - writel(I2C_PXA_SLAVE_ADDR, ISAR); /* set our slave > address */ > >> - writel(I2C_ICR_INIT, ICR); /* set control reg > values */ > >> - writel(I2C_ISR_INIT, ISR); /* set clear interrupt > bits */ > >> - writel(readl(ICR) | ICR_IUE, ICR); /* enable unit */ > >> + andl(~ICR_IUE, &base->icr); /* disable unit */ > >> + > >> + i2c_clk_enable(); > >> + > >> + writel(CONFIG_SYS_I2C_SLAVE, &base->isar);/* set our slave > address > >> */ > >> + writel(I2C_ICR_INIT, &base->icr); /* set control reg > values */ > > > > Why don't you do I2C_ICR_INIT | ICR_IUE here and avoid orl below? > > > > This part of logic is the same which I brought from the pxa/i2c code. > I cannot make sure if I do this change whether it would make harm to > the original > platform or not... So my suggestion is keep it like it is. Is that ok > for you? That should not make a difference, but if we don't have h/w to test I respect your comments. Regards.. Prafulla . . > > Best regards, > Lei
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); */ + writel(readl(CKENB) | (CKENB_4_I2C), CKENB); +#else /* CONFIG_CPU_MONAHANS */ + /* set the global I2C clock on */ + writel(readl(CKEN) | CKEN14_I2C, CKEN); +#endif +} diff --git a/arch/arm/include/asm/arch-pxa/pxa-regs.h b/arch/arm/include/asm/arch-pxa/pxa-regs.h index 65a387f..109fdc0 100644 --- a/arch/arm/include/asm/arch-pxa/pxa-regs.h +++ b/arch/arm/include/asm/arch-pxa/pxa-regs.h @@ -456,62 +456,6 @@ typedef void (*ExcpHndlr) (void) ; IrSR_XMITIR_IR_MODE) /* - * I2C registers - */ -#define IBMR 0x40301680 /* I2C Bus Monitor Register - IBMR */ -#define IDBR 0x40301688 /* I2C Data Buffer Register - IDBR */ -#define ICR 0x40301690 /* I2C Control Register - ICR */ -#define ISR 0x40301698 /* I2C Status Register - ISR */ -#define ISAR 0x403016A0 /* I2C Slave Address Register - ISAR */ - -#ifdef CONFIG_CPU_MONAHANS -#define PWRIBMR 0x40f500C0 /* Power I2C Bus Monitor Register-IBMR */ -#define PWRIDBR 0x40f500C4 /* Power I2C Data Buffer Register-IDBR */ -#define PWRICR 0x40f500C8 /* Power I2C Control Register - ICR */ -#define PWRISR 0x40f500CC /* Power I2C Status Register - ISR */ -#define PWRISAR 0x40f500D0 /* Power I2C Slave Address Register-ISAR */ -#else -#define PWRIBMR 0x40f00180 /* Power I2C Bus Monitor Register-IBMR */ -#define PWRIDBR 0x40f00188 /* Power I2C Data Buffer Register-IDBR */ -#define PWRICR 0x40f00190 /* Power I2C Control Register - ICR */ -#define PWRISR 0x40f00198 /* Power I2C Status Register - ISR */ -#define PWRISAR 0x40f001A0 /* Power I2C Slave Address Register-ISAR */ -#endif - -/* ----- 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 */ - -/* * Serial Audio Controller */ /* FIXME the audio defines collide w/ the SA1111 defines. I don't like these diff --git a/board/innokom/innokom.c b/board/innokom/innokom.c index e658c35..22de7e3 100644 --- a/board/innokom/innokom.c +++ b/board/innokom/innokom.c @@ -45,12 +45,7 @@ DECLARE_GLOBAL_DATA_PTR; */ int i2c_init_board(void) { - int i, icr; - - /* disable I2C controller first, otherwhise it thinks we want to */ - /* talk to the slave port... */ - icr = readl(ICR); - writel(readl(ICR) & ~(ICR_SCLE | ICR_IUE), ICR); + int i; /* set gpio pin low _before_ we change direction to output */ writel(GPIO_bit(70), GPCR(70)); @@ -63,8 +58,6 @@ int i2c_init_board(void) udelay(10); } - writel(icr, ICR); - return 0; } diff --git a/drivers/i2c/mv_i2c.c b/drivers/i2c/mv_i2c.c index 09756a4..734148b 100644 --- a/drivers/i2c/mv_i2c.c +++ b/drivers/i2c/mv_i2c.c @@ -8,6 +8,9 @@ * (C) Copyright 2003 Pengutronix e.K. * Robert Schwebel <r.schwebel@pengutronix.de> * + * (C) Copyright 2011 Marvell Inc. + * Lei Wen <leiwen@marvell.com> + * * See file CREDITS for list of people who contributed to this * project. * @@ -34,24 +37,8 @@ #include <asm/io.h> #ifdef CONFIG_HARD_I2C - -/* - * - CONFIG_SYS_I2C_SPEED - * - I2C_PXA_SLAVE_ADDR - */ - -#include <asm/arch/hardware.h> -#include <asm/arch/pxa-regs.h> #include <i2c.h> - -#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 +#include "mv_i2c.h" #ifdef DEBUG_I2C #define PRINTD(x) printf x @@ -59,20 +46,6 @@ #define PRINTD(x) #endif -/* 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 - /* All transfers are described by this data structure */ struct i2c_msg { u8 condition; @@ -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; + /* * i2c_pxa_reset: - reset the host controller * */ static void i2c_reset(void) { - writel(readl(ICR) & ~ICR_IUE, ICR); /* disable unit */ - writel(readl(ICR) | ICR_UR, ICR); /* reset the unit */ + andl(~ICR_IUE, &base->icr); /* disable unit */ + orl(ICR_UR, &base->icr); /* reset the unit */ udelay(100); - writel(readl(ICR) & ~ICR_IUE, ICR); /* disable unit */ -#ifdef CONFIG_CPU_MONAHANS - /* | CKENB_1_PWM1 | CKENB_0_PWM0); */ - writel(readl(CKENB) | (CKENB_4_I2C), CKENB); -#else /* CONFIG_CPU_MONAHANS */ - /* set the global I2C clock on */ - writel(readl(CKEN) | CKEN14_I2C, CKEN); -#endif - writel(I2C_PXA_SLAVE_ADDR, ISAR); /* set our slave address */ - writel(I2C_ICR_INIT, ICR); /* set control reg values */ - writel(I2C_ISR_INIT, ISR); /* set clear interrupt bits */ - writel(readl(ICR) | ICR_IUE, ICR); /* enable unit */ + andl(~ICR_IUE, &base->icr); /* disable unit */ + + i2c_clk_enable(); + + writel(CONFIG_SYS_I2C_SLAVE, &base->isar);/* set our slave address */ + writel(I2C_ICR_INIT, &base->icr); /* set control reg values */ + writel(I2C_ISR_INIT, &base->isr); /* set clear interrupt bits */ + orl(ICR_IUE, &base->icr); /* enable unit */ udelay(100); } @@ -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 = 10000, isr; - 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); + andl(~ICR_START, &base->icr); + andl(~ICR_STOP, &base->icr); + writel(msg->data, &base->idbr); if (msg->condition == I2C_COND_START) - writel(readl(ICR) | ICR_START, ICR); + orl(ICR_START, &base->icr); if (msg->condition == I2C_COND_STOP) - writel(readl(ICR) | ICR_STOP, ICR); + orl(ICR_STOP, &base->icr); if (msg->acknack == I2C_ACKNAK_SENDNAK) - writel(readl(ICR) | ICR_ACKNAK, ICR); + orl(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); + andl(~ICR_ACKNAK, &base->icr); + andl(~ICR_ALDIE, &base->icr); + orl(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); + orl(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); + andl(~ICR_START, &base->icr); + andl(~ICR_STOP, &base->icr); if (msg->condition == I2C_COND_START) - writel(readl(ICR) | ICR_START, ICR); + orl(ICR_START, &base->icr); if (msg->condition == I2C_COND_STOP) - writel(readl(ICR) | ICR_STOP, ICR); + orl(ICR_STOP, &base->icr); if (msg->acknack == I2C_ACKNAK_SENDNAK) - writel(readl(ICR) | ICR_ACKNAK, ICR); + orl(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); + andl(~ICR_ACKNAK, &base->icr); + andl(~ICR_ALDIE, &base->icr); + orl(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); - + orl(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... */ + icr = readl(&base->icr); + andl(~(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 #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 #define CONFIG_HARD_I2C 1 #define CONFIG_SYS_I2C_SPEED 50000 #define CONFIG_SYS_I2C_SLAVE 0xfe
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@marvell.com> --- Changelog: V2: NO CHANGE V3: clean code sytle issue V4: V5: 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