Re: [PATCH 3/8] dt-bindings: phy: Document PCIe PHY in EcoNet EN751221 and EN7528

From: Caleb James DeLisle

Date: Wed Mar 04 2026 - 04:29:10 EST


On 04/03/2026 09:44, Krzysztof Kozlowski wrote:
On Tue, Mar 03, 2026 at 07:09:43PM +0000, Caleb James DeLisle wrote:
EN751221 and EN7528 SoCs have two PCIe slots, and each one has a
PHY which behaves slightly differently because one slot is Gen1/Gen2
while the other is Gen1 only.

Signed-off-by: Caleb James DeLisle <cjd@xxxxxxxx>
---
.../phy/econet,en751221-pcie-phy.yaml | 57 +++++++++++++++++++
Why are you mixing multiple subsystems in the same patchset? That's
like three or four different ones. Don't, just make it difficult to
apply pieces and understand the dependencies.


Please pardon my ignorance, I was under the impression that a patch should be specific to a subsystem but a patchset should accomplish a goal and avoid introducing unused code. In this case the goal is PCIe support. If you prefer it, I can resend as a PHY patchset and a CLK patchset, and then after those are merged, send the PCIe patchset to use them.



MAINTAINERS | 6 ++
2 files changed, 63 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/econet,en751221-pcie-phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/econet,en751221-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/econet,en751221-pcie-phy.yaml
new file mode 100644
index 000000000000..8e1d3c791c6e
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/econet,en751221-pcie-phy.yaml
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/econet,en751221-pcie-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: EcoNet PCI-Express PHY for EcoNet EN751221 and EN7528
+
+maintainers:
+ - Caleb James DeLisle <cjd@xxxxxxxx>
+
+description:
+ The PCIe PHY supports physical layer functionality for PCIe Gen1 and
+ Gen1/Gen2 ports. On these SoCs, port 0 is a Gen1-only port while
+ port 1 is Gen1/Gen2 capable.
+
+properties:
+ compatible:
+ enum:
+ - econet,en751221-pcie-phy0
+ - econet,en751221-pcie-phy1
What is the difference between phy0 and phy1? This must be explicitly
explained in the description.

If phy1 means "port 1" (although first sentence disagrees, because it
says that THE SAME phy supports two ports), then the names aren't -gen1
and -gen2? Or what are other differences?


In practice, port0 is gen1 only and port1 is gen1/2. But I suppose it makes sense to specify them as gen1 and gen2, so that fact is documented in the DT.



+ - econet,en7528-pcie-phy0
+ - econet,en7528-pcie-phy1
+
+ reg:
+ maxItems: 1
+
+ "#phy-cells":
+ const: 0
+
+required:
+ - compatible
+ - reg
+ - '#phy-cells'
Use consisent quotes.

OK


+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/phy/phy.h>
Where do you use it here?
Whoops, I think I daydreamed that it was needed for "#phy-cells", will drop.

+ soc {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ pcie_phy0: pcie-phy@1faf2000 {
Drop unused label.
OK

+ compatible = "econet,en7528-pcie-phy0";
+ reg = <0x1faf2000 0x1000>;
+ #phy-cells = <0>;
+ };
+
+ pcie_phy1: pcie-phy@1fac0000 {
+ compatible = "econet,en7528-pcie-phy1";
Drop node, same as previous one.

OK


Thanks for the review.

Caleb



Best regards,
Krzysztof