Re: [Qemu-devel] [PATCH v2 13/14] sm501: Add reset function and vmstate descriptor

[BALATON Zoltan <balaton@eik.bme.hu>]

On Thu, 2 Mar 2017, Peter Maydell wrote:
> On 2 March 2017 at 20:18, BALATON Zoltan <balaton@eik.bme.hu> 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 functions
>> because I could define reset function once, otherwise I'd need two versions
>> taking 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?

>> 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.