Re: [PATCH] platform/x86: system76_acpi: Guard System76 EC specific functionality

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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;
> 




[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux