| 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
| ||
|
Date: Tue, 11 Feb 2020 18:10:32 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Mika Westerberg <mika.westerberg@...ux.intel.com>
Cc: Darren Hart <dvhart@...radead.org>,
Lee Jones <lee.jones@...aro.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.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>,
"David E . Box" <david.e.box@...ux.intel.com>,
Guenter Roeck <linux@...ck-us.net>,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
Wim Van Sebroeck <wim@...ux-watchdog.org>,
platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 00/18] platform/x86: Rework intel_scu_ipc and
intel_pmc_ipc drivers
On Tue, Feb 11, 2020 at 04:25:45PM +0300, Mika Westerberg wrote:
> Hi,
>
> Currently both intel_scu_ipc.c and intel_pmc_ipc.c implement the same SCU
> IPC communications with minor differences. This duplication does not make
> much sense so this series reworks the two drivers so that there is only a
> single implementation of the SCU IPC. In addition to that the API will be
> updated to take SCU instance pointer as an argument, and most of the
> callers will be converted to this new API. The old API is left there but
> the plan is to get rid the callers and then the old API as well (this is
> something we are working with Andy Shevchenko).
>
> The intel_pmc_ipc.c is then moved under MFD which suits better for this
> kind of a driver that pretty much sets up the SCU IPC and then creates a
> bunch of platform devices for the things sitting behind the PMC. The driver
> is renamed to intel_pmc_bxt.c which should follow the existing conventions
> under drivers/mfd (and it is only meant for Intel Broxton derivatives).
>
> This is on top of platform-driver-x86.git/for-next branch because there is
> already some cleanup work queued that re-organizes Kconfig and Makefile
> entries.
>
> I have tested this on Intel Joule (Broxton-M) board.
Thank you!
I have bunch of nit picks, but overall it's good to me.
Please, add my Rb tag wherever it applies.
>
> Previous version of the series:
>
> v4: https://www.spinics.net/lists/platform-driver-x86/msg20658.html
> v3: https://www.spinics.net/lists/platform-driver-x86/msg20583.html
> v2: https://www.spinics.net/lists/platform-driver-x86/msg20446.html
> v1: https://www.spinics.net/lists/platform-driver-x86/msg20359.html
>
> Changes from v4:
>
> * Cleanups already merged in v5.6-rc1 reducing this series to 18 patches.
> * Make SCU IPC a simple class, and now intel_scu_ipc_register() creates
> a new device that belongs to the SCU IPC class under the parent.
> * Handle refcounting using the newly created device.
> * We still call try_module_get() in intel_scu_ipc_dev_get() because we
> need to make sure the SCU IPC provider module is not unloaded but the
> SCU IPC device refcount is now increased and decreased as well.
> * Make SCU IPC code to log an error if there is a failure so that callers
> don't need to do that.
> * Replace telemetry_pltconfig_valid() with telemetry_get_pltdata().
> * Move intel_pmc_gcr_update() closer to intel_pmc_gcr_read64().
> * Use more standard "update" pattern in intel_pmc_gcr_update() and move
> check outside of the lock.
> * Use platform_get_irq_optional() instead.
> * Move iTCO resource extraction into separate helper function
> (intel_pmc_get_tco_resources()).
>
> Changes from v3:
>
> * Rename intel_scu_ipc_probe() to intel_scu_ipc_register() and _remove()
> to _unregister() accordingly.
> * Make intel_scu_ipc_register() to perform handle resource request and
> ioremap itself.
> * Add devm_intel_scu_ipc_register().
> * Improve kernel-docs of struct intel_soc_pmic.
> * Add Documentation/ABI/obsolete/sysfs-driver-intel_pmc_bxt to document the
> two sysfs attributes the driver exposes.
> * Fix typos in the MFD driver
> * Drop gcr_data_readq() wrapper
> * No need to check for !pmcdev.gcr_mem_base in the MFD accessors
> * Allocate PMC instance dynamically and pass this from the callers
> (telemetry) as well
> * Take the lock in intel_pmc_s0ix_counter_read() to be consistent with other
> register accessors (and serialize them).
> * Use kstrtoul() return value directly (new patch)
> * Use static const MFD cell and resources where possible. Take a copy of
> these before they get populated and passed to the MFD code.
> * Use module_platform_driver() in the MFD driver
> * Drop dev_dbg() prints.
> * Return -EINVAL instead of -ENXIO when platform_get_resource() for
> mandatory resources.
> * Clarify "residency" in intel_pmc_s0ix_counter_read().
>
> Changes from v2:
>
> * Added review tags from Andy
> * Patch 12: Put intel_scu_ipc_probe() prototype and implementation in one line
> * Patch 12: Correct wording in Kconfig description.
> * Patch 12: Put devm_request_irq() in one line.
> * Patch 14: Add blank line before intel_scu_ipc_dev_update() in header.
> * Patch 14: intel_scu_ipc_dev_update() move 'u8 data' to the next line in header.
> * Patch 14: Drop outlen check in intel_scu_ipc_dev_command_with_size().
> * Patch 16: Added Guenter's ack.
> * Patch 25: Put intel_scu_ipc_dev_command() call in one line.
> * Patch 25: Put intel_scu_ipc_dev_simple_command() call in one line.
> * Patch 32: Drop X86_INTEL_MID dependency from INTEL_SCU_PCI.
> * Patch 34: Split MFD_INTEL_PMC_BXT dependencies one per line.
> * Patch 35: Reorder to happen before patch 34.
> * Patch 35: Drop comma from terminating line.
>
> Changes from v1:
>
> * Update changelog of patch 16 according to what the patch actually does.
> * Add kernel-doc for struct intel_soc_pmic.
> * Move octal permission patch to be before MFD conversion.
> * Convert the intel_pmc_bxt.c to MFD APIs whilst it is being moved under
> drivers/mfd.
>
> Mika Westerberg (18):
> platform/x86: intel_scu_ipc: Split out SCU IPC functionality from the SCU driver
> platform/x86: intel_scu_ipc: Log more information if SCU IPC command fails
> platform/x86: intel_scu_ipc: Introduce new SCU IPC API
> platform/x86: intel_mid_powerbtn: Convert to use new SCU IPC API
> watchdog: intel-mid_wdt: Convert to use new SCU IPC API
> platform/x86: intel_scu_ipcutil: Convert to use new SCU IPC API
> platform/x86: intel_scu_ipc: Add managed function to register SCU IPC
> platform/x86: intel_pmc_ipc: Start using SCU IPC
> mfd: intel_soc_pmic: Add SCU IPC member to struct intel_soc_pmic
> mfd: intel_soc_pmic_bxtwc: Convert to use new SCU IPC API
> mfd: intel_soc_pmic_mrfld: Convert to use new SCU IPC API
> platform/x86: intel_telemetry: Convert to use new SCU IPC API
> platform/x86: intel_pmc_ipc: Drop intel_pmc_ipc_command()
> x86/platform/intel-mid: Add empty stubs for intel_scu_devices_[create|destroy]()
> platform/x86: intel_pmc_ipc: Move PCI IDs to intel_scu_pcidrv.c
> platform/x86: intel_telemetry: Add telemetry_get_pltdata()
> platform/x86: intel_pmc_ipc: Convert to MFD
> MAINTAINERS: Update entry for Intel Broxton PMC driver
>
> .../ABI/obsolete/sysfs-driver-intel_pmc_bxt | 22 +
> MAINTAINERS | 13 +-
> arch/x86/Kconfig | 2 +-
> arch/x86/include/asm/intel-mid.h | 9 +-
> arch/x86/include/asm/intel_pmc_ipc.h | 59 --
> arch/x86/include/asm/intel_scu_ipc.h | 114 ++-
> arch/x86/include/asm/intel_scu_ipc_legacy.h | 84 ++
> arch/x86/include/asm/intel_telemetry.h | 6 +-
> drivers/mfd/Kconfig | 20 +-
> drivers/mfd/Makefile | 1 +
> drivers/mfd/intel_pmc_bxt.c | 491 +++++++++
> drivers/mfd/intel_soc_pmic_bxtwc.c | 34 +-
> drivers/mfd/intel_soc_pmic_mrfld.c | 10 +-
> drivers/platform/x86/Kconfig | 46 +-
> drivers/platform/x86/Makefile | 2 +-
> drivers/platform/x86/intel_mid_powerbtn.c | 15 +-
> drivers/platform/x86/intel_pmc_ipc.c | 949 ------------------
> drivers/platform/x86/intel_scu_ipc.c | 452 +++++++--
> drivers/platform/x86/intel_scu_ipcutil.c | 43 +-
> drivers/platform/x86/intel_scu_pcidrv.c | 68 ++
> drivers/platform/x86/intel_telemetry_core.c | 17 +-
> .../platform/x86/intel_telemetry_debugfs.c | 15 +-
> drivers/platform/x86/intel_telemetry_pltdrv.c | 97 +-
> drivers/usb/typec/tcpm/Kconfig | 2 +-
> drivers/watchdog/intel-mid_wdt.c | 53 +-
> include/linux/mfd/intel_pmc_bxt.h | 21 +
> include/linux/mfd/intel_soc_pmic.h | 15 +
> 27 files changed, 1359 insertions(+), 1301 deletions(-)
> create mode 100644 Documentation/ABI/obsolete/sysfs-driver-intel_pmc_bxt
> delete mode 100644 arch/x86/include/asm/intel_pmc_ipc.h
> create mode 100644 arch/x86/include/asm/intel_scu_ipc_legacy.h
> create mode 100644 drivers/mfd/intel_pmc_bxt.c
> delete mode 100644 drivers/platform/x86/intel_pmc_ipc.c
> create mode 100644 drivers/platform/x86/intel_scu_pcidrv.c
> create mode 100644 include/linux/mfd/intel_pmc_bxt.h
>
> --
> 2.25.0
>
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists