Allow change of baud rate and release of Serial

[nraynaud]

• also remove the peripheral bus clock on release
• usage shown in example.

I tested it with a logic analyser, I see the 'X' at 9600bps and the 'Y' at 19200.

[TheZoq2]

Thanks for the PR, I like the idea, but I'm not suer if its necessary to add the ReleaseToken, TX and RX are unique as it is, right?

[nraynaud]

Rx or Tx would have to carry the type of the pins if they were the release token.

[therealprof]

I still think it would be preferable to implement Read and Write on the serial port itself. This would not only allow one to use the U(S)ART without splitting if it's not actually required for some reason but also to easily free it, and change the baudrate on the fly.

The release token has a bit of yuck factor attached to it and it is also a breaking API change which we should try to reduce to a minimum, especially when done spuriously.

[nraynaud]

I don't see how you can do long running DMA with Read and Write on the serial port itself.

[therealprof]

@nraynaud That's fine, you can still split (or do other fancy advanced stuff). My understanding is you'd like to do Rx/Tx to discover usable baudrates and my suggestion would cater for that. On the other hand I'm not sure how you would safely ensure that nothing is using your DMA setup at the time you're freeing your USART so not allowing to free a split serial port could actually be construed as a safety feature.

[nraynaud]

what would the split() function return?

The way it works (in my working copy), is that the DMA system takes ownership of the Rx or Tx while a transaction is running and it give it back at the end. No Rx or Tx-> impossible to release. That's why release() take both as an argument.

[therealprof]

split() would stay exactly the same as it is now. I'm not worried about release() not working on split pins, if we allow that we need to ensure that nothing is still using the USART, including your DMA, and I don't see how ReleaseToken would address that.

[nraynaud]

I am a bit confused, unless I am mistaken, it's impossible to release an UART that is in use with the code I propose. If it's in use you can't have ownership of Rx or Tx, and the ReleaseToken.release() waits for TC (Transmission Complete).

[therealprof]

As you know DMA works independently of the ALU so even if you have ownership of the Rx/Tx, it doesn't mean that the DMA engine has stopped accessing the registers. I think the only way to guarantee this would be to integrate DMA support directly in the driver.

But again, I'd like to hear a good reason/usecase why we should break compatibility to allow releasing a split' U(S)ART.

[nraynaud]

This discussion is going nowhere. The DMA as it is now has its safety ensured by the type system, there are wait time and checks that ensure that the hardware is in the state reflected in the type of the objects.

If the compatibility is an issue (spoiler: it's not; the DMA will come back, and it won't look exactly the same, and it will break everything, and it's not a big deal), I can create a second split() function that returns a ReleaseToken.

[therealprof]

This discussion is going nowhere.

You're absolutely right, also because I still haven't heard a reason why we need the ReleaseToken abomination...

[nraynaud]

The release token serves to release the serial object after it's been split(). This is currently impossible to do.

[therealprof]

And the motivation/use case why we need to have this is...?

[nraynaud]

scanning for / configuring the baud rate of hayes command devices?

[therealprof]

And would prevent you from doing that on the unsplit serial if we implemented Read/Write as suggested earlier?

[nraynaud]

The fact that the DMA will enter in concurrence with the read/write operations on the serial.

[nraynaud]

Can we merge it now?

[TheZoq2]

I agree with @therealprof, the release token is a somewhat ugly breaking change that we should try to avoid. I'm not familiar with how DMA works so I can't comment on the safety and or unsafety of using DMA on a non-split serial.

However, do you need DMA for detecting baud rate? If we impl Read and Write on the non-split serial object, you can use that to detect the baud rate. Once the baud rate is detected, you can split it into tx and rx to do your high speed DMA supported transfers. Or do you have a use case where that doesn't work?

Also, there is a merge conflict in the examples/serial.rs file

[nraynaud]

Reading the serial byte by byte from the core is a bit complicated, you have to create some kind of block!() with a timeout, you need to keep track of you local buffer, etc.

I just put a DMA reader on the serial, I wait a bit and I check the buffer, if it's still empty I consider that the remote device has not answered on time and I test the next frequency.

I have spent hours trying to avoid the release token (not just "thinking its ugly", I put the work), and I couldn't, and it's inherent to the way the peripherals are wrapped in rust. It's either explicit like I did or Rx or Tx could be the release token (but their type will change).

I'll handle the merge conflict.

[TheZoq2]

This PR has been dormant for a while, sorry about that. I had an excues to look at the DMA implementation recently, and I agree with what @nraynaud is saying. The DMA transfer takes ownership of the tx and/or rx types, does the transfer and only gives them back when the DMA is finished.

In the case of SPI, I think the device might still be active with the last byte after DMA relase, but that can easily be checked by the busy flag.

Regarding the release token, I still don't really see the point. Can't we just add a release function to Tx or Rx which takes the other as an argument, i.e. Tx::release(self, Rx)?

[TheZoq2]

Since this PR has been inactive, i'll go ahead and close it, feel free to re-open it if something changes