* [PATCH] serial: ns16550: Handle zero <clock-frequency> value
@ 2021-02-03 14:42 Bin Meng
2021-02-03 21:42 ` Simon Glass
0 siblings, 1 reply; 6+ messages in thread
From: Bin Meng @ 2021-02-03 14:42 UTC (permalink / raw)
To: u-boot
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 at gmail.com/
Signed-off-by: Bin Meng <bmeng.cn@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;
if (!plat->clock) {
debug("ns16550 clock not defined\n");
return -EINVAL;
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] serial: ns16550: Handle zero <clock-frequency> value
2021-02-03 14:42 [PATCH] serial: ns16550: Handle zero <clock-frequency> value Bin Meng
@ 2021-02-03 21:42 ` Simon Glass
2021-02-04 0:20 ` Bin Meng
0 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2021-02-03 21:42 UTC (permalink / raw)
To: u-boot
Hi Bin,
On Wed, 3 Feb 2021 at 07:42, Bin Meng <bmeng.cn@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 at gmail.com/
>
> Signed-off-by: Bin Meng <bmeng.cn@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?
> if (!plat->clock) {
> debug("ns16550 clock not defined\n");
> return -EINVAL;
> --
> 2.7.4
>
Regards,
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] serial: ns16550: Handle zero <clock-frequency> value
2021-02-03 21:42 ` Simon Glass
@ 2021-02-04 0:20 ` Bin Meng
2021-02-04 0:33 ` Simon Glass
0 siblings, 1 reply; 6+ messages in thread
From: Bin Meng @ 2021-02-04 0:20 UTC (permalink / raw)
To: u-boot
Hi Simon,
On Thu, Feb 4, 2021 at 5:42 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Bin,
>
> On Wed, 3 Feb 2021 at 07:42, Bin Meng <bmeng.cn@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 at gmail.com/
> >
> > Signed-off-by: Bin Meng <bmeng.cn@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.
>
> > if (!plat->clock) {
> > debug("ns16550 clock not defined\n");
> > return -EINVAL;
Regards,
Bin
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] serial: ns16550: Handle zero <clock-frequency> value
2021-02-04 0:20 ` Bin Meng
@ 2021-02-04 0:33 ` Simon Glass
2021-02-04 0:39 ` Bin Meng
2021-02-07 0:16 ` Simon Glass
0 siblings, 2 replies; 6+ messages in thread
From: Simon Glass @ 2021-02-04 0:33 UTC (permalink / raw)
To: u-boot
On Wed, 3 Feb 2021 at 17:20, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Thu, Feb 4, 2021 at 5:42 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Bin,
> >
> > On Wed, 3 Feb 2021 at 07:42, Bin Meng <bmeng.cn@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 at gmail.com/
> > >
> > > Signed-off-by: Bin Meng <bmeng.cn@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?
Reviewed-by: Simon Glass <sjg@chromium.org>
>
> >
> > > if (!plat->clock) {
> > > debug("ns16550 clock not defined\n");
> > > return -EINVAL;
>
> Regards,
> Bin
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] serial: ns16550: Handle zero <clock-frequency> value
2021-02-04 0:33 ` Simon Glass
@ 2021-02-04 0:39 ` Bin Meng
2021-02-07 0:16 ` Simon Glass
1 sibling, 0 replies; 6+ messages in thread
From: Bin Meng @ 2021-02-04 0:39 UTC (permalink / raw)
To: u-boot
Hi Simon,
On Thu, Feb 4, 2021 at 8:33 AM Simon Glass <sjg@chromium.org> wrote:
>
> On Wed, 3 Feb 2021 at 17:20, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Thu, Feb 4, 2021 at 5:42 AM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Bin,
> > >
> > > On Wed, 3 Feb 2021 at 07:42, Bin Meng <bmeng.cn@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 at gmail.com/
> > > >
> > > > Signed-off-by: Bin Meng <bmeng.cn@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@chromium.org>
>
Regards,
Bin
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] serial: ns16550: Handle zero <clock-frequency> value
2021-02-04 0:33 ` Simon Glass
2021-02-04 0:39 ` Bin Meng
@ 2021-02-07 0:16 ` Simon Glass
1 sibling, 0 replies; 6+ messages in thread
From: Simon Glass @ 2021-02-07 0:16 UTC (permalink / raw)
To: u-boot
Hi Simon,
On Thu, Feb 4, 2021 at 8:33 AM Simon Glass <sjg@chromium.org> wrote:
>
> On Wed, 3 Feb 2021 at 17:20, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Thu, Feb 4, 2021 at 5:42 AM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Bin,
> > >
> > > On Wed, 3 Feb 2021 at 07:42, Bin Meng <bmeng.cn@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 at gmail.com/
> > > >
> > > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > > > ---
> > > >
> > > > drivers/serial/ns16550.c | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
Applied to u-boot-dm, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-02-07 0:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-03 14:42 [PATCH] serial: ns16550: Handle zero <clock-frequency> value Bin Meng
2021-02-03 21:42 ` Simon Glass
2021-02-04 0:20 ` Bin Meng
2021-02-04 0:33 ` Simon Glass
2021-02-04 0:39 ` Bin Meng
2021-02-07 0:16 ` Simon Glass
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.