Hi all,
In a xilinx internal board we have shared GEM MDIO configuration with
TI DP83867 phy and for proper phy detection both PHYs need prior separate
GPIO-reset.
Description:
There are two GEM ethernet IPs instances GEM0 and GEM1. GEM0 and GEM1 used
shared MDIO driven by GEM1.
TI PHYs need prior reset (RESET_B) for PHY detection at defined address.
However with current framework limitation " one reset line per PHY present
on the MDIO bus" the other PHY get detected at incorrect address and later
having child PHY node reset property will also not help.
In order to fix this one possible solution is to allow reset-gpios
property to have PHY reset GPIO tuple for each phy. If this
approach looks fine we can make changes and send out a RFC.
mdio: mdio {
#address-cells = <1>;
#size-cells = <0>;
reset-gpios = <&slg7xl45106 5 GPIO_ACTIVE_HIGH>,
<&slg7xl45106 6 GPIO_ACTIVE_HIGH>;
<snip>
phy0: ethernet-phy@4 {
};
phy1: ethernet-phy@8 {
};
};
Thanks,
Radhey
commit 4c5e7a2c0501bd531aad1d0378c589a92cb3cc31
Author: Florian Fainelli [off-list ref]
AuthorDate: Tue Apr 25 11:33:03 2017 -0700
Commit: David S. Miller [off-list ref]
CommitDate: Wed Apr 26 14:45:34 2017 -0400
dt-bindings: mdio: Clarify binding document
The described GPIO reset property is applicable to *all* child PHYs. If
we have one reset line per PHY present on the MDIO bus, these
automatically become properties of the child PHY nodes.
Finally, indicate how the RESET pulse width must be defined, which is
the maximum value of all individual PHYs RESET pulse widths determined
by reading their datasheets.
Fixes: 69226896ad63 ("mdio_bus: Issue GPIO RESET to PHYs.")
Signed-off-by: Florian Fainelli [off-list ref]
Reviewed-by: Roger Quadros [off-list ref]
Signed-off-by: David S. Miller [off-list ref]
Hi all,
In a xilinx internal board we have shared GEM MDIO configuration with
TI DP83867 phy and for proper phy detection both PHYs need prior separate
GPIO-reset.
Description:
There are two GEM ethernet IPs instances GEM0 and GEM1. GEM0 and GEM1 used
shared MDIO driven by GEM1.
TI PHYs need prior reset (RESET_B) for PHY detection at defined address.
However with current framework limitation " one reset line per PHY present
on the MDIO bus" the other PHY get detected at incorrect address and later
having child PHY node reset property will also not help.
In order to fix this one possible solution is to allow reset-gpios
property to have PHY reset GPIO tuple for each phy. If this
approach looks fine we can make changes and send out a RFC.
I don't think your proposed solution would work because there is no way
to disambiguate which 'reset-gpios' property applies to which PHY,
unless you use a 'reset-gpio-names' property which encodes the phy
address in there. But even doing so, if the 'reset-gpios' property is
placed within the MDIO controller node then it applies within its scope
which is the MDIO controller. The other reason why it is wrong is
because the MDIO bus itself may need multiple resets to be toggled to be
put in a functional state. This is probably uncommon for MDIO, but it is
not for other types of peripherals with complex asynchronous reset
circuits (the things you love to hate).
The MDIO bus layer supports something like this which is much more
accurate in describing the reset GPIOs pertaining to each PHY device:
mdio {
..
phy0: ethernet-phy@0 {
reg = <0>;
reset-gpios = <&slg7xl45106 5 GPIO_ACTIVE_HIGH>;
};
phy1: ethernet-phy@8 {
reg = <8>;
reset-gpios = <&slg7xl45106 6 GPIO_ACTIVE_HIGH>;
};
};
The code that will parse that property is in drivers/net/phy/mdio_bus.c
under mdiobus_register_gpiod()/mdiobus_register_reset() and then
mdio_device_reset() called by phy_device_reset() will pulse the per-PHY
device reset line/GPIO.
Are you saying that you tried that and this did not work somehow? Can
you describe in more details how the timing of the reset pulsing affects
the way each TI PHY is going to gets its MDIO address assigned?
Thanks
--
Florian
-----Original Message-----
From: Florian Fainelli <f.fainelli@gmail.com>
Sent: Wednesday, October 27, 2021 10:48 PM
To: Radhey Shyam Pandey <redacted>; netdev@vger.kernel.org;
Andrew Lunn [off-list ref]; Heiner Kallweit [off-list ref];
Russell King [off-list ref]
Cc: Michal Simek <redacted>; Harini Katakam <redacted>
Subject: Re: mdio: separate gpio-reset property per child phy usecase
+PHY library maintainers,
On 10/27/21 5:58 AM, Radhey Shyam Pandey wrote:
quoted
Hi all,
In a xilinx internal board we have shared GEM MDIO configuration with
TI DP83867 phy and for proper phy detection both PHYs need prior
separate GPIO-reset.
Description:
There are two GEM ethernet IPs instances GEM0 and GEM1. GEM0 and GEM1
used shared MDIO driven by GEM1.
TI PHYs need prior reset (RESET_B) for PHY detection at defined address.
However with current framework limitation " one reset line per PHY
present on the MDIO bus" the other PHY get detected at incorrect
address and later having child PHY node reset property will also not help.
In order to fix this one possible solution is to allow reset-gpios
property to have PHY reset GPIO tuple for each phy. If this approach
looks fine we can make changes and send out a RFC.
I don't think your proposed solution would work because there is no way to
disambiguate which 'reset-gpios' property applies to which PHY, unless you use
a 'reset-gpio-names' property which encodes the phy address in there. But
even doing so, if the 'reset-gpios' property is placed within the MDIO controller
node then it applies within its scope which is the MDIO controller. The other
reason why it is wrong is because the MDIO bus itself may need multiple resets
to be toggled to be put in a functional state. This is probably uncommon for
MDIO, but it is not for other types of peripherals with complex asynchronous
reset circuits (the things you love to hate).
The MDIO bus layer supports something like this which is much more accurate
in describing the reset GPIOs pertaining to each PHY device:
mdio {
..
phy0: ethernet-phy@0 {
reg = <0>;
reset-gpios = <&slg7xl45106 5 GPIO_ACTIVE_HIGH>;
};
phy1: ethernet-phy@8 {
reg = <8>;
reset-gpios = <&slg7xl45106 6 GPIO_ACTIVE_HIGH>;
};
};
The code that will parse that property is in drivers/net/phy/mdio_bus.c under
mdiobus_register_gpiod()/mdiobus_register_reset() and then
mdio_device_reset() called by phy_device_reset() will pulse the per-PHY device
reset line/GPIO.
Are you saying that you tried that and this did not work somehow? Can you
describe in more details how the timing of the reset pulsing affects the way
each TI PHY is going to gets its MDIO address assigned?
Yes, having reset-gpios in PHY node is not working. Just to highlight - We are
using external strap configuration for PHY Address configuration. The strap
pin configuration is set by sw stack at a later stage. PHY address on
power on is configured based on sampled values at strap pins which is not
PHY address mentioned in DT. (It could be any PHY Address depending on
strap pins default input). For PHY detect to happen at proper PHY Address
we have call PHY reset (RESET_B) after strap pins are configured otherwise
probe (of_mdiobus_phy_device_register) fails and we see below error:
mdio_bus ff0c0000.ethernet-ffffffff: MDIO device at address 8 is missing.
Thanks,
Radhey
References:
8.5.4PHYAddressConfiguration [DP83867E/IS/CS datasheet]
From: "Russell King (Oracle)" <linux@armlinux.org.uk> Date: 2021-11-03 09:02:21
On Wed, Nov 03, 2021 at 08:50:30AM +0000, Radhey Shyam Pandey wrote:
quoted
-----Original Message-----
From: Florian Fainelli <f.fainelli@gmail.com>
Sent: Wednesday, October 27, 2021 10:48 PM
To: Radhey Shyam Pandey <redacted>; netdev@vger.kernel.org;
Andrew Lunn [off-list ref]; Heiner Kallweit [off-list ref];
Russell King [off-list ref]
Cc: Michal Simek <redacted>; Harini Katakam <redacted>
Subject: Re: mdio: separate gpio-reset property per child phy usecase
+PHY library maintainers,
On 10/27/21 5:58 AM, Radhey Shyam Pandey wrote:
quoted
Hi all,
In a xilinx internal board we have shared GEM MDIO configuration with
TI DP83867 phy and for proper phy detection both PHYs need prior
separate GPIO-reset.
Description:
There are two GEM ethernet IPs instances GEM0 and GEM1. GEM0 and GEM1
used shared MDIO driven by GEM1.
TI PHYs need prior reset (RESET_B) for PHY detection at defined address.
However with current framework limitation " one reset line per PHY
present on the MDIO bus" the other PHY get detected at incorrect
address and later having child PHY node reset property will also not help.
In order to fix this one possible solution is to allow reset-gpios
property to have PHY reset GPIO tuple for each phy. If this approach
looks fine we can make changes and send out a RFC.
I don't think your proposed solution would work because there is no way to
disambiguate which 'reset-gpios' property applies to which PHY, unless you use
a 'reset-gpio-names' property which encodes the phy address in there. But
even doing so, if the 'reset-gpios' property is placed within the MDIO controller
node then it applies within its scope which is the MDIO controller. The other
reason why it is wrong is because the MDIO bus itself may need multiple resets
to be toggled to be put in a functional state. This is probably uncommon for
MDIO, but it is not for other types of peripherals with complex asynchronous
reset circuits (the things you love to hate).
The MDIO bus layer supports something like this which is much more accurate
in describing the reset GPIOs pertaining to each PHY device:
mdio {
..
phy0: ethernet-phy@0 {
reg = <0>;
reset-gpios = <&slg7xl45106 5 GPIO_ACTIVE_HIGH>;
};
phy1: ethernet-phy@8 {
reg = <8>;
reset-gpios = <&slg7xl45106 6 GPIO_ACTIVE_HIGH>;
};
};
The code that will parse that property is in drivers/net/phy/mdio_bus.c under
mdiobus_register_gpiod()/mdiobus_register_reset() and then
mdio_device_reset() called by phy_device_reset() will pulse the per-PHY device
reset line/GPIO.
Are you saying that you tried that and this did not work somehow? Can you
describe in more details how the timing of the reset pulsing affects the way
each TI PHY is going to gets its MDIO address assigned?
Yes, having reset-gpios in PHY node is not working. Just to highlight - We are
using external strap configuration for PHY Address configuration. The strap
pin configuration is set by sw stack at a later stage. PHY address on
power on is configured based on sampled values at strap pins which is not
PHY address mentioned in DT. (It could be any PHY Address depending on
strap pins default input). For PHY detect to happen at proper PHY Address
we have call PHY reset (RESET_B) after strap pins are configured otherwise
probe (of_mdiobus_phy_device_register) fails and we see below error:
mdio_bus ff0c0000.ethernet-ffffffff: MDIO device at address 8 is missing.
This is a well-known problem with placing resets in the PHY node. In
this case, you must add a compatible property as well that matches
"ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}" so that phylib knows the
contents of the ID registers.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
-----Original Message-----
From: Russell King <linux@armlinux.org.uk>
Sent: Wednesday, November 3, 2021 2:32 PM
To: Radhey Shyam Pandey <redacted>
Cc: Florian Fainelli <f.fainelli@gmail.com>; netdev@vger.kernel.org; Andrew
Lunn [off-list ref]; Heiner Kallweit [off-list ref]; Michal
Simek [off-list ref]; Harini Katakam [off-list ref]
Subject: Re: mdio: separate gpio-reset property per child phy usecase
On Wed, Nov 03, 2021 at 08:50:30AM +0000, Radhey Shyam Pandey wrote:
quoted
quoted
-----Original Message-----
From: Florian Fainelli <f.fainelli@gmail.com>
Sent: Wednesday, October 27, 2021 10:48 PM
To: Radhey Shyam Pandey <redacted>; netdev@vger.kernel.org;
Andrew Lunn [off-list ref]; Heiner Kallweit
[off-list ref];
quoted
quoted
Russell King [off-list ref]
Cc: Michal Simek <redacted>; Harini Katakam
[off-list ref]
quoted
quoted
Subject: Re: mdio: separate gpio-reset property per child phy usecase
+PHY library maintainers,
On 10/27/21 5:58 AM, Radhey Shyam Pandey wrote:
quoted
Hi all,
In a xilinx internal board we have shared GEM MDIO configuration with
TI DP83867 phy and for proper phy detection both PHYs need prior
separate GPIO-reset.
Description:
There are two GEM ethernet IPs instances GEM0 and GEM1. GEM0 and
GEM1
quoted
quoted
quoted
used shared MDIO driven by GEM1.
TI PHYs need prior reset (RESET_B) for PHY detection at defined address.
However with current framework limitation " one reset line per PHY
present on the MDIO bus" the other PHY get detected at incorrect
address and later having child PHY node reset property will also not help.
In order to fix this one possible solution is to allow reset-gpios
property to have PHY reset GPIO tuple for each phy. If this approach
looks fine we can make changes and send out a RFC.
I don't think your proposed solution would work because there is no way
to
quoted
quoted
disambiguate which 'reset-gpios' property applies to which PHY, unless you
use
quoted
quoted
a 'reset-gpio-names' property which encodes the phy address in there. But
even doing so, if the 'reset-gpios' property is placed within the MDIO
controller
quoted
quoted
node then it applies within its scope which is the MDIO controller. The
other
quoted
quoted
reason why it is wrong is because the MDIO bus itself may need multiple
resets
quoted
quoted
to be toggled to be put in a functional state. This is probably uncommon for
MDIO, but it is not for other types of peripherals with complex
asynchronous
quoted
quoted
reset circuits (the things you love to hate).
The MDIO bus layer supports something like this which is much more
accurate
quoted
quoted
in describing the reset GPIOs pertaining to each PHY device:
mdio {
..
phy0: ethernet-phy@0 {
reg = <0>;
reset-gpios = <&slg7xl45106 5 GPIO_ACTIVE_HIGH>;
};
phy1: ethernet-phy@8 {
reg = <8>;
reset-gpios = <&slg7xl45106 6 GPIO_ACTIVE_HIGH>;
};
};
The code that will parse that property is in drivers/net/phy/mdio_bus.c
under
quoted
quoted
mdiobus_register_gpiod()/mdiobus_register_reset() and then
mdio_device_reset() called by phy_device_reset() will pulse the per-PHY
device
quoted
quoted
reset line/GPIO.
Are you saying that you tried that and this did not work somehow? Can you
describe in more details how the timing of the reset pulsing affects the way
each TI PHY is going to gets its MDIO address assigned?
Yes, having reset-gpios in PHY node is not working. Just to highlight - We are
using external strap configuration for PHY Address configuration. The strap
pin configuration is set by sw stack at a later stage. PHY address on
power on is configured based on sampled values at strap pins which is not
PHY address mentioned in DT. (It could be any PHY Address depending on
strap pins default input). For PHY detect to happen at proper PHY Address
we have call PHY reset (RESET_B) after strap pins are configured otherwise
probe (of_mdiobus_phy_device_register) fails and we see below error:
mdio_bus ff0c0000.ethernet-ffffffff: MDIO device at address 8 is missing.
This is a well-known problem with placing resets in the PHY node. In
this case, you must add a compatible property as well that matches
"ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}" so that phylib knows the
contents of the ID registers.
Thanks! Using ethernet-phy-id compatible property seems to work.
I think we are good for now, will get back if there is any followup
discussion required.