Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
MIME-Version: 1.0
References: <20231022131252.801090-1-sergio.paracuellos@gmail.com> <0e6a3e08-4f66-4bc1-a8a0-a7bb40472136@linaro.org>
In-Reply-To: <0e6a3e08-4f66-4bc1-a8a0-a7bb40472136@linaro.org>
From:   Sergio Paracuellos <sergio.paracuellos@gmail.com>
Date:   Sun, 22 Oct 2023 18:25:28 +0200
Message-ID: <CAMhs-H8ZFCYU+wJ_k6AMr+43Skzhp4xDD8_xyQPusoGGm6C2VA@mail.gmail.com>
Subject: Re: [PATCH v2] dt-bindings: phy: ralink-usb-phy: convert to dtschema
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc:     devicetree@vger.kernel.org, vkoul@kernel.org, kishon@kernel.org,
        robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
        conor+dt@kernel.org, matthias.bgg@gmail.com,
        angelogioacchino.delregno@collabora.com,
        linux-kernel@vger.kernel.org, linux-phy@lists.infradead.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Krysztof,

On Sun, Oct 22, 2023 at 5:57=E2=80=AFPM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 22/10/2023 15:12, Sergio Paracuellos wrote:
> > Convert the ralink-usb-phy bindings to DT schema.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> > Changes in v2:
> >  - Add missing SPDX License Identifier
> >
> >  .../bindings/phy/ralink-usb-phy.txt           | 23 ------
> >  .../bindings/phy/ralink-usb-phy.yaml          | 70 +++++++++++++++++++
> >  2 files changed, 70 insertions(+), 23 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/phy/ralink-usb-ph=
y.txt
> >  create mode 100644 Documentation/devicetree/bindings/phy/ralink-usb-ph=
y.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/phy/ralink-usb-phy.txt b=
/Documentation/devicetree/bindings/phy/ralink-usb-phy.txt
> > deleted file mode 100644
> > index 9d2868a437ab..000000000000
> > --- a/Documentation/devicetree/bindings/phy/ralink-usb-phy.txt
> > +++ /dev/null
> > @@ -1,23 +0,0 @@
> > -Mediatek/Ralink USB PHY
> > -
> > -Required properties:
> > - - compatible: "ralink,rt3352-usbphy"
> > -            "mediatek,mt7620-usbphy"
> > -            "mediatek,mt7628-usbphy"
> > - - reg: required for "mediatek,mt7628-usbphy", unused otherwise
> > - - #phy-cells: should be 0
> > - - ralink,sysctl: a phandle to a ralink syscon register region
> > - - resets: the two reset controllers for host and device
> > - - reset-names: the names of the 2 reset controllers
> > -
> > -Example:
> > -
> > -usbphy: phy {
> > -     compatible =3D "mediatek,mt7628-usbphy";
> > -     reg =3D <0x10120000 0x1000>;
> > -     #phy-cells =3D <0>;
> > -
> > -     ralink,sysctl =3D <&sysc>;
> > -     resets =3D <&rstctrl 22 &rstctrl 25>;
> > -     reset-names =3D "host", "device";
> > -};
> > diff --git a/Documentation/devicetree/bindings/phy/ralink-usb-phy.yaml =
b/Documentation/devicetree/bindings/phy/ralink-usb-phy.yaml
> > new file mode 100644
> > index 000000000000..b05665f5b641
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/ralink-usb-phy.yaml
>
> Filename matching compatible. Missing vendor prefix, comma.

Sure, will use mediatek,mt7628-usbphy compatible, then.

>
>
> > @@ -0,0 +1,70 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/phy/ralink-usb-phy.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Mediatek/Ralink USB PHY
> > +
> > +maintainers:
> > +  - Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - ralink,rt3352-usbphy
> > +      - mediatek,mt7620-usbphy
> > +      - mediatek,mt7628-usbphy
>
> Keep them ordered alphabetically, please.

Will do.

>
> Blank line here.

True, thanks.

>
>
> > +  reg:
> > +    maxItems: 1
> > +
> > +  "#phy-cells":
> > +    const: 0
> > +
> > +  ralink,sysctl:
> > +    description:
> > +      phandle to a ralink syscon register region.
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +
> > +  resets:
> > +    items:
> > +      - description: USB Host reset controller
> > +      - description: USB Device reset controller
> > +
> > +  reset-names:
> > +    items:
> > +      - const: host
> > +      - const: device
> > +
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: mediatek,mt7628-usbphy
> > +    then:
> > +      required:
> > +        - reg
>
> else:
> is it even valid?

Will add else with reg to false for the rest since looking into driver
code it looks like it is not using for other compatible at all.

>
> Anyway, please put allOf: block after required: block.

Will do.

>
> > +
> > +required:
> > +  - compatible
> > +  - "#phy-cells"
> > +  - ralink,sysctl
> > +  - resets
> > +  - reset-names
> > +
>
>
> Best regards,
> Krzysztof
>

Thanks,
    Sergio Paracuellos