| Message ID | 20220331151440.3643482-1-michael@walle.cc |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [RFC,net-next,1/3] dt-bindings: net: convert mscc-miim to YAML format | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 1 warnings, 55 lines checked |
| robh/patch-applied | fail | build log |
[Forgot the cover letter] Subject: [PATCH RFC net-next 0/3] net: phy: mscc-miim: add MDIO bus frequency support Introduce MDIO bus frequency support. This way the board can have a faster (or maybe slower) bus frequency than the hardware default. Michael Walle (3): dt-bindings: net: convert mscc-miim to YAML format dt-bindings: net: mscc-miim: add clock and clock-frequency net: phy: mscc-miim: add support to set MDIO bus frequency .../devicetree/bindings/net/mscc,miim.yaml | 60 +++++++++++++++++++ .../devicetree/bindings/net/mscc-miim.txt | 26 -------- drivers/net/mdio/mdio-mscc-miim.c | 52 +++++++++++++++- 3 files changed, 110 insertions(+), 28 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/mscc,miim.yaml delete mode 100644 Documentation/devicetree/bindings/net/mscc-miim.txt
> @@ -295,21 +323,41 @@ static int mscc_miim_probe(struct platform_device *pdev) > if (!miim->info) > return -EINVAL; > > - ret = of_mdiobus_register(bus, pdev->dev.of_node); > + miim->clk = devm_clk_get_optional(&pdev->dev, NULL); > + if (IS_ERR(miim->clk)) > + return PTR_ERR(miim->clk); > + > + ret = clk_prepare_enable(miim->clk); > + if (ret) > + return ret; > + > + of_property_read_u32(np, "clock-frequency", &miim->clk_freq); The clock is optional if there is no "clock-frequency" property. If the property does exist, the clock should be mandatory. I don't think it should silently fail setting the bus frequency because the clock is missing. Andrew
Am 2022-03-31 18:28, schrieb Andrew Lunn: >> @@ -295,21 +323,41 @@ static int mscc_miim_probe(struct >> platform_device *pdev) >> if (!miim->info) >> return -EINVAL; >> >> - ret = of_mdiobus_register(bus, pdev->dev.of_node); >> + miim->clk = devm_clk_get_optional(&pdev->dev, NULL); >> + if (IS_ERR(miim->clk)) >> + return PTR_ERR(miim->clk); >> + >> + ret = clk_prepare_enable(miim->clk); >> + if (ret) >> + return ret; >> + >> + of_property_read_u32(np, "clock-frequency", &miim->clk_freq); > > The clock is optional if there is no "clock-frequency" property. If > the property does exist, the clock should be mandatory. I don't think > it should silently fail setting the bus frequency because the clock is > missing. Oh yes, agreed. -michael
On 31/03/2022 17:14, Michael Walle wrote: > Convert the mscc-miim device tree binding to the new YAML format. > > The original binding don't mention if the interrupt property is optional > or not. But on the SparX-5 SoC, for example, the interrupt property isn't > used, thus in the new binding that property is optional. FWIW the driver > doesn't use interrupts at all. > > Signed-off-by: Michael Walle <michael@walle.cc> > --- > .../devicetree/bindings/net/mscc,miim.yaml | 55 +++++++++++++++++++ > .../devicetree/bindings/net/mscc-miim.txt | 26 --------- > 2 files changed, 55 insertions(+), 26 deletions(-) > create mode 100644 Documentation/devicetree/bindings/net/mscc,miim.yaml > delete mode 100644 Documentation/devicetree/bindings/net/mscc-miim.txt > > diff --git a/Documentation/devicetree/bindings/net/mscc,miim.yaml b/Documentation/devicetree/bindings/net/mscc,miim.yaml > new file mode 100644 > index 000000000000..b52bf1732755 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/mscc,miim.yaml > @@ -0,0 +1,55 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/mscc,miim.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Microsemi MII Management Controller (MIIM) > + > +maintainers: > + - Alexandre Belloni <alexandre.belloni@bootlin.com> > + > +allOf: > + - $ref: "mdio.yaml#" > + > +properties: > + compatible: > + enum: > + - mscc,ocelot-miim > + - microchip,lan966x-miim > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > + reg: > + items: > + - description: base address > + - description: associated reset register for internal PHYs > + minItems: 1 > + > + interrupts: true how many? maxItems > + > +required: > + - compatible > + - reg > + - "#address-cells" > + - "#size-cells" > + > +unevaluatedProperties: false > + > +examples: > + - | > + mdio@107009c { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "mscc,ocelot-miim"; > + reg = <0x107009c 0x36>, <0x10700f0 0x8>; Please put the compatible followed by reg at the beginning (first properties). Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/net/mscc,miim.yaml b/Documentation/devicetree/bindings/net/mscc,miim.yaml new file mode 100644 index 000000000000..b52bf1732755 --- /dev/null +++ b/Documentation/devicetree/bindings/net/mscc,miim.yaml @@ -0,0 +1,55 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/mscc,miim.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Microsemi MII Management Controller (MIIM) + +maintainers: + - Alexandre Belloni <alexandre.belloni@bootlin.com> + +allOf: + - $ref: "mdio.yaml#" + +properties: + compatible: + enum: + - mscc,ocelot-miim + - microchip,lan966x-miim + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + + reg: + items: + - description: base address + - description: associated reset register for internal PHYs + minItems: 1 + + interrupts: true + +required: + - compatible + - reg + - "#address-cells" + - "#size-cells" + +unevaluatedProperties: false + +examples: + - | + mdio@107009c { + #address-cells = <1>; + #size-cells = <0>; + compatible = "mscc,ocelot-miim"; + reg = <0x107009c 0x36>, <0x10700f0 0x8>; + interrupts = <14>; + + phy0: ethernet-phy@0 { + reg = <0>; + }; + }; diff --git a/Documentation/devicetree/bindings/net/mscc-miim.txt b/Documentation/devicetree/bindings/net/mscc-miim.txt deleted file mode 100644 index 70e0cb1ee485..000000000000 --- a/Documentation/devicetree/bindings/net/mscc-miim.txt +++ /dev/null @@ -1,26 +0,0 @@ -Microsemi MII Management Controller (MIIM) / MDIO -================================================= - -Properties: -- compatible: must be "mscc,ocelot-miim" or "microchip,lan966x-miim" -- reg: The base address of the MDIO bus controller register bank. Optionally, a - second register bank can be defined if there is an associated reset register - for internal PHYs -- #address-cells: Must be <1>. -- #size-cells: Must be <0>. MDIO addresses have no size component. -- interrupts: interrupt specifier (refer to the interrupt binding) - -Typically an MDIO bus might have several children. - -Example: - mdio@107009c { - #address-cells = <1>; - #size-cells = <0>; - compatible = "mscc,ocelot-miim"; - reg = <0x107009c 0x36>, <0x10700f0 0x8>; - interrupts = <14>; - - phy0: ethernet-phy@0 { - reg = <0>; - }; - };
Convert the mscc-miim device tree binding to the new YAML format. The original binding don't mention if the interrupt property is optional or not. But on the SparX-5 SoC, for example, the interrupt property isn't used, thus in the new binding that property is optional. FWIW the driver doesn't use interrupts at all. Signed-off-by: Michael Walle <michael@walle.cc> --- .../devicetree/bindings/net/mscc,miim.yaml | 55 +++++++++++++++++++ .../devicetree/bindings/net/mscc-miim.txt | 26 --------- 2 files changed, 55 insertions(+), 26 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/mscc,miim.yaml delete mode 100644 Documentation/devicetree/bindings/net/mscc-miim.txt