Re: [Qemu-devel] [PATCH 1/2] m48t59: introduce new year_offset qdev property

[Mark Cave-Ayland]

On 19/01/15 12:06, Artyom Tarasenko wrote:

> On Mon, Jan 19, 2015 at 12:35 PM, Mark Cave-Ayland
> <address@hidden> wrote:
>> Currently the m48t59 device uses the hardware model in order to determine
>> whether the year value is offset from the hardware value. As this will
>> soon be required by the x59 model, change the year offset to a qdev
>> property and update the callers appropriately.
>>
>> Signed-off-by: Mark Cave-Ayland <address@hidden>
>> ---
>>  hw/ppc/ppc405_boards.c    |    2 +-
>>  hw/ppc/prep.c             |    2 +-
>>  hw/sparc/sun4m.c          |    2 +-
>>  hw/sparc64/sun4u.c        |    2 +-
>>  hw/timer/m48t59.c         |   35 ++++++++++++++++-------------------
>>  include/hw/timer/m48t59.h |    5 +++--
>>  6 files changed, 23 insertions(+), 25 deletions(-)
>>
>> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
>> index 1dcea77..2d8d2aa 100644
>> --- a/hw/ppc/ppc405_boards.c
>> +++ b/hw/ppc/ppc405_boards.c
>> @@ -283,7 +283,7 @@ static void ref405ep_init(MachineState *machine)
>>  #ifdef DEBUG_BOARD_INIT
>>      printf("%s: register NVRAM\n", __func__);
>>  #endif
>> -    m48t59_init(NULL, 0xF0000000, 0, 8192, 8);
>> +    m48t59_init(NULL, 0xF0000000, 0, 8192, 68, 8);
>>      /* Load kernel */
>>      linux_boot = (kernel_filename != NULL);
>>      if (linux_boot) {
>> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
>> index 15df7f3..378a368 100644
>> --- a/hw/ppc/prep.c
>> +++ b/hw/ppc/prep.c
>> @@ -543,7 +543,7 @@ static void ppc_prep_init(MachineState *machine)
>>          pci_create_simple(pci_bus, -1, "pci-ohci");
>>      }
>>
>> -    m48t59 = m48t59_init_isa(isa_bus, 0x0074, NVRAM_SIZE, 59);
>> +    m48t59 = m48t59_init_isa(isa_bus, 0x0074, NVRAM_SIZE, 0, 59);
>>      if (m48t59 == NULL)
>>          return;
>>      sysctrl->nvram = m48t59;
>> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
>> index ecd9dc1..cde4a87 100644
>> --- a/hw/sparc/sun4m.c
>> +++ b/hw/sparc/sun4m.c
>> @@ -1012,7 +1012,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef 
>> *hwdef,
>>
>>      lance_init(&nd_table[0], hwdef->le_base, ledma, ledma_irq);
>>
>> -    nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 8);
>> +    nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 68, 8);
>>
>>      slavio_timer_init_all(hwdef->counter_base, slavio_irq[19], 
>> slavio_cpu_irq, smp_cpus);
>>
>> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
>> index 3ff5bd8..6b46511 100644
>> --- a/hw/sparc64/sun4u.c
>> +++ b/hw/sparc64/sun4u.c
>> @@ -873,7 +873,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>>          fd[i] = drive_get(IF_FLOPPY, 0, i);
>>      }
>>      fdctrl_init_isa(isa_bus, fd);
>> -    nvram = m48t59_init_isa(isa_bus, 0x0074, NVRAM_SIZE, 59);
>> +    nvram = m48t59_init_isa(isa_bus, 0x0074, NVRAM_SIZE, 0, 59);
>>
>>      initrd_size = 0;
>>      initrd_addr = 0;
>> diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
>> index 31509d5..0a05100 100644
>> --- a/hw/timer/m48t59.c
>> +++ b/hw/timer/m48t59.c
>> @@ -56,6 +56,7 @@ struct M48t59State {
>>      MemoryRegion iomem;
>>      uint32_t io_base;
>>      uint32_t size;
>> +    uint32_t year_offset;
>>      /* RTC management */
>>      time_t   time_offset;
>>      time_t   stop_time;
>> @@ -346,11 +347,7 @@ void m48t59_write (void *opaque, uint32_t addr, 
>> uint32_t val)
>>         tmp = from_bcd(val);
>>         if (tmp >= 0 && tmp <= 99) {
>>             get_time(NVRAM, &tm);
>> -            if (NVRAM->model == 8) {
>> -                tm.tm_year = from_bcd(val) + 68; // Base year is 1968
>> -            } else {
>> -                tm.tm_year = from_bcd(val);
>> -            }
>> +            tm.tm_year = from_bcd(val) + NVRAM->year_offset;
>>             set_time(NVRAM, &tm);
>>         }
>>          break;
>> @@ -453,11 +450,7 @@ uint32_t m48t59_read (void *opaque, uint32_t addr)
>>      case 0x07FF:
>>          /* year */
>>          get_time(NVRAM, &tm);
>> -        if (NVRAM->model == 8) {
>> -            retval = to_bcd(tm.tm_year - 68); // Base year is 1968
>> -        } else {
>> -            retval = to_bcd(tm.tm_year);
>> -        }
>> +        retval = to_bcd(tm.tm_year - NVRAM->year_offset);
>>          break;
>>      default:
>>          /* Check lock registers state */
>> @@ -641,8 +634,8 @@ static const MemoryRegionOps m48t59_io_ops = {
>>  };
>>
>>  /* Initialisation routine */
>> -M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
>> -                         uint32_t io_base, uint16_t size, int model)
>> +M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base, uint32_t io_base,
>> +                         uint16_t size, uint32_t year_offset, int model)
>>  {
>>      DeviceState *dev;
>>      SysBusDevice *s;
>> @@ -653,6 +646,7 @@ M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
>>      qdev_prop_set_uint32(dev, "model", model);
>>      qdev_prop_set_uint32(dev, "size", size);
>>      qdev_prop_set_uint32(dev, "io_base", io_base);
>> +    qdev_prop_set_uint32(dev, "year_offset", year_offset);
>>      qdev_init_nofail(dev);
>>      s = SYS_BUS_DEVICE(dev);
>>      d = SYSBUS_M48T59(dev);
>> @@ -671,7 +665,7 @@ M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
>>  }
>>
>>  M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
>> -                             int model)
>> +                             uint32_t year_offset, int model)
>>  {
>>      M48t59ISAState *d;
>>      ISADevice *isadev;
>> @@ -683,6 +677,7 @@ M48t59State *m48t59_init_isa(ISABus *bus, uint32_t 
>> io_base, uint16_t size,
>>      qdev_prop_set_uint32(dev, "model", model);
>>      qdev_prop_set_uint32(dev, "size", size);
>>      qdev_prop_set_uint32(dev, "io_base", io_base);
>> +    qdev_prop_set_uint32(dev, "year_offset", year_offset);
>>      qdev_init_nofail(dev);
>>      d = ISA_M48T59(isadev);
>>      s = &d->state;
>> @@ -738,9 +733,10 @@ static int m48t59_init1(SysBusDevice *dev)
>>  }
>>
>>  static Property m48t59_isa_properties[] = {
>> -    DEFINE_PROP_UINT32("size",    M48t59ISAState, state.size,    -1),
>> -    DEFINE_PROP_UINT32("model",   M48t59ISAState, state.model,   -1),
>> -    DEFINE_PROP_UINT32("io_base", M48t59ISAState, state.io_base,  0),
>> +    DEFINE_PROP_UINT32("size",        M48t59ISAState, state.size,       -1),
>> +    DEFINE_PROP_UINT32("model",       M48t59ISAState, state.model,      -1),
>> +    DEFINE_PROP_UINT32("io_base",     M48t59ISAState, state.io_base,     0),
> 
> Formatting-only change?

Yeah. It's one of those files whereby the properties are white-spaced
aligned according to the longest property name, and since "year_offset"
now becomes the longest property name everything else gets shifted...


ATB,

Mark.