On 2020/2/10 18:21, Leon Romanovsky wrote: > On Mon, Feb 10, 2020 at 05:48:05PM +0800, Weihang Li wrote: >> >> >> On 2020/2/10 17:25, Leon Romanovsky wrote: >>>>>> - if (!eq->bt_l0) >>>>>> - return -ENOMEM; >>>>>> - >>>>>> - eq->cur_eqe_ba = eq->l0_dma; >>>>>> - eq->nxt_eqe_ba = 0; >>>>>> + /* alloc a tmp list for storing eq buf address */ >>>>>> + ret = hns_roce_alloc_buf_list(®ion, &buf_list, 1); >>>>>> + if (ret) { >>>>>> + dev_err(hr_dev->dev, "alloc eq buf_list error\n"); >>>>> The same comment like we gave for bnxt driver, no dev_* prints inside >>>>> driver, use ibdev_*. >>>>> >>>>> Thanks >>>>> >>>> Hi Leon, >>>> >>>> map_eq_buf() is called before ib_register_device(), so we can't use >>>> ibdev_* here. >>> As long as map_eq_buf() is called after ib_alloc_device(), you will be fine. >>> >>> Thanks >> >> Hi Leon, >> >> eq is used to queue hardware event, it should be ready before hardware is initialized. >> So we can't call map_eq_buf() after ib_alloc_device(). > > How can it be that your newly added function has hns_roce_dev in the > signature and you didn't call to ib_alloc_device()? > > +static int map_eq_buf(struct hns_roce_dev *hr_dev, struct hns_roce_eq *eq, > + u32 page_shift) > > Thanks > Sorry, I confused ib_alloc_device() and ib_register_device(). What I was about to say is ib_register_device(). Order of these functions in hns driver is: 1. ib_alloc_device() 2. map_eq_buf() 3. ib_register_device() Refer to code in __ibdev_printk(): else if (ibdev) printk("%s%s: %pV", level, dev_name(&ibdev->dev), vaf); If we called ibdev_*() before ib_register_device(), it will print "null" for the device name. And I make a simple test, it will print like this: [ 41.400347] (null): -------------- This is a test!---------- Because map_eq_buf() should be finished before ib_register_device(), so I think we have to use dev_*() in it. >> >> Thanks >> Weihang >> >>> >>>> Thanks for your reminder, another patch that replace other dev_* in >>>> hns driver with ibdev_* is on preparing. >>>> >>>> Weihang >>>> >>>>> . >>>>> >>> . >>> >> > > . >