| Message ID | 1349425003-32523-7-git-send-email-l.majewski@samsung.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
On 05/10/2012 10:16, Lukasz Majewski wrote: > Enable support for power_board_init() method at TRATS board. > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > --- > Changes for v2: > - None > --- > board/samsung/trats/trats.c | 15 +++++++++++---- > include/configs/trats.h | 1 + > 2 files changed, 12 insertions(+), 4 deletions(-) > > diff --git a/board/samsung/trats/trats.c b/board/samsung/trats/trats.c > index 80ec4ad..0285668 100644 > --- a/board/samsung/trats/trats.c > +++ b/board/samsung/trats/trats.c > @@ -68,10 +68,6 @@ int board_init(void) > check_hw_revision(); > printf("HW Revision:\t0x%x\n", board_rev); > > -#if defined(CONFIG_PMIC) > - pmic_init(I2C_5); > -#endif > - > return 0; > } > > @@ -90,6 +86,17 @@ void i2c_init_board(void) > s5p_gpio_direction_output(&gpio2->y4, 1, 1); > } > > +#ifdef CONFIG_POWER_INIT > +int power_board_init(void) > +{ > + > +#ifdef CONFIG_PMIC > + pmic_init(I2C_5); > +#endif > + return 0; > +} > +#endif > + This enforces my comment in previous patch. We have two CONFIG_ options, both must be turned on. So at least one is redundant.IMHO you can drop both of them if power_board_init() is declared weak. Best regards, Stefano Babic
Hi Stefano, > On 05/10/2012 10:16, Lukasz Majewski wrote: > > Enable support for power_board_init() method at TRATS board. > > > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> > > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > > --- > > Changes for v2: > > - None > > --- > > board/samsung/trats/trats.c | 15 +++++++++++---- > > include/configs/trats.h | 1 + > > 2 files changed, 12 insertions(+), 4 deletions(-) > > > > diff --git a/board/samsung/trats/trats.c > > b/board/samsung/trats/trats.c index 80ec4ad..0285668 100644 > > --- a/board/samsung/trats/trats.c > > +++ b/board/samsung/trats/trats.c > > @@ -68,10 +68,6 @@ int board_init(void) > > check_hw_revision(); > > printf("HW Revision:\t0x%x\n", board_rev); > > > > -#if defined(CONFIG_PMIC) > > - pmic_init(I2C_5); > > -#endif > > - > > return 0; > > } > > > > > > @@ -90,6 +86,17 @@ void i2c_init_board(void) > > s5p_gpio_direction_output(&gpio2->y4, 1, 1); > > } > > > > +#ifdef CONFIG_POWER_INIT > > +int power_board_init(void) > > +{ > > + > > +#ifdef CONFIG_PMIC > > + pmic_init(I2C_5); > > +#endif > > + return 0; > > +} > > +#endif > > + > > This enforces my comment in previous patch. We have two CONFIG_ > options, both must be turned on. So at least one is redundant.IMHO > you can drop both of them if power_board_init() is declared weak. I can define power_board_init() as __weak if you are OK with this :-). In this case for sure CONFIG_POWER_INIT could be removed. I agree that CONFIG_PMIC can be removed from this piece of code. > > Best regards, > Stefano Babic >
On 09/10/2012 12:30, Lukasz Majewski wrote: >> This enforces my comment in previous patch. We have two CONFIG_ >> options, both must be turned on. So at least one is redundant.IMHO >> you can drop both of them if power_board_init() is declared weak. > > I can define power_board_init() as __weak if you are OK with this :-). IMHO yes, and call it board_power_init(). We know directly from the name that board_* function are often declared weak. > In this case for sure CONFIG_POWER_INIT could be removed. > > I agree that CONFIG_PMIC can be removed from this piece of code. > >> Best regards, Stefano Babic
Hi Stefano, > On 09/10/2012 12:30, Lukasz Majewski wrote: > > >> This enforces my comment in previous patch. We have two CONFIG_ > >> options, both must be turned on. So at least one is redundant.IMHO > >> you can drop both of them if power_board_init() is declared weak. > > > > I can define power_board_init() as __weak if you are OK with > > this :-). > > IMHO yes, and call it board_power_init(). We know directly from the > name that board_* function are often declared weak. Ok, I will prepare v3 of this patch set. > > > > In this case for sure CONFIG_POWER_INIT could be removed. > > > > I agree that CONFIG_PMIC can be removed from this piece of code. > > > >> > > Best regards, > Stefano Babic > >
diff --git a/board/samsung/trats/trats.c b/board/samsung/trats/trats.c index 80ec4ad..0285668 100644 --- a/board/samsung/trats/trats.c +++ b/board/samsung/trats/trats.c @@ -68,10 +68,6 @@ int board_init(void) check_hw_revision(); printf("HW Revision:\t0x%x\n", board_rev); -#if defined(CONFIG_PMIC) - pmic_init(I2C_5); -#endif - return 0; } @@ -90,6 +86,17 @@ void i2c_init_board(void) s5p_gpio_direction_output(&gpio2->y4, 1, 1); } +#ifdef CONFIG_POWER_INIT +int power_board_init(void) +{ + +#ifdef CONFIG_PMIC + pmic_init(I2C_5); +#endif + return 0; +} +#endif + int dram_init(void) { gd->ram_size = get_ram_size((long *)PHYS_SDRAM_1, PHYS_SDRAM_1_SIZE) + diff --git a/include/configs/trats.h b/include/configs/trats.h index b3b5a3d..37a655a 100644 --- a/include/configs/trats.h +++ b/include/configs/trats.h @@ -235,6 +235,7 @@ #define CONFIG_SOFT_I2C_GPIO_SDA get_multi_sda_pin() #define I2C_INIT multi_i2c_init() +#define CONFIG_POWER_INIT #define CONFIG_PMIC #define CONFIG_PMIC_I2C #define CONFIG_PMIC_MAX8997