[U-Boot] [PATCH 12/13] at91: add hardware i2c drivers

Heiko Schocher
2009-03-30T08:26:06Z

Hello Jean-Christophe,

Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 07:53 Mon 30 Mar , Heiko Schocher wrote:
>> Hello Jean,
>>
>> Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>>> ---
>>> cpu/arm926ejs/at91/Makefile | 6 +
>>> .../arm926ejs/at91/at91cap9_i2c.c | 31 ++--
>>> .../arm926ejs/at91/at91sam9260_i2c.c | 31 ++--
>>> .../arm926ejs/at91/at91sam9261_i2c.c | 31 ++--
>>> .../arm926ejs/at91/at91sam9263_i2c.c | 31 ++--
>>> .../clk.h => cpu/arm926ejs/at91/at91sam9rl_i2c.c | 37 ++--
>>> drivers/i2c/Makefile | 1 +
>>> drivers/i2c/atmel_i2c.c | 234 ++++++++++++++++++++
>>> include/asm-arm/arch-at91/at91_common.h | 2 +
>>> include/asm-arm/arch-at91/at91_twi.h | 68 ++++++
>>> include/asm-arm/arch-at91/clk.h | 4 +
>>> include/asm-arm/arch-at91/hardware.h | 5 +
>>> 12 files changed, 405 insertions(+), 76 deletions(-)
>>> copy include/asm-arm/arch-at91/at91_common.h => cpu/arm926ejs/at91/at91cap9_i2c.c (61%)
>>> copy include/asm-arm/arch-at91/at91_common.h => cpu/arm926ejs/at91/at91sam9260_i2c.c (61%)
>>> copy include/asm-arm/arch-at91/at91_common.h => cpu/arm926ejs/at91/at91sam9261_i2c.c (61%)
>>> copy include/asm-arm/arch-at91/at91_common.h => cpu/arm926ejs/at91/at91sam9263_i2c.c (61%)
>>> copy include/asm-arm/arch-at91/clk.h => cpu/arm926ejs/at91/at91sam9rl_i2c.c (52%)
>>> create mode 100644 drivers/i2c/atmel_i2c.c
>>> create mode 100644 include/asm-arm/arch-at91/at91_twi.h
>>>
>>>
>> [...]
>>> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
>>> index 9c74657..27a5251 100644
>>> --- a/drivers/i2c/Makefile
>>> +++ b/drivers/i2c/Makefile
>>> @@ -25,6 +25,7 @@ include $(TOPDIR)/config.mk
>>>
>>> LIB := $(obj)libi2c.a
>>>
>>> +COBJS-$(CONFIG_I2C_ATMEL) += atmel_i2c.o
>>> COBJS-$(CONFIG_BFIN_TWI_I2C) += bfin-twi_i2c.o
>>> COBJS-$(CONFIG_FSL_I2C) += fsl_i2c.o
>>> COBJS-$(CONFIG_I2C_MXC) += mxc_i2c.o
>>>
>> Independent from your patch, I think we need here a common
>> rule. Either we name in future this define "CONFIG_I2C_XXX"
>> or "CONFIG_XXX_I2C. I vote for using "CONFIG_I2C_XXX"
> it's plan to clean up the driver CONFIG_ just before send the Kconfig patch
> series

Ah, ok, thanks.

>>> + *
>>> +*/
>>> +#include <common.h>
>>> +
>>> +#ifdef CONFIG_HARD_I2C
>>>
>> Why not "#if defined(CONFIG_I2C_ATMEL)" as in the Makefile?
> until the Kconfig I'll prefer to keep the CONFIG_HARD_I2C

ok.

>>> +
>>> +#include <i2c.h>
>>> +#include <asm/io.h>
>>>
>> [...]
>>> + if (alen > 1)
>>> + return 1;
>>> + /* XXX assume an ATMEL AT24C16 */

[...]

>>> diff --git a/include/asm-arm/arch-at91/at91_twi.h b/include/asm-arm/arch-at91/at91_twi.h
>>> new file mode 100644
>>> index 0000000..bb2880f
>>> --- /dev/null
>>> +++ b/include/asm-arm/arch-at91/at91_twi.h
>>> @@ -0,0 +1,68 @@
>>> +/*
>>> + * arch/arm/mach-at91/include/mach/at91_twi.h
>>> + *
>>> + * Copyright (C) 2005 Ivan Kokshaysky
>>> + * Copyright (C) SAN People
>>> + *
>>> + * Two-wire Interface (TWI) registers.
>>> + * Based on AT91RM9200 datasheet revision E.
>>> + *
>>> + * 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.
>>> + */
>>> +
>>> +#ifndef AT91_TWI_H
>>> +#define AT91_TWI_H
>>> +
>>> +#define AT91_TWI_CR 0x00 /* Control Register */
>>> +#define AT91_TWI_START (1 << 0) /* Send a Start Condition */
>>> +#define AT91_TWI_STOP (1 << 1) /* Send a Stop Condition */
>>> +#define AT91_TWI_MSEN (1 << 2) /* Master Transfer Enable */
>>> +#define AT91_TWI_MSDIS (1 << 3) /* Master Transfer Disable */
>>> +#define AT91_TWI_SVEN (1 << 4) /* Slave Transfer Enable [SAM9260 only] */
>>> +#define AT91_TWI_SVDIS (1 << 5) /* Slave Transfer Disable [SAM9260 only] */
>>> +#define AT91_TWI_SWRST (1 << 7) /* Software Reset */
>>>
>> ^^^^^^^^^^^^^^^
>> doule tabs breaks line length (this affects for the
>> whole file)
>>
> I'll prefer to keep it as it because it's an import from Linux

Hmm.. maybe this should be fixed in Linux too.

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