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 <[email protected]> > >> > >> 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
> >
signature.asc
Description: This is a digitally signed message part
_______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
