Thread (21 messages) flat view 21 messages, 3 authors, 2021-08-16

Re: [PATCH v7 08/10] arm64: dts: HiSilicon: Add support for HiKey 970 PCIe controller hardware

From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Date: 2021-08-03 04:27:02
Also in: linux-arm-kernel, linux-pci, lkml

Em Sat, 24 Jul 2021 09:41:50 +0530
Manivannan Sadhasivam [off-list ref] escreveu:
On Fri, Jul 23, 2021 at 08:53:18AM +0200, Mauro Carvalho Chehab wrote:
quoted
Em Thu, 22 Jul 2021 19:06:28 +0530
Manivannan Sadhasivam [off-list ref] escreveu:
  
quoted
On Wed, Jul 21, 2021 at 10:39:10AM +0200, Mauro Carvalho Chehab wrote:  
quoted
From: Manivannan Sadhasivam <redacted>

Add DTS bindings for the HiKey 970 board's PCIe hardware.

Co-developed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Manivannan Sadhasivam <redacted>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 arch/arm64/boot/dts/hisilicon/hi3670.dtsi     | 71 +++++++++++++++++++
 .../boot/dts/hisilicon/hikey970-pmic.dtsi     |  1 -
 drivers/pci/controller/dwc/pcie-kirin.c       | 12 ----
 3 files changed, 71 insertions(+), 13 deletions(-)
diff --git a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
index 1f228612192c..6dfcfcfeedae 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
@@ -177,6 +177,12 @@ sctrl: sctrl@fff0a000 {
 			#clock-cells = <1>;
 		};
 
+		pmctrl: pmctrl@fff31000 {
+			compatible = "hisilicon,hi3670-pmctrl", "syscon";
+			reg = <0x0 0xfff31000 0x0 0x1000>;
+			#clock-cells = <1>;
+		};
+    
Irrelevant change to this patch.  
Huh?

This is used by PCIe PHY, as part of the power on procedures:

	+static int hi3670_pcie_noc_power(struct hi3670_pcie_phy *phy, bool enable)
	+{
	+       struct device *dev = phy->dev;
	+       u32 time = 100;
	+       unsigned int val = NOC_PW_MASK;
	+       int rst;
	+
	+       if (enable)
	+               val = NOC_PW_MASK | NOC_PW_SET_BIT;
	+       else
	+               val = NOC_PW_MASK;
	+       rst = enable ? 1 : 0;
	+
	+       regmap_write(phy->pmctrl, NOC_POWER_IDLEREQ_1, val);

  
Ah... you're hardcoding the syscon compatible in driver. Sorry missed that.

But if these syscon nodes are independent memory regions or belong to non
PCI/PHY memory map, you could've fetched the reference through a DT property
along with the offset then used it in driver.

Like,

	pcie_phy: pcie-phy@fc000000 {
		...
		hisilicon,noc-power-regs = <&pmctrl 0x38c>;
		hisilicon,sctrl-cmos-regs = <&sctrl 0x60>;
		...
	};

The benefit of doing this way is, if the pmctrl, sctrl register layout changes
in future, you can handle it without any issues.
Interesting approach, but probably overkill. I mean, the register mapping
here should be the same for all Kirin 970 PHY based devices. A PHY for a 
different SoC will likely have other differences than just those two regs.

Regards,
Mauro
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help