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: wRUGTyHbq727K0mV9z+2hFUsdZHFYOv9kRZS4MiSagESykDiYf7vXzU6hPKy9qnemx1wqxn+81
 6J10TRY+hWSA==
IronPort-SDR: iUCW69lMqmZAOi1zPlIC7+tmzJafz30CBu++hPNjoMxUjKwrGJfYICMbxDfeJXBT5xQOCXUDrS
 q3UDU0n37wfg==
From:   "Wu, Hao" <hao.wu@intel.com>
To:     "Xu, Yilun" <yilun.xu@intel.com>,
        =?iso-8859-1?Q?Martin_Hundeb=F8ll?= <mhu@silicom.dk>
CC:     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>,
        =?iso-8859-1?Q?Martin_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
Thread-Topic: [PATCH 2/4] fpga: dfl: Move DFH header register macros to
  linux/dfl.h
Thread-Index: AQHXaKX26fHcMFRuKUe6sbK+IT+SDqsikQtw
Date:   Thu, 24 Jun 2021 04:45:43 +0000
Message-ID: <DM6PR11MB3819B96AC0E54AF96EDE728485079@DM6PR11MB3819.namprd11.prod.outlook.com>
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>
 <20210624030120.GA42039@yilunxu-OptiPlex-7050>
In-Reply-To: <20210624030120.GA42039@yilunxu-OptiPlex-7050>
Accept-Language: en-US
Content-Language: en-US
dlp-product: dlpe-windows
dlp-reaction: no-action
dlp-version: 11.5.1.3
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?iso-8859-1?Q?qQbGLRI+FtlED4zrnnjYwWOdwPf24pBVkDvWCZshTDnIDkdlTLsY1fKG4a?=
 =?iso-8859-1?Q?9b1GVreiktRheAXZR5Zna/7WL82CzJawX9nD0CUHR0FdzhxNcUgLo6ee7l?=
 =?iso-8859-1?Q?oC6kYdFhk9gW3iXracq0YrxcB2EC67GEyDIxiAFRA1o3PxxZYIr9RFO3MS?=
 =?iso-8859-1?Q?kgcYkGJ/mPkXsSP9DaV69HnyLjvH7LkK0I4qdqQKZV1mp62AUXUAyjWC6+?=
 =?iso-8859-1?Q?KoX55PzOWuOIcqVP/yCZtwX11zyeCEbrYR2Ei0zFSDlsMFwpI5kvCkBgeZ?=
 =?iso-8859-1?Q?hSz0HVzfouo53+8s4RmD7XS7qQ34wd1WQGz6xKQB6ctt+y9ogWFy8JPjsE?=
 =?iso-8859-1?Q?oWShuLDEzjj/TIWSa2K4/v+ExoENMl95HBrTePi5vCebgpdX//JqeGtrZd?=
 =?iso-8859-1?Q?xT20pXDnsI2LC11f81uV3mBbzt+fTEU74vfzho2yNifjPn9pFAITsEH7rs?=
 =?iso-8859-1?Q?BzP4z/IT78Jx06kPXQeVdZ4ysx/zF1bkcugs7smVZU7Jtfs8XvDxGVfwm+?=
 =?iso-8859-1?Q?YhZEZ3dzsQrUvAq18BF9wT0SHVDukkHcIfTGeKJBs2l8Mv2jjNikTbDErq?=
 =?iso-8859-1?Q?1cLobDeMX1gRT5pkXu+nJzgkHvlCkjCbFgBQBf+BjEg1KW8Qq76anDY94k?=
 =?iso-8859-1?Q?JO8VYH7DAUw8s6YrpQxPX+tdlEe8h/XQK8z6FuMAwZ/HT/QUfulatGr6fH?=
 =?iso-8859-1?Q?Gzh0rzCsz6wpqoZM8bP6UqQki9xekRv5XdVbFRCgGwxwrZ044rBFB4qWtK?=
 =?iso-8859-1?Q?31EylzGkR7p/0YBNI0JwTTY/Wn5spIn+MMtFFrkA3xhWYiXtlIrO7Bd8vG?=
 =?iso-8859-1?Q?QGEOGza70pgVc2aqkgDcd89cyvQ/PmW07C1mVR1BpusidDXW3e2ZlOiiFW?=
 =?iso-8859-1?Q?H6aa47f+s0+UEIJZQwRPKlfhgkFTQWDkqXVXTdTNsCSclL66kHZwgOQ5tb?=
 =?iso-8859-1?Q?C2/0V4elhM0SHlQXwtTMU9TbVu6ua61kBUyWjbjP0fztkSFAJkUZk8grH1?=
 =?iso-8859-1?Q?FTwaqWlAuVahiCYO05pPdBwJ/akNwlQ4HJypIIbNKF8HhznEJdBTZDbEqg?=
 =?iso-8859-1?Q?BaPCNuwnE8L08+unozBLsuIkhdPSt8zGvdzOxmjE55ODCh7BeQfYT8VFYY?=
 =?iso-8859-1?Q?tcLcYOaMxcpretVwqKkpQ4nN47VSWlPt48DFvt4F8nng6c0OZm5SnaF49D?=
 =?iso-8859-1?Q?32AcSIRveGhgXYQEO7crnPHxuXETXm8c7xPn4uL8bpH327GZDvCruHdE4N?=
 =?iso-8859-1?Q?/raCdQB6WpyFX7h+izvUPliDiwl1L4Q7RGstRlV+asuqCDuhQkOPNN/ITF?=
 =?iso-8859-1?Q?Ce539TsJK6FVvCp3Kbkia9uc2hYjQmSjW8fuTx80DeLpJv82vY1q+NEyxK?=
 =?iso-8859-1?Q?D680iPUYqJ?=
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3819.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7d576d09-ef3b-4afe-cebe-08d936caed24
X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Jun 2021 04:45:43.7902
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 227P/dDrQRLlppF9atBynnnzngkTNmX1E+WCuXomEoadcgYL39lNqgMF7e+vHgzqMXMSD97GNdOjoM8ccS23YA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB2889
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> On Wed, Jun 23, 2021 at 01:56:59PM +0200, Martin Hundeb=F8ll 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 subdirector=
ies other
> > > > > > than drivers/fpga, and each DFL driver should have access to th=
e Device
> > > > > > Feature Header (DFH) register, which contains revision and type
> > > > > > information. This change moves the macros specific to the DFH r=
egister
> > > > > > from drivers/fpga/dfl.h to include/linux/dfl.h.
> > > > >
> > > > > Looks like it requires to access the revision info in the next pa=
tch,
> because
> > > > > current dfl_device doesn't expose related information.
> > > > >
> > > > > @Yilun, do you have any concern to expose those info via dfl_devi=
ce?
> > > >
> > > > Exposing these header register definitions are good to me. These re=
gisters
> > > > 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 o=
f
> > > how DFH register is defined, only simple way from dfl device data
> > > structure or some simple helper function, then dfl device driver coul=
d
> > > know all common information from DFH.
> > >
> > > How do you think?
>=20
> It's good idea.
>=20
> >
> > struct dfl_device {} already has "u16 type" and "u16 feature_id", so it=
 would
> make sense to add "u8 feature_rev" as well?
>=20
> I think we may name it "u8 revision".

Sounds good.

Then we don't have to expose detailed registers information to such header =
file.
dfl_device data structure + helper function should be enough. : )

Thanks
Hao

>=20
> Thanks,
> Yilun
>=20
> >
> > // Martin