diff mbox

[U-Boot,2/2] sunxi: Complete i2c support for each supported platform

Message ID 1428146826-24078-3-git-send-email-contact@paulk.fr
State Superseded
Headers show

Commit Message

Paul Kocialkowski April 4, 2015, 11:27 a.m. UTC
Sunxi platforms come with at least 3 TWI (I2C) controllers and some platforms
even have up to 5. This adds support for every controller on each supported
platform, which is especially useful when using expansion ports on single-board-
computers.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 arch/arm/include/asm/arch-sunxi/cpu_sun4i.h |  7 +++
 arch/arm/include/asm/arch-sunxi/gpio.h      | 15 ++++++-
 arch/arm/include/asm/arch-sunxi/i2c.h       |  9 ++++
 board/sunxi/board.c                         | 67 ++++++++++++++++++++++++++++-
 4 files changed, 95 insertions(+), 3 deletions(-)

Comments

Hans de Goede April 4, 2015, 12:18 p.m. UTC | #1
Hi,

On 04-04-15 13:27, Paul Kocialkowski wrote:
> Sunxi platforms come with at least 3 TWI (I2C) controllers and some platforms
> even have up to 5. This adds support for every controller on each supported
> platform, which is especially useful when using expansion ports on single-board-
> computers.
>
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>

Looks good in general, see comments inline for some minor things which need
fixing.

> ---
>   arch/arm/include/asm/arch-sunxi/cpu_sun4i.h |  7 +++
>   arch/arm/include/asm/arch-sunxi/gpio.h      | 15 ++++++-
>   arch/arm/include/asm/arch-sunxi/i2c.h       |  9 ++++
>   board/sunxi/board.c                         | 67 ++++++++++++++++++++++++++++-
>   4 files changed, 95 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h b/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
> index dae6069..f403742 100644
> --- a/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
> +++ b/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
> @@ -94,6 +94,13 @@
>   #define SUNXI_TWI0_BASE			0x01c2ac00
>   #define SUNXI_TWI1_BASE			0x01c2b000
>   #define SUNXI_TWI2_BASE			0x01c2b400
> +#ifdef CONFIG_MACH_SUN6I
> +#define SUNXI_TWI3_BASE			0x01c0b800
> +#endif
> +#ifdef CONFIG_MACH_SUN7I
> +#define SUNXI_TWI3_BASE			0x01c2b800
> +#define SUNXI_TWI4_BASE			0x01c2c000
> +#endif
>
>   #define SUNXI_CAN_BASE			0x01c2bc00
>
> diff --git a/arch/arm/include/asm/arch-sunxi/gpio.h b/arch/arm/include/asm/arch-sunxi/gpio.h
> index f227044..ae7cbb7 100644
> --- a/arch/arm/include/asm/arch-sunxi/gpio.h
> +++ b/arch/arm/include/asm/arch-sunxi/gpio.h
> @@ -148,7 +148,11 @@ enum sunxi_gpio_number {
>   #define SUN6I_GPA_SDC2		5
>   #define SUN6I_GPA_SDC3		4
>
> -#define SUNXI_GPB_TWI0		2
> +#define SUN4I_GPB_TWI0		2
> +#define SUN4I_GPB_TWI1		2
> +#define SUN5I_GPB_TWI1		2
> +#define SUN4I_GPB_TWI2		2
> +#define SUN5I_GPB_TWI2		2
>   #define SUN4I_GPB_UART0		2
>   #define SUN5I_GPB_UART0		2
>
> @@ -160,6 +164,7 @@ enum sunxi_gpio_number {
>   #define SUNXI_GPD_LVDS0		3
>
>   #define SUN5I_GPE_SDC2		3
> +#define SUN8I_GPE_TWI2		3
>
>   #define SUNXI_GPF_SDC0		2
>   #define SUNXI_GPF_UART0		4
> @@ -169,12 +174,20 @@ enum sunxi_gpio_number {
>   #define SUN5I_GPG_SDC1		2
>   #define SUN6I_GPG_SDC1		2
>   #define SUN8I_GPG_SDC1		2
> +#define SUN6I_GPG_TWI3		2
>   #define SUN5I_GPG_UART1		4
>
>   #define SUN4I_GPH_SDC1		5
> +#define SUN6I_GPH_TWI0		2
> +#define SUN8I_GPH_TWI0		2
> +#define SUN6I_GPH_TWI1		2
> +#define SUN8I_GPH_TWI1		2
> +#define SUN6I_GPH_TWI2		2
>   #define SUN6I_GPH_UART0		2
>
>   #define SUNXI_GPI_SDC3		2
> +#define SUN7I_GPI_TWI3		3
> +#define SUN7I_GPI_TWI4		3
>
>   #define SUN6I_GPL0_R_P2WI_SCK	3
>   #define SUN6I_GPL1_R_P2WI_SDA	3
> diff --git a/arch/arm/include/asm/arch-sunxi/i2c.h b/arch/arm/include/asm/arch-sunxi/i2c.h
> index 502e3c6..5bec18c 100644
> --- a/arch/arm/include/asm/arch-sunxi/i2c.h
> +++ b/arch/arm/include/asm/arch-sunxi/i2c.h
> @@ -9,6 +9,15 @@
>   #include <asm/arch/cpu.h>
>
>   #define CONFIG_I2C_MVTWSI_BASE0	SUNXI_TWI0_BASE
> +#define CONFIG_I2C_MVTWSI_BASE1	SUNXI_TWI1_BASE
> +#define CONFIG_I2C_MVTWSI_BASE2	SUNXI_TWI2_BASE
> +#ifdef SUNXI_TWI3_BASE
> +#define CONFIG_I2C_MVTWSI_BASE3	SUNXI_TWI3_BASE
> +#endif
> +#ifdef SUNXI_TWI4_BASE
> +#define CONFIG_I2C_MVTWSI_BASE4	SUNXI_TWI4_BASE
> +#endif
> +
>   /* This is abp0-clk on sun4i/5i/7i / abp1-clk on sun6i/sun8i which is 24MHz */
>   #define CONFIG_SYS_TCLK		24000000
>

This will cause us to register all possible i2c controllers for
each soc, but they might very well not be hooked up to anything,
and the pins of the SoC the code below will now mux to i2c
may even be used for something else, so this needs to be
activated by a Kconfig option (one per i2c controller).

> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index 7633d65..b3eecbb 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -276,9 +276,72 @@ int board_mmc_init(bd_t *bis)
>
>   void i2c_init_board(void)
>   {
> -	sunxi_gpio_set_cfgpin(SUNXI_GPB(0), SUNXI_GPB_TWI0);
> -	sunxi_gpio_set_cfgpin(SUNXI_GPB(1), SUNXI_GPB_TWI0);
> +#if defined(CONFIG_MACH_SUN4I) || defined(CONFIG_MACH_SUN5I) || defined(CONFIG_MACH_SUN7I)
> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(0), SUN4I_GPB_TWI0);
> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(1), SUN4I_GPB_TWI0);
>   	clock_twi_onoff(0, 1);
> +#elif defined(CONFIG_MACH_SUN6I)
> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(14), SUN6I_GPH_TWI0);
> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(15), SUN6I_GPH_TWI0);
> +	clock_twi_onoff(0, 1);
> +#elif defined(CONFIG_MACH_SUN8I)
> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(2), SUN8I_GPH_TWI0);
> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(3), SUN8I_GPH_TWI0);
> +	clock_twi_onoff(0, 1);
> +#endif
> +
> +#if defined(CONFIG_MACH_SUN4I) || defined(CONFIG_MACH_SUN7I)
> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(18), SUN4I_GPB_TWI1);
> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(19), SUN4I_GPB_TWI1);
> +	clock_twi_onoff(1, 1);
> +#elif defined(CONFIG_MACH_SUN5I)
> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(15), SUN5I_GPB_TWI1);
> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(16), SUN5I_GPB_TWI1);
> +	clock_twi_onoff(1, 1);
> +#elif defined(CONFIG_MACH_SUN6I)
> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(16), SUN6I_GPH_TWI1);
> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(17), SUN6I_GPH_TWI1);
> +	clock_twi_onoff(1, 1);
> +#elif defined(CONFIG_MACH_SUN8I)
> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(4), SUN8I_GPH_TWI1);
> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(5), SUN8I_GPH_TWI1);
> +	clock_twi_onoff(1, 1);
> +#endif
> +
> +#if defined(CONFIG_MACH_SUN4I) || defined(CONFIG_MACH_SUN7I)
> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(20), SUN4I_GPB_TWI2);
> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(21), SUN4I_GPB_TWI2);
> +	clock_twi_onoff(2, 1);
> +#elif defined(CONFIG_MACH_SUN5I)
> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(17), SUN5I_GPB_TWI2);
> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(18), SUN5I_GPB_TWI2);
> +	clock_twi_onoff(2, 1);
> +#elif defined(CONFIG_MACH_SUN6I)
> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(18), SUN6I_GPH_TWI2);
> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(19), SUN6I_GPH_TWI2);
> +	clock_twi_onoff(2, 1);
> +#elif defined(CONFIG_MACH_SUN8I)
> +	sunxi_gpio_set_cfgpin(SUNXI_GPE(12), SUN8I_GPE_TWI2);
> +	sunxi_gpio_set_cfgpin(SUNXI_GPE(13), SUN8I_GPE_TWI2);
> +	clock_twi_onoff(2, 1);
> +#endif
> +
> +#if defined(CONFIG_MACH_SUN6I)
> +	sunxi_gpio_set_cfgpin(SUNXI_GPG(10), SUN6I_GPG_TWI3);
> +	sunxi_gpio_set_cfgpin(SUNXI_GPG(11), SUN6I_GPG_TWI3);
> +	clock_twi_onoff(3, 1);
> +#elif defined(CONFIG_MACH_SUN7I)
> +	sunxi_gpio_set_cfgpin(SUNXI_GPI(0), SUN7I_GPI_TWI3);
> +	sunxi_gpio_set_cfgpin(SUNXI_GPI(1), SUN7I_GPI_TWI3);
> +	clock_twi_onoff(3, 1);
> +#endif
> +
> +#if defined(CONFIG_MACH_SUN7I)
> +	sunxi_gpio_set_cfgpin(SUNXI_GPI(2), SUN7I_GPI_TWI4);
> +	sunxi_gpio_set_cfgpin(SUNXI_GPI(3), SUN7I_GPI_TWI4);
> +	clock_twi_onoff(4, 1);
> +#endif
> +

Idem you are muxing pins here which may be used by something else,
and if they are not then they are best left as generic inputs rather
then configured as i2c pins, except when we know that there actually
is an i2c bus connected.

>   #if defined CONFIG_VIDEO_LCD_PANEL_I2C && !(defined CONFIG_SPL_BUILD)
>   	soft_i2c_gpio_sda = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_PANEL_I2C_SDA);
>   	soft_i2c_gpio_scl = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_PANEL_I2C_SCL);
>

Regards,

Hans
Paul Kocialkowski April 4, 2015, 12:54 p.m. UTC | #2
Hi Hans,

Le samedi 04 avril 2015 à 14:18 +0200, Hans de Goede a écrit :
> On 04-04-15 13:27, Paul Kocialkowski wrote:
> > Sunxi platforms come with at least 3 TWI (I2C) controllers and some platforms
> > even have up to 5. This adds support for every controller on each supported
> > platform, which is especially useful when using expansion ports on single-board-
> > computers.
> >
> > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> 
> Looks good in general, see comments inline for some minor things which need
> fixing.

Thanks for the review!

> > ---
> >   arch/arm/include/asm/arch-sunxi/cpu_sun4i.h |  7 +++
> >   arch/arm/include/asm/arch-sunxi/gpio.h      | 15 ++++++-
> >   arch/arm/include/asm/arch-sunxi/i2c.h       |  9 ++++
> >   board/sunxi/board.c                         | 67 ++++++++++++++++++++++++++++-
> >   4 files changed, 95 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h b/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
> > index dae6069..f403742 100644
> > --- a/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
> > +++ b/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
> > @@ -94,6 +94,13 @@
> >   #define SUNXI_TWI0_BASE			0x01c2ac00
> >   #define SUNXI_TWI1_BASE			0x01c2b000
> >   #define SUNXI_TWI2_BASE			0x01c2b400
> > +#ifdef CONFIG_MACH_SUN6I
> > +#define SUNXI_TWI3_BASE			0x01c0b800
> > +#endif
> > +#ifdef CONFIG_MACH_SUN7I
> > +#define SUNXI_TWI3_BASE			0x01c2b800
> > +#define SUNXI_TWI4_BASE			0x01c2c000
> > +#endif
> >
> >   #define SUNXI_CAN_BASE			0x01c2bc00
> >
> > diff --git a/arch/arm/include/asm/arch-sunxi/gpio.h b/arch/arm/include/asm/arch-sunxi/gpio.h
> > index f227044..ae7cbb7 100644
> > --- a/arch/arm/include/asm/arch-sunxi/gpio.h
> > +++ b/arch/arm/include/asm/arch-sunxi/gpio.h
> > @@ -148,7 +148,11 @@ enum sunxi_gpio_number {
> >   #define SUN6I_GPA_SDC2		5
> >   #define SUN6I_GPA_SDC3		4
> >
> > -#define SUNXI_GPB_TWI0		2
> > +#define SUN4I_GPB_TWI0		2
> > +#define SUN4I_GPB_TWI1		2
> > +#define SUN5I_GPB_TWI1		2
> > +#define SUN4I_GPB_TWI2		2
> > +#define SUN5I_GPB_TWI2		2
> >   #define SUN4I_GPB_UART0		2
> >   #define SUN5I_GPB_UART0		2
> >
> > @@ -160,6 +164,7 @@ enum sunxi_gpio_number {
> >   #define SUNXI_GPD_LVDS0		3
> >
> >   #define SUN5I_GPE_SDC2		3
> > +#define SUN8I_GPE_TWI2		3
> >
> >   #define SUNXI_GPF_SDC0		2
> >   #define SUNXI_GPF_UART0		4
> > @@ -169,12 +174,20 @@ enum sunxi_gpio_number {
> >   #define SUN5I_GPG_SDC1		2
> >   #define SUN6I_GPG_SDC1		2
> >   #define SUN8I_GPG_SDC1		2
> > +#define SUN6I_GPG_TWI3		2
> >   #define SUN5I_GPG_UART1		4
> >
> >   #define SUN4I_GPH_SDC1		5
> > +#define SUN6I_GPH_TWI0		2
> > +#define SUN8I_GPH_TWI0		2
> > +#define SUN6I_GPH_TWI1		2
> > +#define SUN8I_GPH_TWI1		2
> > +#define SUN6I_GPH_TWI2		2
> >   #define SUN6I_GPH_UART0		2
> >
> >   #define SUNXI_GPI_SDC3		2
> > +#define SUN7I_GPI_TWI3		3
> > +#define SUN7I_GPI_TWI4		3
> >
> >   #define SUN6I_GPL0_R_P2WI_SCK	3
> >   #define SUN6I_GPL1_R_P2WI_SDA	3
> > diff --git a/arch/arm/include/asm/arch-sunxi/i2c.h b/arch/arm/include/asm/arch-sunxi/i2c.h
> > index 502e3c6..5bec18c 100644
> > --- a/arch/arm/include/asm/arch-sunxi/i2c.h
> > +++ b/arch/arm/include/asm/arch-sunxi/i2c.h
> > @@ -9,6 +9,15 @@
> >   #include <asm/arch/cpu.h>
> >
> >   #define CONFIG_I2C_MVTWSI_BASE0	SUNXI_TWI0_BASE
> > +#define CONFIG_I2C_MVTWSI_BASE1	SUNXI_TWI1_BASE
> > +#define CONFIG_I2C_MVTWSI_BASE2	SUNXI_TWI2_BASE
> > +#ifdef SUNXI_TWI3_BASE
> > +#define CONFIG_I2C_MVTWSI_BASE3	SUNXI_TWI3_BASE
> > +#endif
> > +#ifdef SUNXI_TWI4_BASE
> > +#define CONFIG_I2C_MVTWSI_BASE4	SUNXI_TWI4_BASE
> > +#endif
> > +
> >   /* This is abp0-clk on sun4i/5i/7i / abp1-clk on sun6i/sun8i which is 24MHz */
> >   #define CONFIG_SYS_TCLK		24000000
> >
> 
> This will cause us to register all possible i2c controllers for
> each soc, but they might very well not be hooked up to anything,
> and the pins of the SoC the code below will now mux to i2c
> may even be used for something else, so this needs to be
> activated by a Kconfig option (one per i2c controller).

I agree, I also had this thought on the back of my mind and wasn't sure
whether to do anything about it. I'll submit v2 ASAP.

What would the preferred form for the Kconfig options be?
I suggest using CONFIG_I2C1_ENABLE or CONFIG_SUNXI_I2C1_ENABLE
(preferable the latter, but I don't see much SUNXI prefixing being done
in board/sunxi/Kconfig).

> > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > index 7633d65..b3eecbb 100644
> > --- a/board/sunxi/board.c
> > +++ b/board/sunxi/board.c
> > @@ -276,9 +276,72 @@ int board_mmc_init(bd_t *bis)
> >
> >   void i2c_init_board(void)
> >   {
> > -	sunxi_gpio_set_cfgpin(SUNXI_GPB(0), SUNXI_GPB_TWI0);
> > -	sunxi_gpio_set_cfgpin(SUNXI_GPB(1), SUNXI_GPB_TWI0);
> > +#if defined(CONFIG_MACH_SUN4I) || defined(CONFIG_MACH_SUN5I) || defined(CONFIG_MACH_SUN7I)
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPB(0), SUN4I_GPB_TWI0);
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPB(1), SUN4I_GPB_TWI0);
> >   	clock_twi_onoff(0, 1);
> > +#elif defined(CONFIG_MACH_SUN6I)
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPH(14), SUN6I_GPH_TWI0);
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPH(15), SUN6I_GPH_TWI0);
> > +	clock_twi_onoff(0, 1);
> > +#elif defined(CONFIG_MACH_SUN8I)
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPH(2), SUN8I_GPH_TWI0);
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPH(3), SUN8I_GPH_TWI0);
> > +	clock_twi_onoff(0, 1);
> > +#endif
> > +
> > +#if defined(CONFIG_MACH_SUN4I) || defined(CONFIG_MACH_SUN7I)
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPB(18), SUN4I_GPB_TWI1);
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPB(19), SUN4I_GPB_TWI1);
> > +	clock_twi_onoff(1, 1);
> > +#elif defined(CONFIG_MACH_SUN5I)
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPB(15), SUN5I_GPB_TWI1);
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPB(16), SUN5I_GPB_TWI1);
> > +	clock_twi_onoff(1, 1);
> > +#elif defined(CONFIG_MACH_SUN6I)
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPH(16), SUN6I_GPH_TWI1);
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPH(17), SUN6I_GPH_TWI1);
> > +	clock_twi_onoff(1, 1);
> > +#elif defined(CONFIG_MACH_SUN8I)
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPH(4), SUN8I_GPH_TWI1);
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPH(5), SUN8I_GPH_TWI1);
> > +	clock_twi_onoff(1, 1);
> > +#endif
> > +
> > +#if defined(CONFIG_MACH_SUN4I) || defined(CONFIG_MACH_SUN7I)
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPB(20), SUN4I_GPB_TWI2);
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPB(21), SUN4I_GPB_TWI2);
> > +	clock_twi_onoff(2, 1);
> > +#elif defined(CONFIG_MACH_SUN5I)
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPB(17), SUN5I_GPB_TWI2);
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPB(18), SUN5I_GPB_TWI2);
> > +	clock_twi_onoff(2, 1);
> > +#elif defined(CONFIG_MACH_SUN6I)
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPH(18), SUN6I_GPH_TWI2);
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPH(19), SUN6I_GPH_TWI2);
> > +	clock_twi_onoff(2, 1);
> > +#elif defined(CONFIG_MACH_SUN8I)
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPE(12), SUN8I_GPE_TWI2);
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPE(13), SUN8I_GPE_TWI2);
> > +	clock_twi_onoff(2, 1);
> > +#endif
> > +
> > +#if defined(CONFIG_MACH_SUN6I)
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPG(10), SUN6I_GPG_TWI3);
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPG(11), SUN6I_GPG_TWI3);
> > +	clock_twi_onoff(3, 1);
> > +#elif defined(CONFIG_MACH_SUN7I)
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPI(0), SUN7I_GPI_TWI3);
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPI(1), SUN7I_GPI_TWI3);
> > +	clock_twi_onoff(3, 1);
> > +#endif
> > +
> > +#if defined(CONFIG_MACH_SUN7I)
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPI(2), SUN7I_GPI_TWI4);
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPI(3), SUN7I_GPI_TWI4);
> > +	clock_twi_onoff(4, 1);
> > +#endif
> > +
> 
> Idem you are muxing pins here which may be used by something else,
> and if they are not then they are best left as generic inputs rather
> then configured as i2c pins, except when we know that there actually
> is an i2c bus connected.

Ack

> >   #if defined CONFIG_VIDEO_LCD_PANEL_I2C && !(defined CONFIG_SPL_BUILD)
> >   	soft_i2c_gpio_sda = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_PANEL_I2C_SDA);
> >   	soft_i2c_gpio_scl = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_PANEL_I2C_SCL);
> >
> 
> Regards,
> 
> Hans
Hans de Goede April 4, 2015, 1:29 p.m. UTC | #3
Hi,

On 04-04-15 14:54, Paul Kocialkowski wrote:
> Hi Hans,
>
> Le samedi 04 avril 2015 à 14:18 +0200, Hans de Goede a écrit :
>> On 04-04-15 13:27, Paul Kocialkowski wrote:
>>> Sunxi platforms come with at least 3 TWI (I2C) controllers and some platforms
>>> even have up to 5. This adds support for every controller on each supported
>>> platform, which is especially useful when using expansion ports on single-board-
>>> computers.
>>>
>>> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
>>
>> Looks good in general, see comments inline for some minor things which need
>> fixing.
>
> Thanks for the review!
>
>>> ---
>>>    arch/arm/include/asm/arch-sunxi/cpu_sun4i.h |  7 +++
>>>    arch/arm/include/asm/arch-sunxi/gpio.h      | 15 ++++++-
>>>    arch/arm/include/asm/arch-sunxi/i2c.h       |  9 ++++
>>>    board/sunxi/board.c                         | 67 ++++++++++++++++++++++++++++-
>>>    4 files changed, 95 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h b/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
>>> index dae6069..f403742 100644
>>> --- a/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
>>> +++ b/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
>>> @@ -94,6 +94,13 @@
>>>    #define SUNXI_TWI0_BASE			0x01c2ac00
>>>    #define SUNXI_TWI1_BASE			0x01c2b000
>>>    #define SUNXI_TWI2_BASE			0x01c2b400
>>> +#ifdef CONFIG_MACH_SUN6I
>>> +#define SUNXI_TWI3_BASE			0x01c0b800
>>> +#endif
>>> +#ifdef CONFIG_MACH_SUN7I
>>> +#define SUNXI_TWI3_BASE			0x01c2b800
>>> +#define SUNXI_TWI4_BASE			0x01c2c000
>>> +#endif
>>>
>>>    #define SUNXI_CAN_BASE			0x01c2bc00
>>>
>>> diff --git a/arch/arm/include/asm/arch-sunxi/gpio.h b/arch/arm/include/asm/arch-sunxi/gpio.h
>>> index f227044..ae7cbb7 100644
>>> --- a/arch/arm/include/asm/arch-sunxi/gpio.h
>>> +++ b/arch/arm/include/asm/arch-sunxi/gpio.h
>>> @@ -148,7 +148,11 @@ enum sunxi_gpio_number {
>>>    #define SUN6I_GPA_SDC2		5
>>>    #define SUN6I_GPA_SDC3		4
>>>
>>> -#define SUNXI_GPB_TWI0		2
>>> +#define SUN4I_GPB_TWI0		2
>>> +#define SUN4I_GPB_TWI1		2
>>> +#define SUN5I_GPB_TWI1		2
>>> +#define SUN4I_GPB_TWI2		2
>>> +#define SUN5I_GPB_TWI2		2
>>>    #define SUN4I_GPB_UART0		2
>>>    #define SUN5I_GPB_UART0		2
>>>
>>> @@ -160,6 +164,7 @@ enum sunxi_gpio_number {
>>>    #define SUNXI_GPD_LVDS0		3
>>>
>>>    #define SUN5I_GPE_SDC2		3
>>> +#define SUN8I_GPE_TWI2		3
>>>
>>>    #define SUNXI_GPF_SDC0		2
>>>    #define SUNXI_GPF_UART0		4
>>> @@ -169,12 +174,20 @@ enum sunxi_gpio_number {
>>>    #define SUN5I_GPG_SDC1		2
>>>    #define SUN6I_GPG_SDC1		2
>>>    #define SUN8I_GPG_SDC1		2
>>> +#define SUN6I_GPG_TWI3		2
>>>    #define SUN5I_GPG_UART1		4
>>>
>>>    #define SUN4I_GPH_SDC1		5
>>> +#define SUN6I_GPH_TWI0		2
>>> +#define SUN8I_GPH_TWI0		2
>>> +#define SUN6I_GPH_TWI1		2
>>> +#define SUN8I_GPH_TWI1		2
>>> +#define SUN6I_GPH_TWI2		2
>>>    #define SUN6I_GPH_UART0		2
>>>
>>>    #define SUNXI_GPI_SDC3		2
>>> +#define SUN7I_GPI_TWI3		3
>>> +#define SUN7I_GPI_TWI4		3
>>>
>>>    #define SUN6I_GPL0_R_P2WI_SCK	3
>>>    #define SUN6I_GPL1_R_P2WI_SDA	3
>>> diff --git a/arch/arm/include/asm/arch-sunxi/i2c.h b/arch/arm/include/asm/arch-sunxi/i2c.h
>>> index 502e3c6..5bec18c 100644
>>> --- a/arch/arm/include/asm/arch-sunxi/i2c.h
>>> +++ b/arch/arm/include/asm/arch-sunxi/i2c.h
>>> @@ -9,6 +9,15 @@
>>>    #include <asm/arch/cpu.h>
>>>
>>>    #define CONFIG_I2C_MVTWSI_BASE0	SUNXI_TWI0_BASE
>>> +#define CONFIG_I2C_MVTWSI_BASE1	SUNXI_TWI1_BASE
>>> +#define CONFIG_I2C_MVTWSI_BASE2	SUNXI_TWI2_BASE
>>> +#ifdef SUNXI_TWI3_BASE
>>> +#define CONFIG_I2C_MVTWSI_BASE3	SUNXI_TWI3_BASE
>>> +#endif
>>> +#ifdef SUNXI_TWI4_BASE
>>> +#define CONFIG_I2C_MVTWSI_BASE4	SUNXI_TWI4_BASE
>>> +#endif
>>> +
>>>    /* This is abp0-clk on sun4i/5i/7i / abp1-clk on sun6i/sun8i which is 24MHz */
>>>    #define CONFIG_SYS_TCLK		24000000
>>>
>>
>> This will cause us to register all possible i2c controllers for
>> each soc, but they might very well not be hooked up to anything,
>> and the pins of the SoC the code below will now mux to i2c
>> may even be used for something else, so this needs to be
>> activated by a Kconfig option (one per i2c controller).
>
> I agree, I also had this thought on the back of my mind and wasn't sure
> whether to do anything about it. I'll submit v2 ASAP.
>
> What would the preferred form for the Kconfig options be?
> I suggest using CONFIG_I2C1_ENABLE or CONFIG_SUNXI_I2C1_ENABLE
> (preferable the latter, but I don't see much SUNXI prefixing being done
> in board/sunxi/Kconfig).

I've no preference for the Kconfig name, as for submitting v2 ASAP,
I will not merge this until Heiko has merged the first patch which
will not happen until v2015.04 is released and the new merge window
opens, so no hurry, unless you want to get this of your plate :)

Regards,

Hans


>
>>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
>>> index 7633d65..b3eecbb 100644
>>> --- a/board/sunxi/board.c
>>> +++ b/board/sunxi/board.c
>>> @@ -276,9 +276,72 @@ int board_mmc_init(bd_t *bis)
>>>
>>>    void i2c_init_board(void)
>>>    {
>>> -	sunxi_gpio_set_cfgpin(SUNXI_GPB(0), SUNXI_GPB_TWI0);
>>> -	sunxi_gpio_set_cfgpin(SUNXI_GPB(1), SUNXI_GPB_TWI0);
>>> +#if defined(CONFIG_MACH_SUN4I) || defined(CONFIG_MACH_SUN5I) || defined(CONFIG_MACH_SUN7I)
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(0), SUN4I_GPB_TWI0);
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(1), SUN4I_GPB_TWI0);
>>>    	clock_twi_onoff(0, 1);
>>> +#elif defined(CONFIG_MACH_SUN6I)
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(14), SUN6I_GPH_TWI0);
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(15), SUN6I_GPH_TWI0);
>>> +	clock_twi_onoff(0, 1);
>>> +#elif defined(CONFIG_MACH_SUN8I)
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(2), SUN8I_GPH_TWI0);
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(3), SUN8I_GPH_TWI0);
>>> +	clock_twi_onoff(0, 1);
>>> +#endif
>>> +
>>> +#if defined(CONFIG_MACH_SUN4I) || defined(CONFIG_MACH_SUN7I)
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(18), SUN4I_GPB_TWI1);
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(19), SUN4I_GPB_TWI1);
>>> +	clock_twi_onoff(1, 1);
>>> +#elif defined(CONFIG_MACH_SUN5I)
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(15), SUN5I_GPB_TWI1);
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(16), SUN5I_GPB_TWI1);
>>> +	clock_twi_onoff(1, 1);
>>> +#elif defined(CONFIG_MACH_SUN6I)
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(16), SUN6I_GPH_TWI1);
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(17), SUN6I_GPH_TWI1);
>>> +	clock_twi_onoff(1, 1);
>>> +#elif defined(CONFIG_MACH_SUN8I)
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(4), SUN8I_GPH_TWI1);
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(5), SUN8I_GPH_TWI1);
>>> +	clock_twi_onoff(1, 1);
>>> +#endif
>>> +
>>> +#if defined(CONFIG_MACH_SUN4I) || defined(CONFIG_MACH_SUN7I)
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(20), SUN4I_GPB_TWI2);
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(21), SUN4I_GPB_TWI2);
>>> +	clock_twi_onoff(2, 1);
>>> +#elif defined(CONFIG_MACH_SUN5I)
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(17), SUN5I_GPB_TWI2);
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(18), SUN5I_GPB_TWI2);
>>> +	clock_twi_onoff(2, 1);
>>> +#elif defined(CONFIG_MACH_SUN6I)
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(18), SUN6I_GPH_TWI2);
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(19), SUN6I_GPH_TWI2);
>>> +	clock_twi_onoff(2, 1);
>>> +#elif defined(CONFIG_MACH_SUN8I)
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPE(12), SUN8I_GPE_TWI2);
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPE(13), SUN8I_GPE_TWI2);
>>> +	clock_twi_onoff(2, 1);
>>> +#endif
>>> +
>>> +#if defined(CONFIG_MACH_SUN6I)
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPG(10), SUN6I_GPG_TWI3);
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPG(11), SUN6I_GPG_TWI3);
>>> +	clock_twi_onoff(3, 1);
>>> +#elif defined(CONFIG_MACH_SUN7I)
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPI(0), SUN7I_GPI_TWI3);
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPI(1), SUN7I_GPI_TWI3);
>>> +	clock_twi_onoff(3, 1);
>>> +#endif
>>> +
>>> +#if defined(CONFIG_MACH_SUN7I)
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPI(2), SUN7I_GPI_TWI4);
>>> +	sunxi_gpio_set_cfgpin(SUNXI_GPI(3), SUN7I_GPI_TWI4);
>>> +	clock_twi_onoff(4, 1);
>>> +#endif
>>> +
>>
>> Idem you are muxing pins here which may be used by something else,
>> and if they are not then they are best left as generic inputs rather
>> then configured as i2c pins, except when we know that there actually
>> is an i2c bus connected.
>
> Ack
>
>>>    #if defined CONFIG_VIDEO_LCD_PANEL_I2C && !(defined CONFIG_SPL_BUILD)
>>>    	soft_i2c_gpio_sda = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_PANEL_I2C_SDA);
>>>    	soft_i2c_gpio_scl = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_PANEL_I2C_SCL);
>>>
>>
>> Regards,
>>
>> Hans
>
Paul Kocialkowski April 4, 2015, 8:51 p.m. UTC | #4
Le samedi 04 avril 2015 à 15:29 +0200, Hans de Goede a écrit :
> Hi,
> 
> On 04-04-15 14:54, Paul Kocialkowski wrote:
> > Hi Hans,
> >
> > Le samedi 04 avril 2015 à 14:18 +0200, Hans de Goede a écrit :
> >> On 04-04-15 13:27, Paul Kocialkowski wrote:
> >>> Sunxi platforms come with at least 3 TWI (I2C) controllers and some platforms
> >>> even have up to 5. This adds support for every controller on each supported
> >>> platform, which is especially useful when using expansion ports on single-board-
> >>> computers.
> >>>
> >>> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> >>
> >> Looks good in general, see comments inline for some minor things which need
> >> fixing.
> >
> > Thanks for the review!
> >
> >>> ---
> >>>    arch/arm/include/asm/arch-sunxi/cpu_sun4i.h |  7 +++
> >>>    arch/arm/include/asm/arch-sunxi/gpio.h      | 15 ++++++-
> >>>    arch/arm/include/asm/arch-sunxi/i2c.h       |  9 ++++
> >>>    board/sunxi/board.c                         | 67 ++++++++++++++++++++++++++++-
> >>>    4 files changed, 95 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h b/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
> >>> index dae6069..f403742 100644
> >>> --- a/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
> >>> +++ b/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
> >>> @@ -94,6 +94,13 @@
> >>>    #define SUNXI_TWI0_BASE			0x01c2ac00
> >>>    #define SUNXI_TWI1_BASE			0x01c2b000
> >>>    #define SUNXI_TWI2_BASE			0x01c2b400
> >>> +#ifdef CONFIG_MACH_SUN6I
> >>> +#define SUNXI_TWI3_BASE			0x01c0b800
> >>> +#endif
> >>> +#ifdef CONFIG_MACH_SUN7I
> >>> +#define SUNXI_TWI3_BASE			0x01c2b800
> >>> +#define SUNXI_TWI4_BASE			0x01c2c000
> >>> +#endif
> >>>
> >>>    #define SUNXI_CAN_BASE			0x01c2bc00
> >>>
> >>> diff --git a/arch/arm/include/asm/arch-sunxi/gpio.h b/arch/arm/include/asm/arch-sunxi/gpio.h
> >>> index f227044..ae7cbb7 100644
> >>> --- a/arch/arm/include/asm/arch-sunxi/gpio.h
> >>> +++ b/arch/arm/include/asm/arch-sunxi/gpio.h
> >>> @@ -148,7 +148,11 @@ enum sunxi_gpio_number {
> >>>    #define SUN6I_GPA_SDC2		5
> >>>    #define SUN6I_GPA_SDC3		4
> >>>
> >>> -#define SUNXI_GPB_TWI0		2
> >>> +#define SUN4I_GPB_TWI0		2
> >>> +#define SUN4I_GPB_TWI1		2
> >>> +#define SUN5I_GPB_TWI1		2
> >>> +#define SUN4I_GPB_TWI2		2
> >>> +#define SUN5I_GPB_TWI2		2
> >>>    #define SUN4I_GPB_UART0		2
> >>>    #define SUN5I_GPB_UART0		2
> >>>
> >>> @@ -160,6 +164,7 @@ enum sunxi_gpio_number {
> >>>    #define SUNXI_GPD_LVDS0		3
> >>>
> >>>    #define SUN5I_GPE_SDC2		3
> >>> +#define SUN8I_GPE_TWI2		3
> >>>
> >>>    #define SUNXI_GPF_SDC0		2
> >>>    #define SUNXI_GPF_UART0		4
> >>> @@ -169,12 +174,20 @@ enum sunxi_gpio_number {
> >>>    #define SUN5I_GPG_SDC1		2
> >>>    #define SUN6I_GPG_SDC1		2
> >>>    #define SUN8I_GPG_SDC1		2
> >>> +#define SUN6I_GPG_TWI3		2
> >>>    #define SUN5I_GPG_UART1		4
> >>>
> >>>    #define SUN4I_GPH_SDC1		5
> >>> +#define SUN6I_GPH_TWI0		2
> >>> +#define SUN8I_GPH_TWI0		2
> >>> +#define SUN6I_GPH_TWI1		2
> >>> +#define SUN8I_GPH_TWI1		2
> >>> +#define SUN6I_GPH_TWI2		2
> >>>    #define SUN6I_GPH_UART0		2
> >>>
> >>>    #define SUNXI_GPI_SDC3		2
> >>> +#define SUN7I_GPI_TWI3		3
> >>> +#define SUN7I_GPI_TWI4		3
> >>>
> >>>    #define SUN6I_GPL0_R_P2WI_SCK	3
> >>>    #define SUN6I_GPL1_R_P2WI_SDA	3
> >>> diff --git a/arch/arm/include/asm/arch-sunxi/i2c.h b/arch/arm/include/asm/arch-sunxi/i2c.h
> >>> index 502e3c6..5bec18c 100644
> >>> --- a/arch/arm/include/asm/arch-sunxi/i2c.h
> >>> +++ b/arch/arm/include/asm/arch-sunxi/i2c.h
> >>> @@ -9,6 +9,15 @@
> >>>    #include <asm/arch/cpu.h>
> >>>
> >>>    #define CONFIG_I2C_MVTWSI_BASE0	SUNXI_TWI0_BASE
> >>> +#define CONFIG_I2C_MVTWSI_BASE1	SUNXI_TWI1_BASE
> >>> +#define CONFIG_I2C_MVTWSI_BASE2	SUNXI_TWI2_BASE
> >>> +#ifdef SUNXI_TWI3_BASE
> >>> +#define CONFIG_I2C_MVTWSI_BASE3	SUNXI_TWI3_BASE
> >>> +#endif
> >>> +#ifdef SUNXI_TWI4_BASE
> >>> +#define CONFIG_I2C_MVTWSI_BASE4	SUNXI_TWI4_BASE
> >>> +#endif
> >>> +
> >>>    /* This is abp0-clk on sun4i/5i/7i / abp1-clk on sun6i/sun8i which is 24MHz */
> >>>    #define CONFIG_SYS_TCLK		24000000
> >>>
> >>
> >> This will cause us to register all possible i2c controllers for
> >> each soc, but they might very well not be hooked up to anything,
> >> and the pins of the SoC the code below will now mux to i2c
> >> may even be used for something else, so this needs to be
> >> activated by a Kconfig option (one per i2c controller).
> >
> > I agree, I also had this thought on the back of my mind and wasn't sure
> > whether to do anything about it. I'll submit v2 ASAP.
> >
> > What would the preferred form for the Kconfig options be?
> > I suggest using CONFIG_I2C1_ENABLE or CONFIG_SUNXI_I2C1_ENABLE
> > (preferable the latter, but I don't see much SUNXI prefixing being done
> > in board/sunxi/Kconfig).
> 
> I've no preference for the Kconfig name, as for submitting v2 ASAP,
> I will not merge this until Heiko has merged the first patch which
> will not happen until v2015.04 is released and the new merge window
> opens, so no hurry, unless you want to get this of your plate :)

I just sent out v2 -- I see there is no hurry, but I prefer to get
everything ready now so that I can focus on other things later.

> >>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> >>> index 7633d65..b3eecbb 100644
> >>> --- a/board/sunxi/board.c
> >>> +++ b/board/sunxi/board.c
> >>> @@ -276,9 +276,72 @@ int board_mmc_init(bd_t *bis)
> >>>
> >>>    void i2c_init_board(void)
> >>>    {
> >>> -	sunxi_gpio_set_cfgpin(SUNXI_GPB(0), SUNXI_GPB_TWI0);
> >>> -	sunxi_gpio_set_cfgpin(SUNXI_GPB(1), SUNXI_GPB_TWI0);
> >>> +#if defined(CONFIG_MACH_SUN4I) || defined(CONFIG_MACH_SUN5I) || defined(CONFIG_MACH_SUN7I)
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(0), SUN4I_GPB_TWI0);
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(1), SUN4I_GPB_TWI0);
> >>>    	clock_twi_onoff(0, 1);
> >>> +#elif defined(CONFIG_MACH_SUN6I)
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(14), SUN6I_GPH_TWI0);
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(15), SUN6I_GPH_TWI0);
> >>> +	clock_twi_onoff(0, 1);
> >>> +#elif defined(CONFIG_MACH_SUN8I)
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(2), SUN8I_GPH_TWI0);
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(3), SUN8I_GPH_TWI0);
> >>> +	clock_twi_onoff(0, 1);
> >>> +#endif
> >>> +
> >>> +#if defined(CONFIG_MACH_SUN4I) || defined(CONFIG_MACH_SUN7I)
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(18), SUN4I_GPB_TWI1);
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(19), SUN4I_GPB_TWI1);
> >>> +	clock_twi_onoff(1, 1);
> >>> +#elif defined(CONFIG_MACH_SUN5I)
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(15), SUN5I_GPB_TWI1);
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(16), SUN5I_GPB_TWI1);
> >>> +	clock_twi_onoff(1, 1);
> >>> +#elif defined(CONFIG_MACH_SUN6I)
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(16), SUN6I_GPH_TWI1);
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(17), SUN6I_GPH_TWI1);
> >>> +	clock_twi_onoff(1, 1);
> >>> +#elif defined(CONFIG_MACH_SUN8I)
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(4), SUN8I_GPH_TWI1);
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(5), SUN8I_GPH_TWI1);
> >>> +	clock_twi_onoff(1, 1);
> >>> +#endif
> >>> +
> >>> +#if defined(CONFIG_MACH_SUN4I) || defined(CONFIG_MACH_SUN7I)
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(20), SUN4I_GPB_TWI2);
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(21), SUN4I_GPB_TWI2);
> >>> +	clock_twi_onoff(2, 1);
> >>> +#elif defined(CONFIG_MACH_SUN5I)
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(17), SUN5I_GPB_TWI2);
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPB(18), SUN5I_GPB_TWI2);
> >>> +	clock_twi_onoff(2, 1);
> >>> +#elif defined(CONFIG_MACH_SUN6I)
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(18), SUN6I_GPH_TWI2);
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPH(19), SUN6I_GPH_TWI2);
> >>> +	clock_twi_onoff(2, 1);
> >>> +#elif defined(CONFIG_MACH_SUN8I)
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPE(12), SUN8I_GPE_TWI2);
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPE(13), SUN8I_GPE_TWI2);
> >>> +	clock_twi_onoff(2, 1);
> >>> +#endif
> >>> +
> >>> +#if defined(CONFIG_MACH_SUN6I)
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPG(10), SUN6I_GPG_TWI3);
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPG(11), SUN6I_GPG_TWI3);
> >>> +	clock_twi_onoff(3, 1);
> >>> +#elif defined(CONFIG_MACH_SUN7I)
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPI(0), SUN7I_GPI_TWI3);
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPI(1), SUN7I_GPI_TWI3);
> >>> +	clock_twi_onoff(3, 1);
> >>> +#endif
> >>> +
> >>> +#if defined(CONFIG_MACH_SUN7I)
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPI(2), SUN7I_GPI_TWI4);
> >>> +	sunxi_gpio_set_cfgpin(SUNXI_GPI(3), SUN7I_GPI_TWI4);
> >>> +	clock_twi_onoff(4, 1);
> >>> +#endif
> >>> +
> >>
> >> Idem you are muxing pins here which may be used by something else,
> >> and if they are not then they are best left as generic inputs rather
> >> then configured as i2c pins, except when we know that there actually
> >> is an i2c bus connected.
> >
> > Ack
> >
> >>>    #if defined CONFIG_VIDEO_LCD_PANEL_I2C && !(defined CONFIG_SPL_BUILD)
> >>>    	soft_i2c_gpio_sda = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_PANEL_I2C_SDA);
> >>>    	soft_i2c_gpio_scl = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_PANEL_I2C_SCL);
> >>>
> >>
> >> Regards,
> >>
> >> Hans
> >
diff mbox

Patch

diff --git a/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h b/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
index dae6069..f403742 100644
--- a/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
+++ b/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
@@ -94,6 +94,13 @@ 
 #define SUNXI_TWI0_BASE			0x01c2ac00
 #define SUNXI_TWI1_BASE			0x01c2b000
 #define SUNXI_TWI2_BASE			0x01c2b400
+#ifdef CONFIG_MACH_SUN6I
+#define SUNXI_TWI3_BASE			0x01c0b800
+#endif
+#ifdef CONFIG_MACH_SUN7I
+#define SUNXI_TWI3_BASE			0x01c2b800
+#define SUNXI_TWI4_BASE			0x01c2c000
+#endif
 
 #define SUNXI_CAN_BASE			0x01c2bc00
 
diff --git a/arch/arm/include/asm/arch-sunxi/gpio.h b/arch/arm/include/asm/arch-sunxi/gpio.h
index f227044..ae7cbb7 100644
--- a/arch/arm/include/asm/arch-sunxi/gpio.h
+++ b/arch/arm/include/asm/arch-sunxi/gpio.h
@@ -148,7 +148,11 @@  enum sunxi_gpio_number {
 #define SUN6I_GPA_SDC2		5
 #define SUN6I_GPA_SDC3		4
 
-#define SUNXI_GPB_TWI0		2
+#define SUN4I_GPB_TWI0		2
+#define SUN4I_GPB_TWI1		2
+#define SUN5I_GPB_TWI1		2
+#define SUN4I_GPB_TWI2		2
+#define SUN5I_GPB_TWI2		2
 #define SUN4I_GPB_UART0		2
 #define SUN5I_GPB_UART0		2
 
@@ -160,6 +164,7 @@  enum sunxi_gpio_number {
 #define SUNXI_GPD_LVDS0		3
 
 #define SUN5I_GPE_SDC2		3
+#define SUN8I_GPE_TWI2		3
 
 #define SUNXI_GPF_SDC0		2
 #define SUNXI_GPF_UART0		4
@@ -169,12 +174,20 @@  enum sunxi_gpio_number {
 #define SUN5I_GPG_SDC1		2
 #define SUN6I_GPG_SDC1		2
 #define SUN8I_GPG_SDC1		2
+#define SUN6I_GPG_TWI3		2
 #define SUN5I_GPG_UART1		4
 
 #define SUN4I_GPH_SDC1		5
+#define SUN6I_GPH_TWI0		2
+#define SUN8I_GPH_TWI0		2
+#define SUN6I_GPH_TWI1		2
+#define SUN8I_GPH_TWI1		2
+#define SUN6I_GPH_TWI2		2
 #define SUN6I_GPH_UART0		2
 
 #define SUNXI_GPI_SDC3		2
+#define SUN7I_GPI_TWI3		3
+#define SUN7I_GPI_TWI4		3
 
 #define SUN6I_GPL0_R_P2WI_SCK	3
 #define SUN6I_GPL1_R_P2WI_SDA	3
diff --git a/arch/arm/include/asm/arch-sunxi/i2c.h b/arch/arm/include/asm/arch-sunxi/i2c.h
index 502e3c6..5bec18c 100644
--- a/arch/arm/include/asm/arch-sunxi/i2c.h
+++ b/arch/arm/include/asm/arch-sunxi/i2c.h
@@ -9,6 +9,15 @@ 
 #include <asm/arch/cpu.h>
 
 #define CONFIG_I2C_MVTWSI_BASE0	SUNXI_TWI0_BASE
+#define CONFIG_I2C_MVTWSI_BASE1	SUNXI_TWI1_BASE
+#define CONFIG_I2C_MVTWSI_BASE2	SUNXI_TWI2_BASE
+#ifdef SUNXI_TWI3_BASE
+#define CONFIG_I2C_MVTWSI_BASE3	SUNXI_TWI3_BASE
+#endif
+#ifdef SUNXI_TWI4_BASE
+#define CONFIG_I2C_MVTWSI_BASE4	SUNXI_TWI4_BASE
+#endif
+
 /* This is abp0-clk on sun4i/5i/7i / abp1-clk on sun6i/sun8i which is 24MHz */
 #define CONFIG_SYS_TCLK		24000000
 
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 7633d65..b3eecbb 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -276,9 +276,72 @@  int board_mmc_init(bd_t *bis)
 
 void i2c_init_board(void)
 {
-	sunxi_gpio_set_cfgpin(SUNXI_GPB(0), SUNXI_GPB_TWI0);
-	sunxi_gpio_set_cfgpin(SUNXI_GPB(1), SUNXI_GPB_TWI0);
+#if defined(CONFIG_MACH_SUN4I) || defined(CONFIG_MACH_SUN5I) || defined(CONFIG_MACH_SUN7I)
+	sunxi_gpio_set_cfgpin(SUNXI_GPB(0), SUN4I_GPB_TWI0);
+	sunxi_gpio_set_cfgpin(SUNXI_GPB(1), SUN4I_GPB_TWI0);
 	clock_twi_onoff(0, 1);
+#elif defined(CONFIG_MACH_SUN6I)
+	sunxi_gpio_set_cfgpin(SUNXI_GPH(14), SUN6I_GPH_TWI0);
+	sunxi_gpio_set_cfgpin(SUNXI_GPH(15), SUN6I_GPH_TWI0);
+	clock_twi_onoff(0, 1);
+#elif defined(CONFIG_MACH_SUN8I)
+	sunxi_gpio_set_cfgpin(SUNXI_GPH(2), SUN8I_GPH_TWI0);
+	sunxi_gpio_set_cfgpin(SUNXI_GPH(3), SUN8I_GPH_TWI0);
+	clock_twi_onoff(0, 1);
+#endif
+
+#if defined(CONFIG_MACH_SUN4I) || defined(CONFIG_MACH_SUN7I)
+	sunxi_gpio_set_cfgpin(SUNXI_GPB(18), SUN4I_GPB_TWI1);
+	sunxi_gpio_set_cfgpin(SUNXI_GPB(19), SUN4I_GPB_TWI1);
+	clock_twi_onoff(1, 1);
+#elif defined(CONFIG_MACH_SUN5I)
+	sunxi_gpio_set_cfgpin(SUNXI_GPB(15), SUN5I_GPB_TWI1);
+	sunxi_gpio_set_cfgpin(SUNXI_GPB(16), SUN5I_GPB_TWI1);
+	clock_twi_onoff(1, 1);
+#elif defined(CONFIG_MACH_SUN6I)
+	sunxi_gpio_set_cfgpin(SUNXI_GPH(16), SUN6I_GPH_TWI1);
+	sunxi_gpio_set_cfgpin(SUNXI_GPH(17), SUN6I_GPH_TWI1);
+	clock_twi_onoff(1, 1);
+#elif defined(CONFIG_MACH_SUN8I)
+	sunxi_gpio_set_cfgpin(SUNXI_GPH(4), SUN8I_GPH_TWI1);
+	sunxi_gpio_set_cfgpin(SUNXI_GPH(5), SUN8I_GPH_TWI1);
+	clock_twi_onoff(1, 1);
+#endif
+
+#if defined(CONFIG_MACH_SUN4I) || defined(CONFIG_MACH_SUN7I)
+	sunxi_gpio_set_cfgpin(SUNXI_GPB(20), SUN4I_GPB_TWI2);
+	sunxi_gpio_set_cfgpin(SUNXI_GPB(21), SUN4I_GPB_TWI2);
+	clock_twi_onoff(2, 1);
+#elif defined(CONFIG_MACH_SUN5I)
+	sunxi_gpio_set_cfgpin(SUNXI_GPB(17), SUN5I_GPB_TWI2);
+	sunxi_gpio_set_cfgpin(SUNXI_GPB(18), SUN5I_GPB_TWI2);
+	clock_twi_onoff(2, 1);
+#elif defined(CONFIG_MACH_SUN6I)
+	sunxi_gpio_set_cfgpin(SUNXI_GPH(18), SUN6I_GPH_TWI2);
+	sunxi_gpio_set_cfgpin(SUNXI_GPH(19), SUN6I_GPH_TWI2);
+	clock_twi_onoff(2, 1);
+#elif defined(CONFIG_MACH_SUN8I)
+	sunxi_gpio_set_cfgpin(SUNXI_GPE(12), SUN8I_GPE_TWI2);
+	sunxi_gpio_set_cfgpin(SUNXI_GPE(13), SUN8I_GPE_TWI2);
+	clock_twi_onoff(2, 1);
+#endif
+
+#if defined(CONFIG_MACH_SUN6I)
+	sunxi_gpio_set_cfgpin(SUNXI_GPG(10), SUN6I_GPG_TWI3);
+	sunxi_gpio_set_cfgpin(SUNXI_GPG(11), SUN6I_GPG_TWI3);
+	clock_twi_onoff(3, 1);
+#elif defined(CONFIG_MACH_SUN7I)
+	sunxi_gpio_set_cfgpin(SUNXI_GPI(0), SUN7I_GPI_TWI3);
+	sunxi_gpio_set_cfgpin(SUNXI_GPI(1), SUN7I_GPI_TWI3);
+	clock_twi_onoff(3, 1);
+#endif
+
+#if defined(CONFIG_MACH_SUN7I)
+	sunxi_gpio_set_cfgpin(SUNXI_GPI(2), SUN7I_GPI_TWI4);
+	sunxi_gpio_set_cfgpin(SUNXI_GPI(3), SUN7I_GPI_TWI4);
+	clock_twi_onoff(4, 1);
+#endif
+
 #if defined CONFIG_VIDEO_LCD_PANEL_I2C && !(defined CONFIG_SPL_BUILD)
 	soft_i2c_gpio_sda = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_PANEL_I2C_SDA);
 	soft_i2c_gpio_scl = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_PANEL_I2C_SCL);