diff mbox series

[v3,3/4] platform/chrome: cros_kbd_led_backlight: allow binding through mfd device

Message ID 20240526-cros_ec-kbd-led-framework-v3-3-ee577415a521@weissschuh.net (mailing list archive)
State Handled Elsewhere
Headers show
Series cros_kbd_led_backlight: allow binding through cros_ec mfd device | expand

Commit Message

Thomas Weißschuh May 26, 2024, 6:17 p.m. UTC
The ChromeOS EC used in Framework laptops supports the standard CrOS EC
keyboard backlight protocol.
However the firmware on these laptops doesn't implement the ACPI ID
GOOG0002 that is recognized by cros_kbd_led_backlight and they also
don't use device tree.

Prepare the existing cros_kbd_led_backlight driver to be probed through
the CrOS EC mfd device which works without ACPI or OF support.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/platform/chrome/Kconfig                  |  2 +-
 drivers/platform/chrome/cros_kbd_led_backlight.c | 40 ++++++++++++++++++++++--
 2 files changed, 39 insertions(+), 3 deletions(-)

Comments

Tzung-Bi Shih May 28, 2024, 5:37 a.m. UTC | #1
On Sun, May 26, 2024 at 08:17:17PM +0200, Thomas Weißschuh wrote:
> +#if IS_ENABLED(CONFIG_MFD_CROS_EC_DEV)
[...]
> +static const struct keyboard_led_drvdata keyboard_led_drvdata_ec_pwm_mfd = {
> +	.init = keyboard_led_init_ec_pwm_mfd,
> +	.brightness_set_blocking = keyboard_led_set_brightness_ec_pwm,
> +	.brightness_get = keyboard_led_get_brightness_ec_pwm,

They are only available if IS_ENABLED(CONFIG_CROS_EC).
Thomas Weißschuh May 28, 2024, 5:41 a.m. UTC | #2
On 2024-05-28 05:37:32+0000, Tzung-Bi Shih wrote:
> On Sun, May 26, 2024 at 08:17:17PM +0200, Thomas Weißschuh wrote:
> > +#if IS_ENABLED(CONFIG_MFD_CROS_EC_DEV)
> [...]
> > +static const struct keyboard_led_drvdata keyboard_led_drvdata_ec_pwm_mfd = {
> > +	.init = keyboard_led_init_ec_pwm_mfd,
> > +	.brightness_set_blocking = keyboard_led_set_brightness_ec_pwm,
> > +	.brightness_get = keyboard_led_get_brightness_ec_pwm,
> 
> They are only available if IS_ENABLED(CONFIG_CROS_EC).

config MFD_CROS_EC_DEV
	depends on CROS_EC

Do you prefer this to be spelled out explicitly somewhere?
Tzung-Bi Shih May 28, 2024, 6:53 a.m. UTC | #3
On Tue, May 28, 2024 at 07:41:09AM +0200, Thomas Weißschuh wrote:
> On 2024-05-28 05:37:32+0000, Tzung-Bi Shih wrote:
> > On Sun, May 26, 2024 at 08:17:17PM +0200, Thomas Weißschuh wrote:
> > > +#if IS_ENABLED(CONFIG_MFD_CROS_EC_DEV)
> > [...]
> > > +static const struct keyboard_led_drvdata keyboard_led_drvdata_ec_pwm_mfd = {
> > > +	.init = keyboard_led_init_ec_pwm_mfd,
> > > +	.brightness_set_blocking = keyboard_led_set_brightness_ec_pwm,
> > > +	.brightness_get = keyboard_led_get_brightness_ec_pwm,
> > 
> > They are only available if IS_ENABLED(CONFIG_CROS_EC).
> 
> config MFD_CROS_EC_DEV
> 	depends on CROS_EC
> 
> Do you prefer this to be spelled out explicitly somewhere?

I see.  I missed the part.  The dependency is not obvious from only reading
the C file though.
Tzung-Bi Shih June 3, 2024, 1:57 a.m. UTC | #4
On Sun, May 26, 2024 at 08:17:17PM +0200, Thomas Weißschuh wrote:
> The ChromeOS EC used in Framework laptops supports the standard CrOS EC
> keyboard backlight protocol.
> However the firmware on these laptops doesn't implement the ACPI ID
> GOOG0002 that is recognized by cros_kbd_led_backlight and they also
> don't use device tree.
> 
> Prepare the existing cros_kbd_led_backlight driver to be probed through
> the CrOS EC mfd device which works without ACPI or OF support.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>

The patch overall looks good to me but it depends on previous patch in the
series (for LED_REJECT_NAME_CONFLICT).  Let's wait for review.
Lee Jones June 13, 2024, 2:34 p.m. UTC | #5
On Mon, 03 Jun 2024, Tzung-Bi Shih wrote:

> On Sun, May 26, 2024 at 08:17:17PM +0200, Thomas Weißschuh wrote:
> > The ChromeOS EC used in Framework laptops supports the standard CrOS EC
> > keyboard backlight protocol.
> > However the firmware on these laptops doesn't implement the ACPI ID
> > GOOG0002 that is recognized by cros_kbd_led_backlight and they also
> > don't use device tree.
> > 
> > Prepare the existing cros_kbd_led_backlight driver to be probed through
> > the CrOS EC mfd device which works without ACPI or OF support.
> > 
> > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> 
> The patch overall looks good to me but it depends on previous patch in the
> series (for LED_REJECT_NAME_CONFLICT).  Let's wait for review.

I plan to take this via MFD.

Is someone going to Ack this patch?
Tzung-Bi Shih June 14, 2024, 2:39 a.m. UTC | #6
On Thu, Jun 13, 2024 at 03:34:51PM +0100, Lee Jones wrote:
> On Mon, 03 Jun 2024, Tzung-Bi Shih wrote:
> 
> > On Sun, May 26, 2024 at 08:17:17PM +0200, Thomas Weißschuh wrote:
> > > The ChromeOS EC used in Framework laptops supports the standard CrOS EC
> > > keyboard backlight protocol.
> > > However the firmware on these laptops doesn't implement the ACPI ID
> > > GOOG0002 that is recognized by cros_kbd_led_backlight and they also
> > > don't use device tree.
> > > 
> > > Prepare the existing cros_kbd_led_backlight driver to be probed through
> > > the CrOS EC mfd device which works without ACPI or OF support.
> > > 
> > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > 
> > The patch overall looks good to me but it depends on previous patch in the
> > series (for LED_REJECT_NAME_CONFLICT).  Let's wait for review.
> 
> I plan to take this via MFD.
> 
> Is someone going to Ack this patch?

Acked-by: Tzung-Bi Shih <tzungbi@kernel.org>
diff mbox series

Patch

diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 073616b5b5a0..7dbeb786352a 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -150,7 +150,7 @@  config CROS_EC_PROTO
 
 config CROS_KBD_LED_BACKLIGHT
 	tristate "Backlight LED support for Chrome OS keyboards"
-	depends on LEDS_CLASS && (ACPI || CROS_EC)
+	depends on LEDS_CLASS && (ACPI || CROS_EC || MFD_CROS_EC_DEV)
 	help
 	  This option enables support for the keyboard backlight LEDs on
 	  select Chrome OS systems.
diff --git a/drivers/platform/chrome/cros_kbd_led_backlight.c b/drivers/platform/chrome/cros_kbd_led_backlight.c
index b83e4f328620..78097c8a4966 100644
--- a/drivers/platform/chrome/cros_kbd_led_backlight.c
+++ b/drivers/platform/chrome/cros_kbd_led_backlight.c
@@ -9,6 +9,7 @@ 
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/leds.h>
+#include <linux/mfd/core.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -194,13 +195,46 @@  static const __maybe_unused struct keyboard_led_drvdata keyboard_led_drvdata_ec_
 
 #endif /* IS_ENABLED(CONFIG_CROS_EC) */
 
+#if IS_ENABLED(CONFIG_MFD_CROS_EC_DEV)
+static int keyboard_led_init_ec_pwm_mfd(struct platform_device *pdev)
+{
+	struct cros_ec_dev *ec_dev = dev_get_drvdata(pdev->dev.parent);
+	struct cros_ec_device *cros_ec = ec_dev->ec_dev;
+	struct keyboard_led *keyboard_led = platform_get_drvdata(pdev);
+
+	keyboard_led->ec = cros_ec;
+
+	return 0;
+}
+
+static const struct keyboard_led_drvdata keyboard_led_drvdata_ec_pwm_mfd = {
+	.init = keyboard_led_init_ec_pwm_mfd,
+	.brightness_set_blocking = keyboard_led_set_brightness_ec_pwm,
+	.brightness_get = keyboard_led_get_brightness_ec_pwm,
+	.max_brightness = KEYBOARD_BACKLIGHT_MAX,
+};
+
+#else /* IS_ENABLED(CONFIG_MFD_CROS_EC_DEV) */
+
+static const struct keyboard_led_drvdata keyboard_led_drvdata_ec_pwm_mfd = {};
+
+#endif /* IS_ENABLED(CONFIG_MFD_CROS_EC_DEV) */
+
+static int keyboard_led_is_mfd_device(struct platform_device *pdev)
+{
+	return IS_ENABLED(CONFIG_MFD_CROS_EC_DEV) && mfd_get_cell(pdev);
+}
+
 static int keyboard_led_probe(struct platform_device *pdev)
 {
 	const struct keyboard_led_drvdata *drvdata;
 	struct keyboard_led *keyboard_led;
 	int error;
 
-	drvdata = device_get_match_data(&pdev->dev);
+	if (keyboard_led_is_mfd_device(pdev))
+		drvdata = &keyboard_led_drvdata_ec_pwm_mfd;
+	else
+		drvdata = device_get_match_data(&pdev->dev);
 	if (!drvdata)
 		return -EINVAL;
 
@@ -216,13 +250,15 @@  static int keyboard_led_probe(struct platform_device *pdev)
 	}
 
 	keyboard_led->cdev.name = "chromeos::kbd_backlight";
-	keyboard_led->cdev.flags |= LED_CORE_SUSPENDRESUME;
+	keyboard_led->cdev.flags |= LED_CORE_SUSPENDRESUME | LED_REJECT_NAME_CONFLICT;
 	keyboard_led->cdev.max_brightness = drvdata->max_brightness;
 	keyboard_led->cdev.brightness_set = drvdata->brightness_set;
 	keyboard_led->cdev.brightness_set_blocking = drvdata->brightness_set_blocking;
 	keyboard_led->cdev.brightness_get = drvdata->brightness_get;
 
 	error = devm_led_classdev_register(&pdev->dev, &keyboard_led->cdev);
+	if (error == -EEXIST) /* Already bound via other mechanism */
+		return -ENODEV;
 	if (error)
 		return error;