From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Schocher Date: Sat, 01 Dec 2012 02:35:32 +0100 Subject: [U-Boot] [PATCH v2] mxs: i2c: Implement algorithm to set up arbitrary i2c speed In-Reply-To: <1354289339-24971-1-git-send-email-marex@denx.de> References: <1354280910-17539-1-git-send-email-marex@denx.de> <1354289339-24971-1-git-send-email-marex@denx.de> Message-ID: <50B95EE4.5020005@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello Marek, On 30.11.2012 16:28, Marek Vasut wrote: > This algorithm computes the values of TIMING{0,1,2} registers for the > MX28 I2C block. This algorithm was derived by using a scope, but the > result seems correct. > > The resulting values programmed into the registers do not correlate > with the contents in datasheet. When using the values from the datasheet, > the I2C clock were completely wrong. > > Signed-off-by: Marek Vasut > Cc: Stefano Babic > Cc: Fabio Estevam > Cc: Wolfgang Denk > --- > arch/arm/cpu/arm926ejs/mxs/clock.c | 2 + > arch/arm/include/asm/arch-mxs/clock.h | 1 + > drivers/i2c/mxs_i2c.c | 75 ++++++++++----------------------- > 3 files changed, 26 insertions(+), 52 deletions(-) > > v2: Properly implement XTAL clock retrieval. The i2c clock are derived from the > 24MHz XTAL clock. > [...] > diff --git a/drivers/i2c/mxs_i2c.c b/drivers/i2c/mxs_i2c.c > index 006fb91..b040535 100644 > --- a/drivers/i2c/mxs_i2c.c > +++ b/drivers/i2c/mxs_i2c.c > @@ -28,6 +28,7 @@ > > #include > #include > +#include > #include > #include > #include > @@ -40,6 +41,7 @@ void mxs_i2c_reset(void) > { > struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE; > int ret; > + int speed = i2c_get_bus_speed(); > > ret = mxs_reset_block(&i2c_regs->hw_i2c_ctrl0_reg); > if (ret) { > @@ -53,6 +55,8 @@ void mxs_i2c_reset(void) > &i2c_regs->hw_i2c_ctrl1_clr); > > writel(I2C_QUEUECTRL_PIO_QUEUE_MODE,&i2c_regs->hw_i2c_queuectrl_set); > + > + i2c_set_bus_speed(speed); > } This change is not described in the patch description, please fix. > void mxs_i2c_setup_read(uint8_t chip, int len) > @@ -210,62 +214,29 @@ int i2c_probe(uchar chip) > return ret; > } > > -static struct mxs_i2c_speed_table { [...] > int i2c_set_bus_speed(unsigned int speed) > { > struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE; > - struct mxs_i2c_speed_table *spd = mxs_i2c_speed_to_cfg(speed); > > - if (!spd) { > - printf("MXS I2C: Invalid speed selected (%d Hz)\n", speed); > + uint32_t clk = mxc_get_clock(MXC_XTAL_CLK); > + uint32_t base = ((clk / speed) - 38) / 2; > + uint16_t high_count = base + 3; > + uint16_t low_count = base - 3; > + uint16_t rcv_count = (high_count * 3) / 4; > + uint16_t xmit_count = low_count / 4; a lot of magic constants ... > + > + if (speed> 540000) { > + printf("MXS I2C: Speed too high (%d Hz)\n", speed); > + return -EINVAL; > + } > + > + if (speed< 12000) { > + printf("MXS I2C: Speed too low (%d Hz)\n", speed); > return -EINVAL; > } > > - writel(spd->timing0,&i2c_regs->hw_i2c_timing0); > - writel(spd->timing1,&i2c_regs->hw_i2c_timing1); > + writel((high_count<< 16) | rcv_count,&i2c_regs->hw_i2c_timing0); > + writel((low_count<< 16) | xmit_count,&i2c_regs->hw_i2c_timing1); ^ ^ spaces > > writel((0x0030<< I2C_TIMING2_BUS_FREE_OFFSET) | > (0x0030<< I2C_TIMING2_LEADIN_COUNT_OFFSET), > @@ -277,12 +248,12 @@ int i2c_set_bus_speed(unsigned int speed) > unsigned int i2c_get_bus_speed(void) > { > struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE; > - uint32_t timing0, timing1; > + uint32_t clk = mxc_get_clock(MXC_XTAL_CLK); > + uint32_t timing0; > > timing0 = readl(&i2c_regs->hw_i2c_timing0); > - timing1 = readl(&i2c_regs->hw_i2c_timing1); > > - return mxs_i2c_cfg_to_speed(timing0, timing1); > + return clk / ((((timing0>> 16) - 3) * 2) + 38); ^ spaces ... and here a lot of magic constants too ... as this is a result of measuring ... we should at least note here, that we have this values from a scope session and the values in the manual seem to be not correct ... Hmm... I am a little confused ... you write the i2c_regs->hw_i2c_timing{0,1,2} registers in i2c_set_bus_speed() but you return in i2c_get_bus_speed() just results from reading the i2c_regs->hw_i2c_timing0 register only and do some funny calculations ... is this correct? > } > > void i2c_init(int speed, int slaveadd) Thanks! bye, Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany