linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 1/5] PCI: PM: x86: Drop Intel MID PCI PM support
       [not found] <8003272.NyiUUSuA9g@kreacher>
@ 2021-09-18 13:21 ` Rafael J. Wysocki
  2021-09-19 20:31   ` Andy Shevchenko
  2021-09-19 21:11   ` Ferry Toth
  2021-09-18 13:23 ` [PATCH v1 2/5] PCI: ACPI: PM: Do not use pci_platform_pm_ops for ACPI Rafael J. Wysocki
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2021-09-18 13:21 UTC (permalink / raw)
  To: Linux ACPI, Linux PCI, x86 Maintainers
  Cc: LKML, Bjorn Helgaas, Andy Shevchenko, Mika Westerberg, Len Brown

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Support for Intel MID platforms has mostly gone away with the SFI
support removal in commit 4590d98f5a4f ("sfi: Remove framework for
deprecated firmware"), but there are some pieces of it still in the
tree.  One of them is the MID PCI PM support code which gets in the
way of subsequent PCI PM simplifications and trying to update it is
rather pointless, so get rid of it completely along with the arch
code pieces that are only used by it.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

I am going to post patches removing the rest of MID support from arch/x86/
and elsewhere, but that is still quite a bit of stuff and I don't want this
simple PCI PM series to depend on that work.

---
 arch/x86/include/asm/intel-mid.h  |    3 
 arch/x86/platform/intel-mid/pwr.c |  150 --------------------------------------
 drivers/pci/Makefile              |    1 
 drivers/pci/pci-mid.c             |   77 -------------------
 4 files changed, 231 deletions(-)

Index: linux-pm/drivers/pci/Makefile
===================================================================
--- linux-pm.orig/drivers/pci/Makefile
+++ linux-pm/drivers/pci/Makefile
@@ -22,7 +22,6 @@ obj-$(CONFIG_PCI_ATS)		+= ats.o
 obj-$(CONFIG_PCI_IOV)		+= iov.o
 obj-$(CONFIG_PCI_BRIDGE_EMUL)	+= pci-bridge-emul.o
 obj-$(CONFIG_PCI_LABEL)		+= pci-label.o
-obj-$(CONFIG_X86_INTEL_MID)	+= pci-mid.o
 obj-$(CONFIG_PCI_SYSCALL)	+= syscall.o
 obj-$(CONFIG_PCI_STUB)		+= pci-stub.o
 obj-$(CONFIG_PCI_PF_STUB)	+= pci-pf-stub.o
Index: linux-pm/drivers/pci/pci-mid.c
===================================================================
--- linux-pm.orig/drivers/pci/pci-mid.c
+++ /dev/null
@@ -1,77 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Intel MID platform PM support
- *
- * Copyright (C) 2016, Intel Corporation
- *
- * Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
- */
-
-#include <linux/init.h>
-#include <linux/pci.h>
-
-#include <asm/cpu_device_id.h>
-#include <asm/intel-family.h>
-#include <asm/intel-mid.h>
-
-#include "pci.h"
-
-static bool mid_pci_power_manageable(struct pci_dev *dev)
-{
-	return true;
-}
-
-static int mid_pci_set_power_state(struct pci_dev *pdev, pci_power_t state)
-{
-	return intel_mid_pci_set_power_state(pdev, state);
-}
-
-static pci_power_t mid_pci_get_power_state(struct pci_dev *pdev)
-{
-	return intel_mid_pci_get_power_state(pdev);
-}
-
-static pci_power_t mid_pci_choose_state(struct pci_dev *pdev)
-{
-	return PCI_D3hot;
-}
-
-static int mid_pci_wakeup(struct pci_dev *dev, bool enable)
-{
-	return 0;
-}
-
-static bool mid_pci_need_resume(struct pci_dev *dev)
-{
-	return false;
-}
-
-static const struct pci_platform_pm_ops mid_pci_platform_pm = {
-	.is_manageable	= mid_pci_power_manageable,
-	.set_state	= mid_pci_set_power_state,
-	.get_state	= mid_pci_get_power_state,
-	.choose_state	= mid_pci_choose_state,
-	.set_wakeup	= mid_pci_wakeup,
-	.need_resume	= mid_pci_need_resume,
-};
-
-/*
- * This table should be in sync with the one in
- * arch/x86/platform/intel-mid/pwr.c.
- */
-static const struct x86_cpu_id lpss_cpu_ids[] = {
-	X86_MATCH_INTEL_FAM6_MODEL(ATOM_SALTWELL_MID, NULL),
-	X86_MATCH_INTEL_FAM6_MODEL(ATOM_SILVERMONT_MID, NULL),
-	{}
-};
-
-static int __init mid_pci_init(void)
-{
-	const struct x86_cpu_id *id;
-
-	id = x86_match_cpu(lpss_cpu_ids);
-	if (id)
-		pci_set_platform_pm(&mid_pci_platform_pm);
-	return 0;
-}
-arch_initcall(mid_pci_init);
Index: linux-pm/arch/x86/platform/intel-mid/pwr.c
===================================================================
--- linux-pm.orig/arch/x86/platform/intel-mid/pwr.c
+++ linux-pm/arch/x86/platform/intel-mid/pwr.c
@@ -5,13 +5,6 @@
  * Copyright (C) 2016, Intel Corporation
  *
  * Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
- *
- * Intel MID Power Management Unit device driver handles the South Complex PCI
- * devices such as GPDMA, SPI, I2C, PWM, and so on. By default PCI core
- * modifies bits in PMCSR register in the PCI configuration space. This is not
- * enough on some SoCs like Intel Tangier. In such case PCI core sets a new
- * power state of the device in question through a PM hook registered in struct
- * pci_platform_pm_ops (see drivers/pci/pci-mid.c).
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -103,11 +96,6 @@ struct mid_pwr {
 
 static struct mid_pwr *midpwr;
 
-static u32 mid_pwr_get_state(struct mid_pwr *pwr, int reg)
-{
-	return readl(pwr->regs + PM_SSS(reg));
-}
-
 static void mid_pwr_set_state(struct mid_pwr *pwr, int reg, u32 value)
 {
 	writel(value, pwr->regs + PM_SSC(reg));
@@ -150,143 +138,6 @@ static int mid_pwr_wait_for_cmd(struct m
 	return mid_pwr_wait(pwr);
 }
 
-static int __update_power_state(struct mid_pwr *pwr, int reg, int bit, int new)
-{
-	int curstate;
-	u32 power;
-	int ret;
-
-	/* Check if the device is already in desired state */
-	power = mid_pwr_get_state(pwr, reg);
-	curstate = (power >> bit) & 3;
-	if (curstate == new)
-		return 0;
-
-	/* Update the power state */
-	mid_pwr_set_state(pwr, reg, (power & ~(3 << bit)) | (new << bit));
-
-	/* Send command to SCU */
-	ret = mid_pwr_wait_for_cmd(pwr, CMD_SET_CFG);
-	if (ret)
-		return ret;
-
-	/* Check if the device is already in desired state */
-	power = mid_pwr_get_state(pwr, reg);
-	curstate = (power >> bit) & 3;
-	if (curstate != new)
-		return -EAGAIN;
-
-	return 0;
-}
-
-static pci_power_t __find_weakest_power_state(struct mid_pwr_dev *lss,
-					      struct pci_dev *pdev,
-					      pci_power_t state)
-{
-	pci_power_t weakest = PCI_D3hot;
-	unsigned int j;
-
-	/* Find device in cache or first free cell */
-	for (j = 0; j < LSS_MAX_SHARED_DEVS; j++) {
-		if (lss[j].pdev == pdev || !lss[j].pdev)
-			break;
-	}
-
-	/* Store the desired state in cache */
-	if (j < LSS_MAX_SHARED_DEVS) {
-		lss[j].pdev = pdev;
-		lss[j].state = state;
-	} else {
-		dev_WARN(&pdev->dev, "No room for device in PWRMU LSS cache\n");
-		weakest = state;
-	}
-
-	/* Find the power state we may use */
-	for (j = 0; j < LSS_MAX_SHARED_DEVS; j++) {
-		if (lss[j].state < weakest)
-			weakest = lss[j].state;
-	}
-
-	return weakest;
-}
-
-static int __set_power_state(struct mid_pwr *pwr, struct pci_dev *pdev,
-			     pci_power_t state, int id, int reg, int bit)
-{
-	const char *name;
-	int ret;
-
-	state = __find_weakest_power_state(pwr->lss[id], pdev, state);
-	name = pci_power_name(state);
-
-	ret = __update_power_state(pwr, reg, bit, (__force int)state);
-	if (ret) {
-		dev_warn(&pdev->dev, "Can't set power state %s: %d\n", name, ret);
-		return ret;
-	}
-
-	dev_vdbg(&pdev->dev, "Set power state %s\n", name);
-	return 0;
-}
-
-static int mid_pwr_set_power_state(struct mid_pwr *pwr, struct pci_dev *pdev,
-				   pci_power_t state)
-{
-	int id, reg, bit;
-	int ret;
-
-	id = intel_mid_pwr_get_lss_id(pdev);
-	if (id < 0)
-		return id;
-
-	reg = (id * LSS_PWS_BITS) / 32;
-	bit = (id * LSS_PWS_BITS) % 32;
-
-	/* We support states between PCI_D0 and PCI_D3hot */
-	if (state < PCI_D0)
-		state = PCI_D0;
-	if (state > PCI_D3hot)
-		state = PCI_D3hot;
-
-	mutex_lock(&pwr->lock);
-	ret = __set_power_state(pwr, pdev, state, id, reg, bit);
-	mutex_unlock(&pwr->lock);
-	return ret;
-}
-
-int intel_mid_pci_set_power_state(struct pci_dev *pdev, pci_power_t state)
-{
-	struct mid_pwr *pwr = midpwr;
-	int ret = 0;
-
-	might_sleep();
-
-	if (pwr && pwr->available)
-		ret = mid_pwr_set_power_state(pwr, pdev, state);
-	dev_vdbg(&pdev->dev, "set_power_state() returns %d\n", ret);
-
-	return 0;
-}
-
-pci_power_t intel_mid_pci_get_power_state(struct pci_dev *pdev)
-{
-	struct mid_pwr *pwr = midpwr;
-	int id, reg, bit;
-	u32 power;
-
-	if (!pwr || !pwr->available)
-		return PCI_UNKNOWN;
-
-	id = intel_mid_pwr_get_lss_id(pdev);
-	if (id < 0)
-		return PCI_UNKNOWN;
-
-	reg = (id * LSS_PWS_BITS) / 32;
-	bit = (id * LSS_PWS_BITS) % 32;
-	power = mid_pwr_get_state(pwr, reg);
-	return (__force pci_power_t)((power >> bit) & 3);
-}
-
 void intel_mid_pwr_power_off(void)
 {
 	struct mid_pwr *pwr = midpwr;
@@ -469,7 +320,6 @@ static const struct mid_pwr_device_info
 	.set_initial_state = tng_set_initial_state,
 };
 
-/* This table should be in sync with the one in drivers/pci/pci-mid.c */
 static const struct pci_device_id mid_pwr_pci_ids[] = {
 	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_PENWELL), (kernel_ulong_t)&pnw_info },
 	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_TANGIER), (kernel_ulong_t)&tng_info },
Index: linux-pm/arch/x86/include/asm/intel-mid.h
===================================================================
--- linux-pm.orig/arch/x86/include/asm/intel-mid.h
+++ linux-pm/arch/x86/include/asm/intel-mid.h
@@ -10,9 +10,6 @@
 #include <linux/pci.h>
 
 extern int intel_mid_pci_init(void);
-extern int intel_mid_pci_set_power_state(struct pci_dev *pdev, pci_power_t state);
-extern pci_power_t intel_mid_pci_get_power_state(struct pci_dev *pdev);
-
 extern void intel_mid_pwr_power_off(void);
 
 #define INTEL_MID_PWR_LSS_OFFSET	4




^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v1 2/5] PCI: ACPI: PM: Do not use pci_platform_pm_ops for ACPI
       [not found] <8003272.NyiUUSuA9g@kreacher>
  2021-09-18 13:21 ` [PATCH v1 1/5] PCI: PM: x86: Drop Intel MID PCI PM support Rafael J. Wysocki
@ 2021-09-18 13:23 ` Rafael J. Wysocki
  2021-09-18 13:24 ` [PATCH v1 3/5] PCI: PM: Rearrange code in pci_target_state() Rafael J. Wysocki
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2021-09-18 13:23 UTC (permalink / raw)
  To: Linux ACPI, Linux PCI
  Cc: LKML, Bjorn Helgaas, Andy Shevchenko, Mika Westerberg, x86 Maintainers

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Using struct pci_platform_pm_ops for ACPI adds unnecessary
indirection to the interactions between the PCI core and ACPI PM,
which is also subject to retpolines.

Moreover, it is not particularly clear from the current code that,
as far as PCI PM is concerned, "platform" really means just ACPI as
long as CONFIG_ACPI is set and the ACPI support is not disabled
(either via the kernel command line or because there are no usable
ACPI tables on the system).

To address the above, rework the PCI PM code to invoke ACPI PM
functions directly as needed and drop acpi_pci_platform_pm that
is not necessary any more.  Accordingly, update some of the ACPI PM
functions in question to do extra checks in case the ACPI support
is disabled (which previously was taken care of by avoiding to set
the pci_platform_ops pointer by PCI in those cases).

Having done that, also drop struct pci_platform_pm_ops that has no
more users.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/pci/pci-acpi.c |   42 +++++++++-----------
 drivers/pci/pci.c      |   99 ++++++++++---------------------------------------
 drivers/pci/pci.h      |   77 ++++++++++++++++++--------------------
 3 files changed, 79 insertions(+), 139 deletions(-)

Index: linux-pm/drivers/pci/pci-acpi.c
===================================================================
--- linux-pm.orig/drivers/pci/pci-acpi.c
+++ linux-pm/drivers/pci/pci-acpi.c
@@ -906,10 +906,13 @@ acpi_status pci_acpi_add_pm_notifier(str
  *	choose highest power _SxD or any lower power
  */
 
-static pci_power_t acpi_pci_choose_state(struct pci_dev *pdev)
+pci_power_t acpi_pci_choose_state(struct pci_dev *pdev)
 {
 	int acpi_state, d_max;
 
+	if (acpi_pci_disabled)
+		return PCI_POWER_ERROR;
+
 	if (pdev->no_d3cold)
 		d_max = ACPI_STATE_D3_HOT;
 	else
@@ -965,7 +968,7 @@ int pci_dev_acpi_reset(struct pci_dev *d
 	return 0;
 }
 
-static bool acpi_pci_power_manageable(struct pci_dev *dev)
+bool acpi_pci_power_manageable(struct pci_dev *dev)
 {
 	struct acpi_device *adev = ACPI_COMPANION(&dev->dev);
 
@@ -974,13 +977,13 @@ static bool acpi_pci_power_manageable(st
 	return acpi_device_power_manageable(adev);
 }
 
-static bool acpi_pci_bridge_d3(struct pci_dev *dev)
+bool acpi_pci_bridge_d3(struct pci_dev *dev)
 {
 	const union acpi_object *obj;
 	struct acpi_device *adev;
 	struct pci_dev *rpdev;
 
-	if (!dev->is_hotplug_bridge)
+	if (acpi_pci_disabled || !dev->is_hotplug_bridge)
 		return false;
 
 	/* Assume D3 support if the bridge is power-manageable by ACPI. */
@@ -1008,7 +1011,7 @@ static bool acpi_pci_bridge_d3(struct pc
 	return obj->integer.value == 1;
 }
 
-static int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
+int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
 {
 	struct acpi_device *adev = ACPI_COMPANION(&dev->dev);
 	static const u8 state_conv[] = {
@@ -1046,7 +1049,7 @@ static int acpi_pci_set_power_state(stru
 	return error;
 }
 
-static pci_power_t acpi_pci_get_power_state(struct pci_dev *dev)
+pci_power_t acpi_pci_get_power_state(struct pci_dev *dev)
 {
 	struct acpi_device *adev = ACPI_COMPANION(&dev->dev);
 	static const pci_power_t state_conv[] = {
@@ -1068,7 +1071,7 @@ static pci_power_t acpi_pci_get_power_st
 	return state_conv[state];
 }
 
-static void acpi_pci_refresh_power_state(struct pci_dev *dev)
+void acpi_pci_refresh_power_state(struct pci_dev *dev)
 {
 	struct acpi_device *adev = ACPI_COMPANION(&dev->dev);
 
@@ -1093,17 +1096,23 @@ static int acpi_pci_propagate_wakeup(str
 	return 0;
 }
 
-static int acpi_pci_wakeup(struct pci_dev *dev, bool enable)
+int acpi_pci_wakeup(struct pci_dev *dev, bool enable)
 {
+	if (acpi_pci_disabled)
+		return 0;
+
 	if (acpi_pm_device_can_wakeup(&dev->dev))
 		return acpi_pm_set_device_wakeup(&dev->dev, enable);
 
 	return acpi_pci_propagate_wakeup(dev->bus, enable);
 }
 
-static bool acpi_pci_need_resume(struct pci_dev *dev)
+bool acpi_pci_need_resume(struct pci_dev *dev)
 {
-	struct acpi_device *adev = ACPI_COMPANION(&dev->dev);
+	struct acpi_device *adev;
+
+	if (acpi_pci_disabled)
+		return false;
 
 	/*
 	 * In some cases (eg. Samsung 305V4A) leaving a bridge in suspend over
@@ -1118,6 +1127,7 @@ static bool acpi_pci_need_resume(struct
 	if (!adev || !acpi_device_power_manageable(adev))
 		return false;
 
+	adev = ACPI_COMPANION(&dev->dev);
 	if (adev->wakeup.flags.valid &&
 	    device_may_wakeup(&dev->dev) != !!adev->wakeup.prepare_count)
 		return true;
@@ -1128,17 +1138,6 @@ static bool acpi_pci_need_resume(struct
 	return !!adev->power.flags.dsw_present;
 }
 
-static const struct pci_platform_pm_ops acpi_pci_platform_pm = {
-	.bridge_d3 = acpi_pci_bridge_d3,
-	.is_manageable = acpi_pci_power_manageable,
-	.set_state = acpi_pci_set_power_state,
-	.get_state = acpi_pci_get_power_state,
-	.refresh_state = acpi_pci_refresh_power_state,
-	.choose_state = acpi_pci_choose_state,
-	.set_wakeup = acpi_pci_wakeup,
-	.need_resume = acpi_pci_need_resume,
-};
-
 void acpi_pci_add_bus(struct pci_bus *bus)
 {
 	union acpi_object *obj;
@@ -1448,7 +1447,6 @@ static int __init acpi_pci_init(void)
 	if (acpi_pci_disabled)
 		return 0;
 
-	pci_set_platform_pm(&acpi_pci_platform_pm);
 	acpi_pci_slot_init();
 	acpiphp_init();
 
Index: linux-pm/drivers/pci/pci.c
===================================================================
--- linux-pm.orig/drivers/pci/pci.c
+++ linux-pm/drivers/pci/pci.c
@@ -972,63 +972,6 @@ static void pci_restore_bars(struct pci_
 		pci_update_resource(dev, i);
 }
 
-static const struct pci_platform_pm_ops *pci_platform_pm;
-
-int pci_set_platform_pm(const struct pci_platform_pm_ops *ops)
-{
-	if (!ops->is_manageable || !ops->set_state  || !ops->get_state ||
-	    !ops->choose_state  || !ops->set_wakeup || !ops->need_resume)
-		return -EINVAL;
-	pci_platform_pm = ops;
-	return 0;
-}
-
-static inline bool platform_pci_power_manageable(struct pci_dev *dev)
-{
-	return pci_platform_pm ? pci_platform_pm->is_manageable(dev) : false;
-}
-
-static inline int platform_pci_set_power_state(struct pci_dev *dev,
-					       pci_power_t t)
-{
-	return pci_platform_pm ? pci_platform_pm->set_state(dev, t) : -ENOSYS;
-}
-
-static inline pci_power_t platform_pci_get_power_state(struct pci_dev *dev)
-{
-	return pci_platform_pm ? pci_platform_pm->get_state(dev) : PCI_UNKNOWN;
-}
-
-static inline void platform_pci_refresh_power_state(struct pci_dev *dev)
-{
-	if (pci_platform_pm && pci_platform_pm->refresh_state)
-		pci_platform_pm->refresh_state(dev);
-}
-
-static inline pci_power_t platform_pci_choose_state(struct pci_dev *dev)
-{
-	return pci_platform_pm ?
-			pci_platform_pm->choose_state(dev) : PCI_POWER_ERROR;
-}
-
-static inline int platform_pci_set_wakeup(struct pci_dev *dev, bool enable)
-{
-	return pci_platform_pm ?
-			pci_platform_pm->set_wakeup(dev, enable) : -ENODEV;
-}
-
-static inline bool platform_pci_need_resume(struct pci_dev *dev)
-{
-	return pci_platform_pm ? pci_platform_pm->need_resume(dev) : false;
-}
-
-static inline bool platform_pci_bridge_d3(struct pci_dev *dev)
-{
-	if (pci_platform_pm && pci_platform_pm->bridge_d3)
-		return pci_platform_pm->bridge_d3(dev);
-	return false;
-}
-
 /**
  * pci_raw_set_power_state - Use PCI PM registers to set the power state of
  *			     given PCI device
@@ -1163,7 +1106,7 @@ static int pci_raw_set_power_state(struc
  */
 void pci_update_current_state(struct pci_dev *dev, pci_power_t state)
 {
-	if (platform_pci_get_power_state(dev) == PCI_D3cold ||
+	if (acpi_pci_get_power_state(dev) == PCI_D3cold ||
 	    !pci_device_is_present(dev)) {
 		dev->current_state = PCI_D3cold;
 	} else if (dev->pm_cap) {
@@ -1180,19 +1123,19 @@ void pci_update_current_state(struct pci
  * pci_refresh_power_state - Refresh the given device's power state data
  * @dev: Target PCI device.
  *
- * Ask the platform to refresh the devices power state information and invoke
- * pci_update_current_state() to update its current PCI power state.
+ * Use ACPI (if available) to refresh the devices power state information and
+ * invoke pci_update_current_state() to update its current PCI power state.
  */
 void pci_refresh_power_state(struct pci_dev *dev)
 {
-	if (platform_pci_power_manageable(dev))
-		platform_pci_refresh_power_state(dev);
+	if (acpi_pci_power_manageable(dev))
+		acpi_pci_refresh_power_state(dev);
 
 	pci_update_current_state(dev, dev->current_state);
 }
 
 /**
- * pci_platform_power_transition - Use platform to change device power state
+ * pci_platform_power_transition - Use ACPI to change device power state
  * @dev: PCI device to handle.
  * @state: State to put the device into.
  */
@@ -1200,8 +1143,8 @@ int pci_platform_power_transition(struct
 {
 	int error;
 
-	if (platform_pci_power_manageable(dev)) {
-		error = platform_pci_set_power_state(dev, state);
+	if (acpi_pci_power_manageable(dev)) {
+		error = acpi_pci_set_power_state(dev, state);
 		if (!error)
 			pci_update_current_state(dev, state);
 	} else
@@ -1404,7 +1347,7 @@ pci_power_t pci_choose_state(struct pci_
 	if (!dev->pm_cap)
 		return PCI_D0;
 
-	ret = platform_pci_choose_state(dev);
+	ret = acpi_pci_choose_state(dev);
 	if (ret != PCI_POWER_ERROR)
 		return ret;
 
@@ -2512,13 +2455,13 @@ static int __pci_enable_wake(struct pci_
 			pci_pme_active(dev, true);
 		else
 			ret = 1;
-		error = platform_pci_set_wakeup(dev, true);
+		error = acpi_pci_wakeup(dev, true);
 		if (ret)
 			ret = error;
 		if (!ret)
 			dev->wakeup_prepared = true;
 	} else {
-		platform_pci_set_wakeup(dev, false);
+		acpi_pci_wakeup(dev, false);
 		pci_pme_active(dev, false);
 		dev->wakeup_prepared = false;
 	}
@@ -2571,19 +2514,21 @@ EXPORT_SYMBOL(pci_wake_from_d3);
  * @dev: PCI device
  * @wakeup: Whether or not wakeup functionality will be enabled for the device.
  *
- * Use underlying platform code to find a supported low power state for @dev.
- * If the platform can't manage @dev, return the deepest state from which it
- * can generate wake events, based on any available PME info.
+ * Use ACPI (if available) to find a supported low power state for @dev.  If
+ * ACPI cannot manage PM for @dev, return the deepest state from which it can
+ * generate wake events, based on any available PME info.
  */
 static pci_power_t pci_target_state(struct pci_dev *dev, bool wakeup)
 {
 	pci_power_t target_state = PCI_D3hot;
 
-	if (platform_pci_power_manageable(dev)) {
+	if (acpi_pci_power_manageable(dev)) {
 		/*
-		 * Call the platform to find the target state for the device.
+		 * Use ACPI to find the target state for the device.  If it
+		 * cannot do that or the power state returned by it cannot be
+		 * used, return D3hot.
 		 */
-		pci_power_t state = platform_pci_choose_state(dev);
+		pci_power_t state = acpi_pci_choose_state(dev);
 
 		switch (state) {
 		case PCI_POWER_ERROR:
@@ -2780,13 +2725,13 @@ bool pci_dev_need_resume(struct pci_dev
 	struct device *dev = &pci_dev->dev;
 	pci_power_t target_state;
 
-	if (!pm_runtime_suspended(dev) || platform_pci_need_resume(pci_dev))
+	if (!pm_runtime_suspended(dev) || acpi_pci_need_resume(pci_dev))
 		return true;
 
 	target_state = pci_target_state(pci_dev, device_may_wakeup(dev));
 
 	/*
-	 * If the earlier platform check has not triggered, D3cold is just power
+	 * If the earlier ACPI check has not triggered, D3cold is just power
 	 * removal on top of D3hot, so no need to resume the device in that
 	 * case.
 	 */
@@ -2927,7 +2872,7 @@ bool pci_bridge_d3_possible(struct pci_d
 			return true;
 
 		/* Platform might know better if the bridge supports D3 */
-		if (platform_pci_bridge_d3(bridge))
+		if (acpi_pci_bridge_d3(bridge))
 			return true;
 
 		/*
Index: linux-pm/drivers/pci/pci.h
===================================================================
--- linux-pm.orig/drivers/pci/pci.h
+++ linux-pm/drivers/pci/pci.h
@@ -63,45 +63,6 @@ struct pci_cap_saved_state *pci_find_sav
 #define PCI_PM_D3HOT_WAIT       10	/* msec */
 #define PCI_PM_D3COLD_WAIT      100	/* msec */
 
-/**
- * struct pci_platform_pm_ops - Firmware PM callbacks
- *
- * @bridge_d3: Does the bridge allow entering into D3
- *
- * @is_manageable: returns 'true' if given device is power manageable by the
- *		   platform firmware
- *
- * @set_state: invokes the platform firmware to set the device's power state
- *
- * @get_state: queries the platform firmware for a device's current power state
- *
- * @refresh_state: asks the platform to refresh the device's power state data
- *
- * @choose_state: returns PCI power state of given device preferred by the
- *		  platform; to be used during system-wide transitions from a
- *		  sleeping state to the working state and vice versa
- *
- * @set_wakeup: enables/disables wakeup capability for the device
- *
- * @need_resume: returns 'true' if the given device (which is currently
- *		 suspended) needs to be resumed to be configured for system
- *		 wakeup.
- *
- * If given platform is generally capable of power managing PCI devices, all of
- * these callbacks are mandatory.
- */
-struct pci_platform_pm_ops {
-	bool (*bridge_d3)(struct pci_dev *dev);
-	bool (*is_manageable)(struct pci_dev *dev);
-	int (*set_state)(struct pci_dev *dev, pci_power_t state);
-	pci_power_t (*get_state)(struct pci_dev *dev);
-	void (*refresh_state)(struct pci_dev *dev);
-	pci_power_t (*choose_state)(struct pci_dev *dev);
-	int (*set_wakeup)(struct pci_dev *dev, bool enable);
-	bool (*need_resume)(struct pci_dev *dev);
-};
-
-int pci_set_platform_pm(const struct pci_platform_pm_ops *ops);
 void pci_update_current_state(struct pci_dev *dev, pci_power_t state);
 void pci_refresh_power_state(struct pci_dev *dev);
 int pci_power_up(struct pci_dev *dev);
@@ -725,17 +686,53 @@ int pci_acpi_program_hp_params(struct pc
 extern const struct attribute_group pci_dev_acpi_attr_group;
 void pci_set_acpi_fwnode(struct pci_dev *dev);
 int pci_dev_acpi_reset(struct pci_dev *dev, bool probe);
+bool acpi_pci_power_manageable(struct pci_dev *dev);
+bool acpi_pci_bridge_d3(struct pci_dev *dev);
+int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state);
+pci_power_t acpi_pci_get_power_state(struct pci_dev *dev);
+void acpi_pci_refresh_power_state(struct pci_dev *dev);
+int acpi_pci_wakeup(struct pci_dev *dev, bool enable);
+bool acpi_pci_need_resume(struct pci_dev *dev);
+pci_power_t acpi_pci_choose_state(struct pci_dev *pdev);
 #else
 static inline int pci_dev_acpi_reset(struct pci_dev *dev, bool probe)
 {
 	return -ENOTTY;
 }
-
 static inline void pci_set_acpi_fwnode(struct pci_dev *dev) {}
 static inline int pci_acpi_program_hp_params(struct pci_dev *dev)
 {
 	return -ENODEV;
 }
+static inline bool acpi_pci_power_manageable(struct pci_dev *dev)
+{
+	return false;
+}
+static inline bool acpi_pci_bridge_d3(struct pci_dev *dev)
+{
+	return false;
+}
+static inline int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
+{
+	return -ENODEV;
+}
+static inline pci_power_t acpi_pci_get_power_state(struct pci_dev *dev)
+{
+	return PCI_UNKNOWN;
+}
+static inline void acpi_pci_refresh_power_state(struct pci_dev *dev) {}
+static inline int acpi_pci_wakeup(struct pci_dev *dev, bool enable)
+{
+	return -ENODEV;
+}
+static inline bool acpi_pci_need_resume(struct pci_dev *dev)
+{
+	return false;
+}
+static inline pci_power_t acpi_pci_choose_state(struct pci_dev *pdev)
+{
+	return PCI_POWER_ERROR;
+}
 #endif
 
 #ifdef CONFIG_PCIEASPM




^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v1 3/5] PCI: PM: Rearrange code in  pci_target_state()
       [not found] <8003272.NyiUUSuA9g@kreacher>
  2021-09-18 13:21 ` [PATCH v1 1/5] PCI: PM: x86: Drop Intel MID PCI PM support Rafael J. Wysocki
  2021-09-18 13:23 ` [PATCH v1 2/5] PCI: ACPI: PM: Do not use pci_platform_pm_ops for ACPI Rafael J. Wysocki
@ 2021-09-18 13:24 ` Rafael J. Wysocki
  2021-09-18 13:26 ` [PATCH v1 4/5] PCI: PM: Make pci_choose_state() call pci_target_state() Rafael J. Wysocki
  2021-09-18 13:27 ` [PATCH v1 5/5] PCI: PM: ACPI: Drop unnecessary acpi_pci_power_manageable() calls Rafael J. Wysocki
  4 siblings, 0 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2021-09-18 13:24 UTC (permalink / raw)
  To: Linux ACPI, Linux PCI
  Cc: LKML, Bjorn Helgaas, Andy Shevchenko, Mika Westerberg, x86 Maintainers

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Make pci_target_state() return D3cold or D0 without checking PME
support if the current power state of the device is D3cold or if it
does not support the standard PCI PM, respectively.

Next, drop the tergat_state local variable that has become redundant
from that function.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/pci/pci.c |   23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

Index: linux-pm/drivers/pci/pci.c
===================================================================
--- linux-pm.orig/drivers/pci/pci.c
+++ linux-pm/drivers/pci/pci.c
@@ -2520,8 +2520,6 @@ EXPORT_SYMBOL(pci_wake_from_d3);
  */
 static pci_power_t pci_target_state(struct pci_dev *dev, bool wakeup)
 {
-	pci_power_t target_state = PCI_D3hot;
-
 	if (acpi_pci_power_manageable(dev)) {
 		/*
 		 * Use ACPI to find the target state for the device.  If it
@@ -2533,32 +2531,29 @@ static pci_power_t pci_target_state(stru
 		switch (state) {
 		case PCI_POWER_ERROR:
 		case PCI_UNKNOWN:
-			break;
+			return PCI_D3hot;
+
 		case PCI_D1:
 		case PCI_D2:
 			if (pci_no_d1d2(dev))
-				break;
-			fallthrough;
-		default:
-			target_state = state;
+				return PCI_D3hot;
 		}
 
-		return target_state;
+		return state;
 	}
 
-	if (!dev->pm_cap)
-		target_state = PCI_D0;
-
 	/*
 	 * If the device is in D3cold even though it's not power-manageable by
 	 * the platform, it may have been powered down by non-standard means.
 	 * Best to let it slumber.
 	 */
 	if (dev->current_state == PCI_D3cold)
-		target_state = PCI_D3cold;
+		return PCI_D3cold;
+	else if (!dev->pm_cap)
+		return PCI_D0;
 
 	if (wakeup && dev->pme_support) {
-		pci_power_t state = target_state;
+		pci_power_t state = PCI_D3hot;
 
 		/*
 		 * Find the deepest state from which the device can generate
@@ -2573,7 +2568,7 @@ static pci_power_t pci_target_state(stru
 			return PCI_D0;
 	}
 
-	return target_state;
+	return PCI_D3hot;
 }
 
 /**




^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v1 4/5] PCI: PM: Make pci_choose_state() call pci_target_state()
       [not found] <8003272.NyiUUSuA9g@kreacher>
                   ` (2 preceding siblings ...)
  2021-09-18 13:24 ` [PATCH v1 3/5] PCI: PM: Rearrange code in pci_target_state() Rafael J. Wysocki
@ 2021-09-18 13:26 ` Rafael J. Wysocki
  2021-09-18 13:27 ` [PATCH v1 5/5] PCI: PM: ACPI: Drop unnecessary acpi_pci_power_manageable() calls Rafael J. Wysocki
  4 siblings, 0 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2021-09-18 13:26 UTC (permalink / raw)
  To: Linux ACPI, Linux PCI
  Cc: LKML, Bjorn Helgaas, Andy Shevchenko, Mika Westerberg, x86 Maintainers

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The pci_choose_state() and pci_target_state() implementations are
somewhat divergent without a good reason, because they are used
for similar purposes.

Change the pci_choose_state() implementation to use pci_target_state()
internally except for transitions to the working state of the system
in which case it is expected to return D0.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/pci/pci-acpi.c |    3 --
 drivers/pci/pci.c      |   54 ++++++++++++++-----------------------------------
 2 files changed, 16 insertions(+), 41 deletions(-)

Index: linux-pm/drivers/pci/pci.c
===================================================================
--- linux-pm.orig/drivers/pci/pci.c
+++ linux-pm/drivers/pci/pci.c
@@ -1331,44 +1331,6 @@ int pci_set_power_state(struct pci_dev *
 }
 EXPORT_SYMBOL(pci_set_power_state);
 
-/**
- * pci_choose_state - Choose the power state of a PCI device
- * @dev: PCI device to be suspended
- * @state: target sleep state for the whole system. This is the value
- *	   that is passed to suspend() function.
- *
- * Returns PCI power state suitable for given device and given system
- * message.
- */
-pci_power_t pci_choose_state(struct pci_dev *dev, pm_message_t state)
-{
-	pci_power_t ret;
-
-	if (!dev->pm_cap)
-		return PCI_D0;
-
-	ret = acpi_pci_choose_state(dev);
-	if (ret != PCI_POWER_ERROR)
-		return ret;
-
-	switch (state.event) {
-	case PM_EVENT_ON:
-		return PCI_D0;
-	case PM_EVENT_FREEZE:
-	case PM_EVENT_PRETHAW:
-		/* REVISIT both freeze and pre-thaw "should" use D0 */
-	case PM_EVENT_SUSPEND:
-	case PM_EVENT_HIBERNATE:
-		return PCI_D3hot;
-	default:
-		pci_info(dev, "unrecognized suspend event %d\n",
-			 state.event);
-		BUG();
-	}
-	return PCI_D0;
-}
-EXPORT_SYMBOL(pci_choose_state);
-
 #define PCI_EXP_SAVE_REGS	7
 
 static struct pci_cap_saved_state *_pci_find_saved_cap(struct pci_dev *pci_dev,
@@ -2782,6 +2744,22 @@ void pci_dev_complete_resume(struct pci_
 	spin_unlock_irq(&dev->power.lock);
 }
 
+/**
+ * pci_choose_state - Choose the power state for a PCI device.
+ * @pci_dev: Target PCI device.
+ * @state: Target state for the whole system.
+ *
+ * Returns PCI power state suitable for @pci_dev and @state.
+ */
+pci_power_t pci_choose_state(struct pci_dev *pci_dev, pm_message_t state)
+{
+	if (state.event == PM_EVENT_ON)
+		return PCI_D0;
+
+	return pci_target_state(pci_dev, device_may_wakeup(&pci_dev->dev));
+}
+EXPORT_SYMBOL(pci_choose_state);
+
 void pci_config_pm_runtime_get(struct pci_dev *pdev)
 {
 	struct device *dev = &pdev->dev;
Index: linux-pm/drivers/pci/pci-acpi.c
===================================================================
--- linux-pm.orig/drivers/pci/pci-acpi.c
+++ linux-pm/drivers/pci/pci-acpi.c
@@ -910,9 +910,6 @@ pci_power_t acpi_pci_choose_state(struct
 {
 	int acpi_state, d_max;
 
-	if (acpi_pci_disabled)
-		return PCI_POWER_ERROR;
-
 	if (pdev->no_d3cold)
 		d_max = ACPI_STATE_D3_HOT;
 	else




^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v1 5/5] PCI: PM: ACPI: Drop unnecessary acpi_pci_power_manageable() calls
       [not found] <8003272.NyiUUSuA9g@kreacher>
                   ` (3 preceding siblings ...)
  2021-09-18 13:26 ` [PATCH v1 4/5] PCI: PM: Make pci_choose_state() call pci_target_state() Rafael J. Wysocki
@ 2021-09-18 13:27 ` Rafael J. Wysocki
  4 siblings, 0 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2021-09-18 13:27 UTC (permalink / raw)
  To: Linux ACPI, Linux PCI
  Cc: LKML, Bjorn Helgaas, Andy Shevchenko, Mika Westerberg, x86 Maintainers

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Simplify acpi_pci_power_manageable() and drop two invocations of it
that are not necessary, because the functions called when it returns
'true' do the requisite "power manageable" checks themselves.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/pci/pci-acpi.c |    4 +---
 drivers/pci/pci.c      |   16 +++++-----------
 2 files changed, 6 insertions(+), 14 deletions(-)

Index: linux-pm/drivers/pci/pci.c
===================================================================
--- linux-pm.orig/drivers/pci/pci.c
+++ linux-pm/drivers/pci/pci.c
@@ -1128,9 +1128,7 @@ void pci_update_current_state(struct pci
  */
 void pci_refresh_power_state(struct pci_dev *dev)
 {
-	if (acpi_pci_power_manageable(dev))
-		acpi_pci_refresh_power_state(dev);
-
+	acpi_pci_refresh_power_state(dev);
 	pci_update_current_state(dev, dev->current_state);
 }
 
@@ -1143,14 +1141,10 @@ int pci_platform_power_transition(struct
 {
 	int error;
 
-	if (acpi_pci_power_manageable(dev)) {
-		error = acpi_pci_set_power_state(dev, state);
-		if (!error)
-			pci_update_current_state(dev, state);
-	} else
-		error = -ENODEV;
-
-	if (error && !dev->pm_cap) /* Fall back to PCI_D0 */
+	error = acpi_pci_set_power_state(dev, state);
+	if (!error)
+		pci_update_current_state(dev, state);
+	else if (!dev->pm_cap) /* Fall back to PCI_D0 */
 		dev->current_state = PCI_D0;
 
 	return error;
Index: linux-pm/drivers/pci/pci-acpi.c
===================================================================
--- linux-pm.orig/drivers/pci/pci-acpi.c
+++ linux-pm/drivers/pci/pci-acpi.c
@@ -969,9 +969,7 @@ bool acpi_pci_power_manageable(struct pc
 {
 	struct acpi_device *adev = ACPI_COMPANION(&dev->dev);
 
-	if (!adev)
-		return false;
-	return acpi_device_power_manageable(adev);
+	return adev && acpi_device_power_manageable(adev);
 }
 
 bool acpi_pci_bridge_d3(struct pci_dev *dev)




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 1/5] PCI: PM: x86: Drop Intel MID PCI PM support
  2021-09-18 13:21 ` [PATCH v1 1/5] PCI: PM: x86: Drop Intel MID PCI PM support Rafael J. Wysocki
@ 2021-09-19 20:31   ` Andy Shevchenko
  2021-09-20 10:57     ` Rafael J. Wysocki
  2021-09-19 21:11   ` Ferry Toth
  1 sibling, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2021-09-19 20:31 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux ACPI, Linux PCI, x86 Maintainers, LKML, Bjorn Helgaas,
	Andy Shevchenko, Mika Westerberg, Len Brown

On Sun, Sep 19, 2021 at 9:01 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Support for Intel MID platforms has mostly gone away with the SFI
> support removal in commit 4590d98f5a4f ("sfi: Remove framework for
> deprecated firmware"), but there are some pieces of it still in the
> tree.  One of them is the MID PCI PM support code which gets in the
> way of subsequent PCI PM simplifications and trying to update it is
> rather pointless, so get rid of it completely along with the arch
> code pieces that are only used by it.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>
> I am going to post patches removing the rest of MID support from arch/x86/
> and elsewhere, but that is still quite a bit of stuff and I don't want this
> simple PCI PM series to depend on that work.

This is still being used by MID with ACPI assisted (*) support.
Hence, not ack.

*) ACPI layer is provided by U-Boot and can't fulfill all possible
features that ACPI may use in the Linux kernel.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 1/5] PCI: PM: x86: Drop Intel MID PCI PM support
  2021-09-18 13:21 ` [PATCH v1 1/5] PCI: PM: x86: Drop Intel MID PCI PM support Rafael J. Wysocki
  2021-09-19 20:31   ` Andy Shevchenko
@ 2021-09-19 21:11   ` Ferry Toth
  2021-09-20 10:58     ` Rafael J. Wysocki
  1 sibling, 1 reply; 11+ messages in thread
From: Ferry Toth @ 2021-09-19 21:11 UTC (permalink / raw)
  To: Rafael J. Wysocki, Linux ACPI, Linux PCI, x86 Maintainers
  Cc: LKML, Bjorn Helgaas, Andy Shevchenko, Mika Westerberg, Len Brown

Hi,

Op 18-09-2021 om 15:21 schreef Rafael J. Wysocki:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Support for Intel MID platforms has mostly gone away with the SFI
> support removal in commit 4590d98f5a4f ("sfi: Remove framework for
> deprecated firmware"), but there are some pieces of it still in the
> tree.  One of them is the MID PCI PM support code which gets in the
> way of subsequent PCI PM simplifications and trying to update it is
> rather pointless, so get rid of it completely along with the arch
> code pieces that are only used by it.

Removing PM support for MID will break (among others) Intel Edison, 
which is currently in use and running up to date vanilla kernel (v5.14) 
and user space.

I would happily test updates PM when they appear.

> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> I am going to post patches removing the rest of MID support from arch/x86/
> and elsewhere, but that is still quite a bit of stuff and I don't want this
> simple PCI PM series to depend on that work.
> 
> ---
>   arch/x86/include/asm/intel-mid.h  |    3
>   arch/x86/platform/intel-mid/pwr.c |  150 --------------------------------------
>   drivers/pci/Makefile              |    1
>   drivers/pci/pci-mid.c             |   77 -------------------
>   4 files changed, 231 deletions(-)
> 
> Index: linux-pm/drivers/pci/Makefile
> ===================================================================
> --- linux-pm.orig/drivers/pci/Makefile
> +++ linux-pm/drivers/pci/Makefile
> @@ -22,7 +22,6 @@ obj-$(CONFIG_PCI_ATS)		+= ats.o
>   obj-$(CONFIG_PCI_IOV)		+= iov.o
>   obj-$(CONFIG_PCI_BRIDGE_EMUL)	+= pci-bridge-emul.o
>   obj-$(CONFIG_PCI_LABEL)		+= pci-label.o
> -obj-$(CONFIG_X86_INTEL_MID)	+= pci-mid.o
>   obj-$(CONFIG_PCI_SYSCALL)	+= syscall.o
>   obj-$(CONFIG_PCI_STUB)		+= pci-stub.o
>   obj-$(CONFIG_PCI_PF_STUB)	+= pci-pf-stub.o
> Index: linux-pm/drivers/pci/pci-mid.c
> ===================================================================
> --- linux-pm.orig/drivers/pci/pci-mid.c
> +++ /dev/null
> @@ -1,77 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - * Intel MID platform PM support
> - *
> - * Copyright (C) 2016, Intel Corporation
> - *
> - * Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> - */
> -
> -#include <linux/init.h>
> -#include <linux/pci.h>
> -
> -#include <asm/cpu_device_id.h>
> -#include <asm/intel-family.h>
> -#include <asm/intel-mid.h>
> -
> -#include "pci.h"
> -
> -static bool mid_pci_power_manageable(struct pci_dev *dev)
> -{
> -	return true;
> -}
> -
> -static int mid_pci_set_power_state(struct pci_dev *pdev, pci_power_t state)
> -{
> -	return intel_mid_pci_set_power_state(pdev, state);
> -}
> -
> -static pci_power_t mid_pci_get_power_state(struct pci_dev *pdev)
> -{
> -	return intel_mid_pci_get_power_state(pdev);
> -}
> -
> -static pci_power_t mid_pci_choose_state(struct pci_dev *pdev)
> -{
> -	return PCI_D3hot;
> -}
> -
> -static int mid_pci_wakeup(struct pci_dev *dev, bool enable)
> -{
> -	return 0;
> -}
> -
> -static bool mid_pci_need_resume(struct pci_dev *dev)
> -{
> -	return false;
> -}
> -
> -static const struct pci_platform_pm_ops mid_pci_platform_pm = {
> -	.is_manageable	= mid_pci_power_manageable,
> -	.set_state	= mid_pci_set_power_state,
> -	.get_state	= mid_pci_get_power_state,
> -	.choose_state	= mid_pci_choose_state,
> -	.set_wakeup	= mid_pci_wakeup,
> -	.need_resume	= mid_pci_need_resume,
> -};
> -
> -/*
> - * This table should be in sync with the one in
> - * arch/x86/platform/intel-mid/pwr.c.
> - */
> -static const struct x86_cpu_id lpss_cpu_ids[] = {
> -	X86_MATCH_INTEL_FAM6_MODEL(ATOM_SALTWELL_MID, NULL),
> -	X86_MATCH_INTEL_FAM6_MODEL(ATOM_SILVERMONT_MID, NULL),
> -	{}
> -};
> -
> -static int __init mid_pci_init(void)
> -{
> -	const struct x86_cpu_id *id;
> -
> -	id = x86_match_cpu(lpss_cpu_ids);
> -	if (id)
> -		pci_set_platform_pm(&mid_pci_platform_pm);
> -	return 0;
> -}
> -arch_initcall(mid_pci_init);
> Index: linux-pm/arch/x86/platform/intel-mid/pwr.c
> ===================================================================
> --- linux-pm.orig/arch/x86/platform/intel-mid/pwr.c
> +++ linux-pm/arch/x86/platform/intel-mid/pwr.c
> @@ -5,13 +5,6 @@
>    * Copyright (C) 2016, Intel Corporation
>    *
>    * Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> - *
> - * Intel MID Power Management Unit device driver handles the South Complex PCI
> - * devices such as GPDMA, SPI, I2C, PWM, and so on. By default PCI core
> - * modifies bits in PMCSR register in the PCI configuration space. This is not
> - * enough on some SoCs like Intel Tangier. In such case PCI core sets a new
> - * power state of the device in question through a PM hook registered in struct
> - * pci_platform_pm_ops (see drivers/pci/pci-mid.c).
>    */
>   
>   #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> @@ -103,11 +96,6 @@ struct mid_pwr {
>   
>   static struct mid_pwr *midpwr;
>   
> -static u32 mid_pwr_get_state(struct mid_pwr *pwr, int reg)
> -{
> -	return readl(pwr->regs + PM_SSS(reg));
> -}
> -
>   static void mid_pwr_set_state(struct mid_pwr *pwr, int reg, u32 value)
>   {
>   	writel(value, pwr->regs + PM_SSC(reg));
> @@ -150,143 +138,6 @@ static int mid_pwr_wait_for_cmd(struct m
>   	return mid_pwr_wait(pwr);
>   }
>   
> -static int __update_power_state(struct mid_pwr *pwr, int reg, int bit, int new)
> -{
> -	int curstate;
> -	u32 power;
> -	int ret;
> -
> -	/* Check if the device is already in desired state */
> -	power = mid_pwr_get_state(pwr, reg);
> -	curstate = (power >> bit) & 3;
> -	if (curstate == new)
> -		return 0;
> -
> -	/* Update the power state */
> -	mid_pwr_set_state(pwr, reg, (power & ~(3 << bit)) | (new << bit));
> -
> -	/* Send command to SCU */
> -	ret = mid_pwr_wait_for_cmd(pwr, CMD_SET_CFG);
> -	if (ret)
> -		return ret;
> -
> -	/* Check if the device is already in desired state */
> -	power = mid_pwr_get_state(pwr, reg);
> -	curstate = (power >> bit) & 3;
> -	if (curstate != new)
> -		return -EAGAIN;
> -
> -	return 0;
> -}
> -
> -static pci_power_t __find_weakest_power_state(struct mid_pwr_dev *lss,
> -					      struct pci_dev *pdev,
> -					      pci_power_t state)
> -{
> -	pci_power_t weakest = PCI_D3hot;
> -	unsigned int j;
> -
> -	/* Find device in cache or first free cell */
> -	for (j = 0; j < LSS_MAX_SHARED_DEVS; j++) {
> -		if (lss[j].pdev == pdev || !lss[j].pdev)
> -			break;
> -	}
> -
> -	/* Store the desired state in cache */
> -	if (j < LSS_MAX_SHARED_DEVS) {
> -		lss[j].pdev = pdev;
> -		lss[j].state = state;
> -	} else {
> -		dev_WARN(&pdev->dev, "No room for device in PWRMU LSS cache\n");
> -		weakest = state;
> -	}
> -
> -	/* Find the power state we may use */
> -	for (j = 0; j < LSS_MAX_SHARED_DEVS; j++) {
> -		if (lss[j].state < weakest)
> -			weakest = lss[j].state;
> -	}
> -
> -	return weakest;
> -}
> -
> -static int __set_power_state(struct mid_pwr *pwr, struct pci_dev *pdev,
> -			     pci_power_t state, int id, int reg, int bit)
> -{
> -	const char *name;
> -	int ret;
> -
> -	state = __find_weakest_power_state(pwr->lss[id], pdev, state);
> -	name = pci_power_name(state);
> -
> -	ret = __update_power_state(pwr, reg, bit, (__force int)state);
> -	if (ret) {
> -		dev_warn(&pdev->dev, "Can't set power state %s: %d\n", name, ret);
> -		return ret;
> -	}
> -
> -	dev_vdbg(&pdev->dev, "Set power state %s\n", name);
> -	return 0;
> -}
> -
> -static int mid_pwr_set_power_state(struct mid_pwr *pwr, struct pci_dev *pdev,
> -				   pci_power_t state)
> -{
> -	int id, reg, bit;
> -	int ret;
> -
> -	id = intel_mid_pwr_get_lss_id(pdev);
> -	if (id < 0)
> -		return id;
> -
> -	reg = (id * LSS_PWS_BITS) / 32;
> -	bit = (id * LSS_PWS_BITS) % 32;
> -
> -	/* We support states between PCI_D0 and PCI_D3hot */
> -	if (state < PCI_D0)
> -		state = PCI_D0;
> -	if (state > PCI_D3hot)
> -		state = PCI_D3hot;
> -
> -	mutex_lock(&pwr->lock);
> -	ret = __set_power_state(pwr, pdev, state, id, reg, bit);
> -	mutex_unlock(&pwr->lock);
> -	return ret;
> -}
> -
> -int intel_mid_pci_set_power_state(struct pci_dev *pdev, pci_power_t state)
> -{
> -	struct mid_pwr *pwr = midpwr;
> -	int ret = 0;
> -
> -	might_sleep();
> -
> -	if (pwr && pwr->available)
> -		ret = mid_pwr_set_power_state(pwr, pdev, state);
> -	dev_vdbg(&pdev->dev, "set_power_state() returns %d\n", ret);
> -
> -	return 0;
> -}
> -
> -pci_power_t intel_mid_pci_get_power_state(struct pci_dev *pdev)
> -{
> -	struct mid_pwr *pwr = midpwr;
> -	int id, reg, bit;
> -	u32 power;
> -
> -	if (!pwr || !pwr->available)
> -		return PCI_UNKNOWN;
> -
> -	id = intel_mid_pwr_get_lss_id(pdev);
> -	if (id < 0)
> -		return PCI_UNKNOWN;
> -
> -	reg = (id * LSS_PWS_BITS) / 32;
> -	bit = (id * LSS_PWS_BITS) % 32;
> -	power = mid_pwr_get_state(pwr, reg);
> -	return (__force pci_power_t)((power >> bit) & 3);
> -}
> -
>   void intel_mid_pwr_power_off(void)
>   {
>   	struct mid_pwr *pwr = midpwr;
> @@ -469,7 +320,6 @@ static const struct mid_pwr_device_info
>   	.set_initial_state = tng_set_initial_state,
>   };
>   
> -/* This table should be in sync with the one in drivers/pci/pci-mid.c */
>   static const struct pci_device_id mid_pwr_pci_ids[] = {
>   	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_PENWELL), (kernel_ulong_t)&pnw_info },
>   	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_TANGIER), (kernel_ulong_t)&tng_info },
> Index: linux-pm/arch/x86/include/asm/intel-mid.h
> ===================================================================
> --- linux-pm.orig/arch/x86/include/asm/intel-mid.h
> +++ linux-pm/arch/x86/include/asm/intel-mid.h
> @@ -10,9 +10,6 @@
>   #include <linux/pci.h>
>   
>   extern int intel_mid_pci_init(void);
> -extern int intel_mid_pci_set_power_state(struct pci_dev *pdev, pci_power_t state);
> -extern pci_power_t intel_mid_pci_get_power_state(struct pci_dev *pdev);
> -
>   extern void intel_mid_pwr_power_off(void);
>   
>   #define INTEL_MID_PWR_LSS_OFFSET	4
> 
> 
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 1/5] PCI: PM: x86: Drop Intel MID PCI PM support
  2021-09-19 20:31   ` Andy Shevchenko
@ 2021-09-20 10:57     ` Rafael J. Wysocki
  2021-09-21 12:16       ` Andy Shevchenko
  0 siblings, 1 reply; 11+ messages in thread
From: Rafael J. Wysocki @ 2021-09-20 10:57 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rafael J. Wysocki, Linux ACPI, Linux PCI, x86 Maintainers, LKML,
	Bjorn Helgaas, Andy Shevchenko, Mika Westerberg, Len Brown

On Sun, Sep 19, 2021 at 10:32 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Sun, Sep 19, 2021 at 9:01 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Support for Intel MID platforms has mostly gone away with the SFI
> > support removal in commit 4590d98f5a4f ("sfi: Remove framework for
> > deprecated firmware"), but there are some pieces of it still in the
> > tree.  One of them is the MID PCI PM support code which gets in the
> > way of subsequent PCI PM simplifications and trying to update it is
> > rather pointless, so get rid of it completely along with the arch
> > code pieces that are only used by it.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >
> > I am going to post patches removing the rest of MID support from arch/x86/
> > and elsewhere, but that is still quite a bit of stuff and I don't want this
> > simple PCI PM series to depend on that work.
>
> This is still being used by MID with ACPI assisted (*) support.
> Hence, not ack.
>
> *) ACPI layer is provided by U-Boot and can't fulfill all possible
> features that ACPI may use in the Linux kernel.

OK, good to know.

I'm not sure how this PCI PM stuff works with ACPI.  It looks like
this relies on a specific ordering of arch_initcall() calls for
correctness which is sort of fragile.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 1/5] PCI: PM: x86: Drop Intel MID PCI PM support
  2021-09-19 21:11   ` Ferry Toth
@ 2021-09-20 10:58     ` Rafael J. Wysocki
  0 siblings, 0 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2021-09-20 10:58 UTC (permalink / raw)
  To: Ferry Toth
  Cc: Rafael J. Wysocki, Linux ACPI, Linux PCI, x86 Maintainers, LKML,
	Bjorn Helgaas, Andy Shevchenko, Mika Westerberg, Len Brown

On Sun, Sep 19, 2021 at 11:11 PM Ferry Toth <fntoth@gmail.com> wrote:
>
> Hi,
>
> Op 18-09-2021 om 15:21 schreef Rafael J. Wysocki:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Support for Intel MID platforms has mostly gone away with the SFI
> > support removal in commit 4590d98f5a4f ("sfi: Remove framework for
> > deprecated firmware"), but there are some pieces of it still in the
> > tree.  One of them is the MID PCI PM support code which gets in the
> > way of subsequent PCI PM simplifications and trying to update it is
> > rather pointless, so get rid of it completely along with the arch
> > code pieces that are only used by it.
>
> Removing PM support for MID will break (among others) Intel Edison,
> which is currently in use and running up to date vanilla kernel (v5.14)
> and user space.
>
> I would happily test updates PM when they appear.

Thanks for the information, I'll use a different approach then.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 1/5] PCI: PM: x86: Drop Intel MID PCI PM support
  2021-09-20 10:57     ` Rafael J. Wysocki
@ 2021-09-21 12:16       ` Andy Shevchenko
  2021-09-21 14:23         ` Rafael J. Wysocki
  0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2021-09-21 12:16 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Rafael J. Wysocki, Linux ACPI, Linux PCI, x86 Maintainers, LKML,
	Bjorn Helgaas, Andy Shevchenko, Mika Westerberg, Len Brown

On Mon, Sep 20, 2021 at 1:57 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Sun, Sep 19, 2021 at 10:32 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Sun, Sep 19, 2021 at 9:01 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:

...

> > > I am going to post patches removing the rest of MID support from arch/x86/
> > > and elsewhere, but that is still quite a bit of stuff and I don't want this
> > > simple PCI PM series to depend on that work.
> >
> > This is still being used by MID with ACPI assisted (*) support.
> > Hence, not ack.
> >
> > *) ACPI layer is provided by U-Boot and can't fulfill all possible
> > features that ACPI may use in the Linux kernel.
>
> OK, good to know.
>
> I'm not sure how this PCI PM stuff works with ACPI.

It doesn't that is the point. The PCI is very interesting there and
what I meant is that the ACPI implementation I have provided via
U-Boot does not cover these. If you have any hints/ideas how it may be
handled, I am all ears!

> It looks like
> this relies on a specific ordering of arch_initcall() calls for
> correctness which is sort of fragile.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 1/5] PCI: PM: x86: Drop Intel MID PCI PM support
  2021-09-21 12:16       ` Andy Shevchenko
@ 2021-09-21 14:23         ` Rafael J. Wysocki
  0 siblings, 0 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2021-09-21 14:23 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rafael J. Wysocki, Rafael J. Wysocki, Linux ACPI, Linux PCI,
	x86 Maintainers, LKML, Bjorn Helgaas, Andy Shevchenko,
	Mika Westerberg, Len Brown

On Tue, Sep 21, 2021 at 2:17 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Mon, Sep 20, 2021 at 1:57 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> > On Sun, Sep 19, 2021 at 10:32 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > > On Sun, Sep 19, 2021 at 9:01 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> ...
>
> > > > I am going to post patches removing the rest of MID support from arch/x86/
> > > > and elsewhere, but that is still quite a bit of stuff and I don't want this
> > > > simple PCI PM series to depend on that work.
> > >
> > > This is still being used by MID with ACPI assisted (*) support.
> > > Hence, not ack.
> > >
> > > *) ACPI layer is provided by U-Boot and can't fulfill all possible
> > > features that ACPI may use in the Linux kernel.
> >
> > OK, good to know.
> >
> > I'm not sure how this PCI PM stuff works with ACPI.
>
> It doesn't that is the point. The PCI is very interesting there and
> what I meant is that the ACPI implementation I have provided via
> U-Boot does not cover these.

That's OK.  It just means that these devices are not power-manageable
via ACPI on the platforms in question, but the MID PCI PM code is
present in the kernel, so we don't need analogous code in AML in the
ACPI tables.

My point is that something like the v2 of this patch series
(https://lore.kernel.org/linux-acpi/1800633.tdWV9SEqCh@kreacher/T/#m1ec249724a5ad5ad358b0ed8e149e3926934955d)
is needed to prevent ACPI from overtaking the PM for the PCI devices
on the platform once we've decided to use the MID PM for them.

Now, if it is necessary to use ACPI PM for some devices and the MID PM
for other devices on the same platform, the latter needs to grow a
meaningful "power manageable" function that needs to be used in the
code as appropriate.

> If you have any hints/ideas how it may be handled, I am all ears!

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-09-21 14:23 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <8003272.NyiUUSuA9g@kreacher>
2021-09-18 13:21 ` [PATCH v1 1/5] PCI: PM: x86: Drop Intel MID PCI PM support Rafael J. Wysocki
2021-09-19 20:31   ` Andy Shevchenko
2021-09-20 10:57     ` Rafael J. Wysocki
2021-09-21 12:16       ` Andy Shevchenko
2021-09-21 14:23         ` Rafael J. Wysocki
2021-09-19 21:11   ` Ferry Toth
2021-09-20 10:58     ` Rafael J. Wysocki
2021-09-18 13:23 ` [PATCH v1 2/5] PCI: ACPI: PM: Do not use pci_platform_pm_ops for ACPI Rafael J. Wysocki
2021-09-18 13:24 ` [PATCH v1 3/5] PCI: PM: Rearrange code in pci_target_state() Rafael J. Wysocki
2021-09-18 13:26 ` [PATCH v1 4/5] PCI: PM: Make pci_choose_state() call pci_target_state() Rafael J. Wysocki
2021-09-18 13:27 ` [PATCH v1 5/5] PCI: PM: ACPI: Drop unnecessary acpi_pci_power_manageable() calls Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).