[PATCH] media: s5p-mfc: Always pass NULL to s5p_mfc_cmd_host2risc_v6()

[PATCH] media: s5p-mfc: Always pass NULL to s5p_mfc_cmd_host2risc_v6()

[Nathan Chancellor]

A new warning in clang [1] points out a few places in s5p_mfc_cmd_v6.c
where an uninitialized variable is passed as a const pointer:

  drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c:45:7: error: variable 'h2r_args' is uninitialized when passed as a const pointer argument here [-Werror,-Wuninitialized-const-pointer]
     45 |                                         &h2r_args);
        |                                          ^~~~~~~~
  drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c:133:7: error: variable 'h2r_args' is uninitialized when passed as a const pointer argument here [-Werror,-Wuninitialized-const-pointer]
    133 |                                         &h2r_args);
        |                                          ^~~~~~~~
  drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c:148:7: error: variable 'h2r_args' is uninitialized when passed as a const pointer argument here [-Werror,-Wuninitialized-const-pointer]
    148 |                                         &h2r_args);
        |                                          ^~~~~~~~

The args parameter in s5p_mfc_cmd_host2risc_v6() is never actually used,
so just pass NULL to it in the places where h2r_args is currently
passed, clearing up the warning and not changing the functionality of
the code.

Cc: stable@vger.kernel.org
Fixes: f96f3cfa0bb8 ("[media] s5p-mfc: Update MFC v4l2 driver to support MFC6.x")
Link: https://github.com/llvm/llvm-project/commit/00dacf8c22f065cb52efb14cd091d441f19b319e [1]
Closes: https://github.com/ClangBuiltLinux/linux/issues/2103
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
From what I can tell, it seems like ->cmd_host2risc() is only ever
called from v6 code, which always passes NULL? It seems like it should
be possible to just drop .cmd_host2risc on the v5 side, then update
.cmd_host2risc to only take two parameters? If so, I can send a follow
up as a clean up, so that this can go back relatively conflict free.
---
 .../platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c      | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c
index 47bc3014b5d8..735471c50dbb 100644
--- a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c
+++ b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c
@@ -31,7 +31,6 @@ static int s5p_mfc_cmd_host2risc_v6(struct s5p_mfc_dev *dev, int cmd,
 
 static int s5p_mfc_sys_init_cmd_v6(struct s5p_mfc_dev *dev)
 {
-	struct s5p_mfc_cmd_args h2r_args;
 	const struct s5p_mfc_buf_size_v6 *buf_size = dev->variant->buf_size->priv;
 	int ret;
 
@@ -41,33 +40,23 @@ static int s5p_mfc_sys_init_cmd_v6(struct s5p_mfc_dev *dev)
 
 	mfc_write(dev, dev->ctx_buf.dma, S5P_FIMV_CONTEXT_MEM_ADDR_V6);
 	mfc_write(dev, buf_size->dev_ctx, S5P_FIMV_CONTEXT_MEM_SIZE_V6);
-	return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_SYS_INIT_V6,
-					&h2r_args);
+	return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_SYS_INIT_V6, NULL);
 }
 
 static int s5p_mfc_sleep_cmd_v6(struct s5p_mfc_dev *dev)
 {
-	struct s5p_mfc_cmd_args h2r_args;
-
-	memset(&h2r_args, 0, sizeof(struct s5p_mfc_cmd_args));
-	return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_SLEEP_V6,
-			&h2r_args);
+	return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_SLEEP_V6, NULL);
 }
 
 static int s5p_mfc_wakeup_cmd_v6(struct s5p_mfc_dev *dev)
 {
-	struct s5p_mfc_cmd_args h2r_args;
-
-	memset(&h2r_args, 0, sizeof(struct s5p_mfc_cmd_args));
-	return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_WAKEUP_V6,
-					&h2r_args);
+	return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_WAKEUP_V6, NULL);
 }
 
 /* Open a new instance and get its number */
 static int s5p_mfc_open_inst_cmd_v6(struct s5p_mfc_ctx *ctx)
 {
 	struct s5p_mfc_dev *dev = ctx->dev;
-	struct s5p_mfc_cmd_args h2r_args;
 	int codec_type;
 
 	mfc_debug(2, "Requested codec mode: %d\n", ctx->codec_mode);
@@ -130,14 +119,13 @@ static int s5p_mfc_open_inst_cmd_v6(struct s5p_mfc_ctx *ctx)
 	mfc_write(dev, 0, S5P_FIMV_D_CRC_CTRL_V6); /* no crc */
 
 	return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_OPEN_INSTANCE_V6,
-					&h2r_args);
+					NULL);
 }
 
 /* Close instance */
 static int s5p_mfc_close_inst_cmd_v6(struct s5p_mfc_ctx *ctx)
 {
 	struct s5p_mfc_dev *dev = ctx->dev;
-	struct s5p_mfc_cmd_args h2r_args;
 	int ret = 0;
 
 	dev->curr_ctx = ctx->num;
@@ -145,7 +133,7 @@ static int s5p_mfc_close_inst_cmd_v6(struct s5p_mfc_ctx *ctx)
 		mfc_write(dev, ctx->inst_no, S5P_FIMV_INSTANCE_ID_V6);
 		ret = s5p_mfc_cmd_host2risc_v6(dev,
 					S5P_FIMV_H2R_CMD_CLOSE_INSTANCE_V6,
-					&h2r_args);
+					NULL);
 	} else {
 		ret = -EINVAL;
 	}

---
base-commit: 347e9f5043c89695b01e66b3ed111755afcf1911
change-id: 20250715-media-s5p-mfc-fix-uninit-const-pointer-cbf944ae4b4b

Best regards,
--  
Nathan Chancellor <nathan@kernel.org>

Re: [PATCH] media: s5p-mfc: Always pass NULL to s5p_mfc_cmd_host2risc_v6()

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 5903 bytes --]

Hi Nathan,

Le mardi 15 juillet 2025 à 15:13 -0700, Nathan Chancellor a écrit :
> A new warning in clang [1] points out a few places in s5p_mfc_cmd_v6.c
> where an uninitialized variable is passed as a const pointer:
> 
>   drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c:45:7: error: variable 'h2r_args' is uninitialized when passed as a const pointer argument here [-Werror,-Wuninitialized-const-pointer]
>      45 |                                         &h2r_args);
>         |                                          ^~~~~~~~
>   drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c:133:7: error: variable 'h2r_args' is uninitialized when passed as a const pointer argument here [-Werror,-Wuninitialized-const-pointer]
>     133 |                                         &h2r_args);
>         |                                          ^~~~~~~~
>   drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c:148:7: error: variable 'h2r_args' is uninitialized when passed as a const pointer argument here [-Werror,-Wuninitialized-const-pointer]
>     148 |                                         &h2r_args);
>         |                                          ^~~~~~~~
> 
> The args parameter in s5p_mfc_cmd_host2risc_v6() is never actually used,
> so just pass NULL to it in the places where h2r_args is currently
> passed, clearing up the warning and not changing the functionality of
> the code.
> 
> Cc: stable@vger.kernel.org
> Fixes: f96f3cfa0bb8 ("[media] s5p-mfc: Update MFC v4l2 driver to support MFC6.x")
> Link: https://github.com/llvm/llvm-project/commit/00dacf8c22f065cb52efb14cd091d441f19b319e [1]
> Closes: https://github.com/ClangBuiltLinux/linux/issues/2103
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
> From what I can tell, it seems like ->cmd_host2risc() is only ever
> called from v6 code, which always passes NULL? It seems like it should
> be possible to just drop .cmd_host2risc on the v5 side, then update
> .cmd_host2risc to only take two parameters? If so, I can send a follow
> up as a clean up, so that this can go back relatively conflict free.

It seems so yes. For this specific patch, I would probably rename "args" to
"__unused" to make the reading faster. But does not matter so much if you later
remove it.

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  .../platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c      | 22 +++++-----------------
>  1 file changed, 5 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c
> index 47bc3014b5d8..735471c50dbb 100644
> --- a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c
> +++ b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c
> @@ -31,7 +31,6 @@ static int s5p_mfc_cmd_host2risc_v6(struct s5p_mfc_dev *dev, int cmd,
>  
>  static int s5p_mfc_sys_init_cmd_v6(struct s5p_mfc_dev *dev)
>  {
> -	struct s5p_mfc_cmd_args h2r_args;
>  	const struct s5p_mfc_buf_size_v6 *buf_size = dev->variant->buf_size->priv;
>  	int ret;
>  
> @@ -41,33 +40,23 @@ static int s5p_mfc_sys_init_cmd_v6(struct s5p_mfc_dev *dev)
>  
>  	mfc_write(dev, dev->ctx_buf.dma, S5P_FIMV_CONTEXT_MEM_ADDR_V6);
>  	mfc_write(dev, buf_size->dev_ctx, S5P_FIMV_CONTEXT_MEM_SIZE_V6);
> -	return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_SYS_INIT_V6,
> -					&h2r_args);
> +	return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_SYS_INIT_V6, NULL);
>  }
>  
>  static int s5p_mfc_sleep_cmd_v6(struct s5p_mfc_dev *dev)
>  {
> -	struct s5p_mfc_cmd_args h2r_args;
> -
> -	memset(&h2r_args, 0, sizeof(struct s5p_mfc_cmd_args));
> -	return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_SLEEP_V6,
> -			&h2r_args);
> +	return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_SLEEP_V6, NULL);
>  }
>  
>  static int s5p_mfc_wakeup_cmd_v6(struct s5p_mfc_dev *dev)
>  {
> -	struct s5p_mfc_cmd_args h2r_args;
> -
> -	memset(&h2r_args, 0, sizeof(struct s5p_mfc_cmd_args));
> -	return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_WAKEUP_V6,
> -					&h2r_args);
> +	return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_WAKEUP_V6, NULL);
>  }
>  
>  /* Open a new instance and get its number */
>  static int s5p_mfc_open_inst_cmd_v6(struct s5p_mfc_ctx *ctx)
>  {
>  	struct s5p_mfc_dev *dev = ctx->dev;
> -	struct s5p_mfc_cmd_args h2r_args;
>  	int codec_type;
>  
>  	mfc_debug(2, "Requested codec mode: %d\n", ctx->codec_mode);
> @@ -130,14 +119,13 @@ static int s5p_mfc_open_inst_cmd_v6(struct s5p_mfc_ctx *ctx)
>  	mfc_write(dev, 0, S5P_FIMV_D_CRC_CTRL_V6); /* no crc */
>  
>  	return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_OPEN_INSTANCE_V6,
> -					&h2r_args);
> +					NULL);
>  }
>  
>  /* Close instance */
>  static int s5p_mfc_close_inst_cmd_v6(struct s5p_mfc_ctx *ctx)
>  {
>  	struct s5p_mfc_dev *dev = ctx->dev;
> -	struct s5p_mfc_cmd_args h2r_args;
>  	int ret = 0;
>  
>  	dev->curr_ctx = ctx->num;
> @@ -145,7 +133,7 @@ static int s5p_mfc_close_inst_cmd_v6(struct s5p_mfc_ctx *ctx)
>  		mfc_write(dev, ctx->inst_no, S5P_FIMV_INSTANCE_ID_V6);
>  		ret = s5p_mfc_cmd_host2risc_v6(dev,
>  					S5P_FIMV_H2R_CMD_CLOSE_INSTANCE_V6,
> -					&h2r_args);
> +					NULL);
>  	} else {
>  		ret = -EINVAL;
>  	}
> 
> ---
> base-commit: 347e9f5043c89695b01e66b3ed111755afcf1911
> change-id: 20250715-media-s5p-mfc-fix-uninit-const-pointer-cbf944ae4b4b
> 
> Best regards,
> --  
> Nathan Chancellor <nathan@kernel.org>
> 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] media: s5p-mfc: Always pass NULL to s5p_mfc_cmd_host2risc_v6()

[Nathan Chancellor]

Hi Nicolas,

On Tue, Jul 29, 2025 at 10:24:22AM -0400, Nicolas Dufresne wrote:
> Le mardi 15 juillet 2025 à 15:13 -0700, Nathan Chancellor a écrit :
> > From what I can tell, it seems like ->cmd_host2risc() is only ever
> > called from v6 code, which always passes NULL? It seems like it should
> > be possible to just drop .cmd_host2risc on the v5 side, then update
> > .cmd_host2risc to only take two parameters? If so, I can send a follow
> > up as a clean up, so that this can go back relatively conflict free.
> 
> It seems so yes. For this specific patch, I would probably rename "args" to
> "__unused" to make the reading faster. But does not matter so much if you later
> remove it.

Yes, after this change is picked up in a maintainer's tree, I do plan to
send a patch to remove the "args" parameter altogether. If you really
care, I can certainly rename the parameter in this change to "__unused"
as suggested but if you don't, I will just leave it as is to make
backporting this a little easier.

> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

Thanks a lot for taking a look!

Cheers,
Nathan

Re: [PATCH] media: s5p-mfc: Always pass NULL to s5p_mfc_cmd_host2risc_v6()

[Marek Szyprowski]

On 16.07.2025 00:13, Nathan Chancellor wrote:
> A new warning in clang [1] points out a few places in s5p_mfc_cmd_v6.c
> where an uninitialized variable is passed as a const pointer:
>
>    drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c:45:7: error: variable 'h2r_args' is uninitialized when passed as a const pointer argument here [-Werror,-Wuninitialized-const-pointer]
>       45 |                                         &h2r_args);
>          |                                          ^~~~~~~~
>    drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c:133:7: error: variable 'h2r_args' is uninitialized when passed as a const pointer argument here [-Werror,-Wuninitialized-const-pointer]
>      133 |                                         &h2r_args);
>          |                                          ^~~~~~~~
>    drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c:148:7: error: variable 'h2r_args' is uninitialized when passed as a const pointer argument here [-Werror,-Wuninitialized-const-pointer]
>      148 |                                         &h2r_args);
>          |                                          ^~~~~~~~
>
> The args parameter in s5p_mfc_cmd_host2risc_v6() is never actually used,
> so just pass NULL to it in the places where h2r_args is currently
> passed, clearing up the warning and not changing the functionality of
> the code.
>
> Cc: stable@vger.kernel.org
> Fixes: f96f3cfa0bb8 ("[media] s5p-mfc: Update MFC v4l2 driver to support MFC6.x")
> Link: https://github.com/llvm/llvm-project/commit/00dacf8c22f065cb52efb14cd091d441f19b319e [1]
> Closes: https://github.com/ClangBuiltLinux/linux/issues/2103
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  From what I can tell, it seems like ->cmd_host2risc() is only ever
> called from v6 code, which always passes NULL? It seems like it should
> be possible to just drop .cmd_host2risc on the v5 side, then update
> .cmd_host2risc to only take two parameters? If so, I can send a follow
> up as a clean up, so that this can go back relatively conflict free.
> ---
>   .../platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c      | 22 +++++-----------------
>   1 file changed, 5 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c
> index 47bc3014b5d8..735471c50dbb 100644
> --- a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c
> +++ b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c
> @@ -31,7 +31,6 @@ static int s5p_mfc_cmd_host2risc_v6(struct s5p_mfc_dev *dev, int cmd,
>   
>   static int s5p_mfc_sys_init_cmd_v6(struct s5p_mfc_dev *dev)
>   {
> -	struct s5p_mfc_cmd_args h2r_args;
>   	const struct s5p_mfc_buf_size_v6 *buf_size = dev->variant->buf_size->priv;
>   	int ret;
>   
> @@ -41,33 +40,23 @@ static int s5p_mfc_sys_init_cmd_v6(struct s5p_mfc_dev *dev)
>   
>   	mfc_write(dev, dev->ctx_buf.dma, S5P_FIMV_CONTEXT_MEM_ADDR_V6);
>   	mfc_write(dev, buf_size->dev_ctx, S5P_FIMV_CONTEXT_MEM_SIZE_V6);
> -	return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_SYS_INIT_V6,
> -					&h2r_args);
> +	return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_SYS_INIT_V6, NULL);
>   }
>   
>   static int s5p_mfc_sleep_cmd_v6(struct s5p_mfc_dev *dev)
>   {
> -	struct s5p_mfc_cmd_args h2r_args;
> -
> -	memset(&h2r_args, 0, sizeof(struct s5p_mfc_cmd_args));
> -	return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_SLEEP_V6,
> -			&h2r_args);
> +	return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_SLEEP_V6, NULL);
>   }
>   
>   static int s5p_mfc_wakeup_cmd_v6(struct s5p_mfc_dev *dev)
>   {
> -	struct s5p_mfc_cmd_args h2r_args;
> -
> -	memset(&h2r_args, 0, sizeof(struct s5p_mfc_cmd_args));
> -	return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_WAKEUP_V6,
> -					&h2r_args);
> +	return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_WAKEUP_V6, NULL);
>   }
>   
>   /* Open a new instance and get its number */
>   static int s5p_mfc_open_inst_cmd_v6(struct s5p_mfc_ctx *ctx)
>   {
>   	struct s5p_mfc_dev *dev = ctx->dev;
> -	struct s5p_mfc_cmd_args h2r_args;
>   	int codec_type;
>   
>   	mfc_debug(2, "Requested codec mode: %d\n", ctx->codec_mode);
> @@ -130,14 +119,13 @@ static int s5p_mfc_open_inst_cmd_v6(struct s5p_mfc_ctx *ctx)
>   	mfc_write(dev, 0, S5P_FIMV_D_CRC_CTRL_V6); /* no crc */
>   
>   	return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_OPEN_INSTANCE_V6,
> -					&h2r_args);
> +					NULL);
>   }
>   
>   /* Close instance */
>   static int s5p_mfc_close_inst_cmd_v6(struct s5p_mfc_ctx *ctx)
>   {
>   	struct s5p_mfc_dev *dev = ctx->dev;
> -	struct s5p_mfc_cmd_args h2r_args;
>   	int ret = 0;
>   
>   	dev->curr_ctx = ctx->num;
> @@ -145,7 +133,7 @@ static int s5p_mfc_close_inst_cmd_v6(struct s5p_mfc_ctx *ctx)
>   		mfc_write(dev, ctx->inst_no, S5P_FIMV_INSTANCE_ID_V6);
>   		ret = s5p_mfc_cmd_host2risc_v6(dev,
>   					S5P_FIMV_H2R_CMD_CLOSE_INSTANCE_V6,
> -					&h2r_args);
> +					NULL);
>   	} else {
>   		ret = -EINVAL;
>   	}
>
> ---
> base-commit: 347e9f5043c89695b01e66b3ed111755afcf1911
> change-id: 20250715-media-s5p-mfc-fix-uninit-const-pointer-cbf944ae4b4b
>
> Best regards,
> --
> Nathan Chancellor <nathan@kernel.org>
>
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH] media: s5p-mfc: Always pass NULL to s5p_mfc_cmd_host2risc_v6()

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 1604 bytes --]

Le mardi 29 juillet 2025 à 17:52 -0700, Nathan Chancellor a écrit :
> Hi Nicolas,
> 
> On Tue, Jul 29, 2025 at 10:24:22AM -0400, Nicolas Dufresne wrote:
> > Le mardi 15 juillet 2025 à 15:13 -0700, Nathan Chancellor a écrit :
> > > From what I can tell, it seems like ->cmd_host2risc() is only ever
> > > called from v6 code, which always passes NULL? It seems like it should
> > > be possible to just drop .cmd_host2risc on the v5 side, then update
> > > .cmd_host2risc to only take two parameters? If so, I can send a follow
> > > up as a clean up, so that this can go back relatively conflict free.
> > 
> > It seems so yes. For this specific patch, I would probably rename "args" to
> > "__unused" to make the reading faster. But does not matter so much if you
> > later
> > remove it.
> 
> Yes, after this change is picked up in a maintainer's tree, I do plan to
> send a patch to remove the "args" parameter altogether. If you really
> care, I can certainly rename the parameter in this change to "__unused"
> as suggested but if you don't, I will just leave it as is to make
> backporting this a little easier.
> 
> > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> 
> Thanks a lot for taking a look!

While applying your patch, I realized the Hans merged another version of this
fix, but made by Arnd. It covers the remaining too, so I will mark yours as
superseded now.

https://gitlab.freedesktop.org/linux-media/media-committers/-/commit/7fa37ba25a1dfc084e24ea9acc14bf1fad8af14c

thanks for your work,
Nicolas

> 
> Cheers,
> Nathan

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] media: s5p-mfc: Always pass NULL to s5p_mfc_cmd_host2risc_v6()

[Nathan Chancellor]

Hi Nicolas,

On Wed, Sep 03, 2025 at 01:13:00PM -0400, Nicolas Dufresne wrote:
> Le mardi 29 juillet 2025 à 17:52 -0700, Nathan Chancellor a écrit :
> > Hi Nicolas,
> > 
> > On Tue, Jul 29, 2025 at 10:24:22AM -0400, Nicolas Dufresne wrote:
> > > Le mardi 15 juillet 2025 à 15:13 -0700, Nathan Chancellor a écrit :
> > > > From what I can tell, it seems like ->cmd_host2risc() is only ever
> > > > called from v6 code, which always passes NULL? It seems like it should
> > > > be possible to just drop .cmd_host2risc on the v5 side, then update
> > > > .cmd_host2risc to only take two parameters? If so, I can send a follow
> > > > up as a clean up, so that this can go back relatively conflict free.
> > > 
> > > It seems so yes. For this specific patch, I would probably rename "args" to
> > > "__unused" to make the reading faster. But does not matter so much if you
> > > later
> > > remove it.
> > 
> > Yes, after this change is picked up in a maintainer's tree, I do plan to
> > send a patch to remove the "args" parameter altogether. If you really
> > care, I can certainly rename the parameter in this change to "__unused"
> > as suggested but if you don't, I will just leave it as is to make
> > backporting this a little easier.
> > 
> > > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > 
> > Thanks a lot for taking a look!
> 
> While applying your patch, I realized the Hans merged another version of this
> fix, but made by Arnd. It covers the remaining too, so I will mark yours as
> superseded now.
> 
> https://gitlab.freedesktop.org/linux-media/media-committers/-/commit/7fa37ba25a1dfc084e24ea9acc14bf1fad8af14c

Thanks, Arnd's patch is obviously equally correct and the stable tags
remained so everything looks good from my end. Appreciate you doubling
back to this.

Cheers,
Nathan