Re: [PATCH v4 18/37] mips: baudbase is 115200 by default

[Philippe Mathieu-Daudé]

On 11/25/19 12:26 PM, Philippe Mathieu-Daudé wrote:
> On 11/25/19 11:12 AM, Marc-André Lureau wrote:
> > Hi
> >
> > On Mon, Nov 25, 2019 at 2:07 PM Aleksandar Markovic
> > <address@hidden> wrote:
> > > On Wednesday, November 20, 2019, Marc-André Lureau 
> > > <address@hidden> wrote:
> > > > Signed-off-by: Marc-André Lureau <address@hidden>
> > > > ---
> > > >   hw/mips/mips_mipssim.c | 1 -
> > > >   1 file changed, 1 deletion(-)
> > > >
> > > > diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
> > > > index bfafa4d7e9..3cd0e6eb33 100644
> > > > --- a/hw/mips/mips_mipssim.c
> > > > +++ b/hw/mips/mips_mipssim.c
> > > > @@ -223,7 +223,6 @@ mips_mipssim_init(MachineState *machine)
> > > >       if (serial_hd(0)) {
> > > >           DeviceState *dev = qdev_create(NULL, TYPE_SERIAL_IO);
> > > >
> > > > -        qdev_prop_set_uint32(DEVICE(dev), "baudbase", 115200);
> > > >           qdev_prop_set_chr(dev, "chardev", serial_hd(0));
> > > >           qdev_set_legacy_instance_id(dev, 0x3f8, 2);
> > > >           qdev_init_nofail(dev);
> > > > --
> > >
> > >
> > > Please mention in your commit message where the default baudbase is set.
> >
> > ok
> >
> > > Also, is there a guarantie that default value 115200 will never 
> > > change in future?
> >
> > The level of stability on properties in general is unclear to me.
> >
> > Given that 115200 is standard for serial, it is unlikely to change
> > though.. We can have an assert there instead?
> >
> > Peter, what do you think? thanks
>
> This property confused me by the past. It is _not_ the baudrate.
> It is the input frequency clocking the UART ('XIN' pin, Xtal INput).
>
> Each board has its own frequency, and it can even be variable (the clock 
> domain tree can reconfigure it at a different rate).

Laurent pointed me to the following commit which confirms my interpretation:

$ git show 038eaf82c853
commit 038eaf82c853f3bf8d4c106c0677bbf4adada7de
Author: Stefan Weil <address@hidden>
Date:   Sat Oct 31 11:28:11 2009 +0100

    serial: Add interface to set reference oscillator frequency

    Many (most?) serial interfaces have a programmable
    clock which provides the reference frequency ("baudbase").
    So a fixed baudbase which is only set once can be wrong.

    omap1.c is an example which could use the new interface
    to change baudbase when the programmable clock changes.
    ar7 system emulation (still not part of standard QEMU)
    is similar to omap and already uses serial_set_frequency.

    Signed-off-by: Stefan Weil <address@hidden>
    Signed-off-by: Anthony Liguori <address@hidden>

diff --git a/hw/pc.h b/hw/pc.h
index 15fff8d103..03ffc91536 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -13,6 +13,7 @@ SerialState *serial_mm_init (target_phys_addr_t base, 
int it_shift,
                              qemu_irq irq, int baudbase,
                              CharDriverState *chr, int ioregister);
 SerialState *serial_isa_init(int index, CharDriverState *chr);
+void serial_set_frequency(SerialState *s, uint32_t frequency);

 /* parallel.c */

diff --git a/hw/serial.c b/hw/serial.c
index fa12dcc075..0063260569 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -730,6 +730,13 @@ static void serial_init_core(SerialState *s)
                           serial_event, s);
 }

+/* Change the main reference oscillator frequency. */
+void serial_set_frequency(SerialState *s, uint32_t frequency)
+{
+    s->baudbase = frequency;
+    serial_update_parameters(s);
+}
+