[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-fpga
Subject:    RE: [PATCH v2] fpga: dfl: Allow Port to be linked to FME's DFL
From:       "Zhang, Tianfei" <tianfei.zhang () intel ! com>
Date:       2022-04-26 2:29:25
Message-ID: BN9PR11MB548385FAA79C03AEF3DC2960E3FB9 () BN9PR11MB5483 ! namprd11 ! prod ! outlook ! com
[Download RAW message or body]



> -----Original Message-----
> From: Wu, Hao <hao.wu@intel.com>
> Sent: Tuesday, April 26, 2022 10:23 AM
> To: Xu, Yilun <yilun.xu@intel.com>; Zhang, Tianfei <tianfei.zhang@intel.com>
> Cc: trix@redhat.com; mdf@kernel.org; linux-fpga@vger.kernel.org; Matthew
> Gerlach <matthew.gerlach@linux.intel.com>
> Subject: RE: [PATCH v2] fpga: dfl: Allow Port to be linked to FME's DFL
> 
> > On Tue, Apr 19, 2022 at 03:52:24AM -0400, Tianfei Zhang wrote:
> > > From: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > > 
> > > Currently we use PORTn_OFFSET to locate PORT DFLs, and PORT DFLs are
> > > not connected FME DFL. But for some cases (e.g. Intel Open FPGA
> > > Stack device), PORT DFLs are connected to FME DFL directly, so we
> > > don't need to search PORT DFLs via PORTn_OFFSET again. If BAR value
> > > of PORTn_OFFSET is 0x7
> > > (FME_PORT_OFST_BAR_SKIP) then driver will skip searching the DFL for
> > > that port. If BAR value is invalid, return -EINVAL.
> > > 
> > > Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > > Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
> > > ---
> > > v2: return -EINVAL if bar number invalid.
> > > ---
> > > drivers/fpga/dfl-pci.c | 11 +++++++++++
> > > drivers/fpga/dfl.h     |  1 +
> > > 2 files changed, 12 insertions(+)
> > > 
> > > diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c index
> > > 86ed9e4223d3..5bd6ef231ccc 100644
> > > --- a/drivers/fpga/dfl-pci.c
> > > +++ b/drivers/fpga/dfl-pci.c
> > > @@ -263,6 +263,17 @@ static int find_dfls_by_default(struct pci_dev
> *pcidev,
> > > 			 */
> > > 			bar = FIELD_GET(FME_PORT_OFST_BAR_ID, v);
> > > 			offset = FIELD_GET(FME_PORT_OFST_DFH_OFST, v);
> > > +			if (bar == FME_PORT_OFST_BAR_SKIP) {
> > > +				dev_dbg(&pcidev->dev, "skipping search DFL
> > for port %d on BAR %d\n",
> > > +					i, bar);
> > 
> > I suggest we remove the dev_dbg, it's a normal case in DFL walking.
> > 
> > > +				continue;
> > > +			} else if (bar >= PCI_STD_NUM_BARS) {
> > > +				dev_err(&pcidev->dev, "bad BAR %d for
> > port %d\n",
> > > +					bar, i);
> > > +				ret = -EINVAL;
> > > +				break;
> > 
> > The code is workable, but I suggest we use goto instead of break for
> > error out.
> 
> Maybe "break" is better, if additional code is required after the loop in the
> future, we don't have to modify the goto label code again. Just my personal
> preference.

Yes, Yilun suggested here is goto is better, because this is a hardware issue, it \
should be handle the error immediately.

> 
> > 
> > Thanks,
> > Yilun
> > 
> > > +			}
> > > +
> > > 			start = pci_resource_start(pcidev, bar) + offset;
> > > 			len = pci_resource_len(pcidev, bar) - offset;
> > > 
> > > diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h index
> > > 53572c7aced0..e0f0abfbeb8c 100644
> > > --- a/drivers/fpga/dfl.h
> > > +++ b/drivers/fpga/dfl.h
> > > @@ -91,6 +91,7 @@
> > > #define FME_HDR_PORT_OFST(n)	(0x38 + ((n) * 0x8))
> > > #define FME_HDR_BITSTREAM_ID	0x60
> > > #define FME_HDR_BITSTREAM_MD	0x68
> > > +#define FME_PORT_OFST_BAR_SKIP	7
> > > 
> > > /* FME Fab Capability Register Bitfield */
> > > #define FME_CAP_FABRIC_VERID	GENMASK_ULL(7, 0)	/* Fabric
> > version ID */
> > > --
> > > 2.26.2


[prev in list] [next in list] [prev in thread] [next in thread]