| Message ID | 20240325220325.1452712-1-helgaas@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: ipu-cio2: Remove unnecessary runtime PM power state setting | expand |
Hi Bjorn, On Mon, Mar 25, 2024 at 05:03:25PM -0500, Bjorn Helgaas wrote: > From: Bjorn Helgaas <bhelgaas@google.com> > > ipu-cio2 uses generic power management, and pci_pm_runtime_suspend() and > pci_pm_runtime_resume() already take care of setting the PCI device power > state, so the driver doesn't need to do it explicitly. > > Remove explicit setting to D3hot or D0 during runtime suspend and resume. > > Remove #defines that are no longer used. > > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > Cc: Hyungwoo Yang <hyungwoo.yang@intel.com> > Cc: Rajmohan Mani <rajmohan.mani@intel.com> > Cc: Vijaykumar Ramya <ramya.vijaykumar@intel.com> > Cc: Samu Onkalo <samu.onkalo@intel.com> > Cc: Jouni Högander <jouni.hogander@intel.com> > Cc: Jouni Ukkonen <jouni.ukkonen@intel.com> > Cc: Antti Laakso <antti.laakso@intel.com> > --- > This code was initially added by c2a6a07afe4a ("media: intel-ipu3: cio2: > add new MIPI-CSI2 driver"). > > Even at that time, the explicit power state setting should not have been > necessary, so maybe there's a reason for it. I have no way to test this, > so if it *is* needed, please: > > - Add a comment about the reason and > > - Convert it to use pci_set_power_state() so the PCI core knows about the > change and all the required state transition delays are observed. Thanks for the patch. The device seems to work fine with the patch applied so I presume it wasn't necessary to begin with.
diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c index c42adc5a408d..cdc43373d4e8 100644 --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c @@ -1812,11 +1812,6 @@ static int __maybe_unused cio2_runtime_suspend(struct device *dev) writel(CIO2_D0I3C_I3, base + CIO2_REG_D0I3C); dev_dbg(dev, "cio2 runtime suspend.\n"); - pci_read_config_word(pci_dev, pci_dev->pm_cap + CIO2_PMCSR_OFFSET, &pm); - pm = (pm >> CIO2_PMCSR_D0D3_SHIFT) << CIO2_PMCSR_D0D3_SHIFT; - pm |= CIO2_PMCSR_D3; - pci_write_config_word(pci_dev, pci_dev->pm_cap + CIO2_PMCSR_OFFSET, pm); - return 0; } @@ -1830,10 +1825,6 @@ static int __maybe_unused cio2_runtime_resume(struct device *dev) writel(CIO2_D0I3C_RR, base + CIO2_REG_D0I3C); dev_dbg(dev, "cio2 runtime resume.\n"); - pci_read_config_word(pci_dev, pci_dev->pm_cap + CIO2_PMCSR_OFFSET, &pm); - pm = (pm >> CIO2_PMCSR_D0D3_SHIFT) << CIO2_PMCSR_D0D3_SHIFT; - pci_write_config_word(pci_dev, pci_dev->pm_cap + CIO2_PMCSR_OFFSET, pm); - return 0; } diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.h b/drivers/media/pci/intel/ipu3/ipu3-cio2.h index d731ce8adbe3..d7cb7dae665b 100644 --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.h +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.h @@ -320,10 +320,6 @@ struct pci_dev; #define CIO2_CSIRX_DLY_CNT_TERMEN_DEFAULT 0x4 #define CIO2_CSIRX_DLY_CNT_SETTLE_DEFAULT 0x570 -#define CIO2_PMCSR_OFFSET 4U -#define CIO2_PMCSR_D0D3_SHIFT 2U -#define CIO2_PMCSR_D3 0x3 - struct cio2_csi2_timing { s32 clk_termen; s32 clk_settle;