| Message ID | 20240814181534.218964-3-shentey@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Serial device cleanup | expand |
On Wed, 14 Aug 2024, Bernhard Beschow wrote: > The serial port's frequency is set via the "baudbase" property nowadays. Please keep it as some devices might have registers that set this freq and this function will be needed for emulating that even if it's not emulated currently. Regards, BALATON Zoltan > Signed-off-by: Bernhard Beschow <shentey@gmail.com> > --- > include/hw/char/serial.h | 2 -- > hw/char/serial.c | 7 ------- > 2 files changed, 9 deletions(-) > > diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h > index 6e14099ee7..40aad21df3 100644 > --- a/include/hw/char/serial.h > +++ b/include/hw/char/serial.h > @@ -93,8 +93,6 @@ struct SerialMM { > extern const VMStateDescription vmstate_serial; > extern const MemoryRegionOps serial_io_ops; > > -void serial_set_frequency(SerialState *s, uint32_t frequency); > - > #define TYPE_SERIAL "serial" > OBJECT_DECLARE_SIMPLE_TYPE(SerialState, SERIAL) > > diff --git a/hw/char/serial.c b/hw/char/serial.c > index d8b2db5082..6c5c4a23c7 100644 > --- a/hw/char/serial.c > +++ b/hw/char/serial.c > @@ -951,13 +951,6 @@ static void serial_unrealize(DeviceState *dev) > qemu_unregister_reset(serial_reset, s); > } > > -/* Change the main reference oscillator frequency. */ > -void serial_set_frequency(SerialState *s, uint32_t frequency) > -{ > - s->baudbase = frequency; > - serial_update_parameters(s); > -} > - > const MemoryRegionOps serial_io_ops = { > .read = serial_ioport_read, > .write = serial_ioport_write, >
On 14/08/2024 22:38, BALATON Zoltan wrote: > On Wed, 14 Aug 2024, Bernhard Beschow wrote: >> The serial port's frequency is set via the "baudbase" property nowadays. > > Please keep it as some devices might have registers that set this freq and this > function will be needed for emulating that even if it's not emulated currently. Why not remove the function if it isn't being used? In general having unused code lying around is bad as it tends to bitrot. If someone did want to re-add this functionality then you would likely want to do it with clock API rather than using a separate function. ATB, Mark.
On 15/8/24 10:24, Mark Cave-Ayland wrote: > On 14/08/2024 22:38, BALATON Zoltan wrote: > >> On Wed, 14 Aug 2024, Bernhard Beschow wrote: >>> The serial port's frequency is set via the "baudbase" property nowadays. >> >> Please keep it as some devices might have registers that set this freq >> and this function will be needed for emulating that even if it's not >> emulated currently. > > Why not remove the function if it isn't being used? In general having > unused code lying around is bad as it tends to bitrot. > > If someone did want to re-add this functionality then you would likely > want to do it with clock API rather than using a separate function. I also ended using it in a branch including this old series: https://lore.kernel.org/qemu-devel/20200806130340.17316-1-f4bug@amsat.org/ Not my priority anymore, so I don't mind re-adding it.
Am 15. August 2024 09:26:38 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>: >On 15/8/24 10:24, Mark Cave-Ayland wrote: >> On 14/08/2024 22:38, BALATON Zoltan wrote: >> >>> On Wed, 14 Aug 2024, Bernhard Beschow wrote: >>>> The serial port's frequency is set via the "baudbase" property nowadays. >>> >>> Please keep it as some devices might have registers that set this freq and this function will be needed for emulating that even if it's not emulated currently. >> >> Why not remove the function if it isn't being used? In general having unused code lying around is bad as it tends to bitrot. >> >> If someone did want to re-add this functionality then you would likely want to do it with clock API rather than using a separate function. > >I also ended using it in a branch including this old series: >https://lore.kernel.org/qemu-devel/20200806130340.17316-1-f4bug@amsat.org/ > >Not my priority anymore, so I don't mind re-adding it. > This removal seems debatable. I'd err on the safe side and drop the patch in v2. Any further comments?
diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h index 6e14099ee7..40aad21df3 100644 --- a/include/hw/char/serial.h +++ b/include/hw/char/serial.h @@ -93,8 +93,6 @@ struct SerialMM { extern const VMStateDescription vmstate_serial; extern const MemoryRegionOps serial_io_ops; -void serial_set_frequency(SerialState *s, uint32_t frequency); - #define TYPE_SERIAL "serial" OBJECT_DECLARE_SIMPLE_TYPE(SerialState, SERIAL) diff --git a/hw/char/serial.c b/hw/char/serial.c index d8b2db5082..6c5c4a23c7 100644 --- a/hw/char/serial.c +++ b/hw/char/serial.c @@ -951,13 +951,6 @@ static void serial_unrealize(DeviceState *dev) qemu_unregister_reset(serial_reset, s); } -/* Change the main reference oscillator frequency. */ -void serial_set_frequency(SerialState *s, uint32_t frequency) -{ - s->baudbase = frequency; - serial_update_parameters(s); -} - const MemoryRegionOps serial_io_ops = { .read = serial_ioport_read, .write = serial_ioport_write,
The serial port's frequency is set via the "baudbase" property nowadays. Signed-off-by: Bernhard Beschow <shentey@gmail.com> --- include/hw/char/serial.h | 2 -- hw/char/serial.c | 7 ------- 2 files changed, 9 deletions(-)