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

Paul Kocialkowski contact at paulk.fr
Sat Apr 4 22:51:52 CEST 2015 

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150404/54a15d59/attachment.sig>

More information about the U-Boot mailing list