Patchwork [U-Boot] universal_c210: fix compiler error and compiler warning

login
register
mail settings
Submitter Minkyu Kang
Date Dec. 10, 2012, 6:50 a.m.
Message ID <50C58623.3090308@samsung.com>
Download mbox | patch
Permalink /patch/204823/
State Accepted
Delegated to: Minkyu Kang
Headers show

Comments

Minkyu Kang - Dec. 10, 2012, 6:50 a.m.
This patch fix following errors

universal.c: In function 'init_pmic_lcd':
universal.c:340: warning: implicit declaration of function 'get_pmic'
universal.c:340: warning: initialization makes pointer from integer without a cast
universal.c: In function 'lcd_power_on':
universal.c:431: warning: initialization makes pointer from integer without a cast
universal.c: At top level:
universal.c:335: warning: 'init_pmic_lcd' defined but not used

Signed-off-by: Minkyu Kang <mk7.kang@samsung.com>
Cc: Donghwa Lee <dh09.lee@samsung.com>
Cc: Lukasz Majewski <l.majewski@samsung.com>
---
 board/samsung/universal_c210/universal.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
Wolfgang Denk - Dec. 10, 2012, 9:06 a.m.
Dear Minkyu Kang,

In message <50C58623.3090308@samsung.com> you wrote:
...
> @@ -337,7 +341,7 @@ static void init_pmic_lcd(void)
>  	unsigned char val;
>  	int ret = 0;
>  
> -	struct pmic *p = get_pmic();
> +	struct pmic *p = pmic_get("MAX8998_PMIC");
>  
>  	if (pmic_probe(p))
>  		return;
> @@ -428,7 +432,7 @@ static void reset_lcd(void)
>  
>  static void lcd_power_on(void)
>  {
> -	struct pmic *p = get_pmic();
> +	struct pmic *p = pmic_get("MAX8998_PMIC");
>  
>  	if (pmic_probe(p))
>  		return;

This is unrelated to your patch - but what if pmic_get() returns NULL?

pmic_probe() will crashif you pass it a NULL pointer...

Best regards,

Wolfgang Denk
Ɓukasz Majewski - Dec. 10, 2012, 9:32 a.m.
Hi Wolfgang,

> Dear Minkyu Kang,
> 
> In message <50C58623.3090308@samsung.com> you wrote:
> ...
> > @@ -337,7 +341,7 @@ static void init_pmic_lcd(void)
> >  	unsigned char val;
> >  	int ret = 0;
> >  
> > -	struct pmic *p = get_pmic();
> > +	struct pmic *p = pmic_get("MAX8998_PMIC");
> >  
> >  	if (pmic_probe(p))
> >  		return;
> > @@ -428,7 +432,7 @@ static void reset_lcd(void)
> >  
> >  static void lcd_power_on(void)
> >  {
> > -	struct pmic *p = get_pmic();
> > +	struct pmic *p = pmic_get("MAX8998_PMIC");
> >  
> >  	if (pmic_probe(p))
> >  		return;
> 
> This is unrelated to your patch - but what if pmic_get() returns NULL?
> 
> pmic_probe() will crashif you pass it a NULL pointer...

The PMIC 2.0 uses malloc to allocate pmic structure.

The fix, which has been proposed would work for old pmic.

In the new one PMIC 2.0, we require to test return pointer from
pmic_get() (similar to all malloc allocations):

struct pmic *p = pmic_get("MAX8998_PMIC");
if (!p)
	return -ENODEV;


> 
> Best regards,
> 
> Wolfgang Denk
>
Minkyu Kang - Dec. 10, 2012, 9:59 a.m.
Dear Wolfgang,

On 10/12/12 18:32, Lukasz Majewski wrote:
> Hi Wolfgang,
> 
>> Dear Minkyu Kang,
>>
>> In message <50C58623.3090308@samsung.com> you wrote:
>> ...
>>> @@ -337,7 +341,7 @@ static void init_pmic_lcd(void)
>>>  	unsigned char val;
>>>  	int ret = 0;
>>>  
>>> -	struct pmic *p = get_pmic();
>>> +	struct pmic *p = pmic_get("MAX8998_PMIC");
>>>  
>>>  	if (pmic_probe(p))
>>>  		return;
>>> @@ -428,7 +432,7 @@ static void reset_lcd(void)
>>>  
>>>  static void lcd_power_on(void)
>>>  {
>>> -	struct pmic *p = get_pmic();
>>> +	struct pmic *p = pmic_get("MAX8998_PMIC");
>>>  
>>>  	if (pmic_probe(p))
>>>  		return;
>>
>> This is unrelated to your patch - but what if pmic_get() returns NULL?
>>
>> pmic_probe() will crashif you pass it a NULL pointer...
> 
> The PMIC 2.0 uses malloc to allocate pmic structure.
> 
> The fix, which has been proposed would work for old pmic.
> 
> In the new one PMIC 2.0, we require to test return pointer from
> pmic_get() (similar to all malloc allocations):
> 
> struct pmic *p = pmic_get("MAX8998_PMIC");
> if (!p)
> 	return -ENODEV;
> 

I will fix it by another patch.

Thanks.
Minkyu Kang.
Piotr Wilczek - Dec. 10, 2012, 10:28 a.m.
On 12/10/2012 07:50 AM, Minkyu Kang wrote:
> This patch fix following errors
>
> universal.c: In function 'init_pmic_lcd':
> universal.c:340: warning: implicit declaration of function 'get_pmic'
> universal.c:340: warning: initialization makes pointer from integer without a cast
> universal.c: In function 'lcd_power_on':
> universal.c:431: warning: initialization makes pointer from integer without a cast
> universal.c: At top level:
> universal.c:335: warning: 'init_pmic_lcd' defined but not used
>
> Signed-off-by: Minkyu Kang <mk7.kang@samsung.com>
> Cc: Donghwa Lee <dh09.lee@samsung.com>
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> ---
>   board/samsung/universal_c210/universal.c |    8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/board/samsung/universal_c210/universal.c b/board/samsung/universal_c210/universal.c
> index 3d508be..4869798 100644
> --- a/board/samsung/universal_c210/universal.c
> +++ b/board/samsung/universal_c210/universal.c
> @@ -55,6 +55,8 @@ static int get_hwrev(void)
>   	return board_rev & 0xFF;
>   }
>
> +static void init_pmic_lcd(void);
> +
>   int power_init_board(void)
>   {
>   	int ret;
> @@ -63,6 +65,8 @@ int power_init_board(void)
>   	if (ret)
>   		return ret;
>
> +	init_pmic_lcd();
> +
>   	return 0;
>   }
>
> @@ -337,7 +341,7 @@ static void init_pmic_lcd(void)
>   	unsigned char val;
>   	int ret = 0;
>
> -	struct pmic *p = get_pmic();
> +	struct pmic *p = pmic_get("MAX8998_PMIC");
>
>   	if (pmic_probe(p))
>   		return;
> @@ -428,7 +432,7 @@ static void reset_lcd(void)
>
>   static void lcd_power_on(void)
>   {
> -	struct pmic *p = get_pmic();
> +	struct pmic *p = pmic_get("MAX8998_PMIC");
>
>   	if (pmic_probe(p))
>   		return;
>
Tested-by: Piotr Wilczek <p.wilczek@samsung.com>
Wolfgang Denk - Dec. 10, 2012, 10:44 a.m.
Dear Lukasz Majewski,

In message <20121210103224.372c572e@amdc308.digital.local> you wrote:
> 
> > This is unrelated to your patch - but what if pmic_get() returns NULL?
> > 
> > pmic_probe() will crashif you pass it a NULL pointer...
> 
> The PMIC 2.0 uses malloc to allocate pmic structure.

...and malloc() can fail.

> The fix, which has been proposed would work for old pmic.
> 
> In the new one PMIC 2.0, we require to test return pointer from
> pmic_get() (similar to all malloc allocations):
> 
> struct pmic *p = pmic_get("MAX8998_PMIC");
> if (!p)
> 	return -ENODEV;

So this code here needs fixing.  This is what I wanted to point out.

Best regards,

Wolfgang Denk
Minkyu Kang - Dec. 11, 2012, 4:01 a.m.
On 10/12/12 15:50, Minkyu Kang wrote:
> This patch fix following errors
> 
> universal.c: In function 'init_pmic_lcd':
> universal.c:340: warning: implicit declaration of function 'get_pmic'
> universal.c:340: warning: initialization makes pointer from integer without a cast
> universal.c: In function 'lcd_power_on':
> universal.c:431: warning: initialization makes pointer from integer without a cast
> universal.c: At top level:
> universal.c:335: warning: 'init_pmic_lcd' defined but not used
> 
> Signed-off-by: Minkyu Kang <mk7.kang@samsung.com>
> Cc: Donghwa Lee <dh09.lee@samsung.com>
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> ---
>  board/samsung/universal_c210/universal.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 

applied to u-boot-samsung/resolve

Thanks.
Minkyu Kang.

Patch

diff --git a/board/samsung/universal_c210/universal.c b/board/samsung/universal_c210/universal.c
index 3d508be..4869798 100644
--- a/board/samsung/universal_c210/universal.c
+++ b/board/samsung/universal_c210/universal.c
@@ -55,6 +55,8 @@  static int get_hwrev(void)
 	return board_rev & 0xFF;
 }
 
+static void init_pmic_lcd(void);
+
 int power_init_board(void)
 {
 	int ret;
@@ -63,6 +65,8 @@  int power_init_board(void)
 	if (ret)
 		return ret;
 
+	init_pmic_lcd();
+
 	return 0;
 }
 
@@ -337,7 +341,7 @@  static void init_pmic_lcd(void)
 	unsigned char val;
 	int ret = 0;
 
-	struct pmic *p = get_pmic();
+	struct pmic *p = pmic_get("MAX8998_PMIC");
 
 	if (pmic_probe(p))
 		return;
@@ -428,7 +432,7 @@  static void reset_lcd(void)
 
 static void lcd_power_on(void)
 {
-	struct pmic *p = get_pmic();
+	struct pmic *p = pmic_get("MAX8998_PMIC");
 
 	if (pmic_probe(p))
 		return;