On Thu 2023-04-13 17:18:04, Hans de Goede wrote:
> From: Yauhen Kharuzhy <jekhor@xxxxxxxxx>
>
> Add support for LEDs connected to the Intel Cherry Trail Whiskey Cove
> PMIC. Charger and general-purpose leds are supported. Hardware blinking
> is implemented, breathing is not.
leds->LEDs.
> diff --git a/drivers/leds/leds-cht-wcove.c b/drivers/leds/leds-cht-wcove.c
> new file mode 100644
> index 000000000000..06447804d050
> --- /dev/null
> +++ b/drivers/leds/leds-cht-wcove.c
> @@ -0,0 +1,368 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for LEDs connected to the Intel Cherry Trail Whiskey Cove PMIC
> + *
> + * Copyright 2019 Yauhen Kharuzhy <jekhor@xxxxxxxxx>
> + * Copyright 2023 Hans de Goede <hansg@xxxxxxxxxx>
> + *
> + * Based on Lenovo Yoga Book Android kernel sources
> + */
"sources." Should copyrights from Android be copied here?
> +static const char * const cht_wc_leds_names[CHT_WC_LED_COUNT] = {
> + "cht-wc::" LED_FUNCTION_CHARGING,
> + "cht-wc::" LED_FUNCTION_INDICATOR,
> +};
No need for "cht-wc". Mention color. At least charging LED is
going to be common - document in Documentation/leds/well-known-leds.txt.
> +static int cht_wc_leds_brightness_set(struct led_classdev *cdev,
> + enum led_brightness value)
> +{
> + struct cht_wc_led *led = container_of(cdev, struct cht_wc_led, cdev);
> + int ret;
> +
> + mutex_lock(&led->mutex);
> +
> + if (!value) {
> + ret = regmap_update_bits(led->regmap, led->regs->ctrl,
> + led->regs->on_off_mask, led->regs->off_val);
> + if (ret)
> + dev_err(cdev->dev, "Failed to turn off: %d\n", ret);
> +
> + /* Disable HW blinking */
> + ret = regmap_update_bits(led->regmap, led->regs->fsm,
> + CHT_WC_LED_EFF_MASK, CHT_WC_LED_EFF_ON);
> + if (ret < 0)
> + dev_err(cdev->dev, "Failed to update LED FSM reg: %d\n", ret);
This overwrites previous error. Not sure if it is big deal.
> +static int cht_wc_leds_blink_set(struct led_classdev *cdev,
> + unsigned long *delay_on,
> + unsigned long *delay_off)
> +{
> + struct cht_wc_led *led = container_of(cdev, struct cht_wc_led, cdev);
> + unsigned int ctrl;
> + int ret;
> +
> + mutex_lock(&led->mutex);
> +
> + if (!*delay_on && !*delay_off)
> + *delay_on = *delay_off = 1000;
That's really slow blink; I'd do something faster by default.
> + /*
> + * LED1 might be in hw-controlled mode when this driver gets loaded; and
> + * since the PMIC is always powered by the battery any changes made are
> + * permanent. Save LED1 regs to restore them on remove() or shutdown().
> + */
Fun :-).
Thank you.
Best regards,
Pavel
--
People of Russia, stop Putin before his war on Ukraine escalates.
Attachment:
signature.asc
Description: PGP signature
