qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH-for-9.1 v3 1/2] hw/pci-host/gt64120: Reset config registers d


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH-for-9.1 v3 1/2] hw/pci-host/gt64120: Reset config registers during RESET phase
Date: Fri, 2 Aug 2024 19:01:34 +0200
User-agent: Mozilla Thunderbird

On 1/8/24 19:13, Peter Maydell wrote:
On Thu, 1 Aug 2024 at 18:03, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

+Peter who is tackling our Reset interface limitations,
+Daniel for deprecation advices.

On 1/8/24 17:37, Michael S. Tsirkin wrote:
On Thu, Aug 01, 2024 at 05:30:38PM +0200, BALATON Zoltan wrote:
On Thu, 1 Aug 2024, Philippe Mathieu-Daudé wrote:
Reset config values in the device RESET phase, not only once
when the device is realized, because otherwise the device can
use unknown values at reset.

Mention the datasheet referenced. Remove the "Malta assumptions
ahead" comment since the reset values from the datasheet are used.

Reported-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/pci-host/gt64120.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)


@@ -1231,8 +1238,9 @@ static void gt64120_pci_class_init(ObjectClass *klass, 
void *data)
{
      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
      DeviceClass *dc = DEVICE_CLASS(klass);
+    ResettableClass *rc = RESETTABLE_CLASS(klass);

-    k->realize = gt64120_pci_realize;
+    rc->phases.hold = gt64120_pci_reset_hold;

Why reset_hold and not a simple reset method which is more usual?

DeviceReset is deprecated since 4 years now, see commit c11256aa6f
("hw/core: add Resettable support to BusClass and DeviceClass") and
the effort to convert the legacy interface to this new API:

Peter, Daniel, do we have a way to hint developers about
deprecated API uses (like for versioned machines macros,
commit a35f8577a0 "hw: add macros for deprecation & removal
of versioned machines"), to settle on a release when API
must be converted by?

For reset, the stuff I really want to get converted is
the complex stuff (eg bus reset, cases where a device
reset method needs to call its parent method, etc), and
the ancient legacy stuff (eg qemu_register_reset()).
Converting that will make the reset process more uniform
and allow us to get rid of some annoying inter-compatibility
machinery. (Some of this I've already done -- if you look
at the commits in that list, you'll see that a lot of them
are conversions because those classes were using some API
I wanted to remove like device_class_set_parent_reset().
Still haven't quite got rid of that because s390 CPUs are
doing something a bit awkward...)

Also devices where there's something it needs to do in
reset that should properly be done in a phase other than
'hold' obviously need conversion.

For a simple leaf device reset, a DeviceClass::reset method
and a ResettableClass::reset_hold method are essentially
identical, and the amount of glue code we need to make
the Resettable machinery be able to call a DeviceClass:reset
method is minimal. So I don't care about trying to convert
any of the existing uses in the tree or marking
DeviceClass::reset as deprecated.

If we're adding a reset method to a device which didn't
previously have it, I guess Resettable is preferable,
but I don't feel strongly enough about that to ask for
a change at code-review time, and I suspect I've written
new DeviceClass::reset methods myself.

Thanks for clarifying. I concur Resettable is preferable
when adding a reset method.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]