| Message ID | a04c9c92187ceaee0fd4b8d4721e2a3275d97518.1626157454.git.mchehab+huawei@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | Add support for Hikey 970 PCIe | expand |
On Tue, Jul 13, 2021 at 08:28:37AM +0200, Mauro Carvalho Chehab wrote: > 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? > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > --- > .../devicetree/bindings/pci/kirin-pcie.txt | 16 +++------------- > 1 file changed, 3 insertions(+), 13 deletions(-) > > diff --git a/Documentation/devicetree/bindings/pci/kirin-pcie.txt b/Documentation/devicetree/bindings/pci/kirin-pcie.txt > index 71cac2b74002..a93a8cfa1afb 100644 > --- a/Documentation/devicetree/bindings/pci/kirin-pcie.txt > +++ b/Documentation/devicetree/bindings/pci/kirin-pcie.txt > @@ -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 >; > }; > -- > 2.31.1 > >
Em Tue, 13 Jul 2021 20:28:49 -0600 Rob Herring <robh@kernel.org> escreveu: > On Tue, Jul 13, 2021 at 08:28:37AM +0200, Mauro Carvalho Chehab wrote: > > 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
Em Tue, 13 Jul 2021 20:28:49 -0600 Rob Herring <robh@kernel.org> escreveu: > On Tue, Jul 13, 2021 at 08:28:37AM +0200, Mauro Carvalho Chehab wrote: > > 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> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi index e0eca598af1f..6aaa2f966d74 100644 --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi @@ -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
diff --git a/Documentation/devicetree/bindings/pci/kirin-pcie.txt b/Documentation/devicetree/bindings/pci/kirin-pcie.txt index 71cac2b74002..a93a8cfa1afb 100644 --- a/Documentation/devicetree/bindings/pci/kirin-pcie.txt +++ b/Documentation/devicetree/bindings/pci/kirin-pcie.txt @@ -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 >; };
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. Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> --- .../devicetree/bindings/pci/kirin-pcie.txt | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-)