[PATCH v5 2/4] bsps: New GPIO API & peripherals API framework
Duc Doan
dtbpkmte at gmail.com
Tue Aug 2 07:17:30 UTC 2022
Hello Christian,
On Mon, 2022-08-01 at 20:29 +0200, oss at c-mauderer.de wrote:
> Hello Duc,
>
> Am 01.08.22 um 05:50 schrieb Duc Doan:
> > Hello Christian,
> >
> > On Sat, 2022-07-30 at 17:50 +0200, oss at c-mauderer.de wrote:
> > > Hello Duc,
> > >
> > > Am 24.07.22 um 14:01 schrieb Duc Doan:
> > > > ---
> > > > bsps/include/bsp/gpio2.h | 528
> > > > ++++++++++++++++++++++++
> > > > bsps/include/bsp/periph_api.h | 142 +++++++
> > > > bsps/shared/dev/gpio/gpio.c | 212 ++++++++++
> > > > bsps/shared/dev/periph_api/periph_api.c | 101 +++++
> > > > spec/build/bsps/obj.yml | 4 +-
> > > > 5 files changed, 986 insertions(+), 1 deletion(-)
> > > > create mode 100644 bsps/include/bsp/gpio2.h
> > > > create mode 100644 bsps/include/bsp/periph_api.h
> > > > create mode 100644 bsps/shared/dev/gpio/gpio.c
> > > > create mode 100644 bsps/shared/dev/periph_api/periph_api.c
> > > >
> > > > diff --git a/bsps/include/bsp/gpio2.h
> > > > b/bsps/include/bsp/gpio2.h
> > >
> > > I'm not really happy with the "2" in the name. But at the moment
> > > I
> > > don't
> > > have a much better idea either. But if you call it "gpio2", you
> > > should
> > > call the C files "gpio2" too. Maybe "dev/gpio/gpio.h" similar to
> > > the
> > > "dev/i2c/i2c.h"?
> > >
> >
> > I will rename the C file to "gpio2.c" for now.
> >
> > > > new file mode 100644
> > > > index 0000000000..9cb1c720ab
> > > > --- /dev/null
> > > > +++ b/bsps/include/bsp/gpio2.h
> > > > @@ -0,0 +1,528 @@
> > > > +/* SPDX-License-Identifier: BSD-2-Clause */
> > > > +
> > >
> > > Your file is missing some general doxygen group information. Take
> > > a
> > > look
> > > at for example bsps/include/ofw/ofw.h.
> > >
> >
> > Thanks for the reminder, I will add that.
> >
> > > > +/*
> > > > + * Copyright (C) 2022 Duc Doan (dtbpkmte at gmail.com)
> > > > + *
> > > > + * Redistribution and use in source and binary forms, with or
> > > > without
> > > > + * modification, are permitted provided that the following
> > > > conditions
> > > > + * are met:
> > > > + * 1. Redistributions of source code must retain the above
> > > > copyright
> > > > + * notice, this list of conditions and the following
> > > > disclaimer.
> > > > + * 2. Redistributions in binary form must reproduce the above
> > > > copyright
> > > > + * notice, this list of conditions and the following
> > > > disclaimer
> > > > in the
> > > > + * documentation and/or other materials provided with the
> > > > distribution.
> > > > + *
> > > > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> > > > CONTRIBUTORS "AS IS"
> > > > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> > > > LIMITED TO, THE
> > > > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
> > > > PARTICULAR PURPOSE
> > > > + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
> > > > CONTRIBUTORS BE
> > > > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
> > > > EXEMPLARY, OR
> > > > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
> > > > PROCUREMENT OF
> > > > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
> > > > PROFITS; OR
> > > > BUSINESS
> > > > + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
> > > > LIABILITY,
> > > > WHETHER IN
> > > > + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE
> > > > OR
> > > > OTHERWISE)
> > > > + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
> > > > ADVISED OF THE
> > > > + * POSSIBILITY OF SUCH DAMAGE.
> > > > + */
> > > > +
> > > > +#ifndef LIBBSP_BSP_GPIO2_H
> > > > +#define LIBBSP_BSP_GPIO2_H
> > > > +
> > > > +#include <bsp.h>
> > > > +#include <rtems.h>
> > > > +
> > > > +/**
> > > > + * 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
> > >
> > > How do you manage that the CONFIGURE_GPIO_MAXIMUM_CONTROLLERS
> > > from
> > > the
> > > application is visible here?
> > >
> >
> > I forgot to remove this CONFIGURE_GPIO_MAXIMUM_CONTROLLERS thing.
> > At
> > first I wanted to do something like confdef but couldn't, so I
> > changed
> > to use BSP_GPIO_NUM_CONTROLLERS, which is a build option. I will
> > delete
> > that code. However, if you have a document about how confdef works,
> > it
> > would be helpful for me and I may be able to make that
> > CONFIGURE_GPIO_MAXIMUM_CONTROLLERS work.
>
> I haven't checked since the new build system either. The official way
> is
> still to just add them to confdefs.h similar like all other options.
> Not
> sure whether that is documented somewhere. The better mut much more
> difficult way is to add them via the specification items in
> rtems-central. But I think a BSP option BSP_GPIO_NUM_CONTROLLERS is
> OK
> for now. Please select a sensible default value. I think the bigger
> controllers that I know have about 300 pins and GPIOs with 32 pins
> per
> GPIO. That would be about 10 controllers. So a default could be
> something like 16 and it should work in most cases. Small controllers
> can save a few bytes by reducing the default.
>
> >
> > > > +
> > > > +#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 */
> > > > +
> > > > +/**
> > > > + * @brief Macro to initialize rtems_gpio.
> > > > + *
> > > > + * @param gpioh pointer to GPIO handlers
> > > > + */
> > > > +#define
> > > > RTEMS_GPIO_BUILD_BASE(_gpio_handlers) \
> > > > + (rtems_gpio) { .virtual_pin =
> > > > 0, \
> > > > + .gpio_handlers = ( _gpio_handlers
> > > > ), \
> > > > + .api =
> > > > NULL \
> > > > + };
> > > > +
> > > > +/**
> > > > + * @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
> > >
> > > Maybe the BSP_SPECIFIC should start at a high, fixed offest so
> > > that
> > > adding new general values is simpler. Something like
> > >
> > > RTEMS_GPIO_PINMODE_BSP_SPECIFIC = 100,
> > >
> > > A BSP can than use 100, 101, 102, ... for it's specific
> > > functions. If
> > > someone later adds a generic RTEMS_GPIO_PINMODE_FOO = 4, it won't
> > > result
> > > in any conflicts.
> > >
> > > > +} 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
> > >
> > > Maybe add a "BSP_SPECIFIC" here too. I know of controllers that
> > > can
> > > activate different values for pull up or pull down (like 100k or
> > > 10k).
> > >
> >
> > Sure, I will add that.
> >
> > > > +} 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 *);
> > > > +
> > > > +#include <bsp/periph_api.h>
> > > > +
> > > > +/**
> > > > + * @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 a handler for
> > > > setting
> > > > + * pin mode.
> > > > + *
> > >
> > > I would remove the repeating "This member is ...". Just describe
> > > it
> > > as
> > > "Handler for setting the pin mode." It's clear that it is a
> > > member
> > > because it is in a structure and it's clear that it is a pointer
> > > due
> > > to
> > > it's type. So this information is just redundant and makes it
> > > longer
> > > to
> > > read.
> > >
> > > The same is true for all following handlers.
> > >
> >
> > I saw that in a file (I don't remember which one) so I thought that
> > was
> > the convention. But yes, I also found it pretty redundant. I will
> > remove that.
>
> In general a shorter description is easier to read. If I remember
> correctly, one of my first commits to RTEMS was reworking some
> documentation for a subsystem. I removed quite some of these long
> descriptions in to make them better readable.
>
> >
> > > > + * Pin modes are from rtems_gpio_pin_mode enumeration.
> > >
> > > I think doxygen wants a @ref if you reference some other object.
> > >
> > > > + */
> > > > + 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 handler for
> > > > enabling
> > > > + * interrupt functionality of a pin.
> > > > + */
> > > > + rtems_status_code (*enable_interrupt)(rtems_gpio *);
> > > > +
> > > > + /**
> > > > + * @brief This member is the pointer to a handler for
> > > > disabling
> > > > + * interrupt of a pin.
> > > > + */
> > > > + rtems_status_code (*disable_interrupt)(rtems_gpio *);
> > > > +
> > > > + /**
> > > > + * @brief This member is the pointer to a handler for
> > > > reading
> > > > + * the digital value of a pin.
> > > > + *
> > > > + * The returned value should be in rtems_gpio_pin_state
> > > > enum.
> > > > + */
> > > > + rtems_status_code (*read)(rtems_gpio *,
> > > > rtems_gpio_pin_state
> > > > *);
> > > > +
> > > > + /**
> > > > + * @brief This member is the pointer to a handler for
> > > > writing
> > > > + * a digital value to a pin.
> > > > + */
> > > > + rtems_status_code (*write)(rtems_gpio *,
> > > > rtems_gpio_pin_state);
> > > > +
> > > > + /**
> > > > + * @brief This member is the pointer to a handler for
> > > > toggling
> > > > + * a pin.
> > > > + *
> > > > + * It should change pin state from SET to RESET or vice
> > > > versa.
> > > > + */
> > > > + rtems_status_code (*toggle)(rtems_gpio *);
> > > > +
> > > > +};
> > > > +
> > > > +/**
> > > > + * @brief Structure representing a GPIO object.
> > > > + *
> > > > + * BSP/drivers need to define their own GPIO structures with
> > > > + * rtems_gpio being the first member.
> > > > + */
> > > > +struct rtems_gpio {
> > > > + /**
> > > > + * @brief This member is a virtual pin number, counting
> > > > from
> > > > + * 0 (zero).
> > >
> > > Is that a global 0 or a per controller 0? Can you add that to the
> > > doxygen description?
> > >
> >
> > This is a global 0.
> >
> > > If you have to describe it multiple times: Maybe add a general
> > > GPIO-
> > > API
> > > description. That's what the group right at the beginning of the
> > > file
> > > is
> > > for.
> > >
> >
> > Yes, I will write a general description at the beginning.
> >
> > > > + */
> > > > + uint32_t virtual_pin;
> > > > + /**
> > > > + * @brief This member is a pointer to a structure
> > > > containing
> > > > + * pointers to handlers of a GPIO controller.
> > > > + */
> > > > + const rtems_gpio_handlers *gpio_handlers;
> > > > + /**
> > > > + * @brief This member is a pointer to a peripheral API.
> > > > + */
> > > > + rtems_periph_api *api;
> > > > +};
> > > > +
> > > > +/** @} */
> > > > +
> > > > +/**
> > > > + * @name GPIO System operations
> > > > + *
> > > > + * Functions in this group should not be called by user
> > > > + * applications. They are used by BSP/drivers only.
> > >
> > > An application can add a driver. So the functions can be called
> > > by an
> > > application too.
> > >
> >
> > I will change that.
> >
> > > > + *
> > > > + * @{
> > > > + */
> > > > +
> > > > +/**
> > > > + * @brief Perform initialization for GPIO system and
> > > > + * Peripheral API system.
> > > > + * This function is called automatically using
> > > > + * SYSINIT.
> > > > + */
> > > > +extern void rtems_gpio_start(
> > > > + void
> > > > +);
> > >
> > > You have the sysinit right below the function in the C file. I
> > > think
> > > you
> > > can just remove the prototype and make the function static.
> > >
> > > > +
> > > > +/**
> > > > + * @brief Registers a GPIO controller with GPIO manager.
> > > > + *
> > > > + * This function registers the pointer to BSP/driver-specific
> > > > + * get_gpio() and destroy_gpio() functions. Those two
> > > > functions
> > > > + * are for creating and destroying GPIO objects. It also
> > > > takes
> > > > + * the number of pins of each BSP/driver for mapping into a
> > > > + * flat pin numbering system (virtual pin number).
> > > > + * This function also help register peripherals API get()
> > > > + * and remove() functions.
> > > > + *
> > > > + * @param get_gpio The pointer to BSP/driver-specific
> > > > get_gpio()
> > > > + * @param destroy_gpio The pointer to BSP/driver-specific
> > > > + * destroy_gpio()
> > > > + * @param get_api The pointer to BSP/driver-specific
> > > > get_api()
> > > > + * @param remove_api The pointer to BSP/driver-specific
> > > > remove_api()
> > > > + * @param pin_count The number of GPIO pins in the controller
> > > > + *
> > > > + * @retval RTEMS_SUCCESSFUL Controller registered
> > > > successfully
> > > > + * @retval RTEMS_TOO_MANY if the maximum number of
> > > > controllers
> > > > are
> > > > + * already registered
> > > > + */
> > > > +extern rtems_status_code rtems_gpio_register(
> > > > + rtems_status_code (*get_gpio)(uint32_t, rtems_gpio **),
> > > > + rtems_status_code (*destroy_gpio)(rtems_gpio *),
> > > > + rtems_periph_api *(*get_api)(rtems_gpio *,
> > > > rtems_periph_api_type),
> > > > + rtems_status_code (*remove_api)(rtems_gpio *),
> > > > + uint32_t pin_count
> > > > +);
> > > > +
> > > > +/** @} */
> > > > +
> > > > +/**
> > > > + * @name GPIO BSP functions
> > > > + *
> > > > + * BSP and drivers need to implement these.
> > >
> > > I would replace "BSP" by "drivers" and not mention both. It's not
> > > relevant whether the driver is in the BSP or in the application.
> > >
> > > > + *
> > > > + * @{
> > > > + */
> > > > +
> > > > +/**
> > > > + * @brief Wrapper to perform all BSP controllers registering
> > > > + * with the GPIO manager.
> > > > + *
> > > > + * This function must be implemented by BSP. It should call
> > > > + * rtems_gpio_register() to register each integrated GPIO
> > > > + * controller.
> > > > + */
> > > > +extern rtems_status_code bsp_gpio_register_controllers(
> > > > + void
> > > > +);
> > >
> > > I would remove that function. Let the BSP use SYSINIT for that.
> > > Make
> > > sure that the rtems_periph_api_start is started before the
> > > drivers in
> > > RTEMS_SYSINIT_BSP_PRE_DRIVERS.
> > >
> >
> > Would keeping this function makes it easier for BSP? Like, each BSP
> > does not need to separately use SYSINIT and care about the order;
> > they
> > just need to define a function.
> >
>
> SYSINIT is exactly for that: That a BSP and application can specify
> the
> order itself. A system that supports the drivers should be
> initialized
> before the drivers somewhere in RTEMS_SYSINIT_BSP_PRE_DRIVERS or
> maybe
> even earlier.
>
> Currently you force an order. What if some driver for a serial bus or
> similar needs the GPIO system. It would have to be initialized after
> your SYSINIT entry and therefore know where you put your
> initialization.
> So for these cases you make it more difficult for the driver
> developer
> who want's to use the GPIO system.
>
I see, this makes sense.
> > > > +
> > > > +/** @} */
> > > > +
> > > > +/**
> > > > + * @name GPIO Application operations
> > > > + *
> > > > + * @{
> > > > + */
> > > > +
> > > > +/**
> > > > + * @brief Get the GPIO object containing information
> > > > + * about the specified pin.
> > > > + *
> > > > + * This function maps the virtual pin to intermediate pin,
> > > > + * and pass to the BSP/driver-specific function to get a
> > > > + * GPIO object.
> > > > + *
> > > > + * @note Warning: this function may use malloc(). When you
> > > > + * are done with the GPIO object, call rtems_gpio_destroy()
> > > > + * to avoid memory leak.
> > > > + *
> > > > + * @param virt_pin The virtual pin number.
> > > > + * @param[out] out The pointer to the pointer to the output
> > > > + * GPIO object.
> > > > + *
> > > > + * @retval RTEMS_SUCCESSFUL
> > > > + * @retval RTEMS_UNSTISFIED if the virtual pin number
> > > > + * is invalid (i.e. out of range)
> > > > + */
> > > > +extern rtems_status_code rtems_gpio_get(
> > > > + uint32_t virt_pin,
> > > > + rtems_gpio **out
> > > > +);
> > > > +
> > > > +/**
> > > > + * @brief Destroy a GPIO object
> > > > + *
> > > > + * This function should be called on an GPIO object which is
> > > > + * no longer used to avoid memory leak. Internally it can
> > > > + * use free().
> > > > + *
> > > > + * @param[in] base The pointer to the GPIO object.
> > > > + *
> > > > + * @retval RTEMS_SUCCESSFUL
> > > > + * @retval RTEMS_UNSATISFIED
> > > > + */
> > > > +extern rtems_status_code rtems_gpio_destroy(
> > > > + rtems_gpio *base
> > > > +);
> > > > +
> > > > +/**
> > > > + * @brief Sets the pin mode of a pin.
> > > > + *
> > > > + * This function calls the registered BSP/driver-specific
> > > > handler.
> > > > + * It can be used to set the pin mode for a pin.
> > > > + *
> > > > + * @param[in] base The GPIO object to be configured.
> > > > + * @param mode The pin mode from the enumeration
> > > > rtems_gpio_pin_mode
> > > > + *
> > > > + * @retval RTEMS_SUCCESSFUL GPIO configured successfully.
> > > > + * @retval RTEMS_UNSATISFIED Could not set the pin mode.
> > > > + * @retval @see BSP/driver-specific function for other return
> > > > codes.
> > > > + */
> > > > +extern rtems_status_code rtems_gpio_set_pin_mode(
> > > > + rtems_gpio *base,
> > > > + rtems_gpio_pin_mode mode
> > > > +);
> > > > +
> > > > +/**
> > > > + * @brief Sets the pin's pull resistor configuration.
> > > > + *
> > > > + * This function calls the registered BSP/driver-specific
> > > > handler.
> > > > + * It can be used to set the pull resistor mode for a pin.
> > > > + *
> > > > + * @param[in] base The GPIO object to be configured.
> > > > + * @param mode The pull mode from the enumeration
> > > > rtems_gpio_pull
> > > > + *
> > > > + * @retval RTEMS_SUCCESSFUL GPIO configured successfully.
> > > > + * @retval RTEMS_UNSATISFIED Could not set the pull resistor
> > > > mode.
> > > > + * @retval @see BSP/driver-specific function for other return
> > > > codes.
> > >
> > > The doxygen "@see" wants a reference where it can point to. I
> > > don't
> > > think that you have one for "BSP/driver-specific"?
> > >
> >
> > Oh, I understood it incorrectly. I will change that.
> >
> > > > + */
> > > > +extern rtems_status_code rtems_gpio_set_pull(
> > > > + rtems_gpio *base,
> > > > + rtems_gpio_pull pull
> > > > +);
> > > > +
> > > > +/**
> > > > + * @brief Configure interrupt on a GPIO pin.
> > > > + *
> > > > + * This function calls the registered BSP/driver-specific
> > > > handler.
> > > > + * It can be used to set up a pin for interrupt mode.
> > > > + *
> > > > + * @note This only configures the interrupt but not enable
> > > > it.
> > > > Use
> > > > + * rtems_gpio_enable_interrupt() to actually enable
> > > > interrupt.
> > > > + *
> > > > + * @param[in] base The GPIO object to be configured.
> > > > + * @param isr The pointer to the ISR to be attached to this
> > > > pin.
> > > > + * @param[in] arg The pointer to the argument of the ISR.
> > > > + * @param trig The trigger mode
> > > > + * @param pull The pull resistor mode
> > > > + *
> > > > + * @retval RTEMS_SUCCESSFUL Interrupt configured
> > > > successfully.
> > > > + * @retval RTEMS_UNSATISFIED Could not configure interrupt.
> > > > + * @retval @see BSP/driver-specific function for other return
> > > > codes.
> > > > + */
> > > > +extern rtems_status_code rtems_gpio_configure_interrupt(
> > > > + rtems_gpio *base,
> > > > + rtems_gpio_isr isr,
> > > > + void *arg,
> > > > + rtems_gpio_interrupt_trig trig,
> > > > + rtems_gpio_pull pull
> > > > +);
> > > > +
> > > > +/**
> > > > + * @brief Remove interrupt settings for a pin.
> > > > + *
> > > > + * This function calls the registered BSP/driver-specific
> > > > handler.
> > > > + * It can be used to remove interrupt configuration of a pin
> > > > like
> > > > + * ISR, ISR argument, pin mode, etc.
> > > > + *
> > > > + * @note This only removes the interrupt but not disable it.
> > > > Use
> > > > + * rtems_gpio_disable_interrupt() to actually disable
> > > > + * interrupt.
> > > > + *
> > > > + * @param[in] base The GPIO object to be configured.
> > > > + *
> > > > + * @retval RTEMS_SUCCESSFUL Interrupt removed successfully.
> > > > + * @retval RTEMS_UNSATISFIED Could not remove interrupt.
> > > > + * @retval @see BSP/driver-specific function for other return
> > > > codes.
> > > > + */
> > > > +extern rtems_status_code rtems_gpio_remove_interrupt(
> > > > + rtems_gpio *base
> > > > +);
> > > > +
> > > > +/**
> > > > + * @brief Enable interrupt on a GPIO pin.
> > > > + *
> > > > + * This function calls the registered BSP/driver-specific
> > > > handler.
> > > > + *
> > > > + * @note This function only enables the interrupt (e.g. the
> > > > vector)
> > > > + * but not configure the pin. Use
> > > > + * rtems_gpio_configure_interrupt() for pin
> > > > configuration.
> > > > + *
> > > > + * @param[in] base The GPIO object to be configured.
> > > > + *
> > > > + * @retval RTEMS_SUCCESSFUL Interrupt enabled successfully.
> > > > + * @retval RTEMS_UNSATISFIED Could not enable interrupt.
> > > > + * @retval @see BSP/driver-specific function for other return
> > > > codes.
> > > > + */
> > > > +extern rtems_status_code rtems_gpio_enable_interrupt(
> > > > + rtems_gpio *base
> > > > +);
> > > > +
> > > > +/**
> > > > + * @brief Disable interrupt on a GPIO pin.
> > > > + *
> > > > + * This function calls the registered BSP/driver-specific
> > > > handler.
> > > > + *
> > > > + * @note This function only disables the interrupt (e.g. the
> > > > vector)
> > > > + * but not remove the pin's configurations. Use
> > > > + * rtems_gpio_remove_interrupt() for the latter
> > > > purpose.
> > > > + *
> > > > + * @param[in] base The GPIO object to be configured.
> > > > + *
> > > > + * @retval RTEMS_SUCCESSFUL Interrupt disabled successfully.
> > > > + * @retval RTEMS_UNSATISFIED Could not disable interrupt.
> > > > + * @retval @see BSP/driver-specific function for other return
> > > > codes.
> > > > + */
> > > > +extern rtems_status_code rtems_gpio_disable_interrupt(
> > > > + rtems_gpio *base
> > > > +);
> > > > +
> > > > +/**
> > > > + * @brief Writes a digital value to a pin.
> > > > + *
> > > > + * This function calls the registered BSP/driver-specific
> > > > handler.
> > > > + *
> > > > + * @param[in] base The GPIO object that has information about
> > > > the
> > > > + * pin.
> > > > + * @param value The state to be written to the pin.
> > > > + *
> > > > + * @retval RTEMS_SUCCESSFUL Pin successfully written.
> > > > + * @retval @see BSP/driver-specific function for other return
> > > > codes.
> > > > + */
> > > > +extern rtems_status_code rtems_gpio_write(
> > > > + rtems_gpio *base,
> > > > + rtems_gpio_pin_state value
> > > > +);
> > > > +
> > > > +/**
> > > > + * @brief Reads the digital value of a pin.
> > > > + *
> > > > + * This function calls the registered BSP/driver-specific
> > > > handler.
> > > > + *
> > > > + * @param[in] base The GPIO object that has information about
> > > > the
> > > > + * pin.
> > > > + *
> > > > + * @param[out] value The state of the pin.
> > > > + *
> > > > + * @retval RTEMS_SUCCESSFUL Pin succesfully read.
> > > > + * @retval @see BSP/driver-specific function for other return
> > > > codes.
> > > > + */
> > > > +extern rtems_status_code rtems_gpio_read(
> > > > + rtems_gpio *base,
> > > > + rtems_gpio_pin_state *value
> > > > +);
> > > > +
> > > > +/**
> > > > + * @brief Toggles the state of a GPIO pin.
> > > > + *
> > > > + * This function calls the registered BSP/driver-specific
> > > > handler.
> > > > + *
> > > > + * @param[in] base The GPIO object that has information about
> > > > the
> > > > + * pin.
> > > > + *
> > > > + * @retval RTEMS_SUCCESSFUL Pin successfully toggled.
> > > > + * @retval @see BSP/driver-specific function for other return
> > > > codes.
> > > > + */
> > > > +extern rtems_status_code rtems_gpio_toggle(
> > > > + rtems_gpio *base
> > > > +);
> > > > +
> > > > +/** @} */
> > > > +
> > > > +#ifdef __cplusplus
> > > > +}
> > > > +#endif /* __cplusplus */
> > > > +
> > > > +#endif /* LIBBSP_BSP_GPIO2_H */
> > > > diff --git a/bsps/include/bsp/periph_api.h
> > > > b/bsps/include/bsp/periph_api.h
> > > > new file mode 100644
> > > > index 0000000000..fb02b701dc
> > > > --- /dev/null
> > > > +++ b/bsps/include/bsp/periph_api.h
> > >
> > > Isn't it an API just because you add it here. Is it really
> > > necessary
> > > to
> > > add the "API" in the name?
> > >
> >
> > I was thinking that this periph_api is the base for other APIs, so
> > I
> > added "api" in the name.
> >
> > > I'm not entirely sure yet whether this API is really something
> > > separate
> > > from your GPIO API. The "gpio_start" calls the "periph_api_start"
> > > and
> > > the periph_api functions use gpio internal structures. I think I
> > > would
> > > just add these few functions to the GPIO API.
> > >
> >
> > That is also an option. The reason I separated them is that I
> > thought
> > these additional functions should be somehow separated from the
> > basic
> > GPIO to avoid making GPIO API too complicated. Also, this API is
> > mostly
> > for adding new peripheral APIs and not targetting user application.
> > Users only need to use one function, set_api().
>
> Please note that in our case a user can and often will add his own
> drivers to an application too. Not all drivers are in the BSP. Some
> applications need specialy tuned drivers and therefor will bring
> their
> own ones.
>
> >
> > > I'm still not convinced that this is necessary at all. A
> > > peripheral
> > > has
> > > to know it's pins. But the pin doesn't have to know anything
> > > about
> > > the
> > > connected peripheral. So why do we need that? At the moment it
> > > seems
> > > to
> > > add mainly some complexity and uses some memory.
> > >
> >
> > Logically, a pin doesn't have to know about the connected
> > peripheral.
> > However, this newly added API is just a way to make things easier
> > for
> > users.
> >
> > Without this API, for each peripheral, users need to create new
> > objects
> > that hold information about both the GPIO pins and the handlers of
> > that
> > peripheral. They would have to maintain those objects all the time
> > during the use of that peripheral. If users want to change the
> > functionality, they have to create new objects of that peripheral
> > type.
>
> If the functionallity does not change, the peripheral can just know
> it's
> pin and the user just has to know and handle the peripheral. Only
> during
> the initialization he has to init the pin first and the peripheral
> afterwards. But that's the case with your API too, isn't it?
>
> If the functionality of a pin changes (which is a really rare use
> case)
> the user will need two APIs. So if I understand your code correctly,
> he
> would have to:
>
> - Init GPIO pin.
> - Add API for example for ADC.
> - Use that pin.
> - Remove API for ADC.
> - Add API for example for DAC.
> - Use the pin.
> - Remove API for DAC.
> ...
>
> So he has to switch in or out the API. For this switching he
> eitherhas
> to provide memory for the ADC / DAC API or the memory is dynamically
> allocated on every switch. I think I would rather keep multiple
> pointers
> in my application instead of allocating something every time.
>
> >
> > This API avoids that by reusing the already-existing GPIO pin
> > objects.
> > Users only need a single GPIO object for a pin for all operations,
> > be
> > it basic GPIO or additional peripherals. This creates simplicity
> > for
> > user application at the cost of added memory (one additional
> > pointer
> > member, which is not much in my opinion).
> >
>
> For a single use pin that doesn't have to switch function, the user
> needs only one pointer. The one to the object of the driver with the
> function he uses. The pin can be a part of that object.
>
> > By the way, this newly added API is mostly targeting peripherals
> > that
> > require a single pin like ADC or DAC.
>
> That doesn't make it better. Now you added complexity for very
> special
> cases ;-)
>
Hmm your reasoning makes sense. Should I change so that ADC contains
the pin as a member (and thus remove the peripheral API)?
Best,
Duc
> >
> > > > @@ -0,0 +1,142 @@
> > > > +/* SPDX-License-Identifier: BSD-2-Clause */
> > > > +
> > > > +/*
> > > > + * Copyright (C) 2022 Duc Doan (dtbpkmte at gmail.com)
> > > > + *
> > > > + * Redistribution and use in source and binary forms, with or
> > > > without
> > > > + * modification, are permitted provided that the following
> > > > conditions
> > > > + * are met:
> > > > + * 1. Redistributions of source code must retain the above
> > > > copyright
> > > > + * notice, this list of conditions and the following
> > > > disclaimer.
> > > > + * 2. Redistributions in binary form must reproduce the above
> > > > copyright
> > > > + * notice, this list of conditions and the following
> > > > disclaimer
> > > > in the
> > > > + * documentation and/or other materials provided with the
> > > > distribution.
> > > > + *
> > > > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> > > > CONTRIBUTORS "AS IS"
> > > > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> > > > LIMITED TO, THE
> > > > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
> > > > PARTICULAR PURPOSE
> > > > + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
> > > > CONTRIBUTORS BE
> > > > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
> > > > EXEMPLARY, OR
> > > > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
> > > > PROCUREMENT OF
> > > > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
> > > > PROFITS; OR
> > > > BUSINESS
> > > > + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
> > > > LIABILITY,
> > > > WHETHER IN
> > > > + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE
> > > > OR
> > > > OTHERWISE)
> > > > + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
> > > > ADVISED OF THE
> > > > + * POSSIBILITY OF SUCH DAMAGE.
> > > > + */
> > > > +
> > > > +#ifndef LIBBSP_BSP_PERIPH_API_H
> > > > +#define LIBBSP_BSP_PERIPH_API_H
> > > > +
> > > > +#include <bsp.h>
> > > > +#include <rtems.h>
> > > > +
> > > > +#ifdef __cplusplus
> > > > +extern "C" {
> > > > +#endif /* __cplusplus */
> > > > +
> > > > +/**
> > > > + * @brief Enumeration of supported peripheral APIs.
> > > > + */
> > > > +typedef enum {
> > > > + RTEMS_PERIPH_API_TYPE_ADC
> > >
> > > Would a BSP / Application specific range be usefull?
> > >
> >
> > Hmm, I'm not sure because this is aimed for creating APIs. It's
> > probably useful, so I'll add it.
> >
> > > > +} rtems_periph_api_type;
> > > > +
> > > > +/**
> > > > + * @brief The base structure of peripheral APIs.
> > > > + * All peripheral APIs must have this struct as
> > > > + * their first member.
> > > > + *
> > > > + * @see rtems_adc_api for example implementation.
> > > > + */
> > > > +typedef struct rtems_periph_api rtems_periph_api;
> > > > +
> > > > +#include <bsp/gpio2.h>
> > > > +
> > > > +/**
> > > > + * rtems_periph_api structure definition.
> > > > + */
> > > > +struct rtems_periph_api {
> > > > + /**
> > > > + * @brief This member tells the type of this
> > > > + * API.
> > > > + */
> > > > + rtems_periph_api_type api_type;
> > > > + /**
> > > > + * @brief This member is a pointer to a
> > > > + * BSP/driver-specific function that
> > > > + * performs initialization for the
> > > > + * API.
> > > > + */
> > > > + void (*init)(rtems_gpio *pin);
> > > > +};
> > > > +
> > > > +/**
> > > > + * @brief Register a handler to get API pointer.
> > > > + *
> > > > + * @param[in] get_api pointer to a handler to
> > > > + * get an API with specified type.
> > > > + * @param index the index of the GPIO controller.
> > > > + */
> > > > +void rtems_periph_api_register_api(
> > > > + rtems_periph_api *(*get_api)(rtems_gpio *,
> > > > rtems_periph_api_type),
> > > > + rtems_status_code (*remove_api)(rtems_gpio *),
> > > > + uint32_t index
> > > > +);
> > > > +
> > > > +/**
> > > > + * @brief Performs initialization for peripheral
> > > > + * API system.
> > > > + * This function registers some variables that
> > > > + * the peripheral API utilize from GPIO API.
> > > > + *
> > > > + * @param get_ctrl_index Pointer to a helper
> > > > + * that returns a GPIO controller index
> > > > + * of a pin.
> > > > + * @param num_ctrl Pointer to a variable that
> > > > + * tells the number of registered GPIO
> > > > + * controllers.
> > > > + */
> > > > +void rtems_periph_api_start(
> > > > + uint32_t (*get_ctrl_index)(uint32_t),
> > > > + uint32_t *num_ctrl
> > > > +);
> > > > +
> > > > +/**
> > > > + * @brief Assign an API to a GPIO object and
> > > > + * initialize it.
> > > > + *
> > > > + * @note This function may use malloc().
> > > > + * @note This function calls the handler init().
> > > > + *
> > > > + * @param pin The rtems_gpio object representing
> > > > + * a pin.
> > > > + * @param type The peripheral API type.
> > > > + *
> > > > + * @retval RTEMS_SUCCESSFUL if an API is set
> > > > + * correctly.
> > > > + * @retval RTEMS_UNSATISFIED if the API type is
> > > > + * invalid for this pin.
> > > > + * @retval RTEMS_NO_MEMORY if memory cannot be
> > > > + * allocated for API object.
> > > > + */
> > > > +rtems_status_code rtems_periph_api_set_api(
> > > > + rtems_gpio *pin,
> > > > + rtems_periph_api_type type
> > > > +);
> > > > +
> > > > +/**
> > > > + * @brief Remove the API assigned to a pin by
> > > > + * setting the pointer to NULL.
> > > > + *
> > > > + * @retval RTEMS_SUCCESSFUL
> > > > + */
> > > > +rtems_status_code rtems_periph_api_remove_api(
> > > > + rtems_gpio *pin
> > > > +);
> > > > +
> > > > +#ifdef __cplusplus
> > > > +}
> > > > +#endif /* __cplusplus */
> > > > +
> > > > +#endif /* LIBBSP_BSP_PERIPH_API_H */
> > > > diff --git a/bsps/shared/dev/gpio/gpio.c
> > > > b/bsps/shared/dev/gpio/gpio.c
> > > > new file mode 100644
> > > > index 0000000000..295b9c3738
> > > > --- /dev/null
> > > > +++ b/bsps/shared/dev/gpio/gpio.c
> > > > @@ -0,0 +1,212 @@
> > > > +/* SPDX-License-Identifier: BSD-2-Clause */
> > > > +
> > > > +/*
> > > > + * Copyright (C) 2022 Duc Doan (dtbpkmte at gmail.com)
> > > > + *
> > > > + * Redistribution and use in source and binary forms, with or
> > > > without
> > > > + * modification, are permitted provided that the following
> > > > conditions
> > > > + * are met:
> > > > + * 1. Redistributions of source code must retain the above
> > > > copyright
> > > > + * notice, this list of conditions and the following
> > > > disclaimer.
> > > > + * 2. Redistributions in binary form must reproduce the above
> > > > copyright
> > > > + * notice, this list of conditions and the following
> > > > disclaimer
> > > > in the
> > > > + * documentation and/or other materials provided with the
> > > > distribution.
> > > > + *
> > > > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> > > > CONTRIBUTORS "AS IS"
> > > > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> > > > LIMITED TO, THE
> > > > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
> > > > PARTICULAR PURPOSE
> > > > + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
> > > > CONTRIBUTORS BE
> > > > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
> > > > EXEMPLARY, OR
> > > > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
> > > > PROCUREMENT OF
> > > > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
> > > > PROFITS; OR
> > > > BUSINESS
> > > > + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
> > > > LIABILITY,
> > > > WHETHER IN
> > > > + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE
> > > > OR
> > > > OTHERWISE)
> > > > + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
> > > > ADVISED OF THE
> > > > + * POSSIBILITY OF SUCH DAMAGE.
> > > > + */
> > > > +
> > > > +#include <bsp/gpio2.h>
> > > > +#include <rtems/sysinit.h>
> > > > +
> > > > +/**
> > > > + * @brief An array to store all registered GPIO controllers.
> > > > + */
> > > > +static rtems_status_code
> > > > (*get_gpio_table[CONFIGURE_GPIO_MAXIMUM_CONTROLLERS])(uint32_t,
> > > > rtems_gpio **);
> > > > +
> > > > +/**
> > > > + * @brief An array to store all registered GPIO controllers.
> > > > + */
> > > > +static rtems_status_code
> > > > (*destroy_gpio_table[CONFIGURE_GPIO_MAXIMUM_CONTROLLERS])(rtems
> > > > _gpi
> > > > o *);
> > > > +
> > > > +/**
> > > > + * @brief An array to store the boundaries of pin index of
> > > > + * GPIO controllers.
> > > > + *
> > > > + * Example with 2 16-pin controllers and 1 32-pin controller,
> > > > + * the pin_map will be:
> > > > + * { 0, 16, 32, 64}
> > > > + * Value 0 is always at index 0 for convenience of
> > > > calculation.
> > > > + * The length of this array is always 1+(number of
> > > > controllers).
> > > > + */
> > > > +static uint32_t pin_map[CONFIGURE_GPIO_MAXIMUM_CONTROLLERS+1]
> > > > =
> > > > {0};
> > > > +
> > > > +/**
> > > > + * @brief The number of controllers registered.
> > > > + */
> > > > +static uint32_t num_ctrl = 0;
> > > > +
> > > > +static uint32_t get_ctrl_index(
> > > > + uint32_t virtual_pin
> > > > +);
> > > > +
> > > > +static uint32_t get_ctrl_index(
> > > > + uint32_t virtual_pin
> > > > +)
> > > > +{
> > > > + uint32_t i;
> > > > + // TODO: binary search
> > >
> > > If you add comments that should stay in the code: Please use C
> > > style
> > > comments with /* */
> > >
> > > Regarding the binary search: Is it called during normal useage or
> > > only
> > > on pin initialization. If the later one is the case, I don't
> > > think
> > > that
> > > it has to be optimized.
> > >
> >
> > It is called only in initialization. I'll remove the comment; I was
> > putting there as a reminder but forgot about it...
> >
> > > > + for (i = 1; i <= num_ctrl; ++i) {
> > > > + if (virtual_pin < pin_map[i]) {
> > > > + break;
> > > > + }
> > > > + }
> > > > + return i-1;
> > > > +}
> > > > +
> > > > +rtems_status_code rtems_gpio_register(
> > > > + rtems_status_code (*get_gpio)(uint32_t, rtems_gpio **),
> > > > + rtems_status_code (*destroy_gpio)(rtems_gpio *),
> > > > + rtems_periph_api *(*get_api)(rtems_gpio *,
> > > > rtems_periph_api_type),
> > > > + rtems_status_code (*remove_api)(rtems_gpio *),
> > > > + uint32_t pin_count
> > > > +)
> > > > +{
> > > > +// rtems_interrupt_level level;
> > >
> > > If it is not necessary, please remove commented code completely.
> > >
> > > > +
> > > > + if (num_ctrl == CONFIGURE_GPIO_MAXIMUM_CONTROLLERS)
> > > > + return RTEMS_TOO_MANY;
> > > > +
> > > > +// rtems_interrupt_disable(level);
> > > > + get_gpio_table[num_ctrl] = get_gpio;
> > > > + destroy_gpio_table[num_ctrl] = destroy_gpio;
> > > > +
> > > > + rtems_periph_api_register_api(get_api, remove_api,
> > > > num_ctrl);
> > > > +
> > > > + pin_map[num_ctrl+1] = pin_map[num_ctrl] + pin_count;
> > > > + ++num_ctrl;
> > >
> > > You support only registering controllers. Unregistering is not
> > > supported. Is that correct?
> > >
> >
> > Yes, unregistering is currently not supported.
> >
>
> OK. In that case I'm OK with this code.
>
> > > > +// rtems_interrupt_enable(level);
> > > > +
> > > > + return RTEMS_SUCCESSFUL;
> > > > +}
> > > > +
> > > > +rtems_status_code rtems_gpio_get(
> > > > + uint32_t virt_pin,
> > > > + rtems_gpio **out
> > > > +)
> > > > +{
> > > > + uint32_t i = get_ctrl_index(virt_pin);
> > > > + if (i >= num_ctrl)
> > > > + return RTEMS_UNSATISFIED;
> > > > +
> > > > + uint32_t pin = virt_pin - pin_map[i];
> > > > + rtems_status_code sc = (*get_gpio_table[i])(pin, out);
> > > > + if (sc == RTEMS_SUCCESSFUL) {
> > > > + (*out)->virtual_pin = virt_pin;
> > > > + }
> > > > + return sc;
> > > > +}
> > > > +
> > > > +rtems_status_code rtems_gpio_destroy(
> > > > + rtems_gpio *base
> > > > +)
> > > > +{
> > > > + uint32_t i = get_ctrl_index(base->virtual_pin);
> > > > + if (i >= num_ctrl)
> > > > + return RTEMS_UNSATISFIED;
> > > > + return (*destroy_gpio_table[i])(base);
> > > > +}
> > > > +
> > > > +void rtems_gpio_start(
> > > > + void
> > > > +)
> > > > +{
> > > > + rtems_periph_api_start(get_ctrl_index, &num_ctrl);
> > > > + bsp_gpio_register_controllers();
> > >
> > > See above: I would remove the bsp_gpio_register_controllers() ...
> > >
> > > > +}
> > > > +RTEMS_SYSINIT_ITEM(
> > > > + rtems_gpio_start,
> > > > + RTEMS_SYSINIT_DEVICE_DRIVERS,
> > > > + RTEMS_SYSINIT_ORDER_LAST_BUT_1
> > >
> > > ... and move that to the RTEMS_SYSINIT_BSP_PRE_DRIVERS.
> > >
> > > Best regards
> > >
> > > Christian
> >
> > Thanks for the comments,
> >
> > Duc
> >
>
> Best regards
>
> Christian
More information about the devel
mailing list