Am 21.06.22 um 16:31 schrieb 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
<https://github.com/dtbpkmte/GSoC-2022-RTEMS-Sample-Apps/blob/main/RTEMS_Blink_API/src/init.c>
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.
I know that it is difficult in this case. On the one hand an API should
be portable. On the other hand it should be able to handle very
different hardware. And all that with the lowest possible overhead. In
the end it will be some compromise.
Your goal is to either offer a better compromise than the API that is
currently used for beagle and RPi. Or as an alternative: Don't try to go
for an universal one and focus on your current device like a lot of the
BSPs do. I would prefer a good universal API (better than the Beagle /
RPi one (*)) but I know the problems so I could live with a device
specific too. But note that this is my personal opinion. Others might
have a different one.
(*) In case someone didn't see that (I think it was on discord): The
Beagle / RPi API has two weaknesses in my point of view:
1. It mixes GPIO with Pinmuxing which is not true for all chips. For
example the whole Freescale / NXP devices tend to have separate
controllers for that. Having two APIs for that would be better in my
point of view.
2. It is written with a quite fixed structure in mind. Controllers that
don't have that structure need bigger workarounds which makes the API slow.
. 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.
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.
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.
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.
Best regards
Christian
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
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
