[PATCH 0/2] reset: lan996x: Add reset driver for lan966x phys

[PATCH 0/2] reset: lan996x: Add reset driver for lan966x phys

[Horatiu Vultur]

The lan966x switch is reseted using the 'sparx5-switch-reset' but the
PHYs are still keep in reset. Therefore add this patch to release the
HW reset of the PHYs.

Horatiu Vultur (2):
  dt-bindings: reset: lan966x phy reset driver bindings
  reset: lan966x: Add support for the phy reset on lan966x

 .../bindings/reset/lan966x-phy,rst.yaml       | 53 +++++++++++
 drivers/reset/Kconfig                         |  8 ++
 drivers/reset/Makefile                        |  1 +
 drivers/reset/reset-microchip-lan966x-phy.c   | 93 +++++++++++++++++++
 4 files changed, 155 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/lan966x-phy,rst.yaml
 create mode 100644 drivers/reset/reset-microchip-lan966x-phy.c

-- 
2.33.0

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

[Horatiu Vultur]

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]+$"
+
+  compatible:
+    const: microchip,lan966x-phy-reset
+
+  reg:
+    items:
+      - description: internal phy reset registers
+
+  reg-names:
+    const: phy
+
+  "#reset-cells":
+    const: 1
+
+  external-phy-reset-gpios:
+    description: used for release of reset of the external PHY
+    maxItems: 1
+
+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

[PATCH 2/2] reset: lan966x: Add support for the phy reset on lan966x

[Horatiu Vultur]

This patch adds a driver to release the reset of the PHYs on lan966x.
There are 2 mdio buses on lan966x. One which is internal and has 2 PHYs
and one is external.
On the internal one it is required to release the HW reset of the phys
by writing specific register and while on the external one is required
to toggle a GPIO pin.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 drivers/reset/Kconfig                       |  8 ++
 drivers/reset/Makefile                      |  1 +
 drivers/reset/reset-microchip-lan966x-phy.c | 93 +++++++++++++++++++++
 3 files changed, 102 insertions(+)
 create mode 100644 drivers/reset/reset-microchip-lan966x-phy.c

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 36ce6c8bcf1e..94e1c0e7d343 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -122,6 +122,14 @@ config RESET_MCHP_SPARX5
 	help
 	  This driver supports switch core reset for the Microchip Sparx5 SoC.
 
+config RESET_MCHP_LAN966X_PHY
+	bool "Microchip lan966x phy reset driver"
+	depends on SOC_LAN966 || COMPILE_TEST
+	select MFD_SYSCON
+	help
+	  This driver supports the release of phy reset for the
+	  Microchip lan966x SoC.
+
 config RESET_MESON
 	tristate "Meson Reset Driver"
 	depends on ARCH_MESON || COMPILE_TEST
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 21d46d8869ff..2d7699aafb6c 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_RESET_K210) += reset-k210.o
 obj-$(CONFIG_RESET_LANTIQ) += reset-lantiq.o
 obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
 obj-$(CONFIG_RESET_MCHP_SPARX5) += reset-microchip-sparx5.o
+obj-$(CONFIG_RESET_MCHP_LAN966X_PHY) += reset-microchip-lan966x-phy.o
 obj-$(CONFIG_RESET_MESON) += reset-meson.o
 obj-$(CONFIG_RESET_MESON_AUDIO_ARB) += reset-meson-audio-arb.o
 obj-$(CONFIG_RESET_NPCM) += reset-npcm.o
diff --git a/drivers/reset/reset-microchip-lan966x-phy.c b/drivers/reset/reset-microchip-lan966x-phy.c
new file mode 100644
index 000000000000..63dd6a7bc62f
--- /dev/null
+++ b/drivers/reset/reset-microchip-lan966x-phy.c
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: GPL-2.0+
+#include <linux/gpio/consumer.h>
+#include <linux/of_device.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+
+#define CUPHY_REG_OFF	0x10
+#define CUPHY_REG_BIT	0
+
+struct lan966x_phy_reset_context {
+	void __iomem *internal_phy_ctrl;
+	struct gpio_desc *external_phy_ctrl;
+	struct reset_controller_dev rcdev;
+};
+
+static int lan966x_phy_reset(struct reset_controller_dev *rcdev,
+			     unsigned long id)
+{
+	struct lan966x_phy_reset_context *ctx =
+		container_of(rcdev, struct lan966x_phy_reset_context, rcdev);
+	u32 val;
+
+	/* In case there are external PHYs toggle the GPIO to release the reset
+	 * of the PHYs
+	 */
+	if (ctx->external_phy_ctrl) {
+		gpiod_direction_output(ctx->external_phy_ctrl, 1);
+		gpiod_set_value(ctx->external_phy_ctrl, 0);
+		gpiod_set_value(ctx->external_phy_ctrl, 1);
+		gpiod_set_value(ctx->external_phy_ctrl, 0);
+	}
+
+	/* Release the reset of internal PHY */
+	val = readl(ctx->internal_phy_ctrl + CUPHY_REG_OFF);
+	val |= BIT(CUPHY_REG_BIT);
+	writel(val, ctx->internal_phy_ctrl + CUPHY_REG_OFF);
+
+	return 0;
+}
+
+static const struct reset_control_ops lan966x_phy_reset_ops = {
+	.reset = lan966x_phy_reset,
+};
+
+static int lan966x_phy_reset_probe(struct platform_device *pdev)
+{
+	struct device_node *dn = pdev->dev.of_node;
+	struct lan966x_phy_reset_context *ctx;
+
+	ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ctx->internal_phy_ctrl = devm_platform_ioremap_resource_byname(pdev, "phy");
+	if (IS_ERR(ctx->internal_phy_ctrl))
+		return dev_err_probe(&pdev->dev, PTR_ERR(ctx->internal_phy_ctrl),
+				     "Could not get resource 0\n");
+
+	ctx->external_phy_ctrl = devm_gpiod_get_optional(&pdev->dev,
+							 "external-phy-reset",
+							 GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->external_phy_ctrl))
+		return dev_err_probe(&pdev->dev, PTR_ERR(ctx->external_phy_ctrl),
+				     "Could not get reset GPIO\n");
+
+	ctx->rcdev.owner = THIS_MODULE;
+	ctx->rcdev.nr_resets = 1;
+	ctx->rcdev.ops = &lan966x_phy_reset_ops;
+	ctx->rcdev.of_node = dn;
+
+	return devm_reset_controller_register(&pdev->dev, &ctx->rcdev);
+}
+
+static const struct of_device_id lan966x_phy_reset_of_match[] = {
+	{ .compatible = "microchip,lan966x-phy-reset", },
+	{ }
+};
+
+static struct platform_driver lan966x_phy_reset_driver = {
+	.probe = lan966x_phy_reset_probe,
+	.driver = {
+		.name = "lan966x-phy-reset",
+		.of_match_table = lan966x_phy_reset_of_match,
+	},
+};
+
+static int __init lan966x_phy_reset_init(void)
+{
+	return platform_driver_register(&lan966x_phy_reset_driver);
+}
+postcore_initcall(lan966x_phy_reset_init);
-- 
2.33.0

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

[Rob Herring]

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
> 
> 

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

[Horatiu Vultur]

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
> >
> >

-- 
/Horatiu

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

[Michael Walle]

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/