Dear Heiko Schocher,
> Hello Marek,
[...]
> > 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<common.h>
> > #include<malloc.h>
> >
> > +#include<i2c.h>
> >
> > #include<asm/errno.h>
> > #include<asm/io.h>
> > #include<asm/arch/clock.h>
> >
> > @@ -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.
I suspect I'll move this to a separate patch.
> > 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 ...
True ... and they have no other meaning than to align stuff on the scope ;-)
> > +
> > + 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
Could this be your mailer's issue?
[...]
> ... 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 ...
True.
> 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?
Yes, the speed is configured upon boot anyway, so the timing0 register contains
result of previous call to i2c_set_bus_speed(). And if it doesn't, we can't
properly determine the speed anyway :(
> > }
> >
> > void i2c_init(int speed, int slaveadd)
>
> Thanks!
>
> bye,
> Heiko
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
