tinyusb icon indicating copy to clipboard operation
tinyusb copied to clipboard

Handling of the Renesas RX write protection during initialization

Open MatiMcFly opened this issue 1 year ago • 3 comments

Describe the PR The Renesas RX controllers possess system-wide write protection for certain registers. Setup functions within the tinyUSB stack have to disable this write protection. This happens in order to disable the USB hardware's module stop function (e.g. MSTP(USB0) in dcd_init). I changed the the write protection register's value handling in a way that the previous state is recovered. This leads to a more intuitive behavior of tinyUSB's initialization code

Additional context Normally, peripheral modules are set up sequentially e.g. in a startup task. Bigger projects require lots of different modules whereas every single peripheral needs to have the module stop function disabled. Threrefor, it is wise to disable the write protection at the very beginning of the initialization routine. At the end it would be reenabled again.

With this new behavior, developers don't have to think about register write protection when calling tinyUSB init-functions.

Here are two examples that show how the simplified behavior works:

Old behavior:

void StartTask(void *pv) {
  (void) pv;
  /* disable write protection of module stop functions */
  SYSTEM.PRCR.WORD = SYSTEM_PRCR_PRKEY | SYSTEM_PRCR_PRC1;
  
  /* call peripheral init functions */
  SetupADC();
  SetupXY();

  /* call tinyUSB init */
  UsbDeviceInit();   /* calls e.g. dcd_init() => This function in turn enables register write protection... */
  
  /* disable register protection again */
  SYSTEM.PRCR.WORD = SYSTEM_PRCR_PRKEY | SYSTEM_PRCR_PRC1;

  /* call further init functions
  lwIPInit();

  /*
  :
  :
  */

  /* reenable register write protection */
  SYSTEM.PRCR.WORD = SYSTEM_PRCR_PRKEY;
}

New behavior:

void StartTask(void *pv) {
  (void) pv;
  /* disable write protection of module stop functions */
  SYSTEM.PRCR.WORD = SYSTEM_PRCR_PRKEY | SYSTEM_PRCR_PRC1;
  
  /* call peripheral init functions */
  SetupADC();
  SetupXY();

  /* call tinyUSB init */
  UsbDeviceInit();   /* calls e.g. dcd_init() => Write protection will be disabled (again), but the previous state is recovered. */

  /* call further init functions, without having to disable the write protection a second time */
  lwIPInit();

  /*
  :
  :
  */

  /* reenable register write protection */
  SYSTEM.PRCR.WORD = SYSTEM_PRCR_PRKEY;
}

MatiMcFly avatar Jul 28 '22 12:07 MatiMcFly

I noticed an additional change that needs to be made. It has to do with the software configurable peripheral interrupt of the USB hardware.

The interrupt vector (PERIB_INTB185) needs to know which peripheral triggers it, therefore the following line needs to be added:

ICU.SLIBR185.BYTE = 62; /* set software configurable interrupt vector 185 to USBI0 */

Please add this line to the files "dcd_usba.c" (insert at 632) and "hcd_usba.c" (insert at 554).

Not adding the line means that users of the stack have to add it manually in the stack or outside of tusb_init. Let me know if you prefer a new pull request or if you'd like to get further clarification.

MatiMcFly avatar Jul 29 '22 11:07 MatiMcFly

consider using uint32_t and declaring the variable at the time of attribution

perigoso avatar Aug 03 '22 15:08 perigoso

@perigoso I totally agree with you. I prefer the stdint types over the underlying types, too. Not exactly sure why I used the underlying types. Probably because they were used in the FreeRTOS files. Declaring a variable anywhere else than at the start of a scope can lead to problems when using older C standards such as C89 (I think). I just wanted to avoid that.

Thanks for your proposals though! Since the build seems to have failed it looks like I'm going to have to make an additional PR anyways...

MatiMcFly avatar Aug 05 '22 11:08 MatiMcFly

Commit c1c1238:

Static code checks generate unnecessary warnings when using the CCRX toolchain. This happens if the macro TU_VERIFY_STATIC is used on the same line numbers in different files. The macro only uses the current line number as "unique ID".

The new solution includes the COUNTER macro in the static code check which gets rid of that issue.

MatiMcFly avatar Sep 28 '22 09:09 MatiMcFly