Hello Duc,

Am 02.08.22 um 09:17 schrieb Duc Doan:
[...]
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)?

From my point of view, that would be better. You maybe could think about hinting on that point in the meeting tomorrow. Maybe someone else will add an oppinion too.

Best regards

Christian


Best,

Duc


[...]
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Reply via email to