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

[Sebastian Reichel]

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@xxxxxxx>
> ---
>  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

Attachment: signature.asc
Description: PGP signature