diff mbox series

[RESEND,V2,3/6] platform/x86/intel: Move intel_pmt from MFD to Auxiliary Bus

Message ID 20211208015015.891275-4-david.e.box@linux.intel.com
State New
Headers show
Series Auxiliary bus driver support for Intel PCIe VSEC/DVSEC | expand

Commit Message

David E. Box Dec. 8, 2021, 1:50 a.m. UTC
Intel Platform Monitoring Technology (PMT) support is indicated by presence
of an Intel defined PCIe Designated Vendor Specific Extended Capabilities
(DVSEC) structure with a PMT specific ID. The current MFD implementation
creates child devices for each PMT feature, currently telemetry, watcher,
and crashlog. However DVSEC structures may also be used by Intel to
indicate support for other features. The Out Of Band Management Services
Module (OOBMSM) uses DVSEC to enumerate several features, including PMT.
In order to support them it is necessary to modify the intel_pmt driver to
handle the creation of the child devices more generically. To that end,
modify the driver to create child devices for any VSEC/DVSEC features on
supported devices (indicated by PCI ID).  Additionally, move the
implementation from MFD to the Auxiliary bus.  VSEC/DVSEC features are
really multifunctional PCI devices, not platform devices as MFD was
designed for. Auxiliary bus gives more flexibility by allowing the
definition of custom structures that can be shared between associated
auxiliary devices and the parent device. Also, rename the driver from
intel_pmt to intel_vsec to better reflect the purpose.

This series also removes the current runtime pm support which was not
complete to begin with. None of the current devices require runtime pm.
However the support will be replaced when a device is added that requires
it.

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Reviewed-by: Mark Gross <markgross@kernel.org>
---
V2
  - Clarify status of missing pm support in commit message
  - Clarify why auxiliary bus is preferred in commit message

 MAINTAINERS                                |  12 +-
 drivers/mfd/Kconfig                        |  10 -
 drivers/mfd/Makefile                       |   1 -
 drivers/mfd/intel_pmt.c                    | 261 -------------
 drivers/platform/x86/intel/Kconfig         |  11 +
 drivers/platform/x86/intel/Makefile        |   2 +
 drivers/platform/x86/intel/pmt/Kconfig     |   4 +-
 drivers/platform/x86/intel/pmt/class.c     |  21 +-
 drivers/platform/x86/intel/pmt/class.h     |   5 +-
 drivers/platform/x86/intel/pmt/crashlog.c  |  47 +--
 drivers/platform/x86/intel/pmt/telemetry.c |  46 +--
 drivers/platform/x86/intel/vsec.c          | 408 +++++++++++++++++++++
 drivers/platform/x86/intel/vsec.h          |  43 +++
 13 files changed, 536 insertions(+), 335 deletions(-)
 delete mode 100644 drivers/mfd/intel_pmt.c
 create mode 100644 drivers/platform/x86/intel/vsec.c
 create mode 100644 drivers/platform/x86/intel/vsec.h

Comments

Greg KH Dec. 8, 2021, 4:22 p.m. UTC | #1
On Tue, Dec 07, 2021 at 05:50:12PM -0800, David E. Box wrote:
> +static struct pci_driver intel_vsec_pci_driver = {
> +	.name = "intel_vsec",
> +	.id_table = intel_vsec_pci_ids,
> +	.probe = intel_vsec_pci_probe,
> +};

So when the PCI device is removed from the system you leak resources and
have dangling devices?

Why no PCI remove driver callback?

thanks,

greg k-h
David E. Box Dec. 8, 2021, 5:47 p.m. UTC | #2
On Wed, 2021-12-08 at 17:22 +0100, Greg KH wrote:
> On Tue, Dec 07, 2021 at 05:50:12PM -0800, David E. Box wrote:
> > +static struct pci_driver intel_vsec_pci_driver = {
> > +       .name = "intel_vsec",
> > +       .id_table = intel_vsec_pci_ids,
> > +       .probe = intel_vsec_pci_probe,
> > +};
> 
> So when the PCI device is removed from the system you leak resources and
> have dangling devices?

No.

> 
> Why no PCI remove driver callback?

After probe all resources are device managed. There's nothing to explicitly clean up. When the PCI
device is removed, all aux devices are automatically removed. This is the case for the SDSi driver
as well.

David

> 
> thanks,
> 
> greg k-h
Greg KH Dec. 8, 2021, 6:11 p.m. UTC | #3
On Wed, Dec 08, 2021 at 09:47:26AM -0800, David E. Box wrote:
> On Wed, 2021-12-08 at 17:22 +0100, Greg KH wrote:
> > On Tue, Dec 07, 2021 at 05:50:12PM -0800, David E. Box wrote:
> > > +static struct pci_driver intel_vsec_pci_driver = {
> > > +       .name = "intel_vsec",
> > > +       .id_table = intel_vsec_pci_ids,
> > > +       .probe = intel_vsec_pci_probe,
> > > +};
> > 
> > So when the PCI device is removed from the system you leak resources and
> > have dangling devices?
> 
> No.
> 
> > 
> > Why no PCI remove driver callback?
> 
> After probe all resources are device managed. There's nothing to explicitly clean up. When the PCI
> device is removed, all aux devices are automatically removed. This is the case for the SDSi driver
> as well.

Where is the "automatic cleanup" happening?  As this pci driver is bound
to the PCI device, when the device is removed, what is called in this
driver to remove the resources allocated in the probe callback?

confused,

greg k-h
David E. Box Dec. 8, 2021, 7:09 p.m. UTC | #4
On Wed, 2021-12-08 at 19:11 +0100, Greg KH wrote:
> On Wed, Dec 08, 2021 at 09:47:26AM -0800, David E. Box wrote:
> > On Wed, 2021-12-08 at 17:22 +0100, Greg KH wrote:
> > > On Tue, Dec 07, 2021 at 05:50:12PM -0800, David E. Box wrote:
> > > > +static struct pci_driver intel_vsec_pci_driver = {
> > > > +       .name = "intel_vsec",
> > > > +       .id_table = intel_vsec_pci_ids,
> > > > +       .probe = intel_vsec_pci_probe,
> > > > +};
> > > 
> > > So when the PCI device is removed from the system you leak resources and
> > > have dangling devices?
> > 
> > No.
> > 
> > > 
> > > Why no PCI remove driver callback?
> > 
> > After probe all resources are device managed. There's nothing to explicitly clean up. When the
> > PCI
> > device is removed, all aux devices are automatically removed. This is the case for the SDSi
> > driver
> > as well.
> 
> Where is the "automatic cleanup" happening?  As this pci driver is bound
> to the PCI device, when the device is removed, what is called in this
> driver to remove the resources allocated in the probe callback?
> 
> confused,

devm_add_action_or_reset(&pdev->dev, intel_vsec_remove_aux, auxdev)

intel_vsec_remove_aux() gets called when the PCI device is removed. It calls auxiliary_device_unit()
which in turn calls the auxdev release() function that cleans up resources.

When the auxdev is removed, all resources that were dev_m added by the SDSi driver are released too
which is why it has no remove() either. I'll add the tests that check this.

David

> 
> greg k-h
Greg KH Dec. 8, 2021, 7:21 p.m. UTC | #5
On Wed, Dec 08, 2021 at 11:09:48AM -0800, David E. Box wrote:
> On Wed, 2021-12-08 at 19:11 +0100, Greg KH wrote:
> > On Wed, Dec 08, 2021 at 09:47:26AM -0800, David E. Box wrote:
> > > On Wed, 2021-12-08 at 17:22 +0100, Greg KH wrote:
> > > > On Tue, Dec 07, 2021 at 05:50:12PM -0800, David E. Box wrote:
> > > > > +static struct pci_driver intel_vsec_pci_driver = {
> > > > > +       .name = "intel_vsec",
> > > > > +       .id_table = intel_vsec_pci_ids,
> > > > > +       .probe = intel_vsec_pci_probe,
> > > > > +};
> > > > 
> > > > So when the PCI device is removed from the system you leak resources and
> > > > have dangling devices?
> > > 
> > > No.
> > > 
> > > > 
> > > > Why no PCI remove driver callback?
> > > 
> > > After probe all resources are device managed. There's nothing to explicitly clean up. When the
> > > PCI
> > > device is removed, all aux devices are automatically removed. This is the case for the SDSi
> > > driver
> > > as well.
> > 
> > Where is the "automatic cleanup" happening?  As this pci driver is bound
> > to the PCI device, when the device is removed, what is called in this
> > driver to remove the resources allocated in the probe callback?
> > 
> > confused,
> 
> devm_add_action_or_reset(&pdev->dev, intel_vsec_remove_aux, auxdev)

Wow that is opaque.  Why not do it on remove instead?

> intel_vsec_remove_aux() gets called when the PCI device is removed. It calls auxiliary_device_unit()
> which in turn calls the auxdev release() function that cleans up resources.

Does this happen when the device is removed, or when the binding of
driver <-> device is removed?

> When the auxdev is removed, all resources that were dev_m added by the SDSi driver are released too
> which is why it has no remove() either. I'll add the tests that check this.

Please do so and document it well, as that is an odd "pattern".

thanks,

greg k-h
David E. Box Dec. 8, 2021, 9:30 p.m. UTC | #6
On Wed, 2021-12-08 at 20:21 +0100, Greg KH wrote:
> On Wed, Dec 08, 2021 at 11:09:48AM -0800, David E. Box wrote:
> > On Wed, 2021-12-08 at 19:11 +0100, Greg KH wrote:
> > > On Wed, Dec 08, 2021 at 09:47:26AM -0800, David E. Box wrote:
> > > > On Wed, 2021-12-08 at 17:22 +0100, Greg KH wrote:
> > > > > On Tue, Dec 07, 2021 at 05:50:12PM -0800, David E. Box wrote:
> > > > > > +static struct pci_driver intel_vsec_pci_driver = {
> > > > > > +       .name = "intel_vsec",
> > > > > > +       .id_table = intel_vsec_pci_ids,
> > > > > > +       .probe = intel_vsec_pci_probe,
> > > > > > +};
> > > > > 
> > > > > So when the PCI device is removed from the system you leak resources and
> > > > > have dangling devices?
> > > > 
> > > > No.
> > > > 
> > > > > 
> > > > > Why no PCI remove driver callback?
> > > > 
> > > > After probe all resources are device managed. There's nothing to explicitly clean up. When
> > > > the
> > > > PCI
> > > > device is removed, all aux devices are automatically removed. This is the case for the SDSi
> > > > driver
> > > > as well.
> > > 
> > > Where is the "automatic cleanup" happening?  As this pci driver is bound
> > > to the PCI device, when the device is removed, what is called in this
> > > driver to remove the resources allocated in the probe callback?
> > > 
> > > confused,
> > 
> > devm_add_action_or_reset(&pdev->dev, intel_vsec_remove_aux, auxdev)
> 
> Wow that is opaque.  Why not do it on remove instead?

This code is common for auxdev cleanup. AFAICT most auxiliary bus code is done by drivers that have
some other primary function. They clean up their primary function resources in remove, but they
clean up the auxdev using the method above. In this case the sole purpose of this driver is to
create the auxdev. There are no other resources beyond what the auxdev is using.

Adding runtime pm to the pci driver will change this. Remove will be needed then.

> 
> > intel_vsec_remove_aux() gets called when the PCI device is removed. It calls
> > auxiliary_device_unit()
> > which in turn calls the auxdev release() function that cleans up resources.
> 
> Does this happen when the device is removed, or when the binding of
> driver <-> device is removed?

It happens when the device is removed as tested by unbinding it.

> 
> > When the auxdev is removed, all resources that were dev_m added by the SDSi driver are released
> > too
> > which is why it has no remove() either. I'll add the tests that check this.
> 
> Please do so and document it well, as that is an odd "pattern".

Sure, but I don't think it's that odd in practice given what I already mentioned.

David

> 
> thanks,
> 
> greg k-h
Greg KH Dec. 21, 2021, 7:38 a.m. UTC | #7
On Wed, Dec 08, 2021 at 01:30:06PM -0800, David E. Box wrote:
> On Wed, 2021-12-08 at 20:21 +0100, Greg KH wrote:
> > On Wed, Dec 08, 2021 at 11:09:48AM -0800, David E. Box wrote:
> > > On Wed, 2021-12-08 at 19:11 +0100, Greg KH wrote:
> > > > On Wed, Dec 08, 2021 at 09:47:26AM -0800, David E. Box wrote:
> > > > > On Wed, 2021-12-08 at 17:22 +0100, Greg KH wrote:
> > > > > > On Tue, Dec 07, 2021 at 05:50:12PM -0800, David E. Box wrote:
> > > > > > > +static struct pci_driver intel_vsec_pci_driver = {
> > > > > > > +       .name = "intel_vsec",
> > > > > > > +       .id_table = intel_vsec_pci_ids,
> > > > > > > +       .probe = intel_vsec_pci_probe,
> > > > > > > +};
> > > > > > 
> > > > > > So when the PCI device is removed from the system you leak resources and
> > > > > > have dangling devices?
> > > > > 
> > > > > No.
> > > > > 
> > > > > > 
> > > > > > Why no PCI remove driver callback?
> > > > > 
> > > > > After probe all resources are device managed. There's nothing to explicitly clean up. When
> > > > > the
> > > > > PCI
> > > > > device is removed, all aux devices are automatically removed. This is the case for the SDSi
> > > > > driver
> > > > > as well.
> > > > 
> > > > Where is the "automatic cleanup" happening?  As this pci driver is bound
> > > > to the PCI device, when the device is removed, what is called in this
> > > > driver to remove the resources allocated in the probe callback?
> > > > 
> > > > confused,
> > > 
> > > devm_add_action_or_reset(&pdev->dev, intel_vsec_remove_aux, auxdev)
> > 
> > Wow that is opaque.  Why not do it on remove instead?
> 
> This code is common for auxdev cleanup. AFAICT most auxiliary bus code is done by drivers that have
> some other primary function. They clean up their primary function resources in remove, but they
> clean up the auxdev using the method above. In this case the sole purpose of this driver is to
> create the auxdev. There are no other resources beyond what the auxdev is using.
> 
> Adding runtime pm to the pci driver will change this. Remove will be needed then.

And who will notice that being required when that happens?

Why is there no runtime PM for this driver?  Do you not care about power
consumption?  :)

thanks,

greg k-h
David E. Box Dec. 21, 2021, 4:44 p.m. UTC | #8
On Tue, 2021-12-21 at 08:38 +0100, Greg KH wrote:
> On Wed, Dec 08, 2021 at 01:30:06PM -0800, David E. Box wrote:
> > On Wed, 2021-12-08 at 20:21 +0100, Greg KH wrote:
> > > On Wed, Dec 08, 2021 at 11:09:48AM -0800, David E. Box wrote:
> > > > On Wed, 2021-12-08 at 19:11 +0100, Greg KH wrote:
> > > > > On Wed, Dec 08, 2021 at 09:47:26AM -0800, David E. Box wrote:
> > > > > > On Wed, 2021-12-08 at 17:22 +0100, Greg KH wrote:
> > > > > > > On Tue, Dec 07, 2021 at 05:50:12PM -0800, David E. Box wrote:
> > > > > > > > +static struct pci_driver intel_vsec_pci_driver = {
> > > > > > > > +       .name = "intel_vsec",
> > > > > > > > +       .id_table = intel_vsec_pci_ids,
> > > > > > > > +       .probe = intel_vsec_pci_probe,
> > > > > > > > +};
> > > > > > > 
> > > > > > > So when the PCI device is removed from the system you leak
> > > > > > > resources and
> > > > > > > have dangling devices?
> > > > > > 
> > > > > > No.
> > > > > > 
> > > > > > > Why no PCI remove driver callback?
> > > > > > 
> > > > > > After probe all resources are device managed. There's nothing to
> > > > > > explicitly clean up. When
> > > > > > the
> > > > > > PCI
> > > > > > device is removed, all aux devices are automatically removed. This
> > > > > > is the case for the SDSi
> > > > > > driver
> > > > > > as well.
> > > > > 
> > > > > Where is the "automatic cleanup" happening?  As this pci driver is
> > > > > bound
> > > > > to the PCI device, when the device is removed, what is called in this
> > > > > driver to remove the resources allocated in the probe callback?
> > > > > 
> > > > > confused,
> > > > 
> > > > devm_add_action_or_reset(&pdev->dev, intel_vsec_remove_aux, auxdev)
> > > 
> > > Wow that is opaque.  Why not do it on remove instead?
> > 
> > This code is common for auxdev cleanup. AFAICT most auxiliary bus code is
> > done by drivers that have
> > some other primary function. They clean up their primary function resources
> > in remove, but they
> > clean up the auxdev using the method above. In this case the sole purpose of
> > this driver is to
> > create the auxdev. There are no other resources beyond what the auxdev is
> > using.
> > 
> > Adding runtime pm to the pci driver will change this. Remove will be needed
> > then.
> 
> And who will notice that being required when that happens?
> 
> Why is there no runtime PM for this driver?  Do you not care about power
> consumption?  :)

Of course. :)

There's a backlog of patches waiting for this series. One adds support for the
telemetry device (an auxdev) on the DG2 GPU. This device requires runtime pm in
order for the slot to go D3. But this also requires changes to the telemetry
driver in order for runtime pm to be handled correctly. These and other patches,
including a series to have all current aux drivers use the new drvdata helpers,
are waiting for this.

David

> 
> thanks,
> 
> greg k-h
Greg KH Dec. 21, 2021, 4:54 p.m. UTC | #9
On Tue, Dec 21, 2021 at 08:44:57AM -0800, David E. Box wrote:
> On Tue, 2021-12-21 at 08:38 +0100, Greg KH wrote:
> > On Wed, Dec 08, 2021 at 01:30:06PM -0800, David E. Box wrote:
> > > On Wed, 2021-12-08 at 20:21 +0100, Greg KH wrote:
> > > > On Wed, Dec 08, 2021 at 11:09:48AM -0800, David E. Box wrote:
> > > > > On Wed, 2021-12-08 at 19:11 +0100, Greg KH wrote:
> > > > > > On Wed, Dec 08, 2021 at 09:47:26AM -0800, David E. Box wrote:
> > > > > > > On Wed, 2021-12-08 at 17:22 +0100, Greg KH wrote:
> > > > > > > > On Tue, Dec 07, 2021 at 05:50:12PM -0800, David E. Box wrote:
> > > > > > > > > +static struct pci_driver intel_vsec_pci_driver = {
> > > > > > > > > +       .name = "intel_vsec",
> > > > > > > > > +       .id_table = intel_vsec_pci_ids,
> > > > > > > > > +       .probe = intel_vsec_pci_probe,
> > > > > > > > > +};
> > > > > > > > 
> > > > > > > > So when the PCI device is removed from the system you leak
> > > > > > > > resources and
> > > > > > > > have dangling devices?
> > > > > > > 
> > > > > > > No.
> > > > > > > 
> > > > > > > > Why no PCI remove driver callback?
> > > > > > > 
> > > > > > > After probe all resources are device managed. There's nothing to
> > > > > > > explicitly clean up. When
> > > > > > > the
> > > > > > > PCI
> > > > > > > device is removed, all aux devices are automatically removed. This
> > > > > > > is the case for the SDSi
> > > > > > > driver
> > > > > > > as well.
> > > > > > 
> > > > > > Where is the "automatic cleanup" happening?  As this pci driver is
> > > > > > bound
> > > > > > to the PCI device, when the device is removed, what is called in this
> > > > > > driver to remove the resources allocated in the probe callback?
> > > > > > 
> > > > > > confused,
> > > > > 
> > > > > devm_add_action_or_reset(&pdev->dev, intel_vsec_remove_aux, auxdev)
> > > > 
> > > > Wow that is opaque.  Why not do it on remove instead?
> > > 
> > > This code is common for auxdev cleanup. AFAICT most auxiliary bus code is
> > > done by drivers that have
> > > some other primary function. They clean up their primary function resources
> > > in remove, but they
> > > clean up the auxdev using the method above. In this case the sole purpose of
> > > this driver is to
> > > create the auxdev. There are no other resources beyond what the auxdev is
> > > using.
> > > 
> > > Adding runtime pm to the pci driver will change this. Remove will be needed
> > > then.
> > 
> > And who will notice that being required when that happens?
> > 
> > Why is there no runtime PM for this driver?  Do you not care about power
> > consumption?  :)
> 
> Of course. :)
> 
> There's a backlog of patches waiting for this series. One adds support for the
> telemetry device (an auxdev) on the DG2 GPU. This device requires runtime pm in
> order for the slot to go D3. But this also requires changes to the telemetry
> driver in order for runtime pm to be handled correctly. These and other patches,
> including a series to have all current aux drivers use the new drvdata helpers,
> are waiting for this.

I can take the aux driver drvdata patch now, through my tree, if you
want, no need to make it wait for this tiny driver.

Feel free to send it independant of the existing patchset, and with the
cleanup patches at the same time, should be quite easy to get merged.

thanks,

greg k-h
Hans de Goede Dec. 21, 2021, 5:04 p.m. UTC | #10
Hi,

On 12/21/21 17:54, Greg KH wrote:
> On Tue, Dec 21, 2021 at 08:44:57AM -0800, David E. Box wrote:
>> On Tue, 2021-12-21 at 08:38 +0100, Greg KH wrote:
>>> On Wed, Dec 08, 2021 at 01:30:06PM -0800, David E. Box wrote:
>>>> On Wed, 2021-12-08 at 20:21 +0100, Greg KH wrote:
>>>>> On Wed, Dec 08, 2021 at 11:09:48AM -0800, David E. Box wrote:
>>>>>> On Wed, 2021-12-08 at 19:11 +0100, Greg KH wrote:
>>>>>>> On Wed, Dec 08, 2021 at 09:47:26AM -0800, David E. Box wrote:
>>>>>>>> On Wed, 2021-12-08 at 17:22 +0100, Greg KH wrote:
>>>>>>>>> On Tue, Dec 07, 2021 at 05:50:12PM -0800, David E. Box wrote:
>>>>>>>>>> +static struct pci_driver intel_vsec_pci_driver = {
>>>>>>>>>> +       .name = "intel_vsec",
>>>>>>>>>> +       .id_table = intel_vsec_pci_ids,
>>>>>>>>>> +       .probe = intel_vsec_pci_probe,
>>>>>>>>>> +};
>>>>>>>>>
>>>>>>>>> So when the PCI device is removed from the system you leak
>>>>>>>>> resources and
>>>>>>>>> have dangling devices?
>>>>>>>>
>>>>>>>> No.
>>>>>>>>
>>>>>>>>> Why no PCI remove driver callback?
>>>>>>>>
>>>>>>>> After probe all resources are device managed. There's nothing to
>>>>>>>> explicitly clean up. When
>>>>>>>> the
>>>>>>>> PCI
>>>>>>>> device is removed, all aux devices are automatically removed. This
>>>>>>>> is the case for the SDSi
>>>>>>>> driver
>>>>>>>> as well.
>>>>>>>
>>>>>>> Where is the "automatic cleanup" happening?  As this pci driver is
>>>>>>> bound
>>>>>>> to the PCI device, when the device is removed, what is called in this
>>>>>>> driver to remove the resources allocated in the probe callback?
>>>>>>>
>>>>>>> confused,
>>>>>>
>>>>>> devm_add_action_or_reset(&pdev->dev, intel_vsec_remove_aux, auxdev)
>>>>>
>>>>> Wow that is opaque.  Why not do it on remove instead?
>>>>
>>>> This code is common for auxdev cleanup. AFAICT most auxiliary bus code is
>>>> done by drivers that have
>>>> some other primary function. They clean up their primary function resources
>>>> in remove, but they
>>>> clean up the auxdev using the method above. In this case the sole purpose of
>>>> this driver is to
>>>> create the auxdev. There are no other resources beyond what the auxdev is
>>>> using.
>>>>
>>>> Adding runtime pm to the pci driver will change this. Remove will be needed
>>>> then.
>>>
>>> And who will notice that being required when that happens?
>>>
>>> Why is there no runtime PM for this driver?  Do you not care about power
>>> consumption?  :)
>>
>> Of course. :)
>>
>> There's a backlog of patches waiting for this series. One adds support for the
>> telemetry device (an auxdev) on the DG2 GPU. This device requires runtime pm in
>> order for the slot to go D3. But this also requires changes to the telemetry
>> driver in order for runtime pm to be handled correctly. These and other patches,
>> including a series to have all current aux drivers use the new drvdata helpers,
>> are waiting for this.
> 
> I can take the aux driver drvdata patch now, through my tree, if you
> want, no need to make it wait for this tiny driver.
> 
> Feel free to send it independant of the existing patchset, and with the
> cleanup patches at the same time, should be quite easy to get merged.

If you're going to take that one, can you perhaps take patches
1-3 for 5.17 through your tree as well (patch 3 depends on 2/it) ?

Note there is a v4 of this series, see please use that :)

I assume the follow up patches are also going to need patch 3
(the actual conversion of the driver to aux-bus).

Here is my Ack for the pdx86 bits in patch 3:

Acked-by: Hans de Goede <hdegoede@redhat.com>

And patch 1 and 3 also have acks from the PCI resp. MFD subsys maintainers,
so I guess taking this all upstream through your tree is fine.

That leaves patches 4-6, 4 is the patching adding the new
"Intel Software Defined Silicon driver" sysfs API and I would
like to take some time to thoroughly review the new
userspace API, which I don't see happening before the
Christmas Holidays, so I don't plan to merge 4-6 (which
depends on 3) until after 5.17-rc1.

Regards,

Hans
David E. Box Dec. 21, 2021, 6:16 p.m. UTC | #11
On Tue, 2021-12-21 at 18:04 +0100, Hans de Goede wrote:
> Hi,
> 
> On 12/21/21 17:54, Greg KH wrote:
> > On Tue, Dec 21, 2021 at 08:44:57AM -0800, David E. Box wrote:
> > > On Tue, 2021-12-21 at 08:38 +0100, Greg KH wrote:
> > > > On Wed, Dec 08, 2021 at 01:30:06PM -0800, David E. Box wrote:
> > > > > On Wed, 2021-12-08 at 20:21 +0100, Greg KH wrote:
> > > > > > On Wed, Dec 08, 2021 at 11:09:48AM -0800, David E. Box wrote:
> > > > > > > On Wed, 2021-12-08 at 19:11 +0100, Greg KH wrote:
> > > > > > > > On Wed, Dec 08, 2021 at 09:47:26AM -0800, David E. Box wrote:
> > > > > > > > > On Wed, 2021-12-08 at 17:22 +0100, Greg KH wrote:
> > > > > > > > > > On Tue, Dec 07, 2021 at 05:50:12PM -0800, David E. Box
> > > > > > > > > > wrote:
> > > > > > > > > > > +static struct pci_driver intel_vsec_pci_driver = {
> > > > > > > > > > > +       .name = "intel_vsec",
> > > > > > > > > > > +       .id_table = intel_vsec_pci_ids,
> > > > > > > > > > > +       .probe = intel_vsec_pci_probe,
> > > > > > > > > > > +};
> > > > > > > > > > 
> > > > > > > > > > So when the PCI device is removed from the system you leak
> > > > > > > > > > resources and
> > > > > > > > > > have dangling devices?
> > > > > > > > > 
> > > > > > > > > No.
> > > > > > > > > 
> > > > > > > > > > Why no PCI remove driver callback?
> > > > > > > > > 
> > > > > > > > > After probe all resources are device managed. There's nothing
> > > > > > > > > to
> > > > > > > > > explicitly clean up. When
> > > > > > > > > the
> > > > > > > > > PCI
> > > > > > > > > device is removed, all aux devices are automatically removed.
> > > > > > > > > This
> > > > > > > > > is the case for the SDSi
> > > > > > > > > driver
> > > > > > > > > as well.
> > > > > > > > 
> > > > > > > > Where is the "automatic cleanup" happening?  As this pci driver
> > > > > > > > is
> > > > > > > > bound
> > > > > > > > to the PCI device, when the device is removed, what is called in
> > > > > > > > this
> > > > > > > > driver to remove the resources allocated in the probe callback?
> > > > > > > > 
> > > > > > > > confused,
> > > > > > > 
> > > > > > > devm_add_action_or_reset(&pdev->dev, intel_vsec_remove_aux,
> > > > > > > auxdev)
> > > > > > 
> > > > > > Wow that is opaque.  Why not do it on remove instead?
> > > > > 
> > > > > This code is common for auxdev cleanup. AFAICT most auxiliary bus code
> > > > > is
> > > > > done by drivers that have
> > > > > some other primary function. They clean up their primary function
> > > > > resources
> > > > > in remove, but they
> > > > > clean up the auxdev using the method above. In this case the sole
> > > > > purpose of
> > > > > this driver is to
> > > > > create the auxdev. There are no other resources beyond what the auxdev
> > > > > is
> > > > > using.
> > > > > 
> > > > > Adding runtime pm to the pci driver will change this. Remove will be
> > > > > needed
> > > > > then.
> > > > 
> > > > And who will notice that being required when that happens?
> > > > 
> > > > Why is there no runtime PM for this driver?  Do you not care about power
> > > > consumption?  :)
> > > 
> > > Of course. :)
> > > 
> > > There's a backlog of patches waiting for this series. One adds support for
> > > the
> > > telemetry device (an auxdev) on the DG2 GPU. This device requires runtime
> > > pm in
> > > order for the slot to go D3. But this also requires changes to the
> > > telemetry
> > > driver in order for runtime pm to be handled correctly. These and other
> > > patches,
> > > including a series to have all current aux drivers use the new drvdata
> > > helpers,
> > > are waiting for this.
> > 
> > I can take the aux driver drvdata patch now, through my tree, if you
> > want, no need to make it wait for this tiny driver.
> > 
> > Feel free to send it independant of the existing patchset, and with the
> > cleanup patches at the same time, should be quite easy to get merged.
> 
> If you're going to take that one, can you perhaps take patches
> 1-3 for 5.17 through your tree as well (patch 3 depends on 2/it) ?
> 
> Note there is a v4 of this series, see please use that :)
> 
> I assume the follow up patches are also going to need patch 3
> (the actual conversion of the driver to aux-bus).

Yes.

> 
> Here is my Ack for the pdx86 bits in patch 3:
> 
> Acked-by: Hans de Goede <hdegoede@redhat.com>
> 
> And patch 1 and 3 also have acks from the PCI resp. MFD subsys maintainers,
> so I guess taking this all upstream through your tree is fine.

Should I send 1-3 plus the drvdata cleanup patches I have to Grep? V5?

> 
> That leaves patches 4-6, 4 is the patching adding the new
> "Intel Software Defined Silicon driver" sysfs API and I would
> like to take some time to thoroughly review the new
> userspace API, which I don't see happening before the
> Christmas Holidays, so I don't plan to merge 4-6 (which
> depends on 3) until after 5.17-rc1.

Understood. Thanks.

> 
> Regards,
> 
> Hans
>
Hans de Goede Dec. 21, 2021, 6:38 p.m. UTC | #12
Hi,

On 12/21/21 19:16, David E. Box wrote:
> On Tue, 2021-12-21 at 18:04 +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 12/21/21 17:54, Greg KH wrote:
>>> On Tue, Dec 21, 2021 at 08:44:57AM -0800, David E. Box wrote:
>>>> On Tue, 2021-12-21 at 08:38 +0100, Greg KH wrote:
>>>>> On Wed, Dec 08, 2021 at 01:30:06PM -0800, David E. Box wrote:
>>>>>> On Wed, 2021-12-08 at 20:21 +0100, Greg KH wrote:
>>>>>>> On Wed, Dec 08, 2021 at 11:09:48AM -0800, David E. Box wrote:
>>>>>>>> On Wed, 2021-12-08 at 19:11 +0100, Greg KH wrote:
>>>>>>>>> On Wed, Dec 08, 2021 at 09:47:26AM -0800, David E. Box wrote:
>>>>>>>>>> On Wed, 2021-12-08 at 17:22 +0100, Greg KH wrote:
>>>>>>>>>>> On Tue, Dec 07, 2021 at 05:50:12PM -0800, David E. Box
>>>>>>>>>>> wrote:
>>>>>>>>>>>> +static struct pci_driver intel_vsec_pci_driver = {
>>>>>>>>>>>> +       .name = "intel_vsec",
>>>>>>>>>>>> +       .id_table = intel_vsec_pci_ids,
>>>>>>>>>>>> +       .probe = intel_vsec_pci_probe,
>>>>>>>>>>>> +};
>>>>>>>>>>>
>>>>>>>>>>> So when the PCI device is removed from the system you leak
>>>>>>>>>>> resources and
>>>>>>>>>>> have dangling devices?
>>>>>>>>>>
>>>>>>>>>> No.
>>>>>>>>>>
>>>>>>>>>>> Why no PCI remove driver callback?
>>>>>>>>>>
>>>>>>>>>> After probe all resources are device managed. There's nothing
>>>>>>>>>> to
>>>>>>>>>> explicitly clean up. When
>>>>>>>>>> the
>>>>>>>>>> PCI
>>>>>>>>>> device is removed, all aux devices are automatically removed.
>>>>>>>>>> This
>>>>>>>>>> is the case for the SDSi
>>>>>>>>>> driver
>>>>>>>>>> as well.
>>>>>>>>>
>>>>>>>>> Where is the "automatic cleanup" happening?  As this pci driver
>>>>>>>>> is
>>>>>>>>> bound
>>>>>>>>> to the PCI device, when the device is removed, what is called in
>>>>>>>>> this
>>>>>>>>> driver to remove the resources allocated in the probe callback?
>>>>>>>>>
>>>>>>>>> confused,
>>>>>>>>
>>>>>>>> devm_add_action_or_reset(&pdev->dev, intel_vsec_remove_aux,
>>>>>>>> auxdev)
>>>>>>>
>>>>>>> Wow that is opaque.  Why not do it on remove instead?
>>>>>>
>>>>>> This code is common for auxdev cleanup. AFAICT most auxiliary bus code
>>>>>> is
>>>>>> done by drivers that have
>>>>>> some other primary function. They clean up their primary function
>>>>>> resources
>>>>>> in remove, but they
>>>>>> clean up the auxdev using the method above. In this case the sole
>>>>>> purpose of
>>>>>> this driver is to
>>>>>> create the auxdev. There are no other resources beyond what the auxdev
>>>>>> is
>>>>>> using.
>>>>>>
>>>>>> Adding runtime pm to the pci driver will change this. Remove will be
>>>>>> needed
>>>>>> then.
>>>>>
>>>>> And who will notice that being required when that happens?
>>>>>
>>>>> Why is there no runtime PM for this driver?  Do you not care about power
>>>>> consumption?  :)
>>>>
>>>> Of course. :)
>>>>
>>>> There's a backlog of patches waiting for this series. One adds support for
>>>> the
>>>> telemetry device (an auxdev) on the DG2 GPU. This device requires runtime
>>>> pm in
>>>> order for the slot to go D3. But this also requires changes to the
>>>> telemetry
>>>> driver in order for runtime pm to be handled correctly. These and other
>>>> patches,
>>>> including a series to have all current aux drivers use the new drvdata
>>>> helpers,
>>>> are waiting for this.
>>>
>>> I can take the aux driver drvdata patch now, through my tree, if you
>>> want, no need to make it wait for this tiny driver.
>>>
>>> Feel free to send it independant of the existing patchset, and with the
>>> cleanup patches at the same time, should be quite easy to get merged.
>>
>> If you're going to take that one, can you perhaps take patches
>> 1-3 for 5.17 through your tree as well (patch 3 depends on 2/it) ?
>>
>> Note there is a v4 of this series, see please use that :)
>>
>> I assume the follow up patches are also going to need patch 3
>> (the actual conversion of the driver to aux-bus).
> 
> Yes.
> 
>>
>> Here is my Ack for the pdx86 bits in patch 3:
>>
>> Acked-by: Hans de Goede <hdegoede@redhat.com>
>>
>> And patch 1 and 3 also have acks from the PCI resp. MFD subsys maintainers,
>> so I guess taking this all upstream through your tree is fine.
> 
> Should I send 1-3 plus the drvdata cleanup patches I have to Grep? V5?

No there is no need for that v4 is fine, since no changes have been
requested there is no need to send out a new version.

Regards,

Hans
Greg KH Dec. 22, 2021, 12:57 p.m. UTC | #13
On Tue, Dec 21, 2021 at 07:38:38PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 12/21/21 19:16, David E. Box wrote:
> > On Tue, 2021-12-21 at 18:04 +0100, Hans de Goede wrote:
> >> Hi,
> >>
> >> On 12/21/21 17:54, Greg KH wrote:
> >>> On Tue, Dec 21, 2021 at 08:44:57AM -0800, David E. Box wrote:
> >>>> On Tue, 2021-12-21 at 08:38 +0100, Greg KH wrote:
> >>>>> On Wed, Dec 08, 2021 at 01:30:06PM -0800, David E. Box wrote:
> >>>>>> On Wed, 2021-12-08 at 20:21 +0100, Greg KH wrote:
> >>>>>>> On Wed, Dec 08, 2021 at 11:09:48AM -0800, David E. Box wrote:
> >>>>>>>> On Wed, 2021-12-08 at 19:11 +0100, Greg KH wrote:
> >>>>>>>>> On Wed, Dec 08, 2021 at 09:47:26AM -0800, David E. Box wrote:
> >>>>>>>>>> On Wed, 2021-12-08 at 17:22 +0100, Greg KH wrote:
> >>>>>>>>>>> On Tue, Dec 07, 2021 at 05:50:12PM -0800, David E. Box
> >>>>>>>>>>> wrote:
> >>>>>>>>>>>> +static struct pci_driver intel_vsec_pci_driver = {
> >>>>>>>>>>>> +       .name = "intel_vsec",
> >>>>>>>>>>>> +       .id_table = intel_vsec_pci_ids,
> >>>>>>>>>>>> +       .probe = intel_vsec_pci_probe,
> >>>>>>>>>>>> +};
> >>>>>>>>>>>
> >>>>>>>>>>> So when the PCI device is removed from the system you leak
> >>>>>>>>>>> resources and
> >>>>>>>>>>> have dangling devices?
> >>>>>>>>>>
> >>>>>>>>>> No.
> >>>>>>>>>>
> >>>>>>>>>>> Why no PCI remove driver callback?
> >>>>>>>>>>
> >>>>>>>>>> After probe all resources are device managed. There's nothing
> >>>>>>>>>> to
> >>>>>>>>>> explicitly clean up. When
> >>>>>>>>>> the
> >>>>>>>>>> PCI
> >>>>>>>>>> device is removed, all aux devices are automatically removed.
> >>>>>>>>>> This
> >>>>>>>>>> is the case for the SDSi
> >>>>>>>>>> driver
> >>>>>>>>>> as well.
> >>>>>>>>>
> >>>>>>>>> Where is the "automatic cleanup" happening?  As this pci driver
> >>>>>>>>> is
> >>>>>>>>> bound
> >>>>>>>>> to the PCI device, when the device is removed, what is called in
> >>>>>>>>> this
> >>>>>>>>> driver to remove the resources allocated in the probe callback?
> >>>>>>>>>
> >>>>>>>>> confused,
> >>>>>>>>
> >>>>>>>> devm_add_action_or_reset(&pdev->dev, intel_vsec_remove_aux,
> >>>>>>>> auxdev)
> >>>>>>>
> >>>>>>> Wow that is opaque.  Why not do it on remove instead?
> >>>>>>
> >>>>>> This code is common for auxdev cleanup. AFAICT most auxiliary bus code
> >>>>>> is
> >>>>>> done by drivers that have
> >>>>>> some other primary function. They clean up their primary function
> >>>>>> resources
> >>>>>> in remove, but they
> >>>>>> clean up the auxdev using the method above. In this case the sole
> >>>>>> purpose of
> >>>>>> this driver is to
> >>>>>> create the auxdev. There are no other resources beyond what the auxdev
> >>>>>> is
> >>>>>> using.
> >>>>>>
> >>>>>> Adding runtime pm to the pci driver will change this. Remove will be
> >>>>>> needed
> >>>>>> then.
> >>>>>
> >>>>> And who will notice that being required when that happens?
> >>>>>
> >>>>> Why is there no runtime PM for this driver?  Do you not care about power
> >>>>> consumption?  :)
> >>>>
> >>>> Of course. :)
> >>>>
> >>>> There's a backlog of patches waiting for this series. One adds support for
> >>>> the
> >>>> telemetry device (an auxdev) on the DG2 GPU. This device requires runtime
> >>>> pm in
> >>>> order for the slot to go D3. But this also requires changes to the
> >>>> telemetry
> >>>> driver in order for runtime pm to be handled correctly. These and other
> >>>> patches,
> >>>> including a series to have all current aux drivers use the new drvdata
> >>>> helpers,
> >>>> are waiting for this.
> >>>
> >>> I can take the aux driver drvdata patch now, through my tree, if you
> >>> want, no need to make it wait for this tiny driver.
> >>>
> >>> Feel free to send it independant of the existing patchset, and with the
> >>> cleanup patches at the same time, should be quite easy to get merged.
> >>
> >> If you're going to take that one, can you perhaps take patches
> >> 1-3 for 5.17 through your tree as well (patch 3 depends on 2/it) ?
> >>
> >> Note there is a v4 of this series, see please use that :)
> >>
> >> I assume the follow up patches are also going to need patch 3
> >> (the actual conversion of the driver to aux-bus).
> > 
> > Yes.
> > 
> >>
> >> Here is my Ack for the pdx86 bits in patch 3:
> >>
> >> Acked-by: Hans de Goede <hdegoede@redhat.com>
> >>
> >> And patch 1 and 3 also have acks from the PCI resp. MFD subsys maintainers,
> >> so I guess taking this all upstream through your tree is fine.
> > 
> > Should I send 1-3 plus the drvdata cleanup patches I have to Grep? V5?
> 
> No there is no need for that v4 is fine, since no changes have been
> requested there is no need to send out a new version.

I've taken patches 1-3 of this series now, thanks.

greg k-h
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 43007f2d29e0..cd2b10a86f09 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9752,10 +9752,9 @@  S:	Maintained
 F:	drivers/mfd/intel_soc_pmic*
 F:	include/linux/mfd/intel_soc_pmic*
 
-INTEL PMT DRIVER
-M:	"David E. Box" <david.e.box@linux.intel.com>
-S:	Maintained
-F:	drivers/mfd/intel_pmt.c
+INTEL PMT DRIVERS
+M:	David E. Box <david.e.box@linux.intel.com>
+S:	Supported
 F:	drivers/platform/x86/intel/pmt/
 
 INTEL PRO/WIRELESS 2100, 2200BG, 2915ABG NETWORK CONNECTION SUPPORT
@@ -9822,6 +9821,11 @@  L:	platform-driver-x86@vger.kernel.org
 S:	Maintained
 F:	drivers/platform/x86/intel/uncore-frequency.c
 
+INTEL VENDOR SPECIFIC EXTENDED CAPABILITIES DRIVER
+M:	David E. Box <david.e.box@linux.intel.com>
+S:	Supported
+F:	drivers/platform/x86/intel/vsec.*
+
 INTEL VIRTUAL BUTTON DRIVER
 M:	AceLan Kao <acelan.kao@canonical.com>
 L:	platform-driver-x86@vger.kernel.org
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 3fb480818599..ac7b23eb62c2 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -692,16 +692,6 @@  config MFD_INTEL_PMC_BXT
 	  Register and P-unit access. In addition this creates devices
 	  for iTCO watchdog and telemetry that are part of the PMC.
 
-config MFD_INTEL_PMT
-	tristate "Intel Platform Monitoring Technology (PMT) support"
-	depends on X86 && PCI
-	select MFD_CORE
-	help
-	  The Intel Platform Monitoring Technology (PMT) is an interface that
-	  provides access to hardware monitor registers. This driver supports
-	  Telemetry, Watcher, and Crashlog PMT capabilities/devices for
-	  platforms starting from Tiger Lake.
-
 config MFD_IPAQ_MICRO
 	bool "Atmel Micro ASIC (iPAQ h3100/h3600/h3700) Support"
 	depends on SA1100_H3100 || SA1100_H3600
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 0b1b629aef3e..31734d9318e2 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -211,7 +211,6 @@  obj-$(CONFIG_MFD_INTEL_LPSS)	+= intel-lpss.o
 obj-$(CONFIG_MFD_INTEL_LPSS_PCI)	+= intel-lpss-pci.o
 obj-$(CONFIG_MFD_INTEL_LPSS_ACPI)	+= intel-lpss-acpi.o
 obj-$(CONFIG_MFD_INTEL_PMC_BXT)	+= intel_pmc_bxt.o
-obj-$(CONFIG_MFD_INTEL_PMT)	+= intel_pmt.o
 obj-$(CONFIG_MFD_PALMAS)	+= palmas.o
 obj-$(CONFIG_MFD_VIPERBOARD)    += viperboard.o
 obj-$(CONFIG_MFD_NTXEC)		+= ntxec.o
diff --git a/drivers/mfd/intel_pmt.c b/drivers/mfd/intel_pmt.c
deleted file mode 100644
index dd7eb614c28e..000000000000
--- a/drivers/mfd/intel_pmt.c
+++ /dev/null
@@ -1,261 +0,0 @@ 
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Intel Platform Monitoring Technology PMT driver
- *
- * Copyright (c) 2020, Intel Corporation.
- * All Rights Reserved.
- *
- * Author: David E. Box <david.e.box@linux.intel.com>
- */
-
-#include <linux/bits.h>
-#include <linux/kernel.h>
-#include <linux/mfd/core.h>
-#include <linux/module.h>
-#include <linux/pci.h>
-#include <linux/platform_device.h>
-#include <linux/pm.h>
-#include <linux/pm_runtime.h>
-#include <linux/types.h>
-
-/* Intel DVSEC capability vendor space offsets */
-#define INTEL_DVSEC_ENTRIES		0xA
-#define INTEL_DVSEC_SIZE		0xB
-#define INTEL_DVSEC_TABLE		0xC
-#define INTEL_DVSEC_TABLE_BAR(x)	((x) & GENMASK(2, 0))
-#define INTEL_DVSEC_TABLE_OFFSET(x)	((x) & GENMASK(31, 3))
-#define INTEL_DVSEC_ENTRY_SIZE		4
-
-/* PMT capabilities */
-#define DVSEC_INTEL_ID_TELEMETRY	2
-#define DVSEC_INTEL_ID_WATCHER		3
-#define DVSEC_INTEL_ID_CRASHLOG		4
-
-struct intel_dvsec_header {
-	u16	length;
-	u16	id;
-	u8	num_entries;
-	u8	entry_size;
-	u8	tbir;
-	u32	offset;
-};
-
-enum pmt_quirks {
-	/* Watcher capability not supported */
-	PMT_QUIRK_NO_WATCHER	= BIT(0),
-
-	/* Crashlog capability not supported */
-	PMT_QUIRK_NO_CRASHLOG	= BIT(1),
-
-	/* Use shift instead of mask to read discovery table offset */
-	PMT_QUIRK_TABLE_SHIFT	= BIT(2),
-
-	/* DVSEC not present (provided in driver data) */
-	PMT_QUIRK_NO_DVSEC	= BIT(3),
-};
-
-struct pmt_platform_info {
-	unsigned long quirks;
-	struct intel_dvsec_header **capabilities;
-};
-
-static const struct pmt_platform_info tgl_info = {
-	.quirks = PMT_QUIRK_NO_WATCHER | PMT_QUIRK_NO_CRASHLOG |
-		  PMT_QUIRK_TABLE_SHIFT,
-};
-
-/* DG1 Platform with DVSEC quirk*/
-static struct intel_dvsec_header dg1_telemetry = {
-	.length = 0x10,
-	.id = 2,
-	.num_entries = 1,
-	.entry_size = 3,
-	.tbir = 0,
-	.offset = 0x466000,
-};
-
-static struct intel_dvsec_header *dg1_capabilities[] = {
-	&dg1_telemetry,
-	NULL
-};
-
-static const struct pmt_platform_info dg1_info = {
-	.quirks = PMT_QUIRK_NO_DVSEC,
-	.capabilities = dg1_capabilities,
-};
-
-static int pmt_add_dev(struct pci_dev *pdev, struct intel_dvsec_header *header,
-		       unsigned long quirks)
-{
-	struct device *dev = &pdev->dev;
-	struct resource *res, *tmp;
-	struct mfd_cell *cell;
-	const char *name;
-	int count = header->num_entries;
-	int size = header->entry_size;
-	int id = header->id;
-	int i;
-
-	switch (id) {
-	case DVSEC_INTEL_ID_TELEMETRY:
-		name = "pmt_telemetry";
-		break;
-	case DVSEC_INTEL_ID_WATCHER:
-		if (quirks & PMT_QUIRK_NO_WATCHER) {
-			dev_info(dev, "Watcher not supported\n");
-			return -EINVAL;
-		}
-		name = "pmt_watcher";
-		break;
-	case DVSEC_INTEL_ID_CRASHLOG:
-		if (quirks & PMT_QUIRK_NO_CRASHLOG) {
-			dev_info(dev, "Crashlog not supported\n");
-			return -EINVAL;
-		}
-		name = "pmt_crashlog";
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	if (!header->num_entries || !header->entry_size) {
-		dev_err(dev, "Invalid count or size for %s header\n", name);
-		return -EINVAL;
-	}
-
-	cell = devm_kzalloc(dev, sizeof(*cell), GFP_KERNEL);
-	if (!cell)
-		return -ENOMEM;
-
-	res = devm_kcalloc(dev, count, sizeof(*res), GFP_KERNEL);
-	if (!res)
-		return -ENOMEM;
-
-	if (quirks & PMT_QUIRK_TABLE_SHIFT)
-		header->offset >>= 3;
-
-	/*
-	 * The PMT DVSEC contains the starting offset and count for a block of
-	 * discovery tables, each providing access to monitoring facilities for
-	 * a section of the device. Create a resource list of these tables to
-	 * provide to the driver.
-	 */
-	for (i = 0, tmp = res; i < count; i++, tmp++) {
-		tmp->start = pdev->resource[header->tbir].start +
-			     header->offset + i * (size << 2);
-		tmp->end = tmp->start + (size << 2) - 1;
-		tmp->flags = IORESOURCE_MEM;
-	}
-
-	cell->resources = res;
-	cell->num_resources = count;
-	cell->name = name;
-
-	return devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, cell, 1, NULL, 0,
-				    NULL);
-}
-
-static int pmt_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
-{
-	struct pmt_platform_info *info;
-	unsigned long quirks = 0;
-	bool found_devices = false;
-	int ret, pos = 0;
-
-	ret = pcim_enable_device(pdev);
-	if (ret)
-		return ret;
-
-	info = (struct pmt_platform_info *)id->driver_data;
-
-	if (info)
-		quirks = info->quirks;
-
-	if (info && (info->quirks & PMT_QUIRK_NO_DVSEC)) {
-		struct intel_dvsec_header **header;
-
-		header = info->capabilities;
-		while (*header) {
-			ret = pmt_add_dev(pdev, *header, quirks);
-			if (ret)
-				dev_warn(&pdev->dev,
-					 "Failed to add device for DVSEC id %d\n",
-					 (*header)->id);
-			else
-				found_devices = true;
-
-			++header;
-		}
-	} else {
-		do {
-			struct intel_dvsec_header header;
-			u32 table;
-			u16 vid;
-
-			pos = pci_find_next_ext_capability(pdev, pos, PCI_EXT_CAP_ID_DVSEC);
-			if (!pos)
-				break;
-
-			pci_read_config_word(pdev, pos + PCI_DVSEC_HEADER1, &vid);
-			if (vid != PCI_VENDOR_ID_INTEL)
-				continue;
-
-			pci_read_config_word(pdev, pos + PCI_DVSEC_HEADER2,
-					     &header.id);
-			pci_read_config_byte(pdev, pos + INTEL_DVSEC_ENTRIES,
-					     &header.num_entries);
-			pci_read_config_byte(pdev, pos + INTEL_DVSEC_SIZE,
-					     &header.entry_size);
-			pci_read_config_dword(pdev, pos + INTEL_DVSEC_TABLE,
-					      &table);
-
-			header.tbir = INTEL_DVSEC_TABLE_BAR(table);
-			header.offset = INTEL_DVSEC_TABLE_OFFSET(table);
-
-			ret = pmt_add_dev(pdev, &header, quirks);
-			if (ret)
-				continue;
-
-			found_devices = true;
-		} while (true);
-	}
-
-	if (!found_devices)
-		return -ENODEV;
-
-	pm_runtime_put(&pdev->dev);
-	pm_runtime_allow(&pdev->dev);
-
-	return 0;
-}
-
-static void pmt_pci_remove(struct pci_dev *pdev)
-{
-	pm_runtime_forbid(&pdev->dev);
-	pm_runtime_get_sync(&pdev->dev);
-}
-
-#define PCI_DEVICE_ID_INTEL_PMT_ADL	0x467d
-#define PCI_DEVICE_ID_INTEL_PMT_DG1	0x490e
-#define PCI_DEVICE_ID_INTEL_PMT_OOBMSM	0x09a7
-#define PCI_DEVICE_ID_INTEL_PMT_TGL	0x9a0d
-static const struct pci_device_id pmt_pci_ids[] = {
-	{ PCI_DEVICE_DATA(INTEL, PMT_ADL, &tgl_info) },
-	{ PCI_DEVICE_DATA(INTEL, PMT_DG1, &dg1_info) },
-	{ PCI_DEVICE_DATA(INTEL, PMT_OOBMSM, NULL) },
-	{ PCI_DEVICE_DATA(INTEL, PMT_TGL, &tgl_info) },
-	{ }
-};
-MODULE_DEVICE_TABLE(pci, pmt_pci_ids);
-
-static struct pci_driver pmt_pci_driver = {
-	.name = "intel-pmt",
-	.id_table = pmt_pci_ids,
-	.probe = pmt_pci_probe,
-	.remove = pmt_pci_remove,
-};
-module_pci_driver(pmt_pci_driver);
-
-MODULE_AUTHOR("David E. Box <david.e.box@linux.intel.com>");
-MODULE_DESCRIPTION("Intel Platform Monitoring Technology PMT driver");
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/platform/x86/intel/Kconfig b/drivers/platform/x86/intel/Kconfig
index 38ce3e344589..35a5d1a5eba8 100644
--- a/drivers/platform/x86/intel/Kconfig
+++ b/drivers/platform/x86/intel/Kconfig
@@ -184,4 +184,15 @@  config INTEL_UNCORE_FREQ_CONTROL
 	  To compile this driver as a module, choose M here: the module
 	  will be called intel-uncore-frequency.
 
+config INTEL_VSEC
+	tristate "Intel Vendor Specific Extended Capabilities Driver"
+	depends on PCI
+	select AUXILIARY_BUS
+	help
+	  Adds support for feature drivers exposed using Intel PCIe VSEC and
+	  DVSEC.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called intel_vsec.
+
 endif # X86_PLATFORM_DRIVERS_INTEL
diff --git a/drivers/platform/x86/intel/Makefile b/drivers/platform/x86/intel/Makefile
index 7c24be2423d8..8ecdf709fb17 100644
--- a/drivers/platform/x86/intel/Makefile
+++ b/drivers/platform/x86/intel/Makefile
@@ -26,6 +26,8 @@  intel_int0002_vgpio-y			:= int0002_vgpio.o
 obj-$(CONFIG_INTEL_INT0002_VGPIO)	+= intel_int0002_vgpio.o
 intel_oaktrail-y			:= oaktrail.o
 obj-$(CONFIG_INTEL_OAKTRAIL)		+= intel_oaktrail.o
+intel_vsec-y				:= vsec.o
+obj-$(CONFIG_INTEL_VSEC)		+= intel_vsec.o
 
 # Intel PMIC / PMC / P-Unit drivers
 intel_bxtwc_tmu-y			:= bxtwc_tmu.o
diff --git a/drivers/platform/x86/intel/pmt/Kconfig b/drivers/platform/x86/intel/pmt/Kconfig
index d630f883a717..e916fc966221 100644
--- a/drivers/platform/x86/intel/pmt/Kconfig
+++ b/drivers/platform/x86/intel/pmt/Kconfig
@@ -17,7 +17,7 @@  config INTEL_PMT_CLASS
 
 config INTEL_PMT_TELEMETRY
 	tristate "Intel Platform Monitoring Technology (PMT) Telemetry driver"
-	depends on MFD_INTEL_PMT
+	depends on INTEL_VSEC
 	select INTEL_PMT_CLASS
 	help
 	  The Intel Platform Monitory Technology (PMT) Telemetry driver provides
@@ -29,7 +29,7 @@  config INTEL_PMT_TELEMETRY
 
 config INTEL_PMT_CRASHLOG
 	tristate "Intel Platform Monitoring Technology (PMT) Crashlog driver"
-	depends on MFD_INTEL_PMT
+	depends on INTEL_VSEC
 	select INTEL_PMT_CLASS
 	help
 	  The Intel Platform Monitoring Technology (PMT) crashlog driver provides
diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
index 659b1073033c..1c9e3f3ea41c 100644
--- a/drivers/platform/x86/intel/pmt/class.c
+++ b/drivers/platform/x86/intel/pmt/class.c
@@ -13,6 +13,7 @@ 
 #include <linux/mm.h>
 #include <linux/pci.h>
 
+#include "../vsec.h"
 #include "class.h"
 
 #define PMT_XA_START		0
@@ -281,31 +282,29 @@  static int intel_pmt_dev_register(struct intel_pmt_entry *entry,
 	return ret;
 }
 
-int intel_pmt_dev_create(struct intel_pmt_entry *entry,
-			 struct intel_pmt_namespace *ns,
-			 struct platform_device *pdev, int idx)
+int intel_pmt_dev_create(struct intel_pmt_entry *entry, struct intel_pmt_namespace *ns,
+			 struct intel_vsec_device *intel_vsec_dev, int idx)
 {
+	struct device *dev = &intel_vsec_dev->auxdev.dev;
 	struct intel_pmt_header header;
 	struct resource	*disc_res;
-	int ret = -ENODEV;
+	int ret;
 
-	disc_res = platform_get_resource(pdev, IORESOURCE_MEM, idx);
-	if (!disc_res)
-		return ret;
+	disc_res = &intel_vsec_dev->resource[idx];
 
-	entry->disc_table = devm_platform_ioremap_resource(pdev, idx);
+	entry->disc_table = devm_ioremap_resource(dev, disc_res);
 	if (IS_ERR(entry->disc_table))
 		return PTR_ERR(entry->disc_table);
 
-	ret = ns->pmt_header_decode(entry, &header, &pdev->dev);
+	ret = ns->pmt_header_decode(entry, &header, dev);
 	if (ret)
 		return ret;
 
-	ret = intel_pmt_populate_entry(entry, &header, &pdev->dev, disc_res);
+	ret = intel_pmt_populate_entry(entry, &header, dev, disc_res);
 	if (ret)
 		return ret;
 
-	return intel_pmt_dev_register(entry, ns, &pdev->dev);
+	return intel_pmt_dev_register(entry, ns, dev);
 
 }
 EXPORT_SYMBOL_GPL(intel_pmt_dev_create);
diff --git a/drivers/platform/x86/intel/pmt/class.h b/drivers/platform/x86/intel/pmt/class.h
index 1337019c2873..db11d58867ce 100644
--- a/drivers/platform/x86/intel/pmt/class.h
+++ b/drivers/platform/x86/intel/pmt/class.h
@@ -2,13 +2,14 @@ 
 #ifndef _INTEL_PMT_CLASS_H
 #define _INTEL_PMT_CLASS_H
 
-#include <linux/platform_device.h>
 #include <linux/xarray.h>
 #include <linux/types.h>
 #include <linux/bits.h>
 #include <linux/err.h>
 #include <linux/io.h>
 
+#include "../vsec.h"
+
 /* PMT access types */
 #define ACCESS_BARID		2
 #define ACCESS_LOCAL		3
@@ -47,7 +48,7 @@  struct intel_pmt_namespace {
 bool intel_pmt_is_early_client_hw(struct device *dev);
 int intel_pmt_dev_create(struct intel_pmt_entry *entry,
 			 struct intel_pmt_namespace *ns,
-			 struct platform_device *pdev, int idx);
+			 struct intel_vsec_device *dev, int idx);
 void intel_pmt_dev_destroy(struct intel_pmt_entry *entry,
 			   struct intel_pmt_namespace *ns);
 #endif
diff --git a/drivers/platform/x86/intel/pmt/crashlog.c b/drivers/platform/x86/intel/pmt/crashlog.c
index 1c1021f04d3c..34daf9df168b 100644
--- a/drivers/platform/x86/intel/pmt/crashlog.c
+++ b/drivers/platform/x86/intel/pmt/crashlog.c
@@ -8,6 +8,7 @@ 
  * Author: "Alexander Duyck" <alexander.h.duyck@linux.intel.com>
  */
 
+#include <linux/auxiliary_bus.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/pci.h>
@@ -15,10 +16,9 @@ 
 #include <linux/uaccess.h>
 #include <linux/overflow.h>
 
+#include "../vsec.h"
 #include "class.h"
 
-#define DRV_NAME		"pmt_crashlog"
-
 /* Crashlog discovery header types */
 #define CRASH_TYPE_OOBMSM	1
 
@@ -257,34 +257,34 @@  static struct intel_pmt_namespace pmt_crashlog_ns = {
 /*
  * initialization
  */
-static int pmt_crashlog_remove(struct platform_device *pdev)
+static void pmt_crashlog_remove(struct auxiliary_device *auxdev)
 {
-	struct pmt_crashlog_priv *priv = platform_get_drvdata(pdev);
+	struct pmt_crashlog_priv *priv = auxiliary_get_drvdata(auxdev);
 	int i;
 
 	for (i = 0; i < priv->num_entries; i++)
 		intel_pmt_dev_destroy(&priv->entry[i].entry, &pmt_crashlog_ns);
-
-	return 0;
 }
 
-static int pmt_crashlog_probe(struct platform_device *pdev)
+static int pmt_crashlog_probe(struct auxiliary_device *auxdev,
+			      const struct auxiliary_device_id *id)
 {
+	struct intel_vsec_device *intel_vsec_dev = auxdev_to_ivdev(auxdev);
 	struct pmt_crashlog_priv *priv;
 	size_t size;
 	int i, ret;
 
-	size = struct_size(priv, entry, pdev->num_resources);
-	priv = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
+	size = struct_size(priv, entry, intel_vsec_dev->num_resources);
+	priv = devm_kzalloc(&auxdev->dev, size, GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
-	platform_set_drvdata(pdev, priv);
+	auxiliary_set_drvdata(auxdev, priv);
 
-	for (i = 0; i < pdev->num_resources; i++) {
+	for (i = 0; i < intel_vsec_dev->num_resources; i++) {
 		struct intel_pmt_entry *entry = &priv->entry[i].entry;
 
-		ret = intel_pmt_dev_create(entry, &pmt_crashlog_ns, pdev, i);
+		ret = intel_pmt_dev_create(entry, &pmt_crashlog_ns, intel_vsec_dev, i);
 		if (ret < 0)
 			goto abort_probe;
 		if (ret)
@@ -295,26 +295,30 @@  static int pmt_crashlog_probe(struct platform_device *pdev)
 
 	return 0;
 abort_probe:
-	pmt_crashlog_remove(pdev);
+	pmt_crashlog_remove(auxdev);
 	return ret;
 }
 
-static struct platform_driver pmt_crashlog_driver = {
-	.driver = {
-		.name   = DRV_NAME,
-	},
-	.remove = pmt_crashlog_remove,
-	.probe  = pmt_crashlog_probe,
+static const struct auxiliary_device_id pmt_crashlog_id_table[] = {
+	{ .name = "intel_vsec.crashlog" },
+	{}
+};
+MODULE_DEVICE_TABLE(auxiliary, pmt_crashlog_id_table);
+
+static struct auxiliary_driver pmt_crashlog_aux_driver = {
+	.id_table	= pmt_crashlog_id_table,
+	.remove		= pmt_crashlog_remove,
+	.probe		= pmt_crashlog_probe,
 };
 
 static int __init pmt_crashlog_init(void)
 {
-	return platform_driver_register(&pmt_crashlog_driver);
+	return auxiliary_driver_register(&pmt_crashlog_aux_driver);
 }
 
 static void __exit pmt_crashlog_exit(void)
 {
-	platform_driver_unregister(&pmt_crashlog_driver);
+	auxiliary_driver_unregister(&pmt_crashlog_aux_driver);
 	xa_destroy(&crashlog_array);
 }
 
@@ -323,5 +327,4 @@  module_exit(pmt_crashlog_exit);
 
 MODULE_AUTHOR("Alexander Duyck <alexander.h.duyck@linux.intel.com>");
 MODULE_DESCRIPTION("Intel PMT Crashlog driver");
-MODULE_ALIAS("platform:" DRV_NAME);
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/platform/x86/intel/pmt/telemetry.c b/drivers/platform/x86/intel/pmt/telemetry.c
index 38d52651c572..6b6f3e2a617a 100644
--- a/drivers/platform/x86/intel/pmt/telemetry.c
+++ b/drivers/platform/x86/intel/pmt/telemetry.c
@@ -8,6 +8,7 @@ 
  * Author: "David E. Box" <david.e.box@linux.intel.com>
  */
 
+#include <linux/auxiliary_bus.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/pci.h>
@@ -15,10 +16,9 @@ 
 #include <linux/uaccess.h>
 #include <linux/overflow.h>
 
+#include "../vsec.h"
 #include "class.h"
 
-#define TELEM_DEV_NAME		"pmt_telemetry"
-
 #define TELEM_SIZE_OFFSET	0x0
 #define TELEM_GUID_OFFSET	0x4
 #define TELEM_BASE_OFFSET	0x8
@@ -79,34 +79,33 @@  static struct intel_pmt_namespace pmt_telem_ns = {
 	.pmt_header_decode = pmt_telem_header_decode,
 };
 
-static int pmt_telem_remove(struct platform_device *pdev)
+static void pmt_telem_remove(struct auxiliary_device *auxdev)
 {
-	struct pmt_telem_priv *priv = platform_get_drvdata(pdev);
+	struct pmt_telem_priv *priv = auxiliary_get_drvdata(auxdev);
 	int i;
 
 	for (i = 0; i < priv->num_entries; i++)
 		intel_pmt_dev_destroy(&priv->entry[i], &pmt_telem_ns);
-
-	return 0;
 }
 
-static int pmt_telem_probe(struct platform_device *pdev)
+static int pmt_telem_probe(struct auxiliary_device *auxdev, const struct auxiliary_device_id *id)
 {
+	struct intel_vsec_device *intel_vsec_dev = auxdev_to_ivdev(auxdev);
 	struct pmt_telem_priv *priv;
 	size_t size;
 	int i, ret;
 
-	size = struct_size(priv, entry, pdev->num_resources);
-	priv = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
+	size = struct_size(priv, entry, intel_vsec_dev->num_resources);
+	priv = devm_kzalloc(&auxdev->dev, size, GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
-	platform_set_drvdata(pdev, priv);
+	auxiliary_set_drvdata(auxdev, priv);
 
-	for (i = 0; i < pdev->num_resources; i++) {
+	for (i = 0; i < intel_vsec_dev->num_resources; i++) {
 		struct intel_pmt_entry *entry = &priv->entry[i];
 
-		ret = intel_pmt_dev_create(entry, &pmt_telem_ns, pdev, i);
+		ret = intel_pmt_dev_create(entry, &pmt_telem_ns, intel_vsec_dev, i);
 		if (ret < 0)
 			goto abort_probe;
 		if (ret)
@@ -117,32 +116,35 @@  static int pmt_telem_probe(struct platform_device *pdev)
 
 	return 0;
 abort_probe:
-	pmt_telem_remove(pdev);
+	pmt_telem_remove(auxdev);
 	return ret;
 }
 
-static struct platform_driver pmt_telem_driver = {
-	.driver = {
-		.name   = TELEM_DEV_NAME,
-	},
-	.remove = pmt_telem_remove,
-	.probe  = pmt_telem_probe,
+static const struct auxiliary_device_id pmt_telem_id_table[] = {
+	{ .name = "intel_vsec.telemetry" },
+	{}
+};
+MODULE_DEVICE_TABLE(auxiliary, pmt_telem_id_table);
+
+static struct auxiliary_driver pmt_telem_aux_driver = {
+	.id_table	= pmt_telem_id_table,
+	.remove		= pmt_telem_remove,
+	.probe		= pmt_telem_probe,
 };
 
 static int __init pmt_telem_init(void)
 {
-	return platform_driver_register(&pmt_telem_driver);
+	return auxiliary_driver_register(&pmt_telem_aux_driver);
 }
 module_init(pmt_telem_init);
 
 static void __exit pmt_telem_exit(void)
 {
-	platform_driver_unregister(&pmt_telem_driver);
+	auxiliary_driver_unregister(&pmt_telem_aux_driver);
 	xa_destroy(&telem_array);
 }
 module_exit(pmt_telem_exit);
 
 MODULE_AUTHOR("David E. Box <david.e.box@linux.intel.com>");
 MODULE_DESCRIPTION("Intel PMT Telemetry driver");
-MODULE_ALIAS("platform:" TELEM_DEV_NAME);
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
new file mode 100644
index 000000000000..c3bdd75ed690
--- /dev/null
+++ b/drivers/platform/x86/intel/vsec.c
@@ -0,0 +1,408 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Intel Vendor Specific Extended Capabilities auxiliary bus driver
+ *
+ * Copyright (c) 2021, Intel Corporation.
+ * All Rights Reserved.
+ *
+ * Author: David E. Box <david.e.box@linux.intel.com>
+ *
+ * This driver discovers and creates auxiliary devices for Intel defined PCIe
+ * "Vendor Specific" and "Designated Vendor Specific" Extended Capabilities,
+ * VSEC and DVSEC respectively. The driver supports features on specific PCIe
+ * endpoints that exist primarily to expose them.
+ */
+
+#include <linux/auxiliary_bus.h>
+#include <linux/bits.h>
+#include <linux/kernel.h>
+#include <linux/idr.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/types.h>
+
+#include "vsec.h"
+
+/* Intel DVSEC offsets */
+#define INTEL_DVSEC_ENTRIES		0xA
+#define INTEL_DVSEC_SIZE		0xB
+#define INTEL_DVSEC_TABLE		0xC
+#define INTEL_DVSEC_TABLE_BAR(x)	((x) & GENMASK(2, 0))
+#define INTEL_DVSEC_TABLE_OFFSET(x)	((x) & GENMASK(31, 3))
+#define TABLE_OFFSET_SHIFT		3
+
+static DEFINE_IDA(intel_vsec_ida);
+
+/**
+ * struct intel_vsec_header - Common fields of Intel VSEC and DVSEC registers.
+ * @rev:         Revision ID of the VSEC/DVSEC register space
+ * @length:      Length of the VSEC/DVSEC register space
+ * @id:          ID of the feature
+ * @num_entries: Number of instances of the feature
+ * @entry_size:  Size of the discovery table for each feature
+ * @tbir:        BAR containing the discovery tables
+ * @offset:      BAR offset of start of the first discovery table
+ */
+struct intel_vsec_header {
+	u8	rev;
+	u16	length;
+	u16	id;
+	u8	num_entries;
+	u8	entry_size;
+	u8	tbir;
+	u32	offset;
+};
+
+/* Platform specific data */
+struct intel_vsec_platform_info {
+	struct intel_vsec_header **capabilities;
+	unsigned long quirks;
+};
+
+enum intel_vsec_id {
+	VSEC_ID_TELEMETRY	= 2,
+	VSEC_ID_WATCHER		= 3,
+	VSEC_ID_CRASHLOG	= 4,
+};
+
+static enum intel_vsec_id intel_vsec_allow_list[] = {
+	VSEC_ID_TELEMETRY,
+	VSEC_ID_WATCHER,
+	VSEC_ID_CRASHLOG,
+};
+
+static const char *intel_vsec_name(enum intel_vsec_id id)
+{
+	switch (id) {
+	case VSEC_ID_TELEMETRY:
+		return "telemetry";
+
+	case VSEC_ID_WATCHER:
+		return "watcher";
+
+	case VSEC_ID_CRASHLOG:
+		return "crashlog";
+
+	default:
+		return NULL;
+	}
+}
+
+static bool intel_vsec_allowed(u16 id)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(intel_vsec_allow_list); i++)
+		if (intel_vsec_allow_list[i] == id)
+			return true;
+
+	return false;
+}
+
+static bool intel_vsec_disabled(u16 id, unsigned long quirks)
+{
+	switch (id) {
+	case VSEC_ID_WATCHER:
+		return !!(quirks & VSEC_QUIRK_NO_WATCHER);
+
+	case VSEC_ID_CRASHLOG:
+		return !!(quirks & VSEC_QUIRK_NO_CRASHLOG);
+
+	default:
+		return false;
+	}
+}
+
+static void intel_vsec_remove_aux(void *data)
+{
+	auxiliary_device_delete(data);
+	auxiliary_device_uninit(data);
+}
+
+static void intel_vsec_dev_release(struct device *dev)
+{
+	struct intel_vsec_device *intel_vsec_dev = dev_to_ivdev(dev);
+
+	ida_free(intel_vsec_dev->ida, intel_vsec_dev->auxdev.id);
+	kfree(intel_vsec_dev->resource);
+	kfree(intel_vsec_dev);
+}
+
+static int intel_vsec_add_aux(struct pci_dev *pdev, struct intel_vsec_device *intel_vsec_dev,
+			      const char *name)
+{
+	struct auxiliary_device *auxdev = &intel_vsec_dev->auxdev;
+	int ret;
+
+	ret = ida_alloc(intel_vsec_dev->ida, GFP_KERNEL);
+	if (ret < 0) {
+		kfree(intel_vsec_dev);
+		return ret;
+	}
+
+	auxdev->id = ret;
+	auxdev->name = name;
+	auxdev->dev.parent = &pdev->dev;
+	auxdev->dev.release = intel_vsec_dev_release;
+
+	ret = auxiliary_device_init(auxdev);
+	if (ret < 0) {
+		ida_free(intel_vsec_dev->ida, auxdev->id);
+		kfree(intel_vsec_dev->resource);
+		kfree(intel_vsec_dev);
+		return ret;
+	}
+
+	ret = auxiliary_device_add(auxdev);
+	if (ret < 0) {
+		auxiliary_device_uninit(auxdev);
+		return ret;
+	}
+
+	return devm_add_action_or_reset(&pdev->dev, intel_vsec_remove_aux, auxdev);
+}
+
+static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *header,
+			   unsigned long quirks)
+{
+	struct intel_vsec_device *intel_vsec_dev;
+	struct resource *res, *tmp;
+	int i;
+
+	if (!intel_vsec_allowed(header->id) || intel_vsec_disabled(header->id, quirks))
+		return -EINVAL;
+
+	if (!header->num_entries) {
+		dev_dbg(&pdev->dev, "Invalid 0 entry count for header id %d\n", header->id);
+		return -EINVAL;
+	}
+
+	if (!header->entry_size) {
+		dev_dbg(&pdev->dev, "Invalid 0 entry size for header id %d\n", header->id);
+		return -EINVAL;
+	}
+
+	intel_vsec_dev = kzalloc(sizeof(*intel_vsec_dev), GFP_KERNEL);
+	if (!intel_vsec_dev)
+		return -ENOMEM;
+
+	res = kcalloc(header->num_entries, sizeof(*res), GFP_KERNEL);
+	if (!res) {
+		kfree(intel_vsec_dev);
+		return -ENOMEM;
+	}
+
+	if (quirks & VSEC_QUIRK_TABLE_SHIFT)
+		header->offset >>= TABLE_OFFSET_SHIFT;
+
+	/*
+	 * The DVSEC/VSEC contains the starting offset and count for a block of
+	 * discovery tables. Create a resource array of these tables to the
+	 * auxiliary device driver.
+	 */
+	for (i = 0, tmp = res; i < header->num_entries; i++, tmp++) {
+		tmp->start = pdev->resource[header->tbir].start +
+			     header->offset + i * (header->entry_size * sizeof(u32));
+		tmp->end = tmp->start + (header->entry_size * sizeof(u32)) - 1;
+		tmp->flags = IORESOURCE_MEM;
+	}
+
+	intel_vsec_dev->pcidev = pdev;
+	intel_vsec_dev->resource = res;
+	intel_vsec_dev->num_resources = header->num_entries;
+	intel_vsec_dev->quirks = quirks;
+	intel_vsec_dev->ida = &intel_vsec_ida;
+
+	return intel_vsec_add_aux(pdev, intel_vsec_dev, intel_vsec_name(header->id));
+}
+
+static bool intel_vsec_walk_header(struct pci_dev *pdev, unsigned long quirks,
+				struct intel_vsec_header **header)
+{
+	bool have_devices = false;
+	int ret;
+
+	for ( ; *header; header++) {
+		ret = intel_vsec_add_dev(pdev, *header, quirks);
+		if (ret)
+			dev_info(&pdev->dev, "Could not add device for DVSEC id %d\n",
+				 (*header)->id);
+		else
+			have_devices = true;
+	}
+
+	return have_devices;
+}
+
+static bool intel_vsec_walk_dvsec(struct pci_dev *pdev, unsigned long quirks)
+{
+	bool have_devices = false;
+	int pos = 0;
+
+	do {
+		struct intel_vsec_header header;
+		u32 table, hdr;
+		u16 vid;
+		int ret;
+
+		pos = pci_find_next_ext_capability(pdev, pos, PCI_EXT_CAP_ID_DVSEC);
+		if (!pos)
+			break;
+
+		pci_read_config_dword(pdev, pos + PCI_DVSEC_HEADER1, &hdr);
+		vid = PCI_DVSEC_HEADER1_VID(hdr);
+		if (vid != PCI_VENDOR_ID_INTEL)
+			continue;
+
+		/* Support only revision 1 */
+		header.rev = PCI_DVSEC_HEADER1_REV(hdr);
+		if (header.rev != 1) {
+			dev_info(&pdev->dev, "Unsupported DVSEC revision %d\n", header.rev);
+			continue;
+		}
+
+		header.length = PCI_DVSEC_HEADER1_LEN(hdr);
+
+		pci_read_config_byte(pdev, pos + INTEL_DVSEC_ENTRIES, &header.num_entries);
+		pci_read_config_byte(pdev, pos + INTEL_DVSEC_SIZE, &header.entry_size);
+		pci_read_config_dword(pdev, pos + INTEL_DVSEC_TABLE, &table);
+
+		header.tbir = INTEL_DVSEC_TABLE_BAR(table);
+		header.offset = INTEL_DVSEC_TABLE_OFFSET(table);
+
+		pci_read_config_dword(pdev, pos + PCI_DVSEC_HEADER2, &hdr);
+		header.id = PCI_DVSEC_HEADER2_ID(hdr);
+
+		ret = intel_vsec_add_dev(pdev, &header, quirks);
+		if (ret)
+			continue;
+
+		have_devices = true;
+	} while (true);
+
+	return have_devices;
+}
+
+static bool intel_vsec_walk_vsec(struct pci_dev *pdev, unsigned long quirks)
+{
+	bool have_devices = false;
+	int pos = 0;
+
+	do {
+		struct intel_vsec_header header;
+		u32 table, hdr;
+		int ret;
+
+		pos = pci_find_next_ext_capability(pdev, pos, PCI_EXT_CAP_ID_VNDR);
+		if (!pos)
+			break;
+
+		pci_read_config_dword(pdev, pos + PCI_VNDR_HEADER, &hdr);
+
+		/* Support only revision 1 */
+		header.rev = PCI_VNDR_HEADER_REV(hdr);
+		if (header.rev != 1) {
+			dev_info(&pdev->dev, "Unsupported VSEC revision %d\n", header.rev);
+			continue;
+		}
+
+		header.id = PCI_VNDR_HEADER_ID(hdr);
+		header.length = PCI_VNDR_HEADER_LEN(hdr);
+
+		/* entry, size, and table offset are the same as DVSEC */
+		pci_read_config_byte(pdev, pos + INTEL_DVSEC_ENTRIES, &header.num_entries);
+		pci_read_config_byte(pdev, pos + INTEL_DVSEC_SIZE, &header.entry_size);
+		pci_read_config_dword(pdev, pos + INTEL_DVSEC_TABLE, &table);
+
+		header.tbir = INTEL_DVSEC_TABLE_BAR(table);
+		header.offset = INTEL_DVSEC_TABLE_OFFSET(table);
+
+		ret = intel_vsec_add_dev(pdev, &header, quirks);
+		if (ret)
+			continue;
+
+		have_devices = true;
+	} while (true);
+
+	return have_devices;
+}
+
+static int intel_vsec_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+	struct intel_vsec_platform_info *info;
+	bool have_devices = false;
+	unsigned long quirks = 0;
+	int ret;
+
+	ret = pcim_enable_device(pdev);
+	if (ret)
+		return ret;
+
+	info = (struct intel_vsec_platform_info *)id->driver_data;
+	if (info)
+		quirks = info->quirks;
+
+	if (intel_vsec_walk_dvsec(pdev, quirks))
+		have_devices = true;
+
+	if (intel_vsec_walk_vsec(pdev, quirks))
+		have_devices = true;
+
+	if (info && (info->quirks & VSEC_QUIRK_NO_DVSEC) &&
+	    intel_vsec_walk_header(pdev, quirks, info->capabilities))
+		have_devices = true;
+
+	if (!have_devices)
+		return -ENODEV;
+
+	return 0;
+}
+
+/* TGL info */
+static const struct intel_vsec_platform_info tgl_info = {
+	.quirks = VSEC_QUIRK_NO_WATCHER | VSEC_QUIRK_NO_CRASHLOG | VSEC_QUIRK_TABLE_SHIFT,
+};
+
+/* DG1 info */
+static struct intel_vsec_header dg1_telemetry = {
+	.length = 0x10,
+	.id = 2,
+	.num_entries = 1,
+	.entry_size = 3,
+	.tbir = 0,
+	.offset = 0x466000,
+};
+
+static struct intel_vsec_header *dg1_capabilities[] = {
+	&dg1_telemetry,
+	NULL
+};
+
+static const struct intel_vsec_platform_info dg1_info = {
+	.capabilities = dg1_capabilities,
+	.quirks = VSEC_QUIRK_NO_DVSEC,
+};
+
+#define PCI_DEVICE_ID_INTEL_VSEC_ADL		0x467d
+#define PCI_DEVICE_ID_INTEL_VSEC_DG1		0x490e
+#define PCI_DEVICE_ID_INTEL_VSEC_OOBMSM		0x09a7
+#define PCI_DEVICE_ID_INTEL_VSEC_TGL		0x9a0d
+static const struct pci_device_id intel_vsec_pci_ids[] = {
+	{ PCI_DEVICE_DATA(INTEL, VSEC_ADL, &tgl_info) },
+	{ PCI_DEVICE_DATA(INTEL, VSEC_DG1, &dg1_info) },
+	{ PCI_DEVICE_DATA(INTEL, VSEC_OOBMSM, NULL) },
+	{ PCI_DEVICE_DATA(INTEL, VSEC_TGL, &tgl_info) },
+	{ }
+};
+MODULE_DEVICE_TABLE(pci, intel_vsec_pci_ids);
+
+static struct pci_driver intel_vsec_pci_driver = {
+	.name = "intel_vsec",
+	.id_table = intel_vsec_pci_ids,
+	.probe = intel_vsec_pci_probe,
+};
+module_pci_driver(intel_vsec_pci_driver);
+
+MODULE_AUTHOR("David E. Box <david.e.box@linux.intel.com>");
+MODULE_DESCRIPTION("Intel Extended Capabilities auxiliary bus driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/platform/x86/intel/vsec.h b/drivers/platform/x86/intel/vsec.h
new file mode 100644
index 000000000000..4cc36678e8c5
--- /dev/null
+++ b/drivers/platform/x86/intel/vsec.h
@@ -0,0 +1,43 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _VSEC_H
+#define _VSEC_H
+
+#include <linux/auxiliary_bus.h>
+#include <linux/bits.h>
+
+struct pci_dev;
+struct resource;
+
+enum intel_vsec_quirks {
+	/* Watcher feature not supported */
+	VSEC_QUIRK_NO_WATCHER	= BIT(0),
+
+	/* Crashlog feature not supported */
+	VSEC_QUIRK_NO_CRASHLOG	= BIT(1),
+
+	/* Use shift instead of mask to read discovery table offset */
+	VSEC_QUIRK_TABLE_SHIFT	= BIT(2),
+
+	/* DVSEC not present (provided in driver data) */
+	VSEC_QUIRK_NO_DVSEC	= BIT(3),
+};
+
+struct intel_vsec_device {
+	struct auxiliary_device auxdev;
+	struct pci_dev *pcidev;
+	struct resource *resource;
+	struct ida *ida;
+	unsigned long quirks;
+	int num_resources;
+};
+
+static inline struct intel_vsec_device *dev_to_ivdev(struct device *dev)
+{
+	return container_of(dev, struct intel_vsec_device, auxdev.dev);
+}
+
+static inline struct intel_vsec_device *auxdev_to_ivdev(struct auxiliary_device *auxdev)
+{
+	return container_of(auxdev, struct intel_vsec_device, auxdev);
+}
+#endif