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

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

Hi Henrik,

On Sun, Nov 25, 2012 at 12:38:46PM +0100, Henrik Nordström wrote:
> 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
> 
> diff --git a/arch/arm/cpu/armv7/sunxi/clock.c b/arch/arm/cpu/armv7/sunxi/clock.c
> index 424acfc..b9bbb7d 100644
> --- a/arch/arm/cpu/armv7/sunxi/clock.c
> +++ b/arch/arm/cpu/armv7/sunxi/clock.c
> @@ -42,6 +42,7 @@ int clock_init(void)
>  	return 0;
>  }
>  
> +

No need to add extra newline here.

>  /* Return PLL5 frequency in Hz
>   * Note: Assumes PLL5 reference is 24MHz clock
>   */
> @@ -55,3 +56,17 @@ unsigned int clock_get_pll5(void)
>  	int p = 1 << ((rval >> 16) & 3);
>  	return 24000000 * n * k / p;
>  }
> +
> +int clock_twi_onoff(int port, int state)
> +{
> +	struct sunxi_ccm_reg *const ccm =
> +		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
> +
> +	if (port > 2)
> +		return -1;
> +
> +	/* set the apb1 clock gate for twi */
> +	sr32(&ccm->apb1_gate, 0 + port, 1, state);
> +
> +	return 0;
> +}
> 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
> + */

Add one newline here like in all other .h files.

> +#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 */
> +#define TWI_ADDR_MASK	(0x7f << 1)	/* 7:1bits */
> +#define TWI_XADDR_MASK	0xff		/* 7:0bits for extend slave address */
> +
> +#define TWI_DATA_MASK	0xff		/* 7:0bits for send or received */
> +
> +/* TWI Control Register Bit Fields */
> +/* 1:0 bits reserved */
> +/* set 1 to send A_ACK,then low level on SDA */
> +#define TWI_CTL_ACK	(0x1 << 2)
> +/* INT_FLAG,interrupt status flag: set '1' when interrupt coming */
> +#define TWI_CTL_INTFLG	(0x1 << 3)
> +#define TWI_CTL_STP	(0x1 << 4)	/* M_STP,Automatic clear 0 */
> +#define TWI_CTL_STA	(0x1 << 5)	/* M_STA,atutomatic clear 0 */
> +#define TWI_CTL_BUSEN	(0x1 << 6)	/* BUS_EN, mastr mode should be set 1 */
> +#define TWI_CTL_INTEN	(0x1 << 7)	/* INT_EN */
> +/* 31:8 bit reserved */
> +
> +/*
> + * 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))
> +
> +/* TWI Soft Reset Register Bit Fields & Masks  */
> +/* write 1 to clear 0, when complete soft reset clear 0 */
> +#define TWI_SRST_SRST		(0x1 << 0)
> +
> +/* TWI Enhance Feature Register Bit Fields & Masks  */
> +/* default -- 0x0 */
> +/* 00:no,01: 1byte, 10:2 bytes, 11: 3bytes */
> +#define TWI_EFR_MASK		(0x3 << 0)
> +#define TWI_EFR_WARC_0		(0x0 << 0)
> +#define TWI_EFR_WARC_1		(0x1 << 0)
> +#define TWI_EFR_WARC_2		(0x2 << 0)
> +#define TWI_EFR_WARC_3		(0x3 << 0)
> +
> +/* twi line control register -default value: 0x0000_003a */
> +/* SDA line state control enable ,1:enable;0:disable */
> +#define TWI_LCR_SDA_EN		(0x01 << 0)
> +/* SDA line state control bit, 1:high level;0:low level */
> +#define TWI_LCR_SDA_CTL		(0x01 << 1)
> +/* SCL line state control enable ,1:enable;0:disable */
> +#define TWI_LCR_SCL_EN		(0x01 << 2)
> +/* SCL line state control bit, 1:high level;0:low level */
> +#define TWI_LCR_SCL_CTL		(0x01 << 3)
> +/* current state of SDA,readonly bit */
> +#define TWI_LCR_SDA_STATE_MASK	(0x01 << 4)
> +/* current state of SCL,readonly bit */
> +#define TWI_LCR_SCL_STATE_MASK	(0x01 << 5)
> +/* 31:6bits reserved */
> +#define TWI_LCR_IDLE_STATUS	0x3a
> +
> +/* TWI Status Register Bit Fields & Masks  */
> +#define TWI_STAT_MASK		0xff
> +/* 7:0 bits use only,default is 0xF8 */
> +#define TWI_STAT_BUS_ERR	0x00	/* BUS ERROR */
> +
> +/* Master mode use only */
> +#define TWI_STAT_TX_STA		0x08	/* START condition transmitted */
> +/* Repeated START condition transmitted */
> +#define TWI_STAT_TX_RESTA	0x10
> +/* Address+Write bit transmitted, ACK received */
> +#define TWI_STAT_TX_AW_ACK	0x18
> +/* Address+Write bit transmitted, ACK not received */
> +#define TWI_STAT_TX_AW_NAK	0x20
> +/* data byte transmitted in master mode,ack received */
> +#define TWI_STAT_TXD_ACK	0x28
> +/* data byte transmitted in master mode ,ack not received */
> +#define TWI_STAT_TXD_NAK	0x30
> +/* arbitration lost in address or data byte */
> +#define TWI_STAT_ARBLOST	0x38
> +/* Address+Read bit transmitted, ACK received */
> +#define TWI_STAT_TX_AR_ACK	0x40
> +/* Address+Read bit transmitted, ACK not received */
> +#define TWI_STAT_TX_AR_NAK	0x48
> +/* Second Address byte + Write bit transmitted, ACK received */
> +#define TWI_STAT_TX_2AW_ACK	0xD0
> +/* Second Address byte + Write bit transmitted, ACK received */
> +#define TWI_STAT_TX_2AW_NAK	0xD8
> +/* data byte received in master mode ,ack transmitted */
> +#define TWI_STAT_RXD_ACK	0x50
> +/* date byte received in master mode,not ack transmitted */
> +#define TWI_STAT_RXD_NAK	0x58
> +
> +/* Slave mode use only */
> +/* Slave address+Write bit received, ACK transmitted */
> +#define TWI_STAT_RXWS_ACK	0x60
> +/*
> + * Arbitration lost in address as master, slave address + Write bit received,
> + * ACK transmitted
> + */
> +#define TWI_STAT_ARBLOST_RXWS_ACK 0x68
> +/* General Call address received, ACK transmitted */
> +#define TWI_STAT_RXGCAS_ACK	0x70
> +/*
> + * Arbitration lost in address as master, General Call address received,
> + * ACK transmitted
> + */
> +#define TWI_STAT_ARBLOST_RXGCAS_ACK 0x78
> +/* Data byte received after slave address received, ACK transmitted */
> +#define TWI_STAT_RXDS_ACK	0x80
> +/* Data byte received after slave address received, not ACK transmitted */
> +#define TWI_STAT_RXDS_NAK	0x88
> +/* Data byte received after General Call received, ACK transmitted */
> +#define TWI_STAT_RXDGCAS_ACK	0x90
> +/* Data byte received after General Call received, not ACK transmitted */
> +#define TWI_STAT_RXDGCAS_NAK	0x98
> +/* STOP or repeated START condition received in slave  */
> +#define TWI_STAT_RXSTPS_RXRESTAS 0xA0
> +/* Slave address + Read bit received, ACK transmitted */
> +#define TWI_STAT_RXRS_ACK	0xA8
> +/*
> + * Arbitration lost in address as master, slave address + Read bit received,
> + * ACK transmitted
> + */
> +#define TWI_STAT_ARBLOST_SLAR_ACK 0xB0
> +/* Data byte transmitted in slave mode, ACK received */
> +#define TWI_STAT_TXDS_ACK	0xB8
> +/* Data byte transmitted in slave mode, ACK not received */
> +#define TWI_STAT_TXDS_NAK	0xC0
> +/* Last byte transmitted in slave mode, ACK received */
> +#define TWI_STAT_TXDSL_ACK	0xC8
> +
> +/* 10bit Address, second part of address */
> +/* Second Address byte+Write bit transmitted,ACK received */
> +#define TWI_STAT_TX_SAW_ACK	0xD0
> +/* Second Address byte+Write bit transmitted,ACK not received */
> +#define TWI_STAT_TX_SAW_NAK	0xD8
> +
> +/* No relevant status infomation,INT_FLAG = 0 */
> +#define TWI_STAT_IDLE		0xF8
> +
> +#endif
> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
> index 5dbdbe3..9f929e6 100644
> --- a/drivers/i2c/Makefile
> +++ b/drivers/i2c/Makefile
> @@ -46,6 +46,7 @@ COBJS-$(CONFIG_TSI108_I2C) += tsi108_i2c.o
>  COBJS-$(CONFIG_U8500_I2C) += u8500_i2c.o
>  COBJS-$(CONFIG_SH_I2C) += sh_i2c.o
>  COBJS-$(CONFIG_SH_SH7734_I2C) += sh_sh7734_i2c.o
> +COBJS-$(CONFIG_SUNXI_I2C) += sunxi_i2c.o
>  
>  COBJS	:= $(COBJS-y)
>  SRCS	:= $(COBJS:.o=.c)
> 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;
> +
> +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)
> +{
> +	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)
> +		;
> +
> +	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);
> +}
> +
> +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)
> +		;
> +
> +	if (timeout != 0)
> +		return 0;
> +	else
> +		return -1;
> +}
> +
> +static int i2c_stop(void)
> +{
> +	u32 ctl;
> +
> +	ctl = readl(&i2c_base->ctl) & 0xc0;
> +	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 */
> +	writel(ctl, &i2c_base->ctl);
> +
> +	if (i2c_wait_ctl(TWI_CTL_STA, 0) != 0)
> +		return -1;
> +	if (i2c_wait_irq_status(status) != 0)
> +		return -1;
> +
> +	return 0;
> +}
> +
> +int i2c_do_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
> +{
> +	u32 status;
> +	u32 ctl;
> +
> +	if (i2c_start(TWI_STAT_TX_STA) != 0)
> +		return -1;
> +
> +	/* Send chip address */
> +	if (i2c_send_data(chip << 1 | 0, TWI_STAT_TX_AW_ACK) != 0)
> +		return -1;
> +
> +	/* Send data address */
> +	if (i2c_send_data(addr, TWI_STAT_TXD_ACK) != 0)
> +		return -1;
> +
> +	/* Send restart for read */
> +	if (i2c_start(TWI_STAT_TX_RESTA) != 0)
> +		return -1;
> +
> +	/* Send chip address */
> +	if (i2c_send_data(chip << 1 | 1, TWI_STAT_TX_AR_ACK) != 0)
> +		return -1;
> +
> +	/* Set ACK mode */
> +	ctl = readl(&i2c_base->ctl);
> +	ctl |= TWI_CTL_ACK;
> +	writel(ctl, &i2c_base->ctl);
> +	status = TWI_STAT_RXD_ACK;
> +
> +	/* Read data */
> +	while (len > 0) {
> +		if (len == 1) {
> +			/* Set NACK mode (last byte) */
> +			ctl = readl(&i2c_base->ctl);
> +			ctl &= ~TWI_CTL_ACK;
> +			writel(ctl, &i2c_base->ctl);
> +			status = TWI_STAT_RXD_NAK;
> +		}
> +
> +		i2c_clear_irq();
> +		if (i2c_wait_irq_status(status) != 0)
> +			return -1;
> +
> +		*buffer++ = readl(&i2c_base->data);
> +		len--;
> +	}
> +
> +	return 0;
> +}
> +
> +int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
> +{
> +	int rc = i2c_do_read(chip, addr, alen, buffer, len);
> +
> +	i2c_stop();
> +
> +	return rc;
> +}
> +
> +static int i2c_do_write(uchar chip, uint addr, int alen, uchar *buffer,
> +			int len)
> +{
> +	if (i2c_start(TWI_STAT_TX_STA) != 0)
> +		return -1;
> +
> +	/* Send chip address */
> +	if (i2c_send_data(chip << 1 | 0, TWI_STAT_TX_AW_ACK) != 0)
> +		return -1;
> +
> +	/* Send data address */
> +	if (i2c_send_data(addr, TWI_STAT_TXD_ACK) != 0)
> +		return -1;
> +
> +	/* Send data */
> +	while (len > 0) {
> +		if (i2c_send_data(*buffer++, TWI_STAT_TXD_ACK) != 0)
> +			return -1;
> +		len--;
> +	}
> +
> +	return 0;
> +}
> +
> +int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
> +{
> +	int rc = i2c_do_write(chip, addr, alen, buffer, len);
> +
> +	i2c_stop();
> +
> +	return rc;
> +}
> diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
> index 8a026e0..c2d16fb 100644
> --- a/include/configs/sunxi-common.h
> +++ b/include/configs/sunxi-common.h
> @@ -194,4 +194,12 @@
>  #undef CONFIG_CMD_NET
>  #undef CONFIG_CMD_NFS
>  
> +/* I2C */
> +#define CONFIG_SPL_I2C_SUPPORT
> +#define CONFIG_SYS_I2C_SPEED		400000
> +#define CONFIG_HARD_I2C
> +#define CONFIG_SUNXI_I2C
> +#define CONFIG_SYS_I2C_SLAVE		0x7f
> +#define CONFIG_CMD_I2C

Why don't you do it like this:

#ifdef CONFIG_CMD_I2C
#define CONFIG_SPL_I2C_SUPPORT
#define CONFIG_SYS_I2C_SPEED		400000
#define CONFIG_HARD_I2C
#define CONFIG_SUNXI_I2C
#define CONFIG_SYS_I2C_SLAVE		0x7f
#endif /* CONFIG_CMD_I2C */

That way you can simply turn on i2c support in board configuration file.
If users don't want/need i2c they don't need to use it.

> +
>  #endif /* __CONFIG_H */
> -- 
> 1.7.7.6

Luka

sön 2012-11-25 klockan 15:41 +0100 skrev Luka Perkov:

> Why don't you do it like this:
> 
> #ifdef CONFIG_CMD_I2C
> #define CONFIG_SPL_I2C_SUPPORT
> #define CONFIG_SYS_I2C_SPEED		400000
> #define CONFIG_HARD_I2C
> #define CONFIG_SUNXI_I2C
> #define CONFIG_SYS_I2C_SLAVE		0x7f
> #endif /* CONFIG_CMD_I2C */
> 
> That way you can simply turn on i2c support in board configuration file.
> If users don't want/need i2c they don't need to use it.

SPL I2C support is orthogonal to CMD_I2C.

SPL needs I2C support for PMU control to configure the right CPU core
voltage and some other parameters.

Right now we only have one board which do not really need I2C driver in
SPL, all the others always need I2C driver enabled in SPL.

And we don't really have any board specific config files at the moment.
sunxi-common.h is the shared-by-all board config file, with sun4i.h and
sun5i.h being SoC generation dependent. The known boards are all so
similar that they can use the same main u-boot binary, almost even in
both SoC generations. 

SPL do differ noticeably in parameters from board to board.

Regards
Henrik

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()

> +}
> +
[...]

Dear Henrik Nordström,

In message <1353843526.17518.15.camel@home.hno.se> you wrote:
> 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>

???

> +static struct i2c __attribute__ ((section(".data"))) *i2c_base =
> +	(struct i2c *)0x1c2ac00;

Why would the __attribute__ ((section(".data"))) be needed here???

> +	while ((readl(&i2c_base->reset) & TWI_SRST_SRST))
> +		;

No timeout?

Best regards,

Wolfgang Denk

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

mån 2012-11-26 klockan 12:13 +0100 skrev Heiko Schocher:

> 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?

I hadnät noticed, but indeed there is many similarities, and some
differences. The two controllers are clearly of related origin.


Regards
Henrik