linux-kernel - Re: [PATCH v2 34/36] platform/x86: intel_pmc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200109114753.GF2838@lahna.fi.intel.com>
Date:   Thu, 9 Jan 2020 13:47:53 +0200
From:   Mika Westerberg <mika.westerberg@...ux.intel.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Darren Hart <dvhart@...radead.org>,
        Lee Jones <lee.jones@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H . Peter Anvin" <hpa@...or.com>, x86@...nel.org,
        Zha Qipeng <qipeng.zha@...el.com>,
        Rajneesh Bhardwaj <rajneesh.bhardwaj@...ux.intel.com>,
        "David E . Box" <david.e.box@...ux.intel.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Wim Van Sebroeck <wim@...ux-watchdog.org>,
        platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 34/36] platform/x86: intel_pmc_ipc: Convert to MFD

On Thu, Jan 09, 2020 at 01:43:54PM +0200, Andy Shevchenko wrote:
> On Wed, Jan 08, 2020 at 02:41:59PM +0300, Mika Westerberg wrote:
> > This driver only creates a bunch of platform devices sharing resources
> > belonging to the PMC device. This is pretty much what MFD subsystem is
> > for so move the driver there, renaming it to intel_pmc_bxt.c which
> > should be more clear what it is.
> > 
> > MFD subsystem provides nice helper APIs for subdevice creation so
> > convert the driver to use those. Unfortunately the ACPI device includes
> > separate resources for most of the subdevices so we cannot simply call
> > mfd_add_devices() to create all of them but instead we need to call it
> > separately for each device.
> 
> Comments below, after addressing,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>

Thanks!

> > Signed-off-by: Mika Westerberg <mika.westerberg@...ux.intel.com>
> > ---
> >  drivers/mfd/Kconfig                           |  14 +-
> >  drivers/mfd/Makefile                          |   1 +
> >  .../intel_pmc_ipc.c => mfd/intel_pmc_bxt.c}   | 394 +++++++-----------
> >  drivers/platform/x86/Kconfig                  |  16 +-
> >  drivers/platform/x86/Makefile                 |   1 -
> >  .../platform/x86/intel_telemetry_debugfs.c    |   2 +-
> >  drivers/usb/typec/tcpm/Kconfig                |   2 +-
> >  .../linux/mfd/intel_pmc_bxt.h                 |  11 +-
> >  8 files changed, 171 insertions(+), 270 deletions(-)
> >  rename drivers/{platform/x86/intel_pmc_ipc.c => mfd/intel_pmc_bxt.c} (50%)
> >  rename arch/x86/include/asm/intel_pmc_ipc.h => include/linux/mfd/intel_pmc_bxt.h (83%)
> > 
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index 59515142438e..04542feffe25 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -551,7 +551,7 @@ config INTEL_SOC_PMIC
> >  
> >  config INTEL_SOC_PMIC_BXTWC
> >  	tristate "Support for Intel Broxton Whiskey Cove PMIC"
> > -	depends on INTEL_PMC_IPC
> > +	depends on MFD_INTEL_PMC_BXT
> >  	select MFD_CORE
> >  	select REGMAP_IRQ
> >  	help
> > @@ -632,6 +632,18 @@ config MFD_INTEL_MSIC
> >  	  Passage) chip. This chip embeds audio, battery, GPIO, etc.
> >  	  devices used in Intel Medfield platforms.
> >  
> > +config MFD_INTEL_PMC_BXT
> > +	tristate "Intel PMC Driver for Broxton"
> 
> > +	depends on X86 && X86_PLATFORM_DEVICES && ACPI
> 
> Is the X86_PLATFORM_DEVICES dependency compulsory?
> Quick grep shows that none of drivers (except nouveau) relies on it.

Well, we need that to be able to do the "select INTEL_SCU_IPC" below.
I'm happy to change it if you have a better alternative ;-)

> For the rest two I think we might split one per line to be consistent with
> existing example(s) in  drivers/mfd/Kconfig.

OK

> 
> > +	select INTEL_SCU_IPC
> > +	select MFD_CORE
> > +	help
> > +	  This driver provides support for PMC (Power Management
> > +	  Controller) on Intel Broxton and Apollo Lake. PMC is a
> > +	  multi-function device that exposes IPC, General Control
> > +	  Register and P-unit access. In addition this creates devices
> > +	  for iTCO watchdog and telemetry that are part of the PMC.