Hi, Lubomir,
Thank you for your review!
> On Wed, Jan 27, 2021 at 01:27:17PM +0200, kostap@xxxxxxxxxxx wrote:
> > From: Konstantin Porotchkin <kostap@xxxxxxxxxxx>
> >
> > Add DTS binding for Marvell CP110 UTMI driver
> >
> > Signed-off-by: Konstantin Porotchkin <kostap@xxxxxxxxxxx>
>
> Any chance you could convert the document to YAML so that it could be used
> for automatic validation?
>
[KP] I believe it is possible, but probably should be done by a separate patch.
Here I am extending the existing documentation.
> > ---
> > Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt | 69
> > ++++++++++++++++++--
> > 1 file changed, 63 insertions(+), 6 deletions(-)
...
> > Required Properties:
> >
> > - compatible: Should be one of:
> > * "marvell,a3700-utmi-host-phy" for the PHY connected to
> > - the USB2 host-only controller.
> > + the USB2 host-only controller (for Armada3700 only).
> > * "marvell,a3700-utmi-otg-phy" for the PHY connected to
> > - the USB3 and USB2 OTG capable controller.
> > + the USB3 and USB2 OTG capable controller (for Armada3700 only.
> > + * "marvell,cp110-utmi-phy" (for Armada 7k/8k or CN913x only).
> > - reg: PHY IP register range.
> > - marvell,usb-misc-reg: handle on the "USB miscellaneous registers" shared
> > region covering registers related to both the host
> > - controller and the PHY.
> > -- #phy-cells: Standard property (Documentation: phy-bindings.txt) Should be
> 0.
> > + controller and the PHY (for Armada3700 only).
> > +- marvell,system-controller: should contain a phandle to the system
> > + controller node (for Armada 7k/8k or CN913x only)
>
> I guess this is okay, but referring to a syscon is done in a multitude ways across
> various other bindings; with the most popular being just:
>
> syscon = <&syscon>;
>
> Perhaps consider doing that?
[KP] I was not sure that I can use a generic name inside the PHY entry
if it is not defined as part of the generic PHY properties.
I just did not see a good example of such in PHY bindings documentation.
If it is legal, I can change this entry name to just "syscon".
>
> > +- #phy-cells: Standard property (Documentation: phy-bindings.txt.
> > + Should be 0 (for Armada3700 only).
> > +
> > +
> > +Required properties (child nodes, for Armada 7k/8k/CN913x only):
> > +
> > +- reg: UTMI PHY port ID (0 or 1).
> > +- #phy-cells : Should be 0.
> > +
> > +
> > +Optional Properties (child nodes, for Armada 7k/8k/CN913x only)::
> >
> > +- marvell,cp110-utmi-device-mode: request the driver to connect the UTMI
> PHY
> > + port to USB device controller.
>
> Do you need a separate property for this? Could the driver look at "dr_mode"
> property of the USB controller node to see if it's supposed to be in
> device/peripheral mode?
[KP] Yes, it seems I missed this option. I will try to change the code to support it in version 2.
>
> > Example:
> >
> > +Armada3700
> > usb2_utmi_host_phy: phy@5f000 {
> > compatible = "marvell,armada-3700-utmi-host-phy";
> > reg = <0x5f000 0x800>;
> > @@ -36,3 +67,29 @@ Example:
> > --
> > 2.17.1
> >
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__lists.infradead.org_mailman_listinfo_linux-2Darm-
> 2Dkernel&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=-
> N9sN4p5NSr0JGQoQ_2UCOgAqajG99W1EbSOww0WU8o&m=_ZOAKZShBT3Qj
> uT3RZIld2HoLnlvv6gkbHW9gSvEfI4&s=ggCBpvhDLJ8M6-
> Q41qbt8GRxryUo_mHxLMkUl8Ao5mA&e=
