On Tue, Aug 13, 2024 at 07:31:50AM +0200, Jan Kiszka wrote:

On 12.08.24 17:55, Conor Dooley wrote:

quoted

On Mon, Aug 12, 2024 at 04:15:51PM +0200, Jan Kiszka wrote:

quoted

From: Jan Kiszka <jan.kiszka@siemens.com>

Analogously to the PCI PHY, access to sys_syscon is needed to connect
the USB PHY to its controller.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
CC: Rob Herring <robh@kernel.org>
CC: Krzysztof Kozlowski <krzk+dt@kernel.org>
CC: Conor Dooley <conor+dt@kernel.org>
---
 .../bindings/phy/starfive,jh7110-usb-phy.yaml         | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/starfive,jh7110-usb-phy.yaml b/Documentation/devicetree/bindings/phy/starfive,jh7110-usb-phy.yaml
index 269e9f9f12b6..eaf0050c6f17 100644
--- a/Documentation/devicetree/bindings/phy/starfive,jh7110-usb-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/starfive,jh7110-usb-phy.yaml

@@ -19,6 +19,16 @@ properties:
   "#phy-cells":
     const: 0
 
+  starfive,sys-syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      - items:
+          - description: phandle to System Register Controller sys_syscon node.
+          - description: PHY connect offset of SYS_SYSCONSAIF__SYSCFG register for USB PHY.

Why is having a new property for this required? The devicetree only has
a single usb phy, so isn't it sufficient to look up the syscon by
compatible, rather than via phandle + offset?

I didn't design this, I just copied it from
starfive,jh7110-pcie-phy.yaml. As that already exists, I'm neither sure
we want to change that anymore nor deviate in the pattern here.

To be honest, I think some of the other users of phandle + offset on
this soc were just copy-pasted without thinking about whether or not they
were required too. This one seems like it should just be a lookup by
compatible in the driver instead of by phandle. As a bonus, it will work
with existing devicetrees - whereas your current implementation will
fail to probe on systems that have the old devicetree, a regression for
systems running with that devicetree and downstream firmware.

Cheers,
Conor.

Jan

quoted

quoted

+    description:
+      The phandle to System Register Controller syscon node and the PHY connect offset
+      of SYS_SYSCONSAIF__SYSCFG register. Connect PHY to USB controller.
+
   clocks:
     items:
       - description: PHY 125m

@@ -47,4 +57,5 @@ examples:
                  <&stgcrg 6>;
         clock-names = "125m", "app_125m";
         #phy-cells = <0>;
+        starfive,sys-syscon = <&sys_syscon 0x18>;
     };

-- 
2.43.0

-- 
Siemens AG, Technology
Linux Expert Center