Re: [PATCH v2 3/3] platform/x86: Intel PMT Telemetry capability driver

[David E. Box]

On Fri, 2020-05-08 at 12:57 +0300, Andy Shevchenko wrote:
> On Fri, May 8, 2020 at 5:18 AM David E. Box <
> david.e.box@xxxxxxxxxxxxxxx> wrote:
> > PMT Telemetry is a capability of the Intel Platform Monitoring
> > Technology.
> > The Telemetry capability provides access to device telemetry
> > metrics that
> > provide hardware performance data to users from continuous, memory
> > mapped,
> > read-only register spaces.
> > 
> > Register mappings are not provided by the driver. Instead, a GUID
> > is read
> > from a header for each endpoint. The GUID identifies the device and
> > is to
> > be used with an XML, provided by the vendor, to discover the
> > available set
> > of metrics and their register mapping.  This allows firmware
> > updates to
> > modify the register space without needing to update the driver
> > every time
> > with new mappings. Firmware writes a new GUID in this case to
> > specify the
> > new mapping.  Software tools with access to the associated XML file
> > can
> > then interpret the changes.
> > 
> > This module manages access to all PMT Telemetry endpoints on a
> > system,
> > regardless of the device exporting them. It creates a pmt_telemetry
> > class
> > to manage the list. For each endpoint, sysfs files provide GUID and
> > size
> > information as well as a pointer to the parent device the telemetry
> > comes
> > from. Software may discover the association between endpoints and
> > devices
> > by iterating through the list in sysfs, or by looking for the
> > existence of
> 
> ABI needs documentation.

We will be releasing a Linux software spec for PMT. We are waiting on
public release of the PMT spec. For this patch we did document the
sysfs class ABI.

> 
> > the class folder under the device of interest.  A device node of
> > the same
> > name allows software to then map the telemetry space for direct
> > access.
> 
> ...
> 
> > +config INTEL_PMT_TELEM
> 
> TELEMETRY
> 
> ...
> 
> > +obj-$(CONFIG_INTEL_PMT_TELEM)          += intel_pmt_telem.o
> 
> telemetry
> 
> (Inside the file it's fine to have telem)
> 
> ...
> 
> > +       priv->dvsec = dev_get_platdata(&pdev->dev);
> > +       if (!priv->dvsec) {
> > +               dev_err(&pdev->dev, "Platform data not found\n");
> > +               return -ENODEV;
> > +       }
> 
> I don't see how is it being used?

Good catch :). This was initially used to pass the DVSEC info from the
pci device to the telemetry driver. But with changes all of the needed
info is now read from the driver's memory resource. It was unnoticed
that dvsec  fields are no longer used. Will remove in next version.

Okay on other comments.

David