Patchwork [U-Boot,6/6] WIP: arm: trats: add support for new I2C framework

login
register
mail settings
Submitter Piotr Wilczek
Date Nov. 15, 2012, 8:15 a.m.
Message ID <1352967325-26426-7-git-send-email-p.wilczek@samsung.com>
Download mbox | patch
Permalink /patch/199218/
State Superseded
Delegated to: Heiko Schocher
Headers show

Comments

Piotr Wilczek - Nov. 15, 2012, 8:15 a.m.
This enables new i2c framework on Trats board.
Hardware s3c24x0 i2c driver is used instead of software i2c.

Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
CC: Minkyu Kang <mk7.kang@samsung.com>
---
 board/samsung/trats/trats.c |   26 +++++++++++++++++++++-----
 include/configs/trats.h     |   16 ++++++++++++++--
 2 files changed, 35 insertions(+), 7 deletions(-)
Heiko Schocher - Nov. 16, 2012, 10:45 a.m.
Hello Piotr,

On 15.11.2012 09:15, Piotr Wilczek wrote:
> This enables new i2c framework on Trats board.
> Hardware s3c24x0 i2c driver is used instead of software i2c.
>
> Signed-off-by: Piotr Wilczek<p.wilczek@samsung.com>
> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
> CC: Minkyu Kang<mk7.kang@samsung.com>
> ---
>   board/samsung/trats/trats.c |   26 +++++++++++++++++++++-----
>   include/configs/trats.h     |   16 ++++++++++++++--
>   2 files changed, 35 insertions(+), 7 deletions(-)
>
> diff --git a/board/samsung/trats/trats.c b/board/samsung/trats/trats.c
> index e11a892..71e1975 100644
> --- a/board/samsung/trats/trats.c
> +++ b/board/samsung/trats/trats.c
> @@ -24,8 +24,10 @@
>    */
>
>   #include<common.h>
> +#include<i2c.h>
>   #include<lcd.h>
>   #include<asm/io.h>
> +#include<asm/arch/pinmux.h>
>   #include<asm/arch/cpu.h>
>   #include<asm/arch/gpio.h>
>   #include<asm/arch/mmc.h>
> @@ -61,6 +63,19 @@ static int hwrevision(int rev)
>
>   struct s3c_plat_otg_data s5pc210_otg_data;
>
> +void i2c_init_pinmux(void)
> +{
> +	int i, err;
> +	for (i = 0; i<  CONFIG_MAX_I2C_NUM; i++) {
> +		err = exynos_pinmux_config((PERIPH_ID_I2C0 + i),
> +						PINMUX_FLAG_NONE);
> +		if (err) {
> +			debug("I2C%d not configured\n", (PERIPH_ID_I2C0 + i));
> +			return;
> +		}
> +	}
> +}
> +

Is this change multibus specific? Why?

>   int board_init(void)
>   {
>   	gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;
> @@ -68,6 +83,8 @@ int board_init(void)
>   	check_hw_revision();
>   	printf("HW Revision:\t0x%x\n", board_rev);
>
> +	i2c_init_pinmux();
> +

Why this call not in i2c_init_board(void)?

>   #if defined(CONFIG_PMIC)
>   	pmic_init();
>   #endif
> @@ -77,14 +94,9 @@ int board_init(void)
>
>   void i2c_init_board(void)
>   {
> -	struct exynos4_gpio_part1 *gpio1 =
> -		(struct exynos4_gpio_part1 *)samsung_get_base_gpio_part1();
>   	struct exynos4_gpio_part2 *gpio2 =
>   		(struct exynos4_gpio_part2 *)samsung_get_base_gpio_part2();
>
> -	/* I2C_5 ->  PMIC */
> -	s5p_gpio_direction_output(&gpio1->b, 7, 1);
> -	s5p_gpio_direction_output(&gpio1->b, 6, 1);

Here again, what has this to do with the new multibus framework?

>   	/* I2C_9 ->  FG */
>   	s5p_gpio_direction_output(&gpio2->y4, 0, 1);
>   	s5p_gpio_direction_output(&gpio2->y4, 1, 1);
> @@ -415,6 +427,8 @@ static int lcd_power(void)
>   	int ret = 0;
>   	struct pmic *p = get_pmic();
>
> +	i2c_set_bus_num(0);
                         ^
                         Fix number? Please use here a define.

> +
>   	if (pmic_probe(p))
>   		return 0;
>
> @@ -475,6 +489,8 @@ static int mipi_power(void)
>   	int ret = 0;
>   	struct pmic *p = get_pmic();
>
> +	i2c_set_bus_num(0);
> +
>   	if (pmic_probe(p))
>   		return 0;
>
> diff --git a/include/configs/trats.h b/include/configs/trats.h
> index 106fd37..54423f4 100644
> --- a/include/configs/trats.h
> +++ b/include/configs/trats.h
> @@ -33,6 +33,7 @@
>   #define CONFIG_SAMSUNG		/* in a SAMSUNG core */
>   #define CONFIG_S5P		/* which is in a S5P Family */
>   #define CONFIG_EXYNOS4210	/* which is in a EXYNOS4210 */
> +#define CONFIG_EXYNOS4
>   #define CONFIG_TRATS		/* working with TRATS */
>   #define CONFIG_TIZEN		/* TIZEN lib */
>
> @@ -211,18 +212,29 @@
>   #define CONFIG_SYS_INIT_SP_ADDR	(CONFIG_SYS_LOAD_ADDR - GENERATED_GBL_DATA_SIZE)
>   #define CONFIG_SYS_CACHELINE_SIZE       32
>
> -
>   #define CONFIG_SYS_I2C
>   #define CONFIG_SYS_I2C_SOFT		/* I2C bit-banged */
>   #define CONFIG_SYS_I2C_SOFT_SPEED	50000
>   #define CONFIG_SYS_I2C_SOFT_SLAVE	0xFE
> -#define CONFIG_SYS_I2C_ADAPTERS		{&soft_i2c_adap[0]}

Why you delete here this adapter? You can use now in the new
Framework Hard and soft i2c adapter ...

>   #define CONFIG_SOFT_I2C_READ_REPEATED_START
>   #define CONFIG_SYS_I2C_INIT_BOARD
>   #define CONFIG_I2C_MULTI_BUS
>   #define CONFIG_SOFT_I2C_MULTI_BUS
>   #define CONFIG_SYS_MAX_I2C_BUS	15
>
> +#define CONFIG_HARD_I2C

No longer needed

> +#define CONFIG_DRIVER_S3C24X0_I2C
> +#define CONFIG_MAX_I2C_NUM	8
> +#define CONFIG_SYS_I2C_SPEED	100000
> +#define CONFIG_SYS_I2C_SLAVE	0x0

This defines please rename, as posted in previous EMail ...

> +#define CONFIG_CMD_I2C
> +
> +#define CONFIG_SYS_I2C_ADAPTERS		{&s3c24x0_i2c_adap[5]}
> +#define CONFIG_SYS_NUM_I2C_ADAPTERS 1
> +#define CONFIG_SYS_I2C_BUSSES	{{0, {{{0, "S3C24X0"}, 0x00, 5}}}, \

You use here only one bus, so you do not need to define this here.

> +				}
> +#define CONFIG_SYS_NUM_I2C_BUSSES 1
> +
>   #include<asm/arch/gpio.h>
>
>   /* I2C PMIC */

bye,
Heiko
Piotr Wilczek - Nov. 16, 2012, 12:51 p.m.
Hello Heiko,

> -----Original Message-----
> From: Heiko Schocher [mailto:hs@denx.de]
> Sent: Friday, November 16, 2012 11:46 AM
> To: Piotr Wilczek
> Cc: u-boot@lists.denx.de; Minkyu Kang; Kyungmin Park; Lukasz Majewski;
> Simon Glass; Stephen Warren; Tom Rini
> Subject: Re: [PATCH 6/6] WIP: arm: trats: add support for new I2C
> framework
> 
> Hello Piotr,
> 
> On 15.11.2012 09:15, Piotr Wilczek wrote:
> > This enables new i2c framework on Trats board.
> > Hardware s3c24x0 i2c driver is used instead of software i2c.
> >
> > Signed-off-by: Piotr Wilczek<p.wilczek@samsung.com>
> > Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
> > CC: Minkyu Kang<mk7.kang@samsung.com>
> > ---
> >   board/samsung/trats/trats.c |   26 +++++++++++++++++++++-----
> >   include/configs/trats.h     |   16 ++++++++++++++--
> >   2 files changed, 35 insertions(+), 7 deletions(-)
> >
> > diff --git a/board/samsung/trats/trats.c
> b/board/samsung/trats/trats.c
> > index e11a892..71e1975 100644
> > --- a/board/samsung/trats/trats.c
> > +++ b/board/samsung/trats/trats.c
> > @@ -24,8 +24,10 @@
> >    */
> >
> >   #include<common.h>
> > +#include<i2c.h>
> >   #include<lcd.h>
> >   #include<asm/io.h>
> > +#include<asm/arch/pinmux.h>
> >   #include<asm/arch/cpu.h>
> >   #include<asm/arch/gpio.h>
> >   #include<asm/arch/mmc.h>
> > @@ -61,6 +63,19 @@ static int hwrevision(int rev)
> >
> >   struct s3c_plat_otg_data s5pc210_otg_data;
> >
> > +void i2c_init_pinmux(void)
> > +{
> > +	int i, err;
> > +	for (i = 0; i<  CONFIG_MAX_I2C_NUM; i++) {
> > +		err = exynos_pinmux_config((PERIPH_ID_I2C0 + i),
> > +						PINMUX_FLAG_NONE);
> > +		if (err) {
> > +			debug("I2C%d not configured\n", (PERIPH_ID_I2C0 +
> i));
> > +			return;
> > +		}
> > +	}
> > +}
> > +
> 
> Is this change multibus specific? Why?
GPIO must be set to I2C function when hardware I2C driver is used.
Also, because only one hardware I2C bus is used I should change pins
function for I2C_5 only.

> 
> >   int board_init(void)
> >   {
> >   	gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100; @@ -68,6 +83,8 @@
> > int board_init(void)
> >   	check_hw_revision();
> >   	printf("HW Revision:\t0x%x\n", board_rev);
> >
> > +	i2c_init_pinmux();
> > +
> 
> Why this call not in i2c_init_board(void)?
Ok

> 
> >   #if defined(CONFIG_PMIC)
> >   	pmic_init();
> >   #endif
> > @@ -77,14 +94,9 @@ int board_init(void)
> >
> >   void i2c_init_board(void)
> >   {
> > -	struct exynos4_gpio_part1 *gpio1 =
> > -		(struct exynos4_gpio_part1 *)samsung_get_base_gpio_part1();
> >   	struct exynos4_gpio_part2 *gpio2 =
> >   		(struct exynos4_gpio_part2 *)samsung_get_base_gpio_part2();
> >
> > -	/* I2C_5 ->  PMIC */
> > -	s5p_gpio_direction_output(&gpio1->b, 7, 1);
> > -	s5p_gpio_direction_output(&gpio1->b, 6, 1);
> 
> Here again, what has this to do with the new multibus framework?
Currently only software I2c is used and pins must be set as outputs.
With this patchset I switch from software to hardware I2C for PMIC and I
must change pins function from output to I2C.

> 
> >   	/* I2C_9 ->  FG */
> >   	s5p_gpio_direction_output(&gpio2->y4, 0, 1);
> >   	s5p_gpio_direction_output(&gpio2->y4, 1, 1); @@ -415,6 +427,8 @@
> > static int lcd_power(void)
> >   	int ret = 0;
> >   	struct pmic *p = get_pmic();
> >
> > +	i2c_set_bus_num(0);
>                          ^
>                          Fix number? Please use here a define.
Ok

> 
> > +
> >   	if (pmic_probe(p))
> >   		return 0;
> >
> > @@ -475,6 +489,8 @@ static int mipi_power(void)
> >   	int ret = 0;
> >   	struct pmic *p = get_pmic();
> >
> > +	i2c_set_bus_num(0);
> > +
> >   	if (pmic_probe(p))
> >   		return 0;
> >
> > diff --git a/include/configs/trats.h b/include/configs/trats.h index
> > 106fd37..54423f4 100644
> > --- a/include/configs/trats.h
> > +++ b/include/configs/trats.h
> > @@ -33,6 +33,7 @@
> >   #define CONFIG_SAMSUNG		/* in a SAMSUNG core */
> >   #define CONFIG_S5P		/* which is in a S5P Family */
> >   #define CONFIG_EXYNOS4210	/* which is in a EXYNOS4210 */
> > +#define CONFIG_EXYNOS4
> >   #define CONFIG_TRATS		/* working with TRATS */
> >   #define CONFIG_TIZEN		/* TIZEN lib */
> >
> > @@ -211,18 +212,29 @@
> >   #define CONFIG_SYS_INIT_SP_ADDR	(CONFIG_SYS_LOAD_ADDR -
> GENERATED_GBL_DATA_SIZE)
> >   #define CONFIG_SYS_CACHELINE_SIZE       32
> >
> > -
> >   #define CONFIG_SYS_I2C
> >   #define CONFIG_SYS_I2C_SOFT		/* I2C bit-banged */
> >   #define CONFIG_SYS_I2C_SOFT_SPEED	50000
> >   #define CONFIG_SYS_I2C_SOFT_SLAVE	0xFE
> > -#define CONFIG_SYS_I2C_ADAPTERS		{&soft_i2c_adap[0]}
> 
> Why you delete here this adapter? You can use now in the new Framework
> Hard and soft i2c adapter ...
Yes, soft adapter should be kept.

> 
> >   #define CONFIG_SOFT_I2C_READ_REPEATED_START
> >   #define CONFIG_SYS_I2C_INIT_BOARD
> >   #define CONFIG_I2C_MULTI_BUS
> >   #define CONFIG_SOFT_I2C_MULTI_BUS
> >   #define CONFIG_SYS_MAX_I2C_BUS	15
> >
> > +#define CONFIG_HARD_I2C
> 
> No longer needed
Ok

> 
> > +#define CONFIG_DRIVER_S3C24X0_I2C
> > +#define CONFIG_MAX_I2C_NUM	8
> > +#define CONFIG_SYS_I2C_SPEED	100000
> > +#define CONFIG_SYS_I2C_SLAVE	0x0
> 
> This defines please rename, as posted in previous EMail ...
Ok

> 
> > +#define CONFIG_CMD_I2C
> > +
> > +#define CONFIG_SYS_I2C_ADAPTERS		{&s3c24x0_i2c_adap[5]}
> > +#define CONFIG_SYS_NUM_I2C_ADAPTERS 1
> > +#define CONFIG_SYS_I2C_BUSSES	{{0, {{{0, "S3C24X0"}, 0x00, 5}}}, \
> 
> You use here only one bus, so you do not need to define this here.
Just testing, I will remove it.

> 
> > +				}
> > +#define CONFIG_SYS_NUM_I2C_BUSSES 1
> > +
> >   #include<asm/arch/gpio.h>
> >
> >   /* I2C PMIC */
> 
> bye,
> Heiko
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Best regards,
Piotr Wilczek

Patch

diff --git a/board/samsung/trats/trats.c b/board/samsung/trats/trats.c
index e11a892..71e1975 100644
--- a/board/samsung/trats/trats.c
+++ b/board/samsung/trats/trats.c
@@ -24,8 +24,10 @@ 
  */
 
 #include <common.h>
+#include <i2c.h>
 #include <lcd.h>
 #include <asm/io.h>
+#include <asm/arch/pinmux.h>
 #include <asm/arch/cpu.h>
 #include <asm/arch/gpio.h>
 #include <asm/arch/mmc.h>
@@ -61,6 +63,19 @@  static int hwrevision(int rev)
 
 struct s3c_plat_otg_data s5pc210_otg_data;
 
+void i2c_init_pinmux(void)
+{
+	int i, err;
+	for (i = 0; i < CONFIG_MAX_I2C_NUM; i++) {
+		err = exynos_pinmux_config((PERIPH_ID_I2C0 + i),
+						PINMUX_FLAG_NONE);
+		if (err) {
+			debug("I2C%d not configured\n", (PERIPH_ID_I2C0 + i));
+			return;
+		}
+	}
+}
+
 int board_init(void)
 {
 	gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;
@@ -68,6 +83,8 @@  int board_init(void)
 	check_hw_revision();
 	printf("HW Revision:\t0x%x\n", board_rev);
 
+	i2c_init_pinmux();
+
 #if defined(CONFIG_PMIC)
 	pmic_init();
 #endif
@@ -77,14 +94,9 @@  int board_init(void)
 
 void i2c_init_board(void)
 {
-	struct exynos4_gpio_part1 *gpio1 =
-		(struct exynos4_gpio_part1 *)samsung_get_base_gpio_part1();
 	struct exynos4_gpio_part2 *gpio2 =
 		(struct exynos4_gpio_part2 *)samsung_get_base_gpio_part2();
 
-	/* I2C_5 -> PMIC */
-	s5p_gpio_direction_output(&gpio1->b, 7, 1);
-	s5p_gpio_direction_output(&gpio1->b, 6, 1);
 	/* I2C_9 -> FG */
 	s5p_gpio_direction_output(&gpio2->y4, 0, 1);
 	s5p_gpio_direction_output(&gpio2->y4, 1, 1);
@@ -415,6 +427,8 @@  static int lcd_power(void)
 	int ret = 0;
 	struct pmic *p = get_pmic();
 
+	i2c_set_bus_num(0);
+
 	if (pmic_probe(p))
 		return 0;
 
@@ -475,6 +489,8 @@  static int mipi_power(void)
 	int ret = 0;
 	struct pmic *p = get_pmic();
 
+	i2c_set_bus_num(0);
+
 	if (pmic_probe(p))
 		return 0;
 
diff --git a/include/configs/trats.h b/include/configs/trats.h
index 106fd37..54423f4 100644
--- a/include/configs/trats.h
+++ b/include/configs/trats.h
@@ -33,6 +33,7 @@ 
 #define CONFIG_SAMSUNG		/* in a SAMSUNG core */
 #define CONFIG_S5P		/* which is in a S5P Family */
 #define CONFIG_EXYNOS4210	/* which is in a EXYNOS4210 */
+#define CONFIG_EXYNOS4
 #define CONFIG_TRATS		/* working with TRATS */
 #define CONFIG_TIZEN		/* TIZEN lib */
 
@@ -211,18 +212,29 @@ 
 #define CONFIG_SYS_INIT_SP_ADDR	(CONFIG_SYS_LOAD_ADDR - GENERATED_GBL_DATA_SIZE)
 #define CONFIG_SYS_CACHELINE_SIZE       32
 
-
 #define CONFIG_SYS_I2C
 #define CONFIG_SYS_I2C_SOFT		/* I2C bit-banged */
 #define CONFIG_SYS_I2C_SOFT_SPEED	50000
 #define CONFIG_SYS_I2C_SOFT_SLAVE	0xFE
-#define CONFIG_SYS_I2C_ADAPTERS		{&soft_i2c_adap[0]}
 #define CONFIG_SOFT_I2C_READ_REPEATED_START
 #define CONFIG_SYS_I2C_INIT_BOARD
 #define CONFIG_I2C_MULTI_BUS
 #define CONFIG_SOFT_I2C_MULTI_BUS
 #define CONFIG_SYS_MAX_I2C_BUS	15
 
+#define CONFIG_HARD_I2C
+#define CONFIG_DRIVER_S3C24X0_I2C
+#define CONFIG_MAX_I2C_NUM	8
+#define CONFIG_SYS_I2C_SPEED	100000
+#define CONFIG_SYS_I2C_SLAVE	0x0
+#define CONFIG_CMD_I2C
+
+#define CONFIG_SYS_I2C_ADAPTERS		{&s3c24x0_i2c_adap[5]}
+#define CONFIG_SYS_NUM_I2C_ADAPTERS 1
+#define CONFIG_SYS_I2C_BUSSES	{{0, {{{0, "S3C24X0"}, 0x00, 5}}}, \
+				}
+#define CONFIG_SYS_NUM_I2C_BUSSES 1
+
 #include <asm/arch/gpio.h>
 
 /* I2C PMIC */