Re: [PATCH v2 1/6] Documentation: fpga: dfl: Add documentation for DFHv1

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 4168 bytes --]

On Tue, 27 Sep 2022, matthew.gerlach@linux.intel.com wrote:

> 
> 
> On Fri, 23 Sep 2022, Ilpo Järvinen wrote:
> 
> > On Fri, 23 Sep 2022, matthew.gerlach@linux.intel.com wrote:
> > 
> > > From: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > > 
> > > Add documentation describing the extensions provided by Version
> > > 1 of the Device Feature Header (DFHv1).
> > > 
> > > Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > > ---
> > > v2: s/GUILD/GUID/
> > >     add picture
> > > ---
> > >  Documentation/fpga/dfl.rst | 49 ++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 49 insertions(+)
> > > 
> > > diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst
> > > index 15b670926084..7c786b75b498 100644
> > > --- a/Documentation/fpga/dfl.rst
> > > +++ b/Documentation/fpga/dfl.rst
> > > @@ -561,6 +561,55 @@ new DFL feature via UIO direct access, its feature id
> > > should be added to the
> > >  driver's id_table.
> > > 
> > > 
> > > +Extending the Device Feature Header - DFHv1
> > > +===========================================
> > > +The current 8 bytes of the Device Feature Header, hereafter referred to
> > > as
> > > +to DFHv0, provide very little opportunity for the hardware to describe
> > > itself
> > > +to software. Version 1 of the Device Feature Header (DFHv1) is being
> > > introduced
> > > +to provide increased flexibility and extensibility to hardware designs
> > > using
> > > +Device Feature Lists.  The list below describes some of the goals behind
> > > the
> > > +changes in DFHv1:
> > > +
> > > +* Provide a standardized mechanism for features to describe
> > > +  parameters/capabilities to software.
> > > +* Standardize the use of a GUID for all DFHv1 types.
> > > +* Decouple the location of the DFH from the register space of the feature
> > > itself.
> > > +
> > > +Modeled after PCI Capabilities, DFHv1 Parameters provide a mechanism to
> > > associate
> > > +a list of parameter values to a particular feature.
> > > +
> > > +With DFHv0, not all features types contained a GUID.  DFHv1 makes the
> > > GUID standard
> > > +across all types.
> > > +
> > > +With DFHv0, the register map of a given feature is located immediately
> > > following
> > > +the DFHv0 in the memory space.  With DFHv1, the location of the feature
> > > register
> > > +map can be specified as an offset to the DFHv1 or as an absolute address.
> > > The DFHv1
> > > +structure is shown below:
> > > +
> > > +
> > > +-----------------------------------------------------------------------+
> > > +    |63 Type 60|59 DFH VER 52|51 Rsvd 41|40 EOL|39 Next 16|15 VER 12|11
> > > ID 0|
> > > +
> > > +-----------------------------------------------------------------------+
> > > +    |63                                 GUID_L
> > > 0|
> > > +
> > > +-----------------------------------------------------------------------+
> > > +    |63                                 GUID_H
> > > 0|
> > > +
> > > +-----------------------------------------------------------------------+
> > > +    |63                 Address/Offset                            1|  Rel
> > > 0|
> > > +
> > > +-----------------------------------------------------------------------+
> > 
> > Is something missing here given the layout is claimed (in 2/6) to be:
> > 
> > "DFHv1 Register Offset definitons
> > In DHFv1, DFH + GUID + CSR_START + CSR_SIZE_GROUP + PARAM_HDR + PARAM_DATA"
> > 
> > ?
> 
> 
> I was hesitant to have a picture because the description would then be in two
> places.  I suspect my picture is not clear, but it does line up with the
> offset definitions:
> 
> DFH offset 0x0
> GUID offsets 0x8 and 0x10
> CSR_START offset 0x18
> CSR_SIZE offset 0x20
> First PARAM_HDR, if it exists, is 0x28,
> First PARAM_DATA, if it exists, is 0x30.

I already noted in the other email I figured it out. It was thanks to 
the offsets in the header how I found out where I had misintepreted 
things. I initially had thought PARAM_DATA would be the parameters, both 
headers and param data, but then realized that it's PARAM_HDR+PARAM_DATA 
which is repeated n times.

I don't think there's need to fix anything in here.

-- 
 i.