Re: [PATCH] x86/vRTC: move and tidy convert_hour() and {to,from}_bcd()

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 20 Jul 2023, Jan Beulich wrote:
> This is to avoid the need for forward declarations, which in turn
> addresses a violation of MISRA C:2012 Rule 8.3 ("All declarations of an
> object or function shall use the same names and type qualifiers").
> 
> While doing so,
> - drop inline (leaving the decision to the compiler),
> - add const,
> - add unsigned,
> - correct style.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> --- a/xen/arch/x86/hvm/rtc.c
> +++ b/xen/arch/x86/hvm/rtc.c
> @@ -58,8 +58,6 @@ enum rtc_mode {
>  
>  static void rtc_copy_date(RTCState *s);
>  static void rtc_set_time(RTCState *s);
> -static inline int from_bcd(RTCState *s, int a);
> -static inline int convert_hour(RTCState *s, int hour);
>  
>  static void rtc_update_irq(RTCState *s)
>  {
> @@ -246,6 +244,40 @@ static void cf_check rtc_update_timer2(v
>      spin_unlock(&s->lock);
>  }
>  
> +static unsigned int to_bcd(const RTCState *s, unsigned int a)
> +{
> +    if ( s->hw.cmos_data[RTC_REG_B] & RTC_DM_BINARY )
> +        return a;
> +
> +    return ((a / 10) << 4) | (a % 10);
> +}
> +
> +static unsigned int from_bcd(const RTCState *s, unsigned int a)
> +{
> +    if ( s->hw.cmos_data[RTC_REG_B] & RTC_DM_BINARY )
> +        return a;
> +
> +    return ((a >> 4) * 10) + (a & 0x0f);
> +}
> +
> +/*
> + * Hours in 12 hour mode are in 1-12 range, not 0-11. So we need convert it
> + * before use.
> + */
> +static unsigned int convert_hour(const RTCState *s, unsigned int raw)
> +{
> +    unsigned int hour = from_bcd(s, raw & 0x7f);
> +
> +    if ( !(s->hw.cmos_data[RTC_REG_B] & RTC_24H) )
> +    {
> +        hour %= 12;
> +        if ( raw & 0x80 )
> +            hour += 12;
> +    }
> +
> +    return hour;
> +}
> +
>  /* handle alarm timer */
>  static void alarm_timer_update(RTCState *s)
>  {
> @@ -541,37 +573,6 @@ static int rtc_ioport_write(void *opaque
>      return 1;
>  }
>  
> -static inline int to_bcd(RTCState *s, int a)
> -{
> -    if ( s->hw.cmos_data[RTC_REG_B] & RTC_DM_BINARY )
> -        return a;
> -    else
> -        return ((a / 10) << 4) | (a % 10);
> -}
> -
> -static inline int from_bcd(RTCState *s, int a)
> -{
> -    if ( s->hw.cmos_data[RTC_REG_B] & RTC_DM_BINARY )
> -        return a;
> -    else
> -        return ((a >> 4) * 10) + (a & 0x0f);
> -}
> -
> -/* Hours in 12 hour mode are in 1-12 range, not 0-11.
> - * So we need convert it before using it*/
> -static inline int convert_hour(RTCState *s, int raw)
> -{
> -    int hour = from_bcd(s, raw & 0x7f);
> -
> -    if (!(s->hw.cmos_data[RTC_REG_B] & RTC_24H))
> -    {
> -        hour %= 12;
> -        if (raw & 0x80)
> -            hour += 12;
> -    }
> -    return hour;
> -}
> -
>  static void rtc_set_time(RTCState *s)
>  {
>      struct tm *tm = &s->current_tm;
>