Linux-LEDs Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v8 1/2] platform/chrome: wilco_ec: Add keyboard backlight LED support
@ 2019-10-24 22:28 Nick Crews
  2019-10-24 22:28 ` [PATCH v8 2/2] power_supply: platform/chrome: wilco_ec: Add charging config driver Nick Crews
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Nick Crews @ 2019-10-24 22:28 UTC (permalink / raw)
  To: enric.balletbo, bleung, linux-leds, jacek.anaszewski, pavel
  Cc: linux-kernel, arnd, weiyongjun1, dlaurie, djkurtz, dtor, sjg,
	groeck, Nick Crews, Daniel Campello

The EC is in charge of controlling the keyboard backlight on
the Wilco platform. We expose a standard LED class device
named platform::kbd_backlight.

Since the EC will never change the backlight level of its own accord,
we don't need to implement a brightness_get() method.

Signed-off-by: Nick Crews <ncrews@chromium.org>
Signed-off-by: Daniel Campello <campello@chromium.org>
---
v8 changes:
 -Removed unneeded #includes from keyboard_leds.h

v7 changes:
 -Merged the LED stuff into the core wilco_ec module. This allows
  us to de-duplicate a lot of code, hide a lot of the LED internals
  from the core driver, and generally simplify things.
 -Follow reverse xmas tree variable declaration
 -Remove unneeded warning about BIOS not initializing the LEDs
 -Fix and standardize some comments and log messages.
 -Document all fields of wilco_keyboard_leds_msg
 -rm outdated and useless comment at top of core file.

v6 changes:
 -Rebased patch
 -Fixed bug related to request/response buffer pointers on
 send_kbbl_mesg()
 -Now sends WILCO_KBBL_SUBCMD_SET_STATE instead of
 WILCO_KBBL_SUBCMD_GET_STATE command for keyboard_led_set_brightness()

v5 changes:
 -Rename the LED device to "platform::kbd_backlight", to
 denote that this is the built-in system keyboard.

v4 changes:
 -Call keyboard_led_set_brightness() directly within
  initialize_brightness(), instead of calling the library function.

v3 changes:
 -Since this behaves the same as the standard Chrome OS keyboard
  backlight, rename the led device to "chromeos::kbd_backlight"
 -Move wilco_ec_keyboard_backlight_exists() into core module, so
  that the core does not depend upon the keyboard backlight driver.
 -This required moving some code into wilco-ec.h
 -Refactor out some common code in set_brightness() and
  initialize_brightness()

v2 changes:
 -Remove and fix uses of led vs LED in kconfig
 -Assume BIOS initializes brightness, but double check in probe()
 -Remove get_brightness() callback, as EC never changes brightness
  by itself.
 -Use a __packed struct as message instead of opaque array
 -Add exported wilco_ec_keyboard_leds_exist() so the core driver
  now only creates a platform _device if relevant
 -Fix use of keyboard_led_set_brightness() since it can sleep

BUG=b:141952530
TEST=Built and tested on a Wilco, including rmmod and modprobe of
wilco-ec.

 drivers/platform/chrome/wilco_ec/Makefile     |   3 +-
 drivers/platform/chrome/wilco_ec/core.c       |  13 +-
 .../platform/chrome/wilco_ec/keyboard_leds.c  | 191 ++++++++++++++++++
 include/linux/platform_data/wilco-ec.h        |  13 ++
 4 files changed, 215 insertions(+), 5 deletions(-)
 create mode 100644 drivers/platform/chrome/wilco_ec/keyboard_leds.c

diff --git a/drivers/platform/chrome/wilco_ec/Makefile b/drivers/platform/chrome/wilco_ec/Makefile
index bc817164596e..ecb3145cab18 100644
--- a/drivers/platform/chrome/wilco_ec/Makefile
+++ b/drivers/platform/chrome/wilco_ec/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
-wilco_ec-objs				:= core.o mailbox.o properties.o sysfs.o
+wilco_ec-objs				:= core.o keyboard_leds.o mailbox.o \
+					   properties.o sysfs.o
 obj-$(CONFIG_WILCO_EC)			+= wilco_ec.o
 wilco_ec_debugfs-objs			:= debugfs.o
 obj-$(CONFIG_WILCO_EC_DEBUGFS)		+= wilco_ec_debugfs.o
diff --git a/drivers/platform/chrome/wilco_ec/core.c b/drivers/platform/chrome/wilco_ec/core.c
index 3724bf4b77c6..36c78e52ff3c 100644
--- a/drivers/platform/chrome/wilco_ec/core.c
+++ b/drivers/platform/chrome/wilco_ec/core.c
@@ -5,10 +5,6 @@
  * Copyright 2018 Google LLC
  *
  * This is the entry point for the drivers that control the Wilco EC.
- * This driver is responsible for several tasks:
- * - Initialize the register interface that is used by wilco_ec_mailbox()
- * - Create a platform device which is picked up by the debugfs driver
- * - Create a platform device which is picked up by the RTC driver
  */
 
 #include <linux/acpi.h>
@@ -87,6 +83,15 @@ static int wilco_ec_probe(struct platform_device *pdev)
 		goto unregister_debugfs;
 	}
 
+	/* Set up the keyboard backlight LEDs. */
+	ret = wilco_keyboard_leds_init(ec);
+	if (ret < 0) {
+		dev_err(dev,
+			"Failed to initialize keyboard LEDs: %d\n",
+			ret);
+		goto unregister_rtc;
+	}
+
 	ret = wilco_ec_add_sysfs(ec);
 	if (ret < 0) {
 		dev_err(dev, "Failed to create sysfs entries: %d", ret);
diff --git a/drivers/platform/chrome/wilco_ec/keyboard_leds.c b/drivers/platform/chrome/wilco_ec/keyboard_leds.c
new file mode 100644
index 000000000000..bb0edf51dfda
--- /dev/null
+++ b/drivers/platform/chrome/wilco_ec/keyboard_leds.c
@@ -0,0 +1,191 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Keyboard backlight LED driver for the Wilco Embedded Controller
+ *
+ * Copyright 2019 Google LLC
+ *
+ * Since the EC will never change the backlight level of its own accord,
+ * we don't need to implement a brightness_get() method.
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/leds.h>
+#include <linux/platform_data/wilco-ec.h>
+#include <linux/slab.h>
+
+#define WILCO_EC_COMMAND_KBBL		0x75
+#define WILCO_KBBL_MODE_FLAG_PWM	BIT(1)	/* Set brightness by percent. */
+#define WILCO_KBBL_DEFAULT_BRIGHTNESS   0
+
+struct wilco_keyboard_leds {
+	struct wilco_ec_device *ec;
+	struct led_classdev keyboard;
+};
+
+enum wilco_kbbl_subcommand {
+	WILCO_KBBL_SUBCMD_GET_FEATURES = 0x00,
+	WILCO_KBBL_SUBCMD_GET_STATE    = 0x01,
+	WILCO_KBBL_SUBCMD_SET_STATE    = 0x02,
+};
+
+/**
+ * struct wilco_keyboard_leds_msg - Message to/from EC for keyboard LED control.
+ * @command: Always WILCO_EC_COMMAND_KBBL.
+ * @status: Set by EC to 0 on success, 0xFF on failure.
+ * @subcmd: One of enum wilco_kbbl_subcommand.
+ * @reserved3: Should be 0.
+ * @mode: Bit flags for used mode, we want to use WILCO_KBBL_MODE_FLAG_PWM.
+ * @reserved5to8: Should be 0.
+ * @percent: Brightness in 0-100. Only meaningful in PWM mode.
+ * @reserved10to15: Should be 0.
+ */
+struct wilco_keyboard_leds_msg {
+	u8 command;
+	u8 status;
+	u8 subcmd;
+	u8 reserved3;
+	u8 mode;
+	u8 reserved5to8[4];
+	u8 percent;
+	u8 reserved10to15[6];
+} __packed;
+
+/* Send a request, get a response, and check that the response is good. */
+static int send_kbbl_msg(struct wilco_ec_device *ec,
+			 struct wilco_keyboard_leds_msg *request,
+			 struct wilco_keyboard_leds_msg *response)
+{
+	struct wilco_ec_message msg;
+	int ret;
+
+	memset(&msg, 0, sizeof(msg));
+	msg.type = WILCO_EC_MSG_LEGACY;
+	msg.request_data = request;
+	msg.request_size = sizeof(*request);
+	msg.response_data = response;
+	msg.response_size = sizeof(*response);
+
+	ret = wilco_ec_mailbox(ec, &msg);
+	if (ret < 0) {
+		dev_err(ec->dev,
+			"Failed sending keyboard LEDs command: %d", ret);
+		return ret;
+	}
+
+	if (response->status) {
+		dev_err(ec->dev,
+			"EC reported failure sending keyboard LEDs command: %d",
+			response->status);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int set_kbbl(struct wilco_ec_device *ec, enum led_brightness brightness)
+{
+	struct wilco_keyboard_leds_msg request;
+	struct wilco_keyboard_leds_msg response;
+
+	memset(&request, 0, sizeof(request));
+	request.command = WILCO_EC_COMMAND_KBBL;
+	request.subcmd  = WILCO_KBBL_SUBCMD_SET_STATE;
+	request.mode    = WILCO_KBBL_MODE_FLAG_PWM;
+	request.percent = brightness;
+
+	return send_kbbl_msg(ec, &request, &response);
+}
+
+static int kbbl_exist(struct wilco_ec_device *ec, bool *exists)
+{
+	struct wilco_keyboard_leds_msg request;
+	struct wilco_keyboard_leds_msg response;
+	int ret;
+
+	memset(&request, 0, sizeof(request));
+	request.command = WILCO_EC_COMMAND_KBBL;
+	request.subcmd  = WILCO_KBBL_SUBCMD_GET_FEATURES;
+
+	ret = send_kbbl_msg(ec, &request, &response);
+	if (ret < 0)
+		return ret;
+
+	*exists = response.status != 0xFF;
+
+	return 0;
+}
+
+/**
+ * kbbl_init() - Initialize the state of the keyboard backlight.
+ * @ec: EC device to talk to.
+ *
+ * Gets the current brightness, ensuring that the BIOS already initialized the
+ * backlight to PWM mode. If not in PWM mode, then the current brightness is
+ * meaningless, so set the brightness to WILCO_KBBL_DEFAULT_BRIGHTNESS.
+ *
+ * Return: Final brightness of the keyboard, or negative error code on failure.
+ */
+static int kbbl_init(struct wilco_ec_device *ec)
+{
+	struct wilco_keyboard_leds_msg request;
+	struct wilco_keyboard_leds_msg response;
+	int ret;
+
+	memset(&request, 0, sizeof(request));
+	request.command = WILCO_EC_COMMAND_KBBL;
+	request.subcmd  = WILCO_KBBL_SUBCMD_GET_STATE;
+
+	ret = send_kbbl_msg(ec, &request, &response);
+	if (ret < 0)
+		return ret;
+
+	if (response.mode & WILCO_KBBL_MODE_FLAG_PWM)
+		return response.percent;
+
+	ret = set_kbbl(ec, WILCO_KBBL_DEFAULT_BRIGHTNESS);
+	if (ret < 0)
+		return ret;
+
+	return WILCO_KBBL_DEFAULT_BRIGHTNESS;
+}
+
+static int wilco_keyboard_leds_set(struct led_classdev *cdev,
+				   enum led_brightness brightness)
+{
+	struct wilco_keyboard_leds *wkl =
+		container_of(cdev, struct wilco_keyboard_leds, keyboard);
+	return set_kbbl(wkl->ec, brightness);
+}
+
+int wilco_keyboard_leds_init(struct wilco_ec_device *ec)
+{
+	struct wilco_keyboard_leds *wkl;
+	bool leds_exist;
+	int ret;
+
+	ret = kbbl_exist(ec, &leds_exist);
+	if (ret < 0) {
+		dev_err(ec->dev,
+			"Failed checking keyboard LEDs support: %d", ret);
+		return ret;
+	}
+	if (!leds_exist)
+		return 0;
+
+	wkl = devm_kzalloc(ec->dev, sizeof(*wkl), GFP_KERNEL);
+	if (!wkl)
+		return -ENOMEM;
+
+	wkl->ec = ec;
+	wkl->keyboard.name = "platform::kbd_backlight";
+	wkl->keyboard.max_brightness = 100;
+	wkl->keyboard.flags = LED_CORE_SUSPENDRESUME;
+	wkl->keyboard.brightness_set_blocking = wilco_keyboard_leds_set;
+	ret = kbbl_init(ec);
+	if (ret < 0)
+		return ret;
+	wkl->keyboard.brightness = ret;
+
+	return devm_led_classdev_register(ec->dev, &wkl->keyboard);
+}
diff --git a/include/linux/platform_data/wilco-ec.h b/include/linux/platform_data/wilco-ec.h
index ad03b586a095..0f7df3498a24 100644
--- a/include/linux/platform_data/wilco-ec.h
+++ b/include/linux/platform_data/wilco-ec.h
@@ -120,6 +120,19 @@ struct wilco_ec_message {
  */
 int wilco_ec_mailbox(struct wilco_ec_device *ec, struct wilco_ec_message *msg);
 
+/**
+ * wilco_keyboard_leds_init() - Set up the keyboard backlight LEDs.
+ * @ec: EC device to query.
+ *
+ * After this call, the keyboard backlight will be exposed through a an LED
+ * device at /sys/class/leds.
+ *
+ * This may sleep because it uses wilco_ec_mailbox().
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+int wilco_keyboard_leds_init(struct wilco_ec_device *ec);
+
 /*
  * A Property is typically a data item that is stored to NVRAM
  * by the EC. Each of these data items has an index associated
-- 
2.21.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v8 2/2] power_supply: platform/chrome: wilco_ec: Add charging config driver
  2019-10-24 22:28 [PATCH v8 1/2] platform/chrome: wilco_ec: Add keyboard backlight LED support Nick Crews
@ 2019-10-24 22:28 ` Nick Crews
  2019-11-06 15:16   ` Enric Balletbo i Serra
  2019-10-25 17:33 ` [PATCH v8 1/2] platform/chrome: wilco_ec: Add keyboard backlight LED support Daniel Campello
  2019-11-06 15:15 ` Enric Balletbo i Serra
  2 siblings, 1 reply; 7+ messages in thread
From: Nick Crews @ 2019-10-24 22:28 UTC (permalink / raw)
  To: enric.balletbo, bleung, linux-leds, jacek.anaszewski, pavel
  Cc: linux-kernel, arnd, weiyongjun1, dlaurie, djkurtz, dtor, sjg,
	groeck, Nick Crews

Add a device to control the charging algorithm used on Wilco devices,
which will be picked up by the drivers/power/supply/wilco-charger.c
driver. See Documentation/ABI/testing/sysfs-class-power-wilco for the
userspace interface and other info.

Signed-off-by: Nick Crews <ncrews@chromium.org>
---
 drivers/platform/chrome/wilco_ec/core.c | 15 ++++++++++++++-
 include/linux/platform_data/wilco-ec.h  |  2 ++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/wilco_ec/core.c b/drivers/platform/chrome/wilco_ec/core.c
index 36c78e52ff3c..5210c357feef 100644
--- a/drivers/platform/chrome/wilco_ec/core.c
+++ b/drivers/platform/chrome/wilco_ec/core.c
@@ -98,6 +98,16 @@ static int wilco_ec_probe(struct platform_device *pdev)
 		goto unregister_rtc;
 	}
 
+	/* Register child device to be found by charger config driver. */
+	ec->charger_pdev = platform_device_register_data(dev, "wilco-charger",
+							 PLATFORM_DEVID_AUTO,
+							 NULL, 0);
+	if (IS_ERR(ec->charger_pdev)) {
+		dev_err(dev, "Failed to create charger platform device\n");
+		ret = PTR_ERR(ec->charger_pdev);
+		goto remove_sysfs;
+	}
+
 	/* Register child device that will be found by the telemetry driver. */
 	ec->telem_pdev = platform_device_register_data(dev, "wilco_telem",
 						       PLATFORM_DEVID_AUTO,
@@ -105,11 +115,13 @@ static int wilco_ec_probe(struct platform_device *pdev)
 	if (IS_ERR(ec->telem_pdev)) {
 		dev_err(dev, "Failed to create telemetry platform device\n");
 		ret = PTR_ERR(ec->telem_pdev);
-		goto remove_sysfs;
+		goto unregister_charge_config;
 	}
 
 	return 0;
 
+unregister_charge_config:
+	platform_device_unregister(ec->charger_pdev);
 remove_sysfs:
 	wilco_ec_remove_sysfs(ec);
 unregister_rtc:
@@ -125,6 +137,7 @@ static int wilco_ec_remove(struct platform_device *pdev)
 {
 	struct wilco_ec_device *ec = platform_get_drvdata(pdev);
 
+	platform_device_unregister(ec->charger_pdev);
 	wilco_ec_remove_sysfs(ec);
 	platform_device_unregister(ec->telem_pdev);
 	platform_device_unregister(ec->rtc_pdev);
diff --git a/include/linux/platform_data/wilco-ec.h b/include/linux/platform_data/wilco-ec.h
index 0f7df3498a24..afede15a95bf 100644
--- a/include/linux/platform_data/wilco-ec.h
+++ b/include/linux/platform_data/wilco-ec.h
@@ -29,6 +29,7 @@
  * @data_size: Size of the data buffer used for EC communication.
  * @debugfs_pdev: The child platform_device used by the debugfs sub-driver.
  * @rtc_pdev: The child platform_device used by the RTC sub-driver.
+ * @charger_pdev: Child platform_device used by the charger config sub-driver.
  * @telem_pdev: The child platform_device used by the telemetry sub-driver.
  */
 struct wilco_ec_device {
@@ -41,6 +42,7 @@ struct wilco_ec_device {
 	size_t data_size;
 	struct platform_device *debugfs_pdev;
 	struct platform_device *rtc_pdev;
+	struct platform_device *charger_pdev;
 	struct platform_device *telem_pdev;
 };
 
-- 
2.21.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v8 1/2] platform/chrome: wilco_ec: Add keyboard backlight LED support
  2019-10-24 22:28 [PATCH v8 1/2] platform/chrome: wilco_ec: Add keyboard backlight LED support Nick Crews
  2019-10-24 22:28 ` [PATCH v8 2/2] power_supply: platform/chrome: wilco_ec: Add charging config driver Nick Crews
@ 2019-10-25 17:33 ` Daniel Campello
  2019-10-25 19:18   ` Nick Crews
  2019-11-06 15:15 ` Enric Balletbo i Serra
  2 siblings, 1 reply; 7+ messages in thread
From: Daniel Campello @ 2019-10-25 17:33 UTC (permalink / raw)
  To: Nick Crews
  Cc: enric.balletbo, bleung, linux-leds, jacek.anaszewski, pavel,
	linux-kernel, arnd, weiyongjun1, dlaurie, djkurtz, dtor, sjg,
	groeck

LGTM.
Reviewed-by: Daniel Campello <campello@chromium.org>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v8 1/2] platform/chrome: wilco_ec: Add keyboard backlight LED support
  2019-10-25 17:33 ` [PATCH v8 1/2] platform/chrome: wilco_ec: Add keyboard backlight LED support Daniel Campello
@ 2019-10-25 19:18   ` Nick Crews
  0 siblings, 0 replies; 7+ messages in thread
From: Nick Crews @ 2019-10-25 19:18 UTC (permalink / raw)
  To: Daniel Campello
  Cc: Enric Balletbo i Serra, Benson Leung, linux-leds,
	Jacek Anaszewski, Pavel Machek, linux-kernel, Arnd Bergmann,
	Wei Yongjun, Duncan Laurie, Daniel Kurtz, Dmitry Torokhov,
	Simon Glass, groeck

On Fri, Oct 25, 2019 at 11:33 AM Daniel Campello <campello@chromium.org> wrote:
>
> LGTM.
> Reviewed-by: Daniel Campello <campello@chromium.

FYI I realized some of the imports in keyboard_leds.c are
unneeded, so I sent a v8 version that fixes this. I presume
you would LGTM that one too though.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v8 1/2] platform/chrome: wilco_ec: Add keyboard backlight LED support
  2019-10-24 22:28 [PATCH v8 1/2] platform/chrome: wilco_ec: Add keyboard backlight LED support Nick Crews
  2019-10-24 22:28 ` [PATCH v8 2/2] power_supply: platform/chrome: wilco_ec: Add charging config driver Nick Crews
  2019-10-25 17:33 ` [PATCH v8 1/2] platform/chrome: wilco_ec: Add keyboard backlight LED support Daniel Campello
@ 2019-11-06 15:15 ` Enric Balletbo i Serra
       [not found]   ` <CAHcu+VYdMxoFbcxYxB1FnDSdsM=w9UsVKjF0S48LZGG0ZxKUDA@mail.gmail.com>
  2 siblings, 1 reply; 7+ messages in thread
From: Enric Balletbo i Serra @ 2019-11-06 15:15 UTC (permalink / raw)
  To: Nick Crews, bleung, linux-leds, jacek.anaszewski, pavel
  Cc: linux-kernel, arnd, weiyongjun1, dlaurie, djkurtz, dtor, sjg,
	groeck, Daniel Campello

Hi Daniel,

On 25/10/19 0:28, Nick Crews wrote:
> The EC is in charge of controlling the keyboard backlight on
> the Wilco platform. We expose a standard LED class device
> named platform::kbd_backlight.
> 
> Since the EC will never change the backlight level of its own accord,
> we don't need to implement a brightness_get() method.
> 
> Signed-off-by: Nick Crews <ncrews@chromium.org>
> Signed-off-by: Daniel Campello <campello@chromium.org>

As you know 0day reported an issue that needs to be solved, waiting for a new
version.

Thanks,
 Enric

> ---
> v8 changes:
>  -Removed unneeded #includes from keyboard_leds.h
> 
> v7 changes:
>  -Merged the LED stuff into the core wilco_ec module. This allows
>   us to de-duplicate a lot of code, hide a lot of the LED internals
>   from the core driver, and generally simplify things.
>  -Follow reverse xmas tree variable declaration
>  -Remove unneeded warning about BIOS not initializing the LEDs
>  -Fix and standardize some comments and log messages.
>  -Document all fields of wilco_keyboard_leds_msg
>  -rm outdated and useless comment at top of core file.
> 
> v6 changes:
>  -Rebased patch
>  -Fixed bug related to request/response buffer pointers on
>  send_kbbl_mesg()
>  -Now sends WILCO_KBBL_SUBCMD_SET_STATE instead of
>  WILCO_KBBL_SUBCMD_GET_STATE command for keyboard_led_set_brightness()
> 
> v5 changes:
>  -Rename the LED device to "platform::kbd_backlight", to
>  denote that this is the built-in system keyboard.
> 
> v4 changes:
>  -Call keyboard_led_set_brightness() directly within
>   initialize_brightness(), instead of calling the library function.
> 
> v3 changes:
>  -Since this behaves the same as the standard Chrome OS keyboard
>   backlight, rename the led device to "chromeos::kbd_backlight"
>  -Move wilco_ec_keyboard_backlight_exists() into core module, so
>   that the core does not depend upon the keyboard backlight driver.
>  -This required moving some code into wilco-ec.h
>  -Refactor out some common code in set_brightness() and
>   initialize_brightness()
> 
> v2 changes:
>  -Remove and fix uses of led vs LED in kconfig
>  -Assume BIOS initializes brightness, but double check in probe()
>  -Remove get_brightness() callback, as EC never changes brightness
>   by itself.
>  -Use a __packed struct as message instead of opaque array
>  -Add exported wilco_ec_keyboard_leds_exist() so the core driver
>   now only creates a platform _device if relevant
>  -Fix use of keyboard_led_set_brightness() since it can sleep
> 
> BUG=b:141952530
> TEST=Built and tested on a Wilco, including rmmod and modprobe of
> wilco-ec.
> 
>  drivers/platform/chrome/wilco_ec/Makefile     |   3 +-
>  drivers/platform/chrome/wilco_ec/core.c       |  13 +-
>  .../platform/chrome/wilco_ec/keyboard_leds.c  | 191 ++++++++++++++++++
>  include/linux/platform_data/wilco-ec.h        |  13 ++
>  4 files changed, 215 insertions(+), 5 deletions(-)
>  create mode 100644 drivers/platform/chrome/wilco_ec/keyboard_leds.c
> 
> diff --git a/drivers/platform/chrome/wilco_ec/Makefile b/drivers/platform/chrome/wilco_ec/Makefile
> index bc817164596e..ecb3145cab18 100644
> --- a/drivers/platform/chrome/wilco_ec/Makefile
> +++ b/drivers/platform/chrome/wilco_ec/Makefile
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  
> -wilco_ec-objs				:= core.o mailbox.o properties.o sysfs.o
> +wilco_ec-objs				:= core.o keyboard_leds.o mailbox.o \
> +					   properties.o sysfs.o
>  obj-$(CONFIG_WILCO_EC)			+= wilco_ec.o
>  wilco_ec_debugfs-objs			:= debugfs.o
>  obj-$(CONFIG_WILCO_EC_DEBUGFS)		+= wilco_ec_debugfs.o
> diff --git a/drivers/platform/chrome/wilco_ec/core.c b/drivers/platform/chrome/wilco_ec/core.c
> index 3724bf4b77c6..36c78e52ff3c 100644
> --- a/drivers/platform/chrome/wilco_ec/core.c
> +++ b/drivers/platform/chrome/wilco_ec/core.c
> @@ -5,10 +5,6 @@
>   * Copyright 2018 Google LLC
>   *
>   * This is the entry point for the drivers that control the Wilco EC.
> - * This driver is responsible for several tasks:
> - * - Initialize the register interface that is used by wilco_ec_mailbox()
> - * - Create a platform device which is picked up by the debugfs driver
> - * - Create a platform device which is picked up by the RTC driver
>   */
>  
>  #include <linux/acpi.h>
> @@ -87,6 +83,15 @@ static int wilco_ec_probe(struct platform_device *pdev)
>  		goto unregister_debugfs;
>  	}
>  
> +	/* Set up the keyboard backlight LEDs. */
> +	ret = wilco_keyboard_leds_init(ec);
> +	if (ret < 0) {
> +		dev_err(dev,
> +			"Failed to initialize keyboard LEDs: %d\n",
> +			ret);
> +		goto unregister_rtc;
> +	}
> +
>  	ret = wilco_ec_add_sysfs(ec);
>  	if (ret < 0) {
>  		dev_err(dev, "Failed to create sysfs entries: %d", ret);
> diff --git a/drivers/platform/chrome/wilco_ec/keyboard_leds.c b/drivers/platform/chrome/wilco_ec/keyboard_leds.c
> new file mode 100644
> index 000000000000..bb0edf51dfda
> --- /dev/null
> +++ b/drivers/platform/chrome/wilco_ec/keyboard_leds.c
> @@ -0,0 +1,191 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Keyboard backlight LED driver for the Wilco Embedded Controller
> + *
> + * Copyright 2019 Google LLC
> + *
> + * Since the EC will never change the backlight level of its own accord,
> + * we don't need to implement a brightness_get() method.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/leds.h>
> +#include <linux/platform_data/wilco-ec.h>
> +#include <linux/slab.h>
> +
> +#define WILCO_EC_COMMAND_KBBL		0x75
> +#define WILCO_KBBL_MODE_FLAG_PWM	BIT(1)	/* Set brightness by percent. */
> +#define WILCO_KBBL_DEFAULT_BRIGHTNESS   0
> +
> +struct wilco_keyboard_leds {
> +	struct wilco_ec_device *ec;
> +	struct led_classdev keyboard;
> +};
> +
> +enum wilco_kbbl_subcommand {
> +	WILCO_KBBL_SUBCMD_GET_FEATURES = 0x00,
> +	WILCO_KBBL_SUBCMD_GET_STATE    = 0x01,
> +	WILCO_KBBL_SUBCMD_SET_STATE    = 0x02,
> +};
> +
> +/**
> + * struct wilco_keyboard_leds_msg - Message to/from EC for keyboard LED control.
> + * @command: Always WILCO_EC_COMMAND_KBBL.
> + * @status: Set by EC to 0 on success, 0xFF on failure.
> + * @subcmd: One of enum wilco_kbbl_subcommand.
> + * @reserved3: Should be 0.
> + * @mode: Bit flags for used mode, we want to use WILCO_KBBL_MODE_FLAG_PWM.
> + * @reserved5to8: Should be 0.
> + * @percent: Brightness in 0-100. Only meaningful in PWM mode.
> + * @reserved10to15: Should be 0.
> + */
> +struct wilco_keyboard_leds_msg {
> +	u8 command;
> +	u8 status;
> +	u8 subcmd;
> +	u8 reserved3;
> +	u8 mode;
> +	u8 reserved5to8[4];
> +	u8 percent;
> +	u8 reserved10to15[6];
> +} __packed;
> +
> +/* Send a request, get a response, and check that the response is good. */
> +static int send_kbbl_msg(struct wilco_ec_device *ec,
> +			 struct wilco_keyboard_leds_msg *request,
> +			 struct wilco_keyboard_leds_msg *response)
> +{
> +	struct wilco_ec_message msg;
> +	int ret;
> +
> +	memset(&msg, 0, sizeof(msg));
> +	msg.type = WILCO_EC_MSG_LEGACY;
> +	msg.request_data = request;
> +	msg.request_size = sizeof(*request);
> +	msg.response_data = response;
> +	msg.response_size = sizeof(*response);
> +
> +	ret = wilco_ec_mailbox(ec, &msg);
> +	if (ret < 0) {
> +		dev_err(ec->dev,
> +			"Failed sending keyboard LEDs command: %d", ret);
> +		return ret;
> +	}
> +
> +	if (response->status) {
> +		dev_err(ec->dev,
> +			"EC reported failure sending keyboard LEDs command: %d",
> +			response->status);
> +		return -EIO;
> +	}
> +
> +	return 0;
> +}
> +
> +static int set_kbbl(struct wilco_ec_device *ec, enum led_brightness brightness)
> +{
> +	struct wilco_keyboard_leds_msg request;
> +	struct wilco_keyboard_leds_msg response;
> +
> +	memset(&request, 0, sizeof(request));
> +	request.command = WILCO_EC_COMMAND_KBBL;
> +	request.subcmd  = WILCO_KBBL_SUBCMD_SET_STATE;
> +	request.mode    = WILCO_KBBL_MODE_FLAG_PWM;
> +	request.percent = brightness;
> +
> +	return send_kbbl_msg(ec, &request, &response);
> +}
> +
> +static int kbbl_exist(struct wilco_ec_device *ec, bool *exists)
> +{
> +	struct wilco_keyboard_leds_msg request;
> +	struct wilco_keyboard_leds_msg response;
> +	int ret;
> +
> +	memset(&request, 0, sizeof(request));
> +	request.command = WILCO_EC_COMMAND_KBBL;
> +	request.subcmd  = WILCO_KBBL_SUBCMD_GET_FEATURES;
> +
> +	ret = send_kbbl_msg(ec, &request, &response);
> +	if (ret < 0)
> +		return ret;
> +
> +	*exists = response.status != 0xFF;
> +
> +	return 0;
> +}
> +
> +/**
> + * kbbl_init() - Initialize the state of the keyboard backlight.
> + * @ec: EC device to talk to.
> + *
> + * Gets the current brightness, ensuring that the BIOS already initialized the
> + * backlight to PWM mode. If not in PWM mode, then the current brightness is
> + * meaningless, so set the brightness to WILCO_KBBL_DEFAULT_BRIGHTNESS.
> + *
> + * Return: Final brightness of the keyboard, or negative error code on failure.
> + */
> +static int kbbl_init(struct wilco_ec_device *ec)
> +{
> +	struct wilco_keyboard_leds_msg request;
> +	struct wilco_keyboard_leds_msg response;
> +	int ret;
> +
> +	memset(&request, 0, sizeof(request));
> +	request.command = WILCO_EC_COMMAND_KBBL;
> +	request.subcmd  = WILCO_KBBL_SUBCMD_GET_STATE;
> +
> +	ret = send_kbbl_msg(ec, &request, &response);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (response.mode & WILCO_KBBL_MODE_FLAG_PWM)
> +		return response.percent;
> +
> +	ret = set_kbbl(ec, WILCO_KBBL_DEFAULT_BRIGHTNESS);
> +	if (ret < 0)
> +		return ret;
> +
> +	return WILCO_KBBL_DEFAULT_BRIGHTNESS;
> +}
> +
> +static int wilco_keyboard_leds_set(struct led_classdev *cdev,
> +				   enum led_brightness brightness)
> +{
> +	struct wilco_keyboard_leds *wkl =
> +		container_of(cdev, struct wilco_keyboard_leds, keyboard);
> +	return set_kbbl(wkl->ec, brightness);
> +}
> +
> +int wilco_keyboard_leds_init(struct wilco_ec_device *ec)
> +{
> +	struct wilco_keyboard_leds *wkl;
> +	bool leds_exist;
> +	int ret;
> +
> +	ret = kbbl_exist(ec, &leds_exist);
> +	if (ret < 0) {
> +		dev_err(ec->dev,
> +			"Failed checking keyboard LEDs support: %d", ret);
> +		return ret;
> +	}
> +	if (!leds_exist)
> +		return 0;
> +
> +	wkl = devm_kzalloc(ec->dev, sizeof(*wkl), GFP_KERNEL);
> +	if (!wkl)
> +		return -ENOMEM;
> +
> +	wkl->ec = ec;
> +	wkl->keyboard.name = "platform::kbd_backlight";
> +	wkl->keyboard.max_brightness = 100;
> +	wkl->keyboard.flags = LED_CORE_SUSPENDRESUME;
> +	wkl->keyboard.brightness_set_blocking = wilco_keyboard_leds_set;
> +	ret = kbbl_init(ec);
> +	if (ret < 0)
> +		return ret;
> +	wkl->keyboard.brightness = ret;
> +
> +	return devm_led_classdev_register(ec->dev, &wkl->keyboard);
> +}
> diff --git a/include/linux/platform_data/wilco-ec.h b/include/linux/platform_data/wilco-ec.h
> index ad03b586a095..0f7df3498a24 100644
> --- a/include/linux/platform_data/wilco-ec.h
> +++ b/include/linux/platform_data/wilco-ec.h
> @@ -120,6 +120,19 @@ struct wilco_ec_message {
>   */
>  int wilco_ec_mailbox(struct wilco_ec_device *ec, struct wilco_ec_message *msg);
>  
> +/**
> + * wilco_keyboard_leds_init() - Set up the keyboard backlight LEDs.
> + * @ec: EC device to query.
> + *
> + * After this call, the keyboard backlight will be exposed through a an LED
> + * device at /sys/class/leds.
> + *
> + * This may sleep because it uses wilco_ec_mailbox().
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +int wilco_keyboard_leds_init(struct wilco_ec_device *ec);
> +
>  /*
>   * A Property is typically a data item that is stored to NVRAM
>   * by the EC. Each of these data items has an index associated
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v8 2/2] power_supply: platform/chrome: wilco_ec: Add charging config driver
  2019-10-24 22:28 ` [PATCH v8 2/2] power_supply: platform/chrome: wilco_ec: Add charging config driver Nick Crews
@ 2019-11-06 15:16   ` Enric Balletbo i Serra
  0 siblings, 0 replies; 7+ messages in thread
From: Enric Balletbo i Serra @ 2019-11-06 15:16 UTC (permalink / raw)
  To: Nick Crews, bleung, linux-leds, jacek.anaszewski, pavel
  Cc: linux-kernel, arnd, weiyongjun1, dlaurie, djkurtz, dtor, sjg,
	groeck, Daniel Campello

Hi Nick, Daniel,

On 25/10/19 0:28, Nick Crews wrote:
> Add a device to control the charging algorithm used on Wilco devices,
> which will be picked up by the drivers/power/supply/wilco-charger.c
> driver. See Documentation/ABI/testing/sysfs-class-power-wilco for the
> userspace interface and other info.
> 
> Signed-off-by: Nick Crews <ncrews@chromium.org>

Applied for 5.5.

Thanks,
 Enric

> ---
>  drivers/platform/chrome/wilco_ec/core.c | 15 ++++++++++++++-
>  include/linux/platform_data/wilco-ec.h  |  2 ++
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/chrome/wilco_ec/core.c b/drivers/platform/chrome/wilco_ec/core.c
> index 36c78e52ff3c..5210c357feef 100644
> --- a/drivers/platform/chrome/wilco_ec/core.c
> +++ b/drivers/platform/chrome/wilco_ec/core.c
> @@ -98,6 +98,16 @@ static int wilco_ec_probe(struct platform_device *pdev)
>  		goto unregister_rtc;
>  	}
>  
> +	/* Register child device to be found by charger config driver. */
> +	ec->charger_pdev = platform_device_register_data(dev, "wilco-charger",
> +							 PLATFORM_DEVID_AUTO,
> +							 NULL, 0);
> +	if (IS_ERR(ec->charger_pdev)) {
> +		dev_err(dev, "Failed to create charger platform device\n");
> +		ret = PTR_ERR(ec->charger_pdev);
> +		goto remove_sysfs;
> +	}
> +
>  	/* Register child device that will be found by the telemetry driver. */
>  	ec->telem_pdev = platform_device_register_data(dev, "wilco_telem",
>  						       PLATFORM_DEVID_AUTO,
> @@ -105,11 +115,13 @@ static int wilco_ec_probe(struct platform_device *pdev)
>  	if (IS_ERR(ec->telem_pdev)) {
>  		dev_err(dev, "Failed to create telemetry platform device\n");
>  		ret = PTR_ERR(ec->telem_pdev);
> -		goto remove_sysfs;
> +		goto unregister_charge_config;
>  	}
>  
>  	return 0;
>  
> +unregister_charge_config:
> +	platform_device_unregister(ec->charger_pdev);
>  remove_sysfs:
>  	wilco_ec_remove_sysfs(ec);
>  unregister_rtc:
> @@ -125,6 +137,7 @@ static int wilco_ec_remove(struct platform_device *pdev)
>  {
>  	struct wilco_ec_device *ec = platform_get_drvdata(pdev);
>  
> +	platform_device_unregister(ec->charger_pdev);
>  	wilco_ec_remove_sysfs(ec);
>  	platform_device_unregister(ec->telem_pdev);
>  	platform_device_unregister(ec->rtc_pdev);
> diff --git a/include/linux/platform_data/wilco-ec.h b/include/linux/platform_data/wilco-ec.h
> index 0f7df3498a24..afede15a95bf 100644
> --- a/include/linux/platform_data/wilco-ec.h
> +++ b/include/linux/platform_data/wilco-ec.h
> @@ -29,6 +29,7 @@
>   * @data_size: Size of the data buffer used for EC communication.
>   * @debugfs_pdev: The child platform_device used by the debugfs sub-driver.
>   * @rtc_pdev: The child platform_device used by the RTC sub-driver.
> + * @charger_pdev: Child platform_device used by the charger config sub-driver.
>   * @telem_pdev: The child platform_device used by the telemetry sub-driver.
>   */
>  struct wilco_ec_device {
> @@ -41,6 +42,7 @@ struct wilco_ec_device {
>  	size_t data_size;
>  	struct platform_device *debugfs_pdev;
>  	struct platform_device *rtc_pdev;
> +	struct platform_device *charger_pdev;
>  	struct platform_device *telem_pdev;
>  };
>  
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v8 1/2] platform/chrome: wilco_ec: Add keyboard backlight LED support
       [not found]   ` <CAHcu+VYdMxoFbcxYxB1FnDSdsM=w9UsVKjF0S48LZGG0ZxKUDA@mail.gmail.com>
@ 2019-11-06 16:39     ` Daniel Campello
  0 siblings, 0 replies; 7+ messages in thread
From: Daniel Campello @ 2019-11-06 16:39 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: Nick Crews, bleung, linux-leds, jacek.anaszewski, pavel,
	linux-kernel, arnd, weiyongjun1, dlaurie, djkurtz, dtor, sjg,
	groeck

HI again,

On Wed, Nov 6, 2019 at 9:34 AM Daniel Campello <campello@google.com> wrote:
>
> Hi Enric,
>
> On Wed, Nov 6, 2019 at 8:15 AM Enric Balletbo i Serra <enric.balletbo@collabora.com> wrote:
>>
>> Hi Daniel,
>>
>> On 25/10/19 0:28, Nick Crews wrote:
>> > The EC is in charge of controlling the keyboard backlight on
>> > the Wilco platform. We expose a standard LED class device
>> > named platform::kbd_backlight.
>> >
>> > Since the EC will never change the backlight level of its own accord,
>> > we don't need to implement a brightness_get() method.
>> >
>> > Signed-off-by: Nick Crews <ncrews@chromium.org>
>> > Signed-off-by: Daniel Campello <campello@chromium.org>
>>
>> As you know 0day reported an issue that needs to be solved, waiting for a new
>> version.
>>
>> Thanks,
>>  Enric
>
>
> I just sent a v9 of the patch with the issue addressed.
>
> Thanks,
> Daniel

Resending in plain text format... Sorry about that...
Daniel

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-24 22:28 [PATCH v8 1/2] platform/chrome: wilco_ec: Add keyboard backlight LED support Nick Crews
2019-10-24 22:28 ` [PATCH v8 2/2] power_supply: platform/chrome: wilco_ec: Add charging config driver Nick Crews
2019-11-06 15:16   ` Enric Balletbo i Serra
2019-10-25 17:33 ` [PATCH v8 1/2] platform/chrome: wilco_ec: Add keyboard backlight LED support Daniel Campello
2019-10-25 19:18   ` Nick Crews
2019-11-06 15:15 ` Enric Balletbo i Serra
     [not found]   ` <CAHcu+VYdMxoFbcxYxB1FnDSdsM=w9UsVKjF0S48LZGG0ZxKUDA@mail.gmail.com>
2019-11-06 16:39     ` Daniel Campello

Linux-LEDs Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-leds/0 linux-leds/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-leds linux-leds/ https://lore.kernel.org/linux-leds \
		linux-leds@vger.kernel.org
	public-inbox-index linux-leds

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-leds


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git