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