Non-bus-powered MSP430 support.

[clhenry]

Describe the PR Adding supported for hot plugging while using external power.

Additional context

[cr1901]

It looks fine to me (I think at the time, I only focused on bus-powered applications b/c I found it difficult to reliably put the MSP-EXP430F5529LP into self-powered mode.

I do have a few questions:

1. Is USBCTL &= ~FRSTE; and USBCTL |= FRSTE; intended to prevent a race due to power transients causing a second reset while in the process of handling the first reset? What happens if the reset vector is entered while processing the current reset?

2. I think I can't even find a definition for osal_task_delay when osal_none is being used. Presumably it's a noop and then is optimized out in release mode. I think the whole build should fail due to missing definition when compiled in debug mode.

   It looks like other platforms use an include guard or a board_millis function provided by the application (which is not how I'd do it).

   Personally, I'd use the msp430 __delay_cycles if I absolutely have to have a hardcoded delay, but that's tied to the current CPU frequency. I'm a bit uncomfortable with hard-depending on a timer in the msp430 port, or relying on board_millis to be defined. I'm assuming you're testing without an OS, and so the code works fine without working delays. I would put an include guard for now that doesn't call osal_task_delay when CFG_TUSB_OS == OPT_OS_NONE.

[clhenry]

1. As this was 6 months ago I had to wrack my brain to try and remember the justification. I believe this was a partial implementation of TI's software fix for the USB4 errata affecting the MSP430F5529. According to the linked document FRSTE should be cleared on receipt of the reset interrupt and set on receipt of the suspend, setup and endpoint 0 interrupts. I'll be pushing a change later that correctly implements their recommend fix. In the mean time, if you have any suggestions to address your original concern, I'm all ears.

2. I'm using FreeRTOS for my project. The initial delay in handle_bus_power_event was the shortest I could do in order to get the functionality working. The delay in the loop can be as short as 500 ns per TI's recommendation. As you pointed out, the intrinsic delay function is frequency dependent. I don't mind modify the PR to support a non-OS environment but I'm at a loss as to what would work for everyone if not board_millis. Again, I'm open to recommendations.

[cr1901]

Ack re: the errata; when I did this port originally, I don't recall looking at the errata. That's fine then (incl. your upcoming changes).

I'm using FreeRTOS for my project.

Ahhh I didn't know FreeRTOS ran on MSP430, tbh :D!

I want to wait and hear what @hathach has to say re: "using board_millis in portable/". #1987 is also relevant.

[clhenry]

@hathach I see that the non-os builds are failing on osal_task_delay. Can you chime in with your thoughts on the current discussion?

[cr1901]

@clhenry Just to be clear, @hathach may be very busy, and while I am a collaborator, I mostly focus on backend stuff. I want to wait for @hathach to be less busy so he can give guidance on "sometimes no OS needs a delay" problem.

You're already expected to instantiate an interrupt handler yourself and place a call into tud_int_handler in your application. I'm not against adding a function like e.g. tud_delay that's called by osal_task_delay for no-OS, that the user is expected to fill in for their application if necessary. But adding this functionality requires some care.

[hathach]

sorry, I was a bit busy atm, will check this out whenever I could. Thank you for your patient.

[hathach]

@clhenry Just to be clear, @hathach may be very busy, and while I am a collaborator, I mostly focus on backend stuff. I want to wait for @hathach to be less busy so he can give guidance on "sometimes no OS needs a delay" problem.

You're already expected to instantiate an interrupt handler yourself and place a call into tud_int_handler in your application. I'm not against adding a function like e.g. tud_delay that's called by osal_task_delay for no-OS, that the user is expected to fill in for their application if necessary. But adding this functionality requires some care.

tinyusb tries not to use additional hardware timer, and does not requires an time keeping function from application. For any short delay without rtos, it should use nop since there is no guarantee of actual time keeping function.

actual osal_task_delay() can be implemented as weak default implementation by dcd. Since I leave this pending for too long, I will try to make an update/test to get it merge myself :)