Hi Fredetick,
On Mon, Feb 21, 2011 at 10:37:43PM +0000, Frederick van der Wyck wrote:
> This adds backlight control on the Samsung Q10 laptop, which does not support
> the SABI interface. Also tested successfully on the Dell Latitude X200.
>
Thanks for making the changes, looks mostly good now.
> +
> +static int force;
If it is a bool the call it so.
> +module_param(force, bool, 0);
> +MODULE_PARM_DESC(force,
> + "Disable the DMI check and force the driver to be loaded");
> +
> +static int samsungq10_bl_set_intensity(struct backlight_device *bd)
> +{
> +
> + int brightness = bd->props.brightness;
> + unsigned char c[3] = SAMSUNGQ10_BL_8042_DATA;
> +
> + if (!samsungq10_suspended) {
Hmm, if samsungq10_bl_set_intensity() can indeed run simultaneously with
suspend/resume then you'd need to protect it with spinlock or mutex.
> + c[2] = (unsigned char)brightness;
> + i8042_lock_chip();
> + i8042_command(c, (0x30 << 8) | SAMSUNGQ10_BL_8042_CMD);
> + i8042_unlock_chip();
> + samsungq10_bl_brightness = brightness;
> + }
> + return 0;
> +}
> +
> +static int samsungq10_bl_get_intensity(struct backlight_device *bd)
> +{
> + return samsungq10_bl_brightness;
> +}
> +
> +static const struct backlight_ops samsungq10_bl_ops = {
> + .get_brightness = samsungq10_bl_get_intensity,
> + .update_status = samsungq10_bl_set_intensity,
> +};
> +
> +#ifdef CONFIG_PM
> +static int samsungq10_suspend(struct device *dev)
> +{
> + samsungq10_suspended = true;
> + return 0;
> +}
> +
> +static int samsungq10_resume(struct device *dev)
> +{
> +
> + struct backlight_device *bd = dev_get_drvdata(dev);
> +
> + samsungq10_suspended = false;
> + samsungq10_bl_set_intensity(bd);
> + return 0;
> +}
> +#else
> +#define samsungq10_suspend NULL
> +#define samsungq10_resume NULL
> +#endif
> +
> +static const struct dev_pm_ops samsungq10_pm_ops = {
> + .suspend = samsungq10_suspend,
> + .resume = samsungq10_resume,
> +};
Don't you need to restore backlight when resuming from hibernate? I
think you need to guard you methods with CONFIG_PM_SLEEP and then
use SIMPLE_DEV_PM_OPS().
> +
> +static int __devinit samsungq10_probe(struct platform_device *pdev)
> +{
> +
> + struct backlight_properties props;
> + struct backlight_device *bd;
> +
> + memset(&props, 0, sizeof(struct backlight_properties));
> + props.max_brightness = SAMSUNGQ10_BL_MAX_INTENSITY;
> + bd = backlight_device_register("samsung", &pdev->dev, NULL,
> + &samsungq10_bl_ops, &props);
> + if (IS_ERR(bd))
> + return PTR_ERR(bd);
> +
> + platform_set_drvdata(pdev, bd);
> +
> + bd->props.brightness = SAMSUNGQ10_BL_DEFAULT_INTENSITY;
> + samsungq10_bl_set_intensity(bd);
> +
> + return 0;
> +}
> +
> +static int __devexit samsungq10_remove(struct platform_device *pdev)
> +{
> +
> + struct backlight_device *bd = platform_get_drvdata(pdev);
> +
> + bd->props.brightness = SAMSUNGQ10_BL_DEFAULT_INTENSITY;
> + samsungq10_bl_set_intensity(bd);
> +
> + backlight_device_unregister(bd);
> +
> + return 0;
> +}
> +
> +static struct platform_driver samsungq10_driver = {
> + .driver = {
> + .name = "samsung",
Samsung is way too generic for driver name, maybe call it
KBUILD_MODNAME?
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
