All,
I think the API as it is (for digital GPIO's) is pretty close to
generic. I like my suggestion (gpio_* in my previous mail) better as
more generic. (More configuration is hidden from the application.)
Joel I just read your presentation.
I have come up with a minimal GPIO API based on the above (so
ultimately based on the RPI API) that just covers the GPIO digital out
case but allows expansion (with more defines and functions).
https://devel.rtems.org/wiki/TBR/User/BenGras
Is this an idea? Then we can merge this code implementing a reasonably
generic API without having to flesh out the whole thing.
On Fri, Apr 24, 2015 at 7:11 PM, Joel Sherrill
wrote:
>
>
> 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 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 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 o
