lpc8xx-hal icon indicating copy to clipboard operation
lpc8xx-hal copied to clipboard

USART code should require UARTFRG reset to be cleared

Open hannobraun opened this issue 7 years ago • 1 comments

The USART code uses UARTFRG (via BaudRate), but doesn't make sure that the peripheral reset for UARTFRG is cleared. While I think this is a bug, it doesn't necessarily manifest itself, as the UARTFRG reset is cleared by default.

I think the correct solution to this problem is to add a type parameter to UARTFRG to track whether it's initialized, but that leads to another problem: The methods on syscon::Api can be used to assert or clear the UARTFRG reset, circumventing any type state.

This isn't a problem for the full-blown peripherals, as those have a clear distinction between types that implement ResetControl et al. and public API types that carry type state, and the former are owned by the latter. Maybe the same mechanism should be used for the other types that implement ClockControl, ResetControl, and AnalogBlock.

I did some experimentation in that direction, but decided that the details deserve some further thought. For now, I'm going to add documentation to state that UARTFRG must not be reset, and leave this fix for later.

hannobraun avatar Feb 12 '18 04:02 hannobraun

I'm going to remove the bug label and replace it with enhancement. The USART code is not doing something wrong, it's just not enforcing one of its (documented) requirements. I haven't called this kind of thing a bug in other cases, and I see it more as a missing feature.

Maybe at a later stage, when the API is more bullet-proof than it is now, such issues would be classified as bugs.

hannobraun avatar Feb 16 '18 06:02 hannobraun