Re: [PATCH 2/4] MFD: intel_pmt: Remove OOBMSM device

[David E. Box]

On Thu, 2021-07-01 at 12:23 +0100, Lee Jones wrote:
> On Thu, 01 Jul 2021, Hans de Goede wrote:
> 
> > Hi,
> > 
> > On 6/30/21 11:11 PM, David E. Box wrote:
> > > On Wed, 2021-06-30 at 11:15 +0100, Lee Jones wrote:
> > > > On Thu, 17 Jun 2021, David E. Box wrote:
> > > > 
> > > > > Unlike the other devices in intel_pmt, the Out of Band
> > > > > Management
> > > > > Services
> > > > > Module (OOBMSM) is actually not a PMT dedicated device. It
> > > > > can also
> > > > > be used
> > > > > to describe non-PMT capabilities. Like PMT, these
> > > > > capabilities are
> > > > > also
> > > > > enumerated using PCIe Vendor Specific registers in config
> > > > > space. In
> > > > > order
> > > > > to better support these devices without the confusion of a
> > > > > dependency on
> > > > > MFD_INTEL_PMT, remove the OOBMSM device from intel_pmt so
> > > > > that it
> > > > > can be
> > > > > later placed in its own driver. Since much of the same code
> > > > > will be
> > > > > used by
> > > > > intel_pmt and the new driver, create a new file with symbols
> > > > > to be
> > > > > used by
> > > > > both.
> > > > > 
> > > > > While performing this split we need to also handle the
> > > > > creation of
> > > > > platform
> > > > > devices for the non-PMT capabilities. Currently PMT devices
> > > > > are
> > > > > named by
> > > > > their capability (e.g. pmt_telemetry). Instead, generically
> > > > > name
> > > > > them by
> > > > > their capability ID (e.g. intel_extnd_cap_2). This allows the
> > > > > IDs
> > > > > to be
> > > > > created automatically.  However, to ensure that unsupported
> > > > > devices
> > > > > aren't
> > > > > created, use an allow list to specify supported capabilities.
> > > > > 
> > > > > Signed-off-by: David E. Box <david.e.box@xxxxxxxxxxxxxxx>
> > > > > ---
> > > > >  MAINTAINERS                                |   1 +
> > > > >  drivers/mfd/Kconfig                        |   4 +
> > > > >  drivers/mfd/Makefile                       |   1 +
> > > > >  drivers/mfd/intel_extended_caps.c          | 208
> > > > > +++++++++++++++++++++
> > > > 
> > > > Please consider moving this <whatever this is> out to either
> > > > drivers/pci or drivers/platform/x86.
> > > 
> > > None of the cell drivers are in MFD, only the PCI drivers from
> > > which
> > > the cells are created. I understood that these should be in MFD.
> > > But
> > > moving it to drivers/platform/x86 would be fine with me. That
> > > keeps the
> > > code together in the same subsystem. Comment from Hans or Andy? 
> > 
> > I'm fine with moving everything to drivers/platform/x86, but AFAIK
> > usually the actual code which has the MFD cells and creates the
> > child devices usually lives under drivers/mfd
> 
> Correct.  It must.
> 
> No MFD API users outside of drivers/mfd please.
> 

No problem. But these patches are not child device drivers. They take
the existing intel_pmt MFD code and split it from the device driver
(similar to how intel-lpss core code is split from the acpi and pci bus
drivers). There are 2 drivers now, PMT-only and OOBMSM, that use a
common MFD API. This is why they all reside in MFD in this patchset.
But I could move the API callers to platform/x86.

But I'd like feedback on whether this split is even needed. I'm trying
to manage the fact that one of the devices in intel_pmt will now need
support for new, non-PMT, child devices. So there would be a mismatch
between what the driver and Kconfig are named vs what it actually
supports. I considered adding all the new cells to intel_pmt and
renaming the driver to something more generic, but I understand this
will be messy for OSVs managing Kconfig options. Thanks.

David