[PATCH] Proposal for new GPIO API and example implementation for STM32F4 BSP

[Duc Doan]

>
> Using examples of working APIs is a good idea. But be careful not to
> copy any code from that framework because the license (GPL) wouldn't be
> compatible.


Sure, I don't copy their code but only see what functions should be in a
GPIO API (like read, write, toggle, pinmode).

Is set and reset the only state a pin can have? Or is that field thought
> to be extended with (for example) open drain or similar states if a
> device supports that?
>

I think it can be extended but I have only put in the most basic states for
now.


> Beneath that: Not a fan of SET == 0. I think RESET == 0 would be better
> because than you can use it as a boolean without conversion.
>

You are right, that was my mistake. I have fixed it.

Does that mean it is (for example) a GPIO controller? In that case maybe
> call it "rtems_gpio_ctrl_t". My first guess from the name would have
> been that it is already a pin.
>

I have made changes in my newest patch, and rtems_gpio_t is now essentially
a pin.

Same is true for the structures below: How would I initialize one of
> these objects in the application?
>

The example initialization of those structures can be found in my sample
blink code:
https://github.com/dtbpkmte/GSoC-2022-RTEMS-Sample-Apps/blob/main/RTEMS_Blink_API/src/init.c
. The definition of the structures are defined by BSP. Here are STM32F4's
definitions:
/********************* CODE BEGINS ***************************************/
struct rtems_gpio_t {
    GPIO_TypeDef *port;
    uint16_t pin;
};

/**
  * This structure should be initialized to 0
  *
  * This structure holds configuration options for STM32F4-specific
  * GPIO. The 2 specific modes are Event and Alternate modes. Select
  * a mode by setting the correct field (is_event_mode or is_alternate_mode)
  * to 1.
  */
struct rtems_gpio_config_t {
    uint32_t speed;             /* Speed of the pin. Must be specified */

    uint32_t interrupt_mode;    /* The type of interrupt trigger of the pin
                                   Use if the pin is in Interrupt mode */

    uint32_t is_event_mode;     /* Sets to 1 if the pin is in event mode,
                                   0 other wise.
                                   Use if the pin is in Event mode */
    uint32_t event_mode;        /* The type of event trigger of the pin
                                   Use if the pin is in Event mode */

    uint32_t is_alternate_mode; /* Sets to 1 if the pin is in Alternate
mode,
                                   0 other wise.
                                   Use if the pin is in Alternate mode */
    uint32_t alternate_mode;    /* Open drain or Push-pull mode
                                   Use if the pin is in Alternate mode */
    uint32_t alternate_fn;      /* The alternate function of the pin
                                   Use if the pin is in Alternate mode */
};
/********************* CODE ENDS ***************************************/

What would be done in that function?
>
> Why does the application has to implement it?
>
> Where would it be called?
>

The initialize function is meant to perform setup code for the GPIO. For
example, for the STM32, it would be enabling the clocks of the ports. The
application needs to implement it if they want to use GPIO because the
peripheral needs initialization. They don't have to, though, if they want
to do that initialization somewhere else. This function is called in
bsp_start(). It is already implemented as a weak, empty function, so there
is no problem if the user doesn't implement it.

What would be possible error cases? Is one "UNSATISFIED" enough or
> should it be able to return other errors too (like the return value from
> an I2C transfer because the pin is connected to some SPI-GPIO extender).


 I think "UNSATISFIED" is enough because these are quite low-level. I think
the return values from more complicated things like I2C should be handled
in other functions (like the I2C API), and I want to keep the GPIO thing
portable. I am open to more discussion though, because I am not sure I can
think of all use cases of this GPIO API.

If the write can fail, the read can also fail. Do we need an error
> return value too?
>

Yes, I think that's a good idea. I changed it as below:

/**
  * @brief Reads the digital value of a pin.
  *
  * @param[in] gpiox The GPIO object that owns the pin.
  *
  * @param[out] value The state of the pin.
  *
  * @retval RTEMS_SUCCESSFUL Pin succesfully read.
  * @retval RTEMS_UNSATISFIED Could not read pin.
  */
extern rtems_status_code rtems_gpio_read_pin(rtems_gpio_t *gpiox,
rtems_gpio_pin_state *value);

Best,

Duc Doan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20220621/e1bd0f62/attachment.htm>