Re: [PATCH v2 2/3] platform: arm64: Add driver for EC found in most X1E laptops

[Stephan Gerhold <stephan.gerhold@linaro.org>]

On Thu, Dec 19, 2024 at 09:08:19PM +0100, Maya Matuszczyk wrote:
> Currently it features only reporting that the AP is going to suspend,
> which results in keyboard backlight turning off and the power LED
> slowly blinking on the Lenovo Yoga Slim 7x.
> 
> Honor Magicbook Art 14 and Lenovo Yoga Slim 7x are known to have
> firmware with extensions which would need appropriate handling.
> For reverse engineering the firmware on them I have written a Rust
> utility:
> 
> https://github.com/Maccraft123/it8987-qcom-tool.git
> 
> Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>

Thanks a lot for working on this!

> ---
>  MAINTAINERS                              |   6 +
>  drivers/platform/arm64/Kconfig           |   8 ++
>  drivers/platform/arm64/Makefile          |   1 +
>  drivers/platform/arm64/qcom-x1e-it8987.c | 158 +++++++++++++++++++++++
>  4 files changed, 173 insertions(+)
>  create mode 100644 drivers/platform/arm64/qcom-x1e-it8987.c
> 
> [...]
> diff --git a/drivers/platform/arm64/qcom-x1e-it8987.c b/drivers/platform/arm64/qcom-x1e-it8987.c
> new file mode 100644
> index 000000000000..d27067d6326a
> --- /dev/null
> +++ b/drivers/platform/arm64/qcom-x1e-it8987.c
> @@ -0,0 +1,158 @@
> [...]
> +#define EC_NOTIFY_SCREEN_OFF 0x03
> +#define EC_NOTIFY_SCREEN_ON 0x04

I think these two are specific to the Yoga EC. The 0x03/0x04 value is in
the DSDT of Yoga, but I don't see it for the devkit for example.

We should probably only send these commands for the
"lenovo,yoga-slim7x-ec" compatible? What happens if you just send
EC_NOTIFY_SUSPEND_ENTER and skip sending the screen ones? Keyboard
backlight stays on?

> [...]
> +static const struct of_device_id qcom_x1e_it8987_ec_of_match[] = {
> +	{ .compatible = "lenovo,yoga-slim7x-ec" },

If you're not assigning any special data to this compatible, you can
just drop this line, since it will still probe based on the fallback
compatible below. Or you add some .data here to make the
EC_NOTIFY_SCREEN_ON/OFF conditional to the Yoga compatible. :-)

> +	{ .compatible = "qcom,x1e-it9897-ec" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, qcom_x1e_it8987_ec_of_match);
> +

Thanks,
Stephan