[PATCH] (2 commits squashed into one) Beagle BSP for review

Mon Nov 3 22:00:13 UTC 2014

Good idea, I did it.

On Mon, Nov 3, 2014 at 10:32 PM, Gedare Bloom <gedare at rtems.org> wrote:

> Please send a note to rtems-users, some may be interested to hear this
> addition. -Gedare
>
> On Mon, Nov 3, 2014 at 4:28 PM, Ben Gras <beng at shrike-systems.com> wrote:
> > Indeed. I did right away verify I can build & run & test everything for
> the
> > beaglebones and the bbxm from mainline. So that seems to have gone fine.
> >
> > The supporting tools and RSB stuff I am in contact with Chris about.
> >
> >
> > On Mon, Nov 3, 2014 at 10:23 PM, Joel Sherrill <
> joel.sherrill at oarcorp.com>
> > wrote:
> >>
> >>
> >> 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 <beng at 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 <gedare at 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 <beng at 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 <beng at 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 <gedare at 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 <
> beng at 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 <gedare at 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
> >>>> >>> >> <beng at 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 at rtems.org
> >>>> >>> >> > http://lists.rtems.org/mailman/listinfo/devel
> >>>> >>> >
> >>>> >>> >
> >>>> >>
> >>>> >>
> >>>> >
> >>>
> >>>
> >>
> >>
> >> --
> >> 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
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20141103/9de69117/attachment-0002.html>