Re: [PATCH 2/9] dt-bindings: phy: qcom,m31: Document qcom,m31 USB phy

On 15/06/2023 07:27, Varadarajan Narayanan wrote:

quoted

quoted

+          - enum:
+              - qcom,m31-usb-hsphy

I am confused what's this. If m31 is coming from some IP block provider,
then you are using wrong vendor prefix.
https://www.m31tech.com/download_file/M31_USB.pdf

quoted

+              - qcom,ipq5332-m31-usb-hsphy

This confuses me even more. IPQ m31?

Will change this to m31,usb-hsphy and m31,ipq5332-usb-hsphy respectively.
Will that be acceptable?

m31,ipq5332 seems wrong, as m31 did not create ipq5332. Does the m31
device have some name/version/model? If it is not really known, then I
would just propose to go with qcom,ipq5332-usb-hsphy.

Skip generic compatible ("usb-hsphy") entirely.

And then we have... existing bindings qcom,usb-hs-phy.yaml. Don't create
something similar with difference in the hyphen. Just use device
specific compatible thus device specific filename.

quoted

quoted

+
+  reg:
+    description:
+      Offset and length of the M31 PHY register set

Drop description, obvious.

Ok.

quoted

quoted

+    maxItems: 2
+
+  reg-names:
+    items:
+      - const: m31usb_phy_base
+      - const: qscratch_base

Drop "_base" from both.

Ok. Will drop qscratch_base. This is in the controller space.
Should not come here.

Then drop reg-names entirely.

quoted

quoted

+
+  phy_type:
+    oneOf:
+      - items:
+          - enum:
+              - utmi
+              - ulpi

This does not belong to phy, but to USB node.

This is used by the driver to set a bit during phy init. Hence
have it as a replication of the USB node's entry. If this is not
permissible, is there some way to get this from the USB node,
or any other alternative mechanism?

Shouldn't USB controller choose what type of PHY type it wants?

quoted

quoted

+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
+    hs_m31phy_0: hs_m31phy@5b00 {

Node names should be generic. See also explanation and list of examples
in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

Also, no underscores in node names.

Will change this as usbphy0:hs_m31phy@7b000

This does not solve my comments. I did not write "label" but "node name".

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel