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

Simon Glass sjg at chromium.org
Sun Apr 5 23:52:31 CEST 2015


Hi Paul,

On 5 April 2015 at 14:56, Paul Kocialkowski <contact at paulk.fr> wrote:
> Le dimanche 05 avril 2015 à 12:31 -0600, Simon Glass a écrit :
>> Hi Paul,
>>
>> On 4 April 2015 at 14:49, Paul Kocialkowski <contact at paulk.fr> 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 at 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       | 13 +++++
>> >  board/sunxi/Kconfig                         | 31 ++++++++++++
>> >  board/sunxi/board.c                         | 75 ++++++++++++++++++++++++++++-
>> >  5 files changed, 138 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..5e9586f 100644
>> > --- a/arch/arm/include/asm/arch-sunxi/i2c.h
>> > +++ b/arch/arm/include/asm/arch-sunxi/i2c.h
>> > @@ -9,6 +9,19 @@
>> >  #include <asm/arch/cpu.h>
>> >
>> >  #define CONFIG_I2C_MVTWSI_BASE0        SUNXI_TWI0_BASE
>> > +#ifdef CONFIG_I2C1_ENABLE
>> > +#define CONFIG_I2C_MVTWSI_BASE1        SUNXI_TWI1_BASE
>> > +#endif
>> > +#ifdef CONFIG_I2C2_ENABLE
>> > +#define CONFIG_I2C_MVTWSI_BASE2        SUNXI_TWI2_BASE
>> > +#endif
>> > +#ifdef CONFIG_I2C3_ENABLE
>> > +#define CONFIG_I2C_MVTWSI_BASE3        SUNXI_TWI3_BASE
>> > +#endif
>> > +#ifdef CONFIG_I2C4_ENABLE
>> > +#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/Kconfig b/board/sunxi/Kconfig
>> > index ccc2080..d3b5bad 100644
>> > --- a/board/sunxi/Kconfig
>> > +++ b/board/sunxi/Kconfig
>> > @@ -269,6 +269,37 @@ config USB2_VBUS_PIN
>> >         ---help---
>> >         See USB1_VBUS_PIN help text.
>> >
>> > +config I2C1_ENABLE
>> > +       bool "Enable I2C/TWI controller 1"
>> > +       default n
>> > +       ---help---
>> > +       This allows enabling I2C/TWI controller 1 by muxing its pins, enabling
>> > +       its clock and setting up the bus. This is especially useful on devices
>> > +       with slaves connected to the bus or with pins exposed through e.g. an
>> > +       expansion port/header.
>> > +
>> > +config I2C2_ENABLE
>> > +       bool "Enable I2C/TWI controller 2"
>> > +       default n
>> > +       ---help---
>> > +       See I2C1_ENABLE help text.
>> > +
>> > +if MACH_SUN6I || MACH_SUN7I
>> > +config I2C3_ENABLE
>> > +       bool "Enable I2C/TWI controller 3"
>> > +       default n
>> > +       ---help---
>> > +       See I2C1_ENABLE help text.
>> > +endif
>> > +
>> > +if MACH_SUN7I
>> > +config I2C4_ENABLE
>> > +       bool "Enable I2C/TWI controller 4"
>> > +       default n
>> > +       ---help---
>> > +       See I2C1_ENABLE help text.
>> > +endif
>>
>> It seems wrong to me to add these when they are already in the device
>> tree for the board. Can we not use that?
>
> Well, Hans has a point when saying that some users may use those pins as
> GPIO while some others may use the TWI/I2C functions, so it makes sense
> to make this configurable via Kconfig instead of being statically
> defined.

I don't see the device tree as static in the sense that people can
modify it or create their own just as with Kconfig.

>
>> If you would rather wait until we have driver model I2C on sunxi
>> (mvtwsi, I think) then I'd be happy to do the conversion. It's pretty
>> easy.
>
> I would be happy to see U-Boot on sunxi use devicetree and driver model
> for TWI/I2C as well (provided users can still configure what busses to
> enable). Still, I'd like to see this getting merged as a short term
> solution.

I did some initial work here (pcDuino3) so it should be easy enough to
use it more widely.

The problem with the short term solution is that it's unclear when it
gets revisited. I'd like to see this tidied up rather than creating
additional debt for someone to unpick. It is actually quite hard to
make these sort of changes.

>
>> How can we get sunxi moved over before there is an explosion of these
>> sorts of things (as we have already seen with video options)?
>
> I think Hans will know better (than myself) how to do this right.

In this case we could add a function like

int funcmux_setup_i2c(enum periph_id id)

which sets the I2C pins up for the specified I2C port. Then you can
call this when probing the relevant I2C port.

>
>> > +
>> >  config VIDEO
>> >         boolean "Enable graphical uboot console on HDMI, LCD or VGA"
>> >         default y
>> > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
>> > index 986261a..0fb458b 100644
>> > --- a/board/sunxi/board.c
>> > +++ b/board/sunxi/board.c
>> > @@ -276,9 +276,80 @@ 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
>> > +
>> > +#ifdef CONFIG_I2C1_ENABLE
>> > +#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
>> > +#endif
>> > +
>> > +#ifdef CONFIG_I2C2_ENABLE
>> > +#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
>> > +#endif
>> > +
>> > +#ifdef CONFIG_I2C3_ENABLE
>> > +#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
>> > +#endif
>> > +
>> > +#ifdef CONFIG_I2C4_ENABLE
>> > +#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
>> > +#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);
>> > --

Regards,
Simon


More information about the U-Boot mailing list