From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 5ACA110A88D4 for ; Thu, 26 Mar 2026 16:20:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:References: List-Owner; bh=8YCVyC61HPPZGXgLKCyaDbxjMAkRJ9cAIyfwtOO2dGI=; b=LTXqWviAdLuwtQ L53CvS4H7Ul92xUmaBEP/D1vjAcyHqyHLfDLZjN7c9kUTLf9qp5gT6I3J1TMjcoNKIIyACqXD+4l6 VfZ+6lQBCLMiTBipAgCg0sYHP5s1BZOEkYnYCiBnkpsnm+qDvRVt9K+yosI7Ksp6bK9kWEzc8KjGl sS03OzKHLa1qSmMeUPFo0YnHlReiIuWPvYgIE1y9Pqq2Z/qQwILrEx1OuvCIKclusLgCu/zgyYk4F g2DUM1hU3OlcwHXzjlVQNI2uD5FiZIejw0OwO7t+r4gV2gKyZEnhIhOi1SZrZAR+RsgWoDXmxFyEs JdrB+zhwQMENt/6NOeJg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1w5nRf-00000005q1R-1YJV; Thu, 26 Mar 2026 16:20:31 +0000 Received: from tor.source.kernel.org ([2600:3c04:e001:324:0:1991:8:25]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1w5nRe-00000005q1F-0F0y for linux-arm-kernel@lists.infradead.org; Thu, 26 Mar 2026 16:20:30 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by tor.source.kernel.org (Postfix) with ESMTP id CF4E960053; Thu, 26 Mar 2026 16:19:58 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 58181C116C6; Thu, 26 Mar 2026 16:19:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1774541998; bh=HUtRwEpymq8y1tHT5IuCcbjZ8WgelRutIBJgFxgBfwY=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=F8mHPbZMLywhR2KPKiO47GUlQve1aWTvrP9jUY2zHBn/GgXJAJ8kIosO6KRmT2xPy 0IOXnbxBuiHeZMvRl+TjIGCZL+xLIklcHleCk7JhPz7DuyprdCRvXeIavUjsvsJn3u 2Q1RhEo14xk900QBnukOrxjvVlgBCi/2LncSYTzSdhsFA5hJ9IBKJY83E/k7nfnIj+ 5lTlIF6wm86KROEBrkkqhHauUmxF3ByqGulkPiq2SUiGMalg+b2lo7okZjYoEqA7SE DQIUYImcIBQS+JZCWgUDtLEZ0SBKAUGmugBagt425A+Pxg6+wxIOHa2m/AYEY0slHj ZzEGDAJHfpMMw== Date: Thu, 26 Mar 2026 11:19:57 -0500 From: Bjorn Helgaas To: Vijayanand Jitta , Richard Zhu , Lucas Stach Cc: Nipun Gupta , Nikhil Agarwal , Joerg Roedel , Will Deacon , Robin Murphy , Marc Zyngier , Lorenzo Pieralisi , Thomas Gleixner , Saravana Kannan , Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , Manivannan Sadhasivam , Bjorn Helgaas , Frank Li , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , Juergen Gross , Stefano Stabellini , Oleksandr Tyshchenko , Dmitry Baryshkov , Konrad Dybcio , Bjorn Andersson , Rob Herring , Conor Dooley , Krzysztof Kozlowski , Prakash Gupta , Vikash Garodia , linux-kernel@vger.kernel.org, iommu@lists.linux.dev, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, linux-pci@vger.kernel.org, imx@lists.linux.dev, xen-devel@lists.xenproject.org, linux-arm-msm@vger.kernel.org, Charan Teja Kalla Subject: Re: [PATCH v11 2/3] of: Factor arguments passed to of_map_id() into a struct Message-ID: <20260326161957.GA1324845@bhelgaas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260325-parse_iommu_cells-v11-2-1fefa5c0e82c@oss.qualcomm.com> X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org [cc->to: Richard, Lucas for pci-imx6.c question] On Wed, Mar 25, 2026 at 04:38:23PM +0530, Vijayanand Jitta wrote: > From: Charan Teja Kalla > > Change of_map_id() to take a pointer to struct of_phandle_args > instead of passing target device node and translated IDs separately. > Update all callers accordingly. > > Add an explicit filter_np parameter to of_map_id() and of_map_msi_id() > to separate the filter input from the output. Previously, the target > parameter served dual purpose: as an input filter (if non-NULL, only > match entries targeting that node) and as an output (receiving the > matched node with a reference held). Now filter_np is the explicit > input filter and arg->np is the pure output. > > Previously, of_map_id() would call of_node_put() on the matched node > when a filter was provided, making reference ownership inconsistent. > Remove this internal of_node_put() call so that of_map_id() now always > transfers ownership of the matched node reference to the caller via > arg->np. Callers are now consistently responsible for releasing this > reference with of_node_put(arg->np) when done. > ... Not actually part of *this* patch, and AFAICS this patch is correct as-is, but is it necessary to have different logic around of_node_put() for imx_pcie_add_lut_by_rid() and apple_pcie_enable_device()? > +++ b/drivers/pci/controller/dwc/pci-imx6.c > @@ -1137,6 +1137,8 @@ static void imx_pcie_remove_lut(struct imx_pcie *imx_pcie, u16 rid) > > static int imx_pcie_add_lut_by_rid(struct imx_pcie *imx_pcie, u32 rid) > { > + struct of_phandle_args iommu_spec = {}; > + struct of_phandle_args msi_spec = {}; > struct device *dev = imx_pcie->pci->dev; > struct device_node *target; > u32 sid_i, sid_m; > @@ -1144,7 +1146,12 @@ static int imx_pcie_add_lut_by_rid(struct imx_pcie *imx_pcie, u32 rid) > u32 sid = 0; > > target = NULL; > - err_i = of_map_iommu_id(dev->of_node, rid, &target, &sid_i); > + err_i = of_map_iommu_id(dev->of_node, rid, &iommu_spec); > + if (!err_i) { > + target = iommu_spec.np; > + sid_i = iommu_spec.args[0]; > + } > + > if (target) { > of_node_put(target); Here it's conditional on "target" even though of_node_put() checks internally for non-NULL, so it would be safe without the conditional here. > } else { > @@ -1156,8 +1163,11 @@ static int imx_pcie_add_lut_by_rid(struct imx_pcie *imx_pcie, u32 rid) > err_i = -EINVAL; > } > > - target = NULL; > - err_m = of_map_msi_id(dev->of_node, rid, &target, &sid_m); > + err_m = of_map_msi_id(dev->of_node, rid, NULL, &msi_spec); > + if (!err_m) { > + target = msi_spec.np; > + sid_m = msi_spec.args[0]; > + } > > /* > * err_m target And here (outside the diff context) we also call of_node_put() conditionally: ... else if (target) of_node_put(target); > diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c > index a0937b7b3c4d..c2cffc0659f4 100644 > --- a/drivers/pci/controller/pcie-apple.c > +++ b/drivers/pci/controller/pcie-apple.c > @@ -755,6 +755,7 @@ static int apple_pcie_enable_device(struct pci_host_bridge *bridge, struct pci_d > { > u32 sid, rid = pci_dev_id(pdev); > struct apple_pcie_port *port; > + struct of_phandle_args iommu_spec = {}; > int idx, err; > > port = apple_pcie_get_port(pdev); > @@ -764,10 +765,12 @@ static int apple_pcie_enable_device(struct pci_host_bridge *bridge, struct pci_d > dev_dbg(&pdev->dev, "added to bus %s, index %d\n", > pci_name(pdev->bus->self), port->idx); > > - err = of_map_iommu_id(port->pcie->dev->of_node, rid, NULL, &sid); > + err = of_map_iommu_id(port->pcie->dev->of_node, rid, &iommu_spec); > if (err) > return err; > > + of_node_put(iommu_spec.np); Here we call of_node_put() unconditionally. I think it would be much nicer if imx_pcie_add_lut_by_rid() used the same style as apple_pcie_enable_device() and did the of_node_put() unconditionally. That would untangle the function a bit and make it easier to analyze. > + sid = iommu_spec.args[0]; > mutex_lock(&port->pcie->lock); > > idx = bitmap_find_free_region(port->sid_map, port->sid_map_sz, 0);