On Fri, Feb 12, 2021 at 05:17:53PM -0800, Florian Fainelli wrote: > On 2/12/2021 4:28 PM, 'Robert Hancock' via BCM-KERNEL-FEEDBACK-LIST,PDL > wrote: > > bcm54xx_config_init was modifying the PHY LED configuration to enable link > > and activity indications. However, some SFP modules (such as Bel-Fuse > > SFP-1GBT-06) have no LEDs but use the LED outputs to control the SFP LOS > > signal, and modifying the LED settings will cause the LOS output to > > malfunction. Skip this configuration for PHYs which are bound to an SFP > > bus.
That would be me, sorry. > > Signed-off-by: Robert Hancock <robert.hanc...@calian.com> > > --- > > drivers/net/phy/broadcom.c | 26 +++++++++++++++++--------- > > 1 file changed, 17 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c > > index 78542580f2b2..81a5721f732a 100644 > > --- a/drivers/net/phy/broadcom.c > > +++ b/drivers/net/phy/broadcom.c > > @@ -12,6 +12,7 @@ > > > > #include "bcm-phy-lib.h" > > #include <linux/module.h> > > +#include <linux/netdevice.h> > > #include <linux/phy.h> > > #include <linux/brcmphy.h> > > #include <linux/of.h> > > @@ -365,18 +366,25 @@ static int bcm54xx_config_init(struct phy_device > > *phydev) > > > > bcm54xx_phydsp_config(phydev); > > > > - /* Encode link speed into LED1 and LED3 pair (green/amber). > > + /* For non-SFP setups, encode link speed into LED1 and LED3 pair > > + * (green/amber). > > * Also flash these two LEDs on activity. This means configuring > > * them for MULTICOLOR and encoding link/activity into them. > > + * Don't do this for devices that may be on an SFP module, since > > + * some of these use the LED outputs to control the SFP LOS signal, > > + * and changing these settings will cause LOS to malfunction. > > */ > > - val = BCM5482_SHD_LEDS1_LED1(BCM_LED_SRC_MULTICOLOR1) | > > - BCM5482_SHD_LEDS1_LED3(BCM_LED_SRC_MULTICOLOR1); > > - bcm_phy_write_shadow(phydev, BCM5482_SHD_LEDS1, val); > > - > > - val = BCM_LED_MULTICOLOR_IN_PHASE | > > - BCM5482_SHD_LEDS1_LED1(BCM_LED_MULTICOLOR_LINK_ACT) | > > - BCM5482_SHD_LEDS1_LED3(BCM_LED_MULTICOLOR_LINK_ACT); > > - bcm_phy_write_exp(phydev, BCM_EXP_MULTICOLOR, val); > > + if (!phydev->sfp_bus && > > + (!phydev->attached_dev || !phydev->attached_dev->sfp_bus)) { > > + val = BCM5482_SHD_LEDS1_LED1(BCM_LED_SRC_MULTICOLOR1) | > > + BCM5482_SHD_LEDS1_LED3(BCM_LED_SRC_MULTICOLOR1); > > + bcm_phy_write_shadow(phydev, BCM5482_SHD_LEDS1, val); > > + > > + val = BCM_LED_MULTICOLOR_IN_PHASE | > > + BCM5482_SHD_LEDS1_LED1(BCM_LED_MULTICOLOR_LINK_ACT) | > > + BCM5482_SHD_LEDS1_LED3(BCM_LED_MULTICOLOR_LINK_ACT); > > + bcm_phy_write_exp(phydev, BCM_EXP_MULTICOLOR, val); > > Not sure I can come up with a better solution but this should probably > be made conditional upon the specific SFP module, or a set of specific > SFP modules, whether we can convey those details via a Device Tree > property or by doing a SFP ID lookup. > > Acked-by: Florian Fainelli <f.faine...@gmail.com> Not sure when is the last time I saw an SFP module with activity/link LEDs, the patch was intended for a BCM5464R RJ45 twisted pair setup which I still have. What is the last thing that happened in PHY LED territory? I lost track since Marek's RFC for offloading phy_led_triggers that didn't seem to make any progress. I could try to explicitly request the activity LED to blink on my board via device tree, if it helps.