-----Original Message-----
From: Bin Meng <redacted>
Sent: 14 May 2021 07:48
To: Jessica Clarke <redacted>
Cc: Anup Patel <redacted>; OpenSBI
[off-list ref]
Subject: Re: [PATCH] lib: utils/serial: Support Synopsys DesignWare APB
UART

On Fri, May 14, 2021 at 9:59 AM Jessica Clarke [off-list ref] wrote:

quoted

On 14 May 2021, at 02:27, Bin Meng [off-list ref] wrote:

quoted

On Fri, May 14, 2021 at 9:19 AM Jessica Clarke [off-list ref]

wrote:

quoted

quoted

quoted

On 14 May 2021, at 02:16, Bin Meng [off-list ref] wrote:

quoted

Synopsys DesignWare APB UART is seen on the StarFive JH7100 SoC.
Its programming interface is compatible with the existing 8250
UART driver. Simply add its compatible string to the driver makes
it work with the StarFive JH7100 SoC on a BeagleV board.

With this patch, the generic platform firmware can be used out of
the box on the BeagleV board.

Signed-off-by: Bin Meng <redacted>
---

lib/utils/serial/fdt_serial_uart8250.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/lib/utils/serial/fdt_serial_uart8250.c

b/lib/utils/serial/fdt_serial_uart8250.c
index 918193a..36f364c 100644

--- a/lib/utils/serial/fdt_serial_uart8250.c
+++ b/lib/utils/serial/fdt_serial_uart8250.c

@@ -28,6 +28,7 @@ static int serial_uart8250_init(void *fdt, int

nodeoff, static const struct fdt_match serial_uart8250_match[] = {
     { .compatible = "ns16550" },
     { .compatible = "ns16550a" },
+     { .compatible = "snps,dw-apb-uart" },

If it?s compatible, why does it not use

 compatible = ?snps,dw-apb-uart?, ?ns16550?;

or similar in its FDT?

Good question. By looking at the Linux kernel driver, it is
supported as a variant of 8250 (drivers/tty/serial/8250/8250_dw.c).

I have not looked into details, but I suspect there are some minor
oddities which matter for the kernel. The existing driver is enough
to make OpenSBI happy.

It seems there are various additional clocks and resets you may need
to handle (baudclk is required, apb_pclk is optional and there?s an optional

reset too).

quoted

For this particular board the FDT has a them clocks as all
fixed-clock, but in general this is not a correct implementation of
the driver, as the clocks must be enabled when attaching.

I don't think OpenSBI needs to handle the clock and reset of each peripheral
in its driver. Such should to be done in the bootloader,
i.e.: U-Boot SPL.

Most likely, we will not require to handle clock and reset of each
peripheral. At least, the previous booting stage should set the
"clock-frequency" DT property in UART DT node to be used as console.

Regarding this patch, I am fine with it because the 8250 DT bindings
doesn?t mandate "ns16550" string to be always there. We lot of
ARM64 DTS files having just "snps,dw-apb-uart" as compatible string.

Reviewed-by: Anup Patel <redacted>

Regards,
Anup