On Wed, Jun 07, 2023 at 08:31:50PM +0200, Krzysztof Kozlowski wrote: > On 07/06/2023 12:56, Varadarajan Narayanan wrote: > > Document the M31 USB2 phy present in IPQ5332 > > Full stop. Ok. > > Signed-off-by: Sricharan Ramabadhran <quic_srichara@xxxxxxxxxxx> > > Signed-off-by: Varadarajan Narayanan <quic_varada@xxxxxxxxxxx> > > --- > > .../devicetree/bindings/phy/qcom,m31.yaml | 69 ++++++++++++++++++++++ > > 1 file changed, 69 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/phy/qcom,m31.yaml > > > > diff --git a/Documentation/devicetree/bindings/phy/qcom,m31.yaml b/Documentation/devicetree/bindings/phy/qcom,m31.yaml > > new file mode 100644 > > index 0000000..8ad4ba4 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/phy/qcom,m31.yaml > > @@ -0,0 +1,69 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > + > > Drop stray blank lines. Ok. > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/phy/qcom,m31.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Qualcomm M31 USB PHY > > What is M31? M31 is an external IP integrated into IPQ5332 SoC. > > + > > +maintainers: > > + - Sricharan Ramabadhran <quic_srichara@xxxxxxxxxxx> > > + - Varadarajan Narayanan <quic_varada@xxxxxxxxxxx> > > + > > +description: > > + This file contains documentation for the USB M31 PHY found in Qualcomm > > Drop redundant parts ("This file contains documentation for the"). Ok. > > + IPQ5018, IPQ5332 SoCs. > > + > > +properties: > > + compatible: > > + oneOf: > > That's not oneOf. Ok. > > + - items: > > No items. Ok. > > + - 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 > > > > + - 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? > > + > > + reg: > > + description: > > + Offset and length of the M31 PHY register set > > Drop description, obvious. Ok. > > + 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. > > + > > + 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? > > + > > + resets: > > + maxItems: 1 > > + description: > > + List of phandles and reset pairs, one for each entry in reset-names. > > Drop useless description. Ok. > > + > > + reset-names: > > + items: > > + - const: > > + usb2_phy_reset > > Drop entire reset-names. Ok. > > + > > +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 > > + compatible = "qcom,m31-usb-hsphy"; > > + reg = <0x5b000 0x3000>, > > This is not what your unit address is saying. Will change to 0x0007b000. > > + <0x08af8800 0x400>; > > Align it. Will drop this. This is in the controller space. > > + reg-names = "m31usb_phy_base", > > + "qscratch_base"; > > Align it. Ok. > > + phy_type = "utmi"; > > + resets = <&gcc GCC_QUSB2_0_PHY_BCR>; > > + reset-names = "usb2_phy_reset"; > > + > > + status = "ok"; > > Drop. Ok. Hopefully will get an alternative for the phy_type. Thanks Varada > > + }; > > Best regards, > Krzysztof >
