| Message ID | 20220728143427.13617-4-Sergey.Semin@baikalelectronics.ru (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | PCI: dwc: Add generic resources and Baikal-T1 support | expand |
On Thu, Jul 28, 2022 at 05:34:13PM +0300, Serge Semin wrote: > It's normal to have the DW PCIe RP/EP DT-nodes equipped with the explicit > PHY phandle references. There can be up to 16 PHYs attach in accordance > with the maximum number of supported PCIe lanes. Let's extend the common > DW PCIe controller schema with the 'phys' and 'phy-names' properties > definition. The PHY names are defined with the regexp pattern > '^pcie([0-9]+|-?phy[0-9]*)?$' so to match the names currently supported by > the DW PCIe platform drivers ("pcie": meson; "pciephy": qcom, imx6; > "pcie-phy": uniphier, rockchip, spear13xx; "pcie": intel-gw; "pcie-phy%d": > keystone, dra7xx; "pcie": histb, etc). Though the "pcie%d" format would > the most preferable in this case. No phy-names is my preference. Some string plus an index is utterly pointless. Oh well... > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > --- > > Changelog v3: > - This is a new patch unpinned from the next one: > https://lore.kernel.org/linux-pci/20220503214638.1895-2-Sergey.Semin@baikalelectronics.ru/ > by the Rob' request. (@Rob) > --- > .../bindings/pci/snps,dw-pcie-common.yaml | 15 +++++++++++++++ > .../devicetree/bindings/pci/snps,dw-pcie-ep.yaml | 3 +++ > .../devicetree/bindings/pci/snps,dw-pcie.yaml | 3 +++ > 3 files changed, 21 insertions(+) > > diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml > index 3e992b653d12..627a5d6625ba 100644 > --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml > +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml > @@ -17,6 +17,21 @@ description: > select: false > > properties: > + phys: > + description: > + There can be up to the number of possible lanes PHYs specified. This needs something about being in order of lane number. > + Obviously each specified PHY is supposed to be able to work in the > + PCIe mode with a speed implied by the DWC PCIe controller it is > + attached to. > + minItems: 1 > + maxItems: 16 > + > + phy-names: > + minItems: 1 > + maxItems: 16 > + items: > + pattern: '^pcie([0-9]+|-?phy[0-9]*)?$' Please comment here that pcie[0-9] is the preferred form. Rob
On Mon, Aug 01, 2022 at 11:56:36AM -0600, Rob Herring wrote: > On Thu, Jul 28, 2022 at 05:34:13PM +0300, Serge Semin wrote: > > It's normal to have the DW PCIe RP/EP DT-nodes equipped with the explicit > > PHY phandle references. There can be up to 16 PHYs attach in accordance > > with the maximum number of supported PCIe lanes. Let's extend the common > > DW PCIe controller schema with the 'phys' and 'phy-names' properties > > definition. The PHY names are defined with the regexp pattern > > '^pcie([0-9]+|-?phy[0-9]*)?$' so to match the names currently supported by > > the DW PCIe platform drivers ("pcie": meson; "pciephy": qcom, imx6; > > "pcie-phy": uniphier, rockchip, spear13xx; "pcie": intel-gw; "pcie-phy%d": > > keystone, dra7xx; "pcie": histb, etc). Though the "pcie%d" format would > > the most preferable in this case. > > No phy-names is my preference. Some string plus an index is utterly > pointless. Oh well... Mine too, but we have no choice in this case since the named PHY-properties support has already been advertised by the platform drivers. This patch just provides the bindings for them. Just note the string patterns have been designed in a way to match these bindings. Anyway thanks for telling about the preferred option. Keeping that in mind I won't have doubts what approach to select for the new driver/bindings development. > > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > > > --- > > > > Changelog v3: > > - This is a new patch unpinned from the next one: > > https://lore.kernel.org/linux-pci/20220503214638.1895-2-Sergey.Semin@baikalelectronics.ru/ > > by the Rob' request. (@Rob) > > --- > > .../bindings/pci/snps,dw-pcie-common.yaml | 15 +++++++++++++++ > > .../devicetree/bindings/pci/snps,dw-pcie-ep.yaml | 3 +++ > > .../devicetree/bindings/pci/snps,dw-pcie.yaml | 3 +++ > > 3 files changed, 21 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml > > index 3e992b653d12..627a5d6625ba 100644 > > --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml > > +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml > > @@ -17,6 +17,21 @@ description: > > select: false > > > > properties: > > + phys: > > + description: > > + There can be up to the number of possible lanes PHYs specified. > > This needs something about being in order of lane number. Ok. > > > + Obviously each specified PHY is supposed to be able to work in the > > + PCIe mode with a speed implied by the DWC PCIe controller it is > > + attached to. > > + minItems: 1 > > + maxItems: 16 > > + > > + phy-names: > > + minItems: 1 > > + maxItems: 16 > > + items: > > + pattern: '^pcie([0-9]+|-?phy[0-9]*)?$' > > Please comment here that pcie[0-9] is the preferred form. What about a bit more sophisticated update? phy-names: minItems: 1 maxItems: 16 oneOf: - items: pattern: '^pcie[0-9]+$' - deprecated: true items: pattern: '^pcie(-?phy[0-9]*)?$' -Sergey > > Rob
On Mon, Aug 8, 2022 at 4:36 AM Serge Semin <fancer.lancer@gmail.com> wrote: > > On Mon, Aug 01, 2022 at 11:56:36AM -0600, Rob Herring wrote: > > On Thu, Jul 28, 2022 at 05:34:13PM +0300, Serge Semin wrote: > > > It's normal to have the DW PCIe RP/EP DT-nodes equipped with the explicit > > > PHY phandle references. There can be up to 16 PHYs attach in accordance > > > with the maximum number of supported PCIe lanes. Let's extend the common > > > DW PCIe controller schema with the 'phys' and 'phy-names' properties > > > definition. The PHY names are defined with the regexp pattern > > > '^pcie([0-9]+|-?phy[0-9]*)?$' so to match the names currently supported by > > > the DW PCIe platform drivers ("pcie": meson; "pciephy": qcom, imx6; > > > "pcie-phy": uniphier, rockchip, spear13xx; "pcie": intel-gw; "pcie-phy%d": > > > keystone, dra7xx; "pcie": histb, etc). Though the "pcie%d" format would > > > the most preferable in this case. > > > > > No phy-names is my preference. Some string plus an index is utterly > > pointless. Oh well... > > Mine too, but we have no choice in this case since the named > PHY-properties support has already been advertised by the platform > drivers. This patch just provides the bindings for them. Just note > the string patterns have been designed in a way to match these bindings. > > Anyway thanks for telling about the preferred option. Keeping that in > mind I won't have doubts what approach to select for the new > driver/bindings development. > > > > > > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > > > > > --- > > > > > > Changelog v3: > > > - This is a new patch unpinned from the next one: > > > https://lore.kernel.org/linux-pci/20220503214638.1895-2-Sergey.Semin@baikalelectronics.ru/ > > > by the Rob' request. (@Rob) > > > --- > > > .../bindings/pci/snps,dw-pcie-common.yaml | 15 +++++++++++++++ > > > .../devicetree/bindings/pci/snps,dw-pcie-ep.yaml | 3 +++ > > > .../devicetree/bindings/pci/snps,dw-pcie.yaml | 3 +++ > > > 3 files changed, 21 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml > > > index 3e992b653d12..627a5d6625ba 100644 > > > --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml > > > +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml > > > @@ -17,6 +17,21 @@ description: > > > select: false > > > > > > properties: > > > + phys: > > > + description: > > > + There can be up to the number of possible lanes PHYs specified. > > > > > This needs something about being in order of lane number. > > Ok. > > > > > > + Obviously each specified PHY is supposed to be able to work in the > > > + PCIe mode with a speed implied by the DWC PCIe controller it is > > > + attached to. > > > + minItems: 1 > > > + maxItems: 16 > > > + > > > + phy-names: > > > + minItems: 1 > > > + maxItems: 16 > > > + items: > > > + pattern: '^pcie([0-9]+|-?phy[0-9]*)?$' > > > > > Please comment here that pcie[0-9] is the preferred form. > > What about a bit more sophisticated update? > phy-names: > minItems: 1 > maxItems: 16 > oneOf: > - items: > pattern: '^pcie[0-9]+$' > - deprecated: true > items: > pattern: '^pcie(-?phy[0-9]*)?$' Sure. Rob
diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml index 3e992b653d12..627a5d6625ba 100644 --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml @@ -17,6 +17,21 @@ description: select: false properties: + phys: + description: + There can be up to the number of possible lanes PHYs specified. + Obviously each specified PHY is supposed to be able to work in the + PCIe mode with a speed implied by the DWC PCIe controller it is + attached to. + minItems: 1 + maxItems: 16 + + phy-names: + minItems: 1 + maxItems: 16 + items: + pattern: '^pcie([0-9]+|-?phy[0-9]*)?$' + reset-gpio: deprecated: true description: diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml index 7d05dcba419b..dcd521aed213 100644 --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml @@ -52,4 +52,7 @@ examples: <0xdfc01000 0x0001000>, /* IP registers 2 */ <0xd0000000 0x2000000>; /* Configuration space */ reg-names = "dbi", "dbi2", "addr_space"; + + phys = <&pcie_phy0>, <&pcie_phy1>, <&pcie_phy2>, <&pcie_phy3>; + phy-names = "pcie0", "pcie1", "pcie2", "pcie3"; }; diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml index 2810e9b5cc8d..4a5c8b933b52 100644 --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml @@ -70,5 +70,8 @@ examples: reset-gpios = <&port0 0 1>; + phys = <&pcie_phy>; + phy-names = "pcie"; + num-lanes = <1>; };
It's normal to have the DW PCIe RP/EP DT-nodes equipped with the explicit PHY phandle references. There can be up to 16 PHYs attach in accordance with the maximum number of supported PCIe lanes. Let's extend the common DW PCIe controller schema with the 'phys' and 'phy-names' properties definition. The PHY names are defined with the regexp pattern '^pcie([0-9]+|-?phy[0-9]*)?$' so to match the names currently supported by the DW PCIe platform drivers ("pcie": meson; "pciephy": qcom, imx6; "pcie-phy": uniphier, rockchip, spear13xx; "pcie": intel-gw; "pcie-phy%d": keystone, dra7xx; "pcie": histb, etc). Though the "pcie%d" format would the most preferable in this case. Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> --- Changelog v3: - This is a new patch unpinned from the next one: https://lore.kernel.org/linux-pci/20220503214638.1895-2-Sergey.Semin@baikalelectronics.ru/ by the Rob' request. (@Rob) --- .../bindings/pci/snps,dw-pcie-common.yaml | 15 +++++++++++++++ .../devicetree/bindings/pci/snps,dw-pcie-ep.yaml | 3 +++ .../devicetree/bindings/pci/snps,dw-pcie.yaml | 3 +++ 3 files changed, 21 insertions(+)