On 16-06-2015 17:54, Gedare Bloom wrote:
On Tue, Jun 16, 2015 at 8:14 AM, André Marques
<andre.lousa.marq...@gmail.com> wrote:
Hello ,

Thank you for the responses. Sorry for the late response and lengthy mail.

At this point the features mentioned in the blog post are almost done in the
code (apart from some details). There are some differences from what is
mentioned in the blog, but the overall idea still stands (an blog update is
in order, as well as more documentation in the code).

One major change is related with the interaction with the API. It currently
maintains a data structure which holds the current GPIO status (which pins
are used, what interrupt handlers they have associated, ...), so an
application can either fill a different structure to configure a pin, which
is passed to the API through an API call, or configure the pin directly
through API calls.

The struct approach can be used either to define a new pin to be requested,
or to update its configuration, all done by updating/changing the struct and
doing the same function call to the API. Since the structs can be defined on
a separate file, the application code can be the same across different
platforms, as the only diference is the configuration file that is
used/called. The platform specific configuration (which pin in y platform
corresponds to the warning led?) for an application would then be reduced to
file management.

The API call that does the processing of these structs parses them and does
the necessary API calls to setup/update the GPIO hardware as needed.

By having both a struct based interface and direct API call options an user
can choose what is best for their need. The direct API calls can be
specially useful for an interactive usage, through a shell program.

The current implementation for the above can be seen in [1], [2] and [3].
Note that the gpio.c and gpio.h can be anywhere else at this point, as they
do not have any rpi code anymore (another question is where they could go
then?).

I think the first place to migrate shared code is libbsp/shared. After
that, we can consider whether the code is generic enough to put in
cpukit, which is where other driver frameworks (i2c, libdrvmgr) have
landed recently.

I have moved it to libbsp/shared, and at this point I think we can start the code review process. I have cleaned the code and updated the doxygen and comments, should I post a patch here in two parts (one for the generic API, and other for the RPI implementation) for review, or should it be done in github?


A pending issue is related with pin definitions. It would be useful if after
the pin request to the API, the application could have a gpio_pin_id which
would be used as a reference to an application pin.

Small example with direct API call:

gpio_pin_id warning_led;

gpio_request_pin(RPI_GPIO_23, &warning_led, DIGITAL_OUT, ..);

gpio_set(warning_led);

The RPI_GPIO_23 would be the platform pin number (the API calculates the
bank and pin number from this), but after the request the application uses
the gpio_pin_id to refer to it. The gpio_pin_id would be a simple struct
filled by the API with the calculated bank and pin numbers, which no one
else is supposed (or have the need to) to modify/use this struct other than
to pass it to the API to refer to a pin. This would also avoid the constant
bank/pin numbers calculation and pin number boundary validation that is made
currenty in every API call.

That makes sense. You could define an opaque type to store the
identifier. However, if it is a struct, passing it by value could be
an expensive operation, so care should be taken there. (If you can fit
it in 32-bits, you can union the gpio_pin_id with a uint32_t)
I left this out for the moment.

A final note would be the need for multiple pin operations. The rpi and most
platforms (I suppose) allow several pins to be defined on a single call, by
writting a more complete bitmask to the registers. This could be a challenge
with the struct approach (maybe by having pin arrays, instead of a single
pin in the struct, if they all share the same configuration). As for direct
pin calls maybe through variadic functions.
I just got done writing about zero-length arrays for Yang Qiao
[https://github.com/yangqiao/rtems/commit/38c26a49126e5cff92ae389dba252cb180362c90#commitcomment-11707035],
but perhaps you could use something similar albeit simpler in this
case too. You define the pin as a zero-length array (it should always
be at least 1 though), and then include the number of pins in the
struct.

[1] -
https://github.com/asuol/rtems/tree/GPIO_API/c/src/lib/libbsp/arm/raspberrypi/gpio
[2] -
https://github.com/asuol/rtems/blob/GPIO_API/c/src/lib/libbsp/arm/raspberrypi/include/gpio.h
[3] -
https://github.com/asuol/rtems/blob/GPIO_API/c/src/lib/libbsp/arm/raspberrypi/include/rpi-gpio.h

--André Marques.


On 11-06-2015 15:13, Gedare Bloom wrote:
On Wed, Jun 10, 2015 at 7:09 PM, Chris Johns <chr...@rtems.org> wrote:
On 11/06/2015 3:01 am, Gedare Bloom wrote:
On Mon, Jun 8, 2015 at 5:44 AM, André Marques
<andre.lousa.marq...@gmail.com> wrote:
Hello,

I have just updated my GSOC blog [1] with a detailed post about how a
rtems-wide GPIO API could look like, and at the same time exposing the
current features of the Raspberry Pi GPIO API and how it can evolve to
that
level.

I tried to make it as generic and flexible as possible, but that can be
hard
with the number of platforms where rtems can be used. Api and method
naming
were somewhat overlooked, as well as the definition of possible error
codes
since I am not sure if it would be correct to have a set of error codes
for
this API, or if it should use rtems_status_code, or other.

Current code for the Raspberry Pi GPIO API can be looked at in [2],
where I
am currently carving out the rpi specific code.

I tried to be as clear as possible in the blog, and now I would like to
ask
any interested party to have a look and hopefully point failure points
and
suggestions, or ask for clarifications. It would also be interesting to
hear
the community expectations towards an API such as this one.

Great write-up.
I agree, it is a nice write up.

Here are some comments/questions:
- The "user application must fill a gpio_pin struct for every pin it
needs" should probably be done through some set of API calls that help
filling out that struct. I'd imagine some alloc with initialization,
and dealloc, with most of the complexity in alloc/init.
Being const lets you fill them out when coding as a table which I have
found easy to review plus being const has the advantage on small RAM
devices of only using ROM type storage which they usually have more of.

I think an API to fill the struct in would be confusing and so think at
this point in time we should limit what we do.

OK that is fair. On the other hand, anything we can do to make the
const initializer easier would be good. I especially was concerned by
#ifdef's inside the initializer.

I currently have 3 Zynq variants with similar IO requirements with very
different pin selections due to easier PCB routing and I have a separate
C file with pin definitions per variant. The resulting user code is
clean and simple.

That makes much more sense, where the struct definition will be pulled
in through the build system logic to choose the correct file to
compile.

- The bsp_specific pointer might be better implemented with a
zero-length array
[https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html]. Since you embed
the type of the struct, you can tell whether that array should have
any data in it or not.
- I wonder if eventually we can refactor all this to work with the
libdrvmgr. This is a long-term question but might be one worth
thinking about now. (libdrvmgr mainly focuses on providing
abstractions for device drivers that attached to bus-based i/o
protocols. You may like to take a look at its documentation.)
I have not looked at that API so I cannot comment but I wonder if this
is outside the scope of this GSoC project.

Definitely outside the scope.

Chris

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

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

Reply via email to