mbox series

[0/6] m48t59: remove legacy init functions

Message ID cover.1602965621.git.balaton@eik.bme.hu
Headers show
Series m48t59: remove legacy init functions | expand

Message

Xingtao Yao (Fujitsu)" via Oct. 17, 2020, 8:13 p.m. UTC
This is inspired by Mark's series:

https://lists.nongnu.org/archive/html/qemu-ppc/2020-10/msg00251.html

and implements what I've suggested in review of that series to
simplify it and avoid code churn if implementing my suggestion later.

Regards,
BALATON Zoltan

BALATON Zoltan (4):
  mt48t59: Set default value of base-year property to 1968
  sun4m: use qdev instead of legacy m48t59_init() function
  sun4u: use qdev instead of legacy m48t59_init() function
  ppc405_boards: use qdev instead of legacy m48t59_init() function

Mark Cave-Ayland (2):
  m48t59-isa: remove legacy m48t59_init_isa() function
  m48t59: remove legacy m48t59_init() function

 hw/ppc/ppc405_boards.c  |  3 ++-
 hw/rtc/m48t59-isa.c     | 25 -------------------------
 hw/rtc/m48t59.c         | 37 +------------------------------------
 hw/sparc/sun4m.c        |  5 +++--
 hw/sparc64/sun4u.c      |  6 ++++--
 include/hw/rtc/m48t59.h |  6 ------
 6 files changed, 10 insertions(+), 72 deletions(-)

Comments

Philippe Mathieu-Daudé Oct. 18, 2020, 7:15 a.m. UTC | #1
On 10/17/20 10:13 PM, BALATON Zoltan via wrote:
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   hw/sparc64/sun4u.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index ad5ca2472a..a89ebed6f0 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -671,10 +671,12 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>       pci_ide_create_devs(pci_dev);
>   
>       /* Map NVRAM into I/O (ebus) space */
> -    nvram = m48t59_init(NULL, 0, 0, NVRAM_SIZE, 1968, 59);
> -    s = SYS_BUS_DEVICE(nvram);
> +    dev = qdev_new("sysbus-m48t59");
> +    s = SYS_BUS_DEVICE(dev);
> +    sysbus_realize_and_unref(s, &error_fatal);

I'd use &error_abort here (so if that ever happens, it is
easier to debug it), otherwise:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>       memory_region_add_subregion(pci_address_space_io(ebus), 0x2000,
>                                   sysbus_mmio_get_region(s, 0));
> +    nvram = NVRAM(dev);
>    
>       initrd_size = 0;
>       initrd_addr = 0;
>
Mark Cave-Ayland Oct. 18, 2020, 4:13 p.m. UTC | #2
On 17/10/2020 21:13, BALATON Zoltan via wrote:

> This is inspired by Mark's series:
> 
> https://lists.nongnu.org/archive/html/qemu-ppc/2020-10/msg00251.html
> 
> and implements what I've suggested in review of that series to
> simplify it and avoid code churn if implementing my suggestion later.
> 
> Regards,
> BALATON Zoltan
> 
> BALATON Zoltan (4):
>    mt48t59: Set default value of base-year property to 1968
>    sun4m: use qdev instead of legacy m48t59_init() function
>    sun4u: use qdev instead of legacy m48t59_init() function
>    ppc405_boards: use qdev instead of legacy m48t59_init() function
> 
> Mark Cave-Ayland (2):
>    m48t59-isa: remove legacy m48t59_init_isa() function
>    m48t59: remove legacy m48t59_init() function
> 
>   hw/ppc/ppc405_boards.c  |  3 ++-
>   hw/rtc/m48t59-isa.c     | 25 -------------------------
>   hw/rtc/m48t59.c         | 37 +------------------------------------
>   hw/sparc/sun4m.c        |  5 +++--
>   hw/sparc64/sun4u.c      |  6 ++++--
>   include/hw/rtc/m48t59.h |  6 ------
>   6 files changed, 10 insertions(+), 72 deletions(-)

Unfortunately this arrived too late - I'd already finished the tagging and local 
testing, but didn't get a chance to do the final PR before having to head out yesterday.

I think most people here agree that this code could be improved, but I'm not clear 
that this is the right solution given that Artyom has already pointed out that 40p 
uses 1900 as the base year. There would also be an overlap with the ideas that 
Philippe has expressed in this thread which would cause more code churn later, so if 
this is something that interests you I would suggest starting a separate thread to 
gain consensus as to the desired solution first before working on an updated series.


ATB,

Mark.