[U-Boot] [PATCH 2/5] sunxi: Add i2c support

[Heiko Schocher]

Hello Hans,

Am 09.06.2014 17:15, schrieb Hans de Goede:
> Add support for the i2c controller found on all Allwinner sunxi SoCs,
> this is the same controller as found on the Marvell orion5x and kirkwood
> SoC families, with a slightly different register layout, so this patch uses
> the existing mvtwsi code.
>
> Signed-off-by: Hans de Goede<hdegoede at redhat.com>
> Acked-by: Ian Campbell<ijc at hellion.org.uk>
> ---

as this is a newer version, your commit subject should contain a "v2",
and please add here a short info, what has changed ... look here for
an explanation how this should look like:
http://www.denx.de/wiki/view/U-Boot/Patches#Sending_updated_patch_versions

>   arch/arm/cpu/armv7/sunxi/board.c        |  6 ++++++
>   arch/arm/cpu/armv7/sunxi/u-boot-spl.lds |  5 +++++
>   arch/arm/include/asm/arch-sunxi/i2c.h   | 15 +++++++++++++++
>   board/sunxi/board.c                     |  7 +++++++
>   drivers/i2c/mvtwsi.c                    | 18 ++++++++++++++++++
>   include/configs/sunxi-common.h          |  8 ++++++++
>   6 files changed, 59 insertions(+)
>   create mode 100644 arch/arm/include/asm/arch-sunxi/i2c.h
>
> diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c
> index 1e506b5..024c8c1 100644
> --- a/arch/arm/cpu/armv7/sunxi/board.c
> +++ b/arch/arm/cpu/armv7/sunxi/board.c
> @@ -11,6 +11,7 @@
>    */
>
>   #include<common.h>
> +#include<i2c.h>
>   #include<netdev.h>
>   #include<miiphy.h>
>   #include<serial.h>
> @@ -91,11 +92,16 @@ void s_init(void)
>   	clock_init();
>   	timer_init();
>   	gpio_init();
> +	i2c_init_board();
>
>   #ifdef CONFIG_SPL_BUILD
>   	gd =&gdata;
>   	preloader_console_init();
>
> +#ifdef CONFIG_SPL_I2C_SUPPORT
> +	/* Needed early by sunxi_board_init if PMU is enabled */
> +	i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
> +#endif
>   	sunxi_board_init();
>   #endif
>   }
> diff --git a/arch/arm/cpu/armv7/sunxi/u-boot-spl.lds b/arch/arm/cpu/armv7/sunxi/u-boot-spl.lds
> index c1ae227..53f0cbd 100644
> --- a/arch/arm/cpu/armv7/sunxi/u-boot-spl.lds
> +++ b/arch/arm/cpu/armv7/sunxi/u-boot-spl.lds
> @@ -39,6 +39,11 @@ SECTIONS
>   	.data : { *(SORT_BY_ALIGNMENT(.data*)) }>.sram
>
>   	. = ALIGN(4);
> +	.u_boot_list : {
> +		KEEP(*(SORT(.u_boot_list*)));
> +	}>  .sram
> +
> +	. = ALIGN(4);
>   	__image_copy_end = .;
>   	_end = .;
>

Do you really need this change?

[...]
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index b05d0b9..543b809 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -106,6 +106,13 @@ int board_mmc_init(bd_t *bis)
>   }
>   #endif
>
> +void i2c_init_board(void)
> +{
> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(0), SUNXI_GPB0_TWI0);
> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(1), SUNXI_GPB0_TWI0);
> +	clock_twi_onoff(0, 1);
> +}
> +
>   #ifdef CONFIG_SPL_BUILD
>   void sunxi_board_init(void)
>   {

I accept this here, but i2c_init_board() hysterically intention was
to deblock the bus ... I think we need here another approach ...
board specific initialization should be called from i2c_driver->init() when
the bus is really used ... (and if not used longer a i2c_driver->deinit()
should be called, so for example the pins could be used as gpio ... but
this is a step, I would introduce, if all i2c drivers use the new
framework ... or we have switched complete to the DM approach)

> diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
> index d3457b9..ad1ce8f 100644
> --- a/drivers/i2c/mvtwsi.c
> +++ b/drivers/i2c/mvtwsi.c
> @@ -22,6 +22,8 @@
>   #include<asm/arch/orion5x.h>
>   #elif defined(CONFIG_KIRKWOOD)
>   #include<asm/arch/kirkwood.h>
> +#elif defined(CONFIG_SUNXI)
> +#include<asm/arch/i2c.h>
>   #else
>   #error Driver mvtwsi not supported by SoC or board
>   #endif
> @@ -30,6 +32,20 @@
>    * TWSI register structure
>    */
>
> +#ifdef CONFIG_SUNXI
> +
> +struct  mvtwsi_registers {
> +	u32 slave_address;
> +	u32 xtnd_slave_addr;
> +	u32 data;
> +	u32 control;
> +	u32 status;
> +	u32 baudrate;
> +	u32 soft_reset;
> +};
> +
> +#else
> +
>   struct  mvtwsi_registers {
>   	u32 slave_address;
>   	u32 data;
> @@ -43,6 +59,8 @@ struct  mvtwsi_registers {
>   	u32 soft_reset;
>   };
>
> +#endif
> +
>   /*
>    * Control register fields
>    */

? Thats all? I thought you convert this driver to the new i2c framework?

At least, there should be a change from the i2c_xx() functions to static
and a prefix add to them ... and a "U_BOOT_I2C_ADAP_COMPLETE" at the end ...

> diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
> index 3f04890..42b0d2e 100644
> --- a/include/configs/sunxi-common.h
> +++ b/include/configs/sunxi-common.h
> @@ -161,6 +161,14 @@
>   #undef CONFIG_CMD_NET
>   #undef CONFIG_CMD_NFS
>
> +/* I2C */
> +#define CONFIG_SPL_I2C_SUPPORT
> +#define CONFIG_SYS_I2C
> +#define CONFIG_SYS_I2C_MVTWSI

This define is not in current ML, nor in your patch ... ?

Do this really work?

> +#define CONFIG_SYS_I2C_SPEED		400000
> +#define CONFIG_SYS_I2C_SLAVE		0x7f
> +#define CONFIG_CMD_I2C
> +
>   #ifndef CONFIG_CONS_INDEX
>   #define CONFIG_CONS_INDEX              1       /* UART0 */
>   #endif

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