Re: [PATCH v3 16/29] serial: Add semihosting driver

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 3758 bytes --]

On Mon, Mar 28, 2022 at 11:36:46AM -0400, Sean Anderson wrote:
> 
> 
> On 3/28/22 2:35 AM, Simon Glass wrote:
> > Hi Sean,
> > 
> > On Tue, 22 Mar 2022 at 15:00, Sean Anderson <sean.anderson@seco.com> wrote:
> >>
> >> This adds a serial driver which uses semihosting calls to read and write
> >> to the host's console. For convenience, if CONFIG_DM_SERIAL is enabled,
> >> we will instantiate a serial driver. This allows users to enable this
> >> driver (which has no physical device) without modifying their device
> >> trees or board files. We also implement a non-DM driver for SPL, or for
> >> much faster output in U-Boot proper.
> >>
> >> There are three ways to print to the console:
> >>
> >> Method              Baud
> >> ================== =====
> >> smh_putc in a loop   170
> >> smh_puts            1600
> >> smh_write with :tt 20000
> >> ================== =====
> >>
> >> These speeds were measured using a 175 character message with a J-Link
> >> adapter. For reference, U-Boot typically prints around 2700 characters
> >> during boot on this board. There are two major factors affecting the
> >> speed of these functions. First, each breakpoint incurs a delay. Second,
> >> each debugger memory transaction incurs a delay. smh_putc has a
> >> breakpoint and memory transaction for every character. smh_puts has one
> >> breakpoint, but still has to use a transaction for every character. This
> >> is because we don't know the length up front, so OpenOCD has to check if
> >> each character is nul. smh_write has only one breakpoint and one memory
> >> transfer.
> >>
> >> DM serial drivers can only implement a putc interface, so we are stuck
> >> with the slowest API. Non-DM drivers can implement puts, which is vastly
> >> more efficient. When the driver starts up, we try to open :tt. Since
> >> this is an extension, this may fail. If it does, we fall back to
> >> smh_puts. We don't check :semihosting-features, since there are
> >> nonconforming implementations (OpenOCD) which don't implement it (but
> >> *do* implement :tt).
> >>
> >> Some semihosting implementations (QEMU) don't handle READC properly. To
> >> work around this, we try to use open/read (much like for stdin) if
> >> possible.
> >>
> >> There is no non-blocking I/O available, so we don't implement pending.
> >> This will cause __serial_tstc to always return true. If
> >> CONFIG_SERIAL_RX_BUFFER is enabled, _serial_tstc will try and read
> >> characters forever. To avoid this, we depend on this config being
> >> disabled.
> >>
> >> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> >> ---
> >>
> >> (no changes since v2)
> >>
> >> Changes in v2:
> >> - Fix baud numbers being off by 10
> >> - Fix typos in commit message
> >> - Rename non-DM driver struct to match format of other drivers
> >>
> >>  drivers/serial/Kconfig              |  22 +++++
> >>  drivers/serial/Makefile             |   1 +
> >>  drivers/serial/serial.c             |   2 +
> >>  drivers/serial/serial_semihosting.c | 147 ++++++++++++++++++++++++++++
> >>  include/serial.h                    |   1 +
> >>  5 files changed, 173 insertions(+)
> >>  create mode 100644 drivers/serial/serial_semihosting.c
> >>
> > 
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> > 
> > But please can we drop the non-DM support?
> 
> Unfortunately, Layerscape does not support DM serial. I tried converting
> it, but I ran into some unusual aborts. At the moment, I don't have time
> to debug things further. And I thought that non-DM serial was ok for
> SPL?

It is OK for SPL, and it needs migration for non-SPL.  Can you make
another thread with your conversion-that-fails for layerscape please?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]