Hi, On 07/16/2014 02:03 PM, Bjørn Mork wrote: > Hans de Goede <hdegoede@xxxxxxxxxx> writes: > >> Hi All, >> >> Here is my cleaned-up version of Linus' patch to fix the 2 steps on a >> brightness key press problem. >> >> Changes compared to Linus' version: >> -Move the global variables to inside struct acpi_video_device >> -Rebase on top of Rafael's linux-pm linux-next branch (so on top of the >> revert of changing the brightness_switch_enabled default) >> >> I've tested this on a laptop affected by the 2 steps problem and I can confirm >> that it fixes the 2 steps issue under normal usage. I managed to trigger the >> race by generating a heavy io-load, as exptected hitting the race has no >> unwanted side-effects other then taking 2 steps instead of one. >> >> Bjørn can you test this patch on your system and confirm that it does not >> break things for you please ? Note that in order to apply it you first need >> to do: "git revert 886129a8eebebec", as the revert has not yet reached >> Linus' tree AFAIK. > > Tested and verified that it still works fine for me, so feel free to add > > Tested-by: Bjørn Mork <bjorn@xxxxxxx> > if you like. > Thanks, if there is a need for a v2 I'll add your tested-by. Rafael, if there is not going to be a v2, could you pick up this tested-by then? > But as noted earlier, I believe this poses another risk if anyone has a > config using bool input values: > > -static bool brightness_switch_enabled = 1; > -module_param(brightness_switch_enabled, bool, 0644); > +static int brightness_switch_enabled = -1; > +module_param(brightness_switch_enabled, int, 0644); > The only way I can see that being a problem is if someone is using video.brightness_switch_enabled=N for reasons other then the 2 step problem this patch fixes. That is not unthinkable though. > I wonder if it wouldn't be better to just redefine the behaviour of > "brightness_switch_enabled = 1" to the new delayed response scheme and > avoid changing the type of this parameter? I think that is a good idea, that would also allow to simplify the patch somewhat more. Rafael, what is your take on this ? Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
