[PATCH] (2 commits squashed into one) Beagle BSP for review
Gedare Bloom
gedare at rtems.org
Mon Nov 3 21:32:27 UTC 2014
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
>
>
More information about the devel
mailing list