samd: Fix a lock-up situation at high traffic.

[robert-hh]

This PR fixes a transmit lock-up, which happens, when data is received
and sent at the sime time at moderate to high speeds, like code
which just echoes incoming data.

In my case, an issue was reported here:
micropython/micropython#8521

[pigrew]

It looks like there there's at least one more spot that needs the fix:

USB->DEVICE.INTFLAG.reg |= USB->DEVICE.INTFLAG.reg; // clear pending

Or maybe not?

[robert-hh]

That's possible. The INTXXX registers should not be set with |=. Although that one did not make trouble (yet).

[robert-hh]

Actually that one looks fine. It clears all pending interrupts. So the |= is intional and right.

[hathach]

thank you for the PR, could you tell which specific board/mcu you are having the issue with

[robert-hh]

Thank you for the swift response.
It affects SAM21 and SAMD51 boards. Tested (or to better say failed) with Adafruit ItsysBitsy M4 Express, Adafruit ItsysBitsy M0 Express, Adafruit Feather M4 Express, and Seeed XIAO. But the problem will appear on any SAMD21 and SAMD51 board. With the change, it works fine on these boards. And I use these with a MicroPython port.

One thing which is to be considered: the MicroPython port uses tinyusb v0.12.0. The PR is against the recent master, which is beyond v0.13.0. The version used by MicroPython will be updated some day, but it requires some coordination, because of differences between 0.12.0 and the actual master, e.g. tud_task() vs. tud_task_ext(). Pretty trivial, but has to be taken care of. So I'm considering to make in addition a PR for a branch of v0.12.0 with these changes for SAMD.

[hathach]

superb! Thank you very much for the PR. So far there have been a few issue with |= on set/clear register across codebase on other ports. These are really difficult to troubleshoot, maybe I should spend sometime to revise all the dcd/hcd ports for this.

[hathach]

tud_task() can still be used as it previously use (defined as inline function). There is no need to make any changes to application for this regards. The 2 extra arguments for tud_task_ext() is mostly for using with rtos timeout and/or isr context (place holder).

tinyusb/src/device/usbd.h

Lines 52 to 56 in c7fce32

	TU_ATTR_ALWAYS_INLINE static inline
	void tud_task (void)
	{
	tud_task_ext(UINT32_MAX, false);
	}

[robert-hh]

tud_task() can still be used as it previously use (defined as inline function). There is no need to make any changes to application for this regards.

Thank you for merging and the hint. I got an link error using the master branch. tud_task() is part of a common "busy-wait" macro. So probably the #include is missing. Either way the code has to be changed, and the version of the RP2040 lib as well.

[robert-hh]

So far there have been a few issue with |= on set/clear register across codebase on other ports.

The INTFLAG, INTSET and INTCLR registers have the habit of clearing/setting the respective internal state by writing a '1' value. So writing a 1 causes an action, instead of just storing a value. That's to be checked against the data sheet for the registers.

[hathach]

tud_task() can still be used as it previously use (defined as inline function). There is no need to make any changes to application for this regards.

Thank you for merging and the hint. I got an link error using the master branch. tud_task() is part of a common "busy-wait" macro. So probably the #include is missing. Either way the code has to be changed, and the version of the RP2040 lib as well.

probably due to the fact that tud_task() is an inline function. For rp2040, you may need to udate pico-sdk (rpi will release new version soon enough).

[robert-hh]

For rp2040, you may need to udate pico-sdk

Of course. I tried that already. Not a big deal. Tinyusb uses a named constant that is not defined in the currently used version of the lib.

[hathach]

@robert-hh maybe just wait for a bit of time, there is always some latency for big project like micropython when it comes to update to the sub library such as tinyusb. I do have an plan to release tinyusb in upcoming days.

[robert-hh]

No problem. It will take anyhow another while until the extended SAMD port is merged, and until then I can live with a workaround.