[PATCH 0/2] media: atomisp: two sparse/smatch fixes

[PATCH 0/2] media: atomisp: two sparse/smatch fixes

[Hans Verkuil]

Two small fixes to shut up sparse/smatch.

	Hans

Hans Verkuil (2):
  media: atomisp: make dbgopt static
  media: atomisp: don't use sizeof(NULL)

 drivers/staging/media/atomisp/pci/atomisp_drvfs.c | 2 +-
 drivers/staging/media/atomisp/pci/sh_css.c        | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.43.0

[PATCH 1/2] media: atomisp: make dbgopt static

[Hans Verkuil]

This fixes a sparse warning:

drivers/staging/media/atomisp/pci/atomisp_drvfs.c:40:14: warning: symbol 'dbgopt' was not declared. Should it be static?

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/staging/media/atomisp/pci/atomisp_drvfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_drvfs.c b/drivers/staging/media/atomisp/pci/atomisp_drvfs.c
index 293171da1266..ba7dd569a55a 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_drvfs.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_drvfs.c
@@ -37,7 +37,7 @@
  *        bit 1: running binary
  *        bit 2: memory statistic
  */
-unsigned int dbgopt = OPTION_BIN_LIST;
+static unsigned int dbgopt = OPTION_BIN_LIST;
 
 static inline int iunit_dump_dbgopt(struct atomisp_device *isp,
 				    unsigned int opt)
-- 
2.43.0

[PATCH 2/2] media: atomisp: don't use sizeof(NULL)

[Hans Verkuil]

Check the size of another pointer instead of NULL. This fixes this
smatch warning:

drivers/staging/media/atomisp/pci/sh_css.c:3609 ia_css_pipe_enqueue_buffer() warn: sizeof(NUMBER)?

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/staging/media/atomisp/pci/sh_css.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index 1d1fbda75da1..0d2ef96fabce 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -3606,7 +3606,7 @@ ia_css_pipe_enqueue_buffer(struct ia_css_pipe *pipe,
 
 	assert(pipeline || pipe_id == IA_CSS_PIPE_ID_COPY);
 
-	assert(sizeof(NULL) <= sizeof(ddr_buffer.kernel_ptr));
+	assert(sizeof(pipe) <= sizeof(ddr_buffer.kernel_ptr));
 	ddr_buffer.kernel_ptr = HOST_ADDRESS(NULL);
 	ddr_buffer.cookie_ptr = buffer->driver_cookie;
 	ddr_buffer.timing_data = buffer->timing_data;
-- 
2.43.0

Re: [PATCH 1/2] media: atomisp: make dbgopt static

[Hans Verkuil]

On 02/02/2024 10:21, Hans Verkuil wrote:
> This fixes a sparse warning:
> 
> drivers/staging/media/atomisp/pci/atomisp_drvfs.c:40:14: warning: symbol 'dbgopt' was not declared. Should it be static?
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Fixes: b03301870d9b ("media: atomisp: Replace atomisp_drvfs attr with using driver.dev_groups attr")

> ---
>  drivers/staging/media/atomisp/pci/atomisp_drvfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_drvfs.c b/drivers/staging/media/atomisp/pci/atomisp_drvfs.c
> index 293171da1266..ba7dd569a55a 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_drvfs.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_drvfs.c
> @@ -37,7 +37,7 @@
>   *        bit 1: running binary
>   *        bit 2: memory statistic
>   */
> -unsigned int dbgopt = OPTION_BIN_LIST;
> +static unsigned int dbgopt = OPTION_BIN_LIST;
>  
>  static inline int iunit_dump_dbgopt(struct atomisp_device *isp,
>  				    unsigned int opt)

Re: [PATCH 1/2] media: atomisp: make dbgopt static

[Nicolas Dufresne]

Le vendredi 02 février 2024 à 11:04 +0100, Hans Verkuil a écrit :
> On 02/02/2024 10:21, Hans Verkuil wrote:
> > This fixes a sparse warning:
> > 
> > drivers/staging/media/atomisp/pci/atomisp_drvfs.c:40:14: warning: symbol 'dbgopt' was not declared. Should it be static?
> > 
> > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> 
> Fixes: b03301870d9b ("media: atomisp: Replace atomisp_drvfs attr with using driver.dev_groups attr")

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

> 
> > ---
> >  drivers/staging/media/atomisp/pci/atomisp_drvfs.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/media/atomisp/pci/atomisp_drvfs.c b/drivers/staging/media/atomisp/pci/atomisp_drvfs.c
> > index 293171da1266..ba7dd569a55a 100644
> > --- a/drivers/staging/media/atomisp/pci/atomisp_drvfs.c
> > +++ b/drivers/staging/media/atomisp/pci/atomisp_drvfs.c
> > @@ -37,7 +37,7 @@
> >   *        bit 1: running binary
> >   *        bit 2: memory statistic
> >   */
> > -unsigned int dbgopt = OPTION_BIN_LIST;
> > +static unsigned int dbgopt = OPTION_BIN_LIST;
> >  
> >  static inline int iunit_dump_dbgopt(struct atomisp_device *isp,
> >  				    unsigned int opt)
> 
> 

Re: [PATCH 2/2] media: atomisp: don't use sizeof(NULL)

[Nicolas Dufresne]

Hi,

Le vendredi 02 février 2024 à 10:21 +0100, Hans Verkuil a écrit :
> Check the size of another pointer instead of NULL. This fixes this
> smatch warning:
> 
> drivers/staging/media/atomisp/pci/sh_css.c:3609 ia_css_pipe_enqueue_buffer() warn: sizeof(NUMBER)?
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  drivers/staging/media/atomisp/pci/sh_css.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
> index 1d1fbda75da1..0d2ef96fabce 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css.c
> @@ -3606,7 +3606,7 @@ ia_css_pipe_enqueue_buffer(struct ia_css_pipe *pipe,
>  
>  	assert(pipeline || pipe_id == IA_CSS_PIPE_ID_COPY);
>  
> -	assert(sizeof(NULL) <= sizeof(ddr_buffer.kernel_ptr));
> +	assert(sizeof(pipe) <= sizeof(ddr_buffer.kernel_ptr));

have you considered using a static_assert() instead ? There is no run-time value
involved. Also, I personally think this case would be more readable if you used
"sizeof(void*)", even though un-commonly used in Linux.

With or without any changes:
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

Nicolas

>  	ddr_buffer.kernel_ptr = HOST_ADDRESS(NULL);
>  	ddr_buffer.cookie_ptr = buffer->driver_cookie;
>  	ddr_buffer.timing_data = buffer->timing_data;