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

[Cheng Xu <chengyou@xxxxxxxxxxxxxxxxx>]

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