Re: [PATCH v2 3/4] bsps: Add GPIO API
Hello Karel, On Thu, 2022-07-07 at 14:36 +0200, Karel Gardas wrote: > > Hello Duc, > > just two notes which bothers me most. > > On 7/7/22 13:34, Duc Doan wrote: > > This is the new GPIO API. The header file is > > gpio2.h. > > --- > > bsps/include/bsp/gpio2.h | 538 > > > > bsps/shared/dev/gpio/gpio.c | 196 + > > spec/build/bsps/obj.yml | 2 +- > > 3 files changed, 735 insertions(+), 1 deletion(-) > > create mode 100644 bsps/include/bsp/gpio2.h > > create mode 100644 bsps/shared/dev/gpio/gpio.c > > > > diff --git a/bsps/include/bsp/gpio2.h b/bsps/include/bsp/gpio2.h > > new file mode 100644 > > index 00..e99967cd47 > > --- /dev/null > > +++ b/bsps/include/bsp/gpio2.h > > @@ -0,0 +1,538 @@ > > +/** > > + * @file > > + * > > + * @ingroup rtems_gpio2 > > + * > > + * @brief RTEMS GPIO new API definition. > > + */ > > + > > +/* > > +* Copyright (c) 2022 Duc Doan > > +* > > +* The license and distribution terms for this file may be > > +* found in the file LICENSE in this distribution or at > > +* http://www.rtems.org/license/LICENSE. > > +*/ > > RTEMS project (Joel & others) perform huge effort in a way on > relicensing RTEMS to BSD license. Please use new license header > instead > of this one which is old -- from GPL days. Open few random RTEMS > files > and you will see few license headers. Look for example into ... > randomly > picked :-) -- > bsps/arm/stm32h7/boards/stm/nucleo-h743zi/stm32h7-bspstarthooks.c and > modify this to your name/email... > Thank you for the reminder. I was actually copying from a random source file before. I will change it in the next version. > > > +/** > > + * @brief Performs setup for GPIO functionality. > > + * > > + * This function calls bsp_gpio_register_controllers() and may > > + * perform additional initialization steps for GPIO > > functionality. > > + * It should be called by users before all GPIO operations, > > ideally > > + * when the application starts. > > + */ > > +extern void rtems_gpio_begin( > > + void > > +); > > I don't like this _begin mentality here. I think this should be all > done > by BSP code based on actual BSP configuration. > Something like: > STM32F4_ENABLE_GENERIC_GPIO implemented as optengpio.yml in f4 spec > and > then based on its C preprocessor value just calls > bsp_gpio_register_controllers from probably hook_1 and be done with > that. > > Sorry about that, 'begin' is too much RDBMS hooked thing here and > when I > see 'begin' I also expect 'commit' or 'rollback' somewhere down the > line. :-) > > Thanks, > Karel > I see, it makes sense to call that function automatically. I actually tried to do so using RTEMS_SYSINIT_ITEM but failed. I will try putting it into the hook. Best, Duc Doan ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH v2 3/4] bsps: Add GPIO API
Hello Duc, just two notes which bothers me most. On 7/7/22 13:34, Duc Doan wrote: This is the new GPIO API. The header file is gpio2.h. --- bsps/include/bsp/gpio2.h| 538 bsps/shared/dev/gpio/gpio.c | 196 + spec/build/bsps/obj.yml | 2 +- 3 files changed, 735 insertions(+), 1 deletion(-) create mode 100644 bsps/include/bsp/gpio2.h create mode 100644 bsps/shared/dev/gpio/gpio.c diff --git a/bsps/include/bsp/gpio2.h b/bsps/include/bsp/gpio2.h new file mode 100644 index 00..e99967cd47 --- /dev/null +++ b/bsps/include/bsp/gpio2.h @@ -0,0 +1,538 @@ +/** + * @file + * + * @ingroup rtems_gpio2 + * + * @brief RTEMS GPIO new API definition. + */ + +/* +* Copyright (c) 2022 Duc Doan +* +* The license and distribution terms for this file may be +* found in the file LICENSE in this distribution or at +* http://www.rtems.org/license/LICENSE. +*/ RTEMS project (Joel & others) perform huge effort in a way on relicensing RTEMS to BSD license. Please use new license header instead of this one which is old -- from GPL days. Open few random RTEMS files and you will see few license headers. Look for example into ... randomly picked :-) -- bsps/arm/stm32h7/boards/stm/nucleo-h743zi/stm32h7-bspstarthooks.c and modify this to your name/email... +/** + * @brief Performs setup for GPIO functionality. + * + * This function calls bsp_gpio_register_controllers() and may + * perform additional initialization steps for GPIO functionality. + * It should be called by users before all GPIO operations, ideally + * when the application starts. + */ +extern void rtems_gpio_begin( +void +); I don't like this _begin mentality here. I think this should be all done by BSP code based on actual BSP configuration. Something like: STM32F4_ENABLE_GENERIC_GPIO implemented as optengpio.yml in f4 spec and then based on its C preprocessor value just calls bsp_gpio_register_controllers from probably hook_1 and be done with that. Sorry about that, 'begin' is too much RDBMS hooked thing here and when I see 'begin' I also expect 'commit' or 'rollback' somewhere down the line. :-) Thanks, Karel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
[PATCH v2 3/4] bsps: Add GPIO API
This is the new GPIO API. The header file is
gpio2.h.
---
bsps/include/bsp/gpio2.h| 538
bsps/shared/dev/gpio/gpio.c | 196 +
spec/build/bsps/obj.yml | 2 +-
3 files changed, 735 insertions(+), 1 deletion(-)
create mode 100644 bsps/include/bsp/gpio2.h
create mode 100644 bsps/shared/dev/gpio/gpio.c
diff --git a/bsps/include/bsp/gpio2.h b/bsps/include/bsp/gpio2.h
new file mode 100644
index 00..e99967cd47
--- /dev/null
+++ b/bsps/include/bsp/gpio2.h
@@ -0,0 +1,538 @@
+/**
+ * @file
+ *
+ * @ingroup rtems_gpio2
+ *
+ * @brief RTEMS GPIO new API definition.
+ */
+
+/*
+* Copyright (c) 2022 Duc Doan
+*
+* The license and distribution terms for this file may be
+* found in the file LICENSE in this distribution or at
+* http://www.rtems.org/license/LICENSE.
+*/
+
+#ifndef LIBBSP_BSP_GPIO2_H
+#define LIBBSP_BSP_GPIO2_H
+
+#include
+#include
+
+/**
+ * Configure the maximum number of GPIO controllers used in
+ * a application.
+ *
+ * The macro CONFIGURE_GPIO_MAXIMUM_CONTROLLERS can be
+ * defined in application code. If it is not defined,
+ * it will default to BSP_GPIO_NUM_CONTROLLERS. If BSP's
+ * number of controllers is not defined, it will default
+ * to 1.
+ */
+#ifndef CONFIGURE_GPIO_MAXIMUM_CONTROLLERS
+
+#ifndef BSP_GPIO_NUM_CONTROLLERS
+#define CONFIGURE_GPIO_MAXIMUM_CONTROLLERS 1
+#else
+#define CONFIGURE_GPIO_MAXIMUM_CONTROLLERS BSP_GPIO_NUM_CONTROLLERS
+#endif /* BSP_GPIO_NUM_CONTROLLERS */
+
+#endif /* CONFIGURE_GPIO_MAXIMUM_CONTROLLERS */
+
+#ifdef __cplusplus
+extern "C" {
+#endif /* __cplusplus */
+
+/**
+ * @name GPIO data structures
+ *
+ * @{
+ */
+
+/**
+ * @brief GPIO bit set and reset enumeration.
+ */
+typedef enum {
+RTEMS_GPIO_PIN_RESET = 0,
+RTEMS_GPIO_PIN_SET = 1
+} rtems_gpio_pin_state;
+
+/**
+ * @brief GPIO pin modes.
+ */
+typedef enum {
+RTEMS_GPIO_PINMODE_OUTPUT = 0,
+RTEMS_GPIO_PINMODE_OUTPUT_PP = 0,
+RTEMS_GPIO_PINMODE_OUTPUT_OD = 1,
+RTEMS_GPIO_PINMODE_INPUT = 2,
+RTEMS_GPIO_PINMODE_ANALOG = 3,
+RTEMS_GPIO_PINMODE_BSP_SPECIFIC = 4
+} rtems_gpio_pin_mode;
+
+/**
+ * @brief GPIO pull resistor configuration. Defines pull-up or
+ *pull-down activation.
+ */
+typedef enum {
+RTEMS_GPIO_NOPULL,
+RTEMS_GPIO_PULLUP,
+RTEMS_GPIO_PULLDOWN
+} rtems_gpio_pull;
+
+/**
+ * @brief Interrupt modes enumeration.
+ */
+typedef enum {
+RTEMS_GPIO_INT_TRIG_NONE = 0,
+RTEMS_GPIO_INT_TRIG_FALLING,
+RTEMS_GPIO_INT_TRIG_RISING,
+RTEMS_GPIO_INT_TRIG_BOTH_EDGES,
+RTEMS_GPIO_INT_TRIG_LOW,
+RTEMS_GPIO_INT_TRIG_HIGH
+} rtems_gpio_interrupt_trig;
+
+typedef struct rtems_gpio_handlers rtems_gpio_handlers;
+typedef struct rtems_gpio rtems_gpio;
+/**
+ * @brief Typedef of the function pointer of an ISR.
+ */
+typedef void (*rtems_gpio_isr)(void *);
+
+/**
+ * @brief Structure containing pointers to handlers of a
+ *BSP/driver. Each BSP/driver must define its own
+ *handlers and create an object of this structure
+ *with pointers to those handlers.
+ */
+struct rtems_gpio_handlers {
+/**
+ * @brief This member is the pointer to an initialize handler.
+ *
+ * This handler could be used to perform some set up steps for
+ * a GPIO object (which means a pin or a port).
+ */
+rtems_status_code (*init)(rtems_gpio *);
+
+/**
+ * @brief This member is the pointer to a deinitialize handler.
+ *
+ * This handler could be used to deinitialize a GPIO object.
+ */
+rtems_status_code (*deinit)(rtems_gpio *);
+
+/**
+ * @brief This member is the pointer to a handler for setting
+ *pin mode.
+ *
+ * Pin modes are from rtems_gpio_pin_mode enumeration.
+ */
+rtems_status_code (*set_pin_mode)(rtems_gpio *, rtems_gpio_pin_mode);
+
+/**
+ * @brief This member is the pointer to a handler for setting
+ *pull resistor mode.
+ *
+ * Pull resistor modes are from rtems_gpio_pull enumeration.
+ */
+rtems_status_code (*set_pull)(rtems_gpio *, rtems_gpio_pull);
+
+/**
+ * @brief This member is the pointer to a handler for configuring
+ *interrupt of a pin.
+ *
+ * This handler should register ISR and its argument, interrupt
+ * trigger mode, and pull resister mode for the pin.
+ *
+ * @note Enabling interrupt should be done in enable_interrupt()
+ * handler.
+ */
+rtems_status_code (*configure_interrupt)(rtems_gpio *, rtems_gpio_isr,
void *, rtems_gpio_interrupt_trig, rtems_gpio_pull);
+
+/**
+ * @brief This member is the pointer to a handler for removing
+ *interrupt settings of a pin.
+ *
+ * Interrupt settings can be ISR address, pin configuration, etc.
+ */
+rtems_status_code (*remove_interrupt)(rtems_gpio *);
+
+/**
+ * @brief This member is the pointer to a
