On 11/3/2014 3:06 PM, Ben Gras wrote: > All, > > > Joel merged these and I updated my blog post to reflect the mainline > repo. Thanks Joel! > Thank you Ben for the nice submission!!!
Now to make sure it is reproducible from here and we have merged all the bits into the tools, etc. > On Mon, Nov 3, 2014 at 8:40 PM, Ben Gras <b...@shrike-systems.com > <mailto:b...@shrike-systems.com>> wrote: > > All, > > I have new patches with some last-minute smoothings added; removed > obsolete beagle.cfg, TODO, and separated the more generic ARM > headers into a separate commit. The new 3 commits are attached > (and in my RTEMS github repo). > > Gedare, there is also a diff w.r.t. the previous submission > attached as requested. > > > > On Mon, Nov 3, 2014 at 3:01 PM, Gedare Bloom <ged...@rtems.org > <mailto:ged...@rtems.org>> wrote: > > Hi, > > I don't have time to review, but am OK in principle with > committing > this code as it is tested, with the caveat that my previous > comments > be addressed post-merge. > > If you have a diff / commits on top of what you sent before, > I'd be > glad to give those a quick look. > > Thanks for your contribution! > Gedare > > On Mon, Nov 3, 2014 at 7:20 AM, Ben Gras > <b...@shrike-systems.com <mailto:b...@shrike-systems.com>> wrote: > > All, > > > > Ok, as promised, I rebased and re-tested and have found & > included a > > portable way of making the SD card image (included in > sdcard.sh), to be > > merged with RSB (i.e. some of the tools sdcard.sh relies on > are missing in > > mainline RSB). Some of Gedare's initial feedback is > processed thanks to > > Brandon Matthews. It's tested to run on the original > beaglebone, beaglebone > > black and qemu linaro. (The assumption is it'll run on the > bbxm hardware as > > well as it was before rebasing.) > > > > The result is split into 2 patches to show what was Claas's > initial work. > > This makes them a bit unreadable for the final result from > the patches > > unfortunately. > > > > As before, see > > > > http://www.shrike-systems.com/beagleboard-xm-beaglebone-black-and-everything-else-rtems-on-the-beagles.html > > on how to build all the tools, RTEMS executables, sdcard > images, and run the > > test set from linaro qemu. > > > > > > > > On Sat, Aug 30, 2014 at 5:50 PM, Ben Gras > <b...@shrike-systems.com <mailto:b...@shrike-systems.com>> wrote: > >> > >> All, > >> > >> OK, that seems like a fruitful way to proceed to me. > >> > >> Then I will do some minor cleanups, rebase, do all the > tests again, and > >> re-submit. There's just one problem I know of that I want > to fix before the > >> first commit happens, and that is that the FAT fs made by > mtools doesn't > >> boot on the HW it seems. (It does on the emulator.) A > last-minute change - > >> switching to mtools instead of dosfstools to use to make > the SD card image > >> because the latter is so linux-only. > >> > >> Stay tuned. > >> > >> > >> > >> > >> On Wed, Aug 20, 2014 at 4:20 PM, Gedare Bloom > <ged...@rtems.org <mailto:ged...@rtems.org>> wrote: > >>> > >>> Ben, As far as getting this merged, all of my comments can > be done as > >>> a follow-on commit. -Gedare > >>> > >>> On Thu, Jul 24, 2014 at 4:28 PM, Ben Gras > <b...@shrike-systems.com <mailto:b...@shrike-systems.com>> > >>> wrote: > >>> > Thanks for the fast & detailed review. Let me get back > to it/you. > >>> > > >>> > In the meantime, any other feedback? From anyone I mean. > >>> > > >>> > > >>> > > >>> > On Thu, Jul 24, 2014 at 4:45 PM, Gedare Bloom > <ged...@rtems.org <mailto:ged...@rtems.org>> wrote: > >>> >> > >>> >> Hi Ben, > >>> >> Great work. I have a few comments. I skipped the i2c.h > and i2c.c > >>> >> files. Most of my comments are about style and a few > requests to > >>> >> refactor some of the larger files. The refactoring can > be added to > >>> >> your TODO if you like. Please fix the style issues if > it is not a > >>> >> burden. > >>> >> > >>> >> +++ b/c/src/lib/libbsp/arm/beagle/README > >>> >> +$ ../claas-rtems/configure --target=arm-rtems4.11 > >>> >> --enable-rtemsbsp="beaglebonewhite beagleboardxm" > >>> >> Replace claas-rtems with rtems. If RSB support is > available, make a > >>> >> note about it. > >>> >> > >>> >> +++ b/c/src/lib/libbsp/arm/beagle/TODO > >>> >> [...] > >>> >> open: > >>> >> + . how to handle the interrupt? > >>> >> > >>> >> What does this mean? > >>> >> > >>> >> +++ b/c/src/lib/libbsp/arm/beagle/clock.c > >>> >> Why is the entire file ifdef'd on ARM_MULTILIB_ARCH_V4? > >>> >> > >>> >> It might be sensible to put the struct definitions in a > .h file if > >>> >> these omap registers might be re-usable. > >>> >> > >>> >> +static struct omap_timer_registers regs_v2 = { > >>> >> This might be better to put behind an #if IS_AM335X > since it is not > >>> >> used otherwise? > >>> >> > >>> >> + > >>> >> + > >>> >> + > >>> >> Avoid more than one blank line in a row. > >>> >> > >>> >> +static int done = 0; > >>> >> It would be nice if you got rid of this, but otherwise > give it a more > >>> >> useful name like "mmio_init_done" > >>> >> > >>> >> +static void > beagle_clock_handler_install(rtems_interrupt_handler isr) > >>> >> + if (sc != RTEMS_SUCCESSFUL) { > >>> >> + rtems_fatal_error_occurred(0xdeadbeef); > >>> >> I think there is some capabilities in ARM for bsp fatal > error codes > >>> >> now. They should be preferred to be used to help debug > these fatal > >>> >> conditions. > >>> >> > >>> >> +static inline uint32_t > beagle_clock_nanoseconds_since_last_tick(void) > >>> >> + return (read_frc() - (uint64_t) > last_tick_nanoseconds) * 1000000000 > >>> >> / FRCLOCK_HZ; > >>> >> This line is > 80 characters, please break it or shrink it. > >>> >> > >>> >> +++ b/c/src/lib/libbsp/arm/beagle/console/console-config.c > >>> >> +#define CONSOLE_UART_LSR (*(volatile unsigned int > >>> >> *)(BSP_CONSOLE_UART_BASE+0x14)) > >>> >> Line > 80 characters, even with the spacing modified. > >>> >> > >>> >> +static void beagle_console_init(void) > >>> >> > >>> >> + while ((CONSOLE_SYSS & 1) == 0) > >>> >> + ; > >>> >> Is this a fatal loop or is it waiting for hardware to > clear something? > >>> >> > >>> >> + if ((CONSOLE_LSR & (CONSOLE_LSR_THRE | > CONSOLE_LSR_TEMT)) == > >>> >> CONSOLE_LSR_THRE) { > >>> >> Again > 80 characters. Is the test logically equivalent > to: if ( > >>> >> (CONSOLE_LSR & CONSOLE_LSR_THRE) == CONSOLE_LSR_THRE) > >>> >> > >>> >> + while ((CONSOLE_LSR & CONSOLE_LSR_TEMT) == 0) > >>> >> + ; > >>> >> Is this a fatal loop or is it waiting for hardware? > >>> >> > >>> >> +++ b/c/src/lib/libbsp/arm/beagle/include/bsp.h > >>> >> This bsp.h is really long. Probably it should be > refactored into other > >>> >> headers, including non-public ones. > >>> >> > >>> >> +static inline void dsb(void) > >>> >> +{ > >>> >> + asm volatile("dsb" : : : "memory"); > >>> >> Fix the indentation. > >>> >> > >>> >> +static inline void flush_data_cache(void) > >>> >> Perhaps this should be using > _CPU_cache_flush_entire_data()? Perhaps > >>> >> there is a difference in that the cache manager code > flushes and > >>> >> "cleans" the cache... > >>> >> > >>> >> + > >>> >> + > >>> >> + > >>> >> + > >>> >> Excess newlines. Done a few places in this file. > >>> >> > >>> >> The comments following the defines for various > AM33X_INT_ values go > >>> >> off the end of the 80 column character width. Same for > some other > >>> >> comments following defines for OMAP3_TIMER, > AM33X_DMTIMER1, and > >>> >> AM335X_TIMER_. And further below for the CM_ WKUP and > CM_PER_TIMER7 > >>> >> defines, and CLKSEL_TIMER1MS_CLK_SEL_SEL5. > >>> >> > >>> >> +#define OMAP3_TCLR_AR (1 << 1) /* Autoreload or > one-shot mode > >>> >> */ > >>> >> +#define OMAP3_TCLR_PRE (1 << 5) /* Prescaler on */ > >>> >> +#define OMAP3_TCLR_PTV 2 > >>> >> This PTV is odd compared to the other defines here. Is > it 2 == (1<<1), > >>> >> or is there a typo here? > >>> >> > >>> >> Tabs are used in the OMAP3_CM_ defines, it should be > space characters. > >>> >> Also tabs are used in the read/write actlr, ttbcr, > dacr, rrbr0 > >>> >> functions and the refresh_tlb function. > >>> >> > >>> >> +/* i2c stuff */ > >>> >> +typedef struct { > >>> >> ... > >>> >> +} beagle_i2c; > >>> >> Shouldn't this go in beagle/include/i2c.h? > >>> >> > >>> >> All of this mmu handling code should be refactored. > Where possible, it > >>> >> should use the existing code in arm-cp15.h > >>> >> > >>> >> +++ b/c/src/lib/libbsp/arm/beagle/include/i2c.h > >>> >> This header defines static, non-inline functions. This > doesn't make > >>> >> sense. > >>> >> > >>> >> +++ b/c/src/lib/libbsp/arm/beagle/irq.c > >>> >> +static int irqs_enabled[BSP_INTERRUPT_VECTOR_MAX+1]; > >>> >> This is an array of 512 bytes. You could use a bit > vector comprising 4 > >>> >> unsigned ints for the same purpose. > >>> >> > >>> >> +volatile static int level = 0; > >>> >> Unused variable? > >>> >> > >>> >> +static uint32_t get_mir_reg(int vector, uint32_t *mask) > >>> >> + if(vector < 0) while(1) ; > >>> >> Make this a fatal error? > >>> >> > >>> >> + if(vector < 32) return OMAP3_INTCPS_MIR0; > >>> >> + if(vector < 32) return OMAP3_INTCPS_MIR0; > >>> >> duplicate code. > >>> >> > >>> >> + while(1) ; > >>> >> Make this a fatal error? > >>> >> > >>> >> +rtems_status_code bsp_interrupt_facility_initialize(void) > >>> >> + mmio_write(omap_intr.base + OMAP3_INTCPS_SYSCONFIG, > >>> >> OMAP3_SYSCONFIG_AUTOIDLE); > >>> >> Line length > 80. > >>> >> > >>> >> +++ b/c/src/lib/libbsp/arm/beagle/startup/bspstartmmu.c > >>> >> > >>> >> +//static uint32_t pagetable[ARM_SECTIONS] > __attribute__((aligned > >>> >> (1024*16))); > >>> >> commented-out.. delete it? > >>> >> > >>> >> +BSP_START_TEXT_SECTION void > beagle_setup_mmu_and_cache(void) > >>> >> __attribute__ ((weak)); > >>> >> More than 80 characters. > >>> >> > >>> >> diff --git a/c/src/lib/libbsp/bfin/acinclude.m4 > >>> >> b/c/src/lib/libbsp/bfin/acinclude.m4 > >>> >> index ab6082e..828fd89 100644 > >>> >> --- a/c/src/lib/libbsp/bfin/acinclude.m4 > >>> >> +++ b/c/src/lib/libbsp/bfin/acinclude.m4 > >>> >> diff --git a/c/src/lib/libbsp/powerpc/acinclude.m4 > >>> >> b/c/src/lib/libbsp/powerpc/acinclude.m4 > >>> >> index 6442399..e46fa2b 100644 > >>> >> --- a/c/src/lib/libbsp/powerpc/acinclude.m4 > >>> >> +++ b/c/src/lib/libbsp/powerpc/acinclude.m4 > >>> >> Don't include these changes. Check your tool versions, > and if the > >>> >> correct version of tools does this, provide a separate > patch for > >>> >> generated files. > >>> >> > >>> >> -Gedare > >>> >> > >>> >> On Wed, Jul 23, 2014 at 9:00 PM, Ben Gras > <b...@shrike-systems.com <mailto:b...@shrike-systems.com>> > >>> >> wrote: > >>> >> > All, > >>> >> > > >>> >> > Full details on how to reproduce all the work from source > >>> >> > repositories > >>> >> > to > >>> >> > scripts & utilities to write a complete sd card > booting RTEMS and > >>> >> > test > >>> >> > the > >>> >> > whole thing: > >>> >> > > >>> >> > > >>> >> > > >>> >> > > > http://www.shrike-systems.com/beagleboard-xm-beaglebone-black-and-everything-else-rtems-on-the-beagles.html > >>> >> > > >>> >> > I am submitting the attached patch for review for > merging. If > >>> >> > accepted > >>> >> > for > >>> >> > merging, please use the top two commits on > >>> >> > > >>> >> > https://github.com/bengras/rtems/tree/beaglebone-wip > >>> >> > > >>> >> > which have the same net effect but preserve Claas' > work because of > >>> >> > the > >>> >> > earlier commit. The squashed version attached is for > more convenient > >>> >> > review. > >>> >> > > >>> >> > I was ironing out more wrinkles but given recent > interest it seems > >>> >> > smarter > >>> >> > to merge sooner and keep polishing from mainline. > Nevertheless I > >>> >> > have > >>> >> > put a > >>> >> > lot of work into getting it into good shape already. > >>> >> > > >>> >> > I have rebased everything on the very latest master > and verified > >>> >> > > >>> >> > That building all the tools and utilities from > scratch work, using > >>> >> > the > >>> >> > RTEMS > >>> >> > Source Builder repository (Ubuntu + FreeBSD). > >>> >> > That building the beaglebone and bbxm BSPs and > linking them with all > >>> >> > the > >>> >> > testsuite programs works (Ubuntu + FreeBSD). > >>> >> > That the beaglexm-emulating linaro qemu executes all > of those tests > >>> >> > properly, invoked using a single command line with > the scripts in > >>> >> > the > >>> >> > RTEMS > >>> >> > tools repository, even though not all pass currently > (Ubuntu + > >>> >> > FreeBSD). > >>> >> > That loading & running over JTAG works, both > interactively with gdb > >>> >> > and > >>> >> > in a > >>> >> > batch using gdb and the test runner. > >>> >> > That running RTEMS executables using u-boot on the > beaglebones from > >>> >> > sd > >>> >> > card > >>> >> > work; both with and without MMU enabled at RTEMS > start time. > >>> >> > That Claas' earlier commit builds. > >>> >> > > >>> >> > Thanks so far to Chris and Brandon for help, support, > instructions > >>> >> > and > >>> >> > advice in various forms :) > >>> >> > > >>> >> > Test results on qemu: > >>> >> > Passed: 497 Failed: 3 Timeouts: 1 Invalid: 0 > >>> >> > > >>> >> > The test results on bbxm over jtag (older): > >>> >> > Passed: 475 Failed: 7 Timeouts: 10 Invalid: 0 > >>> >> > > >>> >> > I want to iron out more wrinkles and build support > (ethernet) but > >>> >> > giving > >>> >> > the > >>> >> > bsp more exposure and having it in mainline so i > don't have to keep > >>> >> > rebasing > >>> >> > & testing would be nice at this point. > >>> >> > > >>> >> > > >>> >> > > >>> >> > > >>> >> > _______________________________________________ > >>> >> > devel mailing list > >>> >> > devel@rtems.org <mailto:devel@rtems.org> > >>> >> > http://lists.rtems.org/mailman/listinfo/devel > >>> > > >>> > > >> > >> > > > > > -- 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
_______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel