On 17/07/18 10:32, Jan Beulich wrote: >>>> On 17.07.18 at 11:03, <[email protected]> wrote: >> On 17/07/18 09:35, Jan Beulich wrote: >>> @@ -411,7 +410,13 @@ static int hpet_write( >>> case HPET_Tn_CFG(2): >>> tn = HPET_TN(CFG, addr); >>> >>> - h->hpet.timers[tn].config = hpet_fixup_reg(new_val, old_val, >>> 0x3f4e); >>> + h->hpet.timers[tn].config = hpet_fixup_reg(new_val, old_val, >>> + HPET_TN_LEVEL | >>> + HPET_TN_ENABLE | >>> + HPET_TN_PERIODIC | >>> + HPET_TN_SETVAL | >>> + HPET_TN_32BIT | >>> + HPET_TN_ROUTE); >> This can be rather better reflowed. e.g. >> >> diff --git a/xen/arch/x86/hvm/hpet.c b/xen/arch/x86/hvm/hpet.c >> index f7ef4f7..5986d1d 100644 >> --- a/xen/arch/x86/hvm/hpet.c >> +++ b/xen/arch/x86/hvm/hpet.c >> @@ -411,7 +411,10 @@ static int hpet_write( >> case HPET_Tn_CFG(2): >> tn = HPET_TN(CFG, addr); >> >> - h->hpet.timers[tn].config = hpet_fixup_reg(new_val, old_val, >> 0x3f4e); >> + h->hpet.timers[tn].config = hpet_fixup_reg( >> + new_val, old_val, (HPET_TN_LEVEL | HPET_TN_ENABLE | >> + HPET_TN_PERIODIC | HPET_TN_SETVAL | >> + HPET_TN_32BIT | HPET_TN_ROUTE)); > Whether this is "better" is a matter of taste. To be honest, I pretty > much dislike this indentation style for function calls. I could maybe > be talked into > > h->hpet.timers[tn].config = > hpet_fixup_reg(new_val, old_val, > (HPET_TN_LEVEL | HPET_TN_ENABLE | > HPET_TN_PERIODIC | HPET_TN_SETVAL | > HPET_TN_32BIT | HPET_TN_ROUTE)); > > (albeit it's only marginally better than what you suggest), but clearly > not your variant. > >> Otherwise, Reviewed-by: Andrew Cooper <[email protected]> > Let me know for which of the variant(s) this stands.
I'm happy with either. The main point is not having a silly quantity of extremely right-justifed code. ~Andrew _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
