universal_c210: fix compiler error and compiler warning

U-Boot Minkyu Kang 4 participants 7 messages Dec 10, 2012 – Dec 11, 2012

Share & tools

Minkyu Kang

author

2012-12-10 06:50 UTC #0 lore raw

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 <[email protected]>

Cc Donghwa Lee <[email protected]>

Cc Lukasz Majewski <[email protected]>

---
 board/samsung/universal_c210/universal.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

board/samsung/universal_c210/universal.c +6 -2

index 3d508be..4869798 100644
@@ -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;

Signature · 1 line

1.7.9.5

Wolfgang Denk

2012-12-10 09:06 UTC #1 lore raw

Dear Minkyu Kang,

In message <[email protected]> you wrote:
...

Quoted 10 lines

board/samsung/universal_c210/universal.c +1 -1

@@ -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;

board/samsung/universal_c210/universal.c static void reset_lcd(void) · line 438

Quoted 8 lines

board/samsung/universal_c210/universal.c +1 -1

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

Signature · 7 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Digital computers are themselves more complex than most things people
build: They have very large numbers of states. This makes conceiving,
describing, and testing them hard. Software systems  have  orders-of-
magnitude more states than computers do.           - Fred Brooks, Jr.

Lukasz Majewski

2012-12-10 09:32 UTC #2 lore raw

Hi Wolfgang,

Quoted 20 lines

Dear Minkyu Kang,

In message <[email protected]> you wrote:
...

Quoted 16 lines

board/samsung/universal_c210/universal.c +2 -2

@@ -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");

Quoted 6 lines

Review static void reset_lcd(void) · line 438
Quoted 2 lines
(quoted diff) +0 -0
 	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;

Quoted 5 lines

Best regards,
Wolfgang Denk

Signature · 5 lines

Best regards,

Lukasz Majewski

Samsung Poland R&D Center | Linux Platform Group

Minkyu Kang

author

2012-12-10 09:59 UTC #3 lore raw

Dear Wolfgang,

Quoted 31 lines · Lukasz Majewski

Hi Wolfgang,
Quoted 20 lines
Dear Minkyu Kang,
In message <[email protected]> you wrote:
...
Quoted 16 lines
board/samsung/universal_c210/universal.c +2 -2
@@ -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");
 
Quoted 6 lines
Review static void reset_lcd(void) · line 438
Quoted 2 lines
(quoted diff) +0 -0
 	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.

Quoted 9 lines

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.

Wolfgang Denk

2012-12-10 10:44 UTC #4 lore raw

In reply to Lukasz Majewski

Dear Lukasz Majewski,

Quoted 6 lines · In message <[email protected]> you wrote:

Quoted 4 lines
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.

Quoted 8 lines

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

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"If you want to eat hippopatomus, you've got to pay the  freight."  -
attributed to an IBM guy, about why IBM software uses so much memory

Piotr Wilczek

Tested-by

2012-12-10 10:28 UTC #5 lore raw

In reply to Minkyu Kang

Quoted 50 lines · Minkyu Kang

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 <[email protected]>
Cc Donghwa Lee <[email protected]>
Cc Lukasz Majewski <[email protected]>
---
  board/samsung/universal_c210/universal.c |    8 ++++++--
  1 file changed, 6 insertions(+), 2 deletions(-)
board/samsung/universal_c210/universal.c +5 -1
index 3d508be..4869798 100644
@@ -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;

board/samsung/universal_c210/universal.c static void reset_lcd(void) · line 438

Quoted 8 lines

board/samsung/universal_c210/universal.c +1 -1

@@ -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 <[email protected]>

Minkyu Kang

author

2012-12-11 04:01 UTC #6 lore raw

In reply to Minkyu Kang

Quoted 10 lines · Minkyu Kang

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

Quoted 7 lines

Signed-off-by Minkyu Kang <[email protected]>
Cc Donghwa Lee <[email protected]>
Cc Lukasz Majewski <[email protected]>
---
 board/samsung/universal_c210/universal.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

applied to u-boot-samsung/resolve

Thanks.
Minkyu Kang.