Re: [PATCH] platform: fix samsung brightness min/max calculations

[Jason Stubbs <jasonbstubbs@gmail.com>]

Not sure of the "bump" process, so I'll just go over each part of the patch.

Note that all the changes only affect the sabi_config where min_brightness
is 1 so you might not see in difference on your hardware.

On Wed, 20 Apr 2011 13:58:50 Jason Stubbs wrote:
> --- a/drivers/platform/x86/samsung-laptop.c
> +++ b/drivers/platform/x86/samsung-laptop.c
> @@ -370,15 +370,17 @@ static u8 read_brightness(void)
>  				  &sretval);
>  	if (!retval) {
>  		user_brightness = sretval.retval[0];
> -		if (user_brightness != 0)
> +		if (user_brightness > sabi_config->min_brightness)
>  			user_brightness -= sabi_config->min_brightness;
> +		else
> +			user_brightness = 0;

min_brightness is (and probably will always be) only 0 or 1 in the current
sabi_configs, so this patch doesn't actually change present behaviour. Should
there ever be a min_brightness > 1, however, this patch will be required or
user_brightness will end up going below 0 and wrapping around.

>  static void set_brightness(u8 user_brightness)
>  {
> -	u8 user_level = user_brightness - sabi_config->min_brightness;
> +	u8 user_level = user_brightness + sabi_config->min_brightness;

The sign here is definitely wrong. If the user sets a brightness of 0 when
using a sabi_config with a min_brightness of 1, user_level will become -1 and
that is the value that will be passed to the hardware.

> @@ -782,7 +784,8 @@ static int __init samsung_init(void)
>  	/* create a backlight device to talk to this one */
>  	memset(&props, 0, sizeof(struct backlight_properties));
>  	props.type = BACKLIGHT_PLATFORM;
> -	props.max_brightness = sabi_config->max_brightness;
> +	props.max_brightness = sabi_config->max_brightness -
> +				sabi_config->min_brightness;

Without subtracting the min_brightness, the user is offered a range of 0
through 8 for both sabi_configs. For the min_brightness == 1 case and with
set_brightness() fixed, a user setting of 8 would actually set 9 in hardware.

Regards,
Jason Stubbs