Received-SPF: pass (google.com: domain of linux-kernel+bounces-165623-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Wed, 1 May 2024 18:05:28 +0100
From: Conor Dooley <conor@kernel.org>
To: Inochi Amaoto <inochiama@outlook.com>
Cc: Vinod Koul <vkoul@kernel.org>,
	Kishon Vijay Abraham I <kishon@kernel.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Chen Wang <unicorn_wang@outlook.com>,
	Jisheng Zhang <jszhang@kernel.org>,
	Liu Gui <kenneth.liu@sophgo.com>,
	Jingbao Qiu <qiujingbao.dlmu@gmail.com>, dlan@gentoo.org,
	linux-phy@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] dt-bindings: phy: Add Sophgo CV1800 USB phy
Message-ID: <20240501-effort-virus-9baa07daf429@spud>
References: <IA1PR20MB4953182B8EDB465430289400BB1B2@IA1PR20MB4953.namprd20.prod.outlook.com>
 <IA1PR20MB495355A4186420B78BD78F49BB1B2@IA1PR20MB4953.namprd20.prod.outlook.com>
 <20240430-crummiest-overnight-0f46dba32bb8@spud>
 <IA1PR20MB49535665CF1C89FFB4E1E46DBB192@IA1PR20MB4953.namprd20.prod.outlook.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="LScSdEfhhOhhxX79"
Content-Disposition: inline
In-Reply-To: <IA1PR20MB49535665CF1C89FFB4E1E46DBB192@IA1PR20MB4953.namprd20.prod.outlook.com>


--LScSdEfhhOhhxX79
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, May 01, 2024 at 08:28:47AM +0800, Inochi Amaoto wrote:
> On Tue, Apr 30, 2024 at 06:09:20PM GMT, Conor Dooley wrote:
> > On Mon, Apr 29, 2024 at 08:31:11AM +0800, Inochi Amaoto wrote:
> > > The USB phy of Sophgo CV18XX series SoC needs to sense a pin called
> > > "VBUS_DET" to get the right operation mode. If this pin is not
> > > connected, it only supports setting the mode manually.
> > >=20
> > > Add USB phy bindings for Sophgo CV18XX/SG200X series SoC.
> > >=20
> > > Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
> > > ---
> > >  .../bindings/phy/sophgo,cv1800-usb-phy.yaml   | 68 +++++++++++++++++=
++
> > >  1 file changed, 68 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/phy/sophgo,cv18=
00-usb-phy.yaml
> > >=20
> > > diff --git a/Documentation/devicetree/bindings/phy/sophgo,cv1800-usb-=
phy.yaml b/Documentation/devicetree/bindings/phy/sophgo,cv1800-usb-phy.yaml
> > > new file mode 100644
> > > index 000000000000..7e3382c18d44
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/phy/sophgo,cv1800-usb-phy.yaml
> > > @@ -0,0 +1,68 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/phy/sophgo,cv1800-usb-phy.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Sophgo CV18XX/SG200X USB 2.0 PHY
> > > +
> > > +maintainers:
> > > +  - Inochi Amaoto <inochiama@outlook.com>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: sophgo,cv1800-usb-phy
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  "#phy-cells":
> > > +    const: 0
> > > +
> > > +  clocks:
> > > +    items:
> > > +      - description: PHY clock
> > > +      - description: PHY app clock
> > > +      - description: PHY stb clock
> > > +      - description: PHY lpm clock
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: phy
> > > +      - const: app
> > > +      - const: stb
> > > +      - const: lpm
> > > +
> > > +  vbus_det-gpios:
> >=20
> > "vbus_det-gpios" isn't a common property AFAICT, why does it not get a
> > vendor prefix when the other gpios property does?
> >=20
>=20
> I have seen other binding (such as sunxi phy) uses this property without=
=20
> vendor prefix. So I think it is a common property and don't add perfix=20
> for it. But it is OK for me add the vendor prefix, thanks.

I asked yesterday and Krzysztof said:
"gpios just like supplies - no prefixes, I think."
The other phy used a different property (they're not identical), I did
check that yesterday.

>=20
> > > +    description: GPIO to the USB OTG VBUS detect pin. This should no=
t be
> > > +      defined if vbus_det pin and switch pin are connected, which may
> > > +      break the VBUS detection.
> > > +    maxItems: 1
> > > +
> > > +  sophgo,switch-gpios:
> > > +    description: GPIO array for the phy to control connected switch.=
 For
> > > +      host mode, the driver will set these GPIOs to low one by one. =
For
> > > +      device mode, the driver will set these GPIOs to high in reverse
> > > +      order.
> > > +    maxItems: 2
> >=20
> > You're still missing the itemised description of what each of the gpios
> > here are - how would I know which order to put the GPIOs in?
> >=20
> > Cheers,
> > Conor.
>=20
> In most case, the order depends on hardware design. But following the
> guide sophgo provides, it does have an common order. Is it good to add
> this description as a reference guide? If so, I will add it.

If we can say "gpio 1 connects to xyz on the switch" & "gpio 2 connects
to tuv" then it'll be easier to understand how to write the node.

>=20
> Regards,
> Inochi
>=20
> >=20
> > > +
> > > +required:
> > > +  - compatible
> > > +  - "#phy-cells"
> > > +  - clocks
> > > +  - clock-names
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    phy@48 {
> > > +      compatible =3D "sophgo,cv1800-usb-phy";
> > > +      reg =3D <0x48 0x4>;
> > > +      #phy-cells =3D <0>;
> > > +      clocks =3D <&clk 92>, <&clk 93>,
> > > +               <&clk 94>, <&clk 95>;
> > > +      clock-names =3D "phy", "app", "stb", "lpm";
> > > +    };
> > > +
> > > +...
> > > --
> > > 2.44.0
> > >=20
>=20
>=20

--LScSdEfhhOhhxX79
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZjJ2WAAKCRB4tDGHoIJi
0t3pAP0SMmeNRY7MkfCW54V9OEGUp1dtBrR5XDLqYPsCAYvb5QEA17rw7TzzPlPc
Is2n9IJ026qIajuo2X0UOUZlLuZiRwU=
=S11l
-----END PGP SIGNATURE-----

--LScSdEfhhOhhxX79--