Re: [Intel-wired-lan] [PATCH iwl-next v1 1/1] igc: add support for ethtool.set_phys_id

[Przemek Kitszel <przemyslaw.kitszel@intel.com>]

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: