[PATCH v5 1/2] led: Implement software led blinking
Mikhail Kshevetskiy
mikhail.kshevetskiy at genexis.eu
Sat Jul 13 18:28:33 CEST 2024
On 13.07.2024 18:13, Simon Glass wrote:
> Hi Mikhail,
>
> On Fri, 12 Jul 2024 at 06:25, Mikhail Kshevetskiy
> <mikhail.kshevetskiy at iopsys.eu> wrote:
>> From: Michael Polyntsov <michael.polyntsov at iopsys.eu>
>>
>> If hardware (or driver) doesn't support leds blinking, it's
>> now possible to use software implementation of blinking instead.
>> This relies on cyclic functions.
>>
>> Signed-off-by: Michael Polyntsov <michael.polyntsov at iopsys.eu>
>> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy at iopsys.eu>
>> ---
>> drivers/led/Kconfig | 14 +++++
>> drivers/led/Makefile | 1 +
>> drivers/led/led-uclass.c | 19 +++++--
>> drivers/led/led_sw_blink.c | 106 +++++++++++++++++++++++++++++++++++++
>> include/led.h | 17 ++++++
>> 5 files changed, 154 insertions(+), 3 deletions(-)
>> create mode 100644 drivers/led/led_sw_blink.c
>>
>> diff --git a/drivers/led/Kconfig b/drivers/led/Kconfig
>> index 9837960198d..dc9d4c8a757 100644
>> --- a/drivers/led/Kconfig
>> +++ b/drivers/led/Kconfig
>> @@ -73,6 +73,20 @@ config LED_BLINK
>> This option enables support for this which adds slightly to the
>> code size.
>>
>> +config LED_SW_BLINK
>> + bool "Support software LED blinking"
>> + depends on LED_BLINK
>> + select CYCLIC
>> + help
>> + Turns on led blinking implemented in the software, useful when
>> + the hardware doesn't support led blinking. Half of the period
>> + led will be ON and the rest time it will be OFF. Standard
>> + led commands can be used to configure blinking. Does nothing
>> + if driver supports blinking.
>> + WARNING: Blinking may be inaccurate during execution of time
>> + consuming commands (ex. flash reading). Also it completely
>> + stops during OS booting.
>> +
>> config SPL_LED
>> bool "Enable LED support in SPL"
>> depends on SPL_DM
>> diff --git a/drivers/led/Makefile b/drivers/led/Makefile
>> index 2bcb8589087..e27aa488482 100644
>> --- a/drivers/led/Makefile
>> +++ b/drivers/led/Makefile
>> @@ -4,6 +4,7 @@
>> # Written by Simon Glass <sjg at chromium.org>
>>
>> obj-y += led-uclass.o
>> +obj-$(CONFIG_LED_SW_BLINK) += led_sw_blink.o
>> obj-$(CONFIG_LED_BCM6328) += led_bcm6328.o
>> obj-$(CONFIG_LED_BCM6358) += led_bcm6358.o
>> obj-$(CONFIG_LED_BCM6753) += led_bcm6753.o
>> diff --git a/drivers/led/led-uclass.c b/drivers/led/led-uclass.c
>> index f37bf6a1550..4f5dd66765e 100644
>> --- a/drivers/led/led-uclass.c
>> +++ b/drivers/led/led-uclass.c
>> @@ -58,6 +58,10 @@ int led_set_state(struct udevice *dev, enum led_state_t state)
>> if (!ops->set_state)
>> return -ENOSYS;
>>
>> + if (IS_ENABLED(CONFIG_LED_SW_BLINK) &&
>> + led_sw_on_state_change(dev, state))
>> + return 0;
>> +
>> return ops->set_state(dev, state);
>> }
>>
>> @@ -68,6 +72,12 @@ enum led_state_t led_get_state(struct udevice *dev)
>> if (!ops->get_state)
>> return -ENOSYS;
>>
>> +#ifdef CONFIG_LED_BLINK
> It looks like you can use if (IS_ENABLED...) for that ^
No, LEDST_BLINK depends on CONFIG_LED_BLINK. I can define LEDST_BLINK
unconditionally, but this will produce an issue in the other place
>
>> + if (IS_ENABLED(CONFIG_LED_SW_BLINK) &&
>> + led_sw_is_blinking(dev))
>> + return LEDST_BLINK;
>> +#endif
>> +
>> return ops->get_state(dev);
>> }
>>
>> @@ -76,10 +86,13 @@ int led_set_period(struct udevice *dev, int period_ms)
>> {
>> struct led_ops *ops = led_get_ops(dev);
>>
>> - if (!ops->set_period)
>> - return -ENOSYS;
>> + if (ops->set_period)
>> + return ops->set_period(dev, period_ms);
>> +
>> + if (IS_ENABLED(CONFIG_LED_SW_BLINK))
>> + return led_sw_set_period(dev, period_ms);
>>
>> - return ops->set_period(dev, period_ms);
>> + return -ENOSYS;
>> }
>> #endif
>>
>> diff --git a/drivers/led/led_sw_blink.c b/drivers/led/led_sw_blink.c
>> new file mode 100644
>> index 00000000000..ab56111a60b
>> --- /dev/null
>> +++ b/drivers/led/led_sw_blink.c
>> @@ -0,0 +1,106 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Software blinking helpers
>> + * Copyright (C) 2024 IOPSYS Software Solutions AB
>> + * Author: Mikhail Kshevetskiy <mikhail.kshevetskiy at iopsys.eu>
>> + */
>> +
>> +#include <dm.h>
>> +#include <led.h>
>> +#include <time.h>
>> +#include <stdlib.h>
>> +
>> +static void led_sw_blink(struct cyclic_info *c)
>> +{
>> + struct led_uc_plat *uc_plat;
>> + struct udevice *dev;
>> + struct led_ops *ops;
>> +
>> + uc_plat = container_of(c, struct led_uc_plat, cyclic);
>> + dev = uc_plat->dev;
>> + ops = led_get_ops(dev);
>> +
>> + switch (uc_plat->sw_blink_state) {
>> + case LED_SW_BLINK_ST_OFF:
>> + uc_plat->sw_blink_state = LED_SW_BLINK_ST_ON;
>> + ops->set_state(dev, LEDST_ON);
>> + break;
>> + case LED_SW_BLINK_ST_ON:
>> + uc_plat->sw_blink_state = LED_SW_BLINK_ST_OFF;
>> + ops->set_state(dev, LEDST_OFF);
>> + break;
>> + case LED_SW_BLINK_ST_NOT_READY:
>> + /*
>> + * led_set_period has been called, but
>> + * led_set_state(LDST_BLINK) has not yet,
>> + * so doing nothing
>> + */
>> + break;
>> + default:
>> + break;
>> + }
>> +}
>> +
>> +int led_sw_set_period(struct udevice *dev, int period_ms)
>> +{
>> + struct led_uc_plat *uc_plat = dev_get_uclass_plat(dev);
>> + struct cyclic_info *cyclic = &uc_plat->cyclic;
>> + struct led_ops *ops = led_get_ops(dev);
>> + int half_period_us;
>> + char *name;
>> + int len;
>> +
>> + half_period_us = period_ms * 1000 / 2;
>> +
>> + name = (char *)cyclic->name;
>> + if (name == NULL) {
> if (!name)
>
>> + len = snprintf(NULL, 0, "led_sw_blink_%s", uc_plat->label);
>> + if (len <= 0)
>> + return -ENOMEM;
>> +
>> + name = malloc(len + 1);
>> + if (!name)
>> + return -ENOMEM;
>> +
>> + snprintf(name, len + 1, "led_sw_blink_%s", uc_plat->label);
> I think it would be OK to have a fixed-length name (say 20-30 chars)
> and use snprintf()...would reduce the code size.
ok, will fix
>
>> + }
>> +
>> + if (uc_plat->sw_blink_state == LED_SW_BLINK_ST_DISABLED) {
>> + uc_plat->dev = dev;
>> + cyclic_register(cyclic, led_sw_blink, half_period_us, name);
>> + } else {
>> + cyclic->delay_us = half_period_us;
>> + cyclic->start_time_us = timer_get_us();
>> + }
>> +
>> + uc_plat->sw_blink_state = LED_SW_BLINK_ST_NOT_READY;
>> + ops->set_state(dev, LEDST_OFF);
>> +
>> + return 0;
>> +}
>> +
>> +bool led_sw_is_blinking(struct udevice *dev)
>> +{
>> + struct led_uc_plat *uc_plat = dev_get_uclass_plat(dev);
>> +
>> + return (uc_plat->sw_blink_state > LED_SW_BLINK_ST_NOT_READY);
> drop ()
will fix
>
>> +}
>> +
>> +bool led_sw_on_state_change(struct udevice *dev, enum led_state_t state)
>> +{
>> + struct led_uc_plat *uc_plat = dev_get_uclass_plat(dev);
>> +
>> + if (uc_plat->sw_blink_state != LED_SW_BLINK_ST_DISABLED) {
>> + if (state == LEDST_BLINK) {
>> + /* start blinking on next led_sw_blink() call */
>> + uc_plat->sw_blink_state = LED_SW_BLINK_ST_OFF;
>> + return true;
>> + }
>> +
>> + /* stop blinking */
>> + cyclic_unregister(&uc_plat->cyclic);
>> + uc_plat->sw_blink_state = LED_SW_BLINK_ST_DISABLED;
>> + }
>> +
>> + return false;
>> +}
>> diff --git a/include/led.h b/include/led.h
>> index a6353166289..26955269d3e 100644
>> --- a/include/led.h
>> +++ b/include/led.h
>> @@ -20,6 +20,13 @@ enum led_state_t {
>> LEDST_COUNT,
>> };
>>
>> +enum led_sw_blink_state_t {
>> + LED_SW_BLINK_ST_DISABLED,
>> + LED_SW_BLINK_ST_NOT_READY,
>> + LED_SW_BLINK_ST_OFF,
>> + LED_SW_BLINK_ST_ON,
>> +};
>> +
>> /**
>> * struct led_uc_plat - Platform data the uclass stores about each device
>> *
>> @@ -29,6 +36,11 @@ enum led_state_t {
>> struct led_uc_plat {
>> const char *label;
>> enum led_state_t default_state;
>> +#ifdef CONFIG_LED_SW_BLINK
>> + struct udevice *dev;
>> + struct cyclic_info cyclic;
>> + enum led_sw_blink_state_t sw_blink_state;
>> +#endif
>> };
>>
>> /**
>> @@ -118,4 +130,9 @@ int led_set_period(struct udevice *dev, int period_ms);
>> */
>> int led_bind_generic(struct udevice *parent, const char *driver_name);
>>
>> +/* Internal functions for software blinking. Do not use them in your code */
>> +int led_sw_set_period(struct udevice *dev, int period_ms);
>> +bool led_sw_is_blinking(struct udevice *dev);
>> +bool led_sw_on_state_change(struct udevice *dev, enum led_state_t state);
>> +
>> #endif
>> --
>> 2.39.2
>>
> Regards,
> Simon
More information about the U-Boot
mailing list