Re: [DISCUSSION] fw_devlink: incorrect ancestor fallback for intermediate bus topologies breaks audio dependency resolution

[Mukesh Ojha]

On Wed, Jul 01, 2026 at 06:47:33PM +0530, Ajay Kumar Nandam wrote:
> Hi Saravana Kannan,
> 
> I am Ajay from Qualcomm, and I am working together with Ravi on ADSP
> Subsystem Restart (SSR) support for Qualcomm LPASS audio. During our
> SSR work Ravi observed that restart does not behave as expected — when
> the ADSP restarts, all child nodes of the remoteproc should detach and
> re-attach cleanly once the remoteproc is up. While investigating
> this, Ravi identified an incorrect assumption in fw_devlink's dependency
> resolution logic that causes wrong device links to be created when a
> supplier device lives behind intermediate bus nodes that are populated
> asynchronously. We observed this on a Qualcomm qcs6490-rb3gen2 board
> with LPASS audio drivers, but the assumption is generic and can affect
> any similar topology.
> 
> Problem Summary:
> 
> fw_devlink appears to incorrectly collapse pending dependencies to
> an ancestor device when supplier devices exist behind asynchronously
> created intermediate bus nodes.
> 
> In our topology, q6prmcc is instantiated as a descendant of the ADSP
> remoteproc through asynchronously created glink/gpr bus devices.
> When the remoteproc device binds earlier, fw_devlink appears to assume
> that these intermediate nodes will never become devices, collapses the
> dependency to the remoteproc ancestor, and deletes the pending links.
> As a result, the intended dependency:
>     lpass_va_macro -> q6prmcc
> is replaced by:
>     lpass_va_macro -> remoteproc
> 
> This results in incorrect teardown ordering during ADSP SSR because
> codec drivers are no longer linked to their actual clock supplier.
> 
> Could you please suggest how we can resolve this issue? Is there
> a specific reason for this assumption, as it looks incorrect in our case?
> 
> 
> -------------------------------------------
> 1. AUDIO SYSTEM TOPOLOGY ON qcs6490-rb3gen2
> 
> On qcs6490-rb3gen2 the LPASS audio clock supply chain spans a
> four-level device hierarchy rooted at the ADSP remoteproc. The
> intermediate nodes (glink-edge, gpr) only get their struct devices
> after the ADSP firmware has booted, which happens well after the
> remoteproc driver's own probe() returns.
> 
> The Device Tree hierarchy (from qcs6490-audioreach.dtsi / kodiak.dtsi):
> https://elixir.bootlin.com/linux/v7.1/source/arch/arm64/boot/dts/qcom/qcs6490-audioreach.dtsi#L113
> 
>   remoteproc_adsp                         (ADSP remoteproc)
>       └── remoteproc_adsp_glink           (GLink transport edge)
>               └── gpr                     (GPR bus, compatible="qcom,gpr")
>                       └── service@2       (PRM service, compatible="qcom,q6prm")
>                               └── clock-controller  (q6prmcc)
>                                       compatible = "qcom,q6prm-lpass-clocks"
> 
> The LPASS codec drivers (VA macro, WSA macro, RX macro, TX macro) are
> platform devices under soc@0 that reference q6prmcc as their clock
> supplier via DT phandles:
> 
>   From qcs6490-audioreach.dtsi:
>   ------------------------------
>   &lpass_va_macro {
>       clocks = <&q6prmcc LPASS_CLK_ID_VA_CORE_MCLK ...>,
>                <&q6prmcc LPASS_HW_MACRO_VOTE ...>,
>                <&q6prmcc LPASS_HW_DCODEC_VOTE ...>;
>       clock-names = "mclk", "macro", "dcodec";
>   };
> 
>   &lpass_wsa_macro {
>       clocks = <&q6prmcc LPASS_CLK_ID_TX_CORE_MCLK ...>,
>                ...;
>   };
> 
>   (similarly for lpass_rx_macro and lpass_tx_macro)
> 
> The intended fw_devlink dependency is therefore:
> 
>   lpass_va_macro  (consumer)  ──►  q6prmcc / clock-controller (supplier)
>   lpass_wsa_macro (consumer)  ──►  q6prmcc / clock-controller (supplier)
> 
> This dependency is critical for correct ADSP SSR (Subsystem Restart)
> teardown: all codec drivers must be torn down BEFORE q6prmcc is
> removed, so that no driver calls into a clock controller that no longer
> exists.
> 
> -----------------------------------------
> 2. THE INCORRECT ASSUMPTION IN fw_devlink
> 
> fw_devlink correctly identifies the intended supplier at boot time via
> fwnode links. However, the links are never converted to device links.
> Instead, fallback device links to the remoteproc device are created.
> 
> The sequence of events:
> 
>   1. At boot (~t=0.085s), fw_devlink resolves the DT clock phandles
>      and creates fwnode links:
>        lpass_va_macro  ──fwnode──►  gpr/service@2/clock-controller
>        lpass_wsa_macro ──fwnode──►  gpr/service@2/clock-controller
>      q6prmcc does not exist yet → EAGAIN → fwnode links kept pending.
>      This is correct.
> 
>   2. At ~t=7.597s, remoteproc_adsp probe() completes. This triggers
>      device_links_driver_bound() which calls
>      fw_devlink_pickup_dangling_consumers(remoteproc_dev).
> 
>   3. fw_devlink_pickup_dangling_consumers() walks remoteproc's child
>      fwnodes. It finds the gpr fwnode with no struct device attached.
> 
>   4. Here is the incorrect assumption:
>      __fw_devlink_pickup_dangling_consumers() concludes that because
>      gpr has no struct device at this moment, it will never get one.
>      It marks gpr as FWNODE_FLAG_NOT_DEVICE and moves all of gpr's
>      consumers (including the codec → q6prmcc fwnode links) up to
>      remoteproc.
> 
>   5. The correct fwnode links (codec → q6prmcc) are permanently
>      deleted. Wrong device links (codec → remoteproc) are created.
> 
>   6. At ~t=8.364s, the GPR bus probes and q6prmcc eventually appears.
>      But there are no pending fwnode links left for it — the correct
>      dependency is lost forever.
> 
> The assumption that is wrong:
> 
>   "If a child fwnode has no struct device when its ancestor device
>    binds, it will never get one."
> 
> This assumption does not hold when intermediate bus nodes (glink-edge,
> gpr) are populated asynchronously after their parent device boots remote
> firmware. At the time remoteproc binds, glink-edge and gpr have no
> struct device not because they will never exist, but because they are
> waiting for the ADSP firmware to load — which only happens after
> remoteproc's own probe() returns.

The issue is in __fw_devlink_pickup_dangling_consumers(). When remoteproc
binds, it walks its child fwnodes and collapses any that have no struct
device at that moment, assuming they will never have one. That assumption
is wrong for nodes behind a remoteproc that are populated only after 
firmware boots

The fix is to skip collapsing a fwnode if it has pending supplier links,
since that is a direct signal from the DT that the node is intended to
become a device.

Could you test the patch ? Will send the format patch upon
getting ack from you.

--------------------------------0<---------------------------------------

diff --git a/drivers/base/core.c b/drivers/base/core.c
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -228,6 +228,9 @@ static void __fw_devlink_pickup_dangling_consumers(...)
        if (fwnode->dev && fwnode->dev->bus)
                return;

+       if (!list_empty(&fwnode->consumers))
+               return;
+
        fwnode_set_flag(fwnode, FWNODE_FLAG_NOT_DEVICE);
        __fwnode_links_move_consumers(fwnode, new_sup);


-- 
-Mukesh Ojha