Re: [PATCH v4 3/4] fpga: dfl: add basic support DFHv1

Xu Yilun
2022-11-03T01:45:54Z

On 2022-11-01 at 15:37:19 -0700, matthew.gerlach@linux.intel.com wrote:
>
>
> On Sat, 29 Oct 2022, matthew.gerlach@linux.intel.com wrote:
>
> >
> > >
> > > > if (!finfo)
> > > > return -ENOMEM;
> > > >
> > > > + if (dfh_psize > 0) {
> > > > + memcpy_fromio(finfo->params,
> > > > + binfo->ioaddr + ofst + DFHv1_PARAM_HDR, dfh_psize);
> > > > + finfo->param_size = dfh_psize;
> > > > + }
> > > > +
> > > > finfo->fid = fid;
> > > > finfo->revision = revision;
> > > > - finfo->mmio_res.start = binfo->start + ofst;
> > > > - finfo->mmio_res.end = finfo->mmio_res.start + size - 1;
> > > > + finfo->dfh_version = dfh_ver;
> > > > finfo->mmio_res.flags = IORESOURCE_MEM;
> > > > - finfo->irq_base = irq_base;
> > > > - finfo->nr_irqs = nr_irqs;
> > > > + if (dfh_ver == 1) {
> > > > + v = readq(binfo->ioaddr + ofst + DFHv1_CSR_ADDR);
> > > > + if (v & DFHv1_CSR_ADDR_REL)
> > > > + finfo->mmio_res.start = v & ~DFHv1_CSR_ADDR_REL;
> > > > + else
> > > > + finfo->mmio_res.start = binfo->start + ofst +
> > > > + FIELD_GET(DFHv1_CSR_ADDR_MASK, v);
> > > > +
> > > > + v = readq(binfo->ioaddr + ofst + DFHv1_CSR_SIZE_GRP);
> > > > + finfo->mmio_res.end = finfo->mmio_res.start +
> > > > + FIELD_GET(DFHv1_CSR_SIZE_GRP_SIZE, v) - 1;
> > >
> > > So for dflv1, no feature header resource for dfl_device, is it a problem
> > > for dfl_uio? Does userspace driver need the raw feature header?
> > These are two very good questions. The dfl_uio driver question is
> > particularly relevent because user space is looking at the GUIDs.
> >
>
> In the case of dfl_uio driver, user space will definitely want to look at
> the feature header for the GUID and the parameters. Since DFHv1 can have
> the DFH header and the feature registers in non-contiguous memory locations,
> a resource for the dfl_device will be required. In earlier
> revisions of this patch set, a second resource was added called csr_res
> pointing to the feature's register while mmio_res pointed at the header.
> Do we just need better names or do we need an array of named resources?

Either is OK, you could also name a resource element in an array by
struct resource:name. But my concern is still no overlapping.

Thanks,
Yilun