Re: [PATCH 2/4] fpga: dfl: Move DFH header register macros to linux/dfl.h

[Martin Hundebøll]

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@xxxxxxxxx>
> > > >
> > > > 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?

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

// Martin