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

[Duc Doan]

On Tue, 2022-06-21 at 17:23 +0200, oss at c-mauderer.de wrote:
> OK. So every BSP that want's to use that API will have a different 
> rtems_gpio_config_t (or every other structure) that is defined in
> (for 
> example) bsp.h? The application has to know the details of the
> current 
> BSP and initialize the structure accordingly.
> 
> That means that if I write an application that can run on an STM32 or
> alternatively on some RISC-V based CPU the API will be different. Not
> really portable.
> 

Yes, you are right: rtems_gpio_config_t was defined by each BSP. I have
fixed by making rtems_gpio_t and rtems_gpio_config_t concrete structs
that hold common information among architectures. They also have a
pointer to a bsp-specific structure if required, like in the current
API.
   /**
      * @brief Structure for a GPIO object. It holds information
      *        like port number and pin number/pin mask.
      * 
      */
    typedef struct {
        void *port;                 /* Pointer to the GPIO port */
        uint32_t pin_mask;          /* The pin number or pin mask */
        bool is_expander;           /* If the GPIO is an expander, set to
   true.
                                       Else false. */
        uint32_t expander_id;       /* This field specifies which GPIO
   expander is
                                       in use. In case of using multiple
   expanders,
                                       this field is necessary to handle
   each. */
        void *bsp;                  /* Pointer to BSP-specific data */
    } rtems_gpio_t;
   
   /**
      * @brief Structure for configuration of a GPIO object.
      */
    typedef struct {
        rtems_gpio_pin_mode mode;   /* Pin mode */
        rtems_gpio_pull pull;       /* Pull resistor configuration */
        void *bsp;                  /* Pointer to BSP-specific config */
    } rtems_gpio_config_t;

Hopefully this makes the application code more portable. I have also
updated the blink code:
https://github.com/dtbpkmte/GSoC-2022-RTEMS-Sample-Apps/blob/main/RTEMS_Blink_API/src/init.c
This time, for simple tasks like basic I/O, users won't need to care
about the details of a BSP.

One thing I am not sure about is that do all architectures have ports
and pins for GPIO? I am worried that my structure is still skewed
towards STM32 because I don't have much knowledge about different types
of microcontrollers.

> If you ask me: We have SYSINIT functions for this kind of 
> initializations. If something should be done at about bsp_start, the 
> user can register a function at RTEMS_SYSINIT_BSP_START.

Thank you for the suggestion. This is what I have and it seems to be
working:

   RTEMS_SYSINIT_ITEM(
       rtems_gpio_initialize,
       RTEMS_SYSINIT_BSP_START,
       RTEMS_SYSINIT_ORDER_LAST
   );

> 
> 
> I think I haven't written clearly what I meant: Let's assume a I2C
> chip 
> like the TCA9537 from TI (can be any other GPIO expander from any
> other 
> vendor): You connect it to a I2C bus and control 4 GPIOs with it.
> It's a 
> GPIO so a good GPIO API should be able to handle it.
> 
> In that case the GPIO API would do some I2C transfers under the hood
> if 
> you switch or read a GPIO. So what happens if some error happens
> during 
> one of these transfers. If the only error option is UNSATISFIED, the 
> application can't distinguish the errors. It only knows something
> didn't 
> work. It can't (for example) retry only on certain error cases like
> if 
> the bus is busy.
> 
> Please also note that the I2C GPIO expander is one example where a
> BSP 
> would have two completely different GPIO controllers: A number of 
> integrated ones and a number of I2C ones. A generic API should be
> able 
> to handle that case too.

I understand what you mean now. I have added that capability to by
creating the field is_expander to select between integrated GPIO and
expander. I also have a field called expander_id to select among
multiple expanders. You can see those in the rtems_gpio_t struct above.
The API function prototypes stay the same, but the BSP now need to
implement 2 functions in private: one is the "*_default" function which
controls the built-in GPIO and the other is the "*_ex" function which
controls the expander. Here are the example read and write functions,
which are in bsps/shared/dev/gpio/gpio.c:

   rtems_status_code rtems_gpio_write_pin(rtems_gpio_t *gpiox,
   rtems_gpio_pin_state value) {
       if (gpiox->is_expander) {
           return rtems_gpio_write_pin_ex(gpiox, value);
       } else {
           return rtems_gpio_write_pin_default(gpiox, value);
       }
   }
   
   rtems_status_code rtems_gpio_read_pin(rtems_gpio_t *gpiox,
   rtems_gpio_pin_state *value) {
       if (gpiox->is_expander) {
           return rtems_gpio_read_pin_ex(gpiox, value);
       } else {
           return rtems_gpio_read_pin_default(gpiox, value);
       }
   }
> > 
Please give me more feedback. Thank you very much.

Best,

Duc Doan

>