This patch series cleans up the brcmphy.h header and its numerous unused
phydev->dev_flags, fixes the RXC/TXC clock disabling bit and allows the
BCM54210E PHY to utilize APD.
Changes in v2:
- dropped the patch that attempted to fix a possible discrepancy between
the datasheet and the actual hardware
- added a patch to remove a forward declaration
- do additional flags cleanup
Florian Fainelli (3):
net: phy: broadcom: Avoid forward for bcm54xx_config_clock_delay()
net: phy: broadcom: Remove unused flags
net: phy: broadcom: Allow BCM54210E to configure APD
drivers/net/phy/broadcom.c | 101 +++++++++++++++++++++----------------
include/linux/brcmphy.h | 23 ++++-----
2 files changed, 66 insertions(+), 58 deletions(-)
--
2.25.1
@@ -26,44 +26,6 @@ MODULE_DESCRIPTION("Broadcom PHY driver");MODULE_AUTHOR("Maciej W. Rozycki");MODULE_LICENSE("GPL");-staticintbcm54xx_config_clock_delay(structphy_device*phydev);--staticintbcm54210e_config_init(structphy_device*phydev)-{-intval;--bcm54xx_config_clock_delay(phydev);--if(phydev->dev_flags&PHY_BRCM_EN_MASTER_MODE){-val=phy_read(phydev,MII_CTRL1000);-val|=CTL1000_AS_MASTER|CTL1000_ENABLE_MASTER;-phy_write(phydev,MII_CTRL1000,val);-}--return0;-}--staticintbcm54612e_config_init(structphy_device*phydev)-{-intreg;--bcm54xx_config_clock_delay(phydev);--/* Enable CLK125 MUX on LED4 if ref clock is enabled. */-if(!(phydev->dev_flags&PHY_BRCM_RX_REFCLK_UNUSED)){-interr;--reg=bcm_phy_read_exp(phydev,BCM54612E_EXP_SPARE0);-err=bcm_phy_write_exp(phydev,BCM54612E_EXP_SPARE0,-BCM54612E_LED4_CLK125OUT_EN|reg);--if(err<0)-returnerr;-}--return0;-}-staticintbcm54xx_config_clock_delay(structphy_device*phydev){intrc,val;
@@ -105,6 +67,42 @@ static int bcm54xx_config_clock_delay(struct phy_device *phydev)return0;}+staticintbcm54210e_config_init(structphy_device*phydev)+{+intval;++bcm54xx_config_clock_delay(phydev);++if(phydev->dev_flags&PHY_BRCM_EN_MASTER_MODE){+val=phy_read(phydev,MII_CTRL1000);+val|=CTL1000_AS_MASTER|CTL1000_ENABLE_MASTER;+phy_write(phydev,MII_CTRL1000,val);+}++return0;+}++staticintbcm54612e_config_init(structphy_device*phydev)+{+intreg;++bcm54xx_config_clock_delay(phydev);++/* Enable CLK125 MUX on LED4 if ref clock is enabled. */+if(!(phydev->dev_flags&PHY_BRCM_RX_REFCLK_UNUSED)){+interr;++reg=bcm_phy_read_exp(phydev,BCM54612E_EXP_SPARE0);+err=bcm_phy_write_exp(phydev,BCM54612E_EXP_SPARE0,+BCM54612E_LED4_CLK125OUT_EN|reg);++if(err<0)+returnerr;+}++return0;+}+/* Needs SMDSP clock enabled via bcm54xx_phydsp_config() */staticintbcm50610_a0_workaround(structphy_device*phydev){
We have a number of unused flags defined today and since we are scarce
on space and may need to introduce new flags in the future remove and
shift every existing flag down into a contiguous assignment.
PHY_BCM_FLAGS_MODE_1000BX was only used internally for the BCM54616S
PHY, so we allocate a driver private structure instead to store that
flag instead of canibalizing one from phydev->dev_flags for that
purpose.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/phy/broadcom.c | 19 ++++++++++++++++---
include/linux/brcmphy.h | 21 ++++++++-------------
2 files changed, 24 insertions(+), 16 deletions(-)
BCM54210E/BCM50212E has been verified to work correctly with the
auto-power down configuration done by bcm54xx_adjust_rxrefclk(), add it
to the list of PHYs working.
While we are at it, provide an appropriate name for the bit we are
changing which disables the RXC and TXC during auto-power down when
there is no energy on the cable.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/phy/broadcom.c | 8 +++++---
include/linux/brcmphy.h | 2 +-
2 files changed, 6 insertions(+), 4 deletions(-)
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-02-13 10:32:42
On Fri, Feb 12, 2021 at 07:46:30PM -0800, Florian Fainelli wrote:
Avoid a forward declaration by moving the callers of
bcm54xx_config_clock_delay() below its body.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-02-13 10:34:57
On Fri, Feb 12, 2021 at 07:46:31PM -0800, Florian Fainelli wrote:
We have a number of unused flags defined today and since we are scarce
on space and may need to introduce new flags in the future remove and
shift every existing flag down into a contiguous assignment.
PHY_BCM_FLAGS_MODE_1000BX was only used internally for the BCM54616S
PHY, so we allocate a driver private structure instead to store that
flag instead of canibalizing one from phydev->dev_flags for that
purpose.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
So you want to remove the IBND dev_flags separately, okay.
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-02-13 10:43:55
On Fri, Feb 12, 2021 at 07:46:32PM -0800, Florian Fainelli wrote:
BCM54210E/BCM50212E has been verified to work correctly with the
auto-power down configuration done by bcm54xx_adjust_rxrefclk(), add it
to the list of PHYs working.
While we are at it, provide an appropriate name for the bit we are
changing which disables the RXC and TXC during auto-power down when
there is no energy on the cable.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Curiously enough, my BCM5464R datasheet does say:
The TXC and RXC outputs can be disabled during auto-power down by setting the “1000BASE-T/100BASE-TX/10BASE-T
Spare Control 3 Register (Address 1Ch, Shadow Value 00101),” bit 8 =1.
but when I go to the definition of the register, bit 8 is hidden. Odd.
How can I ensure that the auto power down feature is doing something?
quoted hunk
/* 01010: Auto Power-Down */
#define BCM54XX_SHD_APD 0x0a
On Fri, Feb 12, 2021 at 07:46:32PM -0800, Florian Fainelli wrote:
quoted
BCM54210E/BCM50212E has been verified to work correctly with the
auto-power down configuration done by bcm54xx_adjust_rxrefclk(), add it
to the list of PHYs working.
While we are at it, provide an appropriate name for the bit we are
changing which disables the RXC and TXC during auto-power down when
there is no energy on the cable.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Curiously enough, my BCM5464R datasheet does say:
The TXC and RXC outputs can be disabled during auto-power down by setting the “1000BASE-T/100BASE-TX/10BASE-T
Spare Control 3 Register (Address 1Ch, Shadow Value 00101),” bit 8 =1.
but when I go to the definition of the register, bit 8 is hidden. Odd.
How can I ensure that the auto power down feature is doing something?
I am trying to confirm what the expected power levels should be from the
54210E product engineer so I can give you an estimate of what you should
see with and without while measure the PHY's regulators.
--
Florian
Hello:
This series was applied to netdev/net-next.git (refs/heads/master):
On Fri, 12 Feb 2021 19:46:29 -0800 you wrote:
This patch series cleans up the brcmphy.h header and its numerous unused
phydev->dev_flags, fixes the RXC/TXC clock disabling bit and allows the
BCM54210E PHY to utilize APD.
Changes in v2:
- dropped the patch that attempted to fix a possible discrepancy between
the datasheet and the actual hardware
- added a patch to remove a forward declaration
- do additional flags cleanup
[...]
On Fri, Feb 12, 2021 at 07:46:32PM -0800, Florian Fainelli wrote:
quoted
BCM54210E/BCM50212E has been verified to work correctly with the
auto-power down configuration done by bcm54xx_adjust_rxrefclk(), add it
to the list of PHYs working.
While we are at it, provide an appropriate name for the bit we are
changing which disables the RXC and TXC during auto-power down when
there is no energy on the cable.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Curiously enough, my BCM5464R datasheet does say:
The TXC and RXC outputs can be disabled during auto-power down by setting the “1000BASE-T/100BASE-TX/10BASE-T
Spare Control 3 Register (Address 1Ch, Shadow Value 00101),” bit 8 =1.
but when I go to the definition of the register, bit 8 is hidden. Odd.
How can I ensure that the auto power down feature is doing something?
I am trying to confirm what the expected power levels should be from the
54210E product engineer so I can give you an estimate of what you should
see with and without while measure the PHY's regulators.
Took a while but for the 54210E reference board here are the numbers,
your mileage will vary depending on the supplies, regulator efficiency
and PCB design around the PHY obviously:
BMCR.PDOWN: 86.12 mW
auto-power down: 77.84 mW
auto-power-down, DLL disabled: 30.83 mW
IDDQ-low power: 9.85 mW (requires a RESETn toggle)
IDDQ with soft recovery: 10.75 mW
Interestingly, the 50212E that I am using requires writing the PDOWN bit
and only that bit (not a RMW) in order to get in a correct state, both
LEDs keep flashing when that happens, fixes coming.
When net-next opens back up I will submit patches to support IDDQ with
soft recovery since that is clearly much better than the standard power
down and it does not require a RESETn toggle.
--
Florian
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-03-05 01:08:51
On Tue, Mar 02, 2021 at 07:37:34PM -0800, Florian Fainelli wrote:
Took a while but for the 54210E reference board here are the numbers,
your mileage will vary depending on the supplies, regulator efficiency
and PCB design around the PHY obviously:
BMCR.PDOWN: 86.12 mW
auto-power down: 77.84 mW
Quite curious that the APD power is lower than the normal BMCR.PDOWN
value. As far as my understanding goes, when in APD mode, the PHY even
wakes up from time to time to send pulses to the link partner?
auto-power-down, DLL disabled: 30.83 mW
The jump from simple APD to APD with DLL disabled is pretty big.
Correct me if I'm wrong, but there's an intermediary step which was not
measured, where the CLK125 is disabled but the internal DLL (Delay
Locked Loop?) is still enabled. I think powering off the internal DLL
also implies powering off the CLK125 pin, at least that's how the PHY
driver treats things at the moment. But we don't know if the huge
reduction in power is due just to CLK125 or the DLL (it's more likely
it's due to both, in equal amounts).
Anyway, it's great to have some results which tell us exactly what is
worthwhile and what isn't. In other news, I've added the BCM5464 to the
list of PHYs with APD and I didn't see any issues thus far.
IDDQ-low power: 9.85 mW (requires a RESETn toggle)
IDDQ with soft recovery: 10.75 mW
Interestingly, the 50212E that I am using requires writing the PDOWN bit
and only that bit (not a RMW) in order to get in a correct state, both
LEDs keep flashing when that happens, fixes coming.
When net-next opens back up I will submit patches to support IDDQ with
soft recovery since that is clearly much better than the standard power
down and it does not require a RESETn toggle.
Iddq must be the quiescent supply current, isn't it (but in that case,
I'm a bit confused to not see a value in mA)? Is it an actual operating
mode (I don't see anything about that mentioned in the BCM5464 sheet)
and if it is, what is there exactly to support?
On Tue, Mar 02, 2021 at 07:37:34PM -0800, Florian Fainelli wrote:
quoted
Took a while but for the 54210E reference board here are the numbers,
your mileage will vary depending on the supplies, regulator efficiency
and PCB design around the PHY obviously:
BMCR.PDOWN: 86.12 mW
auto-power down: 77.84 mW
Quite curious that the APD power is lower than the normal BMCR.PDOWN
value. As far as my understanding goes, when in APD mode, the PHY even
wakes up from time to time to send pulses to the link partner?
Auto-power down kicks in when the cable is disconnected. There is
another IDDQ mode that supports energy detection though I am unsure of
when it would be useful for most Linux enabled systems.
quoted
auto-power-down, DLL disabled: 30.83 mW
The jump from simple APD to APD with DLL disabled is pretty big.
Correct me if I'm wrong, but there's an intermediary step which was not
measured, where the CLK125 is disabled but the internal DLL (Delay
Locked Loop?) is still enabled. I think powering off the internal DLL
also implies powering off the CLK125 pin, at least that's how the PHY
driver treats things at the moment. But we don't know if the huge
reduction in power is due just to CLK125 or the DLL (it's more likely
it's due to both, in equal amounts).
Agree, I do not have the break down though.
Anyway, it's great to have some results which tell us exactly what is
worthwhile and what isn't. In other news, I've added the BCM5464 to the
list of PHYs with APD and I didn't see any issues thus far.
quoted
IDDQ-low power: 9.85 mW (requires a RESETn toggle)
IDDQ with soft recovery: 10.75 mW
Interestingly, the 50212E that I am using requires writing the PDOWN bit
and only that bit (not a RMW) in order to get in a correct state, both
LEDs keep flashing when that happens, fixes coming.
When net-next opens back up I will submit patches to support IDDQ with
soft recovery since that is clearly much better than the standard power
down and it does not require a RESETn toggle.
Iddq must be the quiescent supply current, isn't it (but in that case,
I'm a bit confused to not see a value in mA)? Is it an actual operating
mode (I don't see anything about that mentioned in the BCM5464 sheet)
and if it is, what is there exactly to support?
You would put the PHY in IDDQ with soft recovery (or ultra low power)
when you are administratively bringing down the network interface (and
its PHY), or when suspending to a low power state where Wake-on-LAN is
not enabled.
--
Florian