On Tue, 2007-02-06 at 14:30 +0100, Marcin Juszkiewicz wrote:
> From: Marcin Juszkiewicz <[email protected]>
>
> Add control of LCD backlight for Frontpath ProGear HX1050+.
> Patch is based on http://downloads.sf.net/progear/progear-lcd-0.2.tar.gz
> driver by M Schacht.
>
> Signed-Off-By: Marcin Juszkiewicz <[email protected]>
This is very similar to corgi_bl which I have some changes I was
considering to make it a bit more efficient/clean. The same changes
could apply to this driver:
> +static int progearbl_intensity;
> +static struct backlight_properties progearbl_data;
Not sure you need the above line?
> +static struct backlight_device *progear_backlight_device;
You shouldn't need this structure pointer (see below)
> +static int progearbl_get_intensity(struct backlight_device *bd)
> +{
> + return progearbl_intensity;
> +}
Can you read PMU_LPCR and return it (minus HW_LEVEL_MIN) here? corgi_bl
only stores this as it can't read back from the hardware. Either
approach is probably ok (although if the register can be updated by
other means, it should be read).
> +static int progearbl_set_intensity(struct backlight_device *bd)
> +{
> + progearbl_send_intensity(progear_backlight_device);
> + return 0;
> +}
You can use progearbl_send_intensity(bd); here or maybe lose the
function all together?
> +static struct backlight_properties progearbl_data = {
> + .owner = THIS_MODULE,
> + .get_brightness = progearbl_get_intensity,
> + .update_status = progearbl_set_intensity,
progearbl_send_intensity?
> + pci_read_config_byte(sb_dev, SB_MPS1, &temp);
> + pci_write_config_byte(sb_dev, SB_MPS1, temp | 0x20);
> +
> + progear_backlight_device = backlight_device_register("progear-bl",
> + &pdev->dev, NULL, &progearbl_data);
> + if (IS_ERR(progear_backlight_device))
> + return PTR_ERR(progear_backlight_device);
platform_set_drvdata(pdev, progear_backlight_device);
> + progearbl_data.power = FB_BLANK_UNBLANK;
> + progearbl_data.brightness = HW_LEVEL_MAX - HW_LEVEL_MIN;
> + progearbl_data.max_brightness = HW_LEVEL_MAX - HW_LEVEL_MIN;
> + progearbl_send_intensity(progear_backlight_device);
> +
> + return 0;
> +}
> +
> +static int progearbl_remove(struct platform_device *dev)
> +{
> + backlight_device_unregister(progear_backlight_device);
struct backlight_device *bd = platform_get_drvdata(pdev);
backlight_device_unregister(bd);
> + return 0;
> +}
All pretty minor though and if you change the above, it can have an
Acked-by: Richard Purdie <[email protected]>
--
Richard
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
