linux-kernel - Re: [PATCH v3 2/3] power: supply: Add Acer Aspire 1 embedded controller driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 22 Feb 2024 00:41:34 +0100
From: Sebastian Reichel <sebastian.reichel@...labora.com>
To: Nikita Travkin <nikita@...n.ru>
Cc: Rob Herring <robh+dt@...nel.org>, 
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>, 
	cros-qcom-dts-watchers@...omium.org, Andy Gross <agross@...nel.org>, 
	Bjorn Andersson <andersson@...nel.org>, Konrad Dybcio <konrad.dybcio@...aro.org>, 
	Rob Herring <robh@...nel.org>, linux-pm@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v3 2/3] power: supply: Add Acer Aspire 1 embedded
 controller driver

Hi,

On Tue, Feb 20, 2024 at 04:57:13PM +0500, Nikita Travkin wrote:
> Acer Aspire 1 is a Snapdragon 7c based laptop. It uses an embedded
> controller to control the charging and battery management, as well as to
> perform a set of misc functions.
> 
> Unfortunately, while all this functionality is implemented in ACPI, it's
> currently not possible to use ACPI to boot Linux on such Qualcomm
> devices. To allow Linux to still support the features provided by EC,
> this driver reimplments the relevant ACPI parts. This allows us to boot
> the laptop with Device Tree and retain all the features.
> 
> Signed-off-by: Nikita Travkin <nikita@...n.ru>
> ---
>  drivers/power/supply/Kconfig           |  14 +
>  drivers/power/supply/Makefile          |   1 +
>  drivers/power/supply/acer-aspire1-ec.c | 453 +++++++++++++++++++++++++++++++++

I think this belongs into drivers/platform, as it handles all bits of
the EC.

[...]

>  3 files changed, 468 insertions(+)
> 
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index 3e31375491d5..e91a3acecb41 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -985,4 +985,18 @@ config FUEL_GAUGE_MM8013
>  	  the state of charge, temperature, cycle count, actual and design
>  	  capacity, etc.
>  
> +config EC_ACER_ASPIRE1
> +	tristate "Acer Aspire 1 Emedded Controller driver"
> +	depends on I2C
> +	depends on DRM
> +	help
> +	  Say Y here to enable the EC driver for the (Snapdragon-based)
> +	  Acer Aspire 1 laptop. The EC handles battery and charging
> +	  monitoring as well as some misc functions like the lid sensor
> +	  and USB Type-C DP HPD events.
> +
> +	  This driver provides battery and AC status support for the mentioned

I did not see any AC status bits?

> [...]

> +	case POWER_SUPPLY_PROP_PRESENT:
> +		val->intval = 1;

You have an unused ASPIRE_EC_FG_FLAG_PRESENT, that looks like it
should be used here?

> [...]

Otherwise the power-supply bits LGTM.

-- Sebastian

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)