From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ej1-f65.google.com (mail-ej1-f65.google.com [209.85.218.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CE01C145B3E for ; Tue, 23 Dec 2025 22:16:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.65 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766528171; cv=none; b=IkFNxpzKxkKpSAMprqtZsewki9Lob7zJfn3IZaHtAKcSq+4VWwdEtVW3zYrrS0u0ls8NJJ1rX1zPnNExA5GQkKod+oIDMTUU/cPDvaaMdS3T55hODNZR8D5h6+VdyEbc2J0vlGv4UyZ9mtT9vPDZZEFdNaJX9nwTrUcmWq/n9os= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766528171; c=relaxed/simple; bh=yzbHwEjuUh5QG9F4g4O/Qg7p+yrh30QLZ0ZwvpnIAUw=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=fJHbpfhwK+E+uNH5EsycNozetTUUcn0YfewenCNHjxmSzIm3UEciL2QmK2Acpo3BVExwT4J+PtpwItXzFkfa7yXfMAFNG0RfGg3I4Hz9YdbbOKO/D7+qk3LtZWNKYgl4+qILQHHP98RkUk4vdmX6GWdQKURA1GC2qcvOOz2rkro= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=Jp9sk05v; arc=none smtp.client-ip=209.85.218.65 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="Jp9sk05v" Received: by mail-ej1-f65.google.com with SMTP id a640c23a62f3a-b7a6e56193cso899614766b.3 for ; Tue, 23 Dec 2025 14:16:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1766528167; x=1767132967; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=5UyYhutUWgIRgYB1MC5/PxDmOAUg51biGCRnLKpp0Ys=; b=Jp9sk05vgYeIga/DrZdJy/LczFORJYcFwM6vmYh/9mPk0Khw95Euo4sXNRVQ2M6XeM KB3H7M3JQbo0jhbENex6wmW3Z+J0FJvvvDKAbYjQUB9UHSKFRGRrpHO+BabqUjM6ypj2 /N+7nix1CMLsaKizKx3L7v2maKTAm0QoXYROALfr2ISPZOy3LHZ5icpfAiqQlbtlXtkY Fpm+hZV/4PpgOn55XcDp3xf7BFGX9W32ykmBb1hczp+YjKEk/tR610ci6rJXCiJdo+yp WtG61qomilXkBFdQPBYiDeHUtRgGS6FF67pdpMkCvGi+PUYN8xp8eSqMr1X9xGyW+cvM BPIg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1766528167; x=1767132967; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=5UyYhutUWgIRgYB1MC5/PxDmOAUg51biGCRnLKpp0Ys=; b=uDywzLPkJPM8rDxCFBrZcnjfTYxV5XkYINTFHJfwkhiI9ZvxLEQzBFCWyJVwqA+n+b gzGf/5Lamrat+qfqJQdfcNxvJ97/7o9q+r7e1SiKhFlzPQ0f+DZkadP4n50el1XH5Czq fUQyKtBoLc+fWAPTKUY9je1ZQer8sd3n4Fb3EFRa0YFVjk1nz284oPHtn91REH8y13HG MaZUUj0hwRBydufT26y92wd3CWtTZ035buYFYBSUXifeRuQ6AIVZovxeLj0n8H4W9h18 PTkaM2j95JY9Bk9wJNAhfbC9AHTXmxiHGzDz2vEpImKWayKV6M1m+CfEmvA/LCCAnBsk ig1Q== X-Forwarded-Encrypted: i=1; AJvYcCXQlQs4Jmeki3rmGgX65OBWNO9yFMPcowso2wbnLsV7x3Z7BzqMPpZikX341ViBSA1LKxkO6i+YcLNJUFc=@vger.kernel.org X-Gm-Message-State: AOJu0YyoAkkW8l0f8w9qIpRIW2FOoJFuOulDg/Y4xMZLOPYSCgAZXODM bxKbJzsqaqil2HAILdpX9p6wLnit7ed3RjEJveMUZfZpPahSazJLPazaI4gQSojH7x8= X-Gm-Gg: AY/fxX4+X+B1ff84kNhAW8z/m6TESiHA7rD34a1EyUylpAYxIkTSxJ5qQ6JidO0l7Ty x6xZNXE8X/JrkdtMQVg+68I1PCvCd6fbtuxFlRkNTz2PLwepSBzzrEy72VE9ZzgLac5agMq6wmM gSrDVYrfhpUnEkWA9Su8nChEWfEZ0g9oNeSOz5Zg5OR/p6EKx2YLeM46/EBg18G7vEyAGXcWW73 ZdKLVIfvinXioi3uIK6Oc1MOscvKzqWQxR/IAeitjIUr3HeIrEh9KAxvCO4b3mceFrDfHvhu5qL 3Z+1RxsDPJIT64XfO1+96LU3DCzWhaChtWC6nc5i17iVX9t1C5JGYhJFb6r1eesnDxjw8LuUoPv Md6QVW4nYDUDjoQn1GISBQnzIZt83rsClCzGs4LXS50kbl+HbmhqkBMCKGV0FfonO6qp2Hp1SCE 1gp4tDAGNdsX4Pt4N6 X-Google-Smtp-Source: AGHT+IGq+sIPac4pUDc5c2NSoUMS1PyA5G2WwEI/2p2f6em7Z3AmU60xOTfzeGCKspp1fvzaaAIBYw== X-Received: by 2002:a17:907:6ea4:b0:b80:f2e:6e1 with SMTP id a640c23a62f3a-b803722a7demr1668448866b.43.1766528167036; Tue, 23 Dec 2025 14:16:07 -0800 (PST) Received: from linaro.org ([77.64.146.193]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-b8037f4ef1fsm1556445466b.64.2025.12.23.14.16.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 Dec 2025 14:16:06 -0800 (PST) Date: Tue, 23 Dec 2025 23:15:33 +0100 From: Stephan Gerhold To: Jingyi Wang Cc: Bjorn Andersson , Mathieu Poirier , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Manivannan Sadhasivam , aiqun.yu@oss.qualcomm.com, tingwei.zhang@oss.qualcomm.com, trilok.soni@oss.qualcomm.com, yijie.yang@oss.qualcomm.com, linux-arm-msm@vger.kernel.org, linux-remoteproc@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Gokul krishna Krishnakumar Subject: Re: [PATCH v3 4/5] remoteproc: qcom: pas: Add late attach support for subsystems Message-ID: References: <20251223-knp-remoteproc-v3-0-5b09885c55a5@oss.qualcomm.com> <20251223-knp-remoteproc-v3-4-5b09885c55a5@oss.qualcomm.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20251223-knp-remoteproc-v3-4-5b09885c55a5@oss.qualcomm.com> On Tue, Dec 23, 2025 at 01:13:50AM -0800, Jingyi Wang wrote: > From: Gokul krishna Krishnakumar > > Subsystems can be brought out of reset by entities such as bootloaders. > As the irq enablement could be later than subsystem bring up, the state > of subsystem should be checked by reading SMP2P bits and performing ping > test. > > A new qcom_pas_attach() function is introduced. if a crash state is > detected for the subsystem, rproc_report_crash() is called. If the > subsystem is ready either at the first check or within a 5-second timeout > and the ping is successful, it will be marked as "attached". The ready > state could be set by either ready interrupt or handover interrupt. > > If "early_boot" is set by kernel but "subsys_booted" is not completed > within the timeout, It could be the early boot feature is not supported > by other entities. In this case, the state will be marked as RPROC_OFFLINE > so that the PAS driver can load the firmware and start the remoteproc. As > the running state is set once attach function is called, the watchdog or > fatal interrupt received can be handled correctly. > > Signed-off-by: Gokul krishna Krishnakumar > Co-developed-by: Jingyi Wang > Signed-off-by: Jingyi Wang > --- > drivers/remoteproc/qcom_q6v5.c | 87 ++++++++++++++++++++++++++++++++- > drivers/remoteproc/qcom_q6v5.h | 11 ++++- > drivers/remoteproc/qcom_q6v5_adsp.c | 2 +- > drivers/remoteproc/qcom_q6v5_mss.c | 2 +- > drivers/remoteproc/qcom_q6v5_pas.c | 97 ++++++++++++++++++++++++++++++++++++- > drivers/remoteproc/qcom_q6v5_wcss.c | 2 +- > 6 files changed, 195 insertions(+), 6 deletions(-) > > [...] > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c > index 52680ac99589..7e890e18dd82 100644 > --- a/drivers/remoteproc/qcom_q6v5_pas.c > +++ b/drivers/remoteproc/qcom_q6v5_pas.c > [...] > @@ -434,6 +440,85 @@ static unsigned long qcom_pas_panic(struct rproc *rproc) > return qcom_q6v5_panic(&pas->q6v5); > } > > +static int qcom_pas_attach(struct rproc *rproc) > +{ > + int ret; > + struct qcom_pas *pas = rproc->priv; > + bool ready_state; > + bool crash_state; > + > + pas->q6v5.running = true; > + ret = irq_get_irqchip_state(pas->q6v5.fatal_irq, > + IRQCHIP_STATE_LINE_LEVEL, &crash_state); > + > + if (ret) > + goto disable_running; > + > + if (crash_state) { > + dev_err(pas->dev, "Sub system has crashed before driver probe\n"); > + rproc_report_crash(rproc, RPROC_FATAL_ERROR); Have you tested this case? From quick review of the code in remoteproc_core.c I'm skeptical if this will work correctly: 1. Remoteproc is in RPROC_DETACHED state during auto boot 2. qcom_pas_attach() runs and calls rproc_report_crash(), then fails so RPROC_DETACHED state remains 3. rproc_crash_handler_work() sets RPROC_CRASHED and starts recovery 4. rproc_boot_recovery() calls rproc_stop() 5. rproc_stop() calls rproc_stop_subdevices(), but because the remoteproc was never attached, the subdevices were never started. In this situation, rproc_stop_subdevices() should not be called. I would expect you will need to make some minor changes to the remoteproc_core to support handling crashes during RPROC_DETACHED state. I might be reading the code wrong, but please make sure that you simulate this case at runtime and check that it works correctly. > + ret = -EINVAL; > + goto disable_running; > + } > + > + ret = irq_get_irqchip_state(pas->q6v5.ready_irq, > + IRQCHIP_STATE_LINE_LEVEL, &ready_state); > + > + if (ret) > + goto disable_running; > + > + enable_irq(pas->q6v5.handover_irq); > + > + if (unlikely(!ready_state)) { > + /* Set a 5 seconds timeout in case the early boot is delayed */ > + ret = wait_for_completion_timeout(&pas->q6v5.subsys_booted, > + msecs_to_jiffies(EARLY_ATTACH_TIMEOUT_MS)); > + Again, have you tested this case? As I already wrote in v2, I don't see how this case will work reliably in practice. How do you ensure that the handover resources will be kept on during the Linux boot process until the remoteproc has completed booting? Also, above you enable the handover_irq. Let's assume a handover IRQ does come in while you are waiting here. Then q6v5_handover_interrupt() will call q6v5->handover(q6v5); to disable the handover resources (clocks, power domains), but you never enabled those. I would expect that you get some bad reference count warnings in the kernel log. I would still suggest dropping this code entirely. As far as I understand the response from Aiqun(Maria) Yu [1], there is no real use case for this on current platforms. If you want to keep this, you would need to vote for the handover resources during probe() (and perhaps more, this case is quite tricky). Please test all your changes carefully in v4. Thanks, Stephan [1]: https://lore.kernel.org/linux-arm-msm/c15f083d-a2c1-462a-aad4-a72b36fbe1ac@oss.qualcomm.com/