Add RFC for an UART peripheral.

[jfng]

Rendered.

[tpwrules]

Overall, this peripheral comes across as substantially under-opinionated to me. Essentially all the logic and behavior is in the PHY, so this really just specifies a schema for connecting a UART peripheral to the CSR bus, rather than being a peripheral in its own right. But it's limiting because the PHY is going to have to be closely coupled to the peripheral due to the shared interface prescribing the features. I don't think there's anything that could be added without changing this, except for DMA.

I would be strongly in favor of dropping the possibilities of multiple PHYs and just dragging in (or duplicating) amaranth-stdio for the implementation.

Some more specific comments:

• What does "receiver/transmitter held in reset" really mean considering the peripheral does not own the PHYs? Presumably the enable signal should be passed to them, or to the user to hook up an EnableInserter?
• How does the prescaler get wired up? Again, the peripheral does not own the PHYs?
• The baud error presumably depends on source clock frequency?
• Should the read register bit be called valid to match streams?
• The baudrate computation is described with two different syntaxes.
• The peripheral has to have at least one character of buffer for the amaranth-stdio PHY to work, why not allow it to be expanded?
• The amaranth-stdio PHY is not a valid stream transmitter so connecting it here feels slightly dirty.

[whitequark]

I would be strongly in favor of dropping the possibilities of multiple PHYs and just dragging in (or duplicating) amaranth-stdio for the implementation.

The RFC is explicitly designed to be usable with anything that comes with a pair of streams. Furthermore, nothing in Amaranth SoC is supposed to duplicate or even closely couple to Amaranth stdio. I don't expect that to change.

The advantage of being able to couple to anything that's a pair of streams is that the peripheral becomes truly a "basic universal asynchronous receiver/transmitter" peripheral rather than an RS-232 peripheral that most "UART"s are. I consider this a highly desirable property in its own right. It is not intended to replicate the 16550 interface in the slightest.

• The baud error presumably depends on source clock frequency?

I came up with the packing scheme for 3 scale bits and 13 mantissa bits, and it has less than 1% error for all bauds from 9600 to 3M, and all clocks from 32k to 200M, except for these:

Fclk=96_000_000 req=9_600 act=11_718 error=18.07%
Fclk=120_000_000 req=9_600 act=14_648 error=34.46%
Fclk=150_000_000 req=9_600 act=18_310 error=47.57%
Fclk=200_000_000 req=9_600 act=24_414 error=60.68%
Fclk=200_000_000 req=19_200 act=24_414 error=21.36%

safe=378 unsafe=5 unmet=0

Unless you're really into 9600 baud this is good enough. You can also play with the script.

• Should the read register bit be called valid to match streams?

No, that is of no concern to the firmware author.

• The amaranth-stdio PHY is not a valid stream transmitter so connecting it here feels slightly dirty.

The entire current amaranth-stdio PHY will be removed and replaced with a new one based on an actual methodology rather than some scribbles I did five years ago.

[stafverhaegen-chipflow]

I would be strongly in favor of dropping the possibilities of multiple PHYs and just dragging in (or duplicating) amaranth-stdio for the implementation.

The RFC is explicitly designed to be usable with anything that comes with a pair of streams. Furthermore, nothing in Amaranth SoC is supposed to duplicate or even closely couple to Amaranth stdio. I don't expect that to change.

I personally agree with not duplicating _stdio things in _soc. I do find though that current implementation needs quite some boilerplate code for each UART instantiation. I think the divisor and the pins are what will be specific for each instance and the rest will be boilerplate. I think it would be good to have convenience function that has the boilerplate code included.

[whitequark]

I think it would be good to have convenience function that has the boilerplate code included.

Broadly speaking I agree but we are still at the stage where we are finding out what are the conventions and methodologies for building SoC peripherals. So I think such a function can be added at a later stage, when we have gained more understanding of what it means to build a SoC in Amaranth.

(This is also how I approach the core language, so my position should not be surprising.)

[jfng]

Changed to draft status, which will be removed once the UART in amaranth-stdio is rewritten from scratch updated to reach quality standards of upstream Amaranth.

[jfng]

This RFC was discussed during the 2024-03-22 SoC meeting:

• (@zyp, @whitequark) instead of Control and Divisor registers, provide Config and PhyConfig registers whose shapes are user-provided and used in the PHY signatures. Config would only affect the SoC (i.e. memory-mapped) side/domain.
• (@whitequark) the goal of this UART can be described as an acronym for "Universally supports (within reason) any Asynchronous Receiver and/or Transmitter".

[jfng]

Updated to use a PhyConfig register (as suggested above) and shorten some API names.

The layout of the Config register is deliberately left unparameterized:

• the peripheral relies on the existence of an enable field to drive the rst port of the PHY interface.
• downstream peripheral implementations can populate the remaining 7-bit ResR0W0 field while remaining register-compatible with this design iteration.