RE: [PATCH v3 2/2] fpga: dfl: look for vendor specific capability

From: Wu, Hao
Date: Wed Dec 02 2020 - 04:41:29 EST


> > > >
> > > > > + }
> > > > > +
> > > > > + offset = dfl_res & PCI_VNDR_DFLS_RES_OFF_MASK;
> > > > > + if (offset >= len) {
> > > > > + dev_err(&pcidev->dev, "%s bad
> offset %u >= %pa\n",
> > > > > + __func__, offset, &len);
> > > > > + return -EINVAL;
> > > > > + }
> > > > > +
> > > > > + dev_dbg(&pcidev->dev, "%s BAR %d offset 0x%x\n",
> > > > > __func__, bar, offset);
> > > > > +
> > > > > + len -= offset;
> > > > > +
> > > > > + start = pci_resource_start(pcidev, bar) + offset;
> > > > > +
> > > > > + dfl_fpga_enum_info_add_dfl(info, start, len);
> > > >
> > > > That means everytime, we pass [start, endofbar] region to dfl core
> > > > for enumeration, if there are multiple DFLs in one bar, then each range
> > > > ends at the same endofbar, it seems fine as enumeration can be done
> > > > one by one, but ideally the best case is that this capability can provide
> > > > end address or size too, right? It is possible that information can be
> > > > added to the capability as well? then we don't have such limitation.
> > > >
> > > > Hao
> > >
> > > I am not sure having more than one DFL in a bar serves any purpose over
> a
> > > single DFL. Regardless, I think the consistency of just having Offset/BIR
> > > in the VSEC is better than adding more infomation that has little or no
> > > added value.
> >
> > Agreed. Can't you just link the DFLs in that case?
>
> I didn't see the value of more DFLs in one bar either. So I think we'd better
> document it.

Yes, it needs to be documented well, otherwise users may have their own
choices, e.g. link 100 queues together by modify DFH registers of the
queues one by one, or just have them done together in the VSEC. I am not
sure which one is the easier way for logic developer, but at least we need to
document what driver can support.

Thanks
Hao

>
> Thanks,
> Yilun