On 2018/12/5 2:35, Jason Gunthorpe wrote:
> On Sat, Nov 24, 2018 at 05:12:55PM +0800, Lijun Ou wrote:
>> From: Yangyang Li <liyangyang20@xxxxxxxxxx>
>>
>> This patch adds qpc timer and cqc timer allocation
>> support for hardware timeout retransmission in
>> kernel space driver.
>>
>> Signed-off-by: Yangyang Li <liyangyang20@xxxxxxxxxx>
>> drivers/infiniband/hw/hns/hns_roce_cmd.h | 8 +++
>> drivers/infiniband/hw/hns/hns_roce_device.h | 28 ++++++++
>> drivers/infiniband/hw/hns/hns_roce_hem.c | 42 ++++++++++++
>> drivers/infiniband/hw/hns/hns_roce_hem.h | 2 +
>> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 103 +++++++++++++++++++++++++++-
>> drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 24 +++++++
>> drivers/infiniband/hw/hns/hns_roce_main.c | 36 ++++++++++
>> 7 files changed, 242 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/hw/hns/hns_roce_cmd.h b/drivers/infiniband/hw/hns/hns_roce_cmd.h
>> index 5348282..97c5f2c 100644
>> +++ b/drivers/infiniband/hw/hns/hns_roce_cmd.h
>> @@ -93,6 +93,14 @@ enum {
>> HNS_ROCE_CMD_READ_SCC_CTX_BT0 = 0xa4,
>> HNS_ROCE_CMD_WRITE_SCC_CTX_BT0 = 0xa5,
>>
>> + /* QPC TIMER commands */
>> + HNS_ROCE_CMD_WRITE_QPC_TIMER_BT0 = 0x33,
>> + HNS_ROCE_CMD_READ_QPC_TIMER_BT0 = 0x37,
>> +
>> + /* CQC TIMER commands */
>> + HNS_ROCE_CMD_WRITE_CQC_TIMER_BT0 = 0x23,
>> + HNS_ROCE_CMD_READ_CQC_TIMER_BT0 = 0x27,
>> +
>> /* EQC commands */
>> HNS_ROCE_CMD_CREATE_AEQC = 0x80,
>> HNS_ROCE_CMD_MODIFY_AEQC = 0x81,
>> diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
>> index 6fe1871..2a66929 100644
>> +++ b/drivers/infiniband/hw/hns/hns_roce_device.h
>> @@ -486,6 +486,13 @@ struct hns_roce_qp_table {
>> struct hns_roce_hem_table scc_ctx_table;
>> };
>>
>> +struct hns_roce_qpc_timer_table {
>> + struct hns_roce_bitmap bitmap;
>> + spinlock_t lock;
>> + struct radix_tree_root tree;
>
> Same comment as for the SRQ patches, lets use xarray instead of adding
> new radix_tree users
>
>> + struct hns_roce_hem_table table;
>> +};
>> +
>> struct hns_roce_cq_table {
>> struct hns_roce_bitmap bitmap;
>> spinlock_t lock;
>> @@ -499,6 +506,13 @@ struct hns_roce_srq_table {
>> struct hns_roce_hem_table table;
>> };
>>
>> +struct hns_roce_cqc_timer_table {
>> + struct hns_roce_bitmap bitmap;
>> + spinlock_t lock;
>> + struct radix_tree_root tree;
>
> Here too
>
> Although, I'm confused, this adds a new struct and a member called
> 'tree' but the word 'tree' does not appear in this patch otherwise. What is
> going on? Where is the user?
Hi, Jason.
Thanks a lot for your reply.
Only 'table' in struct hns_roce_cqc_timer_table was used in my driver.
'bitmap','lock' and 'tree' were not used.
So hns_roce_qpc_timer_table and hns_roce_cqc_timer_table should be deleted,
and replaced with struct hns_roce_hem_table.
Redundant code should not exist, I should pay more attention to this aspect.
Thanks.
>
> Jason
>
>
