[For 6.1 PATCH] hw/arm: xilinx_zynq: Disconnect the UART clocks temporarily

[For 6.1 PATCH] hw/arm: xilinx_zynq: Disconnect the UART clocks temporarily

[Bin Meng]

As of today, when booting upstream U-Boot for Xilinx Zynq, the UART
does not receive anything. Initial debugging shows that the UART clock
frequency is 0 somehow which prevents the UART from receiving anything.
Note the U-Boot can still output data to the UART tx fifo, which should
not happen, as the design seems to prevent the data transmission when
clock is not enabled but somehow it only applies to the Rx side.

For anyone who is interested to give a try, here is the U-Boot defconfig:
$ make xilinx_zynq_virt_defconfig

and QEMU commands to test U-Boot:
$ qemu-system-arm -M xilinx-zynq-a9 -m 1G -display none -serial null -serial stdio \
    -device loader,file=u-boot-dtb.bin,addr=0x4000000,cpu-num=0

Note U-Boot used to boot properly in QEMU 4.2.0 which is the QEMU
version used in current U-Boot's CI testing. The UART clock changes
were introduced by the following 3 commits:

38867cb7ec90 ("hw/misc/zynq_slcr: add clock generation for uarts")
b636db306e06 ("hw/char/cadence_uart: add clock support")
5b49a34c6800 ("hw/arm/xilinx_zynq: connect uart clocks to slcr")

Looks like we don't have enough time to figure out a proper fix before
6.1.0 release date, let's disconnect the UART clocks temporarily.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

 hw/arm/xilinx_zynq.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 245af81bbb..1bc749f6b8 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -257,16 +257,12 @@ static void zynq_init(MachineState *machine)
     dev = qdev_new(TYPE_CADENCE_UART);
     busdev = SYS_BUS_DEVICE(dev);
     qdev_prop_set_chr(dev, "chardev", serial_hd(0));
-    qdev_connect_clock_in(dev, "refclk",
-                          qdev_get_clock_out(slcr, "uart0_ref_clk"));
     sysbus_realize_and_unref(busdev, &error_fatal);
     sysbus_mmio_map(busdev, 0, 0xE0000000);
     sysbus_connect_irq(busdev, 0, pic[59 - IRQ_OFFSET]);
     dev = qdev_new(TYPE_CADENCE_UART);
     busdev = SYS_BUS_DEVICE(dev);
     qdev_prop_set_chr(dev, "chardev", serial_hd(1));
-    qdev_connect_clock_in(dev, "refclk",
-                          qdev_get_clock_out(slcr, "uart1_ref_clk"));
     sysbus_realize_and_unref(busdev, &error_fatal);
     sysbus_mmio_map(busdev, 0, 0xE0001000);
     sysbus_connect_irq(busdev, 0, pic[82 - IRQ_OFFSET]);
-- 
2.25.1

Re: [For 6.1 PATCH] hw/arm: xilinx_zynq: Disconnect the UART clocks temporarily

[Peter Maydell]

On Sat, 21 Aug 2021 at 16:45, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> As of today, when booting upstream U-Boot for Xilinx Zynq, the UART
> does not receive anything. Initial debugging shows that the UART clock
> frequency is 0 somehow which prevents the UART from receiving anything.
> Note the U-Boot can still output data to the UART tx fifo, which should
> not happen, as the design seems to prevent the data transmission when
> clock is not enabled but somehow it only applies to the Rx side.
>
> For anyone who is interested to give a try, here is the U-Boot defconfig:
> $ make xilinx_zynq_virt_defconfig
>
> and QEMU commands to test U-Boot:
> $ qemu-system-arm -M xilinx-zynq-a9 -m 1G -display none -serial null -serial stdio \
>     -device loader,file=u-boot-dtb.bin,addr=0x4000000,cpu-num=0
>
> Note U-Boot used to boot properly in QEMU 4.2.0 which is the QEMU
> version used in current U-Boot's CI testing. The UART clock changes
> were introduced by the following 3 commits:
>
> 38867cb7ec90 ("hw/misc/zynq_slcr: add clock generation for uarts")
> b636db306e06 ("hw/char/cadence_uart: add clock support")
> 5b49a34c6800 ("hw/arm/xilinx_zynq: connect uart clocks to slcr")
>
> Looks like we don't have enough time to figure out a proper fix before
> 6.1.0 release date, let's disconnect the UART clocks temporarily.

This is too late for 6.1 regardless, I'm afraid.

thanks
-- PMM

Re: [For 6.1 PATCH] hw/arm: xilinx_zynq: Disconnect the UART clocks temporarily

[Bin Meng]

On Sun, Aug 22, 2021 at 2:14 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Sat, 21 Aug 2021 at 16:45, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > As of today, when booting upstream U-Boot for Xilinx Zynq, the UART
> > does not receive anything. Initial debugging shows that the UART clock
> > frequency is 0 somehow which prevents the UART from receiving anything.
> > Note the U-Boot can still output data to the UART tx fifo, which should
> > not happen, as the design seems to prevent the data transmission when
> > clock is not enabled but somehow it only applies to the Rx side.
> >
> > For anyone who is interested to give a try, here is the U-Boot defconfig:
> > $ make xilinx_zynq_virt_defconfig
> >
> > and QEMU commands to test U-Boot:
> > $ qemu-system-arm -M xilinx-zynq-a9 -m 1G -display none -serial null -serial stdio \
> >     -device loader,file=u-boot-dtb.bin,addr=0x4000000,cpu-num=0
> >
> > Note U-Boot used to boot properly in QEMU 4.2.0 which is the QEMU
> > version used in current U-Boot's CI testing. The UART clock changes
> > were introduced by the following 3 commits:
> >
> > 38867cb7ec90 ("hw/misc/zynq_slcr: add clock generation for uarts")
> > b636db306e06 ("hw/char/cadence_uart: add clock support")
> > 5b49a34c6800 ("hw/arm/xilinx_zynq: connect uart clocks to slcr")
> >
> > Looks like we don't have enough time to figure out a proper fix before
> > 6.1.0 release date, let's disconnect the UART clocks temporarily.
>
> This is too late for 6.1 regardless, I'm afraid.

That's too bad :(

I figured out a proper fix, and will send it out soon.

Regards,
Bin