On 4/24/2015 11:48 AM, Ben Gras wrote: > All, > > Big improvement. The video shows that the bit logic for setting & > clearing is likely sound - for the first byte at least ;-). Good test. > > My comments about the code are > - gpio_select_pin write a whole 0 byte instead of updating a single > bit, as we talked about on IRC > - gedare's suggestion to use a single function to update bits in > registers is good - you can use this for gpio_select_pin, gpio_set and > gpio_clear. > - i don't like the addition of arm_delay() in the bsp code. if it's > not bsp-specific (and it isn't), it shouldn't be there. > - the AM335X-specific code isn't made conditional > > My comment about the API that is: > > I don't like it that hardware-specific info (GPIO base registers) is > somewhat external to the BSP. To my mind, the application should only > deal with bank numbers and pin numbers. So either we need 128 more > #defines of the GPIO_PIN struct in the current vein, meaning inline > struct initializers are passed around (ew) and we rely on quite a lot > of app discipline to keep it clean (ew); or we need an api like > > gpio_config(&gpio_conf); /* retrieve number of gpio banks & pins per > bank, possibly more info */ > gpio_select_pin(&gpio_pin, bank_number, pin_number, DIGITAL_OUTPUT); > /* fill in gpio_pin struct to use this pin */ > gpio_set(&gpio_pin); > gpio_clear(&gpio_pin); > gpio_release_pin(&gpio_pin); /* allows repurposing this pin */ > > this way the configuring of gpio_pin (containing hardware-specific > info) is more internal to the BSP, and the app would have to actively > subvert the api (look in the gpio_pin struct) and is less prone to be > written badly by accident (hardcoding hardware addresses in the app). > > Thoughts anyone? We need a generic rtems_gpio_XXX API set. That was what I took at initial shot at with the multio code. I started mentally with separate APIs completely for analog and discretes but since I was handed a baseboard with 32 bit GPIO pins and an add-on board with ADCs, DACs, and more GPIOs, I needed a unified interface. The collection of various IO types on a board is common.
The application should be completely independent of the BSP. My app will have to know that "the red warning light" is bank 1, pin 3 on board X and bank 7, pin 12 when using another board. That is an application issue. The BSP/system configuration has to present a single API to the app. Remember that the system integrator may add boards. So the "system configuration" may include things not on the baseboard. The SIGAda 2009 presentation I sent privately a few days ago had that configuration and was what I did the API work I did for. > > > On Thu, Apr 23, 2015 at 8:35 PM, Ketul Shah <ketulshah1...@gmail.com> wrote: >> Hi all, >> >> Ben Gras thanks for your comments and encouragement for merging with >> mainline as a motivation for me. >> >> I worked and redeveloped code for gpio driver also tried to approach towards >> common api. I tried to address most of the comments. Herewith I attached my >> patch. Would be happy to hear comments on that. >> >> You can also find out my gist or commit on https://github.com/RTEMS/rtems . >> I have been updating my work on https://github.com/ketul93/RTEMS-on-BBB. >> >> Live demo of the updated code is found on :- https://youtu.be/bXurelOA3i0 >> >> Thanks. >> >> Regards, >> ketul >> >> >> On 20 April 2015 at 19:50, Ben Gras <b...@shrike-systems.com> wrote: >>> All, >>> >>> Good contribution, thank you. >>> >>> For GSOC, this is good proof of being able to progress and get >>> something real working based on documentation. Great! >>> >>> If you want this merged with mainline - which I fully encourage! - >>> then I suggest the following should be added to/changed first: >>> >>> - make the code that uses AM335X (beaglebone) specific registers, >>> AM335X-specific :) i.e. add #if IS_AM335X around code that should >>> only execute there. this BSP can be compiled for 2 different SOCs. >>> - let it control all GPIO's - there are 4 banks of 32 each on the >>> AM335x, but you only let the user control GPIO1. there should be a >>> clean interface to control them all (clean means mostly: without >>> duplicating code) >>> - as Gedare mentioned on IRC, copy the RPi API. this is a first >>> (second?) step to finding a generalized GPIO API. >>> - don't add beagleboard.h, but add your definitions to >>> libcpu/arm/shared/include/am335x.h >>> - the code should use a more consistent indenting style, and make >>> variable names more descriptive than 'tmp'. >>> - don't leave the configure changes in like in acinclude.m4 >>> >>> bonus: >>> - add DM3730 (beagleboard) support. >>> >>> Good luck! >>> >>> >>> On Fri, Apr 17, 2015 at 7:51 PM, Ketul Shah <ketulshah1...@gmail.com> >>> wrote: >>>> Hello, >>>> >>>> I have proposed in GSoC on Beagle BSP Improvements. As starting I >>>> started >>>> working for gpio driver development . At this stage I am able to >>>> demonstrate >>>> USR LEDs pattern. >>>> >>>> Videos of that can be found here on YouTube. >>>> https://youtu.be/B3mSsfo-PAQ & >>>> https://youtu.be/M1aKpOhUvv4. >>>> >>>> Herewith I have attached patch.txt file. Alternatively you can have >>>> https://gist.github.com/ketul93/2e0d2c4b8b586be62e1d that includes the >>>> newly >>>> added files code. I would be happy to hear your reviews and changes on >>>> my >>>> work. >>>> >>>> And also I have been updating my work on >>>> https://github.com/ketul93/RTEMS-on-BBB repo. >>>> >>>> It would be great to have your comments on my proposal. >>>> >>>> Thanks and regards, >>>> ketul >>>> >>>> _______________________________________________ >>>> 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 -- Joel Sherrill, Ph.D. Director of Research & Development joel.sherr...@oarcorp.com On-Line Applications Research Ask me about RTEMS: a free RTOS Huntsville AL 35805 Support Available (256) 722-9985 _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
