On Mon, Aug 23, 2021 at 02:42:03PM +1000, Alistair Francis wrote:
> On Mon, Aug 23, 2021 at 12:09 PM Bin Meng <bmeng...@gmail.com> wrote:
> >
> > As of today, when booting upstream U-Boot for Xilinx Zynq, the UART
> > does not receive anything. Debugging shows that the UART input clock
> > frequency is zero which prevents the UART from receiving anything as
> > per the logic in uart_receive().
> >
> > From zynq_slcr_reset_exit() comment, it intends to compute output
> > clocks according to ps_clk and registers. zynq_slcr_compute_clocks()
> > is called to accomplish the task, inside which device_is_in_reset()
> > is called to actually make the attempt in vain.
> >
> > Rework reset_hold() and reset_exit() so that in the reset exit phase,
> > the logic can really compute output clocks in reset_exit().
> >
> > With this change, upstream U-Boot boots properly again with:
> >
> > $ 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
> >
> > Fixes: 38867cb7ec90 ("hw/misc/zynq_slcr: add clock generation for uarts")
> > Signed-off-by: Bin Meng <bmeng...@gmail.com>
>
> Acked-by: Alistair Francis <alistair.fran...@wdc.com>
Reviewed-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
>
> Alistair
>
> > ---
> >
> > hw/misc/zynq_slcr.c | 31 ++++++++++++++++++-------------
> > 1 file changed, 18 insertions(+), 13 deletions(-)
> >
> > diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
> > index 5086e6b7ed..8b70285961 100644
> > --- a/hw/misc/zynq_slcr.c
> > +++ b/hw/misc/zynq_slcr.c
> > @@ -269,6 +269,21 @@ static uint64_t zynq_slcr_compute_clock(const uint64_t
> > periods[],
> > zynq_slcr_compute_clock((plls), (state)->regs[reg], \
> > reg ## _ ## enable_field ## _SHIFT)
> >
> > +static void zynq_slcr_compute_clocks_internal(ZynqSLCRState *s, uint64_t
> > ps_clk)
> > +{
> > + uint64_t io_pll = zynq_slcr_compute_pll(ps_clk,
> > s->regs[R_IO_PLL_CTRL]);
> > + uint64_t arm_pll = zynq_slcr_compute_pll(ps_clk,
> > s->regs[R_ARM_PLL_CTRL]);
> > + uint64_t ddr_pll = zynq_slcr_compute_pll(ps_clk,
> > s->regs[R_DDR_PLL_CTRL]);
> > +
> > + uint64_t uart_mux[4] = {io_pll, io_pll, arm_pll, ddr_pll};
> > +
> > + /* compute uartX reference clocks */
> > + clock_set(s->uart0_ref_clk,
> > + ZYNQ_COMPUTE_CLK(s, uart_mux, R_UART_CLK_CTRL, CLKACT0));
> > + clock_set(s->uart1_ref_clk,
> > + ZYNQ_COMPUTE_CLK(s, uart_mux, R_UART_CLK_CTRL, CLKACT1));
> > +}
> > +
> > /**
> > * Compute and set the ouputs clocks periods.
> > * But do not propagate them further. Connected clocks
> > @@ -283,17 +298,7 @@ static void zynq_slcr_compute_clocks(ZynqSLCRState *s)
> > ps_clk = 0;
> > }
> >
> > - uint64_t io_pll = zynq_slcr_compute_pll(ps_clk,
> > s->regs[R_IO_PLL_CTRL]);
> > - uint64_t arm_pll = zynq_slcr_compute_pll(ps_clk,
> > s->regs[R_ARM_PLL_CTRL]);
> > - uint64_t ddr_pll = zynq_slcr_compute_pll(ps_clk,
> > s->regs[R_DDR_PLL_CTRL]);
> > -
> > - uint64_t uart_mux[4] = {io_pll, io_pll, arm_pll, ddr_pll};
> > -
> > - /* compute uartX reference clocks */
> > - clock_set(s->uart0_ref_clk,
> > - ZYNQ_COMPUTE_CLK(s, uart_mux, R_UART_CLK_CTRL, CLKACT0));
> > - clock_set(s->uart1_ref_clk,
> > - ZYNQ_COMPUTE_CLK(s, uart_mux, R_UART_CLK_CTRL, CLKACT1));
> > + zynq_slcr_compute_clocks_internal(s, ps_clk);
> > }
> >
> > /**
> > @@ -416,7 +421,7 @@ static void zynq_slcr_reset_hold(Object *obj)
> > ZynqSLCRState *s = ZYNQ_SLCR(obj);
> >
> > /* will disable all output clocks */
> > - zynq_slcr_compute_clocks(s);
> > + zynq_slcr_compute_clocks_internal(s, 0);
> > zynq_slcr_propagate_clocks(s);
> > }
> >
> > @@ -425,7 +430,7 @@ static void zynq_slcr_reset_exit(Object *obj)
> > ZynqSLCRState *s = ZYNQ_SLCR(obj);
> >
> > /* will compute output clocks according to ps_clk and registers */
> > - zynq_slcr_compute_clocks(s);
> > + zynq_slcr_compute_clocks_internal(s, clock_get(s->ps_clk));
> > zynq_slcr_propagate_clocks(s);
> > }
> >
> > --
> > 2.25.1
> >
> >
