linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 00/10] Add support for Hikey 970 PCIe
@ 2021-07-21  8:39 Mauro Carvalho Chehab
  2021-07-21  8:39 ` [PATCH v7 08/10] arm64: dts: HiSilicon: Add support for HiKey 970 PCIe controller hardware Mauro Carvalho Chehab
  2021-07-21  8:39 ` [PATCH v7 10/10] phy-hi3670-pcie: Move reset-gpios to the PCIe DT schema Mauro Carvalho Chehab
  0 siblings, 2 replies; 8+ messages in thread
From: Mauro Carvalho Chehab @ 2021-07-21  8:39 UTC (permalink / raw)
  To: Vinod Koul, Bjorn Helgaas, Rob Herring
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab,
	Krzysztof Wilczyński, Binghui Wang, Gustavo Pimentel,
	Jingoo Han, Rob Herring, Xiaowei Song, devicetree,
	linux-arm-kernel, linux-kernel, linux-pci, linux-phy

This series depends on this one:
	https://lore.kernel.org/lkml/cover.1626515862.git.mchehab+huawei@kernel.org/

It is available, with its patch dependencies against v5.14-rc1 at:
	https://github.com/mchehab/linux/commits/pcie-alternate

This series add support at the pcie-kirin dirver for it to use a separate PHY driver.

Yet, in order to preserve the existing DT schema for Kirin 960, it keeps the
Kirin 960 PHY inside the pci driver. I tried to find a way to split it while keeping
the DT schema backward-compatible, but currently the PHY core doesn't allow
that, as it relies on a "phy" property at the pcie node in order to recognize the
PHY driver.

Once the pci-kiring is modified to support an external PHY driver, add a
Kirin 970 PHY and add the needed properties for the HiKey 970 board to
detect the PCIe.

It should be noticed that the HiKey 970 design uses 4 different GPIO pins, one 
for each PERST# signal for each PCIe bus device:

    - GPIO 56 has a pullup logic from 1V8 to 2V5
      connected to a PCIe bridge chip (PEX 8606);
    - GPIO 25 has a pullup logic from 1V8 to 3V3
      connected to the PERST# pin at the M.2 slot;
    - GPIO 220 has a pullup logic from 1V8 to 3V3
      connected to the PERST# pin at the PCIe mini slot;
    - GPIO 203 has a pullup logic from 1V8 to 3V3
      connected to the PERST# pin at the Ethernet chipset.

At the first versions, those were mapped as part of the pci-bus, but the
pci-bus.yaml schema only allows a single PERST# GPIO. So, on v5, those
were moved to the PHY DT schema. However, as Rob complained, on this
version, I opted to add a separate patch (the last one) that moves those
back to the PCIe of-node. 

If such patch 09/09 is accepted, then this patch for the DT schema should 
also be accepted:

   https://github.com/devicetree-org/dt-schema/pull/56

Tested on Hikey970:

  $ lspci
  00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3670 (rev 01)
  01:00.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
  02:01.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
  02:04.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
  02:05.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
  02:07.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
  02:09.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
  06:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 07)

  $ ethtool enp6s0
  Settings for enp6s0:
	Supported ports: [ TP	 MII ]
	Supported link modes:   10baseT/Half 10baseT/Full
	                        100baseT/Half 100baseT/Full
	                        1000baseT/Half 1000baseT/Full
	Supported pause frame use: Symmetric Receive-only
	Supports auto-negotiation: Yes
	Supported FEC modes: Not reported
	Advertised link modes:  10baseT/Half 10baseT/Full
	                        100baseT/Half 100baseT/Full
	                        1000baseT/Half 1000baseT/Full
	Advertised pause frame use: Symmetric Receive-only
	Advertised auto-negotiation: Yes
	Advertised FEC modes: Not reported
	Link partner advertised link modes:  10baseT/Half 10baseT/Full
	                                     100baseT/Half 100baseT/Full
	Link partner advertised pause frame use: Symmetric Receive-only
	Link partner advertised auto-negotiation: Yes
	Link partner advertised FEC modes: Not reported
	Speed: 100Mb/s
	Duplex: Full
	Auto-negotiation: on
	master-slave cfg: preferred slave
	master-slave status: slave
	Port: Twisted Pair
	PHYAD: 0
	Transceiver: external
	MDI-X: Unknown
  netlink error: Operation not permitted
	Link detected: yes

Also tested  that Hikey 960 keeps being supported:

  $ lspci
  00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3660 (rev 01)

---

v7:
- Moved kirin_pcie_match to be closer to the probe function;
- Improved patch description for:
	"PCI: kirin: add support for a PHY layer"
- Added missing MODULE_*() macros on both PCI and PHY drivers;
- Fixed a warning at hisilicon,phy-hi3670-pcie.yaml reported by
  Rob Herring's bot.

v6:
- Use an alternative approach, in order to keep the Kirin 960 PHY internal to 
  the driver, in order to not break the DT schema. The PHY-specific code
  were made self-contained at pcie-kirin, in order to make easier to split
  it in the future, if needed.

v5:
- added "static" to hi3670_pcie_get_eyeparam() declaration on patch 6/8

v4:

- dropped the DTS patch, as it depends on a PMIC-related patch series;
- minor changes at the patch description;
- HiKey and HiSilicon are now using the preferred CamelCase format.


Manivannan Sadhasivam (1):
  arm64: dts: HiSilicon: Add support for HiKey 970 PCIe controller
    hardware

Mauro Carvalho Chehab (9):
  PCI: kirin: Reorganize the PHY logic inside the driver
  PCI: kirin: Add support for a PHY layer
  PCI: kirin: Use regmap for APB registers
  PCI: kirin: Add MODULE_* macros
  dt-bindings: PCI: kirin: Fix compatible string
  dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY
  phy: HiSilicon: Add driver for Kirin 970 PCIe PHY
  dt-bindings: PCI: kirin-pcie.txt: Convert it to yaml
  phy-hi3670-pcie: Move reset-gpios to the PCIe DT schema

 .../bindings/pci/hisilicon,kirin-pcie.yaml    |  87 ++
 .../devicetree/bindings/pci/kirin-pcie.txt    |  50 -
 .../devicetree/bindings/pci/snps,dw-pcie.yaml |   2 +-
 .../phy/hisilicon,phy-hi3670-pcie.yaml        |  88 ++
 MAINTAINERS                                   |   2 +-
 arch/arm64/boot/dts/hisilicon/hi3670.dtsi     |  70 ++
 .../boot/dts/hisilicon/hikey970-pmic.dtsi     |   1 -
 drivers/pci/controller/dwc/pcie-kirin.c       | 413 +++++---
 drivers/phy/hisilicon/Kconfig                 |  10 +
 drivers/phy/hisilicon/Makefile                |   1 +
 drivers/phy/hisilicon/phy-hi3670-pcie.c       | 902 ++++++++++++++++++
 11 files changed, 1429 insertions(+), 197 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
 delete mode 100644 Documentation/devicetree/bindings/pci/kirin-pcie.txt
 create mode 100644 Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml
 create mode 100644 drivers/phy/hisilicon/phy-hi3670-pcie.c

-- 
2.31.1



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v7 08/10] arm64: dts: HiSilicon: Add support for HiKey 970 PCIe controller hardware
  2021-07-21  8:39 [PATCH v7 00/10] Add support for Hikey 970 PCIe Mauro Carvalho Chehab
@ 2021-07-21  8:39 ` Mauro Carvalho Chehab
  2021-07-22 13:36   ` Manivannan Sadhasivam
  2021-08-16 18:26   ` Rob Herring
  2021-07-21  8:39 ` [PATCH v7 10/10] phy-hi3670-pcie: Move reset-gpios to the PCIe DT schema Mauro Carvalho Chehab
  1 sibling, 2 replies; 8+ messages in thread
From: Mauro Carvalho Chehab @ 2021-07-21  8:39 UTC (permalink / raw)
  To: Vinod Koul, Bjorn Helgaas, Rob Herring
  Cc: linuxarm, mauro.chehab, Manivannan Sadhasivam,
	Krzysztof Wilczyński, Binghui Wang, Lorenzo Pieralisi,
	Rob Herring, Wei Xu, Xiaowei Song, devicetree, linux-arm-kernel,
	linux-kernel, linux-pci, Mauro Carvalho Chehab

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

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 <manivannan.sadhasivam@linaro.org>
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>;
+		};
+
 		iomcu: iomcu@ffd7e000 {
 			compatible = "hisilicon,hi3670-iomcu", "syscon";
 			reg = <0x0 0xffd7e000 0x0 0x1000>;
@@ -660,6 +666,71 @@ gpio28: gpio@fff1d000 {
 			clock-names = "apb_pclk";
 		};
 
+		its_pcie: interrupt-controller@f4000000 {
+			compatible = "arm,gic-v3-its";
+			msi-controller;
+			reg = <0x0 0xf5100000 0x0 0x100000>;
+		};
+
+		pcie_phy: pcie-phy@fc000000 {
+			compatible = "hisilicon,hi970-pcie-phy";
+			reg = <0x0 0xfc000000 0x0 0x80000>;
+
+			phy-supply = <&ldo33>;
+
+			clocks = <&crg_ctrl HI3670_CLK_GATE_PCIEPHY_REF>,
+				 <&crg_ctrl HI3670_CLK_GATE_PCIEAUX>,
+				 <&crg_ctrl HI3670_PCLK_GATE_PCIE_PHY>,
+				 <&crg_ctrl HI3670_PCLK_GATE_PCIE_SYS>,
+				 <&crg_ctrl HI3670_ACLK_GATE_PCIE>;
+			clock-names = "phy_ref", "aux",
+				      "apb_phy", "apb_sys",
+				      "aclk";
+
+			reset-gpios = <&gpio7 0 0 >, <&gpio25 2 0 >,
+				      <&gpio3 1 0 >, <&gpio27 4 0 >;
+
+			clkreq-gpios = <&gpio20 6 0 >, <&gpio27 3 0 >,
+				       <&gpio17 0 0 >;
+
+			/* vboost iboost pre post main */
+			hisilicon,eye-diagram-param = <0xFFFFFFFF 0xFFFFFFFF
+						       0xFFFFFFFF 0xFFFFFFFF
+						       0xFFFFFFFF>;
+
+			#phy-cells = <0>;
+		};
+
+		pcie@f4000000 {
+			compatible = "hisilicon,kirin970-pcie";
+			reg = <0x0 0xf4000000 0x0 0x1000000>,
+			      <0x0 0xfc180000 0x0 0x1000>,
+			      <0x0 0xf5000000 0x0 0x2000>;
+			reg-names = "dbi", "apb", "config";
+			bus-range = <0x0  0x1>;
+			msi-parent = <&its_pcie>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+			device_type = "pci";
+			phys = <&pcie_phy>;
+			ranges = <0x02000000 0x0 0x00000000
+				  0x0 0xf6000000
+				  0x0 0x02000000>;
+			num-lanes = <1>;
+			#interrupt-cells = <1>;
+			interrupts = <0 283 4>;
+			interrupt-names = "msi";
+			interrupt-map-mask = <0 0 0 7>;
+			interrupt-map = <0x0 0 0 1
+					 &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
+					<0x0 0 0 2
+					 &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
+					<0x0 0 0 3
+					 &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
+					<0x0 0 0 4
+					 &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
+		};
+
 		/* UFS */
 		ufs: ufs@ff3c0000 {
 			compatible = "hisilicon,hi3670-ufs", "jedec,ufs-2.1";
diff --git a/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi b/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi
index 48c739eacba0..03452e627641 100644
--- a/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi
@@ -73,7 +73,6 @@ ldo33: LDO33 { /* PEX8606 */
 					regulator-name = "ldo33";
 					regulator-min-microvolt = <2500000>;
 					regulator-max-microvolt = <3300000>;
-					regulator-boot-on;
 				};
 
 				ldo34: LDO34 { /* GPS AUX IN VDD */
diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index bfc0513f7b15..9dad14929538 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -347,18 +347,6 @@ static const struct regmap_config pcie_kirin_regmap_conf = {
 	.reg_stride = 4,
 };
 
-/* Registers in PCIeCTRL */
-static inline void kirin_apb_ctrl_writel(struct kirin_pcie *kirin_pcie,
-					 u32 val, u32 reg)
-{
-	writel(val, kirin_pcie->apb_base + reg);
-}
-
-static inline u32 kirin_apb_ctrl_readl(struct kirin_pcie *kirin_pcie, u32 reg)
-{
-	return readl(kirin_pcie->apb_base + reg);
-}
-
 static long kirin_pcie_get_resource(struct kirin_pcie *kirin_pcie,
 				    struct platform_device *pdev)
 {
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v7 10/10] phy-hi3670-pcie: Move reset-gpios to the PCIe DT schema
  2021-07-21  8:39 [PATCH v7 00/10] Add support for Hikey 970 PCIe Mauro Carvalho Chehab
  2021-07-21  8:39 ` [PATCH v7 08/10] arm64: dts: HiSilicon: Add support for HiKey 970 PCIe controller hardware Mauro Carvalho Chehab
@ 2021-07-21  8:39 ` Mauro Carvalho Chehab
  1 sibling, 0 replies; 8+ messages in thread
From: Mauro Carvalho Chehab @ 2021-07-21  8:39 UTC (permalink / raw)
  To: Vinod Koul, Bjorn Helgaas, Rob Herring
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab,
	Kishon Vijay Abraham I, Manivannan Sadhasivam, Rob Herring,
	Wei Xu, devicetree, linux-arm-kernel, linux-kernel, linux-phy

The PHY interface as found on HiKey 970 uses 4 reset-gpios
instead of just one. That seems to be due to electrical
requirements, as, on HiKey 970, the PERST# signal is
provided via one GPIO per connected/available PCIe device:

- GPIO 56 has a pullup logic from 1V8 to 2V5
  connected to a PCIe bridge chip (PEX 8606);
- GPIO 25 has a pullup logic from 1V8 to 3V3
  connected to the PERST# pin at the M.2 slot;
- GPIO 220 has a pullup logic from 1V8 to 3V3
  connected to the PERST# pin at the PCIe mini slot;
- GPIO 203 has a pullup logic from 1V8 to 3V3
  connected to the PERST# pin at the Ethernet chipset.

Originally, this was mapped via the PHY interface, but, as such
design may also be used with different hardware, remap this
to use the pcie-bus DT schema.

This patch depends on a DT schema patch submitted at:
	https://github.com/devicetree-org/dt-schema/pull/56

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 .../phy/hisilicon,phy-hi3670-pcie.yaml        |  7 ---
 arch/arm64/boot/dts/hisilicon/hi3670.dtsi     |  5 +-
 drivers/phy/hisilicon/phy-hi3670-pcie.c       | 54 ++++++++++---------
 3 files changed, 31 insertions(+), 35 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml b/Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml
index a5ea13332cac..86767c53bc91 100644
--- a/Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml
+++ b/Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml
@@ -42,10 +42,6 @@ properties:
       - const: apb_sys
       - const: aclk
 
-  reset-gpios:
-    description: PCI PERST reset GPIOs
-    maxItems: 4
-
   clkreq-gpios:
     description: Clock request GPIOs
     maxItems: 3
@@ -60,7 +56,6 @@ required:
   - reg
   - clocks
   - clock-names
-  - reset-gpios
   - clkreq-gpios
   - hisilicon,eye-diagram-param
   - phy-supply
@@ -86,8 +81,6 @@ examples:
                  <&crg_ctrl HI3670_ACLK_GATE_PCIE>;
         clock-names = "phy_ref", "aux",
                       "apb_phy", "apb_sys", "aclk";
-        reset-gpios = <&gpio7 0 0 >, <&gpio25 2 0 >,
-                      <&gpio3 1 0 >, <&gpio27 4 0 >;
         clkreq-gpios = <&gpio20 6 0 >, <&gpio27 3 0 >, <&gpio17 0 0 >;
         hisilicon,eye-diagram-param = <0xFFFFFFFF 0xFFFFFFFF
                                        0xFFFFFFFF 0xFFFFFFFF 0xFFFFFFFF>;
diff --git a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
index 6dfcfcfeedae..a07790c76b72 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
@@ -687,9 +687,6 @@ pcie_phy: pcie-phy@fc000000 {
 				      "apb_phy", "apb_sys",
 				      "aclk";
 
-			reset-gpios = <&gpio7 0 0 >, <&gpio25 2 0 >,
-				      <&gpio3 1 0 >, <&gpio27 4 0 >;
-
 			clkreq-gpios = <&gpio20 6 0 >, <&gpio27 3 0 >,
 				       <&gpio17 0 0 >;
 
@@ -729,6 +726,8 @@ &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
 					 &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
 					<0x0 0 0 4
 					 &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
+			reset-gpios = <&gpio7 0 0 >, <&gpio25 2 0 >,
+				      <&gpio3 1 0 >, <&gpio27 4 0 >;
 		};
 
 		/* UFS */
diff --git a/drivers/phy/hisilicon/phy-hi3670-pcie.c b/drivers/phy/hisilicon/phy-hi3670-pcie.c
index 0bc0203e9140..82cc5fc4eac2 100644
--- a/drivers/phy/hisilicon/phy-hi3670-pcie.c
+++ b/drivers/phy/hisilicon/phy-hi3670-pcie.c
@@ -553,11 +553,13 @@ static int hi3670_pcie_noc_power(struct hi3670_pcie_phy *phy, bool enable)
 	return 0;
 }
 
-static int hi3670_pcie_get_apb(struct hi3670_pcie_phy *phy)
+static int hi3670_pcie_get_resources_from_pcie(struct hi3670_pcie_phy *phy)
 {
 	struct device_node *pcie_port;
 	struct device *dev = phy->dev;
 	struct device *pcie_dev;
+	char name[32];
+	int i;
 
 	pcie_port = of_get_child_by_name(dev->parent->of_node, "pcie");
 	if (!pcie_port) {
@@ -586,6 +588,27 @@ static int hi3670_pcie_get_apb(struct hi3670_pcie_phy *phy)
 		return -ENODEV;
 	}
 
+	/* perst reset gpios */
+	phy->n_gpio_resets = of_gpio_named_count(pcie_dev->of_node,
+						 "reset-gpios");
+	if (phy->n_gpio_resets > MAX_GPIO_RESETS) {
+		dev_err(dev, "Too many GPIO resets!\n");
+		return -EINVAL;
+	}
+	for (i = 0; i < phy->n_gpio_resets; i++) {
+		phy->gpio_id_reset[i] = of_get_named_gpio(pcie_dev->of_node,
+							  "reset-gpios", i);
+		if (phy->gpio_id_reset[i] < 0)
+			return phy->gpio_id_reset[i];
+
+		sprintf(name, "pcie_perst_%d", i);
+
+		phy->reset_names[i] = devm_kstrdup_const(dev, name,
+							 GFP_KERNEL);
+		if (!phy->reset_names[i])
+			return -ENOMEM;
+	}
+
 	return 0;
 }
 
@@ -644,16 +667,17 @@ static int hi3670_pcie_phy_init(struct phy *generic_phy)
 	int ret;
 
 	/*
-	 * The code under hi3670_pcie_get_apb() need to access the
-	 * DWC APB registers. So, get them from
-	 * the pcie driver's regmap (see pcie-kirin regmap).
+	 * The code under hi3670_pcie_get_resources_from_pcie() need to
+	 * access the reset-gpios and the APB registers, both from the
+	 * pcie-kirin driver.
 	 *
+	 * The APB is obtained via the pcie driver's regmap
 	 * Such kind of resource can only be obtained during the PCIe
 	 * power_on sequence, as the code inside pcie-kirin needs to
 	 * be already probed, as it needs to register the APB regmap.
 	 */
 
-	ret = hi3670_pcie_get_apb(phy);
+	ret = hi3670_pcie_get_resources_from_pcie(phy);
 	if (ret)
 		return ret;
 
@@ -800,26 +824,6 @@ static int hi3670_pcie_phy_get_resources(struct hi3670_pcie_phy *phy,
 	if (IS_ERR(phy->base))
 		return PTR_ERR(phy->base);
 
-	/* perst reset gpios */
-	phy->n_gpio_resets = of_gpio_named_count(np, "reset-gpios");
-	if (phy->n_gpio_resets > MAX_GPIO_RESETS) {
-		dev_err(dev, "Too many GPIO resets!\n");
-		return -EINVAL;
-	}
-	for (i = 0; i < phy->n_gpio_resets; i++) {
-		phy->gpio_id_reset[i] = of_get_named_gpio(dev->of_node,
-							  "reset-gpios", i);
-		if (phy->gpio_id_reset[i] < 0)
-			return phy->gpio_id_reset[i];
-
-		sprintf(name, "pcie_perst_%d", i);
-
-		phy->reset_names[i] = devm_kstrdup_const(dev, name,
-							 GFP_KERNEL);
-		if (!phy->reset_names[i])
-			return -ENOMEM;
-	}
-
 	/* clock request gpios */
 	phy->n_gpio_clkreq = of_gpio_named_count(np, "clkreq-gpios");
 	if (phy->n_gpio_clkreq > MAX_GPIO_CLKREQ) {
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v7 08/10] arm64: dts: HiSilicon: Add support for HiKey 970 PCIe controller hardware
  2021-07-21  8:39 ` [PATCH v7 08/10] arm64: dts: HiSilicon: Add support for HiKey 970 PCIe controller hardware Mauro Carvalho Chehab
@ 2021-07-22 13:36   ` Manivannan Sadhasivam
  2021-07-23  6:53     ` Mauro Carvalho Chehab
  2021-08-16 18:26   ` Rob Herring
  1 sibling, 1 reply; 8+ messages in thread
From: Manivannan Sadhasivam @ 2021-07-22 13:36 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Vinod Koul, Bjorn Helgaas, Rob Herring, linuxarm, mauro.chehab,
	Krzysztof Wilczyński, Binghui Wang, Lorenzo Pieralisi,
	Rob Herring, Wei Xu, Xiaowei Song, devicetree, linux-arm-kernel,
	linux-kernel, linux-pci

On Wed, Jul 21, 2021 at 10:39:10AM +0200, Mauro Carvalho Chehab wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> 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 <manivannan.sadhasivam@linaro.org>
> 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.

>  		iomcu: iomcu@ffd7e000 {
>  			compatible = "hisilicon,hi3670-iomcu", "syscon";
>  			reg = <0x0 0xffd7e000 0x0 0x1000>;
> @@ -660,6 +666,71 @@ gpio28: gpio@fff1d000 {
>  			clock-names = "apb_pclk";
>  		};
>  

[...]

> +			#interrupt-cells = <1>;
> +			interrupts = <0 283 4>;

Use the DT flag for interrupts instead of hardcoded value

> +			interrupt-names = "msi";
> +			interrupt-map-mask = <0 0 0 7>;
> +			interrupt-map = <0x0 0 0 1
> +					 &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
> +					<0x0 0 0 2
> +					 &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
> +					<0x0 0 0 3
> +					 &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
> +					<0x0 0 0 4
> +					 &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
> +		};
> +
>  		/* UFS */
>  		ufs: ufs@ff3c0000 {
>  			compatible = "hisilicon,hi3670-ufs", "jedec,ufs-2.1";
> diff --git a/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi b/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi
> index 48c739eacba0..03452e627641 100644
> --- a/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi
> @@ -73,7 +73,6 @@ ldo33: LDO33 { /* PEX8606 */
>  					regulator-name = "ldo33";
>  					regulator-min-microvolt = <2500000>;
>  					regulator-max-microvolt = <3300000>;
> -					regulator-boot-on;

Again, irrelevant.

>  				};
>  
>  				ldo34: LDO34 { /* GPS AUX IN VDD */
> diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
> index bfc0513f7b15..9dad14929538 100644
> --- a/drivers/pci/controller/dwc/pcie-kirin.c
> +++ b/drivers/pci/controller/dwc/pcie-kirin.c
> @@ -347,18 +347,6 @@ static const struct regmap_config pcie_kirin_regmap_conf = {
>  	.reg_stride = 4,
>  };
>  
> -/* Registers in PCIeCTRL */
> -static inline void kirin_apb_ctrl_writel(struct kirin_pcie *kirin_pcie,
> -					 u32 val, u32 reg)
> -{
> -	writel(val, kirin_pcie->apb_base + reg);
> -}
> -
> -static inline u32 kirin_apb_ctrl_readl(struct kirin_pcie *kirin_pcie, u32 reg)
> -{
> -	return readl(kirin_pcie->apb_base + reg);
> -}
> -

Same here...

Thanks,
Mani

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v7 08/10] arm64: dts: HiSilicon: Add support for HiKey 970 PCIe controller hardware
  2021-07-22 13:36   ` Manivannan Sadhasivam
@ 2021-07-23  6:53     ` Mauro Carvalho Chehab
  2021-07-24  4:11       ` Manivannan Sadhasivam
  0 siblings, 1 reply; 8+ messages in thread
From: Mauro Carvalho Chehab @ 2021-07-23  6:53 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Vinod Koul, Bjorn Helgaas, Rob Herring, linuxarm, mauro.chehab,
	Krzysztof Wilczyński, Binghui Wang, Lorenzo Pieralisi,
	Rob Herring, Wei Xu, Xiaowei Song, devicetree, linux-arm-kernel,
	linux-kernel, linux-pci

Em Thu, 22 Jul 2021 19:06:28 +0530
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> escreveu:

> On Wed, Jul 21, 2021 at 10:39:10AM +0200, Mauro Carvalho Chehab wrote:
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > 
> > 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 <manivannan.sadhasivam@linaro.org>
> > 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);



> 
> >  		iomcu: iomcu@ffd7e000 {
> >  			compatible = "hisilicon,hi3670-iomcu", "syscon";
> >  			reg = <0x0 0xffd7e000 0x0 0x1000>;
> > @@ -660,6 +666,71 @@ gpio28: gpio@fff1d000 {
> >  			clock-names = "apb_pclk";
> >  		};
> >    
> 
> [...]
> 
> > +			#interrupt-cells = <1>;
> > +			interrupts = <0 283 4>;  
> 
> Use the DT flag for interrupts instead of hardcoded value

Do you mean like this?

	interrupts = <0 283 IRQ_TYPE_LEVEL_HIGH>;

> 
> > +			interrupt-names = "msi";
> > +			interrupt-map-mask = <0 0 0 7>;
> > +			interrupt-map = <0x0 0 0 1
> > +					 &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
> > +					<0x0 0 0 2
> > +					 &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
> > +					<0x0 0 0 3
> > +					 &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
> > +					<0x0 0 0 4
> > +					 &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
> > +		};
> > +
> >  		/* UFS */
> >  		ufs: ufs@ff3c0000 {
> >  			compatible = "hisilicon,hi3670-ufs", "jedec,ufs-2.1";
> > diff --git a/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi b/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi
> > index 48c739eacba0..03452e627641 100644
> > --- a/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi
> > +++ b/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi
> > @@ -73,7 +73,6 @@ ldo33: LDO33 { /* PEX8606 */
> >  					regulator-name = "ldo33";
> >  					regulator-min-microvolt = <2500000>;
> >  					regulator-max-microvolt = <3300000>;
> > -					regulator-boot-on;  
> 
> Again, irrelevant.

I'll move it to the USB patch series, where the PMIC is added.

> 
> >  				};
> >  
> >  				ldo34: LDO34 { /* GPS AUX IN VDD */
> > diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
> > index bfc0513f7b15..9dad14929538 100644
> > --- a/drivers/pci/controller/dwc/pcie-kirin.c
> > +++ b/drivers/pci/controller/dwc/pcie-kirin.c
> > @@ -347,18 +347,6 @@ static const struct regmap_config pcie_kirin_regmap_conf = {
> >  	.reg_stride = 4,
> >  };
> >  
> > -/* Registers in PCIeCTRL */
> > -static inline void kirin_apb_ctrl_writel(struct kirin_pcie *kirin_pcie,
> > -					 u32 val, u32 reg)
> > -{
> > -	writel(val, kirin_pcie->apb_base + reg);
> > -}
> > -
> > -static inline u32 kirin_apb_ctrl_readl(struct kirin_pcie *kirin_pcie, u32 reg)
> > -{
> > -	return readl(kirin_pcie->apb_base + reg);
> > -}
> > -  
> 
> Same here...

This hunk should be on patch 03/10. Probably some rebase added it here by
mistake. I'll fix it on v8.

Thanks,
Mauro

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v7 08/10] arm64: dts: HiSilicon: Add support for HiKey 970 PCIe controller hardware
  2021-07-23  6:53     ` Mauro Carvalho Chehab
@ 2021-07-24  4:11       ` Manivannan Sadhasivam
  2021-08-03  4:25         ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 8+ messages in thread
From: Manivannan Sadhasivam @ 2021-07-24  4:11 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Vinod Koul, Bjorn Helgaas, Rob Herring, linuxarm, mauro.chehab,
	Krzysztof Wilczyński, Binghui Wang, Lorenzo Pieralisi,
	Rob Herring, Wei Xu, Xiaowei Song, devicetree, linux-arm-kernel,
	linux-kernel, linux-pci

On Fri, Jul 23, 2021 at 08:53:18AM +0200, Mauro Carvalho Chehab wrote:
> Em Thu, 22 Jul 2021 19:06:28 +0530
> Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> escreveu:
> 
> > On Wed, Jul 21, 2021 at 10:39:10AM +0200, Mauro Carvalho Chehab wrote:
> > > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > 
> > > 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 <manivannan.sadhasivam@linaro.org>
> > > 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.

> 
> > 
> > >  		iomcu: iomcu@ffd7e000 {
> > >  			compatible = "hisilicon,hi3670-iomcu", "syscon";
> > >  			reg = <0x0 0xffd7e000 0x0 0x1000>;
> > > @@ -660,6 +666,71 @@ gpio28: gpio@fff1d000 {
> > >  			clock-names = "apb_pclk";
> > >  		};
> > >    
> > 
> > [...]
> > 
> > > +			#interrupt-cells = <1>;
> > > +			interrupts = <0 283 4>;  
> > 
> > Use the DT flag for interrupts instead of hardcoded value
> 
> Do you mean like this?
> 
> 	interrupts = <0 283 IRQ_TYPE_LEVEL_HIGH>;
> 

yes but you could also use,

	interrupts = <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>;

Thanks,
Mani

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v7 08/10] arm64: dts: HiSilicon: Add support for HiKey 970 PCIe controller hardware
  2021-07-24  4:11       ` Manivannan Sadhasivam
@ 2021-08-03  4:25         ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 8+ messages in thread
From: Mauro Carvalho Chehab @ 2021-08-03  4:25 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Vinod Koul, Bjorn Helgaas, Rob Herring, linuxarm, mauro.chehab,
	Krzysztof Wilczyński, Binghui Wang, Lorenzo Pieralisi,
	Rob Herring, Wei Xu, Xiaowei Song, devicetree, linux-arm-kernel,
	linux-kernel, linux-pci

Em Sat, 24 Jul 2021 09:41:50 +0530
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> escreveu:

> On Fri, Jul 23, 2021 at 08:53:18AM +0200, Mauro Carvalho Chehab wrote:
> > Em Thu, 22 Jul 2021 19:06:28 +0530
> > Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> escreveu:
> >   
> > > On Wed, Jul 21, 2021 at 10:39:10AM +0200, Mauro Carvalho Chehab wrote:  
> > > > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > 
> > > > 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 <manivannan.sadhasivam@linaro.org>
> > > > 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v7 08/10] arm64: dts: HiSilicon: Add support for HiKey 970 PCIe controller hardware
  2021-07-21  8:39 ` [PATCH v7 08/10] arm64: dts: HiSilicon: Add support for HiKey 970 PCIe controller hardware Mauro Carvalho Chehab
  2021-07-22 13:36   ` Manivannan Sadhasivam
@ 2021-08-16 18:26   ` Rob Herring
  1 sibling, 0 replies; 8+ messages in thread
From: Rob Herring @ 2021-08-16 18:26 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Vinod Koul, Bjorn Helgaas, Linuxarm, mauro.chehab,
	Manivannan Sadhasivam, Krzysztof Wilczyński, Binghui Wang,
	Lorenzo Pieralisi, Wei Xu, Xiaowei Song, devicetree,
	linux-arm-kernel, linux-kernel, PCI

On Wed, Jul 21, 2021 at 3:39 AM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>
> 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 <manivannan.sadhasivam@linaro.org>
> 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>;
> +               };
> +
>                 iomcu: iomcu@ffd7e000 {
>                         compatible = "hisilicon,hi3670-iomcu", "syscon";
>                         reg = <0x0 0xffd7e000 0x0 0x1000>;
> @@ -660,6 +666,71 @@ gpio28: gpio@fff1d000 {
>                         clock-names = "apb_pclk";
>                 };
>
> +               its_pcie: interrupt-controller@f4000000 {
> +                       compatible = "arm,gic-v3-its";
> +                       msi-controller;
> +                       reg = <0x0 0xf5100000 0x0 0x100000>;

How does this h/w have a GIC-400 (which is GICv2) and then a GIC v3 ITS?

> +               };
> +
> +               pcie_phy: pcie-phy@fc000000 {
> +                       compatible = "hisilicon,hi970-pcie-phy";
> +                       reg = <0x0 0xfc000000 0x0 0x80000>;
> +
> +                       phy-supply = <&ldo33>;
> +
> +                       clocks = <&crg_ctrl HI3670_CLK_GATE_PCIEPHY_REF>,
> +                                <&crg_ctrl HI3670_CLK_GATE_PCIEAUX>,
> +                                <&crg_ctrl HI3670_PCLK_GATE_PCIE_PHY>,
> +                                <&crg_ctrl HI3670_PCLK_GATE_PCIE_SYS>,
> +                                <&crg_ctrl HI3670_ACLK_GATE_PCIE>;
> +                       clock-names = "phy_ref", "aux",
> +                                     "apb_phy", "apb_sys",
> +                                     "aclk";
> +
> +                       reset-gpios = <&gpio7 0 0 >, <&gpio25 2 0 >,
> +                                     <&gpio3 1 0 >, <&gpio27 4 0 >;
> +
> +                       clkreq-gpios = <&gpio20 6 0 >, <&gpio27 3 0 >,
> +                                      <&gpio17 0 0 >;
> +
> +                       /* vboost iboost pre post main */
> +                       hisilicon,eye-diagram-param = <0xFFFFFFFF 0xFFFFFFFF
> +                                                      0xFFFFFFFF 0xFFFFFFFF
> +                                                      0xFFFFFFFF>;
> +
> +                       #phy-cells = <0>;
> +               };
> +
> +               pcie@f4000000 {
> +                       compatible = "hisilicon,kirin970-pcie";
> +                       reg = <0x0 0xf4000000 0x0 0x1000000>,
> +                             <0x0 0xfc180000 0x0 0x1000>,
> +                             <0x0 0xf5000000 0x0 0x2000>;
> +                       reg-names = "dbi", "apb", "config";
> +                       bus-range = <0x0  0x1>;
> +                       msi-parent = <&its_pcie>;

This means the PCI host doesn't have a MSI controller...

> +                       #address-cells = <3>;
> +                       #size-cells = <2>;
> +                       device_type = "pci";
> +                       phys = <&pcie_phy>;
> +                       ranges = <0x02000000 0x0 0x00000000
> +                                 0x0 0xf6000000
> +                                 0x0 0x02000000>;
> +                       num-lanes = <1>;
> +                       #interrupt-cells = <1>;
> +                       interrupts = <0 283 4>;
> +                       interrupt-names = "msi";

But then this says it does...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-08-16 18:29 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-21  8:39 [PATCH v7 00/10] Add support for Hikey 970 PCIe Mauro Carvalho Chehab
2021-07-21  8:39 ` [PATCH v7 08/10] arm64: dts: HiSilicon: Add support for HiKey 970 PCIe controller hardware Mauro Carvalho Chehab
2021-07-22 13:36   ` Manivannan Sadhasivam
2021-07-23  6:53     ` Mauro Carvalho Chehab
2021-07-24  4:11       ` Manivannan Sadhasivam
2021-08-03  4:25         ` Mauro Carvalho Chehab
2021-08-16 18:26   ` Rob Herring
2021-07-21  8:39 ` [PATCH v7 10/10] phy-hi3670-pcie: Move reset-gpios to the PCIe DT schema Mauro Carvalho Chehab

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).