Re: [PATCH v2 02/10] dt-bindings: soc: aspeed: Add ASPEED PCIe PHY support
From: Krzysztof Kozlowski <krzk@kernel.org>
Date: 2025-07-16 08:23:39
Also in:
linux-arm-kernel, linux-aspeed, linux-gpio, linux-pci, lkml, openbmc
On Tue, Jul 15, 2025 at 11:43:12AM +0800, Jacky Chou wrote:
This PHY is used by many modules. In our design, our PCIe has RC and EP funcitons. On the different function, each driver will use configure and get some information from the PHY interface to do somting that it wants to. Getting link status, setting syscon credits and so on. Therefore, define it as syscon for all modules. Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com> --- .../bindings/soc/aspeed/aspeed,pcie-phy.yaml | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/aspeed/aspeed,pcie-phy.yaml
Phys go to phy, not soc directory. Soc is not a dumping ground.
quoted hunk ↗ jump to hunk
diff --git a/Documentation/devicetree/bindings/soc/aspeed/aspeed,pcie-phy.yaml b/Documentation/devicetree/bindings/soc/aspeed/aspeed,pcie-phy.yaml new file mode 100644 index 000000000000..5fa585d63ca6 --- /dev/null +++ b/Documentation/devicetree/bindings/soc/aspeed/aspeed,pcie-phy.yaml@@ -0,0 +1,44 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/soc/aspeed/aspeed,pcie-phy.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: ASPEED PCIe PHY + +maintainers: + - Jacky Chou <jacky_chou@aspeedtech.com> + +description: + The ASPEED PCIe PHY provides the physical layer interface for PCIe + controllers in the SoC. This node represents the register block for the PCIe + PHY, which is typically accessed by PCIe Root Complex or Endpoint drivers + via syscon. It is used to configure and get the status of the PCIe PHY + hardware, including power management, link training, and other PHY-specific + operations. + +properties: + compatible: + items: + - enum: + - aspeed,pcie-phy
No, see writing bindings.
+ - const: syscon
It's not a syscon, but phy. I don't think you understood previous feedback. Go back to v1. You just send something to pass the review instead of reworking to make it correct.
+
+ reg:
+ maxItems: 1
+
+ "#phy-cells":
+ const: 0
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ syscon@1e6ed200 {wrong name, that's a phy
+ compatible = "aspeed,pcie-phy", "syscon"; + reg = <0x1e6ed200 0x100>;
Incomplete
+ }; -- 2.43.0