mlxsw: Fixes
Subsystems (3)
Petr Machata
maintainercover mlxsw: Fixes
This patchset fixes the following issues:
- During driver de-initialization the driver unregisters the EMAD response trap by setting its action to DISCARD. However the manual only permits TRAP and FORWARD, and future firmware versions will enforce this.
In patch #1, suppress the error message by aligning the driver to the manual and use a FORWARD (NOP) action when unregistering the trap.
- The driver queries the Management Capabilities Mask (MCAM) register during initialization to understand if certain features are supported.
However, not all firmware versions support this register, leading to the driver failing to load.
Patches #2 and #3 fix this issue by treating an error in the register query as an indication that the feature is not supported.
Ido Schimmel (3):
mlxsw: core: Unregister EMAD trap using FORWARD action
mlxsw: core_env: Fix driver initialization with old firmware
mlxsw: pci: Fix driver initialization with old firmware
drivers/net/ethernet/mellanox/mlxsw/core.c | 2 +- drivers/net/ethernet/mellanox/mlxsw/core_env.c | 10 ++++------ drivers/net/ethernet/mellanox/mlxsw/pci.c | 10 ++++------ 3 files changed, 9 insertions(+), 13 deletions(-)
Signature · 1 line
2.43.0
Petr Machata
maintainerPATCH 1/3 mlxsw: core: Unregister EMAD trap using FORWARD action
From: Ido Schimmel <[email protected]>
The device's manual (PRM - Programmer's Reference Manual) classifies the trap that is used to deliver EMAD responses as an "event trap". Among other things, it means that the only actions that can be associated with the trap are TRAP and FORWARD (NOP).
Currently, during driver de-initialization the driver unregisters the trap by setting its action to DISCARD, which violates the above guideline. Future firmware versions will prevent such misuses by returning an error. This does not prevent the driver from working, but an error will be printed to the kernel log during module removal / devlink reload:
mlxsw_spectrum 0000:03:00.0: Reg cmd access status failed (status=7(bad parameter)) mlxsw_spectrum 0000:03:00.0: Reg cmd access failed (reg_id=7003(hpkt),type=write)
Suppress the error message by aligning the driver to the manual and use a FORWARD (NOP) action when unregistering the trap.
--- drivers/net/ethernet/mellanox/mlxsw/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/mellanox/mlxsw/core.c +1 -1
@@ -849,7 +849,7 @@ static void mlxsw_emad_rx_listener_func(struct sk_buff *skb, u16 local_port,static const struct mlxsw_listener mlxsw_emad_rx_listener =MLXSW_RXL (mlxsw_emad_rx_listener_func,ETHEMAD ,TRAP_TO_CPU , false, -EMAD ,DISCARD ); +EMAD ,FORWARD );static int mlxsw_emad_tlv_enable (struct mlxsw_core *mlxsw_core) {
Signature · 1 line
2.43.0
Re: [PATCH net 1/3] mlxsw: core: Unregister EMAD trap using FORWARD action
Quoted 20 lines · Petr Machata
From: Ido Schimmel <[email protected]>
The device's manual (PRM - Programmer's Reference Manual) classifies the trap that is used to deliver EMAD responses as an "event trap". Among other things, it means that the only actions that can be associated with the trap are TRAP and FORWARD (NOP).
Currently, during driver de-initialization the driver unregisters the trap by setting its action to DISCARD, which violates the above guideline. Future firmware versions will prevent such misuses by returning an error. This does not prevent the driver from working, but an error will be printed to the kernel log during module removal / devlink reload:
mlxsw_spectrum0000:03:00.0: Reg cmd access status failed (status=7(bad parameter))mlxsw_spectrum0000:03:00.0: Reg cmd access failed (reg_id=7003(hpkt),type=write)Suppress the error message by aligning the driver to the manual and use a FORWARD (NOP) action when unregistering the trap.
Quoted 6 lines
Fixes 4ec14b7634b2 ("mlxsw: Add interface to access registers and process events")Cc Jiri Pirko <[email protected]>Cc Amit Cohen <[email protected]>Signed-off-by Ido Schimmel <[email protected]>Reviewed-by Petr Machata <[email protected]>Signed-off-by Petr Machata <[email protected]>
Petr Machata
maintainerPATCH 2/3 mlxsw: core_env: Fix driver initialization with old firmware
In reply to Petr MachataFrom: Ido Schimmel <[email protected]>
The driver queries the Management Capabilities Mask (MCAM) register during initialization to understand if it can read up to 128 bytes from transceiver modules.
However, not all firmware versions support this register, leading to the driver failing to load.
Fix by treating an error in the register query as an indication that the feature is not supported.
--- drivers/net/ethernet/mellanox/mlxsw/core_env.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
drivers/net/ethernet/mellanox/mlxsw/core_env.c +4 -6
@@ -1360,17 +1360,15 @@ static struct mlxsw_linecards_event_ops mlxsw_env_event_ops = {static int mlxsw_env_max_module_eeprom_len_query (struct mlxsw_env *mlxsw_env) {char mcam_pl[MLXSW_REG_MCAM_LEN ]; -bool mcia_128b_supported; +bool mcia_128b_supported = false;int err;mlxsw_reg_mcam_pack (mcam_pl,MLXSW_REG_MCAM_FEATURE_GROUP_ENHANCED_FEATURES ); err =mlxsw_reg_query (mlxsw_env->core,MLXSW_REG (mcam), mcam_pl); -if (err) -return err; - -mlxsw_reg_mcam_unpack (mcam_pl,MLXSW_REG_MCAM_MCIA_128B , - &mcia_128b_supported); +if (!err) +mlxsw_reg_mcam_unpack (mcam_pl,MLXSW_REG_MCAM_MCIA_128B , + &mcia_128b_supported); mlxsw_env->max_eeprom_len = mcia_128b_supported ?128 :48 ;
Signature · 1 line
2.43.0
Simon Horman
reviewerRe: [PATCH net 2/3] mlxsw: core_env: Fix driver initialization with old firmware
Quoted 36 lines · Petr Machata
From: Ido Schimmel <[email protected]>
The driver queries the Management Capabilities Mask (MCAM) register during initialization to understand if it can read up to 128 bytes from transceiver modules.
However, not all firmware versions support this register, leading to the driver failing to load.
Fix by treating an error in the register query as an indication that the feature is not supported.
Fixes 1f4aea1f72da ("mlxsw: core_env: Read transceiver module EEPROM in 128 bytes chunks")Reported-by Tim 'mithro' Ansell <[email protected]>Signed-off-by Ido Schimmel <[email protected]>Reviewed-by Petr Machata <[email protected]>Signed-off-by Petr Machata <[email protected]>--- drivers/net/ethernet/mellanox/mlxsw/core_env.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)drivers/net/ethernet/mellanox/mlxsw/core_env.c +1
-1@@ -1360,17 +1360,15 @@ static struct mlxsw_linecards_event_ops mlxsw_env_event_ops = {static int mlxsw_env_max_module_eeprom_len_query (struct mlxsw_env *mlxsw_env) {char mcam_pl[MLXSW_REG_MCAM_LEN ]; -bool mcia_128b_supported; +bool mcia_128b_supported = false;int err;mlxsw_reg_mcam_pack (mcam_pl,MLXSW_REG_MCAM_FEATURE_GROUP_ENHANCED_FEATURES ); err =mlxsw_reg_query (mlxsw_env->core,MLXSW_REG (mcam), mcam_pl);
drivers/net/ethernet/mellanox/mlxsw/core_env.c static struct mlxsw_linecards_event_ops mlxsw_env_event_ops = { · line 1373
Quoted 10 lines
drivers/net/ethernet/mellanox/mlxsw/core_env.c +3
-5-if (err) -return err; - -mlxsw_reg_mcam_unpack (mcam_pl,MLXSW_REG_MCAM_MCIA_128B , - &mcia_128b_supported); +if (!err) +mlxsw_reg_mcam_unpack (mcam_pl,MLXSW_REG_MCAM_MCIA_128B , + &mcia_128b_supported); mlxsw_env->max_eeprom_len = mcia_128b_supported ?128 :48 ;
Hi Petr and Ido,
This function now always returns 0.
Perhaps the return type can be updated to void?
Ido Schimmel
maintainerRe: [PATCH net 2/3] mlxsw: core_env: Fix driver initialization with old firmware
Quoted 2 lines · Simon Horman
This function now always returns 0.
Perhaps the return type can be updated to void?
OK, will change to void.
Thanks
Petr Machata
maintainerPATCH 3/3 mlxsw: pci: Fix driver initialization with old firmware
In reply to Petr MachataFrom: Ido Schimmel <[email protected]>
The driver queries the Management Capabilities Mask (MCAM) register during initialization to understand if a new and deeper reset flow is supported.
However, not all firmware versions support this register, leading to the driver failing to load.
Fix by treating an error in the register query as an indication that the feature is not supported.
--- drivers/net/ethernet/mellanox/mlxsw/pci.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
drivers/net/ethernet/mellanox/mlxsw/pci.c +4 -6
@@ -1545,7 +1545,7 @@ mlxsw_pci_reset(struct mlxsw_pci *mlxsw_pci, const struct pci_device_id *id) {struct pci_dev *pdev = mlxsw_pci->pdev;char mcam_pl[MLXSW_REG_MCAM_LEN ]; -bool pci_reset_supported; +bool pci_reset_supported = false;u32 sys_status;int err;
@@ -1563,11 +1563,9 @@ mlxsw_pci_reset(struct mlxsw_pci *mlxsw_pci, const struct pci_device_id *id)mlxsw_reg_mcam_pack (mcam_pl,MLXSW_REG_MCAM_FEATURE_GROUP_ENHANCED_FEATURES ); err =mlxsw_reg_query (mlxsw_pci->core,MLXSW_REG (mcam), mcam_pl); -if (err) -return err; - -mlxsw_reg_mcam_unpack (mcam_pl,MLXSW_REG_MCAM_PCI_RESET , - &pci_reset_supported); +if (!err) +mlxsw_reg_mcam_unpack (mcam_pl,MLXSW_REG_MCAM_PCI_RESET , + &pci_reset_supported);if (pci_reset_supported) {pci_dbg (pdev,"Starting PCI reset flow\n" );
Signature · 1 line
2.43.0
Re: [PATCH net 3/3] mlxsw: pci: Fix driver initialization with old firmware
Quoted 9 lines · Petr Machata
From: Ido Schimmel <[email protected]>
The driver queries the Management Capabilities Mask (MCAM) register during initialization to understand if a new and deeper reset flow is supported.
However, not all firmware versions support this register, leading to the driver failing to load.
Quoted 8 lines
Fix by treating an error in the register query as an indication that the feature is not supported.
Fixes f257c73e5356 ("mlxsw: pci: Add support for new reset flow")Reported-by Tim 'mithro' Ansell <[email protected]>Signed-off-by Ido Schimmel <[email protected]>Reviewed-by Petr Machata <[email protected]>Signed-off-by Petr Machata <[email protected]>
Re: [PATCH net 3/3] mlxsw: pci: Fix driver initialization with old firmware
In reply to Petr MachataQuoted 10 lines · Petr Machata
From: Ido Schimmel <[email protected]>
The driver queries the Management Capabilities Mask (MCAM) register during initialization to understand if a new and deeper reset flow is supported.
However, not all firmware versions support this register, leading to the driver failing to load.
Quoted 8 lines
Fix by treating an error in the register query as an indication that the feature is not supported.
Fixes f257c73e5356 ("mlxsw: pci: Add support for new reset flow")Reported-by Tim 'mithro' Ansell <[email protected]>Signed-off-by Ido Schimmel <[email protected]>Reviewed-by Petr Machata <[email protected]>Signed-off-by Petr Machata <[email protected]>
Quoted 36 lines
--- drivers/net/ethernet/mellanox/mlxsw/pci.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)drivers/net/ethernet/mellanox/mlxsw/pci.c +4
-7@@ -1545,7 +1545,7 @@ mlxsw_pci_reset(struct mlxsw_pci *mlxsw_pci, const struct pci_device_id *id) {struct pci_dev *pdev = mlxsw_pci->pdev;char mcam_pl[MLXSW_REG_MCAM_LEN ]; -bool pci_reset_supported; +bool pci_reset_supported = false;u32 sys_status;int err;@@ -1563,11 +1563,9 @@ mlxsw_pci_reset(struct mlxsw_pci *mlxsw_pci, const struct pci_device_id *id)mlxsw_reg_mcam_pack (mcam_pl,MLXSW_REG_MCAM_FEATURE_GROUP_ENHANCED_FEATURES ); err =mlxsw_reg_query (mlxsw_pci->core,MLXSW_REG (mcam), mcam_pl); -if (err) -return err; - -mlxsw_reg_mcam_unpack (mcam_pl,MLXSW_REG_MCAM_PCI_RESET , - &pci_reset_supported); +if (!err) +mlxsw_reg_mcam_unpack (mcam_pl,MLXSW_REG_MCAM_PCI_RESET , + &pci_reset_supported);if (pci_reset_supported) {pci_dbg (pdev,"Starting PCI reset flow\n" ); --2.43.0
Signature · 2 lines
Regards, Kalesh A P