Hi Jeremy, On 9/3/20 10:47 PM, Jeremy Soller wrote: > The hwmon driver provides a kernel interface for accessing fan duty > cycle, fan speed, and temperature data as defined by firmware. Up to > eight fans and eight temperature sensors are supported. The actual > number is determined by querying the firmware. > > Signed-off-by: Jeremy Soller <jeremy@xxxxxxxxxxxx> > Cc: platform-driver-x86@xxxxxxxxxxxxxxx Quick self intro: I have take over drivers/platform/x86 maintainership from Andy; and I'm working my way through the backlog of old patches in patchwork: https://patchwork.kernel.org/project/platform-driver-x86/list/ This patch has some initial review remarks from Barnabás Pőcze which need addressing. Can you please send a v2 of this series with Barnabás' remarks to patch 1/2 fixed ? As I mentioned before switching to a sparse-keymap for 2/2 is not really necessary IMHO, but if you want to do it that is fine. Regards, Hans > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 40219bba6801..d2f9c3dcf4b9 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -1158,6 +1158,7 @@ config SONYPI_COMPAT > config SYSTEM76_ACPI > tristate "System76 ACPI Driver" > depends on ACPI > + depends on HWMON > select NEW_LEDS > select LEDS_CLASS > select LEDS_TRIGGERS > diff --git a/drivers/platform/x86/system76_acpi.c b/drivers/platform/x86/system76_acpi.c > index c14fd22ba196..aef0ea34829d 100644 > --- a/drivers/platform/x86/system76_acpi.c > +++ b/drivers/platform/x86/system76_acpi.c > @@ -10,6 +10,8 @@ > */ > > #include <linux/acpi.h> > +#include <linux/hwmon.h> > +#include <linux/hwmon-sysfs.h> > #include <linux/init.h> > #include <linux/kernel.h> > #include <linux/leds.h> > @@ -24,6 +26,9 @@ struct system76_data { > enum led_brightness kb_brightness; > enum led_brightness kb_toggle_brightness; > int kb_color; > + struct device *therm; > + union acpi_object *nfan; > + union acpi_object *ntmp; > }; > > static const struct acpi_device_id device_ids[] = { > @@ -68,6 +73,54 @@ static int system76_get(struct system76_data *data, char *method) > return -1; > } > > +// Get a System76 ACPI device value by name with index > +static int system76_get_index(struct system76_data *data, char *method, int index) > +{ > + union acpi_object obj; > + struct acpi_object_list obj_list; > + acpi_handle handle; > + acpi_status status; > + unsigned long long ret = 0; > + > + obj.type = ACPI_TYPE_INTEGER; > + obj.integer.value = index; > + obj_list.count = 1; > + obj_list.pointer = &obj; > + handle = acpi_device_handle(data->acpi_dev); > + status = acpi_evaluate_integer(handle, method, &obj_list, &ret); > + if (ACPI_SUCCESS(status)) > + return (int)ret; > + else > + return -1; > +} > + > +// Get a System76 ACPI device object by name > +static int system76_get_object(struct system76_data *data, char *method, union acpi_object **obj) > +{ > + acpi_handle handle; > + acpi_status status; > + struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL }; > + > + handle = acpi_device_handle(data->acpi_dev); > + status = acpi_evaluate_object(handle, method, NULL, &buf); > + if (ACPI_SUCCESS(status)) { > + *obj = (union acpi_object *)buf.pointer; > + return 0; > + } else { > + return -1; > + } > +} > + > +// Get a name from a System76 ACPI device object > +static char *system76_name(union acpi_object *obj, int index) > +{ > + if (obj && obj->type == ACPI_TYPE_PACKAGE && index <= obj->package.count) { > + if (obj->package.elements[index].type == ACPI_TYPE_STRING) > + return obj->package.elements[index].string.pointer; > + } > + return NULL; > +} > + > // Set a System76 ACPI device value by name > static int system76_set(struct system76_data *data, char *method, int value) > { > @@ -270,6 +323,120 @@ static void kb_led_hotkey_color(struct system76_data *data) > kb_led_notify(data); > } > > +static umode_t thermal_is_visible( > + const void *drvdata, > + enum hwmon_sensor_types type, > + u32 attr, > + int channel) > +{ > + const struct system76_data *data = drvdata; > + > + if (type == hwmon_fan || type == hwmon_pwm) { > + if (system76_name(data->nfan, channel)) > + return S_IRUGO; > + } else if (type == hwmon_temp) { > + if (system76_name(data->ntmp, channel)) > + return S_IRUGO; > + } > + return 0; > +} > + > +static int thermal_read( > + struct device *dev, > + enum hwmon_sensor_types type, > + u32 attr, > + int channel, > + long *val) > +{ > + struct system76_data *data = dev_get_drvdata(dev); > + int raw; > + > + if (type == hwmon_fan && attr == hwmon_fan_input) { > + raw = system76_get_index(data, "GFAN", channel); > + if (raw >= 0) { > + *val = (long)((raw >> 8) & 0xFFFF); > + return 0; > + } > + } else if (type == hwmon_pwm && attr == hwmon_pwm_input) { > + raw = system76_get_index(data, "GFAN", channel); > + if (raw >= 0) { > + *val = (long)(raw & 0xFF); > + return 0; > + } > + } else if (type == hwmon_temp && attr == hwmon_temp_input) { > + raw = system76_get_index(data, "GTMP", channel); > + if (raw >= 0) { > + *val = (long)(raw * 1000); > + return 0; > + } > + } > + return -EINVAL; > +} > + > +static int thermal_read_string( > + struct device *dev, > + enum hwmon_sensor_types type, > + u32 attr, > + int channel, > + const char **str) > +{ > + struct system76_data *data = dev_get_drvdata(dev); > + > + if (type == hwmon_fan && attr == hwmon_fan_label) { > + *str = system76_name(data->nfan, channel); > + if (*str) > + return 0; > + } else if (type == hwmon_temp && attr == hwmon_temp_label) { > + *str = system76_name(data->ntmp, channel); > + if (*str) > + return 0; > + } > + return -EINVAL; > +} > + > +static const struct hwmon_ops thermal_ops = { > + .is_visible = thermal_is_visible, > + .read = thermal_read, > + .read_string = thermal_read_string, > +}; > + > +// Allocate up to 8 fans and temperatures > +static const struct hwmon_channel_info *thermal_channel_info[] = { > + HWMON_CHANNEL_INFO(fan, > + HWMON_F_INPUT | HWMON_F_LABEL, > + HWMON_F_INPUT | HWMON_F_LABEL, > + HWMON_F_INPUT | HWMON_F_LABEL, > + HWMON_F_INPUT | HWMON_F_LABEL, > + HWMON_F_INPUT | HWMON_F_LABEL, > + HWMON_F_INPUT | HWMON_F_LABEL, > + HWMON_F_INPUT | HWMON_F_LABEL, > + HWMON_F_INPUT | HWMON_F_LABEL), > + HWMON_CHANNEL_INFO(pwm, > + HWMON_PWM_INPUT, > + HWMON_PWM_INPUT, > + HWMON_PWM_INPUT, > + HWMON_PWM_INPUT, > + HWMON_PWM_INPUT, > + HWMON_PWM_INPUT, > + HWMON_PWM_INPUT, > + HWMON_PWM_INPUT), > + HWMON_CHANNEL_INFO(temp, > + HWMON_T_INPUT | HWMON_T_LABEL, > + HWMON_T_INPUT | HWMON_T_LABEL, > + HWMON_T_INPUT | HWMON_T_LABEL, > + HWMON_T_INPUT | HWMON_T_LABEL, > + HWMON_T_INPUT | HWMON_T_LABEL, > + HWMON_T_INPUT | HWMON_T_LABEL, > + HWMON_T_INPUT | HWMON_T_LABEL, > + HWMON_T_INPUT | HWMON_T_LABEL), > + NULL > +}; > + > +static const struct hwmon_chip_info thermal_chip_info = { > + .ops = &thermal_ops, > + .info = thermal_channel_info, > +}; > + > // Handle ACPI notification > static void system76_notify(struct acpi_device *acpi_dev, u32 event) > { > @@ -346,6 +513,17 @@ static int system76_add(struct acpi_device *acpi_dev) > return err; > } > > + system76_get_object(data, "NFAN", &data->nfan); > + system76_get_object(data, "NTMP", &data->ntmp); > + data->therm = devm_hwmon_device_register_with_info( > + &acpi_dev->dev, > + "system76_acpi", > + data, > + &thermal_chip_info, > + NULL); > + if (IS_ERR(data->therm)) > + return PTR_ERR(data->therm); > + > return 0; > } > > @@ -362,6 +540,9 @@ static int system76_remove(struct acpi_device *acpi_dev) > > devm_led_classdev_unregister(&acpi_dev->dev, &data->kb_led); > > + kfree(data->nfan); > + kfree(data->ntmp); > + > system76_get(data, "FINI"); > > return 0; >
