Re: [PATCH for-next v4 03/12] RDMA/erdma: Add the hardware related definitions

[Cheng Xu <chengyou@linux.alibaba.com>]

On 3/18/22 6:27 PM, Wenpeng Liang wrote:
>> +
>> +struct erdma_cmdq_dereg_mr_req {
>> +	u64 hdr;
>> +	u32 cfg0;
>> +};
> 
> A single cfg may not need numbering.
> 
> cfg0->cfg
> 
>> +
>> +/* modify qp cfg0 */
> 
> Ditto.
> 
> cfg0->cfg
> 

Fine will fix.

>> +#define ERDMA_CMD_MODIFY_QP_STATE_MASK GENMASK(31, 24)
>> +#define ERDMA_CMD_MODIFY_QP_CC_MASK GENMASK(23, 20)
>> +#define ERDMA_CMD_MODIFY_QP_QPN_MASK GENMASK(19, 0)
>> +
>> +struct erdma_cmdq_modify_qp_req {
>> +	u64 hdr;
>> +	u32 cfg0;
>> +	u32 cookie;
>> +	u32 dip;
>> +	u32 sip;
>> +	u16 sport;
>> +	u16 dport;
>> +	u32 send_nxt;
>> +	u32 recv_nxt;
>> +};
> 
> Ditto.
> 
> cfg0->cfg
Will fix.

> 
>> +
>> +/* create qp cfg0 */
>> +#define ERDMA_CMD_CREATE_QP_SQ_DEPTH_MASK GENMASK(31, 20)
>> +#define ERDMA_CMD_CREATE_QP_QPN_MASK GENMASK(19, 0)
>> +
>> +/* create qp cfg1 */
>> +#define ERDMA_CMD_CREATE_QP_RQ_DEPTH_MASK GENMASK(31, 20)
>> +#define ERDMA_CMD_CREATE_QP_PD_MASK GENMASK(19, 0)
>> +
> 
>> +
>> +/* Receive Queue Element */
>> +struct erdma_rqe {
>> +	__le16 qe_idx;
>> +	__le16 rsvd;
>> +	__le32 qpn;
>> +	__le32 rsvd2;
>> +	__le32 rsvd3;
>> +	__le64 to;
>> +	__le32 length;
>> +	__le32 stag;
>> +};
> 
> Can these "rsvd" be numbered sequentially?
> 
> rsvd->rsvd0
> rsvd2->rsvd1
> rsvd3->rsvd2
> 

OK, I will pay more attention about this.

Thanks.

>> +
>> +struct erdma_readreq_sqe {
>> +	__le64 hdr;
>> +	__le32 invalid_stag;
>> +	__le32 length;
>> +	__le32 sink_stag;
>> +	__le32 sink_to_low;
>> +	__le32 sink_to_high;
>> +	__le32 rsvd0;
>> +};
> 
> A single rsvd may not need numbering.
> 
> rsvd0->rsvd
> 

Also will fix.
> 
>> +struct erdma_aeqe {
>> +	__le32 hdr;
>> +	__le32 event_data0;
>> +	__le32 event_data1;
>> +	__le32 rsvd2;
>> +};
> 
> Ditto.
> 
> rsvd2->rsvd

will fix.

Thanks,
Cheng Xu

> 
> Thanks,
> Wenpeng