Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 3 Nov 2022 09:36:33 +0800
From:   Xu Yilun <yilun.xu@intel.com>
To:     matthew.gerlach@linux.intel.com
Cc:     hao.wu@intel.com, russell.h.weight@intel.com,
        basheer.ahmed.muddebihal@intel.com, trix@redhat.com,
        mdf@kernel.org, linux-fpga@vger.kernel.org,
        linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
        tianfei.zhang@intel.com, corbet@lwn.net, jirislaby@kernel.org,
        geert+renesas@glider.be, andriy.shevchenko@linux.intel.com,
        niklas.soderlund+renesas@ragnatech.se, macro@orcam.me.uk,
        johan@kernel.org, lukas@wunner.de, ilpo.jarvinen@linux.intel.com,
        marpagan@redhat.com
Subject: Re: [PATCH v4 3/4] fpga: dfl: add basic support DFHv1
Message-ID: <Y2MbIV5lJBgIsA4D@yilunxu-OptiPlex-7050>
References: <20221020212610.697729-1-matthew.gerlach@linux.intel.com>
 <20221020212610.697729-4-matthew.gerlach@linux.intel.com>
 <Y10l3NkIn0gsdVZq@yilunxu-OptiPlex-7050>
 <alpine.DEB.2.22.394.2210290739540.2504975@rhweight-WRK1>
 <alpine.DEB.2.22.394.2211011520290.2767909@rhweight-WRK1>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <alpine.DEB.2.22.394.2211011520290.2767909@rhweight-WRK1>
Precedence: bulk

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