Hi Sandrine,
Putting getc() under a flag that is disabled by default looks like a good idea. It is not used on ST platforms (at least inside ST).
Best regards, Yann
On 9/13/23 08:45, Sandrine Bailleux via TF-A wrote:
Hello Brian, Baruch,
Thank you both for sharing your use cases, that helps a lot. It's now clear to me that we need to keep getc() support in TF-A code base.
This also means we should add tests in TF-A OpenCI to exercise getc(), if possible. This would make sure we do not inadvertently introduce any regression that would break your factory test CLI / firmware recovery setups in the future. Obviously we would not be able to test each and every individual console driver, simply because we do not have all required platforms in TF-A OpenCI, but we could at least cover the ones we have.
It sounds like we are leaning towards putting getc() behind a disabled-by-default build flag. I'll wait for others' opinions on the mailing list about that. Else I'll propose a patch to this effect in the coming weeks.
Best regards, Sandrine
On 9/12/23 18:10, Baruch Siach wrote:
Hi Sandrine,
On Tue, Sep 12 2023, Sandrine Bailleux via TF-A wrote:
Now, even though getc() is not used in upstream TF-A code right now, I realize there might be downstream / internal test setups which need it. For example, for firmware recovery purposes (receiving a backup firmware over a serial interface) or automated tests setups (some script driving a test session using some communication protocol over a serial interface).
Is anyone depending on such use cases?
We have a customer in this situation. Their SoC uses Xmodem as last resort for loading BL2. The serial console is PL011. Upstream driver getc implementation works nicely for this purpose.
If not, then I suggest we consider removing getc() feature altogether. We could always bring it back when there is a real use case for it (it will survive through git history).
At the very least, I would like to disable getc() by default. Enabling it would require setting a build flag.
This is a good compromise, IMHO.
Thanks, baruch