[PATCH net-next 0/2] net: lan966x: remove PHY reset support

[PATCH net-next 0/2] net: lan966x: remove PHY reset support

[Michael Walle]

Remove the unneeded PHY reset node as well as the driver support for it.

This was already discussed [1] and I expect Microchip to Ack on this
removal. Since there is no user, no breakage is expected.

I'm not sure it this should go through net or net-next and if the patches
should have a Fixes: tag or not. In upstream linux there was never any user
of it, so there is no bug to be fixed. But OTOH if the schema fix isn't
backported, then there might be an older schema version still containing
the reset node. Thoughts?

The patches needed for the GPIO part are just waiting to be picked up by
Linus [2,3]. This patch and the GPIO parts are the last pieces of the
puzzle to get ethernet working on the LAN9668 on upstream linux.

[1] https://lore.kernel.org/netdev/20220330110210.3374165-1-michael@walle.cc/
[2] https://lore.kernel.org/linux-gpio/CACRpkdbxmN+SWt95aGHjA2ZGnN61aWaA7c5S4PaG+WePAj=htg@mail.gmail.com/
[3] https://lore.kernel.org/linux-gpio/20220420191926.3411830-1-michael@walle.cc/

Michael Walle (2):
  dt-bindings: net: lan966x: remove PHY reset
  net: lan966x: remove PHY reset support

 .../devicetree/bindings/net/microchip,lan966x-switch.yaml | 2 --
 drivers/net/ethernet/microchip/lan966x/lan966x_main.c     | 8 +-------
 2 files changed, 1 insertion(+), 9 deletions(-)

-- 
2.30.2

[PATCH net-next 1/2] dt-bindings: net: lan966x: remove PHY reset

[Michael Walle]

The PHY reset was intended to be a phandle for a special PHY reset
driver for the integrated PHYs as well as any external PHYs. It turns
out, that the culprit is how the reset of the switch device is done.
In particular, the switch reset also affects other subsystems like
the GPIO and the SGPIO block and it happens to be the case that the
reset lines of the external PHYs are connected to a common GPIO line.
Thus as soon as the switch issues a reset during probe time, all the
external PHYs will go into reset because all the GPIO lines will
switch to input and the pull-down on that signal will take effect.

So even if there was a special PHY reset driver, it (1) won't fix
the root cause of the problem and (2) it won't fix all the other
consumers of GPIO lines which will also be reset.

It turns out, the Ocelot SoC has the same weird behavior (or the
lack of a dedicated switch reset) and there the problem is already
solved and all the bits and pieces are already there and this PHY
reset property isn't not needed at all.

There are no users of this binding. Just remove it.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 .../devicetree/bindings/net/microchip,lan966x-switch.yaml       | 2 --
 1 file changed, 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml b/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
index 131dc5a652de..f3ed708de0eb 100644
--- a/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
+++ b/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
@@ -53,12 +53,10 @@ properties:
   resets:
     items:
       - description: Reset controller used for switch core reset (soft reset)
-      - description: Reset controller used for releasing the phy from reset
 
   reset-names:
     items:
       - const: switch
-      - const: phy
 
   ethernet-ports:
     type: object
-- 
2.30.2

[PATCH net-next 2/2] net: lan966x: remove PHY reset support

[Michael Walle]

The PHY subsystem as well as the MIIM mdio driver (in case of the
integrated PHYs) will take care of the resets. A separate reset driver
isn't needed. There is no in-tree user of this feature. Remove the
support.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/net/ethernet/microchip/lan966x/lan966x_main.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
index 6579c7062aa5..138718f33dbd 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
@@ -937,7 +937,7 @@ static int lan966x_ram_init(struct lan966x *lan966x)
 
 static int lan966x_reset_switch(struct lan966x *lan966x)
 {
-	struct reset_control *switch_reset, *phy_reset;
+	struct reset_control *switch_reset;
 	int val = 0;
 	int ret;
 
@@ -946,13 +946,7 @@ static int lan966x_reset_switch(struct lan966x *lan966x)
 		return dev_err_probe(lan966x->dev, PTR_ERR(switch_reset),
 				     "Could not obtain switch reset");
 
-	phy_reset = devm_reset_control_get_shared(lan966x->dev, "phy");
-	if (IS_ERR(phy_reset))
-		return dev_err_probe(lan966x->dev, PTR_ERR(phy_reset),
-				     "Could not obtain phy reset\n");
-
 	reset_control_reset(switch_reset);
-	reset_control_reset(phy_reset);
 
 	lan_wr(SYS_RESET_CFG_CORE_ENA_SET(0), lan966x, SYS_RESET_CFG);
 	lan_wr(SYS_RAM_INIT_RAM_INIT_SET(1), lan966x, SYS_RAM_INIT);
-- 
2.30.2

Re: [PATCH net-next 0/2] net: lan966x: remove PHY reset support

[Horatiu Vultur]

The 04/28/2022 13:40, Michael Walle wrote:
> 
> Remove the unneeded PHY reset node as well as the driver support for it.
> 
> This was already discussed [1] and I expect Microchip to Ack on this
> removal. Since there is no user, no breakage is expected.

This looks good to me:
Acked-by: Horatiu Vultur <horatiu.vultur@microchip.com>

> 
> I'm not sure it this should go through net or net-next and if the patches
> should have a Fixes: tag or not. In upstream linux there was never any user
> of it, so there is no bug to be fixed. But OTOH if the schema fix isn't
> backported, then there might be an older schema version still containing
> the reset node. Thoughts?
> 
> The patches needed for the GPIO part are just waiting to be picked up by
> Linus [2,3]. This patch and the GPIO parts are the last pieces of the
> puzzle to get ethernet working on the LAN9668 on upstream linux.
> 
> [1] https://lore.kernel.org/netdev/20220330110210.3374165-1-michael@walle.cc/
> [2] https://lore.kernel.org/linux-gpio/CACRpkdbxmN+SWt95aGHjA2ZGnN61aWaA7c5S4PaG+WePAj=htg@mail.gmail.com/
> [3] https://lore.kernel.org/linux-gpio/20220420191926.3411830-1-michael@walle.cc/
> 
> Michael Walle (2):
>   dt-bindings: net: lan966x: remove PHY reset
>   net: lan966x: remove PHY reset support
> 
>  .../devicetree/bindings/net/microchip,lan966x-switch.yaml | 2 --
>  drivers/net/ethernet/microchip/lan966x/lan966x_main.c     | 8 +-------
>  2 files changed, 1 insertion(+), 9 deletions(-)
> 
> --
> 2.30.2
> 

-- 
/Horatiu

Re: [PATCH net-next 0/2] net: lan966x: remove PHY reset support

[Andrew Lunn]

On Thu, Apr 28, 2022 at 01:40:47PM +0200, Michael Walle wrote:
> Remove the unneeded PHY reset node as well as the driver support for it.
> 
> This was already discussed [1] and I expect Microchip to Ack on this
> removal. Since there is no user, no breakage is expected.
> 
> I'm not sure it this should go through net or net-next and if the patches
> should have a Fixes: tag or not. In upstream linux there was never any user
> of it, so there is no bug to be fixed. But OTOH if the schema fix isn't
> backported, then there might be an older schema version still containing
> the reset node. Thoughts?

Is the switch driver usable in the last LTS kernel? Somebody could
build a product around 5.15, and i assume they will have issues?

That could be an argument for backporting.

      Andrew

Re: [PATCH net-next 0/2] net: lan966x: remove PHY reset support

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Thu, 28 Apr 2022 13:40:47 +0200 you wrote:
> Remove the unneeded PHY reset node as well as the driver support for it.
> 
> This was already discussed [1] and I expect Microchip to Ack on this
> removal. Since there is no user, no breakage is expected.
> 
> I'm not sure it this should go through net or net-next and if the patches
> should have a Fixes: tag or not. In upstream linux there was never any user
> of it, so there is no bug to be fixed. But OTOH if the schema fix isn't
> backported, then there might be an older schema version still containing
> the reset node. Thoughts?
> 
> [...]

Here is the summary with links:
  - [net-next,1/2] dt-bindings: net: lan966x: remove PHY reset
    https://git.kernel.org/netdev/net-next/c/4fdabd509df3
  - [net-next,2/2] net: lan966x: remove PHY reset support
    https://git.kernel.org/netdev/net-next/c/5b06ef86826a

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net-next 1/2] dt-bindings: net: lan966x: remove PHY reset

[Rob Herring]

On Thu, Apr 28, 2022 at 6:40 AM Michael Walle <michael@walle.cc> wrote:
>
> The PHY reset was intended to be a phandle for a special PHY reset
> driver for the integrated PHYs as well as any external PHYs. It turns
> out, that the culprit is how the reset of the switch device is done.
> In particular, the switch reset also affects other subsystems like
> the GPIO and the SGPIO block and it happens to be the case that the
> reset lines of the external PHYs are connected to a common GPIO line.
> Thus as soon as the switch issues a reset during probe time, all the
> external PHYs will go into reset because all the GPIO lines will
> switch to input and the pull-down on that signal will take effect.
>
> So even if there was a special PHY reset driver, it (1) won't fix
> the root cause of the problem and (2) it won't fix all the other
> consumers of GPIO lines which will also be reset.
>
> It turns out, the Ocelot SoC has the same weird behavior (or the
> lack of a dedicated switch reset) and there the problem is already
> solved and all the bits and pieces are already there and this PHY
> reset property isn't not needed at all.
>
> There are no users of this binding. Just remove it.

Seems there was 1 user:

/builds/robherring/linux-dt/Documentation/devicetree/bindings/net/microchip,lan966x-switch.example.dtb:
switch@e0000000: resets: [[4294967295, 0], [4294967295, 0]] is too
long
 From schema: /builds/robherring/linux-dt/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
/builds/robherring/linux-dt/Documentation/devicetree/bindings/net/microchip,lan966x-switch.example.dtb:
switch@e0000000: reset-names: ['switch', 'phy'] is too long
 From schema: /builds/robherring/linux-dt/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml

Please fix as this is now failing in linux-next.

Rob

Re: [PATCH net-next 1/2] dt-bindings: net: lan966x: remove PHY reset

[Michael Walle]

Am 2022-05-03 15:04, schrieb Rob Herring:
> On Thu, Apr 28, 2022 at 6:40 AM Michael Walle <michael@walle.cc> wrote:
>> 
>> The PHY reset was intended to be a phandle for a special PHY reset
>> driver for the integrated PHYs as well as any external PHYs. It turns
>> out, that the culprit is how the reset of the switch device is done.
>> In particular, the switch reset also affects other subsystems like
>> the GPIO and the SGPIO block and it happens to be the case that the
>> reset lines of the external PHYs are connected to a common GPIO line.
>> Thus as soon as the switch issues a reset during probe time, all the
>> external PHYs will go into reset because all the GPIO lines will
>> switch to input and the pull-down on that signal will take effect.
>> 
>> So even if there was a special PHY reset driver, it (1) won't fix
>> the root cause of the problem and (2) it won't fix all the other
>> consumers of GPIO lines which will also be reset.
>> 
>> It turns out, the Ocelot SoC has the same weird behavior (or the
>> lack of a dedicated switch reset) and there the problem is already
>> solved and all the bits and pieces are already there and this PHY
>> reset property isn't not needed at all.
>> 
>> There are no users of this binding. Just remove it.
> 
> Seems there was 1 user:
> 
> /builds/robherring/linux-dt/Documentation/devicetree/bindings/net/microchip,lan966x-switch.example.dtb:
> switch@e0000000: resets: [[4294967295, 0], [4294967295, 0]] is too
> long
>  From schema:
> /builds/robherring/linux-dt/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
> /builds/robherring/linux-dt/Documentation/devicetree/bindings/net/microchip,lan966x-switch.example.dtb:
> switch@e0000000: reset-names: ['switch', 'phy'] is too long
>  From schema:
> /builds/robherring/linux-dt/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
> 
> Please fix as this is now failing in linux-next.

Sorry. Should be fixed with
https://lore.kernel.org/netdev/20220503132038.2714128-1-michael@walle.cc/

-michael