| Message ID | 20211013073807.2282230-2-horatiu.vultur@microchip.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v3,1/2] dt-bindings: reset: Add lan966x support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | success | |
| robh/dtbs-check | success |
On Wed, 2021-10-13 at 09:38 +0200, Horatiu Vultur wrote: > This adds support for lan966x. > > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com> > --- > .../devicetree/bindings/reset/microchip,rst.yaml | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/reset/microchip,rst.yaml b/Documentation/devicetree/bindings/reset/microchip,rst.yaml > index 370579aeeca1..fb170ed2c57a 100644 > --- a/Documentation/devicetree/bindings/reset/microchip,rst.yaml > +++ b/Documentation/devicetree/bindings/reset/microchip,rst.yaml > @@ -20,7 +20,11 @@ properties: > pattern: "^reset-controller@[0-9a-f]+$" > > compatible: > - const: microchip,sparx5-switch-reset > + oneOf: > + - items: > + - const: microchip,sparx5-switch-reset > + - items: > + - const: microchip,lan966x-switch-reset > > reg: > items: > @@ -37,6 +41,14 @@ properties: > $ref: "/schemas/types.yaml#/definitions/phandle" > description: syscon used to access CPU reset > > + cuphy-syscon: > + $ref: "/schemas/types.yaml#/definitions/phandle" > + description: syscon used to access CuPHY Can this be used on sparx5? Is it optional on lan966x? > + phy-reset-gpios: > + description: used for release of reset of the external PHY > + maxItems: 1 > + > required: > - compatible > - reg I'd like somebody to reassure me that putting the CuPHY reset and external PHY GPIO reset in the reset controller is the right thing to do. It looks fine to me, but I'm not sure if these should rather be in separate phy nodes that are referenced from the switch. regards Philipp
Hi Philipp, The 10/14/2021 13:55, Philipp Zabel wrote: > > On Wed, 2021-10-13 at 09:38 +0200, Horatiu Vultur wrote: > > This adds support for lan966x. > > > > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com> > > --- > > .../devicetree/bindings/reset/microchip,rst.yaml | 14 +++++++++++++- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/reset/microchip,rst.yaml b/Documentation/devicetree/bindings/reset/microchip,rst.yaml > > index 370579aeeca1..fb170ed2c57a 100644 > > --- a/Documentation/devicetree/bindings/reset/microchip,rst.yaml > > +++ b/Documentation/devicetree/bindings/reset/microchip,rst.yaml > > @@ -20,7 +20,11 @@ properties: > > pattern: "^reset-controller@[0-9a-f]+$" > > > > compatible: > > - const: microchip,sparx5-switch-reset > > + oneOf: > > + - items: > > + - const: microchip,sparx5-switch-reset > > + - items: > > + - const: microchip,lan966x-switch-reset > > > > reg: > > items: > > @@ -37,6 +41,14 @@ properties: > > $ref: "/schemas/types.yaml#/definitions/phandle" > > description: syscon used to access CPU reset > > > > + cuphy-syscon: > > + $ref: "/schemas/types.yaml#/definitions/phandle" > > + description: syscon used to access CuPHY > > Can this be used on sparx5? No, because the sparx5 doesn't have any internal PHYs that need to be released of the reset. > Is it optional on lan966x? No, it is required on lan966x. I will update the binding to show this. > > > + phy-reset-gpios: > > + description: used for release of reset of the external PHY > > + maxItems: 1 > > + > > required: > > - compatible > > - reg > > I'd like somebody to reassure me that putting the CuPHY reset and > external PHY GPIO reset in the reset controller is the right thing to > do. > > It looks fine to me, but I'm not sure if these should rather be in > separate phy nodes that are referenced from the switch. > > regards > Philipp
The 10/14/2021 17:20, Horatiu Vultur wrote: > Hi Philipp, > > The 10/14/2021 13:55, Philipp Zabel wrote: > > > > On Wed, 2021-10-13 at 09:38 +0200, Horatiu Vultur wrote: > > > This adds support for lan966x. > > > > > > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com> > > > --- > > > .../devicetree/bindings/reset/microchip,rst.yaml | 14 +++++++++++++- > > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > > > diff --git a/Documentation/devicetree/bindings/reset/microchip,rst.yaml b/Documentation/devicetree/bindings/reset/microchip,rst.yaml > > > index 370579aeeca1..fb170ed2c57a 100644 > > > --- a/Documentation/devicetree/bindings/reset/microchip,rst.yaml > > > +++ b/Documentation/devicetree/bindings/reset/microchip,rst.yaml > > > @@ -20,7 +20,11 @@ properties: > > > pattern: "^reset-controller@[0-9a-f]+$" > > > > > > compatible: > > > - const: microchip,sparx5-switch-reset > > > + oneOf: > > > + - items: > > > + - const: microchip,sparx5-switch-reset > > > + - items: > > > + - const: microchip,lan966x-switch-reset > > > > > > reg: > > > items: > > > @@ -37,6 +41,14 @@ properties: > > > $ref: "/schemas/types.yaml#/definitions/phandle" > > > description: syscon used to access CPU reset > > > > > > + cuphy-syscon: > > > + $ref: "/schemas/types.yaml#/definitions/phandle" > > > + description: syscon used to access CuPHY > > > > Can this be used on sparx5? > > No, because the sparx5 doesn't have any internal PHYs that need to > be released of the reset. > > > Is it optional on lan966x? > > No, it is required on lan966x. I will update the binding to show this. > > > > > > + phy-reset-gpios: > > > + description: used for release of reset of the external PHY > > > + maxItems: 1 > > > + > > > required: > > > - compatible > > > - reg > > > > I'd like somebody to reassure me that putting the CuPHY reset and > > external PHY GPIO reset in the reset controller is the right thing to > > do. > > > > It looks fine to me, but I'm not sure if these should rather be in > > separate phy nodes that are referenced from the switch. Were you thinking to have just another reset driver('phy-reset') and then the switch to refer to both of them? I like this idea because then is more clear what is doing each driver. > > > > regards > > Philipp > > -- > /Horatiu
On Fri, 2021-10-15 at 16:14 +0200, Horatiu Vultur wrote: [...] > Were you thinking to have just another reset driver('phy-reset') and then > the switch to refer to both of them? > I like this idea because then is more clear what is doing each driver. Yes, especially if there is no requirement to handle switch and PHY resets at the same time: this would allow the sgpio driver to trigger the required switch reset without already releasing the PHYs from reset. regards Philipp
diff --git a/Documentation/devicetree/bindings/reset/microchip,rst.yaml b/Documentation/devicetree/bindings/reset/microchip,rst.yaml index 370579aeeca1..fb170ed2c57a 100644 --- a/Documentation/devicetree/bindings/reset/microchip,rst.yaml +++ b/Documentation/devicetree/bindings/reset/microchip,rst.yaml @@ -20,7 +20,11 @@ properties: pattern: "^reset-controller@[0-9a-f]+$" compatible: - const: microchip,sparx5-switch-reset + oneOf: + - items: + - const: microchip,sparx5-switch-reset + - items: + - const: microchip,lan966x-switch-reset reg: items: @@ -37,6 +41,14 @@ properties: $ref: "/schemas/types.yaml#/definitions/phandle" description: syscon used to access CPU reset + cuphy-syscon: + $ref: "/schemas/types.yaml#/definitions/phandle" + description: syscon used to access CuPHY + + phy-reset-gpios: + description: used for release of reset of the external PHY + maxItems: 1 + required: - compatible - reg
This adds support for lan966x. Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com> --- .../devicetree/bindings/reset/microchip,rst.yaml | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)