[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] goldfish_rtc: Fix non-atomic read behaviour of TIME_LOW/T
Re: [PATCH v2] goldfish_rtc: Fix non-atomic read behaviour of TIME_LOW/TIME_HIGH
Mon, 20 Jul 2020 16:11:11 -0700
On Sat, Jul 18, 2020 at 11:09 AM Peter Maydell <email@example.com> wrote:
> On Sat, 18 Jul 2020 at 15:45, Jessica Clarke <firstname.lastname@example.org> wrote:
> > On 18 Jul 2020, at 08:42, Philippe Mathieu-Daudé <email@example.com> wrote:
> > > Maybe easier to cache the whole u64, this matches RTC_ALARM_LOW /
> > > RTC_ALARM_HIGH pattern (goldfish_rtc_vmstate change not included):
> > We could, but why waste space storing an extra 32 bits you never need?
> > I don't think saving all 64 bits makes it any easier to read, I'd
> > personally even argue it makes it slightly less obvious what's going on.
> You could go either way. (The original Google-written version
> of this device model went for store-the-whole-u64:
> but we don't need to follow their implementation.)
> Since "save the high half" is the version you've written
> and tested, I vote we go with that :-)
> Reviewed-by: Peter Maydell <firstname.lastname@example.org>
Applied to the RISC-V tree, this looks like a bug worth fixing for 5.1
so I'll send a PR soon.
> NB: this is a migration compatibility break for the risc-v
> 'virt' board : up to Alistair whether that's OK or if the
> more awkward compat-maintaining vmstate subsection is worth
> the effort.
I think it's ok
> -- PMM