[PATCH v2] BSP for TMS570LS31x Hercules Development Kit from TI (TMS570LS3137)
Joel Sherrill
joel.sherrill at oarcorp.com
Mon Aug 18 14:53:59 UTC 2014
On 8/18/2014 8:36 AM, Gedare Bloom wrote:
> On Fri, Aug 15, 2014 at 12:32 PM, Premysl Houdek <kom541000 at gmail.com> wrote:
>> Included variants:
>> tms570ls3137_hdk_intram - place code and data into internal SRAM
>> tms570ls3137_hdk_sdram - place code into external SDRAM and data to SRAM
>> tms570ls3137_hdk - variant prepared for stand-alone RTEMS aplication
>> stored and running directly from flash. Not working yet.
>>
>> Chip initialization code not included in BSP.
>> External startup generated by TI's HalCoGen was used for
>> testing and debugging.
>>
>> More information about TMS570 BSP can be found at
>> http://www.rtems.org/wiki/index.php/Tms570
[...]
Gedare appears to have caught what I saw.
>> diff --git a/c/src/lib/libbsp/arm/tms570/include/tms570-pom.h b/c/src/lib/libbsp/arm/tms570/include/tms570-pom.h
>> new file mode 100644
>> index 0000000..e1bbbcb
>> --- /dev/null
>> +++ b/c/src/lib/libbsp/arm/tms570/include/tms570-pom.h
>> @@ -0,0 +1,101 @@
>> +/**
>> + * @file tms570-pom.h
>> + * @ingroup tms570
>> + * @brief Parameter Overlay Module (POM) header file
>> + */
>> +
>> +/*
>> + * Copyright (c) 2014 Pavel Pisa <pisa at cmp.felk.cvut.cz>
>> + *
>> + * Czech Technical University in Prague
>> + * Zikova 1903/4
>> + * 166 36 Praha 6
>> + * Czech Republic
>> + *
>> + * The license and distribution terms for this file may be
>> + * found in the file LICENSE in this distribution or at
>> + * http://www.rtems.org/license/LICENSE.
>> + */
>> +
>> +
>> +#ifndef LIBBSP_ARM_TMS570_POM_H
>> +#define LIBBSP_ARM_TMS570_POM_H
>> +
>> +#include <stdint.h>
>> +
>> +#ifdef __cplusplus
>> +extern "C" {
>> +#endif /* __cplusplus */
>> +
>> +#define TMS570_POM_REGIONS 32
>> +#define TMS570_POM_GLBCTRL_ENABLE 0x000000a0a
>> +
>> +#define TMS570_POM_REGSIZE_DISABLED 0
>> +#define TMS570_POM_REGSIZE_64B 1
>> +#define TMS570_POM_REGSIZE_128B 2
>> +#define TMS570_POM_REGSIZE_256B 3
>> +#define TMS570_POM_REGSIZE_5120B 4
>> +#define TMS570_POM_REGSIZE_1KB 5
>> +#define TMS570_POM_REGSIZE_2KB 6
>> +#define TMS570_POM_REGSIZE_4KB 7
>> +#define TMS570_POM_REGSIZE_8KB 8
>> +#define TMS570_POM_REGSIZE_16KB 9
>> +#define TMS570_POM_REGSIZE_32KB 0xa
>> +#define TMS570_POM_REGSIZE_64KB 0xb
>> +#define TMS570_POM_REGSIZE_128KB 0xc
>> +#define TMS570_POM_REGSIZE_256KB 0xd
>> +
> These defines would look nicer if you used either decimal or hex.
>
Why is it 5120B?
Are these defines for a field within some register? A comment block
above explaining their purpose would be useful.
FWIW similar advice applies to most any random group of constants.
> +/**
> + * @brief Init function of interrupt module
> + *
> + * Resets vectored interrupt interface to default state.
> + * Disables all interrupts.
> + * Set all sources as IRQ (not FIR).
> + *
> + * @retval RTEMS_SUCCESSFUL All is set
> + */
> +rtems_status_code bsp_interrupt_facility_initialize(void)
> +{
> + void (**vim_vec)(void) = (void (**)(void)) 0xFFF82000;
> + unsigned int value = 0x00010203;
> + unsigned int i = 0;
> + uint32_t sctlr;
> +
> + /* Disable interrupts */
> + for(i = 0; i < 3; i++)
> + TMS570_VIM.REQENACLR[i] = 0xffffffff;
> + /* Map default events on interrupt vectors */
> + for(i=0;i<24;i+=1,value += 0x04040404)
> Add more white space around the parens and operators. Also, I believe
> RTEMS prefers to always use { } even if there is only one line
> following the conditional/loop statement. Perhaps this is not
> explicitly stated though.
It isn't but it tends to avoid stupid mistakes. :)
I am not even sure it is reliably followed but for for and while loops,
it really should.
>> + TMS570_VIM.CHANCTRL[i] = value;
>> + /* Set all vectors as IRQ (not FIR) */
>> + TMS570_VIM.FIRQPR[0] = 3;
>> + TMS570_VIM.FIRQPR[1] = 0;
>> + TMS570_VIM.FIRQPR[2] = 0;
>> +
>> + /*_CPU_ISR_install_vector(ARM_EXCEPTION_IRQ, _ARMV4_Exception_interrupt, NULL);*/
>> + for(i = 0; i <= 94; i++)
>> + vim_vec[i] = _ARMV4_Exception_interrupt;
>> +
>> + /* Clear bit VE in SCTLR register to not use VIM IRQ exception bypass*/
>> + asm volatile ("mrc p15, 0, %0, c1, c0, 0\n": "=r" (sctlr));
>> + sctlr &= ~(1 << 24);
>> + asm volatile ("mcr p15, 0, %0, c1, c0, 0\n": : "r" (sctlr));
>> +
>> + return RTEMS_SUCCESSFUL;
>> +}
>> +
>> +
>> +
> Remove extra blank lines.
>
>
>> diff --git a/c/src/lib/libbsp/arm/tms570/network/tms570-ethernet.c b/c/src/lib/libbsp/arm/tms570/network/tms570-ethernet.c
>> new file mode 100644
>> index 0000000..e69de29
>> diff --git a/c/src/lib/libbsp/arm/tms570/network/tms570-ethernet.h b/c/src/lib/libbsp/arm/tms570/network/tms570-ethernet.h
>> new file mode 100644
>> index 0000000..e69de29
> Empty files?
>
>> diff --git a/c/src/lib/libbsp/arm/tms570/pom/tms570-pom.c b/c/src/lib/libbsp/arm/tms570/pom/tms570-pom.c
>> new file mode 100644
>> index 0000000..be2d904
>> --- /dev/null
>> +++ b/c/src/lib/libbsp/arm/tms570/pom/tms570-pom.c
>> @@ -0,0 +1,100 @@
>> +/**
>> + * @file tms570-pom.c
>> + *
>> + * @ingroup tms570
>> + *
>> + * @brief TMS570 Parameter Overlay Module functions definitions.
>> + */
>> +
>> + /*
>> + * Copyright (c) 2014 Pavel Pisa <pisa at cmp.felk.cvut.cz>
>> + *
>> + * Czech Technical University in Prague
>> + * Zikova 1903/4
>> + * 166 36 Praha 6
>> + * Czech Republic
>> + *
>> + * The license and distribution terms for this file may be
>> + * found in the file LICENSE in this distribution or at
>> + * http://www.rtems.org/license/LICENSE.
>> + */
>> +
>> +#include <stdint.h>
>> +#include <bsp/tms570-pom.h>
>> +#include <bsp/linker-symbols.h>
>> +#include <bsp.h>
>> +
>> +/**
>> + * @brief Prints part of the memory
>> + *
>> + * debug function
>> + *
>> + * @retval 0
>> + */
>> +int mem_dump(void *buf, unsigned long start, unsigned long len, int blen)
>> +{
>> + unsigned long addr=start;
>> + volatile unsigned char *p=buf;
>> + int i;
>> +
>> + while ( len ){
>> + printk("%08lX:",addr);
>> + i=len>16?16:len;
>> + addr+=i;
>> + len-=i;
>> + while(i>0){
> Add whitespace.
>
>> + i -= blen;
>> + switch(blen){
>> + case 4:
>> + printk("%08lX%c",(unsigned long)*(volatile uint32_t*)p,i>0?' ':'\n');
>> + break;
>> + case 2:
>> + printk("%04X%c",*(volatile uint16_t*)p,i>0?' ':'\n');
>> + break;
>> + default:
>> + printk("%02X%c",*(volatile uint8_t*)(p),i>0?' ':'\n');
>> + break;
>> + }
>> + p += blen;
>> + }
>> + }
>> + return 0;
>> +}
>> +
The BSP shouldn't be adding this. Is the standard dump buffer code not
sufficient?
I don't recall if it lets you pass in your own fprintf style method like
cpu, stack and period
usage reporting methods. But if it isn't sufficient, we need to fix it.
This should be
generally useful code in cpukit, not in a BSP.
> -Gedare
> _______________________________________________
> 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