From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.mindbit.ro (xs1.mindbit.ro [80.86.107.70]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E6A4C48124F for ; Wed, 5 Aug 2026 16:01:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=80.86.107.70 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785945680; cv=none; b=QtDcsUCQREwwwgDBRMFtzfOoIdqiLuXAv+FsG0Ur5x1yJ6p1SQOddnMgnszfwUJSmq9nEgf+wc+LH9Kkf7nHTSYAMQ2d6JAfT6/nOS0NkGroBqzIBkO6C/DW3uaq30c4nfZ06zAKQVeP7zOIJQ9zh2+JsLhn0J9sP4WpExBcvkY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785945680; c=relaxed/simple; bh=Ubi3+nowkLINSO+PcGCjm5kierYoispSBiInEJhyHdw=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References: Content-Type:MIME-Version; b=ceSAGLhAclIRgWp1Yzw4Nwa/liWvJsyz04myfZambcQGxe4BXHzTPiqetqgFiULLYl9IyF5iDMb0T9lpPr7xFxuLmUKH88WOJcAI1sXCwWJBZxuiMmWQju4lRZO+eHJh5uOcGxfjS4ZdP6xS3ZSILIoLqNM9eHx7Q6eEDnZd1Nk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=rendec.net; spf=pass smtp.mailfrom=rendec.net; dkim=pass (2048-bit key) header.d=rendec.net header.i=@rendec.net header.b=PNi6bp/O; arc=none smtp.client-ip=80.86.107.70 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=rendec.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=rendec.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=rendec.net header.i=@rendec.net header.b="PNi6bp/O" Received: from bat.kanata.rendec.net (unknown [24.114.108.137]) by mail.mindbit.ro (Postfix) with ESMTPSA id ECEAFC22CD; Wed, 5 Aug 2026 19:01:02 +0300 (EEST) DKIM-Filter: OpenDKIM Filter v2.11.0 mail.mindbit.ro ECEAFC22CD DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rendec.net; s=default; t=1785945664; bh=Z8FoKNUn7FgUz4HllsNiRKbAjVhDOXT7yBiHRpX9yP8=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=PNi6bp/Obrs1c+OIL9w1zpfcN34c4v5jE1Q+wRZDBWvCMpPWRNDqXdmgNYwzz93gg QsNqiy+AEo/lZT9idrtuIdfidYrbAbT16suL2pMYa5K61XKrBUrf4AEIvsar2jnm5Y OVD6AXM4HitWzqQmHwi7kojRnTmGt94U+xoEj1S26HssEH+cWACIi1ISzZzAp2ouDJ Ihdx/RcgkHtb42lXn12MCg46vA5pHIHDLvIXKlisFSZIHBKpwe3UGTpms847fu4OpS vmukHbRAOia0n2SvCsYNVyY3KL1B8ZJ3Maudl1JNrQC/4OKbDZ1SbTKp7joYoUyoaY 3BPIliqcVm9qw== Message-ID: <227049e031807e27863b3d6ae950de10cb2c3187.camel@rendec.net> Subject: Re: [PATCH v2] irqchip: imx-irqsteer: Convert to devm_pm_runtime_enable() From: Radu Rendec To: Fabio Estevam Cc: tglx@kernel.org, Frank.Li@nxp.com, imx@lists.linux.dev, linux-kernel@vger.kernel.org Date: Wed, 05 Aug 2026 12:00:59 -0400 In-Reply-To: References: <20260729221416.499781-1-festevam@gmail.com> <5ee0c572d017ca3ee3503c4e1901b8ac5d4f5966.camel@rendec.net> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.58.3 (3.58.3-1.fc43) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 On Tue, 2026-08-04 at 08:09 -0300, Fabio Estevam wrote: > On Sat, Aug 1, 2026 at 3:54=E2=80=AFPM Radu Rendec wrot= e: >=20 > > I take that back. At a closer look, I think both issues identified by > > Sashiko are valid. While the second issue is marked as "pre-existing", > > the proposed patch introduces a new path where the issue can occur > > (leaking the irq domain). >=20 > Would the first version of the patch be acceptable? It certainly avoids the problem of the extra error path that devm_pm_runtime_enable() introduces. But Sashiko had a comment on the first version too, and I think at least the first issue - about the double-disable of the clock (if the device is suspended when it's removed) - is real. In my (very limited) understanding of the runtime_pm framework, there are two ways to handle this cleanly: * Manage the activation and deactivation of the clock indirectly through suspend/resume requests. The driver already enables the clock in its resume callback and disables the clock in its suspend callback. Instead of enabling/disabling the clock explicitly in the probe/remove function, use pm_runtime_get_sync() and pm_runtime_put(), which indirectly resume and respectively suspend the device. This is what drivers/irqchip/irq-renesas-intc-irqpin.c does (except it doesn't manage a clock). * Call pm_runtime_get_sync() at the beginning of the remove function (which requires a matching pm_runtime_put() in the probe function). That will indirectly resume the device first, then it's safe to disable the clock. At a high level, the second option above would like this: static int imx_irqsteer_probe(struct platform_device *pdev) { /* Initial setup / clock enablement */ pm_runtime_set_active(&pdev->dev); pm_runtime_enable(&pdev->dev); /* Mark device idle so it can autosuspend when unused */ pm_runtime_put_autosuspend(&pdev->dev); return 0; } static void imx_irqsteer_remove(struct platform_device *pdev) { pm_runtime_get_sync(&pdev->dev); pm_runtime_disable(&pdev->dev); /* Final clean up and disable clock */ } That's closer to what the driver already does. But the part that I don't like is that the pm_runtime_put_autosuspend() call may suspend the device immediately, and I don't fully understand the implications of that.