diff mbox series

[2/2] Input: cros_ec_keyb - handle x86 detachable/convertible Chromebooks

Message ID 20220526231230.2805147-2-dmitry.torokhov@gmail.com
State Accepted
Commit ba0f32141bc515da2269ebba468741dffe2f9f43
Headers show
Series [1/2] Input: cros_ec_keyb - switch to using generic device properties | expand

Commit Message

Dmitry Torokhov May 26, 2022, 11:12 p.m. UTC
From: Furquan Shaikh <furquan@chromium.org>

Some detachable/convertible x86 Chromebooks use EC buttons/switches
interface to signal volume up/down and other buttons. This configuration is
signalled via presence of GOOG0007 ACPI device. The main keyboard on such
Chromebooks is still using the standard 8042/atkbd combo.

Signed-off-by: Furquan Shaikh <furquan@chromium.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/cros_ec_keyb.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

Comments

Stephen Boyd May 27, 2022, 9:35 p.m. UTC | #1
Quoting Dmitry Torokhov (2022-05-26 16:12:30)
> From: Furquan Shaikh <furquan@chromium.org>
>
> Some detachable/convertible x86 Chromebooks use EC buttons/switches
> interface to signal volume up/down and other buttons. This configuration is
> signalled via presence of GOOG0007 ACPI device. The main keyboard on such
> Chromebooks is still using the standard 8042/atkbd combo.
>
> Signed-off-by: Furquan Shaikh <furquan@chromium.org>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Two questions below.

>  drivers/input/keyboard/cros_ec_keyb.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index e8338b1c5776..c14136b733a9 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -677,14 +678,19 @@ static const struct attribute_group cros_ec_keyb_attr_group = {
>
>  static int cros_ec_keyb_probe(struct platform_device *pdev)
>  {
> -       struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
> +       struct cros_ec_device *ec;
>         struct device *dev = &pdev->dev;
>         struct cros_ec_keyb *ckdev;
>         bool buttons_switches_only = device_get_match_data(dev);
>         int err;
>
> -       if (!dev->of_node)
> -               return -ENODEV;
> +       /*
> +        * If the parent ec device has not been probed yet, defer the probe of
> +        * this keyboard/button driver until later.
> +        */
> +       ec = dev_get_drvdata(pdev->dev.parent);

Does cros_ec populate the child node before setting the drvdata? Or in
ACPI designs this device is created as a child of cros_ec before the
driver probes?

> +       if (!ec)
> +               return -EPROBE_DEFER;
>
>         ckdev = devm_kzalloc(dev, sizeof(*ckdev), GFP_KERNEL);
>         if (!ckdev)
Dmitry Torokhov May 27, 2022, 10:37 p.m. UTC | #2
On Fri, May 27, 2022 at 05:35:36PM -0400, Stephen Boyd wrote:
> Quoting Dmitry Torokhov (2022-05-26 16:12:30)
> > From: Furquan Shaikh <furquan@chromium.org>
> >
> > Some detachable/convertible x86 Chromebooks use EC buttons/switches
> > interface to signal volume up/down and other buttons. This configuration is
> > signalled via presence of GOOG0007 ACPI device. The main keyboard on such
> > Chromebooks is still using the standard 8042/atkbd combo.
> >
> > Signed-off-by: Furquan Shaikh <furquan@chromium.org>
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> 
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> 
> Two questions below.
> 
> >  drivers/input/keyboard/cros_ec_keyb.c | 21 ++++++++++++++++++---
> >  1 file changed, 18 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> > index e8338b1c5776..c14136b733a9 100644
> > --- a/drivers/input/keyboard/cros_ec_keyb.c
> > +++ b/drivers/input/keyboard/cros_ec_keyb.c
> > @@ -677,14 +678,19 @@ static const struct attribute_group cros_ec_keyb_attr_group = {
> >
> >  static int cros_ec_keyb_probe(struct platform_device *pdev)
> >  {
> > -       struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
> > +       struct cros_ec_device *ec;
> >         struct device *dev = &pdev->dev;
> >         struct cros_ec_keyb *ckdev;
> >         bool buttons_switches_only = device_get_match_data(dev);
> >         int err;
> >
> > -       if (!dev->of_node)
> > -               return -ENODEV;
> > +       /*
> > +        * If the parent ec device has not been probed yet, defer the probe of
> > +        * this keyboard/button driver until later.
> > +        */
> > +       ec = dev_get_drvdata(pdev->dev.parent);
> 
> Does cros_ec populate the child node before setting the drvdata? Or in
> ACPI designs this device is created as a child of cros_ec before the
> driver probes?

Yes, ACPI "bus" gets scanned and all device objects are created
regardless of the driver presence and whether probe has completed or
not.

Thanks.
diff mbox series

Patch

diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index e8338b1c5776..c14136b733a9 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -12,6 +12,7 @@ 
 // expensive.
 
 #include <linux/module.h>
+#include <linux/acpi.h>
 #include <linux/bitops.h>
 #include <linux/i2c.h>
 #include <linux/input.h>
@@ -677,14 +678,19 @@  static const struct attribute_group cros_ec_keyb_attr_group = {
 
 static int cros_ec_keyb_probe(struct platform_device *pdev)
 {
-	struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
+	struct cros_ec_device *ec;
 	struct device *dev = &pdev->dev;
 	struct cros_ec_keyb *ckdev;
 	bool buttons_switches_only = device_get_match_data(dev);
 	int err;
 
-	if (!dev->of_node)
-		return -ENODEV;
+	/*
+	 * If the parent ec device has not been probed yet, defer the probe of
+	 * this keyboard/button driver until later.
+	 */
+	ec = dev_get_drvdata(pdev->dev.parent);
+	if (!ec)
+		return -EPROBE_DEFER;
 
 	ckdev = devm_kzalloc(dev, sizeof(*ckdev), GFP_KERNEL);
 	if (!ckdev)
@@ -737,6 +743,14 @@  static int cros_ec_keyb_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id cros_ec_keyb_acpi_match[] = {
+	{ "GOOG0007", true },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, cros_ec_keyb_acpi_match);
+#endif
+
 #ifdef CONFIG_OF
 static const struct of_device_id cros_ec_keyb_of_match[] = {
 	{ .compatible = "google,cros-ec-keyb" },
@@ -754,6 +768,7 @@  static struct platform_driver cros_ec_keyb_driver = {
 	.driver = {
 		.name = "cros-ec-keyb",
 		.of_match_table = of_match_ptr(cros_ec_keyb_of_match),
+		.acpi_match_table = ACPI_PTR(cros_ec_keyb_acpi_match),
 		.pm = &cros_ec_keyb_pm_ops,
 	},
 };