Re: [PATCH net-next v2 4/4] qed*: Add devlink support for configuration attributes.

[Jakub Kicinski <jakub.kicinski@netronome.com>]

On Thu, 4 Jul 2019 06:20:11 -0700, Sudarsana Reddy Kalluru wrote:
> This patch adds implementation for devlink callbacks for reading and
> configuring the device attributes.
> 
> Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Signed-off-by: Ariel Elior <aelior@marvell.com>
> ---
>  Documentation/networking/devlink-params-qede.txt |  72 ++++++++
>  drivers/net/ethernet/qlogic/qed/qed_main.c       |  38 +++++
>  drivers/net/ethernet/qlogic/qede/qede.h          |   3 +
>  drivers/net/ethernet/qlogic/qede/qede_devlink.c  | 202 ++++++++++++++++++++++-
>  drivers/net/ethernet/qlogic/qede/qede_devlink.h  |  23 +++
>  include/linux/qed/qed_if.h                       |  16 ++
>  6 files changed, 353 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/networking/devlink-params-qede.txt
> 
> diff --git a/Documentation/networking/devlink-params-qede.txt b/Documentation/networking/devlink-params-qede.txt
> new file mode 100644
> index 0000000..f78a993
> --- /dev/null
> +++ b/Documentation/networking/devlink-params-qede.txt
> @@ -0,0 +1,72 @@
> +enable_sriov		[DEVICE, GENERIC]
> +			Configuration mode: Permanent
> +
> +iwarp_cmt		[DEVICE, DRIVER-SPECIFIC]
> +			Enable iWARP support over 100G device (CMT mode).
> +			Type: Boolean
> +			Configuration mode: runtime
> +
> +entity_id		[DEVICE, DRIVER-SPECIFIC]
> +			Set the entity ID value to be used for this device
> +			while reading/configuring the devlink attributes.
> +			Type: u8
> +			Configuration mode: runtime

Can you explain what this is?

> +device_capabilities	[DEVICE, DRIVER-SPECIFIC]
> +			Set the entity ID value to be used for this device
> +			while reading/configuring the devlink attributes.
> +			Type: u8
> +			Configuration mode: runtime

Looks like you copied the previous text here.

> +mf_mode			[DEVICE, DRIVER-SPECIFIC]
> +			Configure Multi Function mode for the device.
> +			Supported MF modes and the assoicated values are,
> +			    MF allowed(0), Default(1), SPIO4(2), NPAR1.0(3),
> +			    NPAR1.5(4), NPAR2.0(5), BD(6) and UFP(7)

NPAR should have a proper API in devlink port, what are the other modes?

> +			Type: u8
> +			Configuration mode: Permanent
> +
> +dcbx_mode		[PORT, DRIVER-SPECIFIC]
> +			Configure DCBX mode for the device.
> +			Supported dcbx modes are,
> +			    Disabled(0), IEEE(1), CEE(2) and Dynamic(3)
> +			Type: u8
> +			Configuration mode: Permanent

Why is this a permanent parameter?

> +preboot_oprom		[PORT, DRIVER-SPECIFIC]
> +			Enable Preboot Option ROM.
> +			Type: Boolean
> +			Configuration mode: Permanent

This should definitely not be a driver specific toggle.

> +preboot_boot_protocol	[PORT, DRIVER-SPECIFIC]
> +			Configure preboot Boot protocol.
> +			Possible values are,
> +			    PXE(0), iSCSI Boot(3), FCoE Boot(4) and NONE(7)
> +			Type: u8
> +			Configuration mode: Permanent

Ditto.

> +preboot_vlan		[PORT, DRIVER-SPECIFIC]
> +			Preboot VLAN.
> +			Type: u16
> +			Configuration mode: Permanent
> +
> +preboot_vlan_value	[PORT, DRIVER-SPECIFIC]
> +			Configure Preboot VLAN value.
> +			Type: u16
> +			Configuration mode: Permanent

And these.

> +mba_delay_time		[PORT, DRIVER-SPECIFIC]
> +			Configure MBA Delay Time. Supported range is [0-15].
> +			Type: u8
> +			Configuration mode: Permanent
> +
> +mba_setup_hot_key	[PORT, DRIVER-SPECIFIC]
> +			Configure MBA setup Hot Key. Possible values are,
> +			Ctrl S(0) and Ctrl B(1).
> +			Type: u8
> +			Configuration mode: Permanent
> +
> +mba_hide_setup_prompt	[PORT, DRIVER-SPECIFIC]
> +			Configure MBA hide setup prompt.
> +			Type: Boolean
> +			Configuration mode: Permanent