Beagle bsp improvements RTC driver for beaglebone black

[ragu nath]

Hi All,

My semester exams have started and  I could not find enough time to
work on this.
I have addressed most of the comments but testing is pending.
Once I get time, I will finish it and submit the patch.  Sorry for the delay.

Regards,
Ragunath


On Sun, Apr 5, 2015 at 2:22 AM, Ben Gras <beng at shrike-systems.com> wrote:
> All,
>
> I built & tested the change based on current master. Looks great,
> ragunath! I agree it's worth addressing Joel's comments. I have two
> more:
>
>  - please check in the device read/write functions that you're getting
> the 'minor' argument that you expect (0), and not return valid data
> otherwise.
>  - git reports some spurious white space
>
> can you address these & resubmit? I'll be happy to merge then. Thanks
> for your contribution, very good companion to your GSOC proposal!
>
>
>
> On Wed, Apr 1, 2015 at 7:48 PM, Joel Sherrill <joel.sherrill at oarcorp.com> wrote:
>> I noticed this wasn't merged.
>>
>> Ben.. ?
>>
>> some comments interspersed.
>>
>> On 3/10/2015 4:21 PM, ragu nath wrote:
>>> Hi All,
>>>
>>> I was exploring the beagle code base to find a task to get hands on
>>> experience with RTEMS internals. I found that "Real time Clock (RTC)" support is
>>> missing for beaglebone. I made the effort to add the driver for the device.
>>> Included the patch for the driver. RTEMS already have a RTC framework.
>>> I defined the necessary structures to hook the driver to the framework.
>>>
>>> Enabled the driver by defining the macro
>>> "CONFIGURE_APPLICATION_NEEDS_RTC_DRIVER" and tested the following.
>>> Testing is done from the shell using "date" and "rtc" commands.
>>> 1. Able to read the time from rtc.
>>> 2. Able to set the time
>>> 3. Time values are preserved between reboots
>>>
>>> Pls review the patch and let me know if you have any comments.
>>>
>>> I have following doubts
>>>
>>> 1. The support is applicable only to beaglebone black. It is not valid
>>> for beagleboard. Both of them seem to have same codebase.  Is there any compile
>>> time flags available to not compile the driver for beagleboard?
>>
>> Look in the configure.ac in the BSP. There is already a cpp define generated
>> based on the board variant for DM3730 or AM335x. Do you need more than
>> that? I am not an expert on the differences between the various models so
>> this is just a question.
>>
>> If the RTC has to be ignored completely for some variants, the configure.ac
>> logic can be enhanced to generate a conditional the Makefile.am can use
>> to conditionally build some files.
>>
>>
>>> 2. What is procedure to commit the change (In case the patch is
>>> accepted)? Do we need to open a bug to track the changes?
>> This normally would have been more than enough. But pinging if you don't
>> get a response in a reasonable length of time.
>>
>> The patch didn't get included in my response so here are some odd comments.
>>
>> + "disabl" check spelling.
>> + "Steps to enable RTC" should have "*" at front of each line in comment
>> block
>> + spaces around equal signs
>> + Don't print from driver
>> + I see an unneeded blank line before closing } in some methods
>> + Put ";" on busy spin loops on a separate line so it is obvious
>>    - if there is a known number of maximum spins, it should not
>>      lock up.
>>   - the spins do look the same so moving them to a subroutine
>>     and adding a maximum spin count would be a nice way to
>>    make the code more robust and clearer.
>> + Rather than RTEMS_DEBUG, may want to use a BB_DEBUG
>>    or similar for debug code.
>> + Watch columns lining up. RTC_Table comments don't line up
>>    and value column in register macros in .h don't either.
>> +
>>> Thanks,
>>> Ragunath
>>
>> --
>> 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
>>
>>



-- 
ragu