Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <3628fbc2eb9a8c21dc0742b929ee14da76f9adf5.camel@icenowy.me>
Subject: Re: [PATCH 2/7] dt-bindings: phy: add binding document for
 Allwinner F1C100s USB PHY
From:   Icenowy Zheng <uwu@icenowy.me>
To:     Rob Herring <robh@kernel.org>
Cc:     Kishon Vijay Abraham I <kishon@ti.com>,
        Vinod Koul <vkoul@kernel.org>, Chen-Yu Tsai <wens@csie.org>,
        Jernej Skrabec <jernej.skrabec@gmail.com>,
        Samuel Holland <samuel@sholland.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Bin Liu <b-liu@ti.com>, linux-kernel@vger.kernel.org,
        linux-phy@lists.infradead.org, devicetree@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev,
        linux-usb@vger.kernel.org
Date:   Wed, 08 Jun 2022 22:52:52 +0800
In-Reply-To: <20220608144939.GA1366879-robh@kernel.org>
References: <20220608070452.338006-1-uwu@icenowy.me>
         <20220608070452.338006-3-uwu@icenowy.me>
         <20220608144939.GA1366879-robh@kernel.org>
Organization: Anthon Open-Source Community
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.40.4 
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

=E5=9C=A8 2022-06-08=E6=98=9F=E6=9C=9F=E4=B8=89=E7=9A=84 08:49 -0600=EF=BC=
=8CRob Herring=E5=86=99=E9=81=93=EF=BC=9A
> On Wed, Jun 08, 2022 at 03:04:47PM +0800, Icenowy Zheng wrote:
> > Allwinner F1C100s has the most simple USB PHY among all Allwinner
> > SoCs,
> > because it has only one OTG USB controller, no host-only OHCI/EHCI
> > controllers.
> >=20
> > Add a binding document for it.
> >=20
> > Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> > ---
> > =C2=A0.../phy/allwinner,suniv-f1c100s-usb-phy.yaml=C2=A0 | 83
> > +++++++++++++++++++
> > =C2=A01 file changed, 83 insertions(+)
> > =C2=A0create mode 100644
> > Documentation/devicetree/bindings/phy/allwinner,suniv-f1c100s-usb-
> > phy.yaml
> >=20
> > diff --git a/Documentation/devicetree/bindings/phy/allwinner,suniv-
> > f1c100s-usb-phy.yaml
> > b/Documentation/devicetree/bindings/phy/allwinner,suniv-f1c100s-
> > usb-phy.yaml
> > new file mode 100644
> > index 000000000000..180fa8840bf7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/allwinner,suniv-
> > f1c100s-usb-phy.yaml
> > @@ -0,0 +1,83 @@
> > +# SPDX-License-Identifier: GPL-2.0
>=20
> Dual license please.

I am based on another Allwinner USB PHY binding file in the same
directory, and that file is single licensed. I created a new file
because each variant of the PHY has a single file now.

>=20
> > +%YAML 1.2
> > +---
> > +$id:
> > http://devicetree.org/schemas/phy/allwinner,suniv-f1c100s-usb-phy.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Allwinner F1C100s USB PHY Device Tree Bindings
> > +
> > +maintainers:
> > +=C2=A0 - Chen-Yu Tsai <wens@csie.org>
> > +=C2=A0 - Maxime Ripard <mripard@kernel.org>
> > +
> > +properties:
> > +=C2=A0 "#phy-cells":
> > +=C2=A0=C2=A0=C2=A0 const: 1
> > +
> > +=C2=A0 compatible:
> > +=C2=A0=C2=A0=C2=A0 const: allwinner,suniv-f1c100s-usb-phy
> > +
> > +=C2=A0 reg:
> > +=C2=A0=C2=A0=C2=A0 maxItems: 1
> > +=C2=A0=C2=A0=C2=A0 description: PHY Control registers
> > +
> > +=C2=A0 reg-names:
> > +=C2=A0=C2=A0=C2=A0 const: phy_ctrl
> > +
> > +=C2=A0 clocks:
> > +=C2=A0=C2=A0=C2=A0 maxItems: 1
> > +=C2=A0=C2=A0=C2=A0 description: USB OTG PHY bus clock
> > +
> > +=C2=A0 clock-names:
> > +=C2=A0=C2=A0=C2=A0 const: usb0_phy
>=20
> *-names is not needed with only one entry. Plus, just using the
> module=20
> name is not a great choice.

However the driver expects it...

Should I patch the driver to use no name on F1C100s?

>=20
> > +
> > +=C2=A0 resets:
> > +=C2=A0=C2=A0=C2=A0 maxItems: 1
> > +=C2=A0=C2=A0=C2=A0 description: USB OTG reset
> > +
> > +=C2=A0 reset-names:
> > +=C2=A0=C2=A0=C2=A0 const: usb0_reset
>=20
> Same here.
>=20
> > +=C2=A0 usb0_id_det-gpios:
> > +=C2=A0=C2=A0=C2=A0 maxItems: 1
> > +=C2=A0=C2=A0=C2=A0 description: GPIO to the USB OTG ID pin
> > +
> > +=C2=A0 usb0_vbus_det-gpios:
> > +=C2=A0=C2=A0=C2=A0 maxItems: 1
> > +=C2=A0=C2=A0=C2=A0 description: GPIO to the USB OTG VBUS detect pin
> > +
> > +=C2=A0 usb0_vbus_power-supply:
> > +=C2=A0=C2=A0=C2=A0 description: Power supply to detect the USB OTG VBU=
S
> > +
> > +=C2=A0 usb0_vbus-supply:
> > +=C2=A0=C2=A0=C2=A0 description: Regulator controlling USB OTG VBUS
>=20
> Why the 'usb0_' prefix?
>=20
> Are these GPIOs and Vbus supply connected to the phy? If not, these
> all=20
> belong in a connector node (as that is where they are connected to in
> h/w).

Well these are historical things of phy-sun4i-usb driver too.

>=20
> > +
> > +required:
> > +=C2=A0 - "#phy-cells"
> > +=C2=A0 - compatible
> > +=C2=A0 - clocks
> > +=C2=A0 - clock-names
> > +=C2=A0 - reg
> > +=C2=A0 - reg-names
> > +=C2=A0 - resets
> > +=C2=A0 - reset-names
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +=C2=A0 - |
> > +=C2=A0=C2=A0=C2=A0 #include <dt-bindings/gpio/gpio.h>
> > +=C2=A0=C2=A0=C2=A0 #include <dt-bindings/clock/suniv-f1c100s-ccu.h>
> > +=C2=A0=C2=A0=C2=A0 #include <dt-bindings/reset/suniv-f1c100s-ccu.h>
> > +
> > +=C2=A0=C2=A0=C2=A0 phy@1c13400 {
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 compatible =3D "allwinner,s=
univ-f1c100s-usb-phy";
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 reg =3D <0x01c13400 0x10>;
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 reg-names =3D "phy_ctrl";
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 clocks =3D <&ccu CLK_USB_PH=
Y0>;
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 clock-names =3D "usb0_phy";
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 resets =3D <&ccu RST_USB_PH=
Y0>;
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 reset-names =3D "usb0_reset=
";
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 #phy-cells =3D <1>;
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 usb0_id_det-gpios =3D <&pio=
 4 2 GPIO_ACTIVE_HIGH>;
> > +=C2=A0=C2=A0=C2=A0 };
> > --=20
> > 2.36.0
> >=20
> >=20