On Fri, Sep 15, 2023 at 11:03:40AM -0400, David Ober wrote: > This addition adds in the ability for the system to scan the > MEC172x EC chip in Lenovo ThinkStation systems to get the > current fan RPM speeds and the Maximum speed value for each > fan also provides the current CPU and DIMM thermal status > > Signed-off-by: David Ober <dober6023@xxxxxxxxx> > > Written by David Ober from Lenovo using this gmail address since > my corporate email address does not comply with git email FWIW, this needs to be after '---' Anyway, thinking about this submission makes me even more concerned. This isn't really a driver for MEC172x; it is simply a driver accessing an EC on a set of PCs and/or laptops from Lenovo which uses a vertain API for communication between EC and main CPU. Such ECs are typically accessed through ACPI. Yet, in this driver there is no mention of ACPI, much less any protection against parallel use by ACPI code (that access lock in get_ec_reg() doesn't even protect against parallel access from userspace, much less against parallel access from other drivers or ACPI, for example by using request_region() to reserve the used memory ranges). There needs to be explanations and clarifications - Why this driver will only be used for communication with MEC172X based chips, and why the exact EC chip is relevant in the first place to be mentioned as much as it is. - How it is guaranteed that the EC is not and will never be accessed through ACPI. - How it is guaranteed that there will never be any other kernel drivers accessing the chip. > --- > drivers/hwmon/Kconfig | 10 + > drivers/hwmon/Makefile | 1 + > drivers/hwmon/lenovo-ec-sensors.c | 471 ++++++++++++++++++++++++++++++ Documentation missing. Guenter
