Message ID | 20240129104958.1139787-1-s-vadapalli@ti.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-42567-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7301:2087:b0:106:209c:c626 with SMTP id gs7csp482065dyb; Mon, 29 Jan 2024 02:50:39 -0800 (PST) X-Google-Smtp-Source: AGHT+IH+mc6MU2jahWRLOPnst0cp6+7HrUMChYK2DsF9nHevllEEFyvJ73e9XhN9SLq+05HecO1W X-Received: by 2002:a17:906:16db:b0:a35:8bbd:5ba8 with SMTP id t27-20020a17090616db00b00a358bbd5ba8mr2437112ejd.25.1706525438872; Mon, 29 Jan 2024 02:50:38 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1706525438; cv=pass; d=google.com; s=arc-20160816; b=XJ4MedlLN1KsX375Wm9tym+L3W96YwQP8+A8tXd2oDl2I6O80BTffLNJbHCkdJfx0G gyD6kzUUG9Ni5ZznLwZjExWVoSiYgYEDFdVlbXmSiRL0c2I0XJH2opZrA7bIqLGvKyKk e/nAre5zf81ferGKICHOxDaS1eH6xMAc3cl6ypCsS/BMUo5zgEGAu17e++/OLklzQre8 3sat3kUUBxSPjhkTLcE5ihigftksw/mFjo5S5B2sMjAKPDDrnzYfBBovRK7vqFGSTcYZ nkOBc9S5jrU3l1f/DTXSQxMriB0LfhXBIwgpL+9yuph2zlW4mjPo/yynuh+cs3UGA+td yrFQ== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:cc:to :from:dkim-signature; bh=m0htI7go7or/yzNzVCGjz5hppU2wqyr8vuBJ7cUUKzs=; fh=YpvXH6/8mVaZWOcdM8a2NtLFTkMxsUF5Ys+ZpBjDZX0=; b=vVNmiwB+R9OQ6df4WwUWhc74IkN6W+rlsOuTTObhNXdiMzupLYkXI8sige4k4oE6fr dxfAiuawaQ2lJ6Qt2Ymqu+DHwijbcZQlPYQDp2OBRAKAbaBQ8oy90hZwbWeqzUNjUChL 6ZBGqdbsQR+5iY/R4DpDQ9KO0vmavMqXUga6MqBQ79Hv/qveY+YImHi7E/xjK4KFC/8z oqYAXkTh86PYYjiVdioQvRgeExfXgmDPUdIKhIKqlRSuZOns4sxSocupXfh2G6YDYieZ wBLr4yLsliEiR11kpz1I+H9RZmsdXMiIkWoUvjkL/h4bZLPErualR3dQ4QmKHsMDaQLf TXmA== ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@ti.com header.s=ti-com-17Q1 header.b=pq0MY3Db; arc=pass (i=1 spf=pass spfdomain=ti.com dkim=pass dkdomain=ti.com dmarc=pass fromdomain=ti.com); spf=pass (google.com: domain of linux-kernel+bounces-42567-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) smtp.mailfrom="linux-kernel+bounces-42567-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=QUARANTINE sp=NONE dis=NONE) header.from=ti.com Received: from am.mirrors.kernel.org (am.mirrors.kernel.org. [147.75.80.249]) by mx.google.com with ESMTPS id l8-20020a17090612c800b00a27c14be748si3343169ejb.920.2024.01.29.02.50.38 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 29 Jan 2024 02:50:38 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-42567-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249; Authentication-Results: mx.google.com; dkim=pass header.i=@ti.com header.s=ti-com-17Q1 header.b=pq0MY3Db; arc=pass (i=1 spf=pass spfdomain=ti.com dkim=pass dkdomain=ti.com dmarc=pass fromdomain=ti.com); spf=pass (google.com: domain of linux-kernel+bounces-42567-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) smtp.mailfrom="linux-kernel+bounces-42567-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=QUARANTINE sp=NONE dis=NONE) header.from=ti.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by am.mirrors.kernel.org (Postfix) with ESMTPS id 294B71F235F7 for <ouuuleilei@gmail.com>; Mon, 29 Jan 2024 10:50:38 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 780F757866; Mon, 29 Jan 2024 10:50:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=ti.com header.i=@ti.com header.b="pq0MY3Db" Received: from lelv0142.ext.ti.com (lelv0142.ext.ti.com [198.47.23.249]) (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 BD4A956469; Mon, 29 Jan 2024 10:50:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.47.23.249 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706525423; cv=none; b=fT2wP1YzzGquvFOCNoDc75mQREtd2RCnYkV4yqOAtdpADIbMnqwNoqZGYPE3ez1wFgGjy0l1VrilTloz8YTQ5KK3+oVTKwqrI8nTDmNrEbfV9jiuMZedUde5bs+wLLTZYWH26JsKoPSFDKjf4l6uKOhlnpP0hUtTHAC+2i/ynbE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706525423; c=relaxed/simple; bh=je4BCKddaeJFCgeD8nlqFK65Ey9jQCjtMpDtUG1yxqQ=; h=From:To:CC:Subject:Date:Message-ID:MIME-Version:Content-Type; b=t1+1pACAi6hFvkVYQB4MlYWg5XAEfMoxqTnKIHUV+4lFXOzXUIbtz6XygAQqaMZzcQMtzyHEdqnJ0EOPkaHaRwtaDuVQjrJ7WaANhpPCtMbN4a7ZEWVEGDtXe2KpnptejoMurGCP4MxiGLrKa8fTtTJftkwXhhvrg8lTSXLJ/zY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=ti.com; spf=pass smtp.mailfrom=ti.com; dkim=pass (1024-bit key) header.d=ti.com header.i=@ti.com header.b=pq0MY3Db; arc=none smtp.client-ip=198.47.23.249 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=ti.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ti.com Received: from lelv0265.itg.ti.com ([10.180.67.224]) by lelv0142.ext.ti.com (8.15.2/8.15.2) with ESMTP id 40TAo3CC022630; Mon, 29 Jan 2024 04:50:03 -0600 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1706525403; bh=m0htI7go7or/yzNzVCGjz5hppU2wqyr8vuBJ7cUUKzs=; h=From:To:CC:Subject:Date; b=pq0MY3DbCuwjGFSt3hP10A1JzEymc3OC0rI+bosIlb/TCrzTgTjdsrYhJSPOGuqoF 0l0/lPPOGrYdeHoEcJJbi1tOKkGNDDqgyPEmUEOLAfpmeiGhMueuVSxeIR+6KV2oSn D8HSvHThPk9E+6O0Pt0p0M9XBXhO1sHICKx/OGjI= Received: from DLEE111.ent.ti.com (dlee111.ent.ti.com [157.170.170.22]) by lelv0265.itg.ti.com (8.15.2/8.15.2) with ESMTPS id 40TAo3hD030235 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=FAIL); Mon, 29 Jan 2024 04:50:03 -0600 Received: from DLEE107.ent.ti.com (157.170.170.37) by DLEE111.ent.ti.com (157.170.170.22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.23; Mon, 29 Jan 2024 04:50:02 -0600 Received: from lelvsmtp5.itg.ti.com (10.180.75.250) by DLEE107.ent.ti.com (157.170.170.37) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.23 via Frontend Transport; Mon, 29 Jan 2024 04:50:03 -0600 Received: from uda0492258.dhcp.ti.com (uda0492258.dhcp.ti.com [172.24.227.9]) by lelvsmtp5.itg.ti.com (8.15.2/8.15.2) with ESMTP id 40TAnxhF088113; Mon, 29 Jan 2024 04:49:59 -0600 From: Siddharth Vadapalli <s-vadapalli@ti.com> To: <bhelgaas@google.com>, <lpieralisi@kernel.org>, <kw@linux.com>, <robh@kernel.org>, <vigneshr@ti.com> CC: <linux-pci@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>, <afd@ti.com>, <srk@ti.com>, <s-vadapalli@ti.com> Subject: [PATCH] PCI: j721e: Extend j721e_pcie_ctrl_init() for non syscon nodes Date: Mon, 29 Jan 2024 16:19:58 +0530 Message-ID: <20240129104958.1139787-1-s-vadapalli@ti.com> X-Mailer: git-send-email 2.34.1 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1789421618272019587 X-GMAIL-MSGID: 1789421618272019587 |
Series |
PCI: j721e: Extend j721e_pcie_ctrl_init() for non syscon nodes
|
|
Commit Message
Siddharth Vadapalli
Jan. 29, 2024, 10:49 a.m. UTC
The "ti,syscon-pcie-ctrl" device-tree property is used to obtain
reference to the "pcie_ctrl" register within the System Controller Module
in order to configure the link speed, number of lanes and the mode of
operation of the PCIe controller. The existing implementation of the
"j721e_pcie_ctrl_init()" function handles the case where the compatible for
the System Controller Module node specified using the "ti,syscon-pcie-ctrl"
property is "syscon". Since the System Controller Module can be modelled as
a "simple-bus" as well, extend the implementation of the
"j721e_pcie_ctrl_init()" function to handle the "simple-bus" case.
Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
Hello,
This patch is based on linux-next tagged next-20240129.
The System Controller Module is modelled as a "simple-bus" in J784S4 SoC at
https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi#L45
The existing SoCs such as J721E and J7200 which currently model the node as
a Syscon node will also be updated to model it as a "simple-bus".
Therefore this patch aims to update the driver in order to handle the
migration of the System Controller Module to the "simple-bus" compatible
without breaking PCIe functionality on existing TI SoCs which make use
of the pci-j721e.c driver.
Regards,
Siddharth.
drivers/pci/controller/cadence/pci-j721e.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
Comments
On 1/29/24 4:49 AM, Siddharth Vadapalli wrote: > The "ti,syscon-pcie-ctrl" device-tree property is used to obtain > reference to the "pcie_ctrl" register within the System Controller Module > in order to configure the link speed, number of lanes and the mode of > operation of the PCIe controller. The existing implementation of the > "j721e_pcie_ctrl_init()" function handles the case where the compatible for > the System Controller Module node specified using the "ti,syscon-pcie-ctrl" > property is "syscon". Since the System Controller Module can be modelled as > a "simple-bus" as well, extend the implementation of the > "j721e_pcie_ctrl_init()" function to handle the "simple-bus" case. > > Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com> > --- > > Hello, > > This patch is based on linux-next tagged next-20240129. > > The System Controller Module is modelled as a "simple-bus" in J784S4 SoC at > https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi#L45 > The existing SoCs such as J721E and J7200 which currently model the node as > a Syscon node will also be updated to model it as a "simple-bus". > Therefore this patch aims to update the driver in order to handle the > migration of the System Controller Module to the "simple-bus" compatible > without breaking PCIe functionality on existing TI SoCs which make use > of the pci-j721e.c driver. > > Regards, > Siddharth. > > drivers/pci/controller/cadence/pci-j721e.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c > index 85718246016b..2ace7e78a880 100644 > --- a/drivers/pci/controller/cadence/pci-j721e.c > +++ b/drivers/pci/controller/cadence/pci-j721e.c > @@ -224,12 +224,20 @@ static int j721e_pcie_ctrl_init(struct j721e_pcie *pcie) > { > struct device *dev = pcie->cdns_pcie->dev; > struct device_node *node = dev->of_node; > + struct device_node *scm_conf; > struct of_phandle_args args; > unsigned int offset = 0; > struct regmap *syscon; > int ret; > > - syscon = syscon_regmap_lookup_by_phandle(node, "ti,syscon-pcie-ctrl"); > + scm_conf = of_parse_phandle(node, "ti,syscon-pcie-ctrl", 0); > + if (!scm_conf) { > + dev_err(dev, "unable to get System Controller node\n"); > + return -ENODEV; > + } > + > + syscon = device_node_to_regmap(scm_conf); Turning the entire "simple-bus" region into a regmap using this function is just as broken as having it as a "syscon". The core problem we are solving by getting rid of the blanket syscon nodes is that it causes multiple mappings of the same register. This can cause issues with regmap caching, read–modify–write, etc.. What you want to do is add a subnode to the simple-bus, have that encapsulate just the registers used for PCIe, and have the PCIe node point to that. Then this patch isn't needed. For an example, see how it's done for DSS[0]. Andrew [0] https://github.com/torvalds/linux/blob/41bccc98fb7931d63d03f326a746ac4d429c1dd3/arch/arm64/boot/dts/ti/k3-am65-main.dtsi#L502 > + of_node_put(scm_conf); > if (IS_ERR(syscon)) { > dev_err(dev, "Unable to get ti,syscon-pcie-ctrl regmap\n"); > return PTR_ERR(syscon);
Hello Andrew, On 29/01/24 20:49, Andrew Davis wrote: > On 1/29/24 4:49 AM, Siddharth Vadapalli wrote: .. >> int ret; >> - syscon = syscon_regmap_lookup_by_phandle(node, "ti,syscon-pcie-ctrl"); >> + scm_conf = of_parse_phandle(node, "ti,syscon-pcie-ctrl", 0); >> + if (!scm_conf) { >> + dev_err(dev, "unable to get System Controller node\n"); >> + return -ENODEV; >> + } >> + >> + syscon = device_node_to_regmap(scm_conf); > > Turning the entire "simple-bus" region into a regmap using this > function is just as broken as having it as a "syscon". The core > problem we are solving by getting rid of the blanket syscon nodes > is that it causes multiple mappings of the same register. This > can cause issues with regmap caching, read–modify–write, etc.. > > What you want to do is add a subnode to the simple-bus, have that > encapsulate just the registers used for PCIe, and have the PCIe > node point to that. Then this patch isn't needed. > > For an example, see how it's done for DSS[0]. Thank you for reviewing the patch. I will implement it similar to what's done for DSS as you pointed out. However, what about the existing SoCs which make use of the "ti,syscon-pcie-ctrl" property? Do you suggest that I add another device-tree property for pointing to the PCIE_CTRL register within the CTRL_MMR region, or do you suggest that I reuse the existing "ti,syscon-pcie-ctrl" property differently in the SoCs like J784S4 where the scm_conf node is a "simple-bus"? The "ti,syscon-pcie-ctrl" property as defined in the device-tree bindings has two elements with the first being the phandle to the scm_conf node and the second being the offset of the PCIE_CTRL register. The newer implementation you are suggesting will either require a new property which accepts only one element namely the phandle to the node within scm_conf corresponding to the PCIE_CTRL register. Will adding a new property be acceptable? ..
On 1/29/24 10:50 PM, Siddharth Vadapalli wrote: > Hello Andrew, > > On 29/01/24 20:49, Andrew Davis wrote: >> On 1/29/24 4:49 AM, Siddharth Vadapalli wrote: > > ... > >>> int ret; >>> - syscon = syscon_regmap_lookup_by_phandle(node, "ti,syscon-pcie-ctrl"); >>> + scm_conf = of_parse_phandle(node, "ti,syscon-pcie-ctrl", 0); >>> + if (!scm_conf) { >>> + dev_err(dev, "unable to get System Controller node\n"); >>> + return -ENODEV; >>> + } >>> + >>> + syscon = device_node_to_regmap(scm_conf); >> >> Turning the entire "simple-bus" region into a regmap using this >> function is just as broken as having it as a "syscon". The core >> problem we are solving by getting rid of the blanket syscon nodes >> is that it causes multiple mappings of the same register. This >> can cause issues with regmap caching, read–modify–write, etc.. >> >> What you want to do is add a subnode to the simple-bus, have that >> encapsulate just the registers used for PCIe, and have the PCIe >> node point to that. Then this patch isn't needed. >> >> For an example, see how it's done for DSS[0]. > > Thank you for reviewing the patch. I will implement it similar to what's done > for DSS as you pointed out. However, what about the existing SoCs which make use > of the "ti,syscon-pcie-ctrl" property? Do you suggest that I add another > device-tree property for pointing to the PCIE_CTRL register within the CTRL_MMR > region, or do you suggest that I reuse the existing "ti,syscon-pcie-ctrl" > property differently in the SoCs like J784S4 where the scm_conf node is a > "simple-bus"? > > The "ti,syscon-pcie-ctrl" property as defined in the device-tree bindings has > two elements with the first being the phandle to the scm_conf node and the > second being the offset of the PCIE_CTRL register. The newer implementation you > are suggesting will either require a new property which accepts only one element > namely the phandle to the node within scm_conf corresponding to the PCIE_CTRL > register. Will adding a new property be acceptable? > Why would you need a new property? If there is no offset to the PCIE_CTRL register in the new syscon space then just set the offset = 0x0, easy. Andrew
On 30/01/24 20:30, Andrew Davis wrote: > On 1/29/24 10:50 PM, Siddharth Vadapalli wrote: >> Hello Andrew, >> >> On 29/01/24 20:49, Andrew Davis wrote: >>> On 1/29/24 4:49 AM, Siddharth Vadapalli wrote: >> >> ... >> >>>> int ret; >>>> - syscon = syscon_regmap_lookup_by_phandle(node, "ti,syscon-pcie-ctrl"); >>>> + scm_conf = of_parse_phandle(node, "ti,syscon-pcie-ctrl", 0); >>>> + if (!scm_conf) { >>>> + dev_err(dev, "unable to get System Controller node\n"); >>>> + return -ENODEV; >>>> + } >>>> + >>>> + syscon = device_node_to_regmap(scm_conf); >>> >>> Turning the entire "simple-bus" region into a regmap using this >>> function is just as broken as having it as a "syscon". The core >>> problem we are solving by getting rid of the blanket syscon nodes >>> is that it causes multiple mappings of the same register. This >>> can cause issues with regmap caching, read–modify–write, etc.. >>> >>> What you want to do is add a subnode to the simple-bus, have that >>> encapsulate just the registers used for PCIe, and have the PCIe >>> node point to that. Then this patch isn't needed. >>> >>> For an example, see how it's done for DSS[0]. >> >> Thank you for reviewing the patch. I will implement it similar to what's done >> for DSS as you pointed out. However, what about the existing SoCs which make use >> of the "ti,syscon-pcie-ctrl" property? Do you suggest that I add another >> device-tree property for pointing to the PCIE_CTRL register within the CTRL_MMR >> region, or do you suggest that I reuse the existing "ti,syscon-pcie-ctrl" >> property differently in the SoCs like J784S4 where the scm_conf node is a >> "simple-bus"? >> >> The "ti,syscon-pcie-ctrl" property as defined in the device-tree bindings has >> two elements with the first being the phandle to the scm_conf node and the >> second being the offset of the PCIE_CTRL register. The newer implementation you >> are suggesting will either require a new property which accepts only one element >> namely the phandle to the node within scm_conf corresponding to the PCIE_CTRL >> register. Will adding a new property be acceptable? >> > > Why would you need a new property? If there is no offset to the PCIE_CTRL register > in the new syscon space then just set the offset = 0x0, easy. Seems like a Hack to me considering that the offset will always be zero for non-syscon parent nodes (which will be the convention going forward), implying that the offset might as well be dropped and just the phandle is sufficient. For now I shall implement it as you suggested. Maybe once the syscon nodes in existing SoCs are also converted to simple-bus, then the property can be redefined to just be the phandle to "pcie_ctrl" register. > > Andrew
On 1/30/24 10:53 PM, Siddharth Vadapalli wrote: > > > On 30/01/24 20:30, Andrew Davis wrote: >> On 1/29/24 10:50 PM, Siddharth Vadapalli wrote: >>> Hello Andrew, >>> >>> On 29/01/24 20:49, Andrew Davis wrote: >>>> On 1/29/24 4:49 AM, Siddharth Vadapalli wrote: >>> >>> ... >>> >>>>> int ret; >>>>> - syscon = syscon_regmap_lookup_by_phandle(node, "ti,syscon-pcie-ctrl"); >>>>> + scm_conf = of_parse_phandle(node, "ti,syscon-pcie-ctrl", 0); >>>>> + if (!scm_conf) { >>>>> + dev_err(dev, "unable to get System Controller node\n"); >>>>> + return -ENODEV; >>>>> + } >>>>> + >>>>> + syscon = device_node_to_regmap(scm_conf); >>>> >>>> Turning the entire "simple-bus" region into a regmap using this >>>> function is just as broken as having it as a "syscon". The core >>>> problem we are solving by getting rid of the blanket syscon nodes >>>> is that it causes multiple mappings of the same register. This >>>> can cause issues with regmap caching, read–modify–write, etc.. >>>> >>>> What you want to do is add a subnode to the simple-bus, have that >>>> encapsulate just the registers used for PCIe, and have the PCIe >>>> node point to that. Then this patch isn't needed. >>>> >>>> For an example, see how it's done for DSS[0]. >>> >>> Thank you for reviewing the patch. I will implement it similar to what's done >>> for DSS as you pointed out. However, what about the existing SoCs which make use >>> of the "ti,syscon-pcie-ctrl" property? Do you suggest that I add another >>> device-tree property for pointing to the PCIE_CTRL register within the CTRL_MMR >>> region, or do you suggest that I reuse the existing "ti,syscon-pcie-ctrl" >>> property differently in the SoCs like J784S4 where the scm_conf node is a >>> "simple-bus"? >>> >>> The "ti,syscon-pcie-ctrl" property as defined in the device-tree bindings has >>> two elements with the first being the phandle to the scm_conf node and the >>> second being the offset of the PCIE_CTRL register. The newer implementation you >>> are suggesting will either require a new property which accepts only one element >>> namely the phandle to the node within scm_conf corresponding to the PCIE_CTRL >>> register. Will adding a new property be acceptable? >>> >> >> Why would you need a new property? If there is no offset to the PCIE_CTRL register >> in the new syscon space then just set the offset = 0x0, easy. > > Seems like a Hack to me considering that the offset will always be zero for > non-syscon parent nodes (which will be the convention going forward), implying > that the offset might as well be dropped and just the phandle is sufficient. For If we check the git history, this is actually how it used to be. The offset stuff was added later[0]. Looks like for backwards compat it still works to not provide an offset. Andrew [0] 7aa256234c4c ("PCI: j721e: Get offset within "syscon" from "ti,syscon-pcie-ctrl" phandle arg") > now I shall implement it as you suggested. Maybe once the syscon nodes in > existing SoCs are also converted to simple-bus, then the property can be > redefined to just be the phandle to "pcie_ctrl" register. > >> >> Andrew >
diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c index 85718246016b..2ace7e78a880 100644 --- a/drivers/pci/controller/cadence/pci-j721e.c +++ b/drivers/pci/controller/cadence/pci-j721e.c @@ -224,12 +224,20 @@ static int j721e_pcie_ctrl_init(struct j721e_pcie *pcie) { struct device *dev = pcie->cdns_pcie->dev; struct device_node *node = dev->of_node; + struct device_node *scm_conf; struct of_phandle_args args; unsigned int offset = 0; struct regmap *syscon; int ret; - syscon = syscon_regmap_lookup_by_phandle(node, "ti,syscon-pcie-ctrl"); + scm_conf = of_parse_phandle(node, "ti,syscon-pcie-ctrl", 0); + if (!scm_conf) { + dev_err(dev, "unable to get System Controller node\n"); + return -ENODEV; + } + + syscon = device_node_to_regmap(scm_conf); + of_node_put(scm_conf); if (IS_ERR(syscon)) { dev_err(dev, "Unable to get ti,syscon-pcie-ctrl regmap\n"); return PTR_ERR(syscon);