Re: [PATCH net-next 04/11] net/mlx5_core: HW data structs/types definitions preparation for mlx5 ehternet driver

[Joe Perches <joe@perches.com>]

On Wed, 2015-04-08 at 17:51 +0300, Amir Vadai wrote:
[]
> - Update mlx5_ifc.h to include latest HW structs aligned to PRM rev 0.25

trivia

> diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
[]
> +   Autogenerated file.
> +   Date: 2015-03-09 14:03
> +   Source Document Name: Mellanox <Doc Name>
> +   Source Document Version: 0.24
> +   Generated by adb_to_c.py (EAT.ME Version: 1.0.70)
> +*/

That's a nice little detail to add.

>  #ifndef MLX5_IFC_H
>  #define MLX5_IFC_H
>  
>  enum {
> +	MLX5_EVENT_TYPE_CODING_COMPLETION_EVENTS                   = 0x0,
> +	MLX5_EVENT_TYPE_CODING_PATH_MIGRATED_SUCCEEDED             = 0x1,
> +	MLX5_EVENT_TYPE_CODING_COMMUNICATION_ESTABLISHED           = 0x2,
> +	MLX5_EVENT_TYPE_CODING_SEND_QUEUE_DRAINED                  = 0x3,
> +	MLX5_EVENT_TYPE_CODING_LAST_WQE_REACHED                    = 0x13,
> +	MLX5_EVENT_TYPE_CODING_SRQ_LIMIT                           = 0x14,
> +	MLX5_EVENT_TYPE_CODING_DCT_ALL_CONNECTIONS_CLOSED          = 0x1c,
> +	MLX5_EVENT_TYPE_CODING_DCT_ACCESS_KEY_VIOLATION            = 0x1d,
> +	MLX5_EVENT_TYPE_CODING_CQ_ERROR                            = 0x4,
> +	MLX5_EVENT_TYPE_CODING_LOCAL_WQ_CATASTROPHIC_ERROR         = 0x5,
> +	MLX5_EVENT_TYPE_CODING_PATH_MIGRATION_FAILED               = 0x7,
> +	MLX5_EVENT_TYPE_CODING_PAGE_FAULT_EVENT                    = 0xc,
> +	MLX5_EVENT_TYPE_CODING_INVALID_REQUEST_LOCAL_WQ_ERROR      = 0x10,
> +	MLX5_EVENT_TYPE_CODING_LOCAL_ACCESS_VIOLATION_WQ_ERROR     = 0x11,
> +	MLX5_EVENT_TYPE_CODING_LOCAL_SRQ_CATASTROPHIC_ERROR        = 0x12,
> +	MLX5_EVENT_TYPE_CODING_INTERNAL_ERROR                      = 0x8,
> +	MLX5_EVENT_TYPE_CODING_PORT_STATE_CHANGE                   = 0x9,
> +	MLX5_EVENT_TYPE_CODING_GPIO_EVENT                          = 0x15,
> +	MLX5_EVENT_TYPE_CODING_REMOTE_CONFIGURATION_PROTOCOL_EVENT = 0x19,
> +	MLX5_EVENT_TYPE_CODING_DOORBELL_BLUEFLAME_CONGESTION_EVENT = 0x1a,
> +	MLX5_EVENT_TYPE_CODING_STALL_VL_EVENT                      = 0x1b,
> +	MLX5_EVENT_TYPE_CODING_DROPPED_PACKET_LOGGED_EVENT         = 0x1f,
> +	MLX5_EVENT_TYPE_CODING_COMMAND_INTERFACE_COMPLETION        = 0xa,
> +	MLX5_EVENT_TYPE_CODING_PAGE_REQUEST                        = 0xb
> +};

Curious ordering and numbering.
There's no 0xd or 0xe, 0x16, 0x17, 0x18, 0x1e
Are those reserved?

> +struct mlx5_ifc_flow_table_fields_supported_bits {
> +	u8         outer_dmac[0x1];

This structure has one use, but so far that seems
unused.  Will this be used later?

Decimal is generally easier to read than hex.
The code is generally easier too if instead of an
array definition of size 1 it's a simple type.

If these are autogenerated, and length/position are important,
it could be nice to do something like:

struct <foo> {
	type	array1[decimal_size];	/* Hex offset */
	...
}

[]

> +struct mlx5_ifc_phys_layer_cntrs_bits {
> +	u8         time_since_last_clear_high[0x20];
> +

Why all the blank lines?

If this is autogenerated from a document with comments,
maybe the autogenerator could elide the blank lines too
when in a structure.