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/[email protected]/
[2] https://lore.kernel.org/linux-gpio/CACRpkdbxmN+SWt95aGHjA2ZGnN61aWaA7c5S4PaG+WePAj=htg@mail.gmail.com/
[3] https://lore.kernel.org/linux-gpio/[email protected]/
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
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 <[email protected]>
---
.../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
Hello:
This series was applied to netdev/net-next.git (master)
by David S. Miller <[email protected]>:
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
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
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 <[email protected]>
>
> 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/[email protected]/
> [2] https://lore.kernel.org/linux-gpio/CACRpkdbxmN+SWt95aGHjA2ZGnN61aWaA7c5S4PaG+WePAj=htg@mail.gmail.com/
> [3] https://lore.kernel.org/linux-gpio/[email protected]/
>
> 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
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 <[email protected]>
---
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
On Thu, Apr 28, 2022 at 6:40 AM Michael Walle <[email protected]> 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
Am 2022-05-03 15:04, schrieb Rob Herring:
> On Thu, Apr 28, 2022 at 6:40 AM Michael Walle <[email protected]> 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/[email protected]/
-michael