Re: [U-Boot] [PATCH 03/22] ARM sunxi: I2C driver

Dear Henrik Nordström,

[...]

+static struct i2c __attribute__ ((section(".data"))) *i2c_base =

• (struct i2c *)0x1c2ac00;

I dont think you need this workaround at all ... just stick it into the function, it's a static constant (and #define the constant please)

+void i2c_init(int speed, int slaveaddr) +{

• sunxi_gpio_set_cfgpin(SUNXI_GPB(0), 2);
• sunxi_gpio_set_cfgpin(SUNXI_GPB(1), 2);
• clock_twi_onoff(0, 1);
• /* Enable the i2c bus */
• writel(TWI_CTL_BUSEN, &i2c_base->ctl);
• /* 400KHz operation M=2, N=1, 24MHz APB clock */
• writel(TWI_CLK_DIV(2, 1), &i2c_base->clkr);
• writel(TWI_SRST_SRST, &i2c_base->reset);
• while ((readl(&i2c_base->reset) & TWI_SRST_SRST))

• ;
  

+}

+int i2c_probe(uchar chip) +{

How can this even work?

• return -1;

+}

+static int i2c_wait_ctl(int mask, int state) +{

• int timeout = 0x2ff;
• int value = state ? mask : 0;
• debug("i2c_wait_ctl(%x == %x), ctl=%x, status=%x\n", mask, value,

•      i2c_base->ctl, i2c_base->status);
  

• while (((readl(&i2c_base->ctl) & mask) != value) && timeout-- > 0)

• ;
  

What about you change these to :

while (!--timeout) { if (readl...) break; }

To make it more readable ?

• debug("i2c_wait_ctl(), timeout=%d, ctl=%x, status=%x\n", timeout,

•      i2c_base->ctl, i2c_base->status);
  

• if (timeout != 0)

• return 0;
  

• else

• return -1;
  

+}

+static void i2c_clear_irq(void) +{

• writel(readl(&i2c_base->ctl) & ~TWI_CTL_INTFLG, &i2c_base->ctl);

clrbits_le32()

+}

[...]