Hi
On Mon, Nov 25, 2019 at 5:04 PM Philippe Mathieu-Daudé
<phi...@redhat.com> wrote:
>
> On 11/25/19 1:54 PM, Philippe Mathieu-Daudé wrote:
> > 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
> >>> <aleksandar.m.m...@gmail.com> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On Wednesday, November 20, 2019, Marc-André Lureau
> >>>> <marcandre.lur...@redhat.com> wrote:
> >>>>>
> >>>>> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> >>>>> ---
> >>>>> 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
>
> IOW, until we merge Damien's "Clock framework API" series, I'd:
>
> - rename 'baudbase' -> 'input_frequency_hz'
>
> - set a 0 default value
>
> DEFINE_PROP_UINT32("input-frequency-hz", SerialState,
> input_frequency_hz, 0),
>
> - add a check in serial_realize()
>
> if (s->input_frequency_hz == 0) {
> error_setg(errp,
> "serial: input-frequency-hz property must be set");
> return;
> }
>
> [*] https://www.mail-archive.com/qemu-devel@nongnu.org/msg642174.html
>
This is getting further away from this series goal, and my initial
goal. Let's add this to the backlog. I can drop a FIXME there.
> >> 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 <w...@mail.berlios.de>
> > 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 <w...@mail.berlios.de>
> > Signed-off-by: Anthony Liguori <aligu...@us.ibm.com>
> >
> > 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);
> > +}
> > +
>
