[i2c] [PATCH] Is review of AT91 patch pending?

[Jean Delvare]

On Thu, 18 Oct 2007 19:28:20 -0700, David Brownell wrote:
> Switch over to bitbanged GPIO for I2C.  The i2c-at91 controller
> is fairly broken (gets underruns and overruns easily under load),
> and its driver doesn't properly issue repeated START conditions
> even in the handful of cases where the hardware supports them.
> While the GPIO driver is slower, it doesn't have those issues.
> 
> Signed-off-by: David Brownell <dbrownell at users.sourceforge.net>
> ---
>  arch/arm/mach-at91/at91rm9200_devices.c |   40 ++++++++++++++++----------------
>  1 files changed, 20 insertions(+), 20 deletions(-)
> 

Just one thing:

> --- a/arch/arm/mach-at91/at91rm9200_devices.c
> +++ b/arch/arm/mach-at91/at91rm9200_devices.c
> @@ -14,6 +14,7 @@
>  #include <asm/mach/map.h>
>  
>  #include <linux/platform_device.h>
> +#include <linux/i2c-gpio.h>
>  
>  #include <asm/arch/board.h>
>  #include <asm/arch/gpio.h>
> @@ -432,38 +433,37 @@ void __init at91_add_device_nand(struct 
>  
>  
>  /* --------------------------------------------------------------------
> - *  TWI (i2c)
> + *  TWI (i2c) ... use the GPIO code since this TWI controller is not
> + *  robust (gets overruns and underruns easily under slight loads)
> + *  and currently doesn't understand how to issue repeated STARTs.
>   * -------------------------------------------------------------------- */
>  
> -#if defined(CONFIG_I2C_AT91) || defined(CONFIG_I2C_AT91_MODULE)
> +#if defined(CONFIG_I2C_GPIO) || defined(CONFIG_I2C_GPIO_MODULE)
>  
> -static struct resource twi_resources[] = {
> -	[0] = {
> -		.start	= AT91RM9200_BASE_TWI,
> -		.end	= AT91RM9200_BASE_TWI + SZ_16K - 1,
> -		.flags	= IORESOURCE_MEM,
> -	},
> -	[1] = {
> -		.start	= AT91RM9200_ID_TWI,
> -		.end	= AT91RM9200_ID_TWI,
> -		.flags	= IORESOURCE_IRQ,
> -	},
> +static struct i2c_gpio_platform_data pdata = {
> +	.sda_pin		= AT91_PIN_PA25,
> +	.sda_is_open_drain	= 1,
> +	.scl_pin		= AT91_PIN_PA26,
> +	.scl_is_open_drain	= 1,
> +	.udelay			= 2,		/* ~100 KHz */

The i2c-gpio documentation says: SCL frequency is (500 / udelay) kHz.
So that should be 250 kHz. Not sure if it's a good idea as not all I2C
slaves may support it. Maybe you should set udelay to 5.

Also note, the correct unit spelling is kHz, not KHz.

>  };
>  
>  static struct platform_device at91rm9200_twi_device = {
> -	.name		= "at91_i2c",
> -	.id		= -1,
> -	.resource	= twi_resources,
> -	.num_resources	= ARRAY_SIZE(twi_resources),
> +	.name			= "i2c-gpio",
> +	.id			= -1,
> +	.dev.platform_data	= &pdata,
>  };
>  
>  void __init at91_add_device_i2c(void)
>  {
> -	/* pins used for TWI interface */
> -	at91_set_A_periph(AT91_PIN_PA25, 0);		/* TWD */
> +	/* pins used for TWI interface -- set as GPIO */
> +
> +	/* TWD: at91_set_A_periph(AT91_PIN_PA25, 0); */
> +	at91_set_GPIO_periph(AT91_PIN_PA25, 1);
>  	at91_set_multi_drive(AT91_PIN_PA25, 1);
>  
> -	at91_set_A_periph(AT91_PIN_PA26, 0);		/* TWCK */
> +	/* TWCK: at91_set_A_periph(AT91_PIN_PA26, 0); */
> +	at91_set_GPIO_periph(AT91_PIN_PA26, 1);
>  	at91_set_multi_drive(AT91_PIN_PA26, 1);
>  
>  	platform_device_register(&at91rm9200_twi_device);

-- 
Jean Delvare