Hello,
On Sun, Apr 30, 2023 at 06:58:06PM +0200, Hans de Goede wrote:
> On the Android yb1-x90f/l models there is not ACPI method to control
> the keyboard backlight brightness. Instead the second PWM controller
> is exposed directly to the OS there.
>
> Add support for controlling keyboard backlight brightness on the Android
> model by using the PWM subsystem to directly control the PWM.
>
> The Android model also requires explicitly turning the backlight off
> on suspend, which on the Windows model was done automatically.
>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> Changes in v2:
> - Use YB_KBD_BL_PWM_PERIOD define in yogabook_pdev_pwm_lookup[]
> - Turn off keyboard backlight on suspend
> ---
> drivers/platform/x86/lenovo-yogabook-wmi.c | 31 +++++++++++++++++++++-
> 1 file changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/lenovo-yogabook-wmi.c b/drivers/platform/x86/lenovo-yogabook-wmi.c
> index 0183b75a47e8..b49109d91ec3 100644
> --- a/drivers/platform/x86/lenovo-yogabook-wmi.c
> +++ b/drivers/platform/x86/lenovo-yogabook-wmi.c
> @@ -19,6 +19,7 @@
> #include <linux/leds.h>
> #include <linux/module.h>
> #include <linux/platform_device.h>
> +#include <linux/pwm.h>
> #include <linux/wmi.h>
> #include <linux/workqueue.h>
>
> @@ -26,6 +27,7 @@
>
> #define YB_KBD_BL_DEFAULT 128
> #define YB_KBD_BL_MAX 255
> +#define YB_KBD_BL_PWM_PERIOD 13333
>
> #define YB_PDEV_NAME "yogabook-touch-kbd-digitizer-switch"
>
> @@ -48,6 +50,7 @@ struct yogabook_data {
> struct gpio_desc *pen_touch_event;
> struct gpio_desc *kbd_bl_led_enable;
> struct gpio_desc *backside_hall_gpio;
> + struct pwm_device *kbd_bl_pwm;
> int (*set_kbd_backlight)(struct yogabook_data *data, uint8_t level);
> int pen_touch_irq;
> int backside_hall_irq;
> @@ -267,8 +270,11 @@ static int yogabook_suspend(struct device *dev)
> struct yogabook_data *data = dev_get_drvdata(dev);
>
> set_bit(YB_SUSPENDED, &data->flags);
> -
> flush_work(&data->work);
> +
> + if (test_bit(YB_KBD_IS_ON, &data->flags))
> + data->set_kbd_backlight(data, 0);
> +
> return 0;
> }
>
> @@ -424,8 +430,21 @@ static struct gpiod_lookup_table yogabook_pdev_gpios = {
> },
> };
>
> +static struct pwm_lookup yogabook_pdev_pwm_lookup[] = {
> + PWM_LOOKUP_WITH_MODULE("80862289:00", 0, YB_PDEV_NAME, "kbd_bl_pwm",
> + YB_KBD_BL_PWM_PERIOD, PWM_POLARITY_NORMAL,
> + "pwm-lpss-platform"),
> +};
> +
> static int yogabook_pdev_set_kbd_backlight(struct yogabook_data *data, u8 level)
> {
> + struct pwm_state state = {
> + .period = YB_KBD_BL_PWM_PERIOD,
> + .duty_cycle = YB_KBD_BL_PWM_PERIOD * level / YB_KBD_BL_MAX,
> + .enabled = level,
> + };
> +
> + pwm_apply_state(data->kbd_bl_pwm, &state);
> gpiod_set_value(data->kbd_bl_led_enable, level ? 1 : 0);
> return 0;
> }
> @@ -475,6 +494,16 @@ static int yogabook_pdev_probe(struct platform_device *pdev)
> goto error_put_devs;
> }
>
> + pwm_add_table(yogabook_pdev_pwm_lookup, ARRAY_SIZE(yogabook_pdev_pwm_lookup));
> + data->kbd_bl_pwm = devm_pwm_get(dev, "kbd_bl_pwm");
> + pwm_remove_table(yogabook_pdev_pwm_lookup, ARRAY_SIZE(yogabook_pdev_pwm_lookup));
Keeping the table added just long enough to call devm_pwm_get() looks
very creative to me. Why don't you keep the table around while the
device is available? Also note that a given table must only ever be
added once using pwm_add_table(). If there happen to be two yogabook
devices and .probe() is called a second time, the list of tables might
be corrupted.
I don't know much about x86, but I think the table belongs to where this
"80862289:00" device is created.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
