[13/57] media: atomisp: Remove deferred firmware loading support

Message ID 20230123125205.622152-14-hdegoede@redhat.com (mailing list archive)
State Accepted
Headers
Series media: atomisp: Big power-management changes + lots of fixes |

Commit Message

Hans de Goede Jan. 23, 2023, 12:51 p.m. UTC
  Make atomisp behave like any othet drivers and have it load the firmware
at probe time (as it was already doing by default).

The deferred firmware loading support needlessly complicates the
v4l2_file_operations.open callback (atomisp_open()), getting in
the way of allowing multiple opens like a normal v4l2 device would.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../staging/media/atomisp/pci/atomisp_fops.c  | 25 -----------
 .../staging/media/atomisp/pci/atomisp_fops.h  |  2 -
 .../staging/media/atomisp/pci/atomisp_v4l2.c  | 42 +++++++------------
 3 files changed, 14 insertions(+), 55 deletions(-)
  

Comments

Andy Shevchenko Jan. 23, 2023, 2:37 p.m. UTC | #1
On Mon, Jan 23, 2023 at 01:51:21PM +0100, Hans de Goede wrote:
> Make atomisp behave like any othet drivers and have it load the firmware

other

> at probe time (as it was already doing by default).
> 
> The deferred firmware loading support needlessly complicates the
> v4l2_file_operations.open callback (atomisp_open()), getting in
> the way of allowing multiple opens like a normal v4l2 device would.

So it actually removes that variable and accompanying code w.o.
changing behaviour (as default).

Reviewed-by: Andy Shevchenko <andy@kernel.org>

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  .../staging/media/atomisp/pci/atomisp_fops.c  | 25 -----------
>  .../staging/media/atomisp/pci/atomisp_fops.h  |  2 -
>  .../staging/media/atomisp/pci/atomisp_v4l2.c  | 42 +++++++------------
>  3 files changed, 14 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
> index 682239ea042f..036ad339b344 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
> @@ -757,25 +757,6 @@ static int atomisp_open(struct file *file)
>  	mutex_lock(&isp->mutex);
>  
>  	asd->subdev.devnode = vdev;
> -	/* Deferred firmware loading case. */
> -	if (isp->css_env.isp_css_fw.bytes == 0) {
> -		dev_err(isp->dev, "Deferred firmware load.\n");
> -		isp->firmware = atomisp_load_firmware(isp);
> -		if (!isp->firmware) {
> -			dev_err(isp->dev, "Failed to load ISP firmware.\n");
> -			ret = -ENOENT;
> -			goto error;
> -		}
> -		ret = atomisp_css_load_firmware(isp);
> -		if (ret) {
> -			dev_err(isp->dev, "Failed to init css.\n");
> -			goto error;
> -		}
> -		/* No need to keep FW in memory anymore. */
> -		release_firmware(isp->firmware);
> -		isp->firmware = NULL;
> -		isp->css_env.isp_css_fw.data = NULL;
> -	}
>  
>  	if (!isp->input_cnt) {
>  		dev_err(isp->dev, "no camera attached\n");
> @@ -901,12 +882,6 @@ static int atomisp_release(struct file *file)
>  
>  	atomisp_destroy_pipes_stream_force(asd);
>  
> -	if (defer_fw_load) {
> -		ia_css_unload_firmware();
> -		isp->css_env.isp_css_fw.data = NULL;
> -		isp->css_env.isp_css_fw.bytes = 0;
> -	}
> -
>  	ret = v4l2_subdev_call(isp->flash, core, s_power, 0);
>  	if (ret < 0 && ret != -ENODEV && ret != -ENOIOCTLCMD)
>  		dev_warn(isp->dev, "Failed to power-off flash\n");
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.h b/drivers/staging/media/atomisp/pci/atomisp_fops.h
> index 10e43126b693..2efc5245e571 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_fops.h
> +++ b/drivers/staging/media/atomisp/pci/atomisp_fops.h
> @@ -33,6 +33,4 @@ int atomisp_qbuffers_to_css(struct atomisp_sub_device *asd);
>  
>  extern const struct v4l2_file_operations atomisp_fops;
>  
> -extern bool defer_fw_load;
> -
>  #endif /* __ATOMISP_FOPS_H__ */
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> index aa05c69a5c6b..2a949d3dc5d1 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> @@ -58,12 +58,6 @@ static uint skip_fwload;
>  module_param(skip_fwload, uint, 0644);
>  MODULE_PARM_DESC(skip_fwload, "Skip atomisp firmware load");
>  
> -/* memory optimization: deferred firmware loading */
> -bool defer_fw_load;
> -module_param(defer_fw_load, bool, 0644);
> -MODULE_PARM_DESC(defer_fw_load,
> -		 "Defer FW loading until device is opened (default:disable)");
> -
>  /* cross componnet debug message flag */
>  int dbg_level;
>  module_param(dbg_level, int, 0644);
> @@ -1514,21 +1508,17 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
>  	isp->max_isr_latency = ATOMISP_MAX_ISR_LATENCY;
>  
>  	/* Load isp firmware from user space */
> -	if (!defer_fw_load) {
> -		isp->firmware = atomisp_load_firmware(isp);
> -		if (!isp->firmware) {
> -			err = -ENOENT;
> -			dev_dbg(&pdev->dev, "Firmware load failed\n");
> -			goto load_fw_fail;
> -		}
> +	isp->firmware = atomisp_load_firmware(isp);
> +	if (!isp->firmware) {
> +		err = -ENOENT;
> +		dev_dbg(&pdev->dev, "Firmware load failed\n");
> +		goto load_fw_fail;
> +	}
>  
> -		err = sh_css_check_firmware_version(isp->dev, isp->firmware->data);
> -		if (err) {
> -			dev_dbg(&pdev->dev, "Firmware version check failed\n");
> -			goto fw_validation_fail;
> -		}
> -	} else {
> -		dev_info(&pdev->dev, "Firmware load will be deferred\n");
> +	err = sh_css_check_firmware_version(isp->dev, isp->firmware->data);
> +	if (err) {
> +		dev_dbg(&pdev->dev, "Firmware version check failed\n");
> +		goto fw_validation_fail;
>  	}
>  
>  	pci_set_master(pdev);
> @@ -1628,14 +1618,10 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
>  	}
>  
>  	/* Load firmware into ISP memory */
> -	if (!defer_fw_load) {
> -		err = atomisp_css_load_firmware(isp);
> -		if (err) {
> -			dev_err(&pdev->dev, "Failed to init css.\n");
> -			goto css_init_fail;
> -		}
> -	} else {
> -		dev_dbg(&pdev->dev, "Skip css init.\n");
> +	err = atomisp_css_load_firmware(isp);
> +	if (err) {
> +		dev_err(&pdev->dev, "Failed to init css.\n");
> +		goto css_init_fail;
>  	}
>  	/* Clear FW image from memory */
>  	release_firmware(isp->firmware);
> -- 
> 2.39.0
>
  

Patch

diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
index 682239ea042f..036ad339b344 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
@@ -757,25 +757,6 @@  static int atomisp_open(struct file *file)
 	mutex_lock(&isp->mutex);
 
 	asd->subdev.devnode = vdev;
-	/* Deferred firmware loading case. */
-	if (isp->css_env.isp_css_fw.bytes == 0) {
-		dev_err(isp->dev, "Deferred firmware load.\n");
-		isp->firmware = atomisp_load_firmware(isp);
-		if (!isp->firmware) {
-			dev_err(isp->dev, "Failed to load ISP firmware.\n");
-			ret = -ENOENT;
-			goto error;
-		}
-		ret = atomisp_css_load_firmware(isp);
-		if (ret) {
-			dev_err(isp->dev, "Failed to init css.\n");
-			goto error;
-		}
-		/* No need to keep FW in memory anymore. */
-		release_firmware(isp->firmware);
-		isp->firmware = NULL;
-		isp->css_env.isp_css_fw.data = NULL;
-	}
 
 	if (!isp->input_cnt) {
 		dev_err(isp->dev, "no camera attached\n");
@@ -901,12 +882,6 @@  static int atomisp_release(struct file *file)
 
 	atomisp_destroy_pipes_stream_force(asd);
 
-	if (defer_fw_load) {
-		ia_css_unload_firmware();
-		isp->css_env.isp_css_fw.data = NULL;
-		isp->css_env.isp_css_fw.bytes = 0;
-	}
-
 	ret = v4l2_subdev_call(isp->flash, core, s_power, 0);
 	if (ret < 0 && ret != -ENODEV && ret != -ENOIOCTLCMD)
 		dev_warn(isp->dev, "Failed to power-off flash\n");
diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.h b/drivers/staging/media/atomisp/pci/atomisp_fops.h
index 10e43126b693..2efc5245e571 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_fops.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_fops.h
@@ -33,6 +33,4 @@  int atomisp_qbuffers_to_css(struct atomisp_sub_device *asd);
 
 extern const struct v4l2_file_operations atomisp_fops;
 
-extern bool defer_fw_load;
-
 #endif /* __ATOMISP_FOPS_H__ */
diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index aa05c69a5c6b..2a949d3dc5d1 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -58,12 +58,6 @@  static uint skip_fwload;
 module_param(skip_fwload, uint, 0644);
 MODULE_PARM_DESC(skip_fwload, "Skip atomisp firmware load");
 
-/* memory optimization: deferred firmware loading */
-bool defer_fw_load;
-module_param(defer_fw_load, bool, 0644);
-MODULE_PARM_DESC(defer_fw_load,
-		 "Defer FW loading until device is opened (default:disable)");
-
 /* cross componnet debug message flag */
 int dbg_level;
 module_param(dbg_level, int, 0644);
@@ -1514,21 +1508,17 @@  static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
 	isp->max_isr_latency = ATOMISP_MAX_ISR_LATENCY;
 
 	/* Load isp firmware from user space */
-	if (!defer_fw_load) {
-		isp->firmware = atomisp_load_firmware(isp);
-		if (!isp->firmware) {
-			err = -ENOENT;
-			dev_dbg(&pdev->dev, "Firmware load failed\n");
-			goto load_fw_fail;
-		}
+	isp->firmware = atomisp_load_firmware(isp);
+	if (!isp->firmware) {
+		err = -ENOENT;
+		dev_dbg(&pdev->dev, "Firmware load failed\n");
+		goto load_fw_fail;
+	}
 
-		err = sh_css_check_firmware_version(isp->dev, isp->firmware->data);
-		if (err) {
-			dev_dbg(&pdev->dev, "Firmware version check failed\n");
-			goto fw_validation_fail;
-		}
-	} else {
-		dev_info(&pdev->dev, "Firmware load will be deferred\n");
+	err = sh_css_check_firmware_version(isp->dev, isp->firmware->data);
+	if (err) {
+		dev_dbg(&pdev->dev, "Firmware version check failed\n");
+		goto fw_validation_fail;
 	}
 
 	pci_set_master(pdev);
@@ -1628,14 +1618,10 @@  static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
 	}
 
 	/* Load firmware into ISP memory */
-	if (!defer_fw_load) {
-		err = atomisp_css_load_firmware(isp);
-		if (err) {
-			dev_err(&pdev->dev, "Failed to init css.\n");
-			goto css_init_fail;
-		}
-	} else {
-		dev_dbg(&pdev->dev, "Skip css init.\n");
+	err = atomisp_css_load_firmware(isp);
+	if (err) {
+		dev_err(&pdev->dev, "Failed to init css.\n");
+		goto css_init_fail;
 	}
 	/* Clear FW image from memory */
 	release_firmware(isp->firmware);