Re: [PATCH v2 1/4] NTB : Introduce message library

[Logan Gunthorpe <logang@deltatee.com>]

Hey Atul,

Thanks for this work. I had a couple thoughts:

On 06/05/18 01:20 PM, Atul Raut wrote:
> +enum nt_cmd {
> +	NT_CMD_INVAL = -1,/* invalid spad command */
> +	NT_CMD_SSIZE = 0, /* send out buffer size */
> +	NT_CMD_RSIZE = 1, /* recv in  buffer size */
> +	NT_CMD_SXLAT = 2, /* send in  buffer xlat */
> +	NT_CMD_RXLAT = 3, /* recv out buffer xlat */
> +	NT_CMD_CLEAR = 4, /* clear allocated memory */
> +	NT_STS_DONE  = 5, /* init is done */
> +	NT_STS_LNKUP = 6, /* link up state flag */
> +	NT_QP_LINKS        = 7, /* available QP link */
> +	NT_CMD_NUM_MWS        = 8, /* number of memory windows */
> +	NT_CMD_NUM_QPS        = 9, /* number of QP */
> +	NT_CMD_NTB_VERSION    = 10, /* ntb version */
> +};
>

Should we really encode the type of data sent in the library? I feel
like a good API should be agnostic to what's being sent and allow for
generic data so future users can do different things and this library
doesn't has to list all types of values for all future clients.

> +/**
> + * Messages-base commands interface
> + */
> +#define NT_MSG_CMD		0
> +#define NT_MSG_CMD_WID	        1
> +#define NT_MSG_LDATA		2
> +#define NT_MSG_HDATA		3
> +#define NT_MSG_CNT		4

The current Switchtec hardware has 4 messages but the driver reserves
one for link management. So I'd rather not increase the number of
messages required from 3 to 4. Do we really need a cmd_wid message? Can
this not be encoded in the cmd message?

Logan