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
