Re: [Qemu-devel] [PATCHv3 1/2] m48t59: introduce new base_year qdev property

[Hervé Poussineau]

Hi,

Le 14/02/2015 09:52, Mark Cave-Ayland a écrit :
> 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, create a qdev base_year property to
> represent the base year 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         |   27 +++++++++++++++------------
>   include/hw/timer/m48t59.h |    5 +++--
>   6 files changed, 22 insertions(+), 18 deletions(-)
>
> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
> index 1dcea77..5019f20 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, 2000, 8);
>       /* Load kernel */
>       linux_boot = (kernel_filename != NULL);
>       if (linux_boot) {

Before this patch, m48t59 model 8 (ie m48t08) was handled with a base year of 
1968 by default. Here, you're changing the behaviour. Is is expected, or did 
you meant 1968?

> [...]

> @@ -387,11 +388,7 @@ static void m48t59_write(M48t59State *NVRAM, 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->base_year - 1900;
>             set_time(NVRAM, &tm);
>         }
>           break;

Here, 1968 was the default base year for m48t08 on writes.

> @@ -493,11 +490,7 @@ static uint32_t m48t59_read(M48t59State *NVRAM, 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 + 1900 - NVRAM->base_year) % 100);
>           break;
>       default:
>           /* Check lock registers state */

Here, 1968 was the default base year for m48t08 on reads.

>  [...]

> @@ -809,6 +805,7 @@ static void m48txx_isa_toggle_lock(Nvram *obj, int lock)
>   }
>
>   static Property m48t59_isa_properties[] = {
> +    DEFINE_PROP_INT32("base_year", M48txxISAState, state.base_year, 0),
>       DEFINE_PROP_UINT32("iobase", M48txxISAState, io_base, 0x74),
>       DEFINE_PROP_END_OF_LIST(),
>   };

It seems like tha QEMU way of naming properties is with an hyphen, not with an underscore. So you 
should use "base-year" instead of "base_year".

> @@ -852,6 +849,11 @@ static void m48txx_sysbus_toggle_lock(Nvram *obj, int lock)
>       m48t59_toggle_lock(&d->state, lock);
>   }
>
> +static Property m48t59_sysbus_properties[] = {
> +    DEFINE_PROP_INT32("base_year", M48txxSysBusState, state.base_year, 0),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +

Again here.

Regards,

Hervé