[dpdk-dev] [PATCH v3 02/10] vdpa/sfc: add support for device initialization

[Xia, Chenbo]

> -----Original Message-----
> From: Vijay Kumar Srivastava <vsrivast at xilinx.com>
> Sent: Tuesday, November 2, 2021 3:43 PM
> To: Xia, Chenbo <chenbo.xia at intel.com>; dev at dpdk.org
> Cc: maxime.coquelin at redhat.com; andrew.rybchenko at oktetlabs.ru; Praveen Kumar
> Jain <praveenj at xilinx.com>; Harpreet Singh Anand <hanand at xilinx.com>
> Subject: RE: [PATCH v3 02/10] vdpa/sfc: add support for device initialization
> 
> Hi Chenbo,
> 
> >-----Original Message-----
> >From: Xia, Chenbo <chenbo.xia at intel.com>
> >Sent: Monday, November 1, 2021 5:19 PM
> >To: Vijay Kumar Srivastava <vsrivast at xilinx.com>; dev at dpdk.org
> >Cc: maxime.coquelin at redhat.com; andrew.rybchenko at oktetlabs.ru; Vijay
> >Kumar Srivastava <vsrivast at xilinx.com>
> >Subject: RE: [PATCH v3 02/10] vdpa/sfc: add support for device initialization
> >
> >Hi Vijay,
> >
> >> -----Original Message-----
> >> From: Vijay Srivastava <vijay.srivastava at xilinx.com>
> >> Sent: Friday, October 29, 2021 10:47 PM
> >> To: dev at dpdk.org
> >> Cc: maxime.coquelin at redhat.com; Xia, Chenbo <chenbo.xia at intel.com>;
> >> andrew.rybchenko at oktetlabs.ru; Vijay Kumar Srivastava
> >> <vsrivast at xilinx.com>
> >> Subject: [PATCH v3 02/10] vdpa/sfc: add support for device
> >> initialization
> >>
> >> From: Vijay Kumar Srivastava <vsrivast at xilinx.com>
> >>
> >> Add HW initialization and vDPA device registration support.
> >>
> >> Signed-off-by: Vijay Kumar Srivastava <vsrivast at xilinx.com>
> >> Acked-by: Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>
> >> ---
> [SNIP]
> >> +sfc_vdpa_hw_init(struct sfc_vdpa_adapter *sva) {
> >> +	efx_bar_region_t mem_ebr;
> >> +	efx_nic_t *enp;
> >> +	int rc;
> >> +
> >> +	sfc_vdpa_log_init(sva, "entry");
> >> +
> >> +	sfc_vdpa_log_init(sva, "get family");
> >> +	rc = sfc_efx_family(sva->pdev, &mem_ebr, &sva->family);
> >> +	if (rc != 0)
> >> +		goto fail_family;
> >> +	sfc_vdpa_log_init(sva,
> >> +			  "family is %u, membar is %u,"
> >> +			  "function control window offset is %#" PRIx64,
> >> +			  sva->family, mem_ebr.ebr_index,
> >mem_ebr.ebr_offset);
> >
> >If ebr_idx is int, then %u -> %d. But if it's a bar index, its type should be
> >unsigned int and you should change the definition in sfc common code.
> Yes. It’s BAR index.
> Thanks for the catch. I agree that usage of 'unsigned int' looks better
> for BAR index, however it should result in more changes in similar
> places. Is it OK if we use %d here right now to be consistent with
> current type and address the review note in a follow up patches
> for common/sfc_efx, net/sfc and vdpa/sfc?

Sure. Ok for me.

/Chenbo