Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[Patch 0/3] backlight

5 views
Skip to first unread message

Bruno Prémont

unread,
Feb 20, 2010, 6:40:01 PM2/20/10
to
This series improves backlight_ops's check_fb function, marks
backlight_ops as const and fixes error checking when registering
backlight device.

Bruno
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Bruno Prémont

unread,
Feb 20, 2010, 6:40:02 PM2/20/10
to
Check newly registered backlight_device for error and properly
return error to parent

Signed-off-by: Bruno Prémont <bon...@linux-vserver.org>
---
drivers/platform/x86/classmate-laptop.c | 2 ++
drivers/platform/x86/msi-wmi.c | 4 +++-
drivers/platform/x86/panasonic-laptop.c | 4 +++-
drivers/usb/misc/appledisplay.c | 1 +
drivers/video/bf54x-lq043fb.c | 8 ++++++++
drivers/video/bfin-t350mcqb-fb.c | 8 ++++++++
6 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/classmate-laptop.c b/drivers/platform/x86/classmate-laptop.c
index bfae789..66f6aad 100644
--- a/drivers/platform/x86/classmate-laptop.c
+++ b/drivers/platform/x86/classmate-laptop.c
@@ -463,6 +463,8 @@ static int cmpc_bl_add(struct acpi_device *acpi)

bd = backlight_device_register("cmpc_bl", &acpi->dev,
acpi->handle, &cmpc_bl_ops);
+ if (IS_ERR(bd))
+ return PTR_ERR(bd);
bd->props.max_brightness = 7;
dev_set_drvdata(&acpi->dev, bd);
return 0;
diff --git a/drivers/platform/x86/msi-wmi.c b/drivers/platform/x86/msi-wmi.c
index 5f7cff1..2ffbfcf 100644
--- a/drivers/platform/x86/msi-wmi.c
+++ b/drivers/platform/x86/msi-wmi.c
@@ -251,8 +251,10 @@ static int __init msi_wmi_init(void)
if (!acpi_video_backlight_support()) {
backlight = backlight_device_register(DRV_NAME,
NULL, NULL, &msi_backlight_ops);
- if (IS_ERR(backlight))
+ if (IS_ERR(backlight)) {
+ err = PTR_ERR(backlight);
goto err_free_input;
+ }

backlight->props.max_brightness = ARRAY_SIZE(backlight_map) - 1;
err = bl_get(NULL);
diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
index 9012d8d..9b17343 100644
--- a/drivers/platform/x86/panasonic-laptop.c
+++ b/drivers/platform/x86/panasonic-laptop.c
@@ -644,8 +644,10 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
/* initialize backlight */
pcc->backlight = backlight_device_register("panasonic", NULL, pcc,
&pcc_backlight_ops);
- if (IS_ERR(pcc->backlight))
+ if (IS_ERR(pcc->backlight)) {
+ result = PTR_ERR(pcc->backlight);
goto out_input;
+ }

if (!acpi_pcc_retrieve_biosdata(pcc, pcc->sinf)) {
ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
diff --git a/drivers/usb/misc/appledisplay.c b/drivers/usb/misc/appledisplay.c
index ef79ca2..413f154 100644
--- a/drivers/usb/misc/appledisplay.c
+++ b/drivers/usb/misc/appledisplay.c
@@ -283,6 +283,7 @@ static int appledisplay_probe(struct usb_interface *iface,
&appledisplay_bl_data);
if (IS_ERR(pdata->bd)) {
dev_err(&iface->dev, "Backlight registration failed\n");
+ retval = PTR_ERR(pdata->bd);
goto error;
}

diff --git a/drivers/video/bf54x-lq043fb.c b/drivers/video/bf54x-lq043fb.c
index db4e6f7..a589216 100644
--- a/drivers/video/bf54x-lq043fb.c
+++ b/drivers/video/bf54x-lq043fb.c
@@ -678,6 +678,12 @@ static int __devinit bfin_bf54x_probe(struct platform_device *pdev)
bl_dev =
backlight_device_register("bf54x-bl", NULL, NULL,
&bfin_lq043fb_bl_ops);
+ if (IS_ERR(bl_dev)) {
+ printk(KERN_ERR DRIVER_NAME
+ ": unable to register backlight.\n");
+ ret = -EINVAL;
+ goto out9;
+ }
bl_dev->props.max_brightness = 255;

lcd_dev = lcd_device_register(DRIVER_NAME, &pdev->dev, NULL, &bfin_lcd_ops);
@@ -686,6 +692,8 @@ static int __devinit bfin_bf54x_probe(struct platform_device *pdev)

return 0;

+out9:
+ unregister_framebuffer(fbinfo);
out8:
free_irq(info->irq, info);
out7:
diff --git a/drivers/video/bfin-t350mcqb-fb.c b/drivers/video/bfin-t350mcqb-fb.c
index bc204c6..fe1492b 100644
--- a/drivers/video/bfin-t350mcqb-fb.c
+++ b/drivers/video/bfin-t350mcqb-fb.c
@@ -572,6 +572,12 @@ static int __devinit bfin_t350mcqb_probe(struct platform_device *pdev)
bl_dev =
backlight_device_register("bf52x-bl", NULL, NULL,
&bfin_lq043fb_bl_ops);
+ if (IS_ERR(bl_dev)) {
+ printk(KERN_ERR DRIVER_NAME
+ ": unable to register backlight.\n");
+ ret = -EINVAL;
+ goto out9;
+ }
bl_dev->props.max_brightness = 255;

lcd_dev = lcd_device_register(DRIVER_NAME, NULL, &bfin_lcd_ops);
@@ -580,6 +586,8 @@ static int __devinit bfin_t350mcqb_probe(struct platform_device *pdev)

return 0;

+out9:
+ unregister_framebuffer(fbinfo);
out8:
free_irq(info->irq, info);
out7:
--
1.6.4.4

Harald Welte

unread,
Feb 21, 2010, 4:20:02 AM2/21/10
to
On Sun, Feb 21, 2010 at 12:28:31AM +0100, Bruno Pr�mont wrote:
> Check newly registered backlight_device for error and properly
> return error to parent
>
> Signed-off-by: Bruno Pr�mont <bon...@linux-vserver.org>

Acked-by: Harald Welte <laf...@gnumonks.org>

(for panasonic-laptop.c)
--
- Harald Welte <laf...@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)

Thadeu Lima de Souza Cascardo

unread,
Feb 21, 2010, 8:50:02 AM2/21/10
to
On Sun, Feb 21, 2010 at 12:28:31AM +0100, Bruno Prémont wrote:
> Check newly registered backlight_device for error and properly
> return error to parent
>
> Signed-off-by: Bruno Prémont <bon...@linux-vserver.org>
> ---
> drivers/platform/x86/classmate-laptop.c | 2 ++
> drivers/platform/x86/msi-wmi.c | 4 +++-
> drivers/platform/x86/panasonic-laptop.c | 4 +++-
> drivers/usb/misc/appledisplay.c | 1 +
> drivers/video/bf54x-lq043fb.c | 8 ++++++++
> drivers/video/bfin-t350mcqb-fb.c | 8 ++++++++
> 6 files changed, 25 insertions(+), 2 deletions(-)
>

I think you should split the patch for every driver. Then, every
mantainer may ack only its particular section of the patch.

signature.asc

Anisse Astier

unread,
Feb 24, 2010, 10:40:03 AM2/24/10
to
On Sun, 21 Feb 2010 10:35:29 -0300, Thadeu Lima de Souza Cascardo <casc...@holoscopio.com> wrote :

> On Sun, Feb 21, 2010 at 12:28:31AM +0100, Bruno Prémont wrote:
> > Check newly registered backlight_device for error and properly
> > return error to parent
> >
> > Signed-off-by: Bruno Prémont <bon...@linux-vserver.org>
> > ---
> > drivers/platform/x86/classmate-laptop.c | 2 ++
> > drivers/platform/x86/msi-wmi.c | 4 +++-
> > drivers/platform/x86/panasonic-laptop.c | 4 +++-
> > drivers/usb/misc/appledisplay.c | 1 +
> > drivers/video/bf54x-lq043fb.c | 8 ++++++++
> > drivers/video/bfin-t350mcqb-fb.c | 8 ++++++++
> > 6 files changed, 25 insertions(+), 2 deletions(-)
> >
>
> I think you should split the patch for every driver. Then, every
> mantainer may ack only its particular section of the patch.
>

I agree.

> > diff --git a/drivers/platform/x86/msi-wmi.c b/drivers/platform/x86/msi-wmi.c
> > index 5f7cff1..2ffbfcf 100644
> > --- a/drivers/platform/x86/msi-wmi.c
> > +++ b/drivers/platform/x86/msi-wmi.c
> > @@ -251,8 +251,10 @@ static int __init msi_wmi_init(void)
> > if (!acpi_video_backlight_support()) {
> > backlight = backlight_device_register(DRV_NAME,
> > NULL, NULL, &msi_backlight_ops);
> > - if (IS_ERR(backlight))
> > + if (IS_ERR(backlight)) {
> > + err = PTR_ERR(backlight);
> > goto err_free_input;
> > + }
> >
> > backlight->props.max_brightness = ARRAY_SIZE(backlight_map) - 1;
> > err = bl_get(NULL);

Anyway, the msi-wmi part looks good to me.


Anisse

Bruno Prémont

unread,
Feb 26, 2010, 7:30:03 AM2/26/10
to
Properly return backlight registration error to parent.
Mark struct backlight_ops as const.

Signed-off-by: Bruno Prémont <bon...@linux-vserver.org>
Acked-by: Harald Welte <laf...@gnumonks.org> (registration failure)
---
drivers/platform/x86/panasonic-laptop.c | 4 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
index 9012d8d..9b17343 100644
--- a/drivers/platform/x86/panasonic-laptop.c
+++ b/drivers/platform/x86/panasonic-laptop.c

@@ -352,7 +352,7 @@ static int bl_set_status(struct backlight_device *bd)
return acpi_pcc_write_sset(pcc, SINF_DC_CUR_BRIGHT, bright);
}

-static struct backlight_ops pcc_backlight_ops = {
+static const struct backlight_ops pcc_backlight_ops = {
.get_brightness = bl_get,
.update_status = bl_set_status,
};


@@ -644,8 +644,10 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
/* initialize backlight */
pcc->backlight = backlight_device_register("panasonic", NULL, pcc,
&pcc_backlight_ops);
- if (IS_ERR(pcc->backlight))
+ if (IS_ERR(pcc->backlight)) {
+ result = PTR_ERR(pcc->backlight);
goto out_input;
+ }

if (!acpi_pcc_retrieve_biosdata(pcc, pcc->sinf)) {
ACPI_DEBUG_PRINT((ACPI_DB_ERROR,

Bruno Prémont

unread,
Feb 26, 2010, 7:30:05 AM2/26/10
to
On error while registering backlight, return it to caller instead of
returning 0.

Mark struct backlight_ops as const.

Signed-off-by: Bruno Prémont <bon...@linux-vserver.org>
---
drivers/usb/misc/appledisplay.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/misc/appledisplay.c b/drivers/usb/misc/appledisplay.c
index 1eb9e41..ef79ca2 100644
--- a/drivers/usb/misc/appledisplay.c
+++ b/drivers/usb/misc/appledisplay.c
@@ -179,7 +179,7 @@ static int appledisplay_bl_get_brightness(struct backlight_device *bd)
return pdata->msgdata[1];
}

-static struct backlight_ops appledisplay_bl_data = {
+static const struct backlight_ops appledisplay_bl_data = {
.get_brightness = appledisplay_bl_get_brightness,
.update_status = appledisplay_bl_update_status,
};


@@ -283,6 +283,7 @@ static int appledisplay_probe(struct usb_interface *iface,
&appledisplay_bl_data);
if (IS_ERR(pdata->bd)) {
dev_err(&iface->dev, "Backlight registration failed\n");
+ retval = PTR_ERR(pdata->bd);
goto error;
}

Bruno Prémont

unread,
Feb 26, 2010, 7:30:04 AM2/26/10
to
Check newly registered backlight_device for error and properly
return error to parent
Mark struct backlight_ops as const.

Signed-off-by: Bruno Prémont <bon...@linux-vserver.org>
Acked-by: Mike Frysinger <vap...@gentoo.org> (constify struct backlight_ops)
---
drivers/video/bf54x-lq043fb.c | 10 +++++++++-
drivers/video/bfin-t350mcqb-fb.c | 10 +++++++++-
2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/video/bf54x-lq043fb.c b/drivers/video/bf54x-lq043fb.c
index db4e6f7..a589216 100644
--- a/drivers/video/bf54x-lq043fb.c
+++ b/drivers/video/bf54x-lq043fb.c

@@ -463,7 +463,7 @@ static int bl_get_brightness(struct backlight_device *bd)
return 0;
}

-static struct backlight_ops bfin_lq043fb_bl_ops = {
+static const struct backlight_ops bfin_lq043fb_bl_ops = {
.get_brightness = bl_get_brightness,
};



@@ -678,6 +678,12 @@ static int __devinit bfin_bf54x_probe(struct platform_device *pdev)
bl_dev =
backlight_device_register("bf54x-bl", NULL, NULL,
&bfin_lq043fb_bl_ops);
+ if (IS_ERR(bl_dev)) {
+ printk(KERN_ERR DRIVER_NAME
+ ": unable to register backlight.\n");
+ ret = -EINVAL;
+ goto out9;
+ }
bl_dev->props.max_brightness = 255;

lcd_dev = lcd_device_register(DRIVER_NAME, &pdev->dev, NULL, &bfin_lcd_ops);
@@ -686,6 +692,8 @@ static int __devinit bfin_bf54x_probe(struct platform_device *pdev)

return 0;

+out9:
+ unregister_framebuffer(fbinfo);
out8:
free_irq(info->irq, info);
out7:
diff --git a/drivers/video/bfin-t350mcqb-fb.c b/drivers/video/bfin-t350mcqb-fb.c
index bc204c6..fe1492b 100644
--- a/drivers/video/bfin-t350mcqb-fb.c
+++ b/drivers/video/bfin-t350mcqb-fb.c

@@ -381,7 +381,7 @@ static int bl_get_brightness(struct backlight_device *bd)
return 0;
}

-static struct backlight_ops bfin_lq043fb_bl_ops = {
+static const struct backlight_ops bfin_lq043fb_bl_ops = {
.get_brightness = bl_get_brightness,
};

Bruno Prémont

unread,
Feb 26, 2010, 7:30:04 AM2/26/10
to
Check newly registered backlight_device for error and properly
return error to parent.

Mark struct backlight_ops as const.

Signed-off-by: Bruno Prémont <bon...@linux-vserver.org>
---
drivers/platform/x86/classmate-laptop.c | 2 +++-
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/platform/x86/classmate-laptop.c b/drivers/platform/x86/classmate-laptop.c
index bfae789..66f6aad 100644
--- a/drivers/platform/x86/classmate-laptop.c
+++ b/drivers/platform/x86/classmate-laptop.c

@@ -452,7 +452,7 @@ static int cmpc_bl_update_status(struct backlight_device *bd)
return -1;
}

-static struct backlight_ops cmpc_bl_ops = {
+static const struct backlight_ops cmpc_bl_ops = {
.get_brightness = cmpc_bl_get_brightness,
.update_status = cmpc_bl_update_status
};


@@ -463,6 +463,8 @@ static int cmpc_bl_add(struct acpi_device *acpi)

bd = backlight_device_register("cmpc_bl", &acpi->dev,
acpi->handle, &cmpc_bl_ops);
+ if (IS_ERR(bd))
+ return PTR_ERR(bd);
bd->props.max_brightness = 7;
dev_set_drvdata(&acpi->dev, bd);
return 0;

Bruno Prémont

unread,
Feb 26, 2010, 7:30:04 AM2/26/10
to
Properly return backlight registration error to parent.

Mark struct backlight_ops as const.

Signed-off-by: Bruno Prémont <bon...@linux-vserver.org>
Reviewed-by: Anisse Astier <ani...@astier.eu>
---
drivers/platform/x86/msi-laptop.c | 2 +-
drivers/platform/x86/msi-wmi.c | 6 ++++--
2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
index 759763d..1093ba2 100644
--- a/drivers/platform/x86/msi-laptop.c
+++ b/drivers/platform/x86/msi-laptop.c
@@ -161,7 +161,7 @@ static int bl_update_status(struct backlight_device *b)
return set_lcd_level(b->props.brightness);
}

-static struct backlight_ops msibl_ops = {
+static const struct backlight_ops msibl_ops = {
.get_brightness = bl_get_brightness,
.update_status = bl_update_status,
};
diff --git a/drivers/platform/x86/msi-wmi.c b/drivers/platform/x86/msi-wmi.c
index f5f70d4..5f7cff1 100644
--- a/drivers/platform/x86/msi-wmi.c
+++ b/drivers/platform/x86/msi-wmi.c
@@ -138,7 +138,7 @@ static int bl_set_status(struct backlight_device *bd)
return msi_wmi_set_block(0, backlight_map[bright]);
}

-static struct backlight_ops msi_backlight_ops = {
+static const struct backlight_ops msi_backlight_ops = {


.get_brightness = bl_get,
.update_status = bl_set_status,
};

@@ -251,8 +251,10 @@ static int __init msi_wmi_init(void)
if (!acpi_video_backlight_support()) {
backlight = backlight_device_register(DRV_NAME,
NULL, NULL, &msi_backlight_ops);
- if (IS_ERR(backlight))
+ if (IS_ERR(backlight)) {
+ err = PTR_ERR(backlight);
goto err_free_input;
+ }

backlight->props.max_brightness = ARRAY_SIZE(backlight_map) - 1;
err = bl_get(NULL);

Thadeu Lima de Souza Cascardo

unread,
Feb 26, 2010, 11:30:02 AM2/26/10
to
Mathew,

Please apply the following patch.

Acked-by: Thadeu Lima de Souza Cascardo <casc...@holoscopio.com>

signature.asc

Matthew Garrett

unread,
Feb 26, 2010, 11:40:02 AM2/26/10
to
If Richard applies my patch to rework backlight registration, this won't
apply. Richard, what's the situation there?

--
Matthew Garrett | mj...@srcf.ucam.org

Richard Purdie

unread,
Feb 26, 2010, 11:50:02 AM2/26/10
to
On Fri, 2010-02-26 at 16:32 +0000, Matthew Garrett wrote:
> If Richard applies my patch to rework backlight registration, this won't
> apply. Richard, what's the situation there?

Your patch has priority, publishing new patches at this point in time is
getting a bit late.

Cheers,

Richard

Bruno Prémont

unread,
Feb 26, 2010, 5:30:02 PM2/26/10
to
Hi Richard,

On Fri, 26 February 2010 Richard Purdie <rpu...@rpsys.net> wrote:
> On Fri, 2010-02-26 at 16:32 +0000, Matthew Garrett wrote:
> > If Richard applies my patch to rework backlight registration, this
> > won't apply. Richard, what's the situation there?
>
> Your patch has priority, publishing new patches at this point in time
> is getting a bit late.

I've been resending them, though I got no feedback from you on first
submission about a week ago. (on any of the backlight patches, so
I'm wondering if they reached you at all...)

Regards,
Bruno

0 new messages