Hi,
On 12/16/20 2:39 AM, Barnabás Pőcze wrote:
> This series contains patches that aim to bring more consistency
> to the code; add keyboard backlight control support; add
> "always on USB charging" control support.
> Furthermore, commit 7f363145992cebf4ea760447f1cfdf6f81459683 is reverted
> since it made it impossible to disable/enable the touchpad via the
> ideapad-laptop module and on some devices the method implemented in the
> module works correctly to disable/enable the touchpad.
Thank you for this series, it is good to see all the
cleanups, as well as to see the new functionality.
Patches 1-20 and 22-24 look good to me and you may add my:
Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
To them for v2 of this patch-set.
I have some remarks about patch 21 I will reply to that one
separately.
One minor remark about patch 3/24, normally we put all
the #include <linux/foo.h> includes first (sorted
alphabetically as you already do in the patch) and then
follow up by other / subsys specific include such as
acpi/video.h. Again sorted alphabetically for the file-names
after the subsys dir. I don't think there really is any
preferred order for which subsys headers to include first,
but typically the generic linux/foo.h headers are included
first.
Regards,
Hans
p.s.
About merging this series vs other outstanding ideapad-laptop
changes. The other outstanding changes are quite small, so easy
to rebase. As such I would actually prefer to merge this series
first. So if you can send out a v2 soon-ish, then that would be
great.
>
> Barnabás Pőcze (24):
> platform/x86: ideapad-laptop: remove unnecessary dev_set_drvdata()
> call
> platform/x86: ideapad-laptop: use appropriately typed variable to
> store the return value of ACPI methods
> platform/x86: ideapad-laptop: sort includes lexicographically
> platform/x86: ideapad-laptop: use sysfs_emit()
> platform/x86: ideapad-laptop: use for_each_set_bit() helper to
> simplify event processing
> platform/x86: ideapad-laptop: use msecs_to_jiffies() helper instead of
> hand-crafted formula
> platform/x86: ideapad-laptop: use dev_{err,warn} or appropriate
> variant to display log messages
> platform/x86: ideapad-laptop: convert ACPI helpers to return -EIO in
> case of failure
> platform/x86: ideapad-laptop: always propagate error codes from device
> attributes' show() callback
> platform/x86: ideapad-laptop: misc. device attribute changes
> platform/x86: ideapad-laptop: group and separate (un)related constants
> into enums
> platform/x86: ideapad-laptop: rework and create new ACPI helpers
> platform/x86: ideapad-laptop: rework is_visible() logic
> platform/x86: ideapad-laptop: check for Fn-lock support in HALS
> platform/x86: ideapad-laptop: check for touchpad support in _CFG
> platform/x86: ideapad-laptop: change 'status' debugfs file format
> platform/x86: ideapad-laptop: change 'cfg' debugfs file format
> Revert "platform/x86: ideapad-laptop: Switch touchpad attribute to be
> RO"
> platform/x86: ideapad-laptop: fix checkpatch warnings, more consistent
> style
> platform/x86: ideapad-laptop: send notification about touchpad state
> change to sysfs
> platform/x86: ideapad-laptop: add keyboard backlight control support
> platform/x86: ideapad-laptop: add "always on USB charging" control
> support
> Documentation/ABI: sysfs-platform-ideapad-laptop: update device
> attribute paths
> Documentation/ABI: sysfs-platform-ideapad-laptop: conservation_mode
> and usb_charging
>
> .../ABI/testing/sysfs-platform-ideapad-laptop | 26 +-
> drivers/platform/x86/ideapad-laptop.c | 1047 +++++++++++------
> 2 files changed, 692 insertions(+), 381 deletions(-)
>
