media: atomisp: Fix probe()/remove() power-management

Fix probe()/remove() power-management:

-Currently the driver uses pm_runtime_put_noidle() and relies on
 userspace to open + close the /dev/video# node at least once to
 actually turn the ISP off. Replace the pm_runtime_put_noidle()
 with pm_runtime_put_sync() to make sure that the device is turned
 off without relying on userspace for this.
 This also ensures that atomisp_css_init() is run (by atomisp_power_on())
 if the first userspace process opening /dev/video# wants to do more then
 just query the v4l2-caps.
 As part of this change move the pm setup code in probe() to just before
 calling v4l2_async_nf_register() which registers the /dev/* nodes, so
 that the device is left on for the entirety of the probe() function.

-Remove the turning off of the atomisp from the exit-error path,
 the PCI subsystem and subsequent probe() attempts expect the device
 to be in the on state when probe() fails.
 This also fixes the atomisp driver causing the system to hang / freeze
 when its firmware is missing. This freeze is caused by an unidentified
 bug in the power-off on exit-error code which is now removed.

-Make sure that remove() properly powers on the device by replacing
 pm_runtime_get_noresume() with pm_runtime_get_sync. The PCI subsystem
 and subsequent probe() attempts expect the device to be in the on state
 after unbinding the driver.

-Note this also swaps the order of put()/allow() and forbid()/get()
 so that the sync versions actually work by calling allow() before put()
 and forbid() after get()

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index 7d99b53107b0498f0642127d7e5cf5a909e07f79..6e8c9add35f942033c6fa856ec765c39f8761c9f 100644 (file)
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -1200,7 +1200,6 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
        struct atomisp_device *isp;
        unsigned int start;
        int err, val;
-       u32 irq;
 
        /* Pointer to struct device. */
        atomisp_dev = &pdev->dev;
@@ -1334,11 +1333,8 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
 
        /* Load isp firmware from user space */
        isp->firmware = atomisp_load_firmware(isp);
-       if (!isp->firmware) {
-               err = -ENOENT;
-               dev_dbg(&pdev->dev, "Firmware load failed\n");
-               goto error_power_off;
-       }
+       if (!isp->firmware)
+               return -ENOENT;
 
        err = sh_css_check_firmware_version(isp->dev, isp->firmware->data);
        if (err) {
@@ -1420,6 +1416,28 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
        /* save the iunit context only once after all the values are init'ed. */
        atomisp_save_iunit_reg(isp);
 
+       /* Init ISP memory management */
+       hmm_init();
+
+       err = devm_request_threaded_irq(&pdev->dev, pdev->irq,
+                                       atomisp_isr, atomisp_isr_thread,
+                                       IRQF_SHARED, "isp_irq", isp);
+       if (err) {
+               dev_err(&pdev->dev, "Failed to request irq (%d)\n", err);
+               goto error_unregister_entities;
+       }
+
+       /* Load firmware into ISP memory */
+       err = atomisp_css_load_firmware(isp);
+       if (err) {
+               dev_err(&pdev->dev, "Failed to init css.\n");
+               goto error_free_irq;
+       }
+       /* Clear FW image from memory */
+       release_firmware(isp->firmware);
+       isp->firmware = NULL;
+       isp->css_env.isp_css_fw.data = NULL;
+
        /*
         * The atomisp does not use standard PCI power-management through the
         * PCI config space. Instead this driver directly tells the P-Unit to
@@ -1441,30 +1459,8 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
        cpu_latency_qos_add_request(&isp->pm_qos, PM_QOS_DEFAULT_VALUE);
        dev_pm_domain_set(&pdev->dev, &isp->pm_domain);
 
-       pm_runtime_put_noidle(&pdev->dev);
        pm_runtime_allow(&pdev->dev);
-
-       /* Init ISP memory management */
-       hmm_init();
-
-       err = devm_request_threaded_irq(&pdev->dev, pdev->irq,
-                                       atomisp_isr, atomisp_isr_thread,
-                                       IRQF_SHARED, "isp_irq", isp);
-       if (err) {
-               dev_err(&pdev->dev, "Failed to request irq (%d)\n", err);
-               goto error_unregister_entities;
-       }
-
-       /* Load firmware into ISP memory */
-       err = atomisp_css_load_firmware(isp);
-       if (err) {
-               dev_err(&pdev->dev, "Failed to init css.\n");
-               goto error_free_irq;
-       }
-       /* Clear FW image from memory */
-       release_firmware(isp->firmware);
-       isp->firmware = NULL;
-       isp->css_env.isp_css_fw.data = NULL;
+       pm_runtime_put_sync_suspend(&pdev->dev);
 
        err = v4l2_async_nf_register(&isp->notifier);
        if (err) {
@@ -1477,15 +1473,15 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
        return 0;
 
 error_unload_firmware:
+       pm_runtime_get_sync(&pdev->dev);
+       pm_runtime_forbid(&pdev->dev);
+       dev_pm_domain_set(&pdev->dev, NULL);
+       cpu_latency_qos_remove_request(&isp->pm_qos);
        ia_css_unload_firmware();
 error_free_irq:
        devm_free_irq(&pdev->dev, pdev->irq, isp);
 error_unregister_entities:
        hmm_cleanup();
-       pm_runtime_forbid(&pdev->dev);
-       pm_runtime_get_noresume(&pdev->dev);
-       dev_pm_domain_set(&pdev->dev, NULL);
-       cpu_latency_qos_remove_request(&isp->pm_qos);
        atomisp_unregister_entities(isp);
 error_uninitialize_modules:
        atomisp_uninitialize_modules(isp);
@@ -1494,28 +1490,6 @@ error_irq_uninit:
        pci_free_irq_vectors(pdev);
 error_release_firmware:
        release_firmware(isp->firmware);
-error_power_off:
-       /*
-        * Switch off ISP, as keeping it powered on would prevent
-        * reaching S0ix states.
-        *
-        * The following lines have been copied from atomisp suspend path
-        */
-
-       pci_read_config_dword(pdev, PCI_INTERRUPT_CTRL, &irq);
-       irq &= BIT(INTR_IIR);
-       pci_write_config_dword(pdev, PCI_INTERRUPT_CTRL, irq);
-
-       pci_read_config_dword(pdev, PCI_INTERRUPT_CTRL, &irq);
-       irq &= ~BIT(INTR_IER);
-       pci_write_config_dword(pdev, PCI_INTERRUPT_CTRL, irq);
-
-       atomisp_msi_irq_uninit(isp);
-
-       /* Address later when we worry about the ...field chips */
-       if (IS_ENABLED(CONFIG_PM) && atomisp_mrfld_power(isp, false))
-               dev_err(&pdev->dev, "Failed to switch off ISP\n");
-
        return err;
 }
 
@@ -1525,15 +1499,17 @@ static void atomisp_pci_remove(struct pci_dev *pdev)
 
        atomisp_drvfs_exit();
 
-       ia_css_unload_firmware();
-       devm_free_irq(&pdev->dev, pdev->irq, isp);
-       hmm_cleanup();
-
+       pm_runtime_get_sync(&pdev->dev);
        pm_runtime_forbid(&pdev->dev);
-       pm_runtime_get_noresume(&pdev->dev);
        dev_pm_domain_set(&pdev->dev, NULL);
        cpu_latency_qos_remove_request(&isp->pm_qos);
 
+       /* Undo ia_css_init() from atomisp_power_on() */
+       atomisp_css_uninit(isp);
+       ia_css_unload_firmware();
+       devm_free_irq(&pdev->dev, pdev->irq, isp);
+       hmm_cleanup();
+
        atomisp_unregister_entities(isp);
        atomisp_uninitialize_modules(isp);
        atomisp_msi_irq_uninit(isp);