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

[Xu Yilun <yilun.xu@intel.com>]

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