[1/2] dt-bindings: reset: lan966x phy reset driver bindings

Document the lan966x phy reset device driving bindings.
It is using register access for the internal PHYs and toggles
GPIO for external PHYs.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 .../bindings/reset/lan966x-phy,rst.yaml       | 53 +++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/lan966x-phy,rst.yaml

On Tue, Oct 19, 2021 at 01:52:04PM +0200, Horatiu Vultur wrote:
> Document the lan966x phy reset device driving bindings.
> It is using register access for the internal PHYs and toggles
> GPIO for external PHYs.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  .../bindings/reset/lan966x-phy,rst.yaml       | 53 +++++++++++++++++++
>  1 file changed, 53 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/lan966x-phy,rst.yaml
> 
> diff --git a/Documentation/devicetree/bindings/reset/lan966x-phy,rst.yaml b/Documentation/devicetree/bindings/reset/lan966x-phy,rst.yaml
> new file mode 100644
> index 000000000000..35a32458cafe
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/lan966x-phy,rst.yaml
> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/reset/lan966x-phy,rst.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Microchip Lan966x PHY Reset
> +
> +maintainers:
> +  - Horatiu Vultur <horatiu.vultur@microchip.com>
> +
> +description: |
> +  The Microchip Lan966x Switch provides 2 internal PHY which needs to be
> +  released from reset before they can be accessed. Also it might have external
> +  PHYs which requires to toggle a GPIO before the access to the PHYs.
> +
> +properties:
> +  $nodename:
> +    pattern: "^phy-reset@[0-9a-f]+$"

^reset-controller@[0-9a-f]+$

> +
> +  compatible:
> +    const: microchip,lan966x-phy-reset
> +
> +  reg:
> +    items:
> +      - description: internal phy reset registers

Just: maxItems: 1

> +
> +  reg-names:
> +    const: phy

Not all that useful with only 1 entry.

> +
> +  "#reset-cells":
> +    const: 1
> +
> +  external-phy-reset-gpios:
> +    description: used for release of reset of the external PHY
> +    maxItems: 1

This belongs in the external PHY's node.

> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - "#reset-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    phy_reset: phy-reset@e2010010 {
> +        compatible = "microchip,lan966x-phy-reset";
> +        reg = <0xe2010010 0x14>;
> +        reg-names = "phy";
> +        #reset-cells = <1>;
> +    };
> -- 
> 2.33.0
> 
>

The 10/26/2021 22:13, Rob Herring wrote:

Hi Rob,

> 
> On Tue, Oct 19, 2021 at 01:52:04PM +0200, Horatiu Vultur wrote:
> > Document the lan966x phy reset device driving bindings.
> > It is using register access for the internal PHYs and toggles
> > GPIO for external PHYs.
> >
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > ---
> >  .../bindings/reset/lan966x-phy,rst.yaml       | 53 +++++++++++++++++++
> >  1 file changed, 53 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/reset/lan966x-phy,rst.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/reset/lan966x-phy,rst.yaml b/Documentation/devicetree/bindings/reset/lan966x-phy,rst.yaml
> > new file mode 100644
> > index 000000000000..35a32458cafe
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/reset/lan966x-phy,rst.yaml
> > @@ -0,0 +1,53 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: "http://devicetree.org/schemas/reset/lan966x-phy,rst.yaml#"
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > +
> > +title: Microchip Lan966x PHY Reset
> > +
> > +maintainers:
> > +  - Horatiu Vultur <horatiu.vultur@microchip.com>
> > +
> > +description: |
> > +  The Microchip Lan966x Switch provides 2 internal PHY which needs to be
> > +  released from reset before they can be accessed. Also it might have external
> > +  PHYs which requires to toggle a GPIO before the access to the PHYs.
> > +
> > +properties:
> > +  $nodename:
> > +    pattern: "^phy-reset@[0-9a-f]+$"
> 
> ^reset-controller@[0-9a-f]+$

I will update this

> 
> > +
> > +  compatible:
> > +    const: microchip,lan966x-phy-reset
> > +
> > +  reg:
> > +    items:
> > +      - description: internal phy reset registers
> 
> Just: maxItems: 1

Same here

> 
> > +
> > +  reg-names:
> > +    const: phy
> 
> Not all that useful with only 1 entry.

And here

> 
> > +
> > +  "#reset-cells":
> > +    const: 1
> > +
> > +  external-phy-reset-gpios:
> > +    description: used for release of reset of the external PHY
> > +    maxItems: 1
> 
> This belongs in the external PHY's node.

My problem is if I put this in the external PHY's node, once the switch
gets reset it would reset also the GPIO pin and then it can't connect
to the external PHYs anymore.

The switch will need anyway to call this driver to release the reset of
the internal PHYs, so I was thinking to put also the release of the
external PHYs in the same driver.

Initially we wanted to extend the 'sparx5-switch-reset' driver to do
this but the output of that discussion was to have 2 different drivers,
one for the switch and one for the PHYs.

> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - reg-names
> > +  - "#reset-cells"
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    phy_reset: phy-reset@e2010010 {
> > +        compatible = "microchip,lan966x-phy-reset";
> > +        reg = <0xe2010010 0x14>;
> > +        reg-names = "phy";
> > +        #reset-cells = <1>;
> > +    };
> > --
> > 2.33.0
> >
> >

Hi Horatiu,

> > On Tue, Oct 19, 2021 at 01:52:04PM +0200, Horatiu Vultur wrote:
> > > Document the lan966x phy reset device driving bindings.
> > > It is using register access for the internal PHYs and toggles
> > > GPIO for external PHYs.
> > >
> > > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > > ---
> > >  .../bindings/reset/lan966x-phy,rst.yaml       | 53 +++++++++++++++++++
> > >  1 file changed, 53 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/reset/lan966x-phy,rst.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/reset/lan966x-phy,rst.yaml b/Documentation/devicetree/bindings/reset/lan966x-phy,rst.yaml
> > > new file mode 100644
> > > index 000000000000..35a32458cafe
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/reset/lan966x-phy,rst.yaml
> > > @@ -0,0 +1,53 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: "http://devicetree.org/schemas/reset/lan966x-phy,rst.yaml#"
> > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > > +
> > > +title: Microchip Lan966x PHY Reset
> > > +
> > > +maintainers:
> > > +  - Horatiu Vultur <horatiu.vultur@microchip.com>
> > > +
> > > +description: |
> > > +  The Microchip Lan966x Switch provides 2 internal PHY which needs to be
> > > +  released from reset before they can be accessed. Also it might have external
> > > +  PHYs which requires to toggle a GPIO before the access to the PHYs.

I don't think this driver is needed at all. See more below.

> > > +  external-phy-reset-gpios:
> > > +    description: used for release of reset of the external PHY
> > > +    maxItems: 1
> > 
> > This belongs in the external PHY's node.
> 
> My problem is if I put this in the external PHY's node, once the switch
> gets reset it would reset also the GPIO pin and then it can't connect
> to the external PHYs anymore.

But this will only solve your use case. If there is anything else
connected on the GPIO it will be reset, too. So you'd loose GPIO state
and you just 'fix' the exernal PHY reset lines here.

Fortunately, this is already solved by the shared reset lines. See [1]
for my proposal. Once the GPIO controller isn't reset anymore, we can
describe the reset line to the external PHYs by using the "reset-gpios"
property of the MDIO controller.

> The switch will need anyway to call this driver to release the reset of
> the internal PHYs, so I was thinking to put also the release of the
> external PHYs in the same driver.

There is already support for this in the MDIO driver, see [2]. This
is already used on the Ocelot series.

> Initially we wanted to extend the 'sparx5-switch-reset' driver to do
> this but the output of that discussion was to have 2 different drivers,
> one for the switch and one for the PHYs.

-michael

[1] https://lore.kernel.org/linux-devicetree/20220313154640.63813-1-michael@walle.cc/
[2] https://lore.kernel.org/netdev/20220313002153.11280-1-michael@walle.cc/