Beagle bsp improvements RTC driver for beaglebone black

[Ben Gras]

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
>
>