Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drivers/ws281x: Add gpio_ll and timer based driver #19891

Merged
merged 8 commits into from
Jan 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions boards/common/e104-bt50xxa-tb/include/board.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@ extern "C" {
#define BTN1_MODE GPIO_IN_PU
/** @} */

/**
* @name WS281x RGB LED configuration
* @{
*/
#define WS281X_TIMER_DEV TIMER_DEV(1) /**< Timer device */
#define WS281X_TIMER_MAX_VALUE TIMER_1_MAX_VALUE /**< Timer max value */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not always use TIMER_DEV(1) for WS281X_TIMER_DEV?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/*
...
 * The default value of 2 is chosen because the only platform on which the
 * module is usable is nRF5x, where TIMER_DEV(1) is in use by the radio module.
 * It is strongly advised to explicitly set this timer to a known free timer,
 * as the default may change without notice.
*/

Hmmm reading that I wonder if there will be a conflict with the e104-bt50xxa-tb boards. But I guess that is only if both modules are used at the same time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test doesn't use the radio module, so it can select TIMER_DEV(1) - and we don't have to clutter the board definitions with WS281X_TIMER_DEV when there is no ws281x LED connected.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test doesn't use the radio module, so it can select TIMER_DEV(1)

I still think it is better try not to conflict with a known value. If we add a radio and ws281x test it would fail. This is maybe a decision best left to @chrysn though...

/** @} */

#ifdef __cplusplus
}
#endif
Expand Down
5 changes: 5 additions & 0 deletions boards/common/nrf51/include/cfg_timer_01.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ static const timer_conf_t timer_config[] = {
#define TIMER_0_ISR isr_timer0
#define TIMER_1_ISR isr_timer1

/** See @ref timer_init */
#define TIMER_0_MAX_VALUE 0xffffffff
/** See @ref timer_init */
#define TIMER_1_MAX_VALUE 0xffffffff

#define TIMER_NUMOF ARRAY_SIZE(timer_config)
/** @} */

Expand Down
7 changes: 7 additions & 0 deletions boards/common/nrf51/include/cfg_timer_012.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,13 @@ static const timer_conf_t timer_config[] = {
#define TIMER_1_ISR isr_timer1
#define TIMER_2_ISR isr_timer2

/** See @ref timer_init */
#define TIMER_0_MAX_VALUE 0xffffffff
/** See @ref timer_init */
#define TIMER_1_MAX_VALUE 0xffffffff
/** See @ref timer_init */
#define TIMER_2_MAX_VALUE 0xffffffff

#define TIMER_NUMOF ARRAY_SIZE(timer_config)
/** @} */

Expand Down
15 changes: 15 additions & 0 deletions boards/common/nrf52/include/cfg_timer_default.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,21 @@ static const timer_conf_t timer_config[] = {
#define TIMER_2_ISR isr_timer3
#define TIMER_3_ISR isr_timer4

/** See @ref timer_init */
#define TIMER_0_MAX_VALUE 0xffffffff
/** See @ref timer_init */
#define TIMER_1_MAX_VALUE 0xffffffff
#ifdef NRF_TIMER3
/** See @ref timer_init */
#define TIMER_2_MAX_VALUE 0xffffffff
#endif
/* If there is no NRF_TIMER3 this should be TIMER_2 because the index shifts
* up, but there is only a TIMER4 if there is a TIMER3 too. */
#ifdef NRF_TIMER4
/** See @ref timer_init */
#define TIMER_3_MAX_VALUE 0xffffffff
#endif

#define TIMER_NUMOF ARRAY_SIZE(timer_config)
/** @} */

Expand Down
8 changes: 8 additions & 0 deletions boards/nrf9160dk/include/board.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,14 @@ extern "C" {
#define BTN3_MODE GPIO_IN /**< BTN3 default mode */
/** @} */

/**
* @name WS281x RGB LED configuration
* @{
*/
#define WS281X_TIMER_DEV TIMER_DEV(1) /**< Timer device */
#define WS281X_TIMER_MAX_VALUE TIMER_1_MAX_VALUE /**< Timer max value */
/** @} */

#ifdef __cplusplus
}
#endif
Expand Down
5 changes: 5 additions & 0 deletions boards/nrf9160dk/include/periph_conf.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ static const timer_conf_t timer_config[] = {
#define TIMER_0_ISR isr_timer0 /**< Timer0 IRQ*/
#define TIMER_1_ISR isr_timer1 /**< Timer1 IRQ */

/** See @ref timer_init */
#define TIMER_0_MAX_VALUE 0xffffffff
/** See @ref timer_init */
#define TIMER_1_MAX_VALUE 0xffffffff

#define TIMER_NUMOF ARRAY_SIZE(timer_config) /**< Timer configuration NUMOF */
/** @} */

Expand Down
1 change: 1 addition & 0 deletions cpu/nrf53/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ config CPU_FAM_NRF53
select HAS_PERIPH_GPIO
select HAS_PERIPH_GPIO_IRQ
select HAS_PERIPH_TIMER_PERIODIC
select HAS_PERIPH_TIMER_POLL
select HAS_PERIPH_TIMER_QUERY_FREQS
select HAS_PERIPH_UART_MODECFG
select HAS_PERIPH_WDT
Expand Down
1 change: 1 addition & 0 deletions cpu/nrf5x_common/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ depends on !CPU_FAM_NRF53
select HAS_PERIPH_HWRNG
select HAS_PERIPH_TEMPERATURE
select HAS_PERIPH_TIMER_PERIODIC
select HAS_PERIPH_TIMER_POLL
select HAS_PERIPH_TIMER_QUERY_FREQS
select HAS_PERIPH_RTT_OVERFLOW
select HAS_PERIPH_UART_MODECFG
Expand Down
1 change: 1 addition & 0 deletions cpu/nrf5x_common/Makefile.features
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ FEATURES_PROVIDED += periph_flashpage_in_address_space
FEATURES_PROVIDED += periph_flashpage_pagewise
FEATURES_PROVIDED += periph_gpio periph_gpio_irq
FEATURES_PROVIDED += periph_timer_periodic
FEATURES_PROVIDED += periph_timer_poll
FEATURES_PROVIDED += periph_timer_query_freqs
FEATURES_PROVIDED += periph_uart_modecfg
FEATURES_PROVIDED += periph_wdt periph_wdt_cb
Expand Down
42 changes: 42 additions & 0 deletions cpu/nrf5x_common/include/timer_arch.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Copyright (C) 2015 Jan Wagner <[email protected]>
* 2015-2016 Freie Universität Berlin
* 2019 Inria
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

/**
* @ingroup cpu_nrf5x_common
* @ingroup drivers_periph_timer
* @{
*
* @file
* @brief CPU specific part of the timer API
*
* @author Christian Amsüss <[email protected]>
*/

#ifndef TIMER_ARCH_H
#define TIMER_ARCH_H

#ifdef __cplusplus
extern "C" {
#endif

#ifndef DOXYGEN /* hide implementation specific details from Doxygen */

static inline bool timer_poll_channel(tim_t tim, int channel)
{
return timer_config[tim].dev->EVENTS_COMPARE[channel];
}

#endif /* DOXYGEN */
#ifdef __cplusplus
}
#endif

#endif /* TIMER_ARCH_H */
/** @} */
4 changes: 3 additions & 1 deletion cpu/nrf5x_common/periph/timer.c
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,9 @@ int timer_init(tim_t tim, uint32_t freq, timer_cb_t cb, void *arg)
}

/* enable interrupts */
NVIC_EnableIRQ(timer_config[tim].irqn);
if (cb != NULL) {
NVIC_EnableIRQ(timer_config[tim].irqn);
}
/* start the timer */
dev(tim)->TASKS_START = 1;

Expand Down
1 change: 1 addition & 0 deletions cpu/nrf9160/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ config CPU_FAM_NRF9160
select HAS_PERIPH_GPIO_LL_IRQ
select HAS_PERIPH_GPIO_LL_IRQ_UNMASK
select HAS_PERIPH_TIMER_PERIODIC
select HAS_PERIPH_TIMER_POLL
select HAS_PERIPH_TIMER_QUERY_FREQS
select HAS_PERIPH_UART_MODECFG
select HAS_PERIPH_SPI_GPIO_MODE
Expand Down
34 changes: 34 additions & 0 deletions drivers/include/periph/timer.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@

#include <limits.h>
#include <stdint.h>
#include <stdbool.h>

#include "architecture.h"
#include "periph_cpu.h"
Expand Down Expand Up @@ -295,6 +296,39 @@ uword_t timer_query_channel_numof(tim_t dev);
*/
uint32_t timer_query_freqs(tim_t dev, uword_t index);

#if defined(DOXYGEN)
/**
* @brief Check whether a compare channel has matched
*
* @return true once after the channel has matched.
*
* It is currently not defined whether this keeps returning true after a
* channel has been polled until that channel is set, or whether later calls
* return false.
*
* This is typically used in spin loops that wait for a timer's completion:
*
* ~~~
* while (!timer_poll_channel(tim, chan)) {};
* ~~~
*
* This function is only available on platforms that implement the
* `periph_timer_poll` peripheral in addition to `periph_timer`.
*
*/
/* As this function is polled, it needs to be inlined, so it is typically
* provided through timer_arch.h. If a platform ever does not need to go
* through static inline here, this declaration's condition can be extended to
* be `(defined(MODULE_PERIPH_TIMER_POLL) &&
* !defined(PERIPH_TIMER_PROVIDES_INLINE_POLL_CHANNEL) || defined(DOXYGEN)` or
* similar. */
bool timer_poll_channel(tim_t dev, int channel);
#endif

#if defined(MODULE_PERIPH_TIMER_POLL)
#include "timer_arch.h"
#endif

#ifdef __cplusplus
}
#endif
Expand Down
8 changes: 8 additions & 0 deletions drivers/periph_common/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ rsource "Kconfig.flashpage"

rsource "Kconfig.gpio"

rsource "Kconfig.gpio_ll"

config MODULE_PERIPH_HWRNG
bool "HWRNG peripheral driver"
depends on HAS_PERIPH_HWRNG
Expand Down Expand Up @@ -157,6 +159,12 @@ config MODULE_PERIPH_INIT_RTT
default y if MODULE_PERIPH_INIT
depends on MODULE_PERIPH_RTT

config MODULE_PERIPH_TIMER_POLL
bool "Timer poll"
depends on HAS_PERIPH_TIMER_POLL
help
Enables the timer_poll_channel function.

rsource "Kconfig.sdmmc"
rsource "Kconfig.spi"

Expand Down
22 changes: 22 additions & 0 deletions drivers/periph_common/Kconfig.gpio_ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Copyright (c) 2023 HAW Hamburg
#
# This file is subject to the terms and conditions of the GNU Lesser
# General Public License v2.1. See the file LICENSE in the top level
# directory for more details.
#

menuconfig MODULE_PERIPH_GPIO_LL
bool "Low-level GPIO peripheral driver"
depends on HAS_PERIPH_GPIO_LL

if MODULE_PERIPH_GPIO_LL

config MODULE_PERIPH_GPIO_LL_IRQ_UNMASK
bool "Unmask GPIO peripheral interrupts"
default y
depends on HAS_PERIPH_GPIO_LL_IRQ_UNMASK
help
Enables GPIO peripheral unmasking interrupts without
clearing pending IRQs that came in while masked.

endif # MODULE_PERIPH_GPIO_LL
13 changes: 12 additions & 1 deletion drivers/ws281x/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@

config MODULE_WS281X
bool "WS2812/SK6812 RGB LED (NeoPixel)"
depends on HAS_CPU_CORE_ATMEGA || HAS_ARCH_ESP32 || HAS_ARCH_NATIVE
depends on HAS_CPU_CORE_ATMEGA || HAS_ARCH_ESP32 || HAS_ARCH_NATIVE || HAS_PERIPH_TIMER_POLL
depends on TEST_KCONFIG
select MODULE_XTIMER
select MODULE_WS281X_ATMEGA if HAS_CPU_CORE_ATMEGA
select MODULE_WS281X_VT100 if HAS_ARCH_NATIVE
select MODULE_WS281X_ESP32 if HAS_ARCH_ESP32
select MODULE_WS281X_TIMER_GPIO_LL if HAS_PERIPH_TIMER_POLL

config MODULE_WS281X_ATMEGA
bool
Expand Down Expand Up @@ -45,6 +46,16 @@ config MODULE_WS281X_ESP32_SW
Use the bit-banging software implementation to generate the RGB LED
signal.

config MODULE_WS281X_TIMER_GPIO_LL
bool
depends on HAS_PERIPH_TIMER_POLL
select MODULE_PERIPH_TIMER_POLL
depends on HAS_PERIPH_GPIO_LL
select MODULE_PERIPH_GPIO_LL
help
Use a platform independent bit-banging software implementation to
generate the RGB LED signal.

config HAVE_WS281X
bool
help
Expand Down
11 changes: 10 additions & 1 deletion drivers/ws281x/Makefile.dep
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
FEATURES_REQUIRED_ANY += cpu_core_atmega|arch_esp32|arch_native
# Actually |(periph_timer_poll and periph_gpio_ll), but that's too complex for FEATURES_REQUIRED_ANY to express
FEATURES_REQUIRED_ANY += cpu_core_atmega|arch_esp32|arch_native|periph_timer_poll

ifeq (,$(filter ws281x_%,$(USEMODULE)))
ifneq (,$(filter cpu_core_atmega,$(FEATURES_USED)))
Expand All @@ -10,6 +11,10 @@ ifeq (,$(filter ws281x_%,$(USEMODULE)))
ifneq (,$(filter arch_esp32,$(FEATURES_USED)))
USEMODULE += ws281x_esp32
endif
# Not only looking for the used feature but also for the absence of any more specific driver
ifeq (-periph_timer_poll,$(filter ws281x_%,$(USEMODULE))-$(filter periph_timer_poll,$(FEATURES_USED)))
USEMODULE += ws281x_timer_gpio_ll
endif
endif

ifneq (,$(filter ws281x_atmega,$(USEMODULE)))
Expand All @@ -28,3 +33,7 @@ ifneq (,$(filter ws281x_esp32%,$(USEMODULE)))
USEMODULE += ws281x_esp32_hw
endif
endif

ifneq (,$(filter ws281x_timer_gpio_ll,$(USEMODULE)))
FEATURES_REQUIRED += periph_gpio_ll periph_timer periph_timer_poll
endif
9 changes: 9 additions & 0 deletions drivers/ws281x/include/ws281x_backend.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,15 @@ extern "C" {
#endif
/** @} */

/**
* @name Properties of the timer_gpio_ll backend.
* @{
*/
#ifdef MODULE_WS281X_TIMER_GPIO_LL
#define WS281X_HAVE_INIT (1)
#endif
/** @} */

#ifdef __cplusplus
}
#endif
Expand Down
41 changes: 41 additions & 0 deletions drivers/ws281x/include/ws281x_params.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
* @{
*/
#ifndef WS281X_PARAM_PIN
#define WS281X_PARAM_PIN (GPIO_PIN(0, 0)) /**< GPIO pin connected to the data pin of the first LED */

Check warning on line 34 in drivers/ws281x/include/ws281x_params.h

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters
#endif
#ifndef WS281X_PARAM_NUMOF
#define WS281X_PARAM_NUMOF (8U) /**< Number of LEDs chained */
Expand Down Expand Up @@ -64,6 +64,47 @@
WS281X_PARAMS
};

/** @brief Timer used for WS281x (by the timer_gpio_ll implementation)
*
* A single timer is configured for any number of WS281x strands, so this does
* not need to be part of params.
*
* It is required that the timer has at least 2 channels. (Future versions may
* require a 3rd channel).
*
* It is required that the timer's MAX_VALUE is 2^n-1, which is a trivial but
* not explicitly stated case.
*
* This timer is configured at WS281x initialization time, and kept stopped
* outside of transmissions.
*
* The default value of 2 is chosen because the only platform on which the
* module is usable is nRF5x, where TIMER_DEV(1) is in use by the radio module.
* It is strongly advised to explicitly set this timer to a known free timer,
* as the default may change without notice.
* */
#if !defined(WS281X_TIMER_DEV) || defined(DOXYGEN)
#define WS281X_TIMER_DEV TIMER_DEV(2)
MrKevinWeiss marked this conversation as resolved.
Show resolved Hide resolved
#endif

/** @brief Maximum value of the timer used for WS281x (by the timer_gpio_ll implementation)
*
* This macro needs to be defined to the `TIMER_x_MAX_VALUE` corresponding to
* the `TIMER_DEV(x)` in @ref WS281X_TIMER_DEV.
* */
#ifndef WS281X_TIMER_MAX_VALUE
#define WS281X_TIMER_MAX_VALUE TIMER_2_MAX_VALUE
MrKevinWeiss marked this conversation as resolved.
Show resolved Hide resolved
#endif

/** @brief Frequency for the timer used for WS281x (by the timer_gpio_ll implementation)
*
* This should be set to a frequency that is a close multiple of 3MHz,
* depending on the precise low and high times. A value of 16MHz works well.
* */
#ifndef WS281X_TIMER_FREQ
#define WS281X_TIMER_FREQ 16000000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#define WS281X_TIMER_FREQ 16000000
#define WS281X_TIMER_FREQ MHZ(16)

reads a bit nicer

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like using the MHZ macro causes some conflicts with the ESP so I think it best to just leave it out.

#endif

#ifdef __cplusplus
}
#endif
Expand Down
Loading
Loading