[rpi bsp] add cmdline options

[Pavel Pisa]

Hello Qiao Yang,

On Tuesday 28 of July 2015 01:35:45 Pavel Pisa wrote:
> Hello Qiao Yang,
>
> On Friday 24 of July 2015 18:04:47 QIAO YANG wrote:
> > Some updates for you. I've found out that my previous problem with
> > cmdline due to my minicom didn't wrap up the lines.... I've chosen the
> > interface of vc to retrieve cmdline so that we don't need to parse the
> > atag nor devicetree. Now we can use --video=...  to choose resolution and
> > --console=... to choose console.

I have got to little more testing. Ticker runs correctly
with console on serial port and even with graphics console.
But option --console=fbcons works only if it is the last option
on the line in cmdline.txt. Problem is in the comparison because
if there are more options then string provided by rpi_cmdline_arg()
continues after "fbcons" by blank character. I think that
copy of the string to the user provided buffer is not necessary
in rpi_cmdline_arg(). It would bring up issues with space enough
to fit data etc. I have use simple strncmp to resolve issue.
But something more elegant can be provided.

diff --git a/c/src/lib/libbsp/arm/raspberrypi/console/console_select.c b/c/src/lib/libbsp/arm/raspberrypi
index 74cf78e..ad24d33 100644
--- a/c/src/lib/libbsp/arm/raspberrypi/console/console_select.c
+++ b/c/src/lib/libbsp/arm/raspberrypi/console/console_select.c
@@ -91,7 +91,7 @@ void bsp_console_select(void)
    if (opt)
   {
       opt += sizeof("--console=")-1;
-      if (strcmp(opt,"fbcons") == 0)
+      if (strncmp(opt,"fbcons", sizeof("fbcons"-1)) == 0)
       {
         Console_Port_Minor = BSP_CONSOLE_FB;
         BSPPrintkPort      = BSP_CONSOLE_FB;
@@ -99,8 +99,8 @@ void bsp_console_select(void)
   }
   else
   {
-    Console_Port_Minor = BSP_CONSOLE_FB;
-    BSPPrintkPort      = BSP_CONSOLE_FB;
+    Console_Port_Minor = BSP_CONSOLE_UART0;
+    BSPPrintkPort      = BSP_CONSOLE_UART0;
   }

   /*

> As for the interface to command line through VC, I expect that
> this interface allows access only to value set in config.txt
> but it is not changed by U-boot if used. If that is true
> then use of ATAG or device tree would be better for flexibility.
> It would be reusable on other ARM targets as well.
> But that this can be tried in some follow up project.

It seems to me that behavior is as expected, VideoCore
returns what it parsed from cmdline.txt but arguments
set by U-boot are different. So long term term solution
is to implement shared ARM BSP atags and devicetree parser.
It can be minimal for start only to locate kernel command
line. It would help to other ARM BSPs in general.

MMU setup seems to be OK for me, but what I really
dislike is duplicate of arm-cp15-start.h to RPi.
Shared one should be used.

Les problem is to use different name for
arm_cp15_start_mmu_config_table to not clash
with the definition in header.

I think that the name arm_cp15_start_mmu_config_table
is not part of RTEMS API so it can be changed freely
and if original arm_cp15_start_mmu_config_table
is not defined then incorrect use is catch
easily.

Please, try to resolve this first. Then the suggested
change for strncmp and do more testing.

I suggest to adapt rpi_init_cmdline() a little.

void rpi_init_cmdline(void)
{
  bcm2835_get_cmdline_entries get_cmdline_entries;
  bcm2835_mailbox_get_cmdline(&get_cmdline_entries);
  int i; /*I suggest to be C89 compatible, do not
           mix declaration with code in single block */
  for (i = 0; i < MAX_CMDLINE_LENGTH; ++i)
  {
    _rpi_cmdline[i] = get_cmdline_entries.cmdline[i];
  }
}

It is not necessary to place 512 bytes onto stack.
If you declare get_cmdline_entries static global

static bcm2835_get_cmdline_entries get_cmdline_entries;

Then you can read the line to this global.

If you declare then _rpi_cmdline as

char *_rpi_cmdline;

you only point it into right place and do not need
copy. You should add code, which ensures that command
line is zero terminated if you use strstr() later.

You can use something like


  bcm2835_mailbox_get_cmdline(&get_cmdline_entries);
  get_cmdline_entries.cmdline[sizeof(get_cmdline_entries.cmdline) - 1] = 0;

Generally code division to the functions
and files looks good.

Best wishes, 

            Pavel