Some models need to trigger the EC for the yoga mode control to work
properly. EC triggering consists of a VPC call that needs to be
synchronized. The vpc_mutex is in the ideapad-laptop module, so
synchronization is easier there.
Move the ymc_trigger_ec function to the ideapad-laptop module.
Signed-off-by: Gergo Koteles <soyer@irl.hu>
---
drivers/platform/x86/Kconfig | 1 +
drivers/platform/x86/ideapad-laptop.c | 18 ++++++++++++++
drivers/platform/x86/ideapad-laptop.h | 2 ++
drivers/platform/x86/lenovo-ymc.c | 34 ++++-----------------------
4 files changed, 26 insertions(+), 29 deletions(-)
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 665fa9524986..ddfccc226751 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -477,6 +477,7 @@ config LENOVO_YMC
tristate "Lenovo Yoga Tablet Mode Control"
depends on ACPI_WMI
depends on INPUT
+ depends on IDEAPAD_LAPTOP
select INPUT_SPARSEKMAP
help
This driver maps the Tablet Mode Control switch to SW_TABLET_MODE input
diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 464cc5fd0088..4dcdbb153dda 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -1615,6 +1615,24 @@ static void ideapad_sync_touchpad_state(struct ideapad_private *priv, bool send_
priv->r_touchpad_val = value;
}
+void ideapad_ymc_trigger_ec(void)
+{
+ struct ideapad_private *priv;
+ int ret;
+
+ guard(mutex)(&ideapad_shared_mutex);
+
+ priv = ideapad_shared;
+ if (!priv)
+ return;
+
+ scoped_guard(mutex, &priv->vpc_mutex)
+ ret = write_ec_cmd(priv->adev->handle, VPCCMD_W_YMC, 1);
+ if (ret)
+ dev_warn(&priv->platform_device->dev, "Could not write YMC: %d\n", ret);
+}
+EXPORT_SYMBOL_GPL(ideapad_ymc_trigger_ec);
+
static void ideapad_acpi_notify(acpi_handle handle, u32 event, void *data)
{
struct ideapad_private *priv = data;
diff --git a/drivers/platform/x86/ideapad-laptop.h b/drivers/platform/x86/ideapad-laptop.h
index 4498a96de597..4dcbed842b0b 100644
--- a/drivers/platform/x86/ideapad-laptop.h
+++ b/drivers/platform/x86/ideapad-laptop.h
@@ -13,6 +13,8 @@
#include <linux/jiffies.h>
#include <linux/errno.h>
+void ideapad_ymc_trigger_ec(void);
+
enum {
VPCCMD_R_VPC1 = 0x10,
VPCCMD_R_BL_MAX,
diff --git a/drivers/platform/x86/lenovo-ymc.c b/drivers/platform/x86/lenovo-ymc.c
index e1fbc35504d4..0819e9823ca9 100644
--- a/drivers/platform/x86/lenovo-ymc.c
+++ b/drivers/platform/x86/lenovo-ymc.c
@@ -62,19 +62,14 @@ static const struct dmi_system_id allowed_chasis_types_dmi_table[] = {
struct lenovo_ymc_private {
struct input_dev *input_dev;
- struct acpi_device *ec_acpi_dev;
};
-static void lenovo_ymc_trigger_ec(struct wmi_device *wdev, struct lenovo_ymc_private *priv)
+static void lenovo_ymc_trigger_ec(void)
{
- int err;
-
- if (!priv->ec_acpi_dev)
+ if (!ec_trigger)
return;
- err = write_ec_cmd(priv->ec_acpi_dev->handle, VPCCMD_W_YMC, 1);
- if (err)
- dev_warn(&wdev->dev, "Could not write YMC: %d\n", err);
+ ideapad_ymc_trigger_ec();
}
static const struct key_entry lenovo_ymc_keymap[] = {
@@ -125,11 +120,9 @@ static void lenovo_ymc_notify(struct wmi_device *wdev, union acpi_object *data)
free_obj:
kfree(obj);
- lenovo_ymc_trigger_ec(wdev, priv);
+ lenovo_ymc_trigger_ec();
}
-static void acpi_dev_put_helper(void *p) { acpi_dev_put(p); }
-
static int lenovo_ymc_probe(struct wmi_device *wdev, const void *ctx)
{
struct lenovo_ymc_private *priv;
@@ -149,23 +142,6 @@ static int lenovo_ymc_probe(struct wmi_device *wdev, const void *ctx)
if (!priv)
return -ENOMEM;
- if (ec_trigger) {
- pr_debug("Lenovo YMC enable EC triggering.\n");
- priv->ec_acpi_dev = acpi_dev_get_first_match_dev("VPC2004", NULL, -1);
-
- if (!priv->ec_acpi_dev) {
- dev_err(&wdev->dev, "Could not find EC ACPI device.\n");
- return -ENODEV;
- }
- err = devm_add_action_or_reset(&wdev->dev,
- acpi_dev_put_helper, priv->ec_acpi_dev);
- if (err) {
- dev_err(&wdev->dev,
- "Could not clean up EC ACPI device: %d\n", err);
- return err;
- }
- }
-
input_dev = devm_input_allocate_device(&wdev->dev);
if (!input_dev)
return -ENOMEM;
@@ -192,7 +168,7 @@ static int lenovo_ymc_probe(struct wmi_device *wdev, const void *ctx)
dev_set_drvdata(&wdev->dev, priv);
/* Report the state for the first time on probe */
- lenovo_ymc_trigger_ec(wdev, priv);
+ lenovo_ymc_trigger_ec();
lenovo_ymc_notify(wdev, NULL);
return 0;
}
--
2.45.2On Tue, 9 Jul 2024, Gergo Koteles wrote:
> Some models need to trigger the EC for the yoga mode control to work
> properly. EC triggering consists of a VPC call that needs to be
> synchronized. The vpc_mutex is in the ideapad-laptop module, so
> synchronization is easier there.
>
> Move the ymc_trigger_ec function to the ideapad-laptop module.
>
> Signed-off-by: Gergo Koteles <soyer@irl.hu>
> ---
> drivers/platform/x86/Kconfig | 1 +
> drivers/platform/x86/ideapad-laptop.c | 18 ++++++++++++++
> drivers/platform/x86/ideapad-laptop.h | 2 ++
> drivers/platform/x86/lenovo-ymc.c | 34 ++++-----------------------
> 4 files changed, 26 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 665fa9524986..ddfccc226751 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -477,6 +477,7 @@ config LENOVO_YMC
> tristate "Lenovo Yoga Tablet Mode Control"
> depends on ACPI_WMI
> depends on INPUT
> + depends on IDEAPAD_LAPTOP
> select INPUT_SPARSEKMAP
> help
> This driver maps the Tablet Mode Control switch to SW_TABLET_MODE input
> diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
> index 464cc5fd0088..4dcdbb153dda 100644
> --- a/drivers/platform/x86/ideapad-laptop.c
> +++ b/drivers/platform/x86/ideapad-laptop.c
> @@ -1615,6 +1615,24 @@ static void ideapad_sync_touchpad_state(struct ideapad_private *priv, bool send_
> priv->r_touchpad_val = value;
> }
>
> +void ideapad_ymc_trigger_ec(void)
> +{
> + struct ideapad_private *priv;
> + int ret;
> +
> + guard(mutex)(&ideapad_shared_mutex);
> +
> + priv = ideapad_shared;
> + if (!priv)
> + return;
> +
> + scoped_guard(mutex, &priv->vpc_mutex)
It feels the ordering in this series is off because patch 2 add the
mutex and here you add it into yet another function. If this patch
would be before the other, the mutex changes could all go into the same
patch.
> + ret = write_ec_cmd(priv->adev->handle, VPCCMD_W_YMC, 1);
> + if (ret)
> + dev_warn(&priv->platform_device->dev, "Could not write YMC: %d\n", ret);
> +}
> +EXPORT_SYMBOL_GPL(ideapad_ymc_trigger_ec);
This export should be namespaced.
I'll have to say I can accept this approach only grudgingly... it feels
off even if I understand why you have to do this cross driver thing.
--
i.
> static void ideapad_acpi_notify(acpi_handle handle, u32 event, void *data)
> {
> struct ideapad_private *priv = data;
> diff --git a/drivers/platform/x86/ideapad-laptop.h b/drivers/platform/x86/ideapad-laptop.h
> index 4498a96de597..4dcbed842b0b 100644
> --- a/drivers/platform/x86/ideapad-laptop.h
> +++ b/drivers/platform/x86/ideapad-laptop.h
> @@ -13,6 +13,8 @@
> #include <linux/jiffies.h>
> #include <linux/errno.h>
>
> +void ideapad_ymc_trigger_ec(void);
> +
> enum {
> VPCCMD_R_VPC1 = 0x10,
> VPCCMD_R_BL_MAX,
> diff --git a/drivers/platform/x86/lenovo-ymc.c b/drivers/platform/x86/lenovo-ymc.c
> index e1fbc35504d4..0819e9823ca9 100644
> --- a/drivers/platform/x86/lenovo-ymc.c
> +++ b/drivers/platform/x86/lenovo-ymc.c
> @@ -62,19 +62,14 @@ static const struct dmi_system_id allowed_chasis_types_dmi_table[] = {
>
> struct lenovo_ymc_private {
> struct input_dev *input_dev;
> - struct acpi_device *ec_acpi_dev;
> };
>
> -static void lenovo_ymc_trigger_ec(struct wmi_device *wdev, struct lenovo_ymc_private *priv)
> +static void lenovo_ymc_trigger_ec(void)
> {
> - int err;
> -
> - if (!priv->ec_acpi_dev)
> + if (!ec_trigger)
> return;
>
> - err = write_ec_cmd(priv->ec_acpi_dev->handle, VPCCMD_W_YMC, 1);
> - if (err)
> - dev_warn(&wdev->dev, "Could not write YMC: %d\n", err);
> + ideapad_ymc_trigger_ec();
> }
>
> static const struct key_entry lenovo_ymc_keymap[] = {
> @@ -125,11 +120,9 @@ static void lenovo_ymc_notify(struct wmi_device *wdev, union acpi_object *data)
>
> free_obj:
> kfree(obj);
> - lenovo_ymc_trigger_ec(wdev, priv);
> + lenovo_ymc_trigger_ec();
> }
>
> -static void acpi_dev_put_helper(void *p) { acpi_dev_put(p); }
> -
> static int lenovo_ymc_probe(struct wmi_device *wdev, const void *ctx)
> {
> struct lenovo_ymc_private *priv;
> @@ -149,23 +142,6 @@ static int lenovo_ymc_probe(struct wmi_device *wdev, const void *ctx)
> if (!priv)
> return -ENOMEM;
>
> - if (ec_trigger) {
> - pr_debug("Lenovo YMC enable EC triggering.\n");
> - priv->ec_acpi_dev = acpi_dev_get_first_match_dev("VPC2004", NULL, -1);
> -
> - if (!priv->ec_acpi_dev) {
> - dev_err(&wdev->dev, "Could not find EC ACPI device.\n");
> - return -ENODEV;
> - }
> - err = devm_add_action_or_reset(&wdev->dev,
> - acpi_dev_put_helper, priv->ec_acpi_dev);
> - if (err) {
> - dev_err(&wdev->dev,
> - "Could not clean up EC ACPI device: %d\n", err);
> - return err;
> - }
> - }
> -
> input_dev = devm_input_allocate_device(&wdev->dev);
> if (!input_dev)
> return -ENOMEM;
> @@ -192,7 +168,7 @@ static int lenovo_ymc_probe(struct wmi_device *wdev, const void *ctx)
> dev_set_drvdata(&wdev->dev, priv);
>
> /* Report the state for the first time on probe */
> - lenovo_ymc_trigger_ec(wdev, priv);
> + lenovo_ymc_trigger_ec();
> lenovo_ymc_notify(wdev, NULL);
> return 0;
> }
>
Hi Ilpo,
Thanks for the review.
On Thu, 2024-07-11 at 11:18 +0300, Ilpo Järvinen wrote:
> On Tue, 9 Jul 2024, Gergo Koteles wrote:
>
> > Some models need to trigger the EC for the yoga mode control to work
> > properly. EC triggering consists of a VPC call that needs to be
> > synchronized. The vpc_mutex is in the ideapad-laptop module, so
> > synchronization is easier there.
> >
> > Move the ymc_trigger_ec function to the ideapad-laptop module.
> >
> > Signed-off-by: Gergo Koteles <soyer@irl.hu>
> > ---
> > drivers/platform/x86/Kconfig | 1 +
> > drivers/platform/x86/ideapad-laptop.c | 18 ++++++++++++++
> > drivers/platform/x86/ideapad-laptop.h | 2 ++
> > drivers/platform/x86/lenovo-ymc.c | 34 ++++-----------------------
> > 4 files changed, 26 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > index 665fa9524986..ddfccc226751 100644
> > --- a/drivers/platform/x86/Kconfig
> > +++ b/drivers/platform/x86/Kconfig
> > @@ -477,6 +477,7 @@ config LENOVO_YMC
> > tristate "Lenovo Yoga Tablet Mode Control"
> > depends on ACPI_WMI
> > depends on INPUT
> > + depends on IDEAPAD_LAPTOP
> > select INPUT_SPARSEKMAP
> > help
> > This driver maps the Tablet Mode Control switch to SW_TABLET_MODE input
> > diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
> > index 464cc5fd0088..4dcdbb153dda 100644
> > --- a/drivers/platform/x86/ideapad-laptop.c
> > +++ b/drivers/platform/x86/ideapad-laptop.c
> > @@ -1615,6 +1615,24 @@ static void ideapad_sync_touchpad_state(struct ideapad_private *priv, bool send_
> > priv->r_touchpad_val = value;
> > }
> >
> > +void ideapad_ymc_trigger_ec(void)
> > +{
> > + struct ideapad_private *priv;
> > + int ret;
> > +
> > + guard(mutex)(&ideapad_shared_mutex);
> > +
> > + priv = ideapad_shared;
> > + if (!priv)
> > + return;
> > +
> > + scoped_guard(mutex, &priv->vpc_mutex)
>
> It feels the ordering in this series is off because patch 2 add the
> mutex and here you add it into yet another function. If this patch
> would be before the other, the mutex changes could all go into the same
> patch.
>
I (wrongly) thought it would be easier to explain why a new dependency
is needed.
Mutex changes go into one patch in the v2.
> > + ret = write_ec_cmd(priv->adev->handle, VPCCMD_W_YMC, 1);
> > + if (ret)
> > + dev_warn(&priv->platform_device->dev, "Could not write YMC: %d\n", ret);
> > +}
> > +EXPORT_SYMBOL_GPL(ideapad_ymc_trigger_ec);
>
> This export should be namespaced.
It will be in the v2.
>
> I'll have to say I can accept this approach only grudgingly... it feels
> off even if I understand why you have to do this cross driver thing.
>
Based on the WMI driver development guide, I will add a notification
chain in v2 soon. Looks better.
Best Regards,
Gergo Koteles
© 2016 - 2025 Red Hat, Inc.