Re: [PATCH 1/5] dt-bindings: phy: Add STM32MP25 COMBOPHY bindings

On 8/27/24 15:23, Krzysztof Kozlowski wrote:

On 27/08/2024 14:24, Christian Bruel wrote:

quoted

Document the bindings for STM32 COMBOPHY interface, used to support
the PCIe and USB3 stm32mp25 drivers.
Following entries can be used to tune caracterisation parameters
  - st,output-micro-ohms and st,output-vswing-microvolt bindings entries
to tune the impedance and voltage swing using discrete simulation results
  - st,rx-equalizer register to set the internal rx equalizer filter value.

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>

v1? Or v3?

sorry, forgot to update Subject: it is v3, will resend a v4 with your 
further comments

Regards

Christian

quoted

---
  .../bindings/phy/st,stm32-combophy.yaml       | 144 ++++++++++++++++++
  1 file changed, 144 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/phy/st,stm32-combophy.yaml

diff --git a/Documentation/devicetree/bindings/phy/st,stm32-combophy.yaml b/Documentation/devicetree/bindings/phy/st,stm32-combophy.yaml
new file mode 100644
index 000000000000..c33a843b83a3
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/st,stm32-combophy.yaml

Filename matching compatible.

ok

quoted

@@ -0,0 +1,144 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/st,stm32-combophy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STMicroelectronics STM32MP25 USB3/PCIe COMBOPHY
+
+maintainers:
+  - Christian Bruel <christian.bruel@foss.st.com>
+
+description:
+  Single lane PHY shared (exclusive) between the USB3 and PCIe controllers.
+  Supports 5Gbit/s for USB3 and PCIe gen2 or 2.5Gbit/s for PCIe gen1.
+
+properties:
+  compatible:
+    const: st,stm32mp25-combophy
+
+  reg:
+    maxItems: 1
+
+  "#phy-cells":
+    const: 1
+    description: |
+      The cells contain the following arguments.
+
+      - description: The PHY type

That's some sort of mess. Is it schema within description or schema? Why
two descriptions?

yes, indeed

quoted

+          enum:
+            - PHY_TYPE_USB3
+            - PHY_TYPE_PCIE
+

...

quoted

+required:
+  - compatible
+  - reg
+  - st,syscfg
+  - '#phy-cells'
+  - resets
+  - reset-names
+  - clocks
+  - clock-names
+
+allOf:
+  - if:
+      required:
+        - wakeup-source
+    then:
+      anyOf:
+        - required: [interrupts]
+        - required: [interrupts-extended]
+

I do not see any improvements.

The tag you received was CONDITIONAL. If you do not apply the comments,
you cannot just apply the tag.

my mistake again.

Best regards,
Krzysztof