EC_CMD_PWM_GET_KEYBOARD_BACKLIGHT and EC_CMD_PWM_SET_KEYBOARD_BACKLIGHT
are marked as obsolete in `cros_ec_commands.h`, this patch removes the
usage of these commands from the driver.
The max brightness value is now set to `EC_PWM_MAX_DUTY` (65535) when
running on the EC, as this is the maximum value that the EC can accept.
Signed-off-by: Stephen Horvath <[email protected]>
---
.../platform/chrome/cros_kbd_led_backlight.c | 37 +++++++++++++------
1 file changed, 25 insertions(+), 12 deletions(-)
diff --git a/drivers/platform/chrome/cros_kbd_led_backlight.c b/drivers/platform/chrome/cros_kbd_led_backlight.c
index 06e9a57536af..bc021437e8b1 100644
--- a/drivers/platform/chrome/cros_kbd_led_backlight.c
+++ b/drivers/platform/chrome/cros_kbd_led_backlight.c
@@ -47,10 +47,10 @@ struct keyboard_led_drvdata {
enum led_brightness max_brightness;
};
-#define KEYBOARD_BACKLIGHT_MAX 100
-
#ifdef CONFIG_ACPI
+#define ACPI_KEYBOARD_BACKLIGHT_MAX 100
+
/* Keyboard LED ACPI Device must be defined in firmware */
#define ACPI_KEYBOARD_BACKLIGHT_DEVICE "\\_SB.KBLT"
#define ACPI_KEYBOARD_BACKLIGHT_READ ACPI_KEYBOARD_BACKLIGHT_DEVICE ".KBQC"
@@ -114,7 +114,7 @@ static const struct keyboard_led_drvdata keyboard_led_drvdata_acpi = {
.init = keyboard_led_init_acpi,
.brightness_set = keyboard_led_set_brightness_acpi,
.brightness_get = keyboard_led_get_brightness_acpi,
- .max_brightness = KEYBOARD_BACKLIGHT_MAX,
+ .max_brightness = ACPI_KEYBOARD_BACKLIGHT_MAX,
};
#endif /* CONFIG_ACPI */
@@ -127,18 +127,22 @@ keyboard_led_set_brightness_ec_pwm(struct led_classdev *cdev,
{
struct {
struct cros_ec_command msg;
- struct ec_params_pwm_set_keyboard_backlight params;
+ struct ec_params_pwm_set_duty params;
} __packed buf;
- struct ec_params_pwm_set_keyboard_backlight *params = &buf.params;
+ struct ec_params_pwm_set_duty *params = &buf.params;
struct cros_ec_command *msg = &buf.msg;
struct keyboard_led *keyboard_led = container_of(cdev, struct keyboard_led, cdev);
memset(&buf, 0, sizeof(buf));
- msg->command = EC_CMD_PWM_SET_KEYBOARD_BACKLIGHT;
+ msg->version = 0;
+ msg->command = EC_CMD_PWM_SET_DUTY;
+ msg->insize = 0;
msg->outsize = sizeof(*params);
- params->percent = brightness;
+ params->duty = brightness;
+ params->pwm_type = EC_PWM_TYPE_KB_LIGHT;
+ params->index = 0;
return cros_ec_cmd_xfer_status(keyboard_led->ec, msg);
}
@@ -148,23 +152,32 @@ keyboard_led_get_brightness_ec_pwm(struct led_classdev *cdev)
{
struct {
struct cros_ec_command msg;
- struct ec_response_pwm_get_keyboard_backlight resp;
+ union {
+ struct ec_params_pwm_get_duty params;
+ struct ec_response_pwm_get_duty resp;
+ };
} __packed buf;
- struct ec_response_pwm_get_keyboard_backlight *resp = &buf.resp;
+ struct ec_params_pwm_get_duty *params = &buf.params;
+ struct ec_response_pwm_get_duty *resp = &buf.resp;
struct cros_ec_command *msg = &buf.msg;
struct keyboard_led *keyboard_led = container_of(cdev, struct keyboard_led, cdev);
int ret;
memset(&buf, 0, sizeof(buf));
- msg->command = EC_CMD_PWM_GET_KEYBOARD_BACKLIGHT;
+ msg->version = 0;
+ msg->command = EC_CMD_PWM_GET_DUTY;
msg->insize = sizeof(*resp);
+ msg->outsize = sizeof(*params);
+
+ params->pwm_type = EC_PWM_TYPE_KB_LIGHT;
+ params->index = 0;
ret = cros_ec_cmd_xfer_status(keyboard_led->ec, msg);
if (ret < 0)
return ret;
- return resp->percent;
+ return resp->duty;
}
static int keyboard_led_init_ec_pwm(struct platform_device *pdev)
@@ -186,7 +199,7 @@ static const __maybe_unused struct keyboard_led_drvdata keyboard_led_drvdata_ec_
.init = keyboard_led_init_ec_pwm,
.brightness_set_blocking = keyboard_led_set_brightness_ec_pwm,
.brightness_get = keyboard_led_get_brightness_ec_pwm,
- .max_brightness = KEYBOARD_BACKLIGHT_MAX,
+ .max_brightness = EC_PWM_MAX_DUTY,
};
#else /* IS_ENABLED(CONFIG_CROS_EC) */
--
2.43.0
On Fri, Mar 22, 2024 at 3:34 AM Stephen Horvath
<[email protected]> wrote:
>
> EC_CMD_PWM_GET_KEYBOARD_BACKLIGHT and EC_CMD_PWM_SET_KEYBOARD_BACKLIGHT
> are marked as obsolete in `cros_ec_commands.h`, this patch removes the
> usage of these commands from the driver.
Just because the EC firmware repository marks these as obsolete (and
yes, we copy that header mostly as-is into the kernel repository ...
but it's still a firmware header) doesn't mean it's truly ready to be
removed. I believe the intention is to direct *firmware* developers
not to use them -- any new developments should be using the new
commands.
From a kernel perspective, we could still be supporting old firmware
on old devices, and so we may want/need to continue to support these
commands.
I don't know off the top of my head which firmware branches support
which commands, on devices that have such keyboard backlights. (The
Chromium EC repository is open source though, with various firmware-*
branches still around, so this information is available.) But without
a better explanation as to why these are truly ready to be removed,
I'll say "NAK."
Brian
On 26/3/24 04:45, Brian Norris wrote:
> Just because the EC firmware repository marks these as obsolete (and
> yes, we copy that header mostly as-is into the kernel repository ...
> but it's still a firmware header) doesn't mean it's truly ready to be
> removed. I believe the intention is to direct *firmware* developers
> not to use them -- any new developments should be using the new
> commands.
>
> From a kernel perspective, we could still be supporting old firmware
> on old devices, and so we may want/need to continue to support these
> commands.
Alright that makes sense.
> I don't know off the top of my head which firmware branches support
> which commands, on devices that have such keyboard backlights. (The
> Chromium EC repository is open source though, with various firmware-*
> branches still around, so this information is available.) But without
> a better explanation as to why these are truly ready to be removed,
> I'll say "NAK."
Yeah that's fair enough, my laptop seems to support both so I'll agree
the older commands are probably the safer option.
Thanks a lot for your feedback!
Steve
On Mon, Mar 25, 2024 at 5:30 PM Stephen Horvath
<[email protected]> wrote:
> my laptop seems to support both so I'll agree
> the older commands are probably the safer option.
Huh, it's surprising to me that it supports both, if one was marked
obsolete. But I haven't tracked the development in the EC firmware
that closely recently.
If the old command works, it's probably safe to keep using it. But
it's possible we'll eventually see some device with firmware that
doesn't support the old one, and we'll have to update the kernel
drivers for both. Note that there's an existing driver for the newer
generic PWM command protocol:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pwm/pwm-cros-ec.c?h=v6.8
So far, it's only been used on Device Tree systems as far as I can
tell, and one would probably have to wire it up differently for an
ACPI system. I just mention it as an FYI.
Brian