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 <b...@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.sherr...@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.sherr...@oarcorp.com On-Line Applications Research >> Ask me about RTEMS: a free RTOS Huntsville AL 35805 >> Support Available (256) 722-9985 >> >> -- ragu _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
