| Message ID | 20240522091911.2862403-1-vitaly.lifshits@intel.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Anthony Nguyen |
| Headers | show |
| Series | [iwl-next,v1,1/1] igc: add support for ethtool.set_phys_id | expand |
On 5/22/24 11:19, Vitaly Lifshits wrote: > Add support for ethtool.set_phys_id callback to initiate LED blinking > and stopping them by the ethtool interface. > This is done by storing the initial LEDCTL register value and restoring > it when LED blinking is terminated. > > In addition, moved IGC_LEDCTL related defines from igc_leds.c to > igc_defines.h where they can be included by all of the igc module > files. > > Signed-off-by: Vitaly Lifshits <vitaly.lifshits@intel.com> > Signed-off-by: Menachem Fogel <menachem.fogel@intel.com> sender's SoB should be last. Should, by any chance, Menachem be marked as an author of this change? side note: we should CC netdev and obey netdev rules, like not sending -next content during window being closed for new submissions https://patchwork.hopto.org/net-next.html > --- > drivers/net/ethernet/intel/igc/igc_defines.h | 22 ++++++++++++++ > drivers/net/ethernet/intel/igc/igc_ethtool.c | 32 ++++++++++++++++++++ > drivers/net/ethernet/intel/igc/igc_hw.h | 2 ++ > drivers/net/ethernet/intel/igc/igc_leds.c | 21 +------------ > drivers/net/ethernet/intel/igc/igc_main.c | 2 ++ > 5 files changed, 59 insertions(+), 20 deletions(-) > > diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h b/drivers/net/ethernet/intel/igc/igc_defines.h > index 5f92b3c7c3d4..664d49f10427 100644 > --- a/drivers/net/ethernet/intel/igc/igc_defines.h > +++ b/drivers/net/ethernet/intel/igc/igc_defines.h > @@ -686,4 +686,26 @@ > #define IGC_LTRMAXV_LSNP_REQ 0x00008000 /* LTR Snoop Requirement */ > #define IGC_LTRMAXV_SCALE_SHIFT 10 > > +/* LED ctrl defines */ > +#define IGC_NUM_LEDS 3 > + > +#define IGC_LEDCTL_GLOBAL_BLINK_MODE BIT(5) > +#define IGC_LEDCTL_LED0_MODE_SHIFT 0 > +#define IGC_LEDCTL_LED0_MODE_MASK GENMASK(3, 0) > +#define IGC_LEDCTL_LED0_BLINK BIT(7) > +#define IGC_LEDCTL_LED1_MODE_SHIFT 8 > +#define IGC_LEDCTL_LED1_MODE_MASK GENMASK(11, 8) > +#define IGC_LEDCTL_LED1_BLINK BIT(15) > +#define IGC_LEDCTL_LED2_MODE_SHIFT 16 > +#define IGC_LEDCTL_LED2_MODE_MASK GENMASK(19, 16) > +#define IGC_LEDCTL_LED2_BLINK BIT(23) > + > +#define IGC_LEDCTL_MODE_ON 0x00 > +#define IGC_LEDCTL_MODE_OFF 0x01 > +#define IGC_LEDCTL_MODE_LINK_10 0x05 > +#define IGC_LEDCTL_MODE_LINK_100 0x06 > +#define IGC_LEDCTL_MODE_LINK_1000 0x07 > +#define IGC_LEDCTL_MODE_LINK_2500 0x08 > +#define IGC_LEDCTL_MODE_ACTIVITY 0x0b > + > #endif /* _IGC_DEFINES_H_ */ > diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c > index f2c4f1966bb0..82ece5f95f1e 100644 > --- a/drivers/net/ethernet/intel/igc/igc_ethtool.c > +++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c > @@ -1975,6 +1975,37 @@ static void igc_ethtool_diag_test(struct net_device *netdev, > msleep_interruptible(4 * 1000); > } > > +static int igc_ethtool_set_phys_id(struct net_device *netdev, > + enum ethtool_phys_id_state state) > +{ > + struct igc_adapter *adapter = netdev_priv(netdev); > + struct igc_hw *hw = &adapter->hw; > + u32 ledctl; > + > + switch (state) { > + case ETHTOOL_ID_ACTIVE: > + ledctl = rd32(IGC_LEDCTL); > + > + /* initiate LED1 blinking */ > + ledctl &= ~(IGC_LEDCTL_GLOBAL_BLINK_MODE | > + IGC_LEDCTL_LED1_MODE_MASK | > + IGC_LEDCTL_LED2_MODE_MASK); > + ledctl |= IGC_LEDCTL_LED1_BLINK; > + wr32(IGC_LEDCTL, ledctl); > + break; > + > + case ETHTOOL_ID_INACTIVE: > + /* restore LEDCTL default value */ > + wr32(IGC_LEDCTL, hw->mac.ledctl_default); > + break; > + > + default: > + break; > + } > + > + return 0; > +} > + > static const struct ethtool_ops igc_ethtool_ops = { > .supported_coalesce_params = ETHTOOL_COALESCE_USECS, > .get_drvinfo = igc_ethtool_get_drvinfo, > @@ -2013,6 +2044,7 @@ static const struct ethtool_ops igc_ethtool_ops = { > .get_link_ksettings = igc_ethtool_get_link_ksettings, > .set_link_ksettings = igc_ethtool_set_link_ksettings, > .self_test = igc_ethtool_diag_test, > + .set_phys_id = igc_ethtool_set_phys_id, > }; > > void igc_ethtool_set_ops(struct net_device *netdev) > diff --git a/drivers/net/ethernet/intel/igc/igc_hw.h b/drivers/net/ethernet/intel/igc/igc_hw.h > index e1c572e0d4ef..45b68695bdb7 100644 > --- a/drivers/net/ethernet/intel/igc/igc_hw.h > +++ b/drivers/net/ethernet/intel/igc/igc_hw.h > @@ -95,6 +95,8 @@ struct igc_mac_info { > bool autoneg; > bool autoneg_failed; > bool get_link_status; > + > + u32 ledctl_default; > }; > > struct igc_nvm_operations { > diff --git a/drivers/net/ethernet/intel/igc/igc_leds.c b/drivers/net/ethernet/intel/igc/igc_leds.c > index 3929b25b6ae6..e5eeef240802 100644 > --- a/drivers/net/ethernet/intel/igc/igc_leds.c > +++ b/drivers/net/ethernet/intel/igc/igc_leds.c > @@ -8,26 +8,7 @@ > #include <uapi/linux/uleds.h> > > #include "igc.h" > - > -#define IGC_NUM_LEDS 3 > - > -#define IGC_LEDCTL_LED0_MODE_SHIFT 0 > -#define IGC_LEDCTL_LED0_MODE_MASK GENMASK(3, 0) > -#define IGC_LEDCTL_LED0_BLINK BIT(7) > -#define IGC_LEDCTL_LED1_MODE_SHIFT 8 > -#define IGC_LEDCTL_LED1_MODE_MASK GENMASK(11, 8) > -#define IGC_LEDCTL_LED1_BLINK BIT(15) > -#define IGC_LEDCTL_LED2_MODE_SHIFT 16 > -#define IGC_LEDCTL_LED2_MODE_MASK GENMASK(19, 16) > -#define IGC_LEDCTL_LED2_BLINK BIT(23) > - > -#define IGC_LEDCTL_MODE_ON 0x00 > -#define IGC_LEDCTL_MODE_OFF 0x01 > -#define IGC_LEDCTL_MODE_LINK_10 0x05 > -#define IGC_LEDCTL_MODE_LINK_100 0x06 > -#define IGC_LEDCTL_MODE_LINK_1000 0x07 > -#define IGC_LEDCTL_MODE_LINK_2500 0x08 > -#define IGC_LEDCTL_MODE_ACTIVITY 0x0b > +#include "igc_defines.h" > > #define IGC_SUPPORTED_MODES \ > (BIT(TRIGGER_NETDEV_LINK_2500) | BIT(TRIGGER_NETDEV_LINK_1000) | \ > diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c > index b5bcabab7a1d..97d195853818 100644 > --- a/drivers/net/ethernet/intel/igc/igc_main.c > +++ b/drivers/net/ethernet/intel/igc/igc_main.c > @@ -7070,6 +7070,8 @@ static int igc_probe(struct pci_dev *pdev, > goto err_register; > } > > + hw->mac.ledctl_default = rd32(IGC_LEDCTL); > + > return 0; > > err_register:
On 5/23/2024 1:44 AM, Przemek Kitszel wrote: > On 5/22/24 11:19, Vitaly Lifshits wrote: >> Add support for ethtool.set_phys_id callback to initiate LED blinking >> and stopping them by the ethtool interface. >> This is done by storing the initial LEDCTL register value and restoring >> it when LED blinking is terminated. >> >> In addition, moved IGC_LEDCTL related defines from igc_leds.c to >> igc_defines.h where they can be included by all of the igc module >> files. >> >> Signed-off-by: Vitaly Lifshits <vitaly.lifshits@intel.com> >> Signed-off-by: Menachem Fogel <menachem.fogel@intel.com> > > sender's SoB should be last. > Should, by any chance, Menachem be marked as an author of this change? > > side note: we should CC netdev and obey netdev rules, like not sending > -next content during window being closed for new submissions > https://patchwork.hopto.org/net-next.html > > My understanding is that Intel Wired LAN is a little more relaxed with accepting iwl-next queued submissions even when net-next is closed. They of course won't get sent until the merge window opens, but we can queue and validate them internally during this time. I guess since we started Cc'ing both intel-wired-lan and netdev, that might not be the case anymore? Certainly in cases like this (where netdev isn't even on the cc at all) it does not matter.
On 5/22/2024 12:19, Vitaly Lifshits wrote: > Add support for ethtool.set_phys_id callback to initiate LED blinking > and stopping them by the ethtool interface. > This is done by storing the initial LEDCTL register value and restoring > it when LED blinking is terminated. > > In addition, moved IGC_LEDCTL related defines from igc_leds.c to > igc_defines.h where they can be included by all of the igc module > files. > > Signed-off-by: Vitaly Lifshits <vitaly.lifshits@intel.com> > Signed-off-by: Menachem Fogel <menachem.fogel@intel.com> > --- > drivers/net/ethernet/intel/igc/igc_defines.h | 22 ++++++++++++++ > drivers/net/ethernet/intel/igc/igc_ethtool.c | 32 ++++++++++++++++++++ > drivers/net/ethernet/intel/igc/igc_hw.h | 2 ++ > drivers/net/ethernet/intel/igc/igc_leds.c | 21 +------------ > drivers/net/ethernet/intel/igc/igc_main.c | 2 ++ > 5 files changed, 59 insertions(+), 20 deletions(-) Tested-by: Naama Meir <naamax.meir@linux.intel.com>
diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h b/drivers/net/ethernet/intel/igc/igc_defines.h index 5f92b3c7c3d4..664d49f10427 100644 --- a/drivers/net/ethernet/intel/igc/igc_defines.h +++ b/drivers/net/ethernet/intel/igc/igc_defines.h @@ -686,4 +686,26 @@ #define IGC_LTRMAXV_LSNP_REQ 0x00008000 /* LTR Snoop Requirement */ #define IGC_LTRMAXV_SCALE_SHIFT 10 +/* LED ctrl defines */ +#define IGC_NUM_LEDS 3 + +#define IGC_LEDCTL_GLOBAL_BLINK_MODE BIT(5) +#define IGC_LEDCTL_LED0_MODE_SHIFT 0 +#define IGC_LEDCTL_LED0_MODE_MASK GENMASK(3, 0) +#define IGC_LEDCTL_LED0_BLINK BIT(7) +#define IGC_LEDCTL_LED1_MODE_SHIFT 8 +#define IGC_LEDCTL_LED1_MODE_MASK GENMASK(11, 8) +#define IGC_LEDCTL_LED1_BLINK BIT(15) +#define IGC_LEDCTL_LED2_MODE_SHIFT 16 +#define IGC_LEDCTL_LED2_MODE_MASK GENMASK(19, 16) +#define IGC_LEDCTL_LED2_BLINK BIT(23) + +#define IGC_LEDCTL_MODE_ON 0x00 +#define IGC_LEDCTL_MODE_OFF 0x01 +#define IGC_LEDCTL_MODE_LINK_10 0x05 +#define IGC_LEDCTL_MODE_LINK_100 0x06 +#define IGC_LEDCTL_MODE_LINK_1000 0x07 +#define IGC_LEDCTL_MODE_LINK_2500 0x08 +#define IGC_LEDCTL_MODE_ACTIVITY 0x0b + #endif /* _IGC_DEFINES_H_ */ diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c index f2c4f1966bb0..82ece5f95f1e 100644 --- a/drivers/net/ethernet/intel/igc/igc_ethtool.c +++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c @@ -1975,6 +1975,37 @@ static void igc_ethtool_diag_test(struct net_device *netdev, msleep_interruptible(4 * 1000); } +static int igc_ethtool_set_phys_id(struct net_device *netdev, + enum ethtool_phys_id_state state) +{ + struct igc_adapter *adapter = netdev_priv(netdev); + struct igc_hw *hw = &adapter->hw; + u32 ledctl; + + switch (state) { + case ETHTOOL_ID_ACTIVE: + ledctl = rd32(IGC_LEDCTL); + + /* initiate LED1 blinking */ + ledctl &= ~(IGC_LEDCTL_GLOBAL_BLINK_MODE | + IGC_LEDCTL_LED1_MODE_MASK | + IGC_LEDCTL_LED2_MODE_MASK); + ledctl |= IGC_LEDCTL_LED1_BLINK; + wr32(IGC_LEDCTL, ledctl); + break; + + case ETHTOOL_ID_INACTIVE: + /* restore LEDCTL default value */ + wr32(IGC_LEDCTL, hw->mac.ledctl_default); + break; + + default: + break; + } + + return 0; +} + static const struct ethtool_ops igc_ethtool_ops = { .supported_coalesce_params = ETHTOOL_COALESCE_USECS, .get_drvinfo = igc_ethtool_get_drvinfo, @@ -2013,6 +2044,7 @@ static const struct ethtool_ops igc_ethtool_ops = { .get_link_ksettings = igc_ethtool_get_link_ksettings, .set_link_ksettings = igc_ethtool_set_link_ksettings, .self_test = igc_ethtool_diag_test, + .set_phys_id = igc_ethtool_set_phys_id, }; void igc_ethtool_set_ops(struct net_device *netdev) diff --git a/drivers/net/ethernet/intel/igc/igc_hw.h b/drivers/net/ethernet/intel/igc/igc_hw.h index e1c572e0d4ef..45b68695bdb7 100644 --- a/drivers/net/ethernet/intel/igc/igc_hw.h +++ b/drivers/net/ethernet/intel/igc/igc_hw.h @@ -95,6 +95,8 @@ struct igc_mac_info { bool autoneg; bool autoneg_failed; bool get_link_status; + + u32 ledctl_default; }; struct igc_nvm_operations { diff --git a/drivers/net/ethernet/intel/igc/igc_leds.c b/drivers/net/ethernet/intel/igc/igc_leds.c index 3929b25b6ae6..e5eeef240802 100644 --- a/drivers/net/ethernet/intel/igc/igc_leds.c +++ b/drivers/net/ethernet/intel/igc/igc_leds.c @@ -8,26 +8,7 @@ #include <uapi/linux/uleds.h> #include "igc.h" - -#define IGC_NUM_LEDS 3 - -#define IGC_LEDCTL_LED0_MODE_SHIFT 0 -#define IGC_LEDCTL_LED0_MODE_MASK GENMASK(3, 0) -#define IGC_LEDCTL_LED0_BLINK BIT(7) -#define IGC_LEDCTL_LED1_MODE_SHIFT 8 -#define IGC_LEDCTL_LED1_MODE_MASK GENMASK(11, 8) -#define IGC_LEDCTL_LED1_BLINK BIT(15) -#define IGC_LEDCTL_LED2_MODE_SHIFT 16 -#define IGC_LEDCTL_LED2_MODE_MASK GENMASK(19, 16) -#define IGC_LEDCTL_LED2_BLINK BIT(23) - -#define IGC_LEDCTL_MODE_ON 0x00 -#define IGC_LEDCTL_MODE_OFF 0x01 -#define IGC_LEDCTL_MODE_LINK_10 0x05 -#define IGC_LEDCTL_MODE_LINK_100 0x06 -#define IGC_LEDCTL_MODE_LINK_1000 0x07 -#define IGC_LEDCTL_MODE_LINK_2500 0x08 -#define IGC_LEDCTL_MODE_ACTIVITY 0x0b +#include "igc_defines.h" #define IGC_SUPPORTED_MODES \ (BIT(TRIGGER_NETDEV_LINK_2500) | BIT(TRIGGER_NETDEV_LINK_1000) | \ diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c index b5bcabab7a1d..97d195853818 100644 --- a/drivers/net/ethernet/intel/igc/igc_main.c +++ b/drivers/net/ethernet/intel/igc/igc_main.c @@ -7070,6 +7070,8 @@ static int igc_probe(struct pci_dev *pdev, goto err_register; } + hw->mac.ledctl_default = rd32(IGC_LEDCTL); + return 0; err_register: