mbox series

[PATCH-for-5.2,0/4] hw/char/serial: Use the Clock API to feed the UART reference clock

Message ID 20200806130340.17316-1-f4bug@amsat.org
Headers show
Series hw/char/serial: Use the Clock API to feed the UART reference clock | expand

Message

Philippe Mathieu-Daudé Aug. 6, 2020, 1:03 p.m. UTC
This series improve tracing of multiple UART device in the same
chipset, and allow to use the Clock API to feed each device with
an (updatable) input clock.

Based-on: <20200730165900.7030-1-philmd@redhat.com>
"hw/char: Remove TYPE_SERIAL_IO"

Philippe Mathieu-Daudé (4):
  hw/char/serial: Replace commented DPRINTF() by trace event
  hw/char/serial: Remove old DEBUG_SERIAL commented code
  hw/char/serial: Let SerialState have an 'id' field
  hw/char/serial: Use the Clock API to feed the UART reference clock

 include/hw/char/serial.h |  4 +++
 hw/char/serial.c         | 55 +++++++++++++++++++++++-----------------
 hw/char/trace-events     |  5 ++--
 3 files changed, 39 insertions(+), 25 deletions(-)

Comments

Marc-André Lureau Sept. 1, 2020, 4:54 p.m. UTC | #1
Hi

On Wed, Aug 26, 2020 at 1:52 PM Philippe Mathieu-Daudé <f4bug@amsat.org>
wrote:

> Hi Peter,
>
> Le lun. 24 août 2020 17:20, Peter Maydell <peter.maydell@linaro.org> a
> écrit :
>
>> On Sat, 22 Aug 2020 at 21:00, Philippe Mathieu-Daudé <f4bug@amsat.org>
>> wrote:
>> >
>> > On 8/6/20 3:03 PM, Philippe Mathieu-Daudé wrote:
>> > > This series improve tracing of multiple UART device in the same
>> > > chipset, and allow to use the Clock API to feed each device with
>> > > an (updatable) input clock.
>> > >
>> > > Based-on: <20200730165900.7030-1-philmd@redhat.com>
>> > > "hw/char: Remove TYPE_SERIAL_IO"
>> > >
>> > > Philippe Mathieu-Daudé (4):
>> > >   hw/char/serial: Replace commented DPRINTF() by trace event
>> > >   hw/char/serial: Remove old DEBUG_SERIAL commented code
>> > >   hw/char/serial: Let SerialState have an 'id' field
>> > >   hw/char/serial: Use the Clock API to feed the UART reference clock
>> > >
>> > >  include/hw/char/serial.h |  4 +++
>> > >  hw/char/serial.c         | 55
>> +++++++++++++++++++++++-----------------
>> > >  hw/char/trace-events     |  5 ++--
>> > >  3 files changed, 39 insertions(+), 25 deletions(-)
>> > >
>> >
>> > ping? Should I resend with the typo from patch 4 fixed?
>>
>> Which tree are you expecting the patches to go in via ?
>>
>
> I cc'ed you because having reviewed the Clock API series you are familiar
> with it.
> However I expect this series to be merged by the chardev maintainers.
> In particular to verify the default values (when no input clock provided).
>

Sorry, I am not of much help here. (other than it looks reasonable to me)
Paolo Bonzini Sept. 1, 2020, 5:02 p.m. UTC | #2
On 06/08/20 15:03, Philippe Mathieu-Daudé wrote:
> This series improve tracing of multiple UART device in the same
> chipset, and allow to use the Clock API to feed each device with
> an (updatable) input clock.
> 
> Based-on: <20200730165900.7030-1-philmd@redhat.com>
> "hw/char: Remove TYPE_SERIAL_IO"
> 
> Philippe Mathieu-Daudé (4):
>   hw/char/serial: Replace commented DPRINTF() by trace event
>   hw/char/serial: Remove old DEBUG_SERIAL commented code
>   hw/char/serial: Let SerialState have an 'id' field
>   hw/char/serial: Use the Clock API to feed the UART reference clock
> 
>  include/hw/char/serial.h |  4 +++
>  hw/char/serial.c         | 55 +++++++++++++++++++++++-----------------
>  hw/char/trace-events     |  5 ++--
>  3 files changed, 39 insertions(+), 25 deletions(-)
> 

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Are you planning to deprecate the baudbase property, and instead setting
up the clock already in serial_mm_init?

Thanks,

Paolo
Philippe Mathieu-Daudé Sept. 2, 2020, 10:41 a.m. UTC | #3
On 9/1/20 7:02 PM, Paolo Bonzini wrote:
> On 06/08/20 15:03, Philippe Mathieu-Daudé wrote:
>> This series improve tracing of multiple UART device in the same
>> chipset, and allow to use the Clock API to feed each device with
>> an (updatable) input clock.
>>
>> Based-on: <20200730165900.7030-1-philmd@redhat.com>
>> "hw/char: Remove TYPE_SERIAL_IO"
>>
>> Philippe Mathieu-Daudé (4):
>>   hw/char/serial: Replace commented DPRINTF() by trace event
>>   hw/char/serial: Remove old DEBUG_SERIAL commented code
>>   hw/char/serial: Let SerialState have an 'id' field
>>   hw/char/serial: Use the Clock API to feed the UART reference clock
>>
>>  include/hw/char/serial.h |  4 +++
>>  hw/char/serial.c         | 55 +++++++++++++++++++++++-----------------
>>  hw/char/trace-events     |  5 ++--
>>  3 files changed, 39 insertions(+), 25 deletions(-)
>>
> 
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> Are you planning to deprecate the baudbase property, and instead setting
> up the clock already in serial_mm_init?

I'd love to get there, but unfortunately most of serial_mm_init() have
the baudrate/frequency argument wrong, because it has never been very
clear it was the uart input clock, so most users sets the default
baudrate expected by their guest.

Maybe a sane way to get this slowly fixed is to add a new
serial_mm_init2() documenting it expects an input clock rate
argument (or even better, a connected clock), use it where possible,
deprecate serial_mm_init(), let the maintainer eventually adapt...

I'm not comfortable adding yet another deprecation that will stay
during years.

> 
> Thanks,
> 
> Paolo
> 
>