All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Maydell <peter.maydell@linaro.org>
To: qemu-devel@nongnu.org
Subject: [PATCH 3/6] scripts/coccinelle: New script to add ResetType to hold and exit phases
Date: Fri, 12 Apr 2024 17:08:06 +0100	[thread overview]
Message-ID: <20240412160809.1260625-4-peter.maydell@linaro.org> (raw)
In-Reply-To: <20240412160809.1260625-1-peter.maydell@linaro.org>

We pass a ResetType argument to the Resettable class enter phase
method, but we don't pass it to hold and exit, even though the
callsites have it readily available.  This means that if a device
cared about the ResetType it would need to record it in the enter
phase method to use later on.  We should pass the type to all three
of the phase methods to avoid having to do that.

This coccinelle script adds the ResetType argument to the hold and
exit phases of the Resettable interface.

The first part of the script (rules holdfn_assigned, holdfn_defined,
exitfn_assigned, exitfn_defined) update implementations of the
interface within device models, both to change the signature of their
method implementations and to pass on the reset type when they invoke
reset on some other device.

The second part of the script is various special cases:
 * method callsites in resettable_phase_hold(), resettable_phase_exit()
   and device_phases_reset()
 * updating the typedefs for the methods
 * isl_pmbus_vr.c has some code where one device's reset method directly
   calls the implementation of a different device's method

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
The structure here is a bit of an experiment: usually I would make
the coccinelle script cover the main mechanical change and do the
special cases by hand-editing. But I thought it might be clearer to
have the entire next commit be made by coccinelle, so reviewers don't
have to go hunting through a 99% automated commit for the 1% hand
written part. Let me know whether you like this or not...
---
 scripts/coccinelle/reset-type.cocci | 133 ++++++++++++++++++++++++++++
 1 file changed, 133 insertions(+)
 create mode 100644 scripts/coccinelle/reset-type.cocci

diff --git a/scripts/coccinelle/reset-type.cocci b/scripts/coccinelle/reset-type.cocci
new file mode 100644
index 00000000000..14abdd7bd0c
--- /dev/null
+++ b/scripts/coccinelle/reset-type.cocci
@@ -0,0 +1,133 @@
+// Convert device code using three-phase reset to add a ResetType
+// argument to implementations of ResettableHoldPhase and
+// ResettableEnterPhase methods.
+//
+// Copyright Linaro Ltd 2024
+// SPDX-License-Identifier: GPL-2.0-or-later
+//
+// for dir in include hw target; do \
+// spatch --macro-file scripts/cocci-macro-file.h \
+//        --sp-file scripts/coccinelle/reset-type.cocci \
+//        --keep-comments --smpl-spacing --in-place --include-headers \
+//        --dir $dir; done
+//
+// This coccinelle script aims to produce a complete change that needs
+// no human interaction, so as well as the generic "update device
+// implementations of the hold and exit phase methods" it includes
+// the special-case transformations needed for the core code and for
+// one device model that does something a bit nonstandard. Those
+// special cases are at the end of the file.
+
+// Look for where we use a function as a ResettableHoldPhase method,
+// either by directly assigning it to phases.hold or by calling
+// resettable_class_set_parent_phases, and remember the function name.
+@ holdfn_assigned @
+identifier enterfn, holdfn, exitfn;
+identifier rc;
+expression e;
+@@
+ResettableClass *rc;
+...
+(
+ rc->phases.hold = holdfn;
+|
+ resettable_class_set_parent_phases(rc, enterfn, holdfn, exitfn, e);
+)
+
+// Look for the definition of the function we found in holdfn_assigned,
+// and add the new argument. If the function calls a hold function
+// itself (probably chaining to the parent class reset) then add the
+// new argument there too.
+@ holdfn_defined @
+identifier holdfn_assigned.holdfn;
+typedef Object;
+identifier obj;
+expression parent;
+@@
+-holdfn(Object *obj)
++holdfn(Object *obj, ResetType type)
+{
+    <...
+-    parent.hold(obj)
++    parent.hold(obj, type)
+    ...>
+}
+
+// Similarly for ResettableExitPhase.
+@ exitfn_assigned @
+identifier enterfn, holdfn, exitfn;
+identifier rc;
+expression e;
+@@
+ResettableClass *rc;
+...
+(
+ rc->phases.exit = exitfn;
+|
+ resettable_class_set_parent_phases(rc, enterfn, holdfn, exitfn, e);
+)
+@ exitfn_defined @
+identifier exitfn_assigned.exitfn;
+typedef Object;
+identifier obj;
+expression parent;
+@@
+-exitfn(Object *obj)
++exitfn(Object *obj, ResetType type)
+{
+    <...
+-    parent.exit(obj)
++    parent.exit(obj, type)
+    ...>
+}
+
+// SPECIAL CASES ONLY BELOW HERE
+// We use a python scripted constraint on the position of the match
+// to ensure that they only match in a particular function. See
+// https://public-inbox.org/git/alpine.DEB.2.21.1808240652370.2344@hadrien/
+// which recommends this as the way to do "match only in this function".
+
+// Special case: isl_pmbus_vr.c has some reset methods calling others directly
+@ isl_pmbus_vr @
+identifier obj;
+@@
+- isl_pmbus_vr_exit_reset(obj);
++ isl_pmbus_vr_exit_reset(obj, type);
+
+// Special case: device_phases_reset() needs to pass RESET_TYPE_COLD
+@ device_phases_reset_hold @
+expression obj;
+identifier rc;
+identifier phase;
+position p : script:python() { p[0].current_element == "device_phases_reset" };
+@@
+- rc->phases.phase(obj)@p
++ rc->phases.phase(obj, RESET_TYPE_COLD)
+
+// Special case: in resettable_phase_hold() and resettable_phase_exit()
+// we need to pass through the ResetType argument to the method being called
+@ resettable_phase_hold @
+expression obj;
+identifier rc;
+position p : script:python() { p[0].current_element == "resettable_phase_hold" };
+@@
+- rc->phases.hold(obj)@p
++ rc->phases.hold(obj, type)
+@ resettable_phase_exit @
+expression obj;
+identifier rc;
+position p : script:python() { p[0].current_element == "resettable_phase_exit" };
+@@
+- rc->phases.exit(obj)@p
++ rc->phases.exit(obj, type)
+// Special case: the typedefs for the methods need to declare the new argument
+@ phase_typedef_hold @
+identifier obj;
+@@
+- typedef void (*ResettableHoldPhase)(Object *obj);
++ typedef void (*ResettableHoldPhase)(Object *obj, ResetType type);
+@ phase_typedef_exit @
+identifier obj;
+@@
+- typedef void (*ResettableExitPhase)(Object *obj);
++ typedef void (*ResettableExitPhase)(Object *obj, ResetType type);
-- 
2.34.1



  parent reply	other threads:[~2024-04-12 16:08 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-12 16:08 [PATCH 0/6] reset: Add RESET_TYPE_SNAPSHOT_LOAD Peter Maydell
2024-04-12 16:08 ` [PATCH 1/6] hw/misc: Don't special case RESET_TYPE_COLD in npcm7xx_clk, gcr Peter Maydell
2024-04-12 17:21   ` Richard Henderson
2024-04-12 17:32   ` Philippe Mathieu-Daudé
2024-04-16  9:08   ` Luc Michel
2024-04-12 16:08 ` [PATCH 2/6] allwinner-i2c, adm1272: Use device_cold_reset() for software-triggered reset Peter Maydell
2024-04-12 17:22   ` Richard Henderson
2024-04-16  9:09   ` Luc Michel
2024-04-12 16:08 ` Peter Maydell [this message]
2024-04-16  9:10   ` [PATCH 3/6] scripts/coccinelle: New script to add ResetType to hold and exit phases Luc Michel
2024-04-12 16:08 ` [PATCH 4/6] hw, target: Add ResetType argument to hold and exit phase methods Peter Maydell
2024-04-12 16:49   ` Edgar E. Iglesias
2024-04-12 17:27   ` Richard Henderson
2024-04-16  9:10   ` Luc Michel
2024-04-12 16:08 ` [PATCH 5/6] docs/devel/reset: Update to new API for " Peter Maydell
2024-04-12 17:27   ` Richard Henderson
2024-04-12 17:29   ` Philippe Mathieu-Daudé
2024-04-16  9:10   ` Luc Michel
2024-04-12 16:08 ` [PATCH 6/6] reset: Add RESET_TYPE_SNAPSHOT_LOAD Peter Maydell
2024-04-12 17:30   ` Richard Henderson
2024-04-12 17:31   ` Philippe Mathieu-Daudé
2024-04-16  9:21   ` Luc Michel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240412160809.1260625-4-peter.maydell@linaro.org \
    --to=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.