linux-kernel - Re: [PATCH 1/2] dt-bindings: reset: lan966x phy reset driver bindings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20211029125848.k4vwmzxpljbwlve6@soft-dev3-1.localhost>
Date:   Fri, 29 Oct 2021 14:58:48 +0200
From:   Horatiu Vultur <horatiu.vultur@...rochip.com>
To:     Rob Herring <robh@...nel.org>
CC:     <p.zabel@...gutronix.de>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] dt-bindings: reset: lan966x phy reset driver bindings

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@...rochip.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@...rochip.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@...10010 {
> > +        compatible = "microchip,lan966x-phy-reset";
> > +        reg = <0xe2010010 0x14>;
> > +        reg-names = "phy";
> > +        #reset-cells = <1>;
> > +    };
> > --
> > 2.33.0
> >
> >

-- 
/Horatiu