[PATCH] serial: ns16550: Handle zero <clock-frequency> value

[Bin Meng]

Hi Simon,

On Thu, Feb 4, 2021 at 8:33 AM Simon Glass <sjg at chromium.org> wrote:
>
> On Wed, 3 Feb 2021 at 17:20, Bin Meng <bmeng.cn at gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Thu, Feb 4, 2021 at 5:42 AM Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Hi Bin,
> > >
> > > On Wed, 3 Feb 2021 at 07:42, Bin Meng <bmeng.cn at gmail.com> wrote:
> > > >
> > > > A working device tree node of ns16550 should never be populated
> > > > with value zero for the <clock-frequency> property. Unfortunately
> > > > this is the case for the QEMU ppce500 target.
> > > >
> > > > Let's try to assign plat->clock to CONFIG_SYS_NS16550_CLK as the
> > > > last resort to handle such case.
> > > >
> > > > This commit should be reverted when:
> > > >
> > > > - The following QEMU patch [1] is merged, and
> > > > - U-Boot CI has upgraded its QEMU version that contains the fix
> > > >
> > > > [1] http://patchwork.ozlabs.org/project/qemu-devel/patch/1612362288-22216-2-git-send-email-bmeng.cn@gmail.com/
> > > >
> > > > Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
> > > > ---
> > > >
> > > >  drivers/serial/ns16550.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> > > > index da903c1..a1593a2 100644
> > > > --- a/drivers/serial/ns16550.c
> > > > +++ b/drivers/serial/ns16550.c
> > > > @@ -563,6 +563,8 @@ int ns16550_serial_of_to_plat(struct udevice *dev)
> > > >         if (!plat->clock)
> > > >                 plat->clock = dev_read_u32_default(dev, "clock-frequency",
> > > >                                                    CONFIG_SYS_NS16550_CLK);
> > > > +       if (!plat->clock)
> > > > +               plat->clock = CONFIG_SYS_NS16550_CLK;
> > >
> > > This is already done in the line above...does that not work?
> >
> > The line above only works if there is no <clock-frequency> in the
> > device tree. Since the device tree does provide a <clock-frequency>,
> > the default one was never used.
>
> Yes you said that in the commit message, but there was 15 minutes
> between be reading that and the patch :-(
>
> I really don't like adding this sort of thing globally though. Do you
> think we could make it happen only for qemu?

Correct, that's why I said we should revert this patch once the 2
conditions are met.

I am working on migrating QEMU ppce500 to driver model. Without this
patch the U-Boot does not boot.

>
> Reviewed-by: Simon Glass <sjg at chromium.org>
>

Regards,
Bin