On Sun, Jun 21, 2015 at 7:04 PM, André Marques
<andre.lousa.marq...@gmail.com> wrote:
> 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?
>
Here if you're confident it's ready.

>>
>>> 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