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

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 06/10/2014 08:33 AM, Heiko Schocher wrote:
> 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@redhat.com>
>> Acked-by: Ian Campbell<ijc@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

Ok, will do.

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

Yes, the sunxi spl uses the i2c bus to program the pmic
voltages before enabling dram.

> 
> [...]
>> 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?

I did a separate patch for that:
"[PATCH 1/5] mvtwsi: convert to CONFIG_SYS_I2C framework"

http://lists.denx.de/pipermail/u-boot/2014-June/181352.html

Which was also send directly to you.

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

Yes this is all done in the "[PATCH 1/5] mvtwsi: convert to CONFIG_SYS_I2C framework"
patch.

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

"[PATCH 1/5] mvtwsi: convert to CONFIG_SYS_I2C framework"

Renames CONFIG_I2C_MVTWSI to CONFIG_SYS_I2C_MVTWSI

> Do this really work?

With the above patch applied first, yes it does.

>> +#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

Regards,

Hans