| Message ID | 1542961598-91107-3-git-send-email-oulijun@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | Misc fixes for hip08 | expand |
On Fri, Nov 23, 2018 at 04:26:36PM +0800, Lijun Ou wrote: > This patch refactors the code of implementing qp state transition. > > Signed-off-by: Lijun Ou <oulijun@huawei.com> > --- > drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 16 +--------------- > drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 19 +++++++++++++++++++ > 2 files changed, 20 insertions(+), 15 deletions(-) "Refactor the code" means that it improves something, like line count or readability. Sometimes, it is needed to remove code duplication, but in your case, you simply moved a couple of lines from one place to another. Thanks > > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > index 3528f2f..66d66e9 100644 > --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > @@ -3846,21 +3846,7 @@ static int hns_roce_v2_modify_qp(struct ib_qp *ibqp, > qpc_mask); > if (ret) > goto out; > - } else if ((cur_state == IB_QPS_RTS && new_state == IB_QPS_RTS) || > - (cur_state == IB_QPS_SQE && new_state == IB_QPS_RTS) || > - (cur_state == IB_QPS_RTS && new_state == IB_QPS_SQD) || > - (cur_state == IB_QPS_SQD && new_state == IB_QPS_SQD) || > - (cur_state == IB_QPS_SQD && new_state == IB_QPS_RTS) || > - (cur_state == IB_QPS_INIT && new_state == IB_QPS_RESET) || > - (cur_state == IB_QPS_RTR && new_state == IB_QPS_RESET) || > - (cur_state == IB_QPS_RTS && new_state == IB_QPS_RESET) || > - (cur_state == IB_QPS_ERR && new_state == IB_QPS_RESET) || > - (cur_state == IB_QPS_INIT && new_state == IB_QPS_ERR) || > - (cur_state == IB_QPS_RTR && new_state == IB_QPS_ERR) || > - (cur_state == IB_QPS_RTS && new_state == IB_QPS_ERR) || > - (cur_state == IB_QPS_SQD && new_state == IB_QPS_ERR) || > - (cur_state == IB_QPS_SQE && new_state == IB_QPS_ERR) || > - (cur_state == IB_QPS_ERR && new_state == IB_QPS_ERR)) { > + } else if (V2_QP_SUPPORT_STATE(cur_state, new_state)) { > /* Nothing */ > ; > } else { > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h > index e6e3d8fb..7b308ac 100644 > --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h > @@ -133,6 +133,25 @@ > (step_idx == 1 && hop_num == 1) || \ > (step_idx == 2 && hop_num == 2)) > > +#define V2_QP_SUPPORT_STATE(cur_state, new_state) \ > + ((cur_state == IB_QPS_RTS && new_state == IB_QPS_RTS) || \ > + (cur_state == IB_QPS_SQE && new_state == IB_QPS_RTS) || \ > + (cur_state == IB_QPS_RTS && new_state == IB_QPS_SQD) || \ > + (cur_state == IB_QPS_SQD && new_state == IB_QPS_SQD) || \ > + (cur_state == IB_QPS_SQD && new_state == IB_QPS_RTS) || \ > + (cur_state == IB_QPS_INIT && new_state == IB_QPS_RESET) || \ > + (cur_state == IB_QPS_RTR && new_state == IB_QPS_RESET) || \ > + (cur_state == IB_QPS_RTS && new_state == IB_QPS_RESET) || \ > + (cur_state == IB_QPS_ERR && new_state == IB_QPS_RESET) || \ > + (cur_state == IB_QPS_SQD && new_state == IB_QPS_RESET) || \ > + (cur_state == IB_QPS_SQE && new_state == IB_QPS_RESET) || \ > + (cur_state == IB_QPS_INIT && new_state == IB_QPS_ERR) || \ > + (cur_state == IB_QPS_RTR && new_state == IB_QPS_ERR) || \ > + (cur_state == IB_QPS_RTS && new_state == IB_QPS_ERR) || \ > + (cur_state == IB_QPS_SQD && new_state == IB_QPS_ERR) || \ > + (cur_state == IB_QPS_SQE && new_state == IB_QPS_ERR) || \ > + (cur_state == IB_QPS_ERR && new_state == IB_QPS_ERR)) > + > #define CMD_CSQ_DESC_NUM 1024 > #define CMD_CRQ_DESC_NUM 1024 > > -- > 1.9.1 >
On Fri, Nov 23, 2018 at 08:10:04PM +0200, Leon Romanovsky wrote: > On Fri, Nov 23, 2018 at 04:26:36PM +0800, Lijun Ou wrote: > > This patch refactors the code of implementing qp state transition. > > > > Signed-off-by: Lijun Ou <oulijun@huawei.com> > > drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 16 +--------------- > > drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 19 +++++++++++++++++++ > > 2 files changed, 20 insertions(+), 15 deletions(-) > > "Refactor the code" means that it improves something, like line count or readability. > Sometimes, it is needed to remove code duplication, but in your case, you simply moved a > couple of lines from one place to another. > > Thanks > > > > > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > > index 3528f2f..66d66e9 100644 > > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > > @@ -3846,21 +3846,7 @@ static int hns_roce_v2_modify_qp(struct ib_qp *ibqp, > > qpc_mask); > > if (ret) > > goto out; > > - } else if ((cur_state == IB_QPS_RTS && new_state == IB_QPS_RTS) || > > - (cur_state == IB_QPS_SQE && new_state == IB_QPS_RTS) || > > - (cur_state == IB_QPS_RTS && new_state == IB_QPS_SQD) || > > - (cur_state == IB_QPS_SQD && new_state == IB_QPS_SQD) || > > - (cur_state == IB_QPS_SQD && new_state == IB_QPS_RTS) || > > - (cur_state == IB_QPS_INIT && new_state == IB_QPS_RESET) || > > - (cur_state == IB_QPS_RTR && new_state == IB_QPS_RESET) || > > - (cur_state == IB_QPS_RTS && new_state == IB_QPS_RESET) || > > - (cur_state == IB_QPS_ERR && new_state == IB_QPS_RESET) || > > - (cur_state == IB_QPS_INIT && new_state == IB_QPS_ERR) || > > - (cur_state == IB_QPS_RTR && new_state == IB_QPS_ERR) || > > - (cur_state == IB_QPS_RTS && new_state == IB_QPS_ERR) || > > - (cur_state == IB_QPS_SQD && new_state == IB_QPS_ERR) || > > - (cur_state == IB_QPS_SQE && new_state == IB_QPS_ERR) || > > - (cur_state == IB_QPS_ERR && new_state == IB_QPS_ERR)) { > > + } else if (V2_QP_SUPPORT_STATE(cur_state, new_state)) { > > /* Nothing */ > > ; > > } else { > > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h > > index e6e3d8fb..7b308ac 100644 > > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h > > @@ -133,6 +133,25 @@ > > (step_idx == 1 && hop_num == 1) || \ > > (step_idx == 2 && hop_num == 2)) > > > > +#define V2_QP_SUPPORT_STATE(cur_state, new_state) \ > > + ((cur_state == IB_QPS_RTS && new_state == IB_QPS_RTS) || \ > > + (cur_state == IB_QPS_SQE && new_state == IB_QPS_RTS) || \ > > + (cur_state == IB_QPS_RTS && new_state == IB_QPS_SQD) || \ > > + (cur_state == IB_QPS_SQD && new_state == IB_QPS_SQD) || \ > > + (cur_state == IB_QPS_SQD && new_state == IB_QPS_RTS) || \ > > + (cur_state == IB_QPS_INIT && new_state == IB_QPS_RESET) || \ > > + (cur_state == IB_QPS_RTR && new_state == IB_QPS_RESET) || \ > > + (cur_state == IB_QPS_RTS && new_state == IB_QPS_RESET) || \ > > + (cur_state == IB_QPS_ERR && new_state == IB_QPS_RESET) || \ > > + (cur_state == IB_QPS_SQD && new_state == IB_QPS_RESET) || \ > > + (cur_state == IB_QPS_SQE && new_state == IB_QPS_RESET) || \ > > + (cur_state == IB_QPS_INIT && new_state == IB_QPS_ERR) || \ > > + (cur_state == IB_QPS_RTR && new_state == IB_QPS_ERR) || \ > > + (cur_state == IB_QPS_RTS && new_state == IB_QPS_ERR) || \ > > + (cur_state == IB_QPS_SQD && new_state == IB_QPS_ERR) || \ > > + (cur_state == IB_QPS_SQE && new_state == IB_QPS_ERR) || \ > > + (cur_state == IB_QPS_ERR && new_state == IB_QPS_ERR)) And please don't use macros if a static inline function will do, and don't use marcos wrongly by forgetting to enclose parameters with () Jason
在 2018/11/24 2:10, Leon Romanovsky 写道: > On Fri, Nov 23, 2018 at 04:26:36PM +0800, Lijun Ou wrote: >> This patch refactors the code of implementing qp state transition. >> >> Signed-off-by: Lijun Ou <oulijun@huawei.com> >> --- >> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 16 +--------------- >> drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 19 +++++++++++++++++++ >> 2 files changed, 20 insertions(+), 15 deletions(-) > "Refactor the code" means that it improves something, like line count or readability. > Sometimes, it is needed to remove code duplication, but in your case, you simply moved a > couple of lines from one place to another. > > Thanks Yes, I want to reduce the complexity by sourcemonitor. Maybe I could rewrite the commit. >> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c >> index 3528f2f..66d66e9 100644 >> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c >> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c >> @@ -3846,21 +3846,7 @@ static int hns_roce_v2_modify_qp(struct ib_qp *ibqp, >> qpc_mask); >> if (ret) >> goto out; >> - } else if ((cur_state == IB_QPS_RTS && new_state == IB_QPS_RTS) || >> - (cur_state == IB_QPS_SQE && new_state == IB_QPS_RTS) || >> - (cur_state == IB_QPS_RTS && new_state == IB_QPS_SQD) || >> - (cur_state == IB_QPS_SQD && new_state == IB_QPS_SQD) || >> - (cur_state == IB_QPS_SQD && new_state == IB_QPS_RTS) || >> - (cur_state == IB_QPS_INIT && new_state == IB_QPS_RESET) || >> - (cur_state == IB_QPS_RTR && new_state == IB_QPS_RESET) || >> - (cur_state == IB_QPS_RTS && new_state == IB_QPS_RESET) || >> - (cur_state == IB_QPS_ERR && new_state == IB_QPS_RESET) || >> - (cur_state == IB_QPS_INIT && new_state == IB_QPS_ERR) || >> - (cur_state == IB_QPS_RTR && new_state == IB_QPS_ERR) || >> - (cur_state == IB_QPS_RTS && new_state == IB_QPS_ERR) || >> - (cur_state == IB_QPS_SQD && new_state == IB_QPS_ERR) || >> - (cur_state == IB_QPS_SQE && new_state == IB_QPS_ERR) || >> - (cur_state == IB_QPS_ERR && new_state == IB_QPS_ERR)) { >> + } else if (V2_QP_SUPPORT_STATE(cur_state, new_state)) { >> /* Nothing */ >> ; >> } else { >> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h >> index e6e3d8fb..7b308ac 100644 >> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h >> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h >> @@ -133,6 +133,25 @@ >> (step_idx == 1 && hop_num == 1) || \ >> (step_idx == 2 && hop_num == 2)) >> >> +#define V2_QP_SUPPORT_STATE(cur_state, new_state) \ >> + ((cur_state == IB_QPS_RTS && new_state == IB_QPS_RTS) || \ >> + (cur_state == IB_QPS_SQE && new_state == IB_QPS_RTS) || \ >> + (cur_state == IB_QPS_RTS && new_state == IB_QPS_SQD) || \ >> + (cur_state == IB_QPS_SQD && new_state == IB_QPS_SQD) || \ >> + (cur_state == IB_QPS_SQD && new_state == IB_QPS_RTS) || \ >> + (cur_state == IB_QPS_INIT && new_state == IB_QPS_RESET) || \ >> + (cur_state == IB_QPS_RTR && new_state == IB_QPS_RESET) || \ >> + (cur_state == IB_QPS_RTS && new_state == IB_QPS_RESET) || \ >> + (cur_state == IB_QPS_ERR && new_state == IB_QPS_RESET) || \ >> + (cur_state == IB_QPS_SQD && new_state == IB_QPS_RESET) || \ >> + (cur_state == IB_QPS_SQE && new_state == IB_QPS_RESET) || \ >> + (cur_state == IB_QPS_INIT && new_state == IB_QPS_ERR) || \ >> + (cur_state == IB_QPS_RTR && new_state == IB_QPS_ERR) || \ >> + (cur_state == IB_QPS_RTS && new_state == IB_QPS_ERR) || \ >> + (cur_state == IB_QPS_SQD && new_state == IB_QPS_ERR) || \ >> + (cur_state == IB_QPS_SQE && new_state == IB_QPS_ERR) || \ >> + (cur_state == IB_QPS_ERR && new_state == IB_QPS_ERR)) >> + >> #define CMD_CSQ_DESC_NUM 1024 >> #define CMD_CRQ_DESC_NUM 1024 >> >> -- >> 1.9.1 >>
在 2018/11/24 2:17, Jason Gunthorpe 写道: > On Fri, Nov 23, 2018 at 08:10:04PM +0200, Leon Romanovsky wrote: >> On Fri, Nov 23, 2018 at 04:26:36PM +0800, Lijun Ou wrote: >>> This patch refactors the code of implementing qp state transition. >>> >>> Signed-off-by: Lijun Ou <oulijun@huawei.com> >>> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 16 +--------------- >>> drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 19 +++++++++++++++++++ >>> 2 files changed, 20 insertions(+), 15 deletions(-) >> "Refactor the code" means that it improves something, like line count or readability. >> Sometimes, it is needed to remove code duplication, but in your case, you simply moved a >> couple of lines from one place to another. >> >> Thanks >> >>> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c >>> index 3528f2f..66d66e9 100644 >>> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c >>> @@ -3846,21 +3846,7 @@ static int hns_roce_v2_modify_qp(struct ib_qp *ibqp, >>> qpc_mask); >>> if (ret) >>> goto out; >>> - } else if ((cur_state == IB_QPS_RTS && new_state == IB_QPS_RTS) || >>> - (cur_state == IB_QPS_SQE && new_state == IB_QPS_RTS) || >>> - (cur_state == IB_QPS_RTS && new_state == IB_QPS_SQD) || >>> - (cur_state == IB_QPS_SQD && new_state == IB_QPS_SQD) || >>> - (cur_state == IB_QPS_SQD && new_state == IB_QPS_RTS) || >>> - (cur_state == IB_QPS_INIT && new_state == IB_QPS_RESET) || >>> - (cur_state == IB_QPS_RTR && new_state == IB_QPS_RESET) || >>> - (cur_state == IB_QPS_RTS && new_state == IB_QPS_RESET) || >>> - (cur_state == IB_QPS_ERR && new_state == IB_QPS_RESET) || >>> - (cur_state == IB_QPS_INIT && new_state == IB_QPS_ERR) || >>> - (cur_state == IB_QPS_RTR && new_state == IB_QPS_ERR) || >>> - (cur_state == IB_QPS_RTS && new_state == IB_QPS_ERR) || >>> - (cur_state == IB_QPS_SQD && new_state == IB_QPS_ERR) || >>> - (cur_state == IB_QPS_SQE && new_state == IB_QPS_ERR) || >>> - (cur_state == IB_QPS_ERR && new_state == IB_QPS_ERR)) { >>> + } else if (V2_QP_SUPPORT_STATE(cur_state, new_state)) { >>> /* Nothing */ >>> ; >>> } else { >>> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h >>> index e6e3d8fb..7b308ac 100644 >>> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h >>> @@ -133,6 +133,25 @@ >>> (step_idx == 1 && hop_num == 1) || \ >>> (step_idx == 2 && hop_num == 2)) >>> >>> +#define V2_QP_SUPPORT_STATE(cur_state, new_state) \ >>> + ((cur_state == IB_QPS_RTS && new_state == IB_QPS_RTS) || \ >>> + (cur_state == IB_QPS_SQE && new_state == IB_QPS_RTS) || \ >>> + (cur_state == IB_QPS_RTS && new_state == IB_QPS_SQD) || \ >>> + (cur_state == IB_QPS_SQD && new_state == IB_QPS_SQD) || \ >>> + (cur_state == IB_QPS_SQD && new_state == IB_QPS_RTS) || \ >>> + (cur_state == IB_QPS_INIT && new_state == IB_QPS_RESET) || \ >>> + (cur_state == IB_QPS_RTR && new_state == IB_QPS_RESET) || \ >>> + (cur_state == IB_QPS_RTS && new_state == IB_QPS_RESET) || \ >>> + (cur_state == IB_QPS_ERR && new_state == IB_QPS_RESET) || \ >>> + (cur_state == IB_QPS_SQD && new_state == IB_QPS_RESET) || \ >>> + (cur_state == IB_QPS_SQE && new_state == IB_QPS_RESET) || \ >>> + (cur_state == IB_QPS_INIT && new_state == IB_QPS_ERR) || \ >>> + (cur_state == IB_QPS_RTR && new_state == IB_QPS_ERR) || \ >>> + (cur_state == IB_QPS_RTS && new_state == IB_QPS_ERR) || \ >>> + (cur_state == IB_QPS_SQD && new_state == IB_QPS_ERR) || \ >>> + (cur_state == IB_QPS_SQE && new_state == IB_QPS_ERR) || \ >>> + (cur_state == IB_QPS_ERR && new_state == IB_QPS_ERR)) > And please don't use macros if a static inline function will do, and > don't use marcos wrongly by forgetting to enclose parameters with () > > Jason > > . Hi, jason if use static function, it will add complexity by sourcemonitor tool for new function. thanks >
On Fri, Dec 07, 2018 at 04:46:32PM +0800, oulijun wrote: > 在 2018/11/24 2:17, Jason Gunthorpe 写道: > > On Fri, Nov 23, 2018 at 08:10:04PM +0200, Leon Romanovsky wrote: > >> On Fri, Nov 23, 2018 at 04:26:36PM +0800, Lijun Ou wrote: > >>> This patch refactors the code of implementing qp state transition. > >>> > >>> Signed-off-by: Lijun Ou <oulijun@huawei.com> > >>> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 16 +--------------- > >>> drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 19 +++++++++++++++++++ > >>> 2 files changed, 20 insertions(+), 15 deletions(-) > >> "Refactor the code" means that it improves something, like line count or readability. > >> Sometimes, it is needed to remove code duplication, but in your case, you simply moved a > >> couple of lines from one place to another. > >> > >> Thanks > >> > >>> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > >>> index 3528f2f..66d66e9 100644 > >>> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > >>> @@ -3846,21 +3846,7 @@ static int hns_roce_v2_modify_qp(struct ib_qp *ibqp, > >>> qpc_mask); > >>> if (ret) > >>> goto out; > >>> - } else if ((cur_state == IB_QPS_RTS && new_state == IB_QPS_RTS) || > >>> - (cur_state == IB_QPS_SQE && new_state == IB_QPS_RTS) || > >>> - (cur_state == IB_QPS_RTS && new_state == IB_QPS_SQD) || > >>> - (cur_state == IB_QPS_SQD && new_state == IB_QPS_SQD) || > >>> - (cur_state == IB_QPS_SQD && new_state == IB_QPS_RTS) || > >>> - (cur_state == IB_QPS_INIT && new_state == IB_QPS_RESET) || > >>> - (cur_state == IB_QPS_RTR && new_state == IB_QPS_RESET) || > >>> - (cur_state == IB_QPS_RTS && new_state == IB_QPS_RESET) || > >>> - (cur_state == IB_QPS_ERR && new_state == IB_QPS_RESET) || > >>> - (cur_state == IB_QPS_INIT && new_state == IB_QPS_ERR) || > >>> - (cur_state == IB_QPS_RTR && new_state == IB_QPS_ERR) || > >>> - (cur_state == IB_QPS_RTS && new_state == IB_QPS_ERR) || > >>> - (cur_state == IB_QPS_SQD && new_state == IB_QPS_ERR) || > >>> - (cur_state == IB_QPS_SQE && new_state == IB_QPS_ERR) || > >>> - (cur_state == IB_QPS_ERR && new_state == IB_QPS_ERR)) { > >>> + } else if (V2_QP_SUPPORT_STATE(cur_state, new_state)) { > >>> /* Nothing */ > >>> ; > >>> } else { > >>> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h > >>> index e6e3d8fb..7b308ac 100644 > >>> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h > >>> @@ -133,6 +133,25 @@ > >>> (step_idx == 1 && hop_num == 1) || \ > >>> (step_idx == 2 && hop_num == 2)) > >>> > >>> +#define V2_QP_SUPPORT_STATE(cur_state, new_state) \ > >>> + ((cur_state == IB_QPS_RTS && new_state == IB_QPS_RTS) || \ > >>> + (cur_state == IB_QPS_SQE && new_state == IB_QPS_RTS) || \ > >>> + (cur_state == IB_QPS_RTS && new_state == IB_QPS_SQD) || \ > >>> + (cur_state == IB_QPS_SQD && new_state == IB_QPS_SQD) || \ > >>> + (cur_state == IB_QPS_SQD && new_state == IB_QPS_RTS) || \ > >>> + (cur_state == IB_QPS_INIT && new_state == IB_QPS_RESET) || \ > >>> + (cur_state == IB_QPS_RTR && new_state == IB_QPS_RESET) || \ > >>> + (cur_state == IB_QPS_RTS && new_state == IB_QPS_RESET) || \ > >>> + (cur_state == IB_QPS_ERR && new_state == IB_QPS_RESET) || \ > >>> + (cur_state == IB_QPS_SQD && new_state == IB_QPS_RESET) || \ > >>> + (cur_state == IB_QPS_SQE && new_state == IB_QPS_RESET) || \ > >>> + (cur_state == IB_QPS_INIT && new_state == IB_QPS_ERR) || \ > >>> + (cur_state == IB_QPS_RTR && new_state == IB_QPS_ERR) || \ > >>> + (cur_state == IB_QPS_RTS && new_state == IB_QPS_ERR) || \ > >>> + (cur_state == IB_QPS_SQD && new_state == IB_QPS_ERR) || \ > >>> + (cur_state == IB_QPS_SQE && new_state == IB_QPS_ERR) || \ > >>> + (cur_state == IB_QPS_ERR && new_state == IB_QPS_ERR)) > > And please don't use macros if a static inline function will do, and > > don't use marcos wrongly by forgetting to enclose parameters with () > > > > Jason > > > > . > Hi, jason > if use static function, it will add complexity by sourcemonitor tool for new function. No idea what that is. 'do not write functions as macros' trumps any tool you may be using. Jason
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c index 3528f2f..66d66e9 100644 --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c @@ -3846,21 +3846,7 @@ static int hns_roce_v2_modify_qp(struct ib_qp *ibqp, qpc_mask); if (ret) goto out; - } else if ((cur_state == IB_QPS_RTS && new_state == IB_QPS_RTS) || - (cur_state == IB_QPS_SQE && new_state == IB_QPS_RTS) || - (cur_state == IB_QPS_RTS && new_state == IB_QPS_SQD) || - (cur_state == IB_QPS_SQD && new_state == IB_QPS_SQD) || - (cur_state == IB_QPS_SQD && new_state == IB_QPS_RTS) || - (cur_state == IB_QPS_INIT && new_state == IB_QPS_RESET) || - (cur_state == IB_QPS_RTR && new_state == IB_QPS_RESET) || - (cur_state == IB_QPS_RTS && new_state == IB_QPS_RESET) || - (cur_state == IB_QPS_ERR && new_state == IB_QPS_RESET) || - (cur_state == IB_QPS_INIT && new_state == IB_QPS_ERR) || - (cur_state == IB_QPS_RTR && new_state == IB_QPS_ERR) || - (cur_state == IB_QPS_RTS && new_state == IB_QPS_ERR) || - (cur_state == IB_QPS_SQD && new_state == IB_QPS_ERR) || - (cur_state == IB_QPS_SQE && new_state == IB_QPS_ERR) || - (cur_state == IB_QPS_ERR && new_state == IB_QPS_ERR)) { + } else if (V2_QP_SUPPORT_STATE(cur_state, new_state)) { /* Nothing */ ; } else { diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h index e6e3d8fb..7b308ac 100644 --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h @@ -133,6 +133,25 @@ (step_idx == 1 && hop_num == 1) || \ (step_idx == 2 && hop_num == 2)) +#define V2_QP_SUPPORT_STATE(cur_state, new_state) \ + ((cur_state == IB_QPS_RTS && new_state == IB_QPS_RTS) || \ + (cur_state == IB_QPS_SQE && new_state == IB_QPS_RTS) || \ + (cur_state == IB_QPS_RTS && new_state == IB_QPS_SQD) || \ + (cur_state == IB_QPS_SQD && new_state == IB_QPS_SQD) || \ + (cur_state == IB_QPS_SQD && new_state == IB_QPS_RTS) || \ + (cur_state == IB_QPS_INIT && new_state == IB_QPS_RESET) || \ + (cur_state == IB_QPS_RTR && new_state == IB_QPS_RESET) || \ + (cur_state == IB_QPS_RTS && new_state == IB_QPS_RESET) || \ + (cur_state == IB_QPS_ERR && new_state == IB_QPS_RESET) || \ + (cur_state == IB_QPS_SQD && new_state == IB_QPS_RESET) || \ + (cur_state == IB_QPS_SQE && new_state == IB_QPS_RESET) || \ + (cur_state == IB_QPS_INIT && new_state == IB_QPS_ERR) || \ + (cur_state == IB_QPS_RTR && new_state == IB_QPS_ERR) || \ + (cur_state == IB_QPS_RTS && new_state == IB_QPS_ERR) || \ + (cur_state == IB_QPS_SQD && new_state == IB_QPS_ERR) || \ + (cur_state == IB_QPS_SQE && new_state == IB_QPS_ERR) || \ + (cur_state == IB_QPS_ERR && new_state == IB_QPS_ERR)) + #define CMD_CSQ_DESC_NUM 1024 #define CMD_CRQ_DESC_NUM 1024
This patch refactors the code of implementing qp state transition. Signed-off-by: Lijun Ou <oulijun@huawei.com> --- drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 16 +--------------- drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 15 deletions(-)