Patchwork [U-Boot,v2,06/21] pmic: Enable power_board_init() support at TRATS

login
register
mail settings
Submitter Łukasz Majewski
Date Oct. 5, 2012, 8:16 a.m.
Message ID <1349425003-32523-7-git-send-email-l.majewski@samsung.com>
Download mbox | patch
Permalink /patch/189430/
State Changes Requested
Delegated to: Stefano Babic
Headers show

Comments

Łukasz Majewski - Oct. 5, 2012, 8:16 a.m.
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(-)
Stefano Babic - Oct. 9, 2012, 8:56 a.m.
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
Łukasz Majewski - Oct. 9, 2012, 10:30 a.m.
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
>
Stefano Babic - Oct. 9, 2012, 11:37 a.m.
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
Łukasz Majewski - Oct. 9, 2012, 12:06 p.m.
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
> 
>

Patch

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