On Wednesday 03 December 2014 09:43:21 Darren Hart wrote: > On Sun, Nov 23, 2014 at 03:50:45PM +0100, Pali Rohár wrote: > > This patch adds support for configuring keyboard backlight > > settings on supported Dell laptops. It exports kernel leds > > interface and uses Dell SMBIOS tokens or keyboard class > > interface. > > > > With this patch it is possible to set: > > * keyboard backlight level > > * timeout after which will be backlight automatically turned > > off * input activity triggers (keyboard, touchpad, mouse) > > which enable backlight * ambient light settings > > > > Settings are exported via sysfs: > > /sys/class/leds/dell::kbd_backlight/ > > > > Code is based on newly released documentation by Dell in > > libsmbios project. > > Hi Pali, > > I would really like to see this broken up. Possibly adding > levels and timeouts as separate patches for example. It is > quite difficult to review this all at once in a reasonable > amount of time. > It is really hard to split this patch into more parts which every one will compile and ideally also work. In my opinion every single git commit should be possible to compile and should also work (it helps also other developers to use git bisect). And because we need to pass all (previous unchanged) values in smbios call we need to parse all of them. I understand that it could be hard to review long patch, but there are more parts which interacts and do not work without other. Also some of settings (e.g keyboard backlight level) could be changed via different ways (and on some machines only one is working) and also that smbios keyboard interface has complicated logic... > During this review I caught a few more CodingStyle violations, > and raised some questions about the timeout and levels > mechanisms. > I will fix style problems in next v3 patch. What to do with Dan Carpenter patch which fixing kbd_timeout handling? Should I integrate it into v3? It does not apply on top of next v3 patch, because there will be new comment about quirk plus style fixes... -- Pali Rohár pali.rohar@xxxxxxxxx
Attachment:
signature.asc
Description: This is a digitally signed message part.
