Skip to contentSkip to conversation

imxfb: correct location of callbacks in suspend and resume

PATCH New Uwe Kleine-König 3 participants 6 messages Jan 11, 2010 – Jan 28, 2010

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 imxfb: correct location of callbacks in suspend and resume

The probe function passes a pointer to a struct fb_info to platform_set_drvdata, so don't interpret the return value of platform_get_drvdata as a pointer to struct imxfb_info.

Cc Sascha Hauer <[email protected]>
Cc linux-arm-kernel at lists.infradead.org
---
 drivers/video/imxfb.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
drivers/video/imxfb.c +4 -2
index 66358fa..b4b6dec 100644
@@ -593,7 +593,8 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
  */
 static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
 {
-	struct imxfb_info *fbi = platform_get_drvdata(dev);
+	struct fb_info *info = platform_get_drvdata(dev);
+	struct imxfb_info *fbi = info->par;
 
 	pr_debug("%s\n", __func__);
 
@@ -603,7 +604,8 @@ static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
 
 static int imxfb_resume(struct platform_device *dev)
 {
-	struct imxfb_info *fbi = platform_get_drvdata(dev);
+	struct fb_info *info = platform_get_drvdata(dev);
+	struct imxfb_info *fbi = info->par;
 
 	pr_debug("%s\n", __func__);
 
Signature · 1 line
1.6.5.2
maintainer
#1 lore raw

PATCH 1/1 imxfb: correct location of callbacks in suspend and resume

Quoted 7 lines · Uwe Kleine-K?nig

The probe function passes a pointer to a struct fb_info to platform_set_drvdata, so don't interpret the return value of platform_get_drvdata as a pointer to struct imxfb_info.

Cc Sascha Hauer <[email protected]>
Cc linux-arm-kernel at lists.infradead.org
Quoted 32 lines
---
 drivers/video/imxfb.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
drivers/video/imxfb.c +4 -2
index 66358fa..b4b6dec 100644
@@ -593,7 +593,8 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
  */
 static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
 {
-	struct imxfb_info *fbi = platform_get_drvdata(dev);
+	struct fb_info *info = platform_get_drvdata(dev);
+	struct imxfb_info *fbi = info->par;
 
 	pr_debug("%s\n", __func__);
 
@@ -603,7 +604,8 @@ static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
 
 static int imxfb_resume(struct platform_device *dev)
 {
-	struct imxfb_info *fbi = platform_get_drvdata(dev);
+	struct fb_info *info = platform_get_drvdata(dev);
+	struct imxfb_info *fbi = info->par;
 
 	pr_debug("%s\n", __func__);
 
Signature · 1 line
1.6.5.2
Signature · 4 lines
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
#2 lore raw

PATCH 1/1 imxfb: correct location of callbacks in suspend and resume

In reply to Uwe Kleine-König

The probe function passes a pointer to a struct fb_info to platform_set_drvdata, so don't interpret the return value of platform_get_drvdata as a pointer to struct imxfb_info.

Cc linux-arm-kernel at lists.infradead.org
---
 drivers/video/imxfb.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
drivers/video/imxfb.c +4 -2
index 66358fa..b4b6dec 100644
@@ -593,7 +593,8 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
  */
 static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
 {
-	struct imxfb_info *fbi = platform_get_drvdata(dev);
+	struct fb_info *info = platform_get_drvdata(dev);
+	struct imxfb_info *fbi = info->par;
 
 	pr_debug("%s\n", __func__);
 
@@ -603,7 +604,8 @@ static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
 
 static int imxfb_resume(struct platform_device *dev)
 {
-	struct imxfb_info *fbi = platform_get_drvdata(dev);
+	struct fb_info *info = platform_get_drvdata(dev);
+	struct imxfb_info *fbi = info->par;
 
 	pr_debug("%s\n", __func__);
 
Signature · 1 line
1.6.6
#3 lore raw
Quoted 26 lines · On Mon, 25 Jan 2010 16:37:34 +0100 Uwe Kleine-K__nig…

The probe function passes a pointer to a struct fb_info to platform_set_drvdata, so don't interpret the return value of platform_get_drvdata as a pointer to struct imxfb_info.

Cc linux-arm-kernel at lists.infradead.org
---
 drivers/video/imxfb.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
drivers/video/imxfb.c +2 -1
index 66358fa..b4b6dec 100644
@@ -593,7 +593,8 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
  */
 static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
 {
-	struct imxfb_info *fbi = platform_get_drvdata(dev);
+	struct fb_info *info = platform_get_drvdata(dev);
+	struct imxfb_info *fbi = info->par;
 
 	pr_debug("%s\n", __func__);
 

drivers/video/imxfb.c static int imxfb_suspend(struct platform_device *dev, pm_message_t state) · line 611

Quoted 9 lines
drivers/video/imxfb.c +2 -1
@@ -603,7 +604,8 @@ static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
 
 static int imxfb_resume(struct platform_device *dev)
 {
-	struct imxfb_info *fbi = platform_get_drvdata(dev);
+	struct fb_info *info = platform_get_drvdata(dev);
+	struct imxfb_info *fbi = info->par;
 
 	pr_debug("%s\n", __func__);
 

What were the runtime effects of this bug?

#4 lore raw

Hello,

Quoted 36 lines · Andrew Morton
Quoted 33 lines · On Mon, 25 Jan 2010 16:37:34 +0100 Uwe Kleine-K__nig…

The probe function passes a pointer to a struct fb_info to platform_set_drvdata, so don't interpret the return value of platform_get_drvdata as a pointer to struct imxfb_info.

Cc linux-arm-kernel at lists.infradead.org
---
 drivers/video/imxfb.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
drivers/video/imxfb.c +4 -2
index 66358fa..b4b6dec 100644
@@ -593,7 +593,8 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
  */
 static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
 {
-	struct imxfb_info *fbi = platform_get_drvdata(dev);
+	struct fb_info *info = platform_get_drvdata(dev);
+	struct imxfb_info *fbi = info->par;
 
 	pr_debug("%s\n", __func__);
 
@@ -603,7 +604,8 @@ static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
 
 static int imxfb_resume(struct platform_device *dev)
 {
-	struct imxfb_info *fbi = platform_get_drvdata(dev);
+	struct fb_info *info = platform_get_drvdata(dev);
+	struct imxfb_info *fbi = info->par;
 
Quoted 4 lines

drivers/video/imxfb.c static int imxfb_suspend(struct platform_device *dev, pm_message_t state) · line 611

Quoted 2 lines
drivers/video/imxfb.c +0 -0
 	pr_debug("%s\n", __func__);
 

What were the runtime effects of this bug?

For me it was that in the original imxfb_info *fbi backlight_power was NULL but in imxfb_suspend it was 4 resulting in an oops as imxfb_suspend calls imxfb_disable_controller(fbi) which in turn has

	if (fbi->backlight_power)
		fbi->backlight_power(0);

.

Best regards
Uwe

Signature · 2 lines
Pengutronix e.K.                              | Uwe Kleine-K?nig            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |
#5 lore raw
Quoted 7 lines · On Thu, 28 Jan 2010 08:41:01 +0100 Uwe Kleine-K__nig…
Quoted 1 line

What were the runtime effects of this bug?

For me it was that in the original imxfb_info *fbi backlight_power was NULL but in imxfb_suspend it was 4 resulting in an oops as imxfb_suspend calls imxfb_disable_controller(fbi) which in turn has

	if (fbi->backlight_power)
		fbi->backlight_power(0);

OK, thanks, that was important info.

I rescheduled the patch for 2.6.33 and marked it for 2.6.32.x backporting.