Hi, On 12/22/21 19:51, Tim Crawford wrote: > Certain functionality or its implementation in System76 EC firmware may > be different to the proprietary ODM EC firmware. Introduce a new bool, > `has_open_ec`, to guard our specific logic. Detect the use of this by > looking for a custom ACPI method name used in System76 firmware. > > Signed-off-by: Tim Crawford <tcrawford@xxxxxxxxxxxx> Thank you for your patch, I've applied this patch to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans I will also add this to the pdx86/fixes branch and send it out to Linus with the next pdx86 fixes pull-req for Linus (since this fixes changes introduced in 5.16). Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually will be included in the pdx86 pull-request to Linus for the next merge-window. Regards, Hans > --- > drivers/platform/x86/system76_acpi.c | 58 ++++++++++++++-------------- > 1 file changed, 30 insertions(+), 28 deletions(-) > > diff --git a/drivers/platform/x86/system76_acpi.c b/drivers/platform/x86/system76_acpi.c > index 8b292ee95a14..7299ad08c838 100644 > --- a/drivers/platform/x86/system76_acpi.c > +++ b/drivers/platform/x86/system76_acpi.c > @@ -35,6 +35,7 @@ struct system76_data { > union acpi_object *nfan; > union acpi_object *ntmp; > struct input_dev *input; > + bool has_open_ec; > }; > > static const struct acpi_device_id device_ids[] = { > @@ -279,20 +280,12 @@ static struct acpi_battery_hook system76_battery_hook = { > > static void system76_battery_init(void) > { > - acpi_handle handle; > - > - handle = ec_get_handle(); > - if (handle && acpi_has_method(handle, "GBCT")) > - battery_hook_register(&system76_battery_hook); > + battery_hook_register(&system76_battery_hook); > } > > static void system76_battery_exit(void) > { > - acpi_handle handle; > - > - handle = ec_get_handle(); > - if (handle && acpi_has_method(handle, "GBCT")) > - battery_hook_unregister(&system76_battery_hook); > + battery_hook_unregister(&system76_battery_hook); > } > > // Get the airplane mode LED brightness > @@ -673,6 +666,10 @@ static int system76_add(struct acpi_device *acpi_dev) > acpi_dev->driver_data = data; > data->acpi_dev = acpi_dev; > > + // Some models do not run open EC firmware. Check for an ACPI method > + // that only exists on open EC to guard functionality specific to it. > + data->has_open_ec = acpi_has_method(acpi_device_handle(data->acpi_dev), "NFAN"); > + > err = system76_get(data, "INIT"); > if (err) > return err; > @@ -718,27 +715,31 @@ static int system76_add(struct acpi_device *acpi_dev) > if (err) > goto error; > > - err = system76_get_object(data, "NFAN", &data->nfan); > - if (err) > - goto error; > + if (data->has_open_ec) { > + err = system76_get_object(data, "NFAN", &data->nfan); > + if (err) > + goto error; > > - err = system76_get_object(data, "NTMP", &data->ntmp); > - if (err) > - goto error; > + err = system76_get_object(data, "NTMP", &data->ntmp); > + if (err) > + goto error; > > - data->therm = devm_hwmon_device_register_with_info(&acpi_dev->dev, > - "system76_acpi", data, &thermal_chip_info, NULL); > - err = PTR_ERR_OR_ZERO(data->therm); > - if (err) > - goto error; > + data->therm = devm_hwmon_device_register_with_info(&acpi_dev->dev, > + "system76_acpi", data, &thermal_chip_info, NULL); > + err = PTR_ERR_OR_ZERO(data->therm); > + if (err) > + goto error; > > - system76_battery_init(); > + system76_battery_init(); > + } > > return 0; > > error: > - kfree(data->ntmp); > - kfree(data->nfan); > + if (data->has_open_ec) { > + kfree(data->ntmp); > + kfree(data->nfan); > + } > return err; > } > > @@ -749,14 +750,15 @@ static int system76_remove(struct acpi_device *acpi_dev) > > data = acpi_driver_data(acpi_dev); > > - system76_battery_exit(); > + if (data->has_open_ec) { > + system76_battery_exit(); > + kfree(data->nfan); > + kfree(data->ntmp); > + } > > devm_led_classdev_unregister(&acpi_dev->dev, &data->ap_led); > devm_led_classdev_unregister(&acpi_dev->dev, &data->kb_led); > > - kfree(data->nfan); > - kfree(data->ntmp); > - > system76_get(data, "FINI"); > > return 0; >
