Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: NKEkMgnOEX6gyctcOxl4TGnGGDOx6qtiSEty7Qd1xhzuoCxEs0bWUDAE4Td7IfovEUaUAjdRve
 n8i5yev8N1fw==
IronPort-SDR: MFTPXEUqxZsu4NobG6Xruu3TraKUQ1MD94yXs+uRuPlLtq+KoQorhUweEgiBnFMZZLvbO8A/tm
 /2bluF/uriBg==
Date:   Thu, 24 Jun 2021 11:01:20 +0800
From:   Xu Yilun <yilun.xu@intel.com>
To:     Martin =?iso-8859-1?Q?Hundeb=F8ll?= <mhu@silicom.dk>
Cc:     "Wu, Hao" <hao.wu@intel.com>, Tom Rix <trix@redhat.com>,
        Moritz Fischer <mdf@kernel.org>,
        Jean Delvare <jdelvare@suse.com>,
        Guenter Roeck <linux@roeck-us.net>,
        Lee Jones <lee.jones@linaro.org>,
        Mark Brown <broonie@kernel.org>,
        Martin =?iso-8859-1?Q?Hundeb=F8ll?= <mhu@geanix.com>,
        "linux-fpga@vger.kernel.org" <linux-fpga@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-hwmon@vger.kernel.org" <linux-hwmon@vger.kernel.org>,
        "linux-spi@vger.kernel.org" <linux-spi@vger.kernel.org>,
        Debarati Biswas <debaratix.biswas@intel.com>,
        "Weight, Russell H" <russell.h.weight@intel.com>
Subject: Re: [PATCH 2/4] fpga: dfl: Move DFH header register macros to
  linux/dfl.h
Message-ID: <20210624030120.GA42039@yilunxu-OptiPlex-7050>
References: <20210621070621.431482-1-mhu@silicom.dk>
 <20210621070621.431482-3-mhu@silicom.dk>
 <DM6PR11MB3819FE54D2C399DDBBC38FBA850A9@DM6PR11MB3819.namprd11.prod.outlook.com>
 <20210622052205.GB27046@yilunxu-OptiPlex-7050>
 <DM6PR11MB3819B9B2F357B9B03F7707B685099@DM6PR11MB3819.namprd11.prod.outlook.com>
 <0257dcaf-348a-375d-6ed8-657974208e30@silicom.dk>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <0257dcaf-348a-375d-6ed8-657974208e30@silicom.dk>
Precedence: bulk

On Wed, Jun 23, 2021 at 01:56:59PM +0200, Martin Hundeb?ll wrote:
> 
> 
> On 22/06/2021 09.39, Wu, Hao wrote:
> > > On Mon, Jun 21, 2021 at 06:19:15PM +0800, Wu, Hao wrote:
> > > > > Subject: [PATCH 2/4] fpga: dfl: Move DFH header register macros to
> > > linux/dfl.h
> > > > > 
> > > > > From: Debarati Biswas <debaratix.biswas@intel.com>
> > > > > 
> > > > > Device Feature List (DFL) drivers may be defined in subdirectories other
> > > > > than drivers/fpga, and each DFL driver should have access to the Device
> > > > > Feature Header (DFH) register, which contains revision and type
> > > > > information. This change moves the macros specific to the DFH register
> > > > > from drivers/fpga/dfl.h to include/linux/dfl.h.
> > > > 
> > > > Looks like it requires to access the revision info in the next patch, because
> > > > current dfl_device doesn't expose related information.
> > > > 
> > > > @Yilun, do you have any concern to expose those info via dfl_device?
> > > 
> > > Exposing these header register definitions are good to me. These registers
> > > are in DFL device's MMIO region, so it is good to share these info with
> > > all DFL drivers.
> > 
> > I mean expose revision via dfl_device, as dfl core already reads the DFL
> > header, it sounds duplicate read in each dfl device driver. And if we
> > consider this as a common need from dfl device driver, then the code
> > can be moved to a common place as well.
> > 
> > I hope from dfl device driver side, it doesn't need to know details of
> > how DFH register is defined, only simple way from dfl device data
> > structure or some simple helper function, then dfl device driver could
> > know all common information from DFH.
> > 
> > How do you think?

It's good idea.

> 
> struct dfl_device {} already has "u16 type" and "u16 feature_id", so it would make sense to add "u8 feature_rev" as well?

I think we may name it "u8 revision".

Thanks,
Yilun

> 
> // Martin