diff mbox series

[2/5] dt-bindings: phy: mediatek, mt7621-pci-phy: add clock entries

Message ID 20210506111531.21978-3-sergio.paracuellos@gmail.com
State Superseded
Headers show
Series phy: ralink: mt7621-pci-phy: some improvements | expand

Commit Message

Sergio Paracuellos May 6, 2021, 11:15 a.m. UTC
MT7621 SoC clock driver has already mainlined in
'commit 48df7a26f470 ("clk: ralink: add clock driver for mt7621 SoC")'
Hence update schema with the add of the entries related to
clock.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 .../bindings/phy/mediatek,mt7621-pci-phy.yaml        | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Rob Herring (Arm) May 7, 2021, 10:12 p.m. UTC | #1
On Thu, May 06, 2021 at 01:15:28PM +0200, Sergio Paracuellos wrote:
> MT7621 SoC clock driver has already mainlined in
> 'commit 48df7a26f470 ("clk: ralink: add clock driver for mt7621 SoC")'
> Hence update schema with the add of the entries related to
> clock.
> 
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  .../bindings/phy/mediatek,mt7621-pci-phy.yaml        | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml b/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml
> index 0ccaded3f245..d8614ef8995c 100644
> --- a/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml
> @@ -16,6 +16,14 @@ properties:
>    reg:
>      maxItems: 1
>  
> +  clocks:
> +    maxItems: 1
> +    description:
> +      PHY reference clock. Must contain an entry in clock-names.
> +
> +  clock-names:
> +    const: sys_clk

You don't really need -names when there is only 1.

> +
>    "#phy-cells":
>      const: 1
>      description: selects if the phy is dual-ported
> @@ -23,6 +31,8 @@ properties:
>  required:
>    - compatible
>    - reg
> +  - clocks
> +  - clock-names

Technically, you can't add new properties and make them required as that 
breaks the ABI. If that's okay here, explain it in the commit message. 

>    - "#phy-cells"
>  
>  additionalProperties: false
> @@ -32,5 +42,7 @@ examples:
>      pcie0_phy: pcie-phy@1e149000 {
>        compatible = "mediatek,mt7621-pci-phy";
>        reg = <0x1e149000 0x0700>;
> +      clocks = <&sysc 0>;
> +      clock-names = "sys_clk";
>        #phy-cells = <1>;
>      };
> -- 
> 2.25.1
>
Sergio Paracuellos May 8, 2021, 6:40 a.m. UTC | #2
Hi Rob,

On Sat, May 8, 2021 at 12:12 AM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, May 06, 2021 at 01:15:28PM +0200, Sergio Paracuellos wrote:
> > MT7621 SoC clock driver has already mainlined in
> > 'commit 48df7a26f470 ("clk: ralink: add clock driver for mt7621 SoC")'
> > Hence update schema with the add of the entries related to
> > clock.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> >  .../bindings/phy/mediatek,mt7621-pci-phy.yaml        | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml b/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml
> > index 0ccaded3f245..d8614ef8995c 100644
> > --- a/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml
> > +++ b/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml
> > @@ -16,6 +16,14 @@ properties:
> >    reg:
> >      maxItems: 1
> >
> > +  clocks:
> > +    maxItems: 1
> > +    description:
> > +      PHY reference clock. Must contain an entry in clock-names.
> > +
> > +  clock-names:
> > +    const: sys_clk
>
> You don't really need -names when there is only 1.

Ok, I will drop this property then and
>
> > +
> >    "#phy-cells":
> >      const: 1
> >      description: selects if the phy is dual-ported
> > @@ -23,6 +31,8 @@ properties:
> >  required:
> >    - compatible
> >    - reg
> > +  - clocks
> > +  - clock-names
>
> Technically, you can't add new properties and make them required as that
> breaks the ABI. If that's okay here, explain it in the commit message.

So until now no clock driver existed and things were not properly
being done in driver code directly accessing registers to get the
clock frequency to properly configure the phy. Since the new clock
driver enters into the scene, make this mandatory force to update both
driver and dtb, which is something pretty normal when upgrading the
kind of devices using this SoC. So I think it should be finde to make
this a requirement.

>
> >    - "#phy-cells"
> >
> >  additionalProperties: false
> > @@ -32,5 +42,7 @@ examples:
> >      pcie0_phy: pcie-phy@1e149000 {
> >        compatible = "mediatek,mt7621-pci-phy";
> >        reg = <0x1e149000 0x0700>;
> > +      clocks = <&sysc 0>;
> > +      clock-names = "sys_clk";
> >        #phy-cells = <1>;
> >      };
> > --
> > 2.25.1
> >

Best regards,
    Sergio Paracuellos
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml b/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml
index 0ccaded3f245..d8614ef8995c 100644
--- a/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml
@@ -16,6 +16,14 @@  properties:
   reg:
     maxItems: 1
 
+  clocks:
+    maxItems: 1
+    description:
+      PHY reference clock. Must contain an entry in clock-names.
+
+  clock-names:
+    const: sys_clk
+
   "#phy-cells":
     const: 1
     description: selects if the phy is dual-ported
@@ -23,6 +31,8 @@  properties:
 required:
   - compatible
   - reg
+  - clocks
+  - clock-names
   - "#phy-cells"
 
 additionalProperties: false
@@ -32,5 +42,7 @@  examples:
     pcie0_phy: pcie-phy@1e149000 {
       compatible = "mediatek,mt7621-pci-phy";
       reg = <0x1e149000 0x0700>;
+      clocks = <&sysc 0>;
+      clock-names = "sys_clk";
       #phy-cells = <1>;
     };