On Sat, 22 Mar 2025, Antheas Kapenekakis wrote:
> The X1 and X1 mini lineups feature an LED nested within their turbo
> button. When turbo takeover is not enabled, the turbo button allows
> the device to switch from 18W to 25W TDP. When the device is in the
> 25W TDP mode, the LED is turned on.
>
> However, when we engage turbo takeover, the turbo led remains on its
> last state, which might be illuminated and cannot be currently
> controlled. Therefore, add the register that controls it under sysfs,
> to allow userspace to turn it off once engaging turbo takeover and
> then control it as they wish.
>
> 2024 OneXPlayer devices, other than the X1s, do not have a turbo LED.
> However, earlier models do, so this can be extended to them as well
> when the register for it is found.
>
> Reviewed-by: Thomas Weißschuh <linux@xxxxxxxxxxxxxx>
> Reviewed-by: Derek J. Clark <derekjohn.clark@xxxxxxxxx>
> Signed-off-by: Antheas Kapenekakis <lkml@xxxxxxxxxxx>
> ---
> drivers/platform/x86/oxpec.c | 84 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 84 insertions(+)
>
> diff --git a/drivers/platform/x86/oxpec.c b/drivers/platform/x86/oxpec.c
> index ee37070ec54fc..384d525402def 100644
> --- a/drivers/platform/x86/oxpec.c
> +++ b/drivers/platform/x86/oxpec.c
> @@ -87,6 +87,12 @@ static enum oxp_board board;
>
> #define OXP_TURBO_RETURN_VAL 0x00 /* Common return val */
>
> +/* X1 Turbo LED */
> +#define OXP_X1_TURBO_LED_REG 0x57
> +
> +#define OXP_X1_TURBO_LED_OFF 0x01
> +#define OXP_X1_TURBO_LED_ON 0x02
> +
> static const struct dmi_system_id dmi_table[] = {
> {
> .matches = {
> @@ -434,6 +440,73 @@ static ssize_t tt_toggle_show(struct device *dev,
>
> static DEVICE_ATTR_RW(tt_toggle);
>
> +/* Callbacks for turbo LED attribute */
> +static umode_t tt_led_is_visible(struct kobject *kobj,
> + struct attribute *attr, int n)
> +{
> + switch (board) {
> + case oxp_x1:
> + return attr->mode;
> + default:
> + break;
> + }
> + return 0;
> +}
> +
> +static ssize_t tt_led_store(struct device *dev,
> + struct device_attribute *attr, const char *buf,
> + size_t count)
> +{
> + u8 reg, val;
> + bool value;
> + int rval;
> +
> + rval = kstrtobool(buf, &value);
> + if (rval)
> + return rval;
> +
> + switch (board) {
> + case oxp_x1:
> + reg = OXP_X1_TURBO_LED_REG;
> + val = value ? OXP_X1_TURBO_LED_ON : OXP_X1_TURBO_LED_OFF;
> + break;
> + default:
> + return -EINVAL;
> + }
> + rval = write_to_ec(reg, val);
> +
> + if (rval)
Please don't put empty lines in between call and its error handling.
Please change "rval" to "ret".
(I'd add also a patch to convert the another rval to ret if it remains and
change "retval" too to ret for consistency).
> + return rval;
> +
> + return count;
> +}
> +
> +static ssize_t tt_led_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + int retval;
> + long enval;
> + long val;
> + u8 reg;
> +
> + switch (board) {
> + case oxp_x1:
> + reg = OXP_X1_TURBO_LED_REG;
> + enval = OXP_X1_TURBO_LED_ON;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + retval = read_from_ec(reg, 1, &val);
> + if (retval)
> + return retval;
> +
> + return sysfs_emit(buf, "%d\n", val == enval);
> +}
> +
> +static DEVICE_ATTR_RW(tt_led);
> +
> /* PWM enable/disable functions */
> static int oxp_pwm_enable(void)
> {
> @@ -691,8 +764,19 @@ static const struct attribute_group oxp_tt_toggle_attribute_group = {
> .attrs = oxp_tt_toggle_attrs,
> };
>
> +static struct attribute *oxp_tt_led_attrs[] = {
> + &dev_attr_tt_led.attr,
> + NULL
> +};
> +
> +static const struct attribute_group oxp_tt_led_attribute_group = {
> + .is_visible = tt_led_is_visible,
> + .attrs = oxp_tt_led_attrs,
> +};
> +
> static const struct attribute_group *oxp_ec_groups[] = {
> &oxp_tt_toggle_attribute_group,
> + &oxp_tt_led_attribute_group,
> NULL
> };
>
>
--
i.
