[PATCH] Beagle BSP Improvements (GPIO driver)

Fri Apr 24 17:11:51 UTC 2015

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 <ketulshah1993 at 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 <beng at 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 <ketulshah1993 at 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 at rtems.org
>>>> http://lists.rtems.org/mailman/listinfo/devel
>>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel

-- 
Joel Sherrill, Ph.D.             Director of Research & Development
joel.sherrill at OARcorp.com        On-Line Applications Research
Ask me about RTEMS: a free RTOS  Huntsville AL 35805
Support Available                (256) 722-9985