Skip to content
Closed
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
1 change: 1 addition & 0 deletions drivers/led/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
zephyr_sources_ifdef(CONFIG_LED_GPIO leds_gpio.c)
zephyr_sources_ifdef(CONFIG_LP3943 lp3943.c)
zephyr_sources_ifdef(CONFIG_LP5562 lp5562.c)
zephyr_sources_ifdef(CONFIG_PCA9633 pca9633.c)
Expand Down
1 change: 1 addition & 0 deletions drivers/led/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ config LED_INIT_PRIORITY
help
System initialization priority for LED drivers.

source "drivers/led/Kconfig.gpio"
source "drivers/led/Kconfig.lp3943"
source "drivers/led/Kconfig.lp5562"
source "drivers/led/Kconfig.pca9633"
Expand Down
27 changes: 27 additions & 0 deletions drivers/led/Kconfig.gpio
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#
# Copyright (c) 2018 Manivannan Sadhasivam
#
# SPDX-License-Identifier: Apache-2.0
#

config LED_GPIO
bool "LED GPIO driver"
depends on GPIO
help
Enable software driven LED using GPIO pin.

# ---------- LED 0 ----------

config LED_GPIO_0
bool "Enable LED GPIO 0"
depends on LED_GPIO
help
Enable LED GPIO 0.

# ---------- LED 1 ----------

config LED_GPIO_1
bool "Enable LED GPIO 1"
depends on LED_GPIO
help
Enable LED GPIO 1.
2 changes: 1 addition & 1 deletion drivers/led/Kconfig.lp3943
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ menuconfig LP3943
states at a specified rate. Each channel can drive up to 25 mA
per LED.

if !HAS_DTS_I2C
if I2C && !HAS_DTS_I2C

config LP3943_DEV_NAME
string "LP3943 device name"
Expand Down
139 changes: 139 additions & 0 deletions drivers/led/leds_gpio.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
/*
* Copyright (c) 2018 Manivannan Sadhasivam
*
* SPDX-License-Identifier: Apache-2.0
*/

/**
* @file
* @brief LEDs driver for GPIOs
*
* This driver controls LED devices attached to GPIO pins which
* are declared under leds node in board devicetree. The driver
* takes care of configuring the pin to output state before using it.
*
* TODO: Implement Software Blinking
*/

#include <device.h>
#include <gpio.h>
#include <led.h>
#include <misc/util.h>
#include <zephyr.h>

#define SYS_LOG_LEVEL CONFIG_SYS_LOG_LED_LEVEL
#include <logging/sys_log.h>

#include "led_context.h"

enum led_gpio_modes {
LED_OFF,
LED_ON,
};

struct led_gpio_cfg {
char *gpio_port;
u8_t gpio_pin;
u8_t gpio_polarity;
};

struct led_gpio_data {
struct device *gpio;
struct led_data dev_data;
};

static inline int led_gpio_on(struct device *dev, u32_t led)
Copy link
Member

Choose a reason for hiding this comment

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

Is 'led' argument actually required ?
We should be able to retrieve it from *dev.
And I think this is the interest of the API, so user doesn't care of the PIN

Copy link
Member Author

Choose a reason for hiding this comment

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

led argument is there only for the purpose of LED API and yes, we can retrieve it from *dev. Anyway, passing the led argument gives us the possibility of checking the correct pin in led_on/led_off as below:

        /* Make sure that the LED is valid */
        if (led != cfg->gpio_pin)
                return -EINVAL;

Will include the above check.

Copy link
Member

Choose a reason for hiding this comment

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

@Mani-Sadhasivam ,

Anyway, passing the led argument gives us the possibility of checking the correct pin in led_on/led_off

In the other hand not having 'led' as arguments makes this kind of issue impossible.
I still prefer this is removed from the API.

Copy link
Member Author

Choose a reason for hiding this comment

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

In the other hand not having 'led' as arguments makes this kind of issue impossible.
I still prefer this is removed from the API.

@erwango No. This only holds true for leds-gpio driver, for other drivers there is no GPIO information in cfg.

Copy link
Member

Choose a reason for hiding this comment

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

I agree the argument should stay and the driver should check it.

Copy link
Member

Choose a reason for hiding this comment

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

Can you check following PR to see if it wouldn't provide similar service and spare the argument?
#12056

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see how #12056 relates. I also agree that since this is being done as an implementation for an API that is designed to support multiple LEDs the argument must stay. However, see review summary.

{
const struct led_gpio_cfg *cfg = dev->config->config_info;
struct led_gpio_data *data = dev->driver_data;

/* Make sure that the LED is valid */
if (led != cfg->gpio_pin) {
return -EINVAL;
}

/*
* Check for active high or active low defined in DT. For
* simplicity, non zero cases are considered as active high
* and zeroes are active low.
*/
gpio_pin_write(data->gpio, led,
(cfg->gpio_polarity & GPIO_INT_ACTIVE_HIGH) != 0);

return 0;
}

static inline int led_gpio_off(struct device *dev, u32_t led)
{
const struct led_gpio_cfg *cfg = dev->config->config_info;
struct led_gpio_data *data = dev->driver_data;

/* Make sure that the LED is valid */
if (led != cfg->gpio_pin) {
return -EINVAL;
}

/*
* Check for active high or active low defined in DT. For
* simplicity, non zero cases are considered as active high
* and zeroes are active low.
*/
gpio_pin_write(data->gpio, led,
(cfg->gpio_polarity & GPIO_INT_ACTIVE_HIGH) == 0);

return 0;
}

static int led_gpio_init(struct device *dev)
{
const struct led_gpio_cfg *cfg = dev->config->config_info;
struct led_gpio_data *data = dev->driver_data;
struct led_data *dev_data = &data->dev_data;

data->gpio = device_get_binding(cfg->gpio_port);
if (!data->gpio) {
SYS_LOG_DBG("Failed to get GPIO device");
return -EINVAL;
}

/* Set LED pin as output */
gpio_pin_configure(data->gpio, cfg->gpio_pin, GPIO_DIR_OUT);

/* Blinking and Brightness are not supported yet */
dev_data->min_period = 0;
dev_data->max_period = 0;
dev_data->min_brightness = 0;
dev_data->max_brightness = 0;

return 0;
}

static const struct led_driver_api led_gpio_api = {
.on = led_gpio_on,
.off = led_gpio_off,
};

#define DEFINE_LED_GPIO(_num) \
\
static struct led_gpio_data led_gpio_data_##_num; \
\
static const struct led_gpio_cfg led_gpio_cfg_##_num = { \
.gpio_port = LED##_num##_GPIO_CONTROLLER, \
.gpio_pin = LED##_num##_GPIO_PIN, \
.gpio_polarity = LED##_num##_GPIO_FLAGS, \
}; \
\
DEVICE_AND_API_INIT(led_gpio_##_num, LED##_num##_LABEL, \
led_gpio_init, \
&led_gpio_data_##_num, \
&led_gpio_cfg_##_num, \
POST_KERNEL, CONFIG_LED_INIT_PRIORITY, \
&led_gpio_api)

#ifdef CONFIG_LED_GPIO_0
DEFINE_LED_GPIO(0);
#endif

#ifdef CONFIG_LED_GPIO_1
DEFINE_LED_GPIO(1);
#endif
5 changes: 2 additions & 3 deletions samples/basic/blinky/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ Blinky Application
Overview
********

The Blinky example shows how to configure GPIO pins as outputs which can also be
used to drive LEDs on the hardware usually delivered as "User LEDs" on many of
the supported boards in Zephyr.
The Blinky example shows how to drive LEDs on the hardware usually
delivered as "User LEDs" on many of the supported boards in Zephyr.

Requirements
************
Expand Down
3 changes: 3 additions & 0 deletions samples/basic/blinky/prj.conf
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
CONFIG_GPIO=y
CONFIG_SERIAL=n
CONFIG_LED=y
CONFIG_LED_GPIO=y
CONFIG_LED_GPIO_0=y
17 changes: 8 additions & 9 deletions samples/basic/blinky/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,26 @@

#include <zephyr.h>
#include <device.h>
#include <gpio.h>
#include <led.h>

#define LED_PORT LED0_GPIO_CONTROLLER
#define LED LED0_GPIO_PIN
#define LED_DEV_NAME LED0_LABEL
#define LED LED0_GPIO_PIN

/* 1000 msec = 1 sec */
#define SLEEP_TIME 1000

void main(void)
{
int cnt = 0;
struct device *dev;

dev = device_get_binding(LED_PORT);
/* Set LED pin as output */
gpio_pin_configure(dev, LED, GPIO_DIR_OUT);
dev = device_get_binding(LED_DEV_NAME);

while (1) {
/* Set pin to HIGH/LOW every 1 second */
gpio_pin_write(dev, LED, cnt % 2);
cnt++;
led_on(dev, LED);
k_sleep(SLEEP_TIME);

led_off(dev, LED);
k_sleep(SLEEP_TIME);
}
}