Add support for Hikey 970 PCIe
Subsystems (6)
v5 cover Add support for Hikey 970 PCIe
As requested by Rob Herring, this series split the PHY part into a separate driver. Then, it adds support for Kirin 970 on a single patch.
With this change, the PHY-specific device tree bindings for Kirin 960 moved to its own PHY properties.
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
Partially tested on Hikey 960[1]:
$ lspci 00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3660 (rev 01)
[1] The Hikey 960 doesn't come with any internal PCIe device.
Its hardware supports just an external device via a M.2 slot that doesn't support SATA. I ordered a NVMe device to test, but the vendor is currently out of supply. It should take 3-4 weeks to arrive here. I'll run an extra test on it once it arrives.
---
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.
Mauro Carvalho Chehab (8):
dt-bindings: phy: Add bindings for HiKey 960 PCIe PHY
dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY
dt-bindings: PCI: kirin: Fix compatible string
dt-bindings: PCI: kirin: Drop PHY properties
phy: HiSilicon: Add driver for Kirin 960 PCIe PHY
phy: HiSilicon: add driver for Kirin 970 PCIe PHY
PCI: kirin: Drop the PHY logic from the driver
PCI: kirin: Use regmap for APB registers
.../devicetree/bindings/pci/kirin-pcie.txt | 21 +- .../phy/hisilicon,phy-hi3660-pcie.yaml | 82 ++ .../phy/hisilicon,phy-hi3670-pcie.yaml | 101 ++ arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 29 +- drivers/pci/controller/dwc/pcie-kirin.c | 298 ++---- drivers/phy/hisilicon/Kconfig | 20 + drivers/phy/hisilicon/Makefile | 2 + drivers/phy/hisilicon/phy-hi3660-pcie.c | 325 +++++++ drivers/phy/hisilicon/phy-hi3670-pcie.c | 892 ++++++++++++++++++ 9 files changed, 1500 insertions(+), 270 deletions(-) create mode 100644 Documentation/devicetree/bindings/phy/hisilicon,phy-hi3660-pcie.yaml create mode 100644 Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml create mode 100644 drivers/phy/hisilicon/phy-hi3660-pcie.c create mode 100644 drivers/phy/hisilicon/phy-hi3670-pcie.c
Signature · 1 line
2.31.1
PATCH v5 1/8 dt-bindings: phy: Add bindings for HiKey 960 PCIe PHY
Document the bindings for HiKey 960 (hi3660) PCIe PHY interface, supported via the pcie-kirin driver.
--- .../phy/hisilicon,phy-hi3660-pcie.yaml | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/hisilicon,phy-hi3660-pcie.yaml
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3660-pcie.yaml new file +82 -0
@@ -0,0 +1,82 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/phy/hisilicon,phy-hi3660-pcie.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: HiSilicon Kirin960 PCIe PHY + +maintainers: + - Mauro Carvalho Chehab <[email protected]> + +description: |+ + Bindings for PCIe PHY on HiSilicon Kirin 960. + +properties: + compatible: + const: hisilicon,hi960-pcie-phy + + "#phy-cells": + const: 0 + + reg: + maxItems: 1 + description: PHY Control registers + + reg-names: + const: phy + + clocks: + items: + - description: PCIe PHY clock + - description: PCIe AUX clock + - description: PCIe APB PHY clock + - description: PCIe APB SYS clock + - description: PCIe ACLK clock + + clock-names: + items: + - const: pcie_phy_ref + - const: pcie_aux + - const: pcie_apb_phy + - const: pcie_apb_sys + - const: pcie_aclk + + reset-gpios: + description: PCI PERST reset GPIO + +required: + - "#phy-cells" + - compatible + - reg + - reg-names + - clocks + - clock-names + - reset-gpios + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/hi3660-clock.h> + + bus { + #address-cells = <2>; + #size-cells = <2>; + pcie_phy: pcie-phy@f3f2000 { + compatible = "hisilicon,hi960-pcie-phy"; + reg = <0x0 0xf3f20000 0x0 0x40000>; + reg-names = "phy"; + clocks = <&crg_ctrl HI3660_PCIEPHY_REF>, + <&crg_ctrl HI3660_CLK_GATE_PCIEAUX>, + <&crg_ctrl HI3660_PCLK_GATE_PCIE_PHY>, + <&crg_ctrl HI3660_PCLK_GATE_PCIE_SYS>, + <&crg_ctrl HI3660_ACLK_GATE_PCIE>; + clock-names = "pcie_phy_ref", "pcie_aux", + "pcie_apb_phy", "pcie_apb_sys", + "pcie_aclk"; + reset-gpios = <&gpio11 1 0 >; + #phy-cells = <0>; + }; + }; +...
Signature · 1 line
2.31.1
Rob Herring
maintainerRe: [PATCH v5 1/8] dt-bindings: phy: Add bindings for HiKey 960 PCIe PHY
Quoted 32 lines · Mauro Carvalho Chehab
Document the bindings for HiKey 960 (hi3660) PCIe PHY interface, supported via the pcie-kirin driver.
Signed-off-by Mauro Carvalho Chehab <[email protected]>--- .../phy/hisilicon,phy-hi3660-pcie.yaml | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/hisilicon,phy-hi3660-pcie.yamlDocumentation/devicetree/bindings/phy/hisilicon,phy-hi3660-pcie.yaml new file +17
-0@@ -0,0 +1,82 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/phy/hisilicon,phy-hi3660-pcie.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: HiSilicon Kirin960 PCIe PHY + +maintainers: + - Mauro Carvalho Chehab <[email protected]> + +description: |+ + Bindings for PCIe PHY on HiSilicon Kirin 960. + +properties: + compatible: + const: hisilicon,hi960-pcie-phy
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3660-pcie.yaml line 27
Quoted 10 lines
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3660-pcie.yaml new file +10
-0+ + "#phy-cells": + const: 0 + + reg: + maxItems: 1 + description: PHY Control registers + + reg-names: + const: phy
You don't really need reg-names with only 1.
Quoted 6 lines
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3660-pcie.yaml new file +6
-0+ + clocks: + items: + - description: PCIe PHY clock + - description: PCIe AUX clock + - description: PCIe APB PHY clock
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3660-pcie.yaml line 43
Quoted 10 lines
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3660-pcie.yaml new file +10
-0+ - description: PCIe APB SYS clock + - description: PCIe ACLK clock + + clock-names: + items: + - const: pcie_phy_ref + - const: pcie_aux + - const: pcie_apb_phy + - const: pcie_apb_sys + - const: pcie_aclk
'pcie_' is redundant. Drop.
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3660-pcie.yaml line 46
Quoted 3 lines
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3660-pcie.yaml new file +3
-0+ + reset-gpios: + description: PCI PERST reset GPIO
maxItems: 1
Though this belongs in the PCIE node.
Quoted 40 lines
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3660-pcie.yaml new file +36
-0+ +required: + - "#phy-cells" + - compatible + - reg + - reg-names + - clocks + - clock-names + - reset-gpios + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/hi3660-clock.h> + + bus { + #address-cells = <2>; + #size-cells = <2>; + pcie_phy: pcie-phy@f3f2000 { + compatible = "hisilicon,hi960-pcie-phy"; + reg = <0x0 0xf3f20000 0x0 0x40000>; + reg-names = "phy"; + clocks = <&crg_ctrl HI3660_PCIEPHY_REF>, + <&crg_ctrl HI3660_CLK_GATE_PCIEAUX>, + <&crg_ctrl HI3660_PCLK_GATE_PCIE_PHY>, + <&crg_ctrl HI3660_PCLK_GATE_PCIE_SYS>, + <&crg_ctrl HI3660_ACLK_GATE_PCIE>; + clock-names = "pcie_phy_ref", "pcie_aux", + "pcie_apb_phy", "pcie_apb_sys", + "pcie_aclk"; + reset-gpios = <&gpio11 1 0 >; + #phy-cells = <0>; + }; + }; +...Signature · 1 line
2.31.1
PATCH v5 2/8 dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY
In reply to Mauro Carvalho ChehabDocument the bindings for HiKey 970 (hi3670) PCIe PHY interface, supported via the pcie-kirin driver.
--- .../phy/hisilicon,phy-hi3670-pcie.yaml | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml new file +101 -0
@@ -0,0 +1,101 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/phy/hisilicon,phy-hi3670-pcie.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: HiSilicon Kirin970 PCIe PHY + +maintainers: + - Mauro Carvalho Chehab <[email protected]> + +description: |+ + Bindings for PCIe PHY on HiSilicon Kirin 970. + +properties: + compatible: + const: hisilicon,hi970-pcie-phy + + "#phy-cells": + const: 0 + + reg: + maxItems: 1 + description: PHY Control registers + + reg-names: + const: phy + + phy-supply: + description: The PCIe PHY power supply + + clocks: + items: + - description: PCIe PHY clock + - description: PCIe AUX clock + - description: PCIe APB PHY clock + - description: PCIe APB SYS clock + - description: PCIe ACLK clock + + clock-names: + items: + - const: pcie_phy_ref + - const: pcie_aux + - const: pcie_apb_phy + - const: pcie_apb_sys + - const: pcie_aclk + + reset-gpios: + description: PCI PERST reset GPIOs + maxItems: 4 + + clkreq-gpios: + description: Clock request GPIOs + maxItems: 3 + + hisilicon,eye-diagram-param: + $ref: /schemas/types.yaml#/definitions/uint32-array + description: Eye diagram for phy. + +required: + - "#phy-cells" + - compatible + - reg + - reg-names + - clocks + - clock-names + - reset-gpios + - clkreq-gpios + - hisilicon,eye-diagram-param + - phy-supply + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/hi3670-clock.h> + + bus { + #address-cells = <2>; + #size-cells = <2>; + pcie_phy: pcie-phy@fc000000 { + compatible = "hisilicon,hi970-pcie-phy"; + reg = <0x0 0xfc000000 0x0 0x80000>; + reg-names = "phy"; + #phy-cells = <0>; + 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 = "pcie_phy_ref", "pcie_aux", + "pcie_apb_phy", "pcie_apb_sys", "pcie_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>; + }; + }; +...
Signature · 1 line
2.31.1
Rob Herring
maintainerRe: [PATCH v5 2/8] dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY
Quoted 2 lines · Mauro Carvalho Chehab
Document the bindings for HiKey 970 (hi3670) PCIe PHY interface, supported via the pcie-kirin driver.
Same comments on this one.
Quoted 53 lines
Signed-off-by Mauro Carvalho Chehab <[email protected]>--- .../phy/hisilicon,phy-hi3670-pcie.yaml | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yamlDocumentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml new file +40
-0@@ -0,0 +1,101 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/phy/hisilicon,phy-hi3670-pcie.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: HiSilicon Kirin970 PCIe PHY + +maintainers: + - Mauro Carvalho Chehab <[email protected]> + +description: |+ + Bindings for PCIe PHY on HiSilicon Kirin 970. + +properties: + compatible: + const: hisilicon,hi970-pcie-phy + + "#phy-cells": + const: 0 + + reg: + maxItems: 1 + description: PHY Control registers + + reg-names: + const: phy + + phy-supply: + description: The PCIe PHY power supply + + clocks: + items: + - description: PCIe PHY clock + - description: PCIe AUX clock + - description: PCIe APB PHY clock + - description: PCIe APB SYS clock + - description: PCIe ACLK clock + + clock-names:
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml line 50
Quoted 10 lines
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml new file +10
-0+ items: + - const: pcie_phy_ref + - const: pcie_aux + - const: pcie_apb_phy + - const: pcie_apb_sys + - const: pcie_aclk + + reset-gpios: + description: PCI PERST reset GPIOs + maxItems: 4
Hiding the 4 ports in the phy?
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml line 58
Quoted 8 lines
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml new file +8
-0+ + clkreq-gpios: + description: Clock request GPIOs + maxItems: 3 + + hisilicon,eye-diagram-param: + $ref: /schemas/types.yaml#/definitions/uint32-array + description: Eye diagram for phy.
Is there a size to this array?
Quoted 47 lines
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml new file +43
-0+ +required: + - "#phy-cells" + - compatible + - reg + - reg-names + - clocks + - clock-names + - reset-gpios + - clkreq-gpios + - hisilicon,eye-diagram-param + - phy-supply + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/hi3670-clock.h> + + bus { + #address-cells = <2>; + #size-cells = <2>; + pcie_phy: pcie-phy@fc000000 { + compatible = "hisilicon,hi970-pcie-phy"; + reg = <0x0 0xfc000000 0x0 0x80000>; + reg-names = "phy"; + #phy-cells = <0>; + 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 = "pcie_phy_ref", "pcie_aux", + "pcie_apb_phy", "pcie_apb_sys", "pcie_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>; + }; + }; +...Signature · 1 line
2.31.1
Re: [PATCH v5 2/8] dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY
Em Tue, 13 Jul 2021 20:26:49 -0600
Rob Herring <[email protected]> escreveu:
Quoted 7 lines · Mauro Carvalho Chehab
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml line 50
Quoted 4 lines
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml new file +3
-0+ reset-gpios: + description: PCI PERST reset GPIOs + maxItems: 4Hiding the 4 ports in the phy?
Rob,
I'm not trying to hide anything.
There are several differences with regards to how PERST# is handled between HiKey 960 and HiKey 970.
From hardware perspective, you can see the schematics of both boards:
https://github.com/96boards/documentation/raw/master/consumer/hikey/hikey960/hardware-docs/HiKey960_SoC_Reference_Manual.pdf
https://www.96boards.org/documentation/consumer/hikey/hikey970/hardware-docs/files/hikey970-schematics.pdf
The 960 PHY has the SoC directly connected to a PCIE M.2 slot (model 10130616) without any external bridge chipset. It uses a single GPIO (GPIO 089) for the PERST# signal, connected via a voltage converter (from 1.8V to 3.3V).
$ lspci 00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3660 (rev 01)
The 970 PHY has an external PCI bridge chipset (PLX Technology PEX 8606). Besides the bridge, the hardware comes with an Ethernet PCI adapter, a M.2 slot and a mini-PCIe connector. Each one with its own PERST# signal, mapped to different GPIO pins, and each one using its own voltage converter.
$ 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)
On other words, there are 4 GPIOs mapped to different PERST# pins in the hardware:
- GPIO 56 is connected to the PERST# pin at PEX 8606;
- GPIO 25 is connected to the PERST# pin at the M.2 slot;
- GPIO 220 is connected to the PERST# pin at the PCIe mini slot;
- GPIO 203 is connected to the PERST# pin at the Ethernet chipset.
Maybe due to different electrical requirements, the hardware design use different GPIOs instead of feeding them altogether.
Anyway, the fact is that the PHY on 970 has 4 different GPIOs that are need in order for the hardware to work. and this is specific to this particular PHY.
Now, from software perspective, the power on sequence on Hikey 960 finishes sending PERST# signal to the M.2 slot:
static int hi3660_pcie_phy_power_on(struct phy *generic_phy)
{
...
/* perst assert Endpoint */
if (!gpio_request(phy->gpio_id_reset, "pcie_perst")) {
usleep_range(REF_2_PERST_MIN, REF_2_PERST_MAX);
ret = gpio_direction_output(phy->gpio_id_reset, 1);
if (ret)
goto disable_clks;
usleep_range(PERST_2_ACCESS_MIN, PERST_2_ACCESS_MAX);
return 0;
}
disable_clks:
kirin_pcie_clk_ctrl(phy, false);
return ret;
}The 970 PHY, however, sends PERST# signal in the middle of the power on sequence, as, after sending reset, it needs to wait for the hardware to stabilize, in order to setup an eye diagram at the PHY:
static int hi3670_pcie_phy_power_on(struct phy *generic_phy)
{
...
/* perst assert Endpoints */
usleep_range(21000, 23000);
for (i = 0; i < phy->n_gpio_resets; i++) {
ret = gpio_direction_output(phy->gpio_id_reset[i], 1);
if (ret)
return ret;
}
usleep_range(10000, 11000);
ret = is_pipe_clk_stable(phy);
if (!ret)
goto disable_clks;
hi3670_pcie_set_eyeparam(phy);
ret = hi3670_pcie_noc_power(phy, false);
if (ret)
goto disable_clks;
return 0;
disable_clks:
kirin_pcie_clk_ctrl(phy, false);
return ret;
}IMO, it makes a lot more sense to map this on DT as part of the PHY and not as part of the PCIe, but no matter how it is mapped, this PHY still requires 4 GPIOs for PERST#.
Thanks,
Mauro
Rob Herring
maintainerRe: [PATCH v5 2/8] dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY
Quoted 56 lines · Mauro Carvalho Chehab
Em Tue, 13 Jul 2021 20:26:49 -0600
Rob Herring <[email protected]> escreveu:Quoted 7 lines · Mauro Carvalho Chehab
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml line 50
Quoted 4 lines
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml new file +3
-0+ reset-gpios: + description: PCI PERST reset GPIOs + maxItems: 4Hiding the 4 ports in the phy?
Rob,
I'm not trying to hide anything.
There are several differences with regards to how PERST# is handled between HiKey 960 and HiKey 970.
From hardware perspective, you can see the schematics of both boards:
https://github.com/96boards/documentation/raw/master/consumer/hikey/hikey960/hardware-docs/HiKey960_SoC_Reference_Manual.pdf
https://www.96boards.org/documentation/consumer/hikey/hikey970/hardware-docs/files/hikey970-schematics.pdfThe 960 PHY has the SoC directly connected to a PCIE M.2 slot (model 10130616) without any external bridge chipset. It uses a single GPIO (GPIO 089) for the PERST# signal, connected via a voltage converter (from 1.8V to 3.3V).
$ lspci 00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3660 (rev 01)The 970 PHY has an external PCI bridge chipset (PLX Technology PEX 8606). Besides the bridge, the hardware comes with an Ethernet PCI adapter, a M.2 slot and a mini-PCIe connector. Each one with its own PERST# signal, mapped to different GPIO pins, and each one using its own voltage converter.
$ 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)On other words, there are 4 GPIOs mapped to different PERST# pins in the hardware:
- GPIO 56 is connected to the PERST# pin at PEX 8606;
- GPIO 25 is connected to the PERST# pin at the M.2 slot;
- GPIO 220 is connected to the PERST# pin at the PCIe mini slot;
- GPIO 203 is connected to the PERST# pin at the Ethernet chipset.
Quoted 6 lines
Maybe due to different electrical requirements, the hardware design use different GPIOs instead of feeding them altogether.
Anyway, the fact is that the PHY on 970 has 4 different GPIOs that are need in order for the hardware to work. and this is specific to this particular PHY.
This hierarchy could be done on any board. It has nothing to do with the PHY.
Quoted 54 lines
Now, from software perspective, the power on sequence on Hikey 960 finishes sending PERST# signal to the M.2 slot:
static int
hi3660_pcie_phy_power_on(struct phy *generic_phy){
.../* perst assert Endpoint */ if (!gpio_request(phy->gpio_id_reset, "pcie_perst")) { usleep_range(REF_2_PERST_MIN, REF_2_PERST_MAX); ret = gpio_direction_output(phy->gpio_id_reset, 1); if (ret) goto disable_clks; usleep_range(PERST_2_ACCESS_MIN, PERST_2_ACCESS_MAX); return 0; } disable_clks: kirin_pcie_clk_ctrl(phy, false); return ret; }The 970 PHY, however, sends PERST# signal in the middle of the power on sequence, as, after sending reset, it needs to wait for the hardware to stabilize, in order to setup an eye diagram at the PHY:
static int
hi3670_pcie_phy_power_on(struct phy *generic_phy){
.../* perst assert Endpoints */ usleep_range(21000, 23000); for (i = 0; i < phy->n_gpio_resets; i++) { ret = gpio_direction_output(phy->gpio_id_reset[i], 1); if (ret) return ret; } usleep_range(10000, 11000); ret = is_pipe_clk_stable(phy); if (!ret) goto disable_clks; hi3670_pcie_set_eyeparam(phy); ret = hi3670_pcie_noc_power(phy, false); if (ret) goto disable_clks; return 0; disable_clks: kirin_pcie_clk_ctrl(phy, false); return ret; }
Quoted 3 lines
IMO, it makes a lot more sense to map this on DT as part of the PHY and not as part of the PCIe, but no matter how it is mapped, this PHY still requires 4 GPIOs for PERST#.
It does not because PERST# control is part of PCIe for every other driver.
Rob
Re: [PATCH v5 2/8] dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY
Em Wed, 14 Jul 2021 08:17:05 -0600
Rob Herring <[email protected]> escreveu:
Quoted 58 lines
Quoted 56 lines · Mauro Carvalho Chehab
Em Tue, 13 Jul 2021 20:26:49 -0600
Rob Herring <[email protected]> escreveu:Quoted 7 lines · Mauro Carvalho Chehab
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml line 50
Quoted 4 lines
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml new file +3
-0+ reset-gpios: + description: PCI PERST reset GPIOs + maxItems: 4Hiding the 4 ports in the phy?
Rob,
I'm not trying to hide anything.
There are several differences with regards to how PERST# is handled between HiKey 960 and HiKey 970.
From hardware perspective, you can see the schematics of both boards:
https://github.com/96boards/documentation/raw/master/consumer/hikey/hikey960/hardware-docs/HiKey960_SoC_Reference_Manual.pdf
https://www.96boards.org/documentation/consumer/hikey/hikey970/hardware-docs/files/hikey970-schematics.pdfThe 960 PHY has the SoC directly connected to a PCIE M.2 slot (model 10130616) without any external bridge chipset. It uses a single GPIO (GPIO 089) for the PERST# signal, connected via a voltage converter (from 1.8V to 3.3V).
$ lspci 00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3660 (rev 01)The 970 PHY has an external PCI bridge chipset (PLX Technology PEX 8606). Besides the bridge, the hardware comes with an Ethernet PCI adapter, a M.2 slot and a mini-PCIe connector. Each one with its own PERST# signal, mapped to different GPIO pins, and each one using its own voltage converter.
$ 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)On other words, there are 4 GPIOs mapped to different PERST# pins in the hardware:
- GPIO 56 is connected to the PERST# pin at PEX 8606;
- GPIO 25 is connected to the PERST# pin at the M.2 slot;
- GPIO 220 is connected to the PERST# pin at the PCIe mini slot;
- GPIO 203 is connected to the PERST# pin at the Ethernet chipset.
Quoted 8 lines
Quoted 6 lines
Maybe due to different electrical requirements, the hardware design use different GPIOs instead of feeding them altogether.
Anyway, the fact is that the PHY on 970 has 4 different GPIOs that are need in order for the hardware to work. and this is specific to this particular PHY.
This hierarchy could be done on any board. It has nothing to do with the PHY.
True, but right now, the pci-bus.yaml prevents it, as it allows just one reset GPIO[1]:
reset-gpios:
description: GPIO controlled connection to PERST# signal
maxItems: 1
[1] https://github.com/robherring/dt-schema/blob/master/schemas/pci/pci-bus.yaml
If the schema will be changed to allow multiple reset-gpios, It should be possible to keep this at the pcie-kirin.c driver with something like:
static int kirin_pcie_power_on(struct kirin_pcie *kirin_pcie)
{
int ret;
ret = phy_init(kirin_pcie->phy);
if (ret)
return ret;
ret = phy_power_on(kirin_pcie->phy);
/* perst assert Endpoints */
usleep_range(21000, 23000);
for (i = 0; i < phy->n_gpio_resets; i++) {
ret = gpio_direction_output(phy->gpio_id_reset[i], 1);
if (ret) {
phy_power_off(kirin_pcie->phy);
return ret;
}
}
usleep_range(10000, 11000);
return phy_reset(kirin_pcie->phy);
}This would work for both 960 - where phy_reset() is not needed, and for 970, where it would set the eye diagram for the PHY.
Should I send a patch for pci-bus.yaml via github?
Thanks,
Mauro
Manivannan Sadhasivam
maintainerRe: [PATCH v5 2/8] dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY
In reply to Mauro Carvalho ChehabHi Mauro,
Quoted 55 lines · Mauro Carvalho Chehab
Em Tue, 13 Jul 2021 20:26:49 -0600
Rob Herring <[email protected]> escreveu:Quoted 7 lines · Mauro Carvalho Chehab
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml line 50
Quoted 4 lines
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml new file +3
-0+ reset-gpios: + description: PCI PERST reset GPIOs + maxItems: 4Hiding the 4 ports in the phy?
Rob,
I'm not trying to hide anything.
There are several differences with regards to how PERST# is handled between HiKey 960 and HiKey 970.
From hardware perspective, you can see the schematics of both boards:
https://github.com/96boards/documentation/raw/master/consumer/hikey/hikey960/hardware-docs/HiKey960_SoC_Reference_Manual.pdf
https://www.96boards.org/documentation/consumer/hikey/hikey970/hardware-docs/files/hikey970-schematics.pdfThe 960 PHY has the SoC directly connected to a PCIE M.2 slot (model 10130616) without any external bridge chipset. It uses a single GPIO (GPIO 089) for the PERST# signal, connected via a voltage converter (from 1.8V to 3.3V).
$ lspci 00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3660 (rev 01)The 970 PHY has an external PCI bridge chipset (PLX Technology PEX 8606). Besides the bridge, the hardware comes with an Ethernet PCI adapter, a M.2 slot and a mini-PCIe connector. Each one with its own PERST# signal, mapped to different GPIO pins, and each one using its own voltage converter.
$ 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)On other words, there are 4 GPIOs mapped to different PERST# pins in the hardware:
- GPIO 56 is connected to the PERST# pin at PEX 8606;
- GPIO 25 is connected to the PERST# pin at the M.2 slot;
- GPIO 220 is connected to the PERST# pin at the PCIe mini slot;
- GPIO 203 is connected to the PERST# pin at the Ethernet chipset.
Quoted 7 lines
Maybe due to different electrical requirements, the hardware design use different GPIOs instead of feeding them altogether.
Anyway, the fact is that the PHY on 970 has 4 different GPIOs that are need in order for the hardware to work. and this is specific to this particular PHY.
I'm not sure about this. That fact that the PCIe device's PERST# signal wired to different GPIOs doesn't mean that those GPIOs belong to the PHY. Those GPIOs should be independent of the PCIe core controlled manually by the driver.
I think this issue is somewhat similar to the one we are dealing on the Qcom platforms [1] where each PCIe device uses a different GPIO and voltage config to operate. And those need to be active for the link training to succeed.
So perhaps we should aim for a common solution? The GPIO and voltage layout should be described in DT for each port exposed by the SoC/board.
Thanks,
Mani
Re: [PATCH v5 2/8] dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY
Hi Mani,
Em Wed, 14 Jul 2021 23:12:25 +0530
Manivannan Sadhasivam <[email protected]> escreveu:
Quoted 71 lines
Hi Mauro,
Quoted 55 lines · Mauro Carvalho Chehab
Em Tue, 13 Jul 2021 20:26:49 -0600
Rob Herring <[email protected]> escreveu:Quoted 7 lines · Mauro Carvalho Chehab
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml line 50
Quoted 4 lines
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml new file +3
-0+ reset-gpios: + description: PCI PERST reset GPIOs + maxItems: 4Hiding the 4 ports in the phy?
Rob,
I'm not trying to hide anything.
There are several differences with regards to how PERST# is handled between HiKey 960 and HiKey 970.
From hardware perspective, you can see the schematics of both boards:
https://github.com/96boards/documentation/raw/master/consumer/hikey/hikey960/hardware-docs/HiKey960_SoC_Reference_Manual.pdf
https://www.96boards.org/documentation/consumer/hikey/hikey970/hardware-docs/files/hikey970-schematics.pdfThe 960 PHY has the SoC directly connected to a PCIE M.2 slot (model 10130616) without any external bridge chipset. It uses a single GPIO (GPIO 089) for the PERST# signal, connected via a voltage converter (from 1.8V to 3.3V).
$ lspci 00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3660 (rev 01)The 970 PHY has an external PCI bridge chipset (PLX Technology PEX 8606). Besides the bridge, the hardware comes with an Ethernet PCI adapter, a M.2 slot and a mini-PCIe connector. Each one with its own PERST# signal, mapped to different GPIO pins, and each one using its own voltage converter.
$ 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)On other words, there are 4 GPIOs mapped to different PERST# pins in the hardware:
- GPIO 56 is connected to the PERST# pin at PEX 8606;
- GPIO 25 is connected to the PERST# pin at the M.2 slot;
- GPIO 220 is connected to the PERST# pin at the PCIe mini slot;
- GPIO 203 is connected to the PERST# pin at the Ethernet chipset.Quoted 7 lines
Maybe due to different electrical requirements, the hardware design use different GPIOs instead of feeding them altogether.
Anyway, the fact is that the PHY on 970 has 4 different GPIOs that are need in order for the hardware to work. and this is specific to this particular PHY.
I'm not sure about this. That fact that the PCIe device's PERST# signal wired to different GPIOs doesn't mean that those GPIOs belong to the PHY. Those GPIOs should be independent of the PCIe core controlled manually by the driver.
Quoted 7 lines
I think this issue is somewhat similar to the one we are dealing on the Qcom platforms [1] where each PCIe device uses a different GPIO and voltage config to operate. And those need to be active for the link training to succeed.
So perhaps we should aim for a common solution? The GPIO and voltage layout should be described in DT for each port exposed by the SoC/board.
It doesn't seem to be the same case. In the case of Hikey 970[1], it uses one pullup for each GPIO to convert from 1V8 to the voltage needed by each chipset. There's no regulators envolved: the output voltage is fixed. From the diagram, it seems that PEX8606 uses 2V5 (via U2602), while the other 3 GPIOs (M.2, PCIe mini and Ethernet) use 3V3. Interesting enough, the layout uses a separate gate chip for Ethernet (U3107), while both M.2 and PCIe mini use two ports at the same gate chip (U2801).
I can think on a couple of reasons why the hardware designers opted to use 4 different GPIOs:
- the PCIe external bridge required a lower voltage;
- the current drained by each output port could be different;
- the chips used by the pullup could have some sort of protection to minimize the risk of damaging the hardware due to a bad contact and/or short circuit (for M.2 and PCIe mini connectors).
In any case, from DT perspective, the only thing that makes sense to expose are the 4 GPIOs used by the PERST# signal, as the output voltages and max currents are fixed.
Conceptually, IMO, such pullup logic can be considered as part of the physical layer, but, as Rob pointed, all PCIe devices need to have PERST#. So, they could be just mapped as gpio-resets at the PCIe OF node.
In summary, IMO, mapping those 4 GPIOs via DT can be done by either:
1. Place all of them at the PCIe OF node;
2. Place all of them at the PHY OF node;
3. Place one of them at the PCIe, and the other three at PHY.
IMO, (3) is messier. So, I would go either for (1) or (2). Between them, I don't have any preferences, as both would work.
Right now, pci-bus.yaml doesn't allow (1), as it limits reset-gpios to a maximum of 1 item, but this is easily fixable, but it requires a patch against https://github.com/devicetree-org/dt-schema to raise MaxItems to at least 4.
Rob,
Among the above alternatives, what do you prefer?
Thanks,
Mauro
Re: [PATCH v5 2/8] dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY
Hi Rob/Bjorn,
I rebased my repository in a way that the PHY for Kirin 960 won't be touched. As I explained before:
https://lore.kernel.org/lkml/[email protected]/
the only way of doing that without modifying the PHY core and doing ugly hacks is to not split Kirin 960 PHY from the pcie-kirin driver.
So, the approach I took was to make the core code at the pcie-kirin driver generic, ensuring that the internal PHY code is self-contained.
Then, I added support for using a PHY from drivers/phy. This way, the DT schema for Kirin 960 won't be touched. The patch series is at:
https://github.com/mchehab/linux/commits/pcie-alternate
Now, this is the only pending issue on my TODO list:
Em Thu, 15 Jul 2021 08:37:09 +0200
Mauro Carvalho Chehab <[email protected]> escreveu:
Quoted 109 lines
Quoted 63 lines
Quoted 50 lines
Quoted 5 lines
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml line 50
Quoted 3 lines
Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml new file +3
-0+ reset-gpios: + description: PCI PERST reset GPIOs + maxItems: 4Hiding the 4 ports in the phy?
Rob,
I'm not trying to hide anything.
There are several differences with regards to how PERST# is handled between HiKey 960 and HiKey 970.
From hardware perspective, you can see the schematics of both boards:
https://github.com/96boards/documentation/raw/master/consumer/hikey/hikey960/hardware-docs/HiKey960_SoC_Reference_Manual.pdf
https://www.96boards.org/documentation/consumer/hikey/hikey970/hardware-docs/files/hikey970-schematics.pdfThe 960 PHY has the SoC directly connected to a PCIE M.2 slot (model 10130616) without any external bridge chipset. It uses a single GPIO (GPIO 089) for the PERST# signal, connected via a voltage converter (from 1.8V to 3.3V).
$ lspci 00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3660 (rev 01)The 970 PHY has an external PCI bridge chipset (PLX Technology PEX 8606). Besides the bridge, the hardware comes with an Ethernet PCI adapter, a M.2 slot and a mini-PCIe connector. Each one with its own PERST# signal, mapped to different GPIO pins, and each one using its own voltage converter.
$ 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)On other words, there are 4 GPIOs mapped to different PERST# pins in the hardware:
- GPIO 56 is connected to the PERST# pin at PEX 8606;
- GPIO 25 is connected to the PERST# pin at the M.2 slot;
- GPIO 220 is connected to the PERST# pin at the PCIe mini slot;
- GPIO 203 is connected to the PERST# pin at the Ethernet chipset.Quoted 7 lines
Maybe due to different electrical requirements, the hardware design use different GPIOs instead of feeding them altogether.
Anyway, the fact is that the PHY on 970 has 4 different GPIOs that are need in order for the hardware to work. and this is specific to this particular PHY.
I'm not sure about this. That fact that the PCIe device's PERST# signal wired to different GPIOs doesn't mean that those GPIOs belong to the PHY. Those GPIOs should be independent of the PCIe core controlled manually by the driver.
Quoted 7 lines
I think this issue is somewhat similar to the one we are dealing on the Qcom platforms [1] where each PCIe device uses a different GPIO and voltage config to operate. And those need to be active for the link training to succeed.
So perhaps we should aim for a common solution? The GPIO and voltage layout should be described in DT for each port exposed by the SoC/board.
It doesn't seem to be the same case. In the case of Hikey 970[1], it uses one pullup for each GPIO to convert from 1V8 to the voltage needed by each chipset. There's no regulators envolved: the output voltage is fixed. From the diagram, it seems that PEX8606 uses 2V5 (via U2602), while the other 3 GPIOs (M.2, PCIe mini and Ethernet) use 3V3. Interesting enough, the layout uses a separate gate chip for Ethernet (U3107), while both M.2 and PCIe mini use two ports at the same gate chip (U2801).
I can think on a couple of reasons why the hardware designers opted to use 4 different GPIOs:
- the PCIe external bridge required a lower voltage;
- the current drained by each output port could be different;
- the chips used by the pullup could have some sort of protection to minimize the risk of damaging the hardware due to a bad contact and/or short circuit (for M.2 and PCIe mini connectors).In any case, from DT perspective, the only thing that makes sense to expose are the 4 GPIOs used by the PERST# signal, as the output voltages and max currents are fixed.
Conceptually, IMO, such pullup logic can be considered as part of the physical layer, but, as Rob pointed, all PCIe devices need to have PERST#. So, they could be just mapped as gpio-resets at the PCIe OF node.
In summary, IMO, mapping those 4 GPIOs via DT can be done by either:
1. Place all of them at the PCIe OF node;
2. Place all of them at the PHY OF node;
3. Place one of them at the PCIe, and the other three at PHY.IMO, (3) is messier. So, I would go either for (1) or (2). Between them, I don't have any preferences, as both would work.
Quoted 8 lines
Right now,
pci-bus.yamldoesn't allow (1), as it limits reset-gpios to a maximum of 1 item, but this is easily fixable, but it requires a patch against https://github.com/devicetree-org/dt-schema to raise MaxItems to at least 4.Rob,
Among the above alternatives, what do you prefer?
At the patch series I mentioned, I'm keeping the PERST# signals as part of the PHY layer, just because DT currently produces a warning if maxItems > 1 for gpio-resets for PCI bus.
In the case it was decided to allow the PCI bus to have more than one PERST# pin, I already sent a patch to DT-schema:
https://github.com/devicetree-org/dt-schema/pull/56
But I'll need to change the patch series to consider such approach, if this will be the case.
So, please let me know how do you prefer to proceed with that.
Thanks!
Mauro
Signature · 3 lines
linux-phy mailing list [email protected] https://lists.infradead.org/mailman/listinfo/linux-phy
Re: [PATCH v5 2/8] dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY
In reply to Manivannan SadhasivamHi Mani,
Em Wed, 14 Jul 2021 23:12:25 +0530
Manivannan Sadhasivam <[email protected]> escreveu:
Quoted 10 lines
I'm not sure about this. That fact that the PCIe device's PERST# signal wired to different GPIOs doesn't mean that those GPIOs belong to the PHY. Those GPIOs should be independent of the PCIe core controlled manually by the driver.
I think this issue is somewhat similar to the one we are dealing on the Qcom platforms [1] where each PCIe device uses a different GPIO and voltage config to operate. And those need to be active for the link training to succeed.
Quoted 7 lines
So perhaps we should aim for a common solution? The GPIO and voltage layout should be described in DT for each port exposed by the SoC/board.
Thanks,
Mani
After re-visiting this issue, I'm starting to think that this should be mapped as something similar to:
pcie@xxxx {
...
slot {
slot#1 {
// clock, power supply, reset pins, etc
}
slot#2 {
// clock, power supply, reset pins, etc
}
...
}
};E. g. placing each specific PCIe device requirement inside the pcie or phy, as it should be up to the driver to initialize each PCIe child-specific requirements when the hardware is ready for that.
---
A longer explanation why this should be initialized during PHY power on sequence:
On my tests with Kirin 970, there are some steps to be done before enabling the clocks and sending PERST# signals, plus some extra steps to run after PERST# is sent to all devices.
While playing with PHY split, I noticed that Linux and/or the SoC is very sensitive to an specific probing order. If such order is not followed, an ARM SError happens and the Kernel panics with something similar to:
[ 1.837458] SError Interrupt on CPU0, code 0xbf000002 -- SError [ 1.837462] CPU: 0 PID: 74 Comm: kworker/0:1 Not tainted 5.8.0+ #205 [ 1.837463] Hardware name: HiKey970 (DT) [ 1.837465] Workqueue: events deferred_probe_work_func [ 1.837467] pstate: 20000005 (nzCv daif -PAN -UAO BTYPE=--) [ 1.837468] pc : _raw_spin_unlock_irqrestore+0x18/0x50 [ 1.837469] lr : regmap_unlock_spinlock+0x14/0x20 ... [ 1.837507] Kernel panic - not syncing: Asynchronous SError Interrupt
One example is with regards to the clocks required for the PCIe to work:
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>;
If them aren't initialized at the expected order, the Kernel hangs. The same applies to the slot-specific clocks.
So, basically, the driver needs to initialize them on this sequence:
1. PHY ref clock;
2. APB sys and phy clock;
3. aclk and aux_clk;
<some settings at the PHY hardware>
4. slot-specific clocks.
failing to follow a valid power-on sequence crashes the Kernel.
Thanks,
Mauro
PATCH v5 3/8 dt-bindings: PCI: kirin: Fix compatible string
In reply to Mauro Carvalho ChehabThe pcie-kirin driver doesn't declare a hisilicon,kirin-pcie. Also, remove the useless comment after the description, as other compat will be supported by the same driver in the future.
--- Documentation/devicetree/bindings/pci/kirin-pcie.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/pci/kirin-pcie.txt +2 -2
@@ -9,7 +9,7 @@ Additional properties are described here: Required properties - compatible: - "hisilicon,kirin960-pcie" for PCIe of Kirin960 SoC + "hisilicon,kirin960-pcie" - reg: Should contain rc_dbi, apb, phy, config registers location and length. - reg-names: Must include the following entries: "dbi": controller configuration registers;
@@ -23,7 +23,7 @@ Optional properties: Example based on kirin960: pcie@f4000000 { - compatible = "hisilicon,kirin-pcie"; + compatible = "hisilicon,kirin960-pcie"; reg = <0x0 0xf4000000 0x0 0x1000>, <0x0 0xff3fe000 0x0 0x1000>, <0x0 0xf3f20000 0x0 0x40000>, <0x0 0xF4000000 0 0x2000>; reg-names = "dbi","apb","phy", "config";
Signature · 1 line
2.31.1
Re: [PATCH v5 3/8] dt-bindings: PCI: kirin: Fix compatible string
Quoted 9 lines · Mauro Carvalho Chehab
The pcie-kirin driver doesn't declare a hisilicon,kirin-pcie. Also, remove the useless comment after the description, as other compat will be supported by the same driver in the future.
Signed-off-by Mauro Carvalho Chehab <[email protected]>--- Documentation/devicetree/bindings/pci/kirin-pcie.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
PATCH v5 4/8 dt-bindings: PCI: kirin: Drop PHY properties
In reply to Mauro Carvalho ChehabThere are several properties there that belong to the PHY interface. Drop them, as a new binding file will describe the PHY properties for Kirin 960.
--- .../devicetree/bindings/pci/kirin-pcie.txt | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-)
Documentation/devicetree/bindings/pci/kirin-pcie.txt +3 -13
@@ -10,13 +10,11 @@ Additional properties are described here: Required properties - compatible: "hisilicon,kirin960-pcie" -- reg: Should contain rc_dbi, apb, phy, config registers location and length. +- reg: Should contain rc_dbi, apb, config registers location and length. - reg-names: Must include the following entries: "dbi": controller configuration registers; "apb": apb Ctrl register defined by Kirin; - "phy": apb PHY register defined by Kirin; "config": PCIe configuration space registers. -- reset-gpios: The GPIO to generate PCIe PERST# assert and deassert signal. Optional properties:
@@ -25,8 +23,8 @@ Example based on kirin960: pcie@f4000000 { compatible = "hisilicon,kirin960-pcie"; reg = <0x0 0xf4000000 0x0 0x1000>, <0x0 0xff3fe000 0x0 0x1000>, - <0x0 0xf3f20000 0x0 0x40000>, <0x0 0xF4000000 0 0x2000>; - reg-names = "dbi","apb","phy", "config"; + <0x0 0xF4000000 0 0x2000>; + reg-names = "dbi","apb", "config"; bus-range = <0x0 0x1>; #address-cells = <3>; #size-cells = <2>;
@@ -39,12 +37,4 @@ Example based on kirin960: <0x0 0 0 2 &gic 0 0 0 283 4>, <0x0 0 0 3 &gic 0 0 0 284 4>, <0x0 0 0 4 &gic 0 0 0 285 4>; - clocks = <&crg_ctrl HI3660_PCIEPHY_REF>, - <&crg_ctrl HI3660_CLK_GATE_PCIEAUX>, - <&crg_ctrl HI3660_PCLK_GATE_PCIE_PHY>, - <&crg_ctrl HI3660_PCLK_GATE_PCIE_SYS>, - <&crg_ctrl HI3660_ACLK_GATE_PCIE>; - clock-names = "pcie_phy_ref", "pcie_aux", - "pcie_apb_phy", "pcie_apb_sys", "pcie_aclk"; - reset-gpios = <&gpio11 1 0 >; };
Signature · 1 line
2.31.1
Rob Herring
maintainerRe: [PATCH v5 4/8] dt-bindings: PCI: kirin: Drop PHY properties
Quoted 3 lines · Mauro Carvalho Chehab
There are several properties there that belong to the PHY interface. Drop them, as a new binding file will describe the PHY properties for Kirin 960.
Folks are okay with an incompatible change on hikey960?
Quoted 53 lines
Signed-off-by Mauro Carvalho Chehab <[email protected]>--- .../devicetree/bindings/pci/kirin-pcie.txt | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-)Documentation/devicetree/bindings/pci/kirin-pcie.txt +3
-13@@ -10,13 +10,11 @@ Additional properties are described here: Required properties - compatible: "hisilicon,kirin960-pcie" -- reg: Should contain rc_dbi, apb, phy, config registers location and length. +- reg: Should contain rc_dbi, apb, config registers location and length. - reg-names: Must include the following entries: "dbi": controller configuration registers; "apb": apb Ctrl register defined by Kirin; - "phy": apb PHY register defined by Kirin; "config": PCIe configuration space registers. -- reset-gpios: The GPIO to generate PCIe PERST# assert and deassert signal. Optional properties:@@ -25,8 +23,8 @@ Example based on kirin960: pcie@f4000000 { compatible = "hisilicon,kirin960-pcie"; reg = <0x0 0xf4000000 0x0 0x1000>, <0x0 0xff3fe000 0x0 0x1000>, - <0x0 0xf3f20000 0x0 0x40000>, <0x0 0xF4000000 0 0x2000>; - reg-names = "dbi","apb","phy", "config"; + <0x0 0xF4000000 0 0x2000>; + reg-names = "dbi","apb", "config"; bus-range = <0x0 0x1>; #address-cells = <3>; #size-cells = <2>;@@ -39,12 +37,4 @@ Example based on kirin960: <0x0 0 0 2 &gic 0 0 0 283 4>, <0x0 0 0 3 &gic 0 0 0 284 4>, <0x0 0 0 4 &gic 0 0 0 285 4>; - clocks = <&crg_ctrl HI3660_PCIEPHY_REF>, - <&crg_ctrl HI3660_CLK_GATE_PCIEAUX>, - <&crg_ctrl HI3660_PCLK_GATE_PCIE_PHY>, - <&crg_ctrl HI3660_PCLK_GATE_PCIE_SYS>, - <&crg_ctrl HI3660_ACLK_GATE_PCIE>; - clock-names = "pcie_phy_ref", "pcie_aux", - "pcie_apb_phy", "pcie_apb_sys", "pcie_aclk"; - reset-gpios = <&gpio11 1 0 >; };Signature · 1 line
2.31.1
Re: [PATCH v5 4/8] dt-bindings: PCI: kirin: Drop PHY properties
Em Tue, 13 Jul 2021 20:28:49 -0600
Rob Herring <[email protected]> escreveu:
Quoted 6 lines · Mauro Carvalho Chehab
Quoted 3 lines
There are several properties there that belong to the PHY interface. Drop them, as a new binding file will describe the PHY properties for Kirin 960.
Folks are okay with an incompatible change on hikey960?
I hope so ;-)
I mean, it should be easy to add a backward-compatible code that would make the PHY driver to use the pci-bus old schema if there's no PHY entry at DT.
However, this is not enough, as the PHY driver won't be loaded/probed without at least this at hi3660.dtsi:
pcie_phy: pcie-phy@f3f2000 {
compatible = "hisilicon,hi960-pcie-phy";
};So, some (probably ugly) hack would be needed at pcie-kirin, in order to make it to manually load and probe the PHY driver, if it founds (for instance) "phy" reg-name as a pcie-kirin property.
Thanks,
Mauro
Re: [PATCH v5 4/8] dt-bindings: PCI: kirin: Drop PHY properties
In reply to Rob HerringEm Tue, 13 Jul 2021 20:28:49 -0600
Rob Herring <[email protected]> escreveu:
Quoted 6 lines · Mauro Carvalho Chehab
Quoted 3 lines
There are several properties there that belong to the PHY interface. Drop them, as a new binding file will describe the PHY properties for Kirin 960.
Folks are okay with an incompatible change on hikey960?
Accepting an incompatible change here seems the right thing to do.
Another possibility would be to create a "pcie-kirin-with-phy" driver that would be identical to the existing one, except for the absence of a PHY and using a different compatible string.
-
Long answer:
There aren't many alternatives here, if we want to split the PHY out of the driver, as you requested.
I've been scratching my head in order to find a way that would keep the Hikey960 a separate PHY driver, with a proper DT schema, but capable of also parse the original DT schema.
See, making the phy driver parse the PCIE-based OF-node data is trivial (I have already a patch doing that), but it will require at least some DT schema additions, in order to add a pcie_phy node[1]:
<snip>
arch/arm64/boot/dts/hisilicon/hi3660.dtsi +6 -0
@@ -1001,6 +1001,11 @@ spi3: spi@ff3b3000 { status = "disabled"; }; + pcie_phy: pcie-phy@f3f2000 { + compatible = "hisilicon,hi960-pcie-phy"; + #phy-cells = <0>; + }; + pcie@f4000000 { compatible = "hisilicon,kirin960-pcie"; reg = <0x0 0xf4000000 0x0 0x1000>,
@@ -1012,6 +1017,7 @@ pcie@f4000000 { #address-cells = <3>; #size-cells = <2>; device_type = "pci"; + phys = <&pcie_phy>; ranges = <0x02000000 0x0 0x00000000 0x0 0xf6000000 0x0 0x02000000>;
</snip>
[1] or, alternatively, the pcie-kirin driver would need to dynamically populate DT with the above, as some ACPI drivers do when the firmware is broken.
Without a PHY representation at the DT schema, the PHY driver won't be recognized by pcie-kirin.
See, even if the pcie-kirin driver would be changed to register the PHY without DT, with:
phy = devm_of_phy_get(dev, NULL, "hi3660_pcie_phy");
The phy_get() implementation will internally ignore a non-DT PHY, as internally, it uses of_property_match_string() if the caller driver has of_node:
struct phy *phy_get(struct device *dev, const char *string)
{
int index = 0;
struct phy *phy;
struct device_link *link;
if (dev->of_node) {
if (string)
index = of_property_match_string(dev->of_node, "phy-names",
string);
else
index = 0;
phy = _of_phy_get(dev->of_node, index);
} else {
if (string == NULL) {
dev_WARN(dev, "missing string\n");
return ERR_PTR(-EINVAL);
}
phy = phy_find(dev, string);
}Thanks,
Mauro
PATCH v5 5/8 phy: HiSilicon: Add driver for Kirin 960 PCIe PHY
In reply to Mauro Carvalho ChehabThe pcie-kirin PCIe driver contains internally a PHY interface for a Kirin 960. As we'll be adding support also for Kirin 970 PHY, we need to first split the PHY into a separate driver.
So, add a new driver with just the PHY part ofdrivers/pci/controller/dwc/pcie-kirin.c.
--- drivers/phy/hisilicon/Kconfig | 10 + drivers/phy/hisilicon/Makefile | 1 + drivers/phy/hisilicon/phy-hi3660-pcie.c | 325 ++++++++++++++++++++++++ 3 files changed, 336 insertions(+) create mode 100644 drivers/phy/hisilicon/phy-hi3660-pcie.c
drivers/phy/hisilicon/Kconfig +10 -0
@@ -23,6 +23,16 @@ config PHY_HI3660_USB To compile this driver as a module, choose M here. +config PHY_HI3660_PCIE + tristate "hi3660 PCIe PHY support" + depends on (ARCH_HISI && ARM64) || COMPILE_TEST + select GENERIC_PHY + select MFD_SYSCON + help + Enable this to support the HiSilicon hi3660 PCIe PHY. + + To compile this driver as a module, choose M here. + config PHY_HI3670_USB tristate "hi3670 USB PHY support" depends on (ARCH_HISI && ARM64) || COMPILE_TEST
drivers/phy/hisilicon/Makefile +1 -0
@@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0-only obj-$(CONFIG_PHY_HI6220_USB) += phy-hi6220-usb.o obj-$(CONFIG_PHY_HI3660_USB) += phy-hi3660-usb3.o +obj-$(CONFIG_PHY_HI3660_PCIE) += phy-hi3660-pcie.o obj-$(CONFIG_PHY_HI3670_USB) += phy-hi3670-usb3.o obj-$(CONFIG_PHY_HISTB_COMBPHY) += phy-histb-combphy.o obj-$(CONFIG_PHY_HISI_INNO_USB2) += phy-hisi-inno-usb2.o
drivers/phy/hisilicon/phy-hi3660-pcie.c new file +325 -0
325 lines not on this page · raw message
Signature · 1 line
2.31.1
PATCH v5 6/8 phy: HiSilicon: add driver for Kirin 970 PCIe PHY
In reply to Mauro Carvalho ChehabThe Kirin 970 PHY is somewhat similar to the Kirin 960, but it does a lot more. Add the needed bits for PCIe to start working on HiKey 970 boards.
--- .../devicetree/bindings/pci/kirin-pcie.txt | 1 + drivers/pci/controller/dwc/pcie-kirin.c | 1 + drivers/phy/hisilicon/Kconfig | 10 + drivers/phy/hisilicon/Makefile | 1 + drivers/phy/hisilicon/phy-hi3670-pcie.c | 892 ++++++++++++++++++ 5 files changed, 905 insertions(+) create mode 100644 drivers/phy/hisilicon/phy-hi3670-pcie.c
Documentation/devicetree/bindings/pci/kirin-pcie.txt +1 -0
@@ -10,6 +10,7 @@ Additional properties are described here: Required properties - compatible: "hisilicon,kirin960-pcie" + "hisilicon,kirin970-pcie" - reg: Should contain rc_dbi, apb, config registers location and length. - reg-names: Must include the following entries: "dbi": controller configuration registers;
drivers/pci/controller/dwc/pcie-kirin.c +1 -0
@@ -472,6 +472,7 @@ static int kirin_pcie_probe(struct platform_device *pdev)static const struct of_device_id kirin_pcie_match[] = { { .compatible ="hisilicon,kirin960-pcie" }, + { .compatible ="hisilicon,kirin970-pcie" }, {}, };
drivers/phy/hisilicon/Kconfig +10 -0
@@ -43,6 +43,16 @@ config PHY_HI3670_USB To compile this driver as a module, choose M here. +config PHY_HI3670_PCIE + tristate "hi3670 PCIe PHY support" + depends on (ARCH_HISI && ARM64) || COMPILE_TEST + select GENERIC_PHY + select MFD_SYSCON + help + Enable this to support the HiSilicon hi3670 PCIe PHY. + + To compile this driver as a module, choose M here. + config PHY_HISTB_COMBPHY tristate "HiSilicon STB SoCs COMBPHY support" depends on (ARCH_HISI && ARM64) || COMPILE_TEST
drivers/phy/hisilicon/Makefile +1 -0
@@ -3,6 +3,7 @@ obj-$(CONFIG_PHY_HI6220_USB) += phy-hi6220-usb.o obj-$(CONFIG_PHY_HI3660_USB) += phy-hi3660-usb3.o obj-$(CONFIG_PHY_HI3660_PCIE) += phy-hi3660-pcie.o obj-$(CONFIG_PHY_HI3670_USB) += phy-hi3670-usb3.o +obj-$(CONFIG_PHY_HI3670_PCIE) += phy-hi3670-pcie.o obj-$(CONFIG_PHY_HISTB_COMBPHY) += phy-histb-combphy.o obj-$(CONFIG_PHY_HISI_INNO_USB2) += phy-hisi-inno-usb2.o obj-$(CONFIG_PHY_HIX5HD2_SATA) += phy-hix5hd2-sata.o
drivers/phy/hisilicon/phy-hi3670-pcie.c new file +892 -0
892 lines not on this page · raw message
Signature · 1 line
2.31.1
PATCH v5 7/8 PCI: kirin: Drop the PHY logic from the driver
In reply to Mauro Carvalho ChehabThe pcie-kirin PCIe driver contains internally a PHY interface for a Kirin 960. Drop it and add support for using the new PHY driver for Kirin 960, updating DT accordingly.
--- arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 29 ++- drivers/pci/controller/dwc/pcie-kirin.c | 252 +++------------------- 2 files changed, 47 insertions(+), 234 deletions(-)
arch/arm64/boot/dts/hisilicon/hi3660.dtsi +18 -11
@@ -995,17 +995,33 @@ spi3: spi@ff3b3000 { status = "disabled"; }; + pcie_phy: pcie-phy@f3f2000 { + compatible = "hisilicon,hi960-pcie-phy"; + reg = <0x0 0xf3f20000 0x0 0x40000>; + reg-names = "phy"; + clocks = <&crg_ctrl HI3660_PCIEPHY_REF>, + <&crg_ctrl HI3660_CLK_GATE_PCIEAUX>, + <&crg_ctrl HI3660_PCLK_GATE_PCIE_PHY>, + <&crg_ctrl HI3660_PCLK_GATE_PCIE_SYS>, + <&crg_ctrl HI3660_ACLK_GATE_PCIE>; + clock-names = "pcie_phy_ref", "pcie_aux", + "pcie_apb_phy", "pcie_apb_sys", + "pcie_aclk"; + reset-gpios = <&gpio11 1 0 >; + #phy-cells = <0>; + }; + pcie@f4000000 { compatible = "hisilicon,kirin960-pcie"; reg = <0x0 0xf4000000 0x0 0x1000>, <0x0 0xff3fe000 0x0 0x1000>, - <0x0 0xf3f20000 0x0 0x40000>, <0x0 0xf5000000 0x0 0x2000>; - reg-names = "dbi", "apb", "phy", "config"; + reg-names = "dbi", "apb", "config"; bus-range = <0x0 0x1>; #address-cells = <3>; #size-cells = <2>; device_type = "pci"; + phys = <&pcie_phy>; ranges = <0x02000000 0x0 0x00000000 0x0 0xf6000000 0x0 0x02000000>;
@@ -1022,15 +1038,6 @@ &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>; - clocks = <&crg_ctrl HI3660_PCIEPHY_REF>, - <&crg_ctrl HI3660_CLK_GATE_PCIEAUX>, - <&crg_ctrl HI3660_PCLK_GATE_PCIE_PHY>, - <&crg_ctrl HI3660_PCLK_GATE_PCIE_SYS>, - <&crg_ctrl HI3660_ACLK_GATE_PCIE>; - clock-names = "pcie_phy_ref", "pcie_aux", - "pcie_apb_phy", "pcie_apb_sys", - "pcie_aclk"; - reset-gpios = <&gpio11 1 0 >; }; /* UFS */
drivers/pci/controller/dwc/pcie-kirin.c +29 -223
335 lines not on this page · raw message
Signature · 1 line
2.31.1
PATCH v5 8/8 PCI: kirin: Use regmap for APB registers
In reply to Mauro Carvalho ChehabThe PHY layer need to access APB registers too, for Kirin 970. So, place them into a named regmap.
--- drivers/pci/controller/dwc/pcie-kirin.c | 49 +++++++++++++------------ 1 file changed, 26 insertions(+), 23 deletions(-)
drivers/pci/controller/dwc/pcie-kirin.c +26 -23
@@ -54,28 +54,30 @@struct kirin_pcie {struct dw_pcie *pci;struct phy *phy; -void __iomem *apb_base; +struct regmap *apb; }; -/* 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 const struct regmap_config pcie_kirin_regmap_conf = { + .name ="kirin_pcie_apb" , + .reg_bits =32 , + .val_bits =32 , + .reg_stride =4 , +};static long kirin_pcie_get_resource (struct kirin_pcie *kirin_pcie,struct platform_device *pdev) { - kirin_pcie->apb_base = -devm_platform_ioremap_resource_byname (pdev,"apb" ); -if (IS_ERR (kirin_pcie->apb_base)) -return PTR_ERR (kirin_pcie->apb_base); +struct device *dev = &pdev->dev; +void __iomem *apb_base; + + apb_base =devm_platform_ioremap_resource_byname (pdev,"apb" ); +if (IS_ERR (apb_base)) +return PTR_ERR (apb_base); + + kirin_pcie->apb =devm_regmap_init_mmio (dev, apb_base, + &pcie_kirin_regmap_conf); +if (IS_ERR (kirin_pcie->apb)) +return PTR_ERR (kirin_pcie->apb);return 0 ; }
@@ -96,13 +98,13 @@ static void kirin_pcie_sideband_dbi_w_mode(struct kirin_pcie *kirin_pcie, {u32 val; - val =kirin_apb_ctrl_readl (kirin_pcie,SOC_PCIECTRL_CTRL0_ADDR ); +regmap_read (kirin_pcie->apb,SOC_PCIECTRL_CTRL0_ADDR , &val);if (on) val = val |PCIE_ELBI_SLV_DBI_ENABLE ;else val = val & ~PCIE_ELBI_SLV_DBI_ENABLE ; -kirin_apb_ctrl_writel (kirin_pcie, val,SOC_PCIECTRL_CTRL0_ADDR ); +regmap_write (kirin_pcie->apb,SOC_PCIECTRL_CTRL0_ADDR , val); } static void kirin_pcie_sideband_dbi_r_mode(struct kirin_pcie *kirin_pcie,
@@ -110,13 +112,13 @@ static void kirin_pcie_sideband_dbi_r_mode(struct kirin_pcie *kirin_pcie, {u32 val; - val =kirin_apb_ctrl_readl (kirin_pcie,SOC_PCIECTRL_CTRL1_ADDR ); +regmap_read (kirin_pcie->apb,SOC_PCIECTRL_CTRL1_ADDR , &val);if (on) val = val |PCIE_ELBI_SLV_DBI_ENABLE ;else val = val & ~PCIE_ELBI_SLV_DBI_ENABLE ; -kirin_apb_ctrl_writel (kirin_pcie, val,SOC_PCIECTRL_CTRL1_ADDR ); +regmap_write (kirin_pcie->apb,SOC_PCIECTRL_CTRL1_ADDR , val); } static int kirin_pcie_rd_own_conf(struct pci_bus *bus, unsigned int devfn,
@@ -176,8 +178,9 @@ static void kirin_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base,static int kirin_pcie_link_up (struct dw_pcie *pci) {struct kirin_pcie *kirin_pcie =to_kirin_pcie (pci); -u32 val =kirin_apb_ctrl_readl (kirin_pcie,PCIE_APB_PHY_STATUS0 ); +u32 val; +regmap_read (kirin_pcie->apb,PCIE_APB_PHY_STATUS0 , &val);if ((val &PCIE_LINKUP_ENABLE ) ==PCIE_LINKUP_ENABLE )return 1 ;
@@ -189,8 +192,8 @@ static int kirin_pcie_start_link(struct dw_pcie *pci)struct kirin_pcie *kirin_pcie =to_kirin_pcie (pci);/* assert LTSSM enable */ -kirin_apb_ctrl_writel (kirin_pcie,PCIE_LTSSM_ENABLE_BIT , -PCIE_APP_LTSSM_ENABLE ); +regmap_write (kirin_pcie->apb,PCIE_APP_LTSSM_ENABLE , +PCIE_LTSSM_ENABLE_BIT );return 0 ; }
Signature · 1 line
2.31.1