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

Heiko Schocher
2012-11-26T11:13:57Z

Hello Hendrik,

On 25.11.2012 12:38, Henrik Nordstr?m wqrote:
> A basic basic driver for the I2C controller found in Allwinner
> sunXi (A10& A13) SoCs.
>
> Signed-off-by: Henrik Nordstrom<henrik@henriknordstrom.net>
> Signed-off-by: Stefan Roese<sr@denx.de>
> ---
> arch/arm/cpu/armv7/sunxi/clock.c | 15 ++
> arch/arm/include/asm/arch-sunxi/i2c.h | 185 ++++++++++++++++++++++
> drivers/i2c/Makefile | 1 +
> drivers/i2c/sunxi_i2c.c | 278 +++++++++++++++++++++++++++++++++
> include/configs/sunxi-common.h | 8 +
> 5 files changed, 487 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/include/asm/arch-sunxi/i2c.h
> create mode 100644 drivers/i2c/sunxi_i2c.c

One question: Did you looked at the i2c driver in
drivers/i2c/mvtwsi.c ? I just looked roughly over
it, there seems some differences, but there are
some equal defines, for example the TWI_CTL_*, TWI_STAT_*
defines, not too big difference in the register structure ...
maybe it is worth to check, if it is possible to extend the
existing driver to fit in your needs?

Beside of that, only some more nitpicking comments (beside of the
comments from Wolfgang, Marek, Henrik, Luka) ...

[...]
> diff --git a/arch/arm/include/asm/arch-sunxi/i2c.h b/arch/arm/include/asm/arch-sunxi/i2c.h
> new file mode 100644
> index 0000000..9a6e168
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-sunxi/i2c.h
> @@ -0,0 +1,185 @@
> +/*
> + * (C) Copyright 2012 Henrik Nordstrom<henrik@henriknordstrom.net>
> + *
> + * Based on sun4i linux kernle i2c.h
> + * (C) Copyright 2007-2012
> + * Allwinner Technology Co., Ltd.<www.allwinnertech.com>
> + * Tom Cubie<tanglaing@allwinnertech.com>
> + * Victor Wei<weiziheng@allwinnertech.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 _SUNXI_I2C_H_
> +#define _SUNXI_I2C_H_
> +
> +struct i2c {
> + u32 saddr; /* 31:8bit res,7-1bit for slave addr,0 bit for GCE */
> + u32 xsaddr; /* 31:8bit res,7-0bit for second addr in 10bit addr */
> + u32 data; /* 31:8bit res, 7-0bit send or receive data byte */
> + u32 ctl; /* INT_EN,BUS_EN,M_STA,INT_FLAG,A_ACK */
> + u32 status; /* 28 interrupt types + 0xF8 normal type = 29 */
> + u32 clkr; /* 31:7bit res,6-3bit,CLK_M,2-0bit CLK_N */
> + u32 reset; /* 31:1bit res;0bit,write 1 to clear 0. */
> + u32 efr; /* 31:2bit res,1:0 bit data byte follow read comand */
> + u32 lctl; /* 31:6bits res 5:0bit for sda&scl control */
> +};
> +
> +/* TWI address register */
> +#define TWI_GCE_EN (0x1<< 0) /* gen call addr enable slave mode */
^
space, please fix all places.

> +#define TWI_ADDR_MASK (0x7f<< 1) /* 7:1bits */
> +#define TWI_XADDR_MASK 0xff /* 7:0bits for extend slave address */
[...]
> +/*
> + * TWI Clock Register Bit Fields& Masks,default value:0x0000_0000
> + * Fin is APB CLOCK INPUT;
> + * Fsample = F0 = Fin/2^CLK_N;
> + * F1 = F0/(CLK_M+1);
> + *
> + * Foscl = F1/10 = Fin/(2^CLK_N * (CLK_M+1)*10);
> + * Foscl is clock SCL;standard mode:100KHz or fast mode:400KHz
> + */
> +
> +#define TWI_CLK_DIV_M (0xF<< 3) /* 6:3bit */
> +#define TWI_CLK_DIV_N (0x7<< 0) /* 2:0bit */
> +#define TWI_CLK_DIV(N, M) ((((N)& 0xF)<< 3) | (((M)& 0x7)<< 0))
^
here space too.

[...]
> diff --git a/drivers/i2c/sunxi_i2c.c b/drivers/i2c/sunxi_i2c.c
> new file mode 100644
> index 0000000..6bf5309
> --- /dev/null
> +++ b/drivers/i2c/sunxi_i2c.c
> @@ -0,0 +1,278 @@
> +/*
> + * Copyright (c) 2012 Henrik Nordstrom<henrik@henriknordstrom.net>
> + *
> + * 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
> + */
> +
> +#include<common.h>
> +#include<i2c.h>
> +#include<asm/io.h>
> +#include<asm/arch/cpu.h>
> +#include<asm/arch/i2c.h>
> +#include<asm/arch/gpio.h>
> +#include<asm/arch/clock.h>
> +
> +static struct i2c __attribute__ ((section(".data"))) *i2c_base =
> + (struct i2c *)0x1c2ac00;

please use a define.

> +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);

Hmm.. could we make this configurable?

> + writel(TWI_SRST_SRST,&i2c_base->reset);
> +
> + while ((readl(&i2c_base->reset)& TWI_SRST_SRST))
> + ;
> +}
> +
> +int i2c_probe(uchar chip)
> +{
> + return -1;
> +}
> +
> +static int i2c_wait_ctl(int mask, int state)
> +{
> + int timeout = 0x2ff;

Fix timeout, why?

> + 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)
> + ;
> +
> + 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);
^ ^
space
> +}
> +
> +static int i2c_wait_irq(void)
> +{
> + return i2c_wait_ctl(TWI_CTL_INTFLG, 1);
> +}
> +
> +static int i2c_wait_status(int status)
> +{
> + int timeout = 0x2ff;
> +
> + while (readl(&i2c_base->status) != status&& timeout--> 0)
> + ;
> +
> + if (timeout != 0)
> + return 0;
> + else
> + return -1;
> +}
> +
> +static int i2c_wait_irq_status(int status)
> +{
> + if (i2c_wait_irq() != 0)
> + return -1;
> +
> + if (readl(&i2c_base->status) != status)
> + return -1;
> +
> + return 0;
> +}
> +
> +static int i2c_wait_bus_idle(void)
> +{
> + int timeout = 0x2ff;
> +
> + while (readl(&i2c_base->lctl) != 0x3a&& timeout--> 0)
^
please use a define here
> + ;
> +
> + if (timeout != 0)
> + return 0;
> + else
> + return -1;
> +}
> +
> +static int i2c_stop(void)
> +{
> + u32 ctl;
> +
> + ctl = readl(&i2c_base->ctl)& 0xc0;
^
please use a define here, too
> + ctl |= TWI_CTL_STP;
> +
> + writel(ctl,&i2c_base->ctl);
> +
> + /* dummy to delay one I/O operation to make sure it's started */
> + (void)readl(&i2c_base->ctl);
> +
> + if (i2c_wait_ctl(TWI_CTL_STP, 0) != 0)
> + return -1;
> + if (i2c_wait_status(TWI_STAT_IDLE))
> + return -1;
> + if (i2c_wait_bus_idle() != 0)
> + return -1;
> +
> + return 0;
> +}
> +
> +static int i2c_send_data(u8 data, u8 status)
> +{
> + debug("i2c_write(%02x, %x), ctl=%x, status=%x\n", data, status,
> + i2c_base->ctl, i2c_base->status);
> +
> + writel(data,&i2c_base->data);
> + i2c_clear_irq();
> +
> + if (i2c_wait_irq_status(status) != 0)
> + return -1;
> +
> + return 0;
> +}
> +
> +static int i2c_start(int status)
> +{
> + u32 ctl;
> +
> + debug("i2c_start(%x), ctl=%x, status=%x\n", status, i2c_base->ctl,
> + i2c_base->status);
> + /* Check that the controller is idle */
> + if (status == TWI_STAT_TX_STA
> + && readl(&i2c_base->status) != TWI_STAT_IDLE) {
> + return -1;
> + }
> +
> + writel(0,&i2c_base->efr);
> +
> + /* Send start */
> + ctl = readl(&i2c_base->ctl);
> + ctl |= TWI_CTL_STA; /* Set start bit */
> + ctl&= ~TWI_CTL_INTFLG; /* Clear int flag */
^
space

> + writel(ctl,&i2c_base->ctl);
^
space
> +
> + if (i2c_wait_ctl(TWI_CTL_STA, 0) != 0)
> + return -1;
> + if (i2c_wait_irq_status(status) != 0)
> + return -1;
> +
> + return 0;
> +}

bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany