|
From: | BALATON Zoltan |
Subject: | Re: [Qemu-trivial] [Qemu-devel] [PATCH v2 13/14] sm501: Add reset function and vmstate descriptor |
Date: | Thu, 2 Mar 2017 22:05:52 +0100 (CET) |
User-agent: | Alpine 2.20 (BSF 67 2015-01-07) |
On Thu, 2 Mar 2017, BALATON Zoltan wrote:
On Thu, 2 Mar 2017, Peter Maydell wrote:On 2 March 2017 at 20:18, BALATON Zoltan <address@hidden> wrote:On Thu, 2 Mar 2017, Peter Maydell wrote:Don't use qemu_register_reset(). Set the appropriate dc->reset function pointers instead.Any reason for that? This way I could save two more boilerplate functionsbecause I could define reset function once, otherwise I'd need two versionstaking sysbus and pci states just to extract the SM501State function and call this function. Do you still think I should do that instead?qemu_register_reset is a pre-QOM method for doing reset; the standard QOM way of saying "my device has some reset behaviour" is to set its reset method pointer. Code calling qemu_register_reset() is generally either (a) doing something kind of weird or (b) old device code that hasn't yet been converted to QOM.Hmm, does having a device with both sysbus and pci versions qualify as weird? Does adding a comment saying that this way we don't need two more reset functions just to call the registered one justifies using qemu_register_reset() or this function is deprecated and should not be used and should go with the separate reset functions instead?
On second thought I may go with the separate reset functions because then I can also update the bus type in the misc_control (not that anything I know depends on it but we can get it right).
So should it be 4 patches: reset for pci, reset for sysbus, vmstate for pci,vmstate for sysbus or 2 patches: reset for both, vmstate for both?I think I'd go with 2 patches, since the two are going to share the bulk of the implementation in each case.OK, thanks for the explanation in previous reply as well, I'll do this then.
So I may also add the reset function already in the QOMify and Add PCI version patches then only one more patch is needed for the vmstate.
[Prev in Thread] | Current Thread | [Next in Thread] |