* Re: [PATCH 0/2] net: stmmac: allow sharing MDIO lines
@ 2023-08-08 14:09 ` Bartosz Golaszewski
0 siblings, 0 replies; 34+ messages in thread
From: Bartosz Golaszewski @ 2023-08-08 14:09 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexandre Torgue, Jose Abreu, Maxime Coquelin, Andrew Halaney,
Alex Elder, Srini Kandagatla, netdev, devicetree, linux-kernel,
linux-stm32, linux-arm-kernel, Bartosz Golaszewski
On Tue, Aug 8, 2023 at 3:26 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Tue, Aug 08, 2023 at 10:13:09AM +0200, Bartosz Golaszewski wrote:
> > Ok so upon some further investigation, the actual culprit is in stmmac
> > platform code - it always tries to register an MDIO bus - independent
> > of whether there is an actual mdio child node - unless the MAC is
> > marked explicitly as having a fixed-link.
> >
> > When I fixed that, MAC1's probe is correctly deferred until MAC0 has
> > created the MDIO bus.
> >
> > Even so, isn't it useful to actually reference the shared MDIO bus in some way?
> >
> > If the schematics look something like this:
> >
> > -------- -------
> > | MAC0 |--MDIO-----| PHY |
> > -------- | | -------
> > | |
> > -------- | | -------
> > | MAC1 |-- ----| PHY |
> > -------- -------
> >
> > Then it would make sense to model it on the device tree?
>
> So I think what you're saying is that MAC0 and MAC1's have MDIO bus
> masters, and the hardware designer decided to tie both together to
> a single set of clock and data lines, which then go to two PHYs.
The schematics I have are not very clear on that, but now that you
mention this, it's most likely the case.
>
> In that case, I would strongly advise only registering one MDIO bus,
> and avoid registering the second one - thereby preventing any issues
> caused by both MDIO bus masters trying to talk at the same time.
>
I sent a patch for that earlier today.
> The PHYs should be populated in firmware on just one of the buses.
>
> You will also need to ensure that whatever registers the bus does
> make sure that the clocks necessary for communicating on the bus
> are under control of the MDIO bus code and not the ethernet MAC
> code. We've run into problems in the past where this has not been
> the case, and it means - taking your example above - that when MAC1
> wants to talk to its PHY, if MAC0 isn't alive it can't.
Good point, but it's worse than that: when MAC0 is unbound, it will
unregister the MDIO bus and destroy all PHY devices. These are not
refcounted so they will literally go from under MAC1. Not sure how
this can be dealt with?
>
> So just be aware of the clocking situation and make sure that your
> MDIO bus code is managing the clocks necessary for the MDIO bus
> master to work.
Doesn't seem like stmmac is ready for it as it is now so this is going
to be fun...
Bartosz
>
> In regard to sharing of the MDIO bus signals between two bus
> masters, I do not believe that is permissible - there's no
> collision detection in hardware like there is on I涎. So
> having two MDIO bus masters talking at the same time would
> end up corrupting the MDC (clock) and MDIO (data) signals if
> both were active at the same time.
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/2] net: stmmac: allow sharing MDIO lines
2023-08-08 14:09 ` Bartosz Golaszewski
@ 2023-08-08 14:25 ` Andrew Lunn
-1 siblings, 0 replies; 34+ messages in thread
From: Andrew Lunn @ 2023-08-08 14:25 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Russell King (Oracle), David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Alexandre Torgue, Jose Abreu, Maxime Coquelin,
Andrew Halaney, Alex Elder, Srini Kandagatla, netdev, devicetree,
linux-kernel, linux-stm32, linux-arm-kernel, Bartosz Golaszewski
> > On Tue, Aug 08, 2023 at 10:13:09AM +0200, Bartosz Golaszewski wrote:
> > > Ok so upon some further investigation, the actual culprit is in stmmac
> > > platform code - it always tries to register an MDIO bus - independent
> > > of whether there is an actual mdio child node - unless the MAC is
> > > marked explicitly as having a fixed-link.
> > >
> > > When I fixed that, MAC1's probe is correctly deferred until MAC0 has
> > > created the MDIO bus.
> > >
> > > Even so, isn't it useful to actually reference the shared MDIO bus in some way?
> > >
> > > If the schematics look something like this:
> > >
> > > -------- -------
> > > | MAC0 |--MDIO-----| PHY |
> > > -------- | | -------
> > > | |
> > > -------- | | -------
> > > | MAC1 |-- ----| PHY |
> > > -------- -------
> > >
> > > Then it would make sense to model it on the device tree?
> >
> > So I think what you're saying is that MAC0 and MAC1's have MDIO bus
> > masters, and the hardware designer decided to tie both together to
> > a single set of clock and data lines, which then go to two PHYs.
>
> The schematics I have are not very clear on that, but now that you
> mention this, it's most likely the case.
I hope not. That would be very broken. As Russell pointed out, MDIO is
not multi-master. You need to check with the hardware designer if the
schematics are not clear.
> Good point, but it's worse than that: when MAC0 is unbound, it will
> unregister the MDIO bus and destroy all PHY devices. These are not
> refcounted so they will literally go from under MAC1. Not sure how
> this can be dealt with?
unbinding is not a normal operation. So i would just live with it, and
if root decides to shoot herself in the foot, that is her choice.
Andrew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/2] net: stmmac: allow sharing MDIO lines
@ 2023-08-08 14:25 ` Andrew Lunn
0 siblings, 0 replies; 34+ messages in thread
From: Andrew Lunn @ 2023-08-08 14:25 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Russell King (Oracle), David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Alexandre Torgue, Jose Abreu, Maxime Coquelin,
Andrew Halaney, Alex Elder, Srini Kandagatla, netdev, devicetree,
linux-kernel, linux-stm32, linux-arm-kernel, Bartosz Golaszewski
> > On Tue, Aug 08, 2023 at 10:13:09AM +0200, Bartosz Golaszewski wrote:
> > > Ok so upon some further investigation, the actual culprit is in stmmac
> > > platform code - it always tries to register an MDIO bus - independent
> > > of whether there is an actual mdio child node - unless the MAC is
> > > marked explicitly as having a fixed-link.
> > >
> > > When I fixed that, MAC1's probe is correctly deferred until MAC0 has
> > > created the MDIO bus.
> > >
> > > Even so, isn't it useful to actually reference the shared MDIO bus in some way?
> > >
> > > If the schematics look something like this:
> > >
> > > -------- -------
> > > | MAC0 |--MDIO-----| PHY |
> > > -------- | | -------
> > > | |
> > > -------- | | -------
> > > | MAC1 |-- ----| PHY |
> > > -------- -------
> > >
> > > Then it would make sense to model it on the device tree?
> >
> > So I think what you're saying is that MAC0 and MAC1's have MDIO bus
> > masters, and the hardware designer decided to tie both together to
> > a single set of clock and data lines, which then go to two PHYs.
>
> The schematics I have are not very clear on that, but now that you
> mention this, it's most likely the case.
I hope not. That would be very broken. As Russell pointed out, MDIO is
not multi-master. You need to check with the hardware designer if the
schematics are not clear.
> Good point, but it's worse than that: when MAC0 is unbound, it will
> unregister the MDIO bus and destroy all PHY devices. These are not
> refcounted so they will literally go from under MAC1. Not sure how
> this can be dealt with?
unbinding is not a normal operation. So i would just live with it, and
if root decides to shoot herself in the foot, that is her choice.
Andrew
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/2] net: stmmac: allow sharing MDIO lines
2023-08-08 14:25 ` Andrew Lunn
@ 2023-08-08 14:30 ` Bartosz Golaszewski
-1 siblings, 0 replies; 34+ messages in thread
From: Bartosz Golaszewski @ 2023-08-08 14:30 UTC (permalink / raw)
To: Andrew Lunn
Cc: Russell King (Oracle), David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Alexandre Torgue, Jose Abreu, Maxime Coquelin,
Andrew Halaney, Alex Elder, Srini Kandagatla, netdev, devicetree,
linux-kernel, linux-stm32, linux-arm-kernel, Bartosz Golaszewski
On Tue, Aug 8, 2023 at 4:25 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > On Tue, Aug 08, 2023 at 10:13:09AM +0200, Bartosz Golaszewski wrote:
> > > > Ok so upon some further investigation, the actual culprit is in stmmac
> > > > platform code - it always tries to register an MDIO bus - independent
> > > > of whether there is an actual mdio child node - unless the MAC is
> > > > marked explicitly as having a fixed-link.
> > > >
> > > > When I fixed that, MAC1's probe is correctly deferred until MAC0 has
> > > > created the MDIO bus.
> > > >
> > > > Even so, isn't it useful to actually reference the shared MDIO bus in some way?
> > > >
> > > > If the schematics look something like this:
> > > >
> > > > -------- -------
> > > > | MAC0 |--MDIO-----| PHY |
> > > > -------- | | -------
> > > > | |
> > > > -------- | | -------
> > > > | MAC1 |-- ----| PHY |
> > > > -------- -------
> > > >
> > > > Then it would make sense to model it on the device tree?
> > >
> > > So I think what you're saying is that MAC0 and MAC1's have MDIO bus
> > > masters, and the hardware designer decided to tie both together to
> > > a single set of clock and data lines, which then go to two PHYs.
> >
> > The schematics I have are not very clear on that, but now that you
> > mention this, it's most likely the case.
>
> I hope not. That would be very broken. As Russell pointed out, MDIO is
> not multi-master. You need to check with the hardware designer if the
> schematics are not clear.
Sorry, it was not very clear. It's the case that two MDIO masters
share the MDC and data lines.
>
> > Good point, but it's worse than that: when MAC0 is unbound, it will
> > unregister the MDIO bus and destroy all PHY devices. These are not
> > refcounted so they will literally go from under MAC1. Not sure how
> > this can be dealt with?
>
> unbinding is not a normal operation. So i would just live with it, and
> if root decides to shoot herself in the foot, that is her choice.
>
I disagree. Unbinding is very much a normal operation. Less so for
platform devices but still, it is there for a reason and should be
expected to work correctly. Or at the very least not crash and burn
the system.
On the other hand, I like your approach because I may get away without
having to fix it. But if I were to fix it - I would reference the MDIO
bus from the secondary mac by phandle and count its references before
dropping it. :)
Bartosz
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/2] net: stmmac: allow sharing MDIO lines
@ 2023-08-08 14:30 ` Bartosz Golaszewski
0 siblings, 0 replies; 34+ messages in thread
From: Bartosz Golaszewski @ 2023-08-08 14:30 UTC (permalink / raw)
To: Andrew Lunn
Cc: Russell King (Oracle), David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Alexandre Torgue, Jose Abreu, Maxime Coquelin,
Andrew Halaney, Alex Elder, Srini Kandagatla, netdev, devicetree,
linux-kernel, linux-stm32, linux-arm-kernel, Bartosz Golaszewski
On Tue, Aug 8, 2023 at 4:25 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > On Tue, Aug 08, 2023 at 10:13:09AM +0200, Bartosz Golaszewski wrote:
> > > > Ok so upon some further investigation, the actual culprit is in stmmac
> > > > platform code - it always tries to register an MDIO bus - independent
> > > > of whether there is an actual mdio child node - unless the MAC is
> > > > marked explicitly as having a fixed-link.
> > > >
> > > > When I fixed that, MAC1's probe is correctly deferred until MAC0 has
> > > > created the MDIO bus.
> > > >
> > > > Even so, isn't it useful to actually reference the shared MDIO bus in some way?
> > > >
> > > > If the schematics look something like this:
> > > >
> > > > -------- -------
> > > > | MAC0 |--MDIO-----| PHY |
> > > > -------- | | -------
> > > > | |
> > > > -------- | | -------
> > > > | MAC1 |-- ----| PHY |
> > > > -------- -------
> > > >
> > > > Then it would make sense to model it on the device tree?
> > >
> > > So I think what you're saying is that MAC0 and MAC1's have MDIO bus
> > > masters, and the hardware designer decided to tie both together to
> > > a single set of clock and data lines, which then go to two PHYs.
> >
> > The schematics I have are not very clear on that, but now that you
> > mention this, it's most likely the case.
>
> I hope not. That would be very broken. As Russell pointed out, MDIO is
> not multi-master. You need to check with the hardware designer if the
> schematics are not clear.
Sorry, it was not very clear. It's the case that two MDIO masters
share the MDC and data lines.
>
> > Good point, but it's worse than that: when MAC0 is unbound, it will
> > unregister the MDIO bus and destroy all PHY devices. These are not
> > refcounted so they will literally go from under MAC1. Not sure how
> > this can be dealt with?
>
> unbinding is not a normal operation. So i would just live with it, and
> if root decides to shoot herself in the foot, that is her choice.
>
I disagree. Unbinding is very much a normal operation. Less so for
platform devices but still, it is there for a reason and should be
expected to work correctly. Or at the very least not crash and burn
the system.
On the other hand, I like your approach because I may get away without
having to fix it. But if I were to fix it - I would reference the MDIO
bus from the secondary mac by phandle and count its references before
dropping it. :)
Bartosz
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/2] net: stmmac: allow sharing MDIO lines
2023-08-08 14:30 ` Bartosz Golaszewski
@ 2023-08-08 14:44 ` Andrew Halaney
-1 siblings, 0 replies; 34+ messages in thread
From: Andrew Halaney @ 2023-08-08 14:44 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Andrew Lunn, Russell King (Oracle), David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alexandre Torgue, Jose Abreu,
Maxime Coquelin, Alex Elder, Srini Kandagatla, netdev, devicetree,
linux-kernel, linux-stm32, linux-arm-kernel, Bartosz Golaszewski
On Tue, Aug 08, 2023 at 04:30:05PM +0200, Bartosz Golaszewski wrote:
> On Tue, Aug 8, 2023 at 4:25 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > > On Tue, Aug 08, 2023 at 10:13:09AM +0200, Bartosz Golaszewski wrote:
> > > > > Ok so upon some further investigation, the actual culprit is in stmmac
> > > > > platform code - it always tries to register an MDIO bus - independent
> > > > > of whether there is an actual mdio child node - unless the MAC is
> > > > > marked explicitly as having a fixed-link.
> > > > >
> > > > > When I fixed that, MAC1's probe is correctly deferred until MAC0 has
> > > > > created the MDIO bus.
> > > > >
> > > > > Even so, isn't it useful to actually reference the shared MDIO bus in some way?
> > > > >
> > > > > If the schematics look something like this:
> > > > >
> > > > > -------- -------
> > > > > | MAC0 |--MDIO-----| PHY |
> > > > > -------- | | -------
> > > > > | |
> > > > > -------- | | -------
> > > > > | MAC1 |-- ----| PHY |
> > > > > -------- -------
> > > > >
> > > > > Then it would make sense to model it on the device tree?
> > > >
> > > > So I think what you're saying is that MAC0 and MAC1's have MDIO bus
> > > > masters, and the hardware designer decided to tie both together to
> > > > a single set of clock and data lines, which then go to two PHYs.
> > >
> > > The schematics I have are not very clear on that, but now that you
> > > mention this, it's most likely the case.
> >
> > I hope not. That would be very broken. As Russell pointed out, MDIO is
> > not multi-master. You need to check with the hardware designer if the
> > schematics are not clear.
>
> Sorry, it was not very clear. It's the case that two MDIO masters
> share the MDC and data lines.
I'll make the water muddier (hopefully clearer?). I have access to the
board schematic (not SIP/SOM stuff though), but that should help here.
MAC0 owns its own MDIO bus (we'll call it MDIO0). It is pinmuxed to
gpio8/gpio9 for mdc/mdio. MAC1 owns its own bus (MDIO1) which is
pinmuxed to gpio21/22.
On MDIO0 there are two SGMII ethernet phys. One is connected to MAC0,
one is connected to MAC1.
MDIO1 is not connected to anything on the board. So there is only one
MDIO master, MAC0 on MDIO0, and it manages the ethernet phy for both
MAC0/MAC1.
Does that make sense? I don't think from a hardware design standpoint
this is violating anything, it isn't a multimaster setup on MDIO.
>
> >
> > > Good point, but it's worse than that: when MAC0 is unbound, it will
> > > unregister the MDIO bus and destroy all PHY devices. These are not
> > > refcounted so they will literally go from under MAC1. Not sure how
> > > this can be dealt with?
> >
> > unbinding is not a normal operation. So i would just live with it, and
> > if root decides to shoot herself in the foot, that is her choice.
> >
>
> I disagree. Unbinding is very much a normal operation. Less so for
> platform devices but still, it is there for a reason and should be
> expected to work correctly. Or at the very least not crash and burn
> the system.
>
> On the other hand, I like your approach because I may get away without
> having to fix it. But if I were to fix it - I would reference the MDIO
> bus from the secondary mac by phandle and count its references before
> dropping it. :)
>
> Bartosz
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/2] net: stmmac: allow sharing MDIO lines
@ 2023-08-08 14:44 ` Andrew Halaney
0 siblings, 0 replies; 34+ messages in thread
From: Andrew Halaney @ 2023-08-08 14:44 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Andrew Lunn, Russell King (Oracle), David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alexandre Torgue, Jose Abreu,
Maxime Coquelin, Alex Elder, Srini Kandagatla, netdev, devicetree,
linux-kernel, linux-stm32, linux-arm-kernel, Bartosz Golaszewski
On Tue, Aug 08, 2023 at 04:30:05PM +0200, Bartosz Golaszewski wrote:
> On Tue, Aug 8, 2023 at 4:25 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > > On Tue, Aug 08, 2023 at 10:13:09AM +0200, Bartosz Golaszewski wrote:
> > > > > Ok so upon some further investigation, the actual culprit is in stmmac
> > > > > platform code - it always tries to register an MDIO bus - independent
> > > > > of whether there is an actual mdio child node - unless the MAC is
> > > > > marked explicitly as having a fixed-link.
> > > > >
> > > > > When I fixed that, MAC1's probe is correctly deferred until MAC0 has
> > > > > created the MDIO bus.
> > > > >
> > > > > Even so, isn't it useful to actually reference the shared MDIO bus in some way?
> > > > >
> > > > > If the schematics look something like this:
> > > > >
> > > > > -------- -------
> > > > > | MAC0 |--MDIO-----| PHY |
> > > > > -------- | | -------
> > > > > | |
> > > > > -------- | | -------
> > > > > | MAC1 |-- ----| PHY |
> > > > > -------- -------
> > > > >
> > > > > Then it would make sense to model it on the device tree?
> > > >
> > > > So I think what you're saying is that MAC0 and MAC1's have MDIO bus
> > > > masters, and the hardware designer decided to tie both together to
> > > > a single set of clock and data lines, which then go to two PHYs.
> > >
> > > The schematics I have are not very clear on that, but now that you
> > > mention this, it's most likely the case.
> >
> > I hope not. That would be very broken. As Russell pointed out, MDIO is
> > not multi-master. You need to check with the hardware designer if the
> > schematics are not clear.
>
> Sorry, it was not very clear. It's the case that two MDIO masters
> share the MDC and data lines.
I'll make the water muddier (hopefully clearer?). I have access to the
board schematic (not SIP/SOM stuff though), but that should help here.
MAC0 owns its own MDIO bus (we'll call it MDIO0). It is pinmuxed to
gpio8/gpio9 for mdc/mdio. MAC1 owns its own bus (MDIO1) which is
pinmuxed to gpio21/22.
On MDIO0 there are two SGMII ethernet phys. One is connected to MAC0,
one is connected to MAC1.
MDIO1 is not connected to anything on the board. So there is only one
MDIO master, MAC0 on MDIO0, and it manages the ethernet phy for both
MAC0/MAC1.
Does that make sense? I don't think from a hardware design standpoint
this is violating anything, it isn't a multimaster setup on MDIO.
>
> >
> > > Good point, but it's worse than that: when MAC0 is unbound, it will
> > > unregister the MDIO bus and destroy all PHY devices. These are not
> > > refcounted so they will literally go from under MAC1. Not sure how
> > > this can be dealt with?
> >
> > unbinding is not a normal operation. So i would just live with it, and
> > if root decides to shoot herself in the foot, that is her choice.
> >
>
> I disagree. Unbinding is very much a normal operation. Less so for
> platform devices but still, it is there for a reason and should be
> expected to work correctly. Or at the very least not crash and burn
> the system.
>
> On the other hand, I like your approach because I may get away without
> having to fix it. But if I were to fix it - I would reference the MDIO
> bus from the secondary mac by phandle and count its references before
> dropping it. :)
>
> Bartosz
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/2] net: stmmac: allow sharing MDIO lines
2023-08-08 14:44 ` Andrew Halaney
@ 2023-08-08 15:10 ` Russell King (Oracle)
-1 siblings, 0 replies; 34+ messages in thread
From: Russell King (Oracle) @ 2023-08-08 15:10 UTC (permalink / raw)
To: Andrew Halaney
Cc: Bartosz Golaszewski, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Alexandre Torgue, Jose Abreu, Maxime Coquelin,
Alex Elder, Srini Kandagatla, netdev, devicetree, linux-kernel,
linux-stm32, linux-arm-kernel, Bartosz Golaszewski
On Tue, Aug 08, 2023 at 09:44:16AM -0500, Andrew Halaney wrote:
> On Tue, Aug 08, 2023 at 04:30:05PM +0200, Bartosz Golaszewski wrote:
> > On Tue, Aug 8, 2023 at 4:25 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > > > On Tue, Aug 08, 2023 at 10:13:09AM +0200, Bartosz Golaszewski wrote:
> > > > > > Ok so upon some further investigation, the actual culprit is in stmmac
> > > > > > platform code - it always tries to register an MDIO bus - independent
> > > > > > of whether there is an actual mdio child node - unless the MAC is
> > > > > > marked explicitly as having a fixed-link.
> > > > > >
> > > > > > When I fixed that, MAC1's probe is correctly deferred until MAC0 has
> > > > > > created the MDIO bus.
> > > > > >
> > > > > > Even so, isn't it useful to actually reference the shared MDIO bus in some way?
> > > > > >
> > > > > > If the schematics look something like this:
> > > > > >
> > > > > > -------- -------
> > > > > > | MAC0 |--MDIO-----| PHY |
> > > > > > -------- | | -------
> > > > > > | |
> > > > > > -------- | | -------
> > > > > > | MAC1 |-- ----| PHY |
> > > > > > -------- -------
> > > > > >
> > > > > > Then it would make sense to model it on the device tree?
> > > > >
> > > > > So I think what you're saying is that MAC0 and MAC1's have MDIO bus
> > > > > masters, and the hardware designer decided to tie both together to
> > > > > a single set of clock and data lines, which then go to two PHYs.
> > > >
> > > > The schematics I have are not very clear on that, but now that you
> > > > mention this, it's most likely the case.
> > >
> > > I hope not. That would be very broken. As Russell pointed out, MDIO is
> > > not multi-master. You need to check with the hardware designer if the
> > > schematics are not clear.
> >
> > Sorry, it was not very clear. It's the case that two MDIO masters
> > share the MDC and data lines.
>
> I'll make the water muddier (hopefully clearer?). I have access to the
> board schematic (not SIP/SOM stuff though), but that should help here.
>
> MAC0 owns its own MDIO bus (we'll call it MDIO0). It is pinmuxed to
> gpio8/gpio9 for mdc/mdio. MAC1 owns its own bus (MDIO1) which is
> pinmuxed to gpio21/22.
>
> On MDIO0 there are two SGMII ethernet phys. One is connected to MAC0,
> one is connected to MAC1.
>
> MDIO1 is not connected to anything on the board. So there is only one
> MDIO master, MAC0 on MDIO0, and it manages the ethernet phy for both
> MAC0/MAC1.
>
> Does that make sense? I don't think from a hardware design standpoint
> this is violating anything, it isn't a multimaster setup on MDIO.
That all sounds sane, thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/2] net: stmmac: allow sharing MDIO lines
@ 2023-08-08 15:10 ` Russell King (Oracle)
0 siblings, 0 replies; 34+ messages in thread
From: Russell King (Oracle) @ 2023-08-08 15:10 UTC (permalink / raw)
To: Andrew Halaney
Cc: Bartosz Golaszewski, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Alexandre Torgue, Jose Abreu, Maxime Coquelin,
Alex Elder, Srini Kandagatla, netdev, devicetree, linux-kernel,
linux-stm32, linux-arm-kernel, Bartosz Golaszewski
On Tue, Aug 08, 2023 at 09:44:16AM -0500, Andrew Halaney wrote:
> On Tue, Aug 08, 2023 at 04:30:05PM +0200, Bartosz Golaszewski wrote:
> > On Tue, Aug 8, 2023 at 4:25 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > > > On Tue, Aug 08, 2023 at 10:13:09AM +0200, Bartosz Golaszewski wrote:
> > > > > > Ok so upon some further investigation, the actual culprit is in stmmac
> > > > > > platform code - it always tries to register an MDIO bus - independent
> > > > > > of whether there is an actual mdio child node - unless the MAC is
> > > > > > marked explicitly as having a fixed-link.
> > > > > >
> > > > > > When I fixed that, MAC1's probe is correctly deferred until MAC0 has
> > > > > > created the MDIO bus.
> > > > > >
> > > > > > Even so, isn't it useful to actually reference the shared MDIO bus in some way?
> > > > > >
> > > > > > If the schematics look something like this:
> > > > > >
> > > > > > -------- -------
> > > > > > | MAC0 |--MDIO-----| PHY |
> > > > > > -------- | | -------
> > > > > > | |
> > > > > > -------- | | -------
> > > > > > | MAC1 |-- ----| PHY |
> > > > > > -------- -------
> > > > > >
> > > > > > Then it would make sense to model it on the device tree?
> > > > >
> > > > > So I think what you're saying is that MAC0 and MAC1's have MDIO bus
> > > > > masters, and the hardware designer decided to tie both together to
> > > > > a single set of clock and data lines, which then go to two PHYs.
> > > >
> > > > The schematics I have are not very clear on that, but now that you
> > > > mention this, it's most likely the case.
> > >
> > > I hope not. That would be very broken. As Russell pointed out, MDIO is
> > > not multi-master. You need to check with the hardware designer if the
> > > schematics are not clear.
> >
> > Sorry, it was not very clear. It's the case that two MDIO masters
> > share the MDC and data lines.
>
> I'll make the water muddier (hopefully clearer?). I have access to the
> board schematic (not SIP/SOM stuff though), but that should help here.
>
> MAC0 owns its own MDIO bus (we'll call it MDIO0). It is pinmuxed to
> gpio8/gpio9 for mdc/mdio. MAC1 owns its own bus (MDIO1) which is
> pinmuxed to gpio21/22.
>
> On MDIO0 there are two SGMII ethernet phys. One is connected to MAC0,
> one is connected to MAC1.
>
> MDIO1 is not connected to anything on the board. So there is only one
> MDIO master, MAC0 on MDIO0, and it manages the ethernet phy for both
> MAC0/MAC1.
>
> Does that make sense? I don't think from a hardware design standpoint
> this is violating anything, it isn't a multimaster setup on MDIO.
That all sounds sane, thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/2] net: stmmac: allow sharing MDIO lines
2023-08-08 14:44 ` Andrew Halaney
@ 2023-08-08 15:15 ` Andrew Lunn
-1 siblings, 0 replies; 34+ messages in thread
From: Andrew Lunn @ 2023-08-08 15:15 UTC (permalink / raw)
To: Andrew Halaney
Cc: Bartosz Golaszewski, Russell King (Oracle), David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alexandre Torgue, Jose Abreu,
Maxime Coquelin, Alex Elder, Srini Kandagatla, netdev, devicetree,
linux-kernel, linux-stm32, linux-arm-kernel, Bartosz Golaszewski
> I'll make the water muddier (hopefully clearer?). I have access to the
> board schematic (not SIP/SOM stuff though), but that should help here.
>
> MAC0 owns its own MDIO bus (we'll call it MDIO0). It is pinmuxed to
> gpio8/gpio9 for mdc/mdio. MAC1 owns its own bus (MDIO1) which is
> pinmuxed to gpio21/22.
>
> On MDIO0 there are two SGMII ethernet phys. One is connected to MAC0,
> one is connected to MAC1.
>
> MDIO1 is not connected to anything on the board. So there is only one
> MDIO master, MAC0 on MDIO0, and it manages the ethernet phy for both
> MAC0/MAC1.
>
> Does that make sense? I don't think from a hardware design standpoint
> this is violating anything, it isn't a multimaster setup on MDIO.
Thanks for taking a detailed look at the schematics. This is how i
would expect it to be.
> > > > Good point, but it's worse than that: when MAC0 is unbound, it will
> > > > unregister the MDIO bus and destroy all PHY devices. These are not
> > > > refcounted so they will literally go from under MAC1. Not sure how
> > > > this can be dealt with?
> > >
> > > unbinding is not a normal operation. So i would just live with it, and
> > > if root decides to shoot herself in the foot, that is her choice.
> > >
> >
> > I disagree. Unbinding is very much a normal operation.
What do you use it for?
I don't think i've ever manually done it. Maybe as part of a script to
unbind the FTDI driver from an FTDI device in order to use user space
tools to program the EEPROM? But that is about it.
I actually expect many unbind operations are broken because it is very
rarely used.
Andrew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 0/2] net: stmmac: allow sharing MDIO lines
@ 2023-08-08 15:15 ` Andrew Lunn
0 siblings, 0 replies; 34+ messages in thread
From: Andrew Lunn @ 2023-08-08 15:15 UTC (permalink / raw)
To: Andrew Halaney
Cc: Bartosz Golaszewski, Russell King (Oracle), David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alexandre Torgue, Jose Abreu,
Maxime Coquelin, Alex Elder, Srini Kandagatla, netdev, devicetree,
linux-kernel, linux-stm32, linux-arm-kernel, Bartosz Golaszewski
> I'll make the water muddier (hopefully clearer?). I have access to the
> board schematic (not SIP/SOM stuff though), but that should help here.
>
> MAC0 owns its own MDIO bus (we'll call it MDIO0). It is pinmuxed to
> gpio8/gpio9 for mdc/mdio. MAC1 owns its own bus (MDIO1) which is
> pinmuxed to gpio21/22.
>
> On MDIO0 there are two SGMII ethernet phys. One is connected to MAC0,
> one is connected to MAC1.
>
> MDIO1 is not connected to anything on the board. So there is only one
> MDIO master, MAC0 on MDIO0, and it manages the ethernet phy for both
> MAC0/MAC1.
>
> Does that make sense? I don't think from a hardware design standpoint
> this is violating anything, it isn't a multimaster setup on MDIO.
Thanks for taking a detailed look at the schematics. This is how i
would expect it to be.
> > > > Good point, but it's worse than that: when MAC0 is unbound, it will
> > > > unregister the MDIO bus and destroy all PHY devices. These are not
> > > > refcounted so they will literally go from under MAC1. Not sure how
> > > > this can be dealt with?
> > >
> > > unbinding is not a normal operation. So i would just live with it, and
> > > if root decides to shoot herself in the foot, that is her choice.
> > >
> >
> > I disagree. Unbinding is very much a normal operation.
What do you use it for?
I don't think i've ever manually done it. Maybe as part of a script to
unbind the FTDI driver from an FTDI device in order to use user space
tools to program the EEPROM? But that is about it.
I actually expect many unbind operations are broken because it is very
rarely used.
Andrew
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 0/2] net: stmmac: allow sharing MDIO lines
2023-08-08 15:15 ` Andrew Lunn
@ 2023-08-08 15:27 ` Russell King (Oracle)
-1 siblings, 0 replies; 34+ messages in thread
From: Russell King (Oracle) @ 2023-08-08 15:27 UTC (permalink / raw)
To: Andrew Lunn
Cc: Andrew Halaney, Bartosz Golaszewski, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alexandre Torgue, Jose Abreu,
Maxime Coquelin, Alex Elder, Srini Kandagatla, netdev, devicetree,
linux-kernel, linux-stm32, linux-arm-kernel, Bartosz Golaszewski
On Tue, Aug 08, 2023 at 05:15:45PM +0200, Andrew Lunn wrote:
> > > > > Good point, but it's worse than that: when MAC0 is unbound, it will
> > > > > unregister the MDIO bus and destroy all PHY devices. These are not
> > > > > refcounted so they will literally go from under MAC1. Not sure how
> > > > > this can be dealt with?
> > > >
> > > > unbinding is not a normal operation. So i would just live with it, and
> > > > if root decides to shoot herself in the foot, that is her choice.
> > > >
> > >
> > > I disagree. Unbinding is very much a normal operation.
>
> What do you use it for?
>
> I don't think i've ever manually done it. Maybe as part of a script to
> unbind the FTDI driver from an FTDI device in order to use user space
> tools to program the EEPROM? But that is about it.
>
> I actually expect many unbind operations are broken because it is very
> rarely used.
rmmod! Particularly useful during driver development, I tend to use it
extensively - and it has the advantage of testing those unbind paths!
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/2] net: stmmac: allow sharing MDIO lines
@ 2023-08-08 15:27 ` Russell King (Oracle)
0 siblings, 0 replies; 34+ messages in thread
From: Russell King (Oracle) @ 2023-08-08 15:27 UTC (permalink / raw)
To: Andrew Lunn
Cc: Andrew Halaney, Bartosz Golaszewski, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alexandre Torgue, Jose Abreu,
Maxime Coquelin, Alex Elder, Srini Kandagatla, netdev, devicetree,
linux-kernel, linux-stm32, linux-arm-kernel, Bartosz Golaszewski
On Tue, Aug 08, 2023 at 05:15:45PM +0200, Andrew Lunn wrote:
> > > > > Good point, but it's worse than that: when MAC0 is unbound, it will
> > > > > unregister the MDIO bus and destroy all PHY devices. These are not
> > > > > refcounted so they will literally go from under MAC1. Not sure how
> > > > > this can be dealt with?
> > > >
> > > > unbinding is not a normal operation. So i would just live with it, and
> > > > if root decides to shoot herself in the foot, that is her choice.
> > > >
> > >
> > > I disagree. Unbinding is very much a normal operation.
>
> What do you use it for?
>
> I don't think i've ever manually done it. Maybe as part of a script to
> unbind the FTDI driver from an FTDI device in order to use user space
> tools to program the EEPROM? But that is about it.
>
> I actually expect many unbind operations are broken because it is very
> rarely used.
rmmod! Particularly useful during driver development, I tend to use it
extensively - and it has the advantage of testing those unbind paths!
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/2] net: stmmac: allow sharing MDIO lines
2023-08-08 15:15 ` Andrew Lunn
@ 2023-08-08 18:26 ` Bartosz Golaszewski
-1 siblings, 0 replies; 34+ messages in thread
From: Bartosz Golaszewski @ 2023-08-08 18:26 UTC (permalink / raw)
To: Andrew Lunn
Cc: Andrew Halaney, Russell King (Oracle), David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alexandre Torgue, Jose Abreu,
Maxime Coquelin, Alex Elder, Srini Kandagatla, netdev, devicetree,
linux-kernel, linux-stm32, linux-arm-kernel, Bartosz Golaszewski
On Tue, Aug 8, 2023 at 5:15 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > I'll make the water muddier (hopefully clearer?). I have access to the
> > board schematic (not SIP/SOM stuff though), but that should help here.
> >
> > MAC0 owns its own MDIO bus (we'll call it MDIO0). It is pinmuxed to
> > gpio8/gpio9 for mdc/mdio. MAC1 owns its own bus (MDIO1) which is
> > pinmuxed to gpio21/22.
> >
> > On MDIO0 there are two SGMII ethernet phys. One is connected to MAC0,
> > one is connected to MAC1.
> >
> > MDIO1 is not connected to anything on the board. So there is only one
> > MDIO master, MAC0 on MDIO0, and it manages the ethernet phy for both
> > MAC0/MAC1.
> >
> > Does that make sense? I don't think from a hardware design standpoint
> > this is violating anything, it isn't a multimaster setup on MDIO.
>
> Thanks for taking a detailed look at the schematics. This is how i
> would expect it to be.
>
> > > > > Good point, but it's worse than that: when MAC0 is unbound, it will
> > > > > unregister the MDIO bus and destroy all PHY devices. These are not
> > > > > refcounted so they will literally go from under MAC1. Not sure how
> > > > > this can be dealt with?
> > > >
> > > > unbinding is not a normal operation. So i would just live with it, and
> > > > if root decides to shoot herself in the foot, that is her choice.
> > > >
> > >
> > > I disagree. Unbinding is very much a normal operation.
>
> What do you use it for?
>
> I don't think i've ever manually done it. Maybe as part of a script to
> unbind the FTDI driver from an FTDI device in order to use user space
> tools to program the EEPROM? But that is about it.
>
> I actually expect many unbind operations are broken because it is very
> rarely used.
>
When I say "device unbind", I don't just mean manual unbinding using
sysfs. I mean any code path (rmmod, unplugging the USB, etc.) that
leads to the device being detached from its driver. This is a
perfectly normal situation and should work correctly.
I won't be fixing it for this series but may end up looking into
establishing some kind of device links between MACs and their "remote"
PHYs that would allow to safely unbind them.
Bart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/2] net: stmmac: allow sharing MDIO lines
@ 2023-08-08 18:26 ` Bartosz Golaszewski
0 siblings, 0 replies; 34+ messages in thread
From: Bartosz Golaszewski @ 2023-08-08 18:26 UTC (permalink / raw)
To: Andrew Lunn
Cc: Andrew Halaney, Russell King (Oracle), David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alexandre Torgue, Jose Abreu,
Maxime Coquelin, Alex Elder, Srini Kandagatla, netdev, devicetree,
linux-kernel, linux-stm32, linux-arm-kernel, Bartosz Golaszewski
On Tue, Aug 8, 2023 at 5:15 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > I'll make the water muddier (hopefully clearer?). I have access to the
> > board schematic (not SIP/SOM stuff though), but that should help here.
> >
> > MAC0 owns its own MDIO bus (we'll call it MDIO0). It is pinmuxed to
> > gpio8/gpio9 for mdc/mdio. MAC1 owns its own bus (MDIO1) which is
> > pinmuxed to gpio21/22.
> >
> > On MDIO0 there are two SGMII ethernet phys. One is connected to MAC0,
> > one is connected to MAC1.
> >
> > MDIO1 is not connected to anything on the board. So there is only one
> > MDIO master, MAC0 on MDIO0, and it manages the ethernet phy for both
> > MAC0/MAC1.
> >
> > Does that make sense? I don't think from a hardware design standpoint
> > this is violating anything, it isn't a multimaster setup on MDIO.
>
> Thanks for taking a detailed look at the schematics. This is how i
> would expect it to be.
>
> > > > > Good point, but it's worse than that: when MAC0 is unbound, it will
> > > > > unregister the MDIO bus and destroy all PHY devices. These are not
> > > > > refcounted so they will literally go from under MAC1. Not sure how
> > > > > this can be dealt with?
> > > >
> > > > unbinding is not a normal operation. So i would just live with it, and
> > > > if root decides to shoot herself in the foot, that is her choice.
> > > >
> > >
> > > I disagree. Unbinding is very much a normal operation.
>
> What do you use it for?
>
> I don't think i've ever manually done it. Maybe as part of a script to
> unbind the FTDI driver from an FTDI device in order to use user space
> tools to program the EEPROM? But that is about it.
>
> I actually expect many unbind operations are broken because it is very
> rarely used.
>
When I say "device unbind", I don't just mean manual unbinding using
sysfs. I mean any code path (rmmod, unplugging the USB, etc.) that
leads to the device being detached from its driver. This is a
perfectly normal situation and should work correctly.
I won't be fixing it for this series but may end up looking into
establishing some kind of device links between MACs and their "remote"
PHYs that would allow to safely unbind them.
Bart
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/2] net: stmmac: allow sharing MDIO lines
2023-08-08 18:26 ` Bartosz Golaszewski
@ 2023-08-08 18:38 ` Russell King (Oracle)
-1 siblings, 0 replies; 34+ messages in thread
From: Russell King (Oracle) @ 2023-08-08 18:38 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Andrew Lunn, Andrew Halaney, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Alexandre Torgue, Jose Abreu, Maxime Coquelin,
Alex Elder, Srini Kandagatla, netdev, devicetree, linux-kernel,
linux-stm32, linux-arm-kernel, Bartosz Golaszewski
On Tue, Aug 08, 2023 at 08:26:22PM +0200, Bartosz Golaszewski wrote:
> When I say "device unbind", I don't just mean manual unbinding using
> sysfs. I mean any code path (rmmod, unplugging the USB, etc.) that
> leads to the device being detached from its driver. This is a
> perfectly normal situation and should work correctly.
>
> I won't be fixing it for this series but may end up looking into
> establishing some kind of device links between MACs and their "remote"
> PHYs that would allow to safely unbind them.
I don't think you're the first to suggest that!
That gets difficult - because although the PHY may be a different
driver, the MDIO bus may be provided by the _same_ hardware as the
ethernet MAC itself. So you end up with a circular dependency - the
PHY device depends on the MDIO bus device (which is the ethernet MAC)
and then you make the ethernet MAC depend on the PHY device.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/2] net: stmmac: allow sharing MDIO lines
@ 2023-08-08 18:38 ` Russell King (Oracle)
0 siblings, 0 replies; 34+ messages in thread
From: Russell King (Oracle) @ 2023-08-08 18:38 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Andrew Lunn, Andrew Halaney, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Alexandre Torgue, Jose Abreu, Maxime Coquelin,
Alex Elder, Srini Kandagatla, netdev, devicetree, linux-kernel,
linux-stm32, linux-arm-kernel, Bartosz Golaszewski
On Tue, Aug 08, 2023 at 08:26:22PM +0200, Bartosz Golaszewski wrote:
> When I say "device unbind", I don't just mean manual unbinding using
> sysfs. I mean any code path (rmmod, unplugging the USB, etc.) that
> leads to the device being detached from its driver. This is a
> perfectly normal situation and should work correctly.
>
> I won't be fixing it for this series but may end up looking into
> establishing some kind of device links between MACs and their "remote"
> PHYs that would allow to safely unbind them.
I don't think you're the first to suggest that!
That gets difficult - because although the PHY may be a different
driver, the MDIO bus may be provided by the _same_ hardware as the
ethernet MAC itself. So you end up with a circular dependency - the
PHY device depends on the MDIO bus device (which is the ethernet MAC)
and then you make the ethernet MAC depend on the PHY device.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/2] net: stmmac: allow sharing MDIO lines
2023-08-08 14:09 ` Bartosz Golaszewski
@ 2023-08-08 14:30 ` Russell King (Oracle)
-1 siblings, 0 replies; 34+ messages in thread
From: Russell King (Oracle) @ 2023-08-08 14:30 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexandre Torgue, Jose Abreu, Maxime Coquelin, Andrew Halaney,
Alex Elder, Srini Kandagatla, netdev, devicetree, linux-kernel,
linux-stm32, linux-arm-kernel, Bartosz Golaszewski
On Tue, Aug 08, 2023 at 04:09:11PM +0200, Bartosz Golaszewski wrote:
> On Tue, Aug 8, 2023 at 3:26 PM Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> >
> > On Tue, Aug 08, 2023 at 10:13:09AM +0200, Bartosz Golaszewski wrote:
> > > Ok so upon some further investigation, the actual culprit is in stmmac
> > > platform code - it always tries to register an MDIO bus - independent
> > > of whether there is an actual mdio child node - unless the MAC is
> > > marked explicitly as having a fixed-link.
> > >
> > > When I fixed that, MAC1's probe is correctly deferred until MAC0 has
> > > created the MDIO bus.
> > >
> > > Even so, isn't it useful to actually reference the shared MDIO bus in some way?
> > >
> > > If the schematics look something like this:
> > >
> > > -------- -------
> > > | MAC0 |--MDIO-----| PHY |
> > > -------- | | -------
> > > | |
> > > -------- | | -------
> > > | MAC1 |-- ----| PHY |
> > > -------- -------
> > >
> > > Then it would make sense to model it on the device tree?
> >
> > So I think what you're saying is that MAC0 and MAC1's have MDIO bus
> > masters, and the hardware designer decided to tie both together to
> > a single set of clock and data lines, which then go to two PHYs.
>
> The schematics I have are not very clear on that, but now that you
> mention this, it's most likely the case.
>
> >
> > In that case, I would strongly advise only registering one MDIO bus,
> > and avoid registering the second one - thereby preventing any issues
> > caused by both MDIO bus masters trying to talk at the same time.
> >
>
> I sent a patch for that earlier today.
>
> > The PHYs should be populated in firmware on just one of the buses.
> >
> > You will also need to ensure that whatever registers the bus does
> > make sure that the clocks necessary for communicating on the bus
> > are under control of the MDIO bus code and not the ethernet MAC
> > code. We've run into problems in the past where this has not been
> > the case, and it means - taking your example above - that when MAC1
> > wants to talk to its PHY, if MAC0 isn't alive it can't.
>
> Good point, but it's worse than that: when MAC0 is unbound, it will
> unregister the MDIO bus and destroy all PHY devices. These are not
> refcounted so they will literally go from under MAC1. Not sure how
> this can be dealt with?
That has been a problem in the past, where a MII bus has been
registered by a driver, and then because its probe defers, the MII
bus gets torn down.
The "simple" solution to this is... try to avoid registering the MII
bus until you're sure that the probing will not defer. It is far from
perfect, since there's still the opportunity to unbind the driver
causing the MII bus to vanish along with the PHYs.
I have mentioned trying to address the issue of PHY drivers being
unbound in the past, and there's been some improvements with that,
but if the phy_device vanishes while something is using it, it
certainly will not end well. phylib is not the only case of this,
there are numerous instances of it. One of the recent ones that
I happened to be reminded of today is the pcs-rzn1-miic thing...
If you have a look at miic_create() and consider what would happen
if:
if (!pdev || !platform_get_drvdata(pdev))
return ERR_PTR(-EPROBE_DEFER);
... another thread ended up executing miic_remove() for this
platform device at this very point ...
miic_port = kzalloc(sizeof(*miic_port), GFP_KERNEL);
if (!miic_port)
return ERR_PTR(-ENOMEM);
miic = platform_get_drvdata(pdev);
device_link_add(dev, miic->dev, DL_FLAG_AUTOREMOVE_CONSUMER);
The devm allocation for "miic" would be freed, so either miic
ends up a stale pointer if it happened after this point, or
if miic_remove() completes, then platform_get_drvdata() returns
NULL and we oops the kernel here.
It's an unlikely race, but it's still a race. Sadly, the kernel
is getting riddled with things like this. I used to point these
things out, but having been shouted down many times I've given
up raising it.
Another example is the direct rendering manager bridge code
(drm_bridge).
I suggest a similar approach to not caring too much about this
for your own sanity... providing it doesn't actually cause a
problem!
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 0/2] net: stmmac: allow sharing MDIO lines
@ 2023-08-08 14:30 ` Russell King (Oracle)
0 siblings, 0 replies; 34+ messages in thread
From: Russell King (Oracle) @ 2023-08-08 14:30 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexandre Torgue, Jose Abreu, Maxime Coquelin, Andrew Halaney,
Alex Elder, Srini Kandagatla, netdev, devicetree, linux-kernel,
linux-stm32, linux-arm-kernel, Bartosz Golaszewski
On Tue, Aug 08, 2023 at 04:09:11PM +0200, Bartosz Golaszewski wrote:
> On Tue, Aug 8, 2023 at 3:26 PM Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> >
> > On Tue, Aug 08, 2023 at 10:13:09AM +0200, Bartosz Golaszewski wrote:
> > > Ok so upon some further investigation, the actual culprit is in stmmac
> > > platform code - it always tries to register an MDIO bus - independent
> > > of whether there is an actual mdio child node - unless the MAC is
> > > marked explicitly as having a fixed-link.
> > >
> > > When I fixed that, MAC1's probe is correctly deferred until MAC0 has
> > > created the MDIO bus.
> > >
> > > Even so, isn't it useful to actually reference the shared MDIO bus in some way?
> > >
> > > If the schematics look something like this:
> > >
> > > -------- -------
> > > | MAC0 |--MDIO-----| PHY |
> > > -------- | | -------
> > > | |
> > > -------- | | -------
> > > | MAC1 |-- ----| PHY |
> > > -------- -------
> > >
> > > Then it would make sense to model it on the device tree?
> >
> > So I think what you're saying is that MAC0 and MAC1's have MDIO bus
> > masters, and the hardware designer decided to tie both together to
> > a single set of clock and data lines, which then go to two PHYs.
>
> The schematics I have are not very clear on that, but now that you
> mention this, it's most likely the case.
>
> >
> > In that case, I would strongly advise only registering one MDIO bus,
> > and avoid registering the second one - thereby preventing any issues
> > caused by both MDIO bus masters trying to talk at the same time.
> >
>
> I sent a patch for that earlier today.
>
> > The PHYs should be populated in firmware on just one of the buses.
> >
> > You will also need to ensure that whatever registers the bus does
> > make sure that the clocks necessary for communicating on the bus
> > are under control of the MDIO bus code and not the ethernet MAC
> > code. We've run into problems in the past where this has not been
> > the case, and it means - taking your example above - that when MAC1
> > wants to talk to its PHY, if MAC0 isn't alive it can't.
>
> Good point, but it's worse than that: when MAC0 is unbound, it will
> unregister the MDIO bus and destroy all PHY devices. These are not
> refcounted so they will literally go from under MAC1. Not sure how
> this can be dealt with?
That has been a problem in the past, where a MII bus has been
registered by a driver, and then because its probe defers, the MII
bus gets torn down.
The "simple" solution to this is... try to avoid registering the MII
bus until you're sure that the probing will not defer. It is far from
perfect, since there's still the opportunity to unbind the driver
causing the MII bus to vanish along with the PHYs.
I have mentioned trying to address the issue of PHY drivers being
unbound in the past, and there's been some improvements with that,
but if the phy_device vanishes while something is using it, it
certainly will not end well. phylib is not the only case of this,
there are numerous instances of it. One of the recent ones that
I happened to be reminded of today is the pcs-rzn1-miic thing...
If you have a look at miic_create() and consider what would happen
if:
if (!pdev || !platform_get_drvdata(pdev))
return ERR_PTR(-EPROBE_DEFER);
... another thread ended up executing miic_remove() for this
platform device at this very point ...
miic_port = kzalloc(sizeof(*miic_port), GFP_KERNEL);
if (!miic_port)
return ERR_PTR(-ENOMEM);
miic = platform_get_drvdata(pdev);
device_link_add(dev, miic->dev, DL_FLAG_AUTOREMOVE_CONSUMER);
The devm allocation for "miic" would be freed, so either miic
ends up a stale pointer if it happened after this point, or
if miic_remove() completes, then platform_get_drvdata() returns
NULL and we oops the kernel here.
It's an unlikely race, but it's still a race. Sadly, the kernel
is getting riddled with things like this. I used to point these
things out, but having been shouted down many times I've given
up raising it.
Another example is the direct rendering manager bridge code
(drm_bridge).
I suggest a similar approach to not caring too much about this
for your own sanity... providing it doesn't actually cause a
problem!
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 34+ messages in thread