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.

Sign up for GitHub

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

Jump to bottom

worldsemi,ws2812-(gpio|rpi_pico-pio) pin property change into gpios #68514

Merged
merged 3 commits into from
Mar 6, 2024
Merged

worldsemi,ws2812-(gpio|rpi_pico-pio) pin property change into gpios #68514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
af0a407
drivers: led_strip: ws2812_gpio: Rename in-gpios property to gpios
soburi Feb 5, 2024
25e8d00
drivers: led_strip: ws2812_rpi_pico_pio: Change `output-pin` to `gpios`
soburi Feb 5, 2024
20c16e4
samples: drivers: led_ws2812: Rename boards files to follow hwmv2
soburi Mar 5, 2024
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
2 changes: 1 addition & 1 deletion boards/adafruit/kb2040/adafruit_kb2040.dts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@

ws2812: ws2812 {
status = "okay";
output-pin = <17>;
gpios = <&gpio0 17 GPIO_ACTIVE_HIGH>;
chain-length = <1>;
color-mapping = <LED_COLOR_ID_GREEN
LED_COLOR_ID_RED
Expand Down
2 changes: 1 addition & 1 deletion boards/adafruit/qt_py_rp2040/adafruit_qt_py_rp2040.dts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@

ws2812: ws2812 {
status = "okay";
output-pin = <12>;
gpios = <&gpio0 12 GPIO_ACTIVE_HIGH>;
chain-length = <1>;
color-mapping = <LED_COLOR_ID_GREEN
LED_COLOR_ID_RED
Expand Down
6 changes: 6 additions & 0 deletions doc/releases/migration-guide-3.7.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,12 @@ Input
Interrupt Controller
====================

LED Strip
=========

* The property ``in-gpios`` defined in :dtcompatible:`worldsemi,ws2812-gpio` has been
renamed to ``gpios``.

Sensors
=======

Expand Down
10 changes: 5 additions & 5 deletions drivers/led_strip/ws2812_gpio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ LOG_MODULE_REGISTER(ws2812_gpio);
#include <zephyr/dt-bindings/led/led.h>

struct ws2812_gpio_cfg {
struct gpio_dt_spec in_gpio;
struct gpio_dt_spec gpio;
uint8_t num_colors;
const uint8_t *color_mapping;
};
Expand Down Expand Up @@ -91,7 +91,7 @@ static int send_buf(const struct device *dev, uint8_t *buf, size_t len)
{
const struct ws2812_gpio_cfg *config = dev->config;
volatile uint32_t *base = (uint32_t *)&NRF_GPIO->OUTSET;
const uint32_t val = BIT(config->in_gpio.pin);
const uint32_t val = BIT(config->gpio.pin);
struct onoff_manager *mgr =
z_nrf_clock_control_get_onoff(CLOCK_CONTROL_NRF_SUBSYS_HF);
struct onoff_client cli;
Expand Down Expand Up @@ -216,7 +216,7 @@ static const uint8_t ws2812_gpio_##idx##_color_mapping[] = \
const struct ws2812_gpio_cfg *cfg = dev->config; \
uint8_t i; \
\
if (!gpio_is_ready_dt(&cfg->in_gpio)) { \
if (!gpio_is_ready_dt(&cfg->gpio)) { \
LOG_ERR("GPIO device not ready"); \
return -ENODEV; \
} \
Expand All @@ -236,13 +236,13 @@ static const uint8_t ws2812_gpio_##idx##_color_mapping[] = \
} \
} \
\
return gpio_pin_configure_dt(&cfg->in_gpio, GPIO_OUTPUT); \
return gpio_pin_configure_dt(&cfg->gpio, GPIO_OUTPUT); \
} \
\
WS2812_COLOR_MAPPING(idx); \
\
static const struct ws2812_gpio_cfg ws2812_gpio_##idx##_cfg = { \
.in_gpio = GPIO_DT_SPEC_INST_GET(idx, in_gpios), \
.gpio = GPIO_DT_SPEC_INST_GET(idx, gpios), \
.num_colors = WS2812_NUM_COLORS(idx), \
.color_mapping = ws2812_gpio_##idx##_color_mapping, \
}; \
Expand Down
9 changes: 5 additions & 4 deletions drivers/led_strip/ws2812_rpi_pico_pio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

#include <zephyr/drivers/gpio.h>
#include <zephyr/drivers/pinctrl.h>
#include <zephyr/drivers/led_strip.h>
#include <zephyr/drivers/misc/pio_rpi_pico/pio_rpi_pico.h>
Expand All @@ -21,7 +22,7 @@ struct ws2812_led_strip_data {

struct ws2812_led_strip_config {
const struct device *piodev;
uint32_t output_pin;
const uint8_t gpio_pin;
simonguinot marked this conversation as resolved.
Show resolved Hide resolved
uint8_t num_colors;
uint32_t frequency;
const uint8_t *const color_mapping;
Expand Down Expand Up @@ -52,11 +53,11 @@ static int ws2812_led_strip_sm_init(const struct device *dev)
}

sm_config_set_sideset(&sm_config, 1, false, false);
sm_config_set_sideset_pins(&sm_config, config->output_pin);
sm_config_set_sideset_pins(&sm_config, config->gpio_pin);
sm_config_set_out_shift(&sm_config, false, true, (config->num_colors == 4 ? 32 : 24));
sm_config_set_fifo_join(&sm_config, PIO_FIFO_JOIN_TX);
sm_config_set_clkdiv(&sm_config, clkdiv);
pio_sm_set_consecutive_pindirs(pio, sm, config->output_pin, 1, true);
pio_sm_set_consecutive_pindirs(pio, sm, config->gpio_pin, 1, true);
pio_sm_init(pio, sm, -1, &sm_config);
pio_sm_set_enabled(pio, sm, true);

Expand Down Expand Up @@ -187,7 +188,7 @@ static int ws2812_rpi_pico_pio_init(const struct device *dev)
\
static const struct ws2812_led_strip_config ws2812_led_strip_##node##_config = { \
.piodev = DEVICE_DT_GET(DT_PARENT(DT_PARENT(node))), \
.output_pin = DT_PROP(node, output_pin), \
.gpio_pin = DT_GPIO_PIN_BY_IDX(node, gpios, 0), \
.num_colors = DT_PROP_LEN(node, color_mapping), \
.color_mapping = ws2812_led_strip_##node##_color_mapping, \
.reset_delay = DT_PROP(node, reset_delay), \
Expand Down
10 changes: 1 addition & 9 deletions dts/bindings/led_strip/worldsemi,ws2812-gpio.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,4 @@ description: |

compatible: "worldsemi,ws2812-gpio"

include: [base.yaml, ws2812.yaml]

properties:
in-gpios:
type: phandle-array
required: true
description: |
GPIO phandle and specifier for the pin connected to the daisy
chain's input pin. Exactly one pin should be given.
include: [base.yaml, ws2812-gpio.yaml]
10 changes: 4 additions & 6 deletions dts/bindings/led_strip/worldsemi,ws2812-rpi_pico-pio.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,16 @@ child-binding:
Worldsemi WS2812 or compatible LED strip driver based on RaspberryPi Pico's PIO
The LED strip node can put up to 4 instances under a single PIO node.

include: ws2812.yaml
include: ws2812-gpio.yaml

properties:
output-pin:
type: int
required: true
gpios:
description: |
Select the output pin.
Inherited from ws2812-gpio.yaml.

Note: This driver does not configure the output pin.
You need to configure the pin with pinctrl that is in the parent node configuration
for use by PIO.
for use by PIO. This property only uses the GPIO pin number and ignores flags.

frequency:
type: int
Expand Down
15 changes: 15 additions & 0 deletions dts/bindings/led_strip/ws2812-gpio.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Copyright (c) 2024, TOKITA Hiroshi
# SPDX-License-Identifier: Apache-2.0

include: ws2812.yaml

description: |
Common definition GPIO based WS2812 node

properties:
gpios:
type: phandle-array
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this really be a phandle rather than a phandle-array? At least at the quick glance, in order to have a system with multiple LED chains, you already need to have multiple entries in the device tree for each chain. I guess it may not make sense to take in multiple GPIO pins here if the code is only going to use the first one.

Then isn't this compounded by the fact that the PIO code can only operate on a single GPIO pin?

Copy link
Member Author

@soburi soburi Feb 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The gpios name may be treated specially during devicetree processing.
In particular, since it includes processing to link with *-names, even when specifying a single GPIO pin, there are many places where the name is *gpios in the phandle-array type. It also follows this convention.

zephyr/scripts/dts/python-devicetree/src/devicetree/edtlib.py

Lines 1752 to 1757 in 40b53d5

if not specifier_space:
if prop.name.endswith("gpios"):
# There's some slight special-casing for *-gpios properties in that
# e.g. foo-gpios still maps to #gpio-cells rather than
# #foo-gpio-cells
specifier_space = "gpio"

Copy link
Contributor

Choose a reason for hiding this comment

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

Thats good enough for me.

Copy link
Member

Choose a reason for hiding this comment

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

Just to clarify: phandle-array is not for storing an array of phandles (that would be type: phandles), it is for storing a list of phandles and specifiers (e.g. GPIO port, pin, and flags).

Using a phandle-array type here seems correct to me.

See https://docs.zephyrproject.org/latest/build/dts/bindings-syntax.html#type for further information of the various devicetree types.

required: true
description: |
GPIO phandle and specifier for the pin connected to the
led-strip. Exactly one pin should be given.
2 changes: 1 addition & 1 deletion samples/drivers/led_ws2812/boards/bbc_microbit.overlay
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
LED_COLOR_ID_RED
LED_COLOR_ID_BLUE>;
/* P0: */
in-gpios = <&gpio0 3 0>;
gpios = <&gpio0 3 0>;
};

aliases {
Expand Down
0 ...rs/led_ws2812/boards/esp32s3_devkitm.conf → ...oards/esp32s3_devkitm_esp32s3_procpu.conf
View file
Open in desktop
File renamed without changes.
0 ...led_ws2812/boards/esp32s3_devkitm.overlay → ...ds/esp32s3_devkitm_esp32s3_procpu.overlay
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion samples/drivers/led_ws2812/boards/nrf51dk_nrf51822.overlay
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
* Arduino D11 / P0.25, which was chosen to match the pin
* used in nrf52dk_nrf52832.overlay.
*/
in-gpios = <&gpio0 25 0>;
gpios = <&gpio0 25 0>;
};

aliases {
Expand Down
0 .../led_ws2812/boards/nrf5340dk_nrf5340.conf → ...2812/boards/nrf5340dk_nrf5340_cpuapp.conf
View file
Open in desktop
File renamed without changes.
0 ...d_ws2812/boards/nrf5340dk_nrf5340.overlay → ...2/boards/nrf5340dk_nrf5340_cpuapp.overlay
View file
Open in desktop
File renamed without changes.
Loading