On Mon, Mar 12, 2018 at 6:46 PM, Matan Ziv-Av <matan@xxxxxxxxxxx> wrote: > > Adds a driver for hotkeys, leds, battery charge limiter and fan mode on > LG Gram laptops. Thanks for the patch, my concerns below. > Documentation/laptops/lg-laptop.txt | 71 +++ This should be in reST file format. > drivers/platform/x86/lg-laptop.c | 724 ++++++++++++++++++++++++++++++++++++ You have too many subtle issues, such as: - redundant empty lines - missed empty lines - redundant conditionals (before kstrtox() call) - shadowing returned error codes (kstrtox() usage) - etc, I even would not like to look at So, please, run checkpatch.pl and see if it complains about anything. Besides that, the approaches you choose might be old or racy: - device attribute handling (use sysfs attribute group) - btw, where is the ABI description? Why did you choose so complex way to instantiate a driver? All those ACPI handlers and additional burden for platform device... Can it be simplified? Can you utilize WMI bus as much as possible. -- With Best Regards, Andy Shevchenko
