Page 6 of 11

Re: [DEV] RP2040 support

Posted: Sun Apr 04, 2021 4:33 pm
by Giovanni
Well this should not happen, threads should stay in their own instance, let me check.

Giovanni

Re: [DEV] RP2040 support

Posted: Sun Apr 04, 2021 5:39 pm
by Giovanni
Hi Bob,

Could you put an assertion like this:

Code: Select all

chDbgAssert(port_get_core_id() == __sch_get_currthread(currcore)->owner->core_id, "");


Where you do CTRACE?

Does CTRACE use static variables that could be overwritten? by simply putting those assertions in the demo code threads does not trigger an error.

Giovanni

Re: [DEV] RP2040 support

Posted: Sun Apr 04, 2021 6:24 pm
by Giovanni
Please try the latest commit, I found a problem in chSchReadyI() that could explain it.

Giovanni

Re: [DEV] RP2040 support

Posted: Sun Apr 04, 2021 9:47 pm
by FXCoder
Hi Giovanni,
Well that change to chSchReadyI() fixed keeping the affinity of the thread to its original core.
However, now the CTRACE output stalls until I type a key (which is how I invoke a shell instance on the console channel).
More detail below.
If the entire (not big) project would be helpful I can zip it up and send to you (update: done via PM).
--
Bob

Code: Select all

ChibiOS/RT Shell
RP2040> ^D
logout
*** Trace resumed by user or command ***
[     791.008][INFO ][      main.c 0099][         main] MAIN > Main (C0) running on core 0
======== Trace output was stalled here until I typed a key to get the shell ===========
[     792.000][INFO ][   c1_main.c 0118][         main] MAIN > Main (C1) running on core 1
[     794.008][INFO ][      main.c 0099][         main] MAIN > Main (C0) running on core 0
[     801.691][INFO ][   console.c 0241][      console] CON  > Enter command mode on console channel

*** Trace suspended - type ^D or use the 'exit' command to resume trace ***

ChibiOS/RT Shell
RP2040>


With regard to the implementation of CTRACE...
CTRACE is a macro which invokes a trace_print function to do the trace output.

Code: Select all

/**
 * @brief The trace macro
 */
#define CTRACE(lvl, id, format, args...)                                    \
    do {                                                                    \
      if (current_trace_level > TL_OFF) {                                   \
        trace_print(lvl, id, __FILE__, __LINE__, format, ##args);           \
      }                                                                     \
    } while (0)


Then trace_print() uses a BSEM to control exclusive access to the IO channel.

Code: Select all

/**
 * @brief Output trace message.
 *
 * @param[in] lvl     Trace level for this trace message
 * @param[in] id      Trace source identifier
 * @param[in] path    File path origin of trace message
 * @param[in] line    Line number in source file
 * @param[in] format  Output format string
 * @param[in] ...     Variable arguments list
 *
 */
void trace_print(trace_level_t lvl, trace_source_t id,
                  char* path, uint32_t line, char* format, ...) {

#if (CH_CFG_USE_TIMESTAMP == TRUE)
 systimestamp_t timestamp = chVTGetTimeStamp();
 time_secs_t secs = chTimeTS2S(timestamp);
 time_msecs_t msecs = chTSSecMS(timestamp);
#else
  systime_t timestamp = chVTGetSystemTime();
  time_secs_t secs = TIME_I2S(timestamp % 1000);
  time_msecs_t msecs = TIME_I2MS(timestamp % 1000);
#endif

  if (console != NULL && lvl <= current_trace_level) {
      if (sysIsTraceOutputAvailable()) {

        /* Wait for trace output stream to be available. */
        chBSemWait(&trace_sem);

/* Handle differing trace sources. */
        switch (id) {

          default:

            /* Output time stamp. */
            if (TRACE_SHOW_TIME) {
              chprintf((BaseSequentialStream*)console, TRACE_OPEN_FIELD);
              chprintf((BaseSequentialStream*)console, TRACE_TIME_FORMAT,
                            (time_secs_t)secs, msecs);
              chprintf((BaseSequentialStream*)console, TRACE_CLOSE_FIELD);
            }

            /* Next print the trace level string. */
            chprintf((BaseSequentialStream*)console, TRACE_OPEN_FIELD);
            trace_render_level_name(lvl, (BaseSequentialStream*)console,
                                                     TRACE_LEVEL_FORMAT);
            chprintf((BaseSequentialStream*)console, TRACE_CLOSE_FIELD);

            /* Then the file name. */
            if (TRACE_SHOW_FILE) {
              /* Get the file name only from the path. */
              chprintf((BaseSequentialStream*)console, TRACE_OPEN_FIELD);
              char* file = ((strrchr(path, '/') != NULL) ?
                             strrchr(path, '/') + 1 : path);
              chprintf((BaseSequentialStream*)console, TRACE_FILE_FORMAT, file,
                                                line);
              chprintf((BaseSequentialStream*)console, TRACE_CLOSE_FIELD);
            }

            /* Next is thread. */
            if (TRACE_SHOW_THREAD) {
              chprintf((BaseSequentialStream*)console, TRACE_OPEN_FIELD);
              chprintf((BaseSequentialStream*)console, TRACE_THREAD_FORMAT,
                                              chThdGetSelfX()->name);
              chprintf((BaseSequentialStream*)console, TRACE_CLOSE_FIELD);
            }

            /* And finally the trace source data. */
            chprintf((BaseSequentialStream*)console, TRACE_SOURCE_FORMAT,
                                            trace_get_source_name(id));
            break;

          case _INDENT: {
            /* Indent skips all the identifiers in a line to enable
               multi-line output as a table of data. */
            chprintf((BaseSequentialStream*)console, "%s", TRACE_TAB);
            break;
          }
        } /* End switch (...) */

        /* Render the arguments. */
        va_list args;
        va_start(args, format);
        chvprintf((BaseSequentialStream*)console, format, args);
        va_end(args);
        chprintf((BaseSequentialStream*)console, "\r\n");

        /* Release access to the trace output stream. */
        chBSemSignal(&trace_sem);
      }
  }
    /* Capture any error message in the errors store. Get exclusive access
     before writing. */
  if (lvl == TL_ERR) {
    chBSemWait(&trace_sem);
    uint8_t strcnt = chsnprintf(error_list[error_counter],
        ERROR_LIST_LENGTH, TRACE_TIME_FORMAT" ", (time_secs_t)secs, msecs);
    va_list args;
    va_start(args, format);
    chvsnprintf(&error_list[error_counter][strcnt],
        ERROR_LIST_LENGTH - strcnt, format, args);
    va_end(args);
    error_counter = (error_counter + 1) % ERROR_LIST_SIZE;
    chBSemSignal(&trace_sem);
  }
}

Re: [DEV] RP2040 support

Posted: Mon Apr 05, 2021 4:33 am
by FXCoder
Further notes:
1. Running tickless 1MHz rate with delta 20 creates the problem
2. Running classic 10KHz runs well
--
Bob

Re: [DEV] RP2040 support

Posted: Mon Apr 05, 2021 7:13 am
by Giovanni
Hi Bob,

1MHz is quite high for an M0, I noticed that you need to increase the delta to more than 20 especially with no optimization AND/OR debug options activated. Increasing delta or running the demo with better options should fix the problem, I think this cannot considered a bug, just matter of proper settings. Even the test suite fails when the delta is too low.

Thanks for confirming that the fix actually fixed the affinity problem, I need to do some cleanup in that code area.

Giovanni

Re: [DEV] RP2040 support

Posted: Mon Apr 05, 2021 7:34 am
by FXCoder
All good. Yes the 1MHz default was pushing things rather hard.

Q: For registry will that use ch_port_data to be able to "see" all threads in the system?
I started doing some twiddling in shell_cmd.c to add a core field and next was to walk the instances to list all threads which requires changes to chregistry I expect.
--
Bob

Re: [DEV] RP2040 support

Posted: Mon Apr 05, 2021 7:52 am
by Giovanni
There are two thread registries, we need something to track the active instances, it is a missing mechanism.

Debuggers can do that by looking for variable names "ch", "ch1" etc. We need a runtime centralized "root" structure, ch_port_data should be part of it. Perhaps we should introduce APIs to "enumerate" instances and for searching instances by name, note that the "name" string is not currently used anywhere, some kind of instances registry.

Giovanni

Re: [DEV] RP2040 support

Posted: Fri Apr 16, 2021 2:10 pm
by Giovanni
I just committed a DMA driver, it is modeled after the STM32 one but it needs testing. I will try it using the SPI template you put already in place (Bob), it should be an "easy" driver.

The DMA driver is core-aware, the core taking a DMA channel is also the one getting the completion callbacks from that channel.

Giovanni

Re: [DEV] RP2040 support

Posted: Sat Apr 17, 2021 1:49 pm
by Giovanni
SPI driver committed but I will be unable to give it a try until next week, I left my logic analyser at office....

Giovanni