All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
@ 2023-04-18 11:23 ` Simon Horman
  0 siblings, 0 replies; 24+ messages in thread
From: Simon Horman @ 2023-04-18 11:23 UTC (permalink / raw)
  To: Florian Fainelli, Greg Kroah-Hartman
  Cc: Broadcom internal kernel review list, Nathan Chancellor,
	Nick Desaulniers, Tom Rix, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, llvm

vchiq_platform_init() is only used in this file so it can be static.

clang-16 with W=1 reports:

 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for function 'vchiq_platform_init' [-Werror,-Wmissing-prototypes]
 int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
     ^
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:1: note: declare 'static' if the function is not intended to be used outside of this translation unit

Signed-off-by: Simon Horman <horms@kernel.org>
---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 40dd62cf7399..59312b9f2058 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -462,7 +462,8 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
 	cleanup_pagelistinfo(instance, pagelistinfo);
 }
 
-int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
+static int
+vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
 {
 	struct device *dev = &pdev->dev;
 	struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
@ 2023-04-18 11:23 ` Simon Horman
  0 siblings, 0 replies; 24+ messages in thread
From: Simon Horman @ 2023-04-18 11:23 UTC (permalink / raw)
  To: Florian Fainelli, Greg Kroah-Hartman
  Cc: Broadcom internal kernel review list, Nathan Chancellor,
	Nick Desaulniers, Tom Rix, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, llvm

vchiq_platform_init() is only used in this file so it can be static.

clang-16 with W=1 reports:

 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for function 'vchiq_platform_init' [-Werror,-Wmissing-prototypes]
 int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
     ^
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:1: note: declare 'static' if the function is not intended to be used outside of this translation unit

Signed-off-by: Simon Horman <horms@kernel.org>
---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 40dd62cf7399..59312b9f2058 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -462,7 +462,8 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
 	cleanup_pagelistinfo(instance, pagelistinfo);
 }
 
-int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
+static int
+vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
 {
 	struct device *dev = &pdev->dev;
 	struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
  2023-04-18 11:23 ` Simon Horman
@ 2023-04-18 18:39   ` Nick Desaulniers
  -1 siblings, 0 replies; 24+ messages in thread
From: Nick Desaulniers @ 2023-04-18 18:39 UTC (permalink / raw)
  To: Simon Horman
  Cc: Florian Fainelli, Greg Kroah-Hartman,
	Broadcom internal kernel review list, Nathan Chancellor, Tom Rix,
	linux-rpi-kernel, linux-arm-kernel, linux-staging, llvm

On Tue, Apr 18, 2023 at 4:23 AM Simon Horman <horms@kernel.org> wrote:
>
> vchiq_platform_init() is only used in this file so it can be static.
>
> clang-16 with W=1 reports:
>
>  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for function 'vchiq_platform_init' [-Werror,-Wmissing-prototypes]
>  int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
>      ^
>  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
>
> Signed-off-by: Simon Horman <horms@kernel.org>

I would have kept the return type on the original line and just added
the explicit linkage on top, but it's fine and checkpatch doesn't
complain. Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index 40dd62cf7399..59312b9f2058 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -462,7 +462,8 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
>         cleanup_pagelistinfo(instance, pagelistinfo);
>  }
>
> -int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> +static int
> +vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
>  {
>         struct device *dev = &pdev->dev;
>         struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
@ 2023-04-18 18:39   ` Nick Desaulniers
  0 siblings, 0 replies; 24+ messages in thread
From: Nick Desaulniers @ 2023-04-18 18:39 UTC (permalink / raw)
  To: Simon Horman
  Cc: Florian Fainelli, Greg Kroah-Hartman,
	Broadcom internal kernel review list, Nathan Chancellor, Tom Rix,
	linux-rpi-kernel, linux-arm-kernel, linux-staging, llvm

On Tue, Apr 18, 2023 at 4:23 AM Simon Horman <horms@kernel.org> wrote:
>
> vchiq_platform_init() is only used in this file so it can be static.
>
> clang-16 with W=1 reports:
>
>  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for function 'vchiq_platform_init' [-Werror,-Wmissing-prototypes]
>  int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
>      ^
>  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
>
> Signed-off-by: Simon Horman <horms@kernel.org>

I would have kept the return type on the original line and just added
the explicit linkage on top, but it's fine and checkpatch doesn't
complain. Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index 40dd62cf7399..59312b9f2058 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -462,7 +462,8 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
>         cleanup_pagelistinfo(instance, pagelistinfo);
>  }
>
> -int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> +static int
> +vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
>  {
>         struct device *dev = &pdev->dev;
>         struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);
>


-- 
Thanks,
~Nick Desaulniers

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
  2023-04-18 11:23 ` Simon Horman
@ 2023-04-18 18:44   ` Nathan Chancellor
  -1 siblings, 0 replies; 24+ messages in thread
From: Nathan Chancellor @ 2023-04-18 18:44 UTC (permalink / raw)
  To: Simon Horman
  Cc: Florian Fainelli, Greg Kroah-Hartman,
	Broadcom internal kernel review list, Nick Desaulniers, Tom Rix,
	linux-rpi-kernel, linux-arm-kernel, linux-staging, llvm

On Tue, Apr 18, 2023 at 01:23:11PM +0200, Simon Horman wrote:
> vchiq_platform_init() is only used in this file so it can be static.
> 
> clang-16 with W=1 reports:
> 
>  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for function 'vchiq_platform_init' [-Werror,-Wmissing-prototypes]
>  int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
>      ^
>  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
> 
> Signed-off-by: Simon Horman <horms@kernel.org>

Introduced by commit 89cc4218f640 ("staging: vchiq_arm: drop unnecessary
declarations"), not sure it is worth a fixes tag since it is a W=1
warning.

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index 40dd62cf7399..59312b9f2058 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -462,7 +462,8 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
>  	cleanup_pagelistinfo(instance, pagelistinfo);
>  }
>  
> -int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> +static int
> +vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
@ 2023-04-18 18:44   ` Nathan Chancellor
  0 siblings, 0 replies; 24+ messages in thread
From: Nathan Chancellor @ 2023-04-18 18:44 UTC (permalink / raw)
  To: Simon Horman
  Cc: Florian Fainelli, Greg Kroah-Hartman,
	Broadcom internal kernel review list, Nick Desaulniers, Tom Rix,
	linux-rpi-kernel, linux-arm-kernel, linux-staging, llvm

On Tue, Apr 18, 2023 at 01:23:11PM +0200, Simon Horman wrote:
> vchiq_platform_init() is only used in this file so it can be static.
> 
> clang-16 with W=1 reports:
> 
>  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for function 'vchiq_platform_init' [-Werror,-Wmissing-prototypes]
>  int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
>      ^
>  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
> 
> Signed-off-by: Simon Horman <horms@kernel.org>

Introduced by commit 89cc4218f640 ("staging: vchiq_arm: drop unnecessary
declarations"), not sure it is worth a fixes tag since it is a W=1
warning.

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index 40dd62cf7399..59312b9f2058 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -462,7 +462,8 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
>  	cleanup_pagelistinfo(instance, pagelistinfo);
>  }
>  
> -int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> +static int
> +vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
  2023-04-18 18:39   ` Nick Desaulniers
@ 2023-04-19  5:39     ` Simon Horman
  -1 siblings, 0 replies; 24+ messages in thread
From: Simon Horman @ 2023-04-19  5:39 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Florian Fainelli, Greg Kroah-Hartman,
	Broadcom internal kernel review list, Nathan Chancellor, Tom Rix,
	linux-rpi-kernel, linux-arm-kernel, linux-staging, llvm

On Tue, Apr 18, 2023 at 11:39:58AM -0700, Nick Desaulniers wrote:
> On Tue, Apr 18, 2023 at 4:23 AM Simon Horman <horms@kernel.org> wrote:
> >
> > vchiq_platform_init() is only used in this file so it can be static.
> >
> > clang-16 with W=1 reports:
> >
> >  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for function 'vchiq_platform_init' [-Werror,-Wmissing-prototypes]
> >  int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> >      ^
> >  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
> >
> > Signed-off-by: Simon Horman <horms@kernel.org>
> 
> I would have kept the return type on the original line and just added
> the explicit linkage on top, but it's fine and checkpatch doesn't
> complain. Thanks for the patch!

Thanks, I wasn't sure which was the best way to to on that.
I'll keep this in mind in future.

> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> 
> > ---
> >  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > index 40dd62cf7399..59312b9f2058 100644
> > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > @@ -462,7 +462,8 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
> >         cleanup_pagelistinfo(instance, pagelistinfo);
> >  }
> >
> > -int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> > +static int
> > +vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> >  {
> >         struct device *dev = &pdev->dev;
> >         struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);
> >
> 
> 
> -- 
> Thanks,
> ~Nick Desaulniers
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
@ 2023-04-19  5:39     ` Simon Horman
  0 siblings, 0 replies; 24+ messages in thread
From: Simon Horman @ 2023-04-19  5:39 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Florian Fainelli, Greg Kroah-Hartman,
	Broadcom internal kernel review list, Nathan Chancellor, Tom Rix,
	linux-rpi-kernel, linux-arm-kernel, linux-staging, llvm

On Tue, Apr 18, 2023 at 11:39:58AM -0700, Nick Desaulniers wrote:
> On Tue, Apr 18, 2023 at 4:23 AM Simon Horman <horms@kernel.org> wrote:
> >
> > vchiq_platform_init() is only used in this file so it can be static.
> >
> > clang-16 with W=1 reports:
> >
> >  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for function 'vchiq_platform_init' [-Werror,-Wmissing-prototypes]
> >  int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> >      ^
> >  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
> >
> > Signed-off-by: Simon Horman <horms@kernel.org>
> 
> I would have kept the return type on the original line and just added
> the explicit linkage on top, but it's fine and checkpatch doesn't
> complain. Thanks for the patch!

Thanks, I wasn't sure which was the best way to to on that.
I'll keep this in mind in future.

> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> 
> > ---
> >  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > index 40dd62cf7399..59312b9f2058 100644
> > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > @@ -462,7 +462,8 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
> >         cleanup_pagelistinfo(instance, pagelistinfo);
> >  }
> >
> > -int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> > +static int
> > +vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> >  {
> >         struct device *dev = &pdev->dev;
> >         struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);
> >
> 
> 
> -- 
> Thanks,
> ~Nick Desaulniers
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
  2023-04-18 18:44   ` Nathan Chancellor
@ 2023-04-19  5:42     ` Simon Horman
  -1 siblings, 0 replies; 24+ messages in thread
From: Simon Horman @ 2023-04-19  5:42 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Florian Fainelli, Greg Kroah-Hartman,
	Broadcom internal kernel review list, Nick Desaulniers, Tom Rix,
	linux-rpi-kernel, linux-arm-kernel, linux-staging, llvm

On Tue, Apr 18, 2023 at 11:44:19AM -0700, Nathan Chancellor wrote:
> On Tue, Apr 18, 2023 at 01:23:11PM +0200, Simon Horman wrote:
> > vchiq_platform_init() is only used in this file so it can be static.
> > 
> > clang-16 with W=1 reports:
> > 
> >  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for function 'vchiq_platform_init' [-Werror,-Wmissing-prototypes]
> >  int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> >      ^
> >  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
> > 
> > Signed-off-by: Simon Horman <horms@kernel.org>
> 
> Introduced by commit 89cc4218f640 ("staging: vchiq_arm: drop unnecessary
> declarations"), not sure it is worth a fixes tag since it is a W=1
> warning.

Thanks. For some reason it didn't occur to me to check which
patch introduced the issue. Perhaps because, as you say, it's a W=1
issue, and I don't think that warrants a fixes tag.

I could include some information in the commit message, like
"Introduced by ..." if desired.

> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> 
> > ---
> >  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > index 40dd62cf7399..59312b9f2058 100644
> > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > @@ -462,7 +462,8 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
> >  	cleanup_pagelistinfo(instance, pagelistinfo);
> >  }
> >  
> > -int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> > +static int
> > +vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> >  {
> >  	struct device *dev = &pdev->dev;
> >  	struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);
> > 
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
@ 2023-04-19  5:42     ` Simon Horman
  0 siblings, 0 replies; 24+ messages in thread
From: Simon Horman @ 2023-04-19  5:42 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Florian Fainelli, Greg Kroah-Hartman,
	Broadcom internal kernel review list, Nick Desaulniers, Tom Rix,
	linux-rpi-kernel, linux-arm-kernel, linux-staging, llvm

On Tue, Apr 18, 2023 at 11:44:19AM -0700, Nathan Chancellor wrote:
> On Tue, Apr 18, 2023 at 01:23:11PM +0200, Simon Horman wrote:
> > vchiq_platform_init() is only used in this file so it can be static.
> > 
> > clang-16 with W=1 reports:
> > 
> >  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for function 'vchiq_platform_init' [-Werror,-Wmissing-prototypes]
> >  int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> >      ^
> >  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
> > 
> > Signed-off-by: Simon Horman <horms@kernel.org>
> 
> Introduced by commit 89cc4218f640 ("staging: vchiq_arm: drop unnecessary
> declarations"), not sure it is worth a fixes tag since it is a W=1
> warning.

Thanks. For some reason it didn't occur to me to check which
patch introduced the issue. Perhaps because, as you say, it's a W=1
issue, and I don't think that warrants a fixes tag.

I could include some information in the commit message, like
"Introduced by ..." if desired.

> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> 
> > ---
> >  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > index 40dd62cf7399..59312b9f2058 100644
> > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > @@ -462,7 +462,8 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
> >  	cleanup_pagelistinfo(instance, pagelistinfo);
> >  }
> >  
> > -int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> > +static int
> > +vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> >  {
> >  	struct device *dev = &pdev->dev;
> >  	struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);
> > 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
  2023-04-18 11:23 ` Simon Horman
@ 2023-04-19  6:37   ` Stefan Wahren
  -1 siblings, 0 replies; 24+ messages in thread
From: Stefan Wahren @ 2023-04-19  6:37 UTC (permalink / raw)
  To: Simon Horman
  Cc: Broadcom internal kernel review list, Nathan Chancellor,
	Nick Desaulniers, Tom Rix, Greg Kroah-Hartman, linux-rpi-kernel,
	linux-arm-kernel, linux-staging, llvm, Florian Fainelli

Hi Simon,

Am 18.04.23 um 13:23 schrieb Simon Horman:
> vchiq_platform_init() is only used in this file so it can be static.
> 
> clang-16 with W=1 reports:
> 
>   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for function 'vchiq_platform_init' [-Werror,-Wmissing-prototypes]
>   int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
>       ^
>   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
> 
> Signed-off-by: Simon Horman <horms@kernel.org>

this is not the first attempt to fix this [1]. But maybe this has been 
fixed in the meantime.

[1] - 
https://lore.kernel.org/linux-staging/20221022043548.1671644-1-scottjcrouch@gmail.com/#t

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
@ 2023-04-19  6:37   ` Stefan Wahren
  0 siblings, 0 replies; 24+ messages in thread
From: Stefan Wahren @ 2023-04-19  6:37 UTC (permalink / raw)
  To: Simon Horman
  Cc: Broadcom internal kernel review list, Nathan Chancellor,
	Nick Desaulniers, Tom Rix, Greg Kroah-Hartman, linux-rpi-kernel,
	linux-arm-kernel, linux-staging, llvm, Florian Fainelli

Hi Simon,

Am 18.04.23 um 13:23 schrieb Simon Horman:
> vchiq_platform_init() is only used in this file so it can be static.
> 
> clang-16 with W=1 reports:
> 
>   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for function 'vchiq_platform_init' [-Werror,-Wmissing-prototypes]
>   int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
>       ^
>   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
> 
> Signed-off-by: Simon Horman <horms@kernel.org>

this is not the first attempt to fix this [1]. But maybe this has been 
fixed in the meantime.

[1] - 
https://lore.kernel.org/linux-staging/20221022043548.1671644-1-scottjcrouch@gmail.com/#t

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
  2023-04-19  6:37   ` Stefan Wahren
@ 2023-04-19  8:51     ` Simon Horman
  -1 siblings, 0 replies; 24+ messages in thread
From: Simon Horman @ 2023-04-19  8:51 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: Broadcom internal kernel review list, Nathan Chancellor,
	Nick Desaulniers, Tom Rix, Greg Kroah-Hartman, linux-rpi-kernel,
	linux-arm-kernel, linux-staging, llvm, Florian Fainelli

On Wed, Apr 19, 2023 at 08:37:58AM +0200, Stefan Wahren wrote:
> Hi Simon,
> 
> Am 18.04.23 um 13:23 schrieb Simon Horman:
> > vchiq_platform_init() is only used in this file so it can be static.
> > 
> > clang-16 with W=1 reports:
> > 
> >   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for function 'vchiq_platform_init' [-Werror,-Wmissing-prototypes]
> >   int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> >       ^
> >   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
> > 
> > Signed-off-by: Simon Horman <horms@kernel.org>
> 
> this is not the first attempt to fix this [1]. But maybe this has been fixed
> in the meantime.
> 
> [1] - https://lore.kernel.org/linux-staging/20221022043548.1671644-1-scottjcrouch@gmail.com/#t

Thanks,

I see in that thread that Greg notes that "this breaks the build in a very
horrible and strange way" [2].

I'm a bit surprised as I didn't observe such a thing.
But I also don't dispute it - clearly my testing was inadequate.

I'll try and dig a bit deeper into this one.
Perhaps starting with the report from the kernel test robot [3]

[2] https://lore.kernel.org/linux-staging/Y1Okpjbi2kKU2GFz@kroah.com/
[3] https://lore.kernel.org/linux-staging/202210230127.p1Dp295m-lkp@intel.com/

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
@ 2023-04-19  8:51     ` Simon Horman
  0 siblings, 0 replies; 24+ messages in thread
From: Simon Horman @ 2023-04-19  8:51 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: Broadcom internal kernel review list, Nathan Chancellor,
	Nick Desaulniers, Tom Rix, Greg Kroah-Hartman, linux-rpi-kernel,
	linux-arm-kernel, linux-staging, llvm, Florian Fainelli

On Wed, Apr 19, 2023 at 08:37:58AM +0200, Stefan Wahren wrote:
> Hi Simon,
> 
> Am 18.04.23 um 13:23 schrieb Simon Horman:
> > vchiq_platform_init() is only used in this file so it can be static.
> > 
> > clang-16 with W=1 reports:
> > 
> >   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for function 'vchiq_platform_init' [-Werror,-Wmissing-prototypes]
> >   int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> >       ^
> >   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
> > 
> > Signed-off-by: Simon Horman <horms@kernel.org>
> 
> this is not the first attempt to fix this [1]. But maybe this has been fixed
> in the meantime.
> 
> [1] - https://lore.kernel.org/linux-staging/20221022043548.1671644-1-scottjcrouch@gmail.com/#t

Thanks,

I see in that thread that Greg notes that "this breaks the build in a very
horrible and strange way" [2].

I'm a bit surprised as I didn't observe such a thing.
But I also don't dispute it - clearly my testing was inadequate.

I'll try and dig a bit deeper into this one.
Perhaps starting with the report from the kernel test robot [3]

[2] https://lore.kernel.org/linux-staging/Y1Okpjbi2kKU2GFz@kroah.com/
[3] https://lore.kernel.org/linux-staging/202210230127.p1Dp295m-lkp@intel.com/

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
  2023-04-19  8:51     ` Simon Horman
@ 2023-04-19 21:21       ` Nick Desaulniers
  -1 siblings, 0 replies; 24+ messages in thread
From: Nick Desaulniers @ 2023-04-19 21:21 UTC (permalink / raw)
  To: Simon Horman, Arnd Bergmann
  Cc: Stefan Wahren, Broadcom internal kernel review list,
	Nathan Chancellor, Tom Rix, Greg Kroah-Hartman, linux-rpi-kernel,
	linux-arm-kernel, linux-staging, llvm, Florian Fainelli

+ Arnd

On Wed, Apr 19, 2023 at 1:51 AM Simon Horman <horms@kernel.org> wrote:
>
> On Wed, Apr 19, 2023 at 08:37:58AM +0200, Stefan Wahren wrote:
> > Hi Simon,
> >
> > Am 18.04.23 um 13:23 schrieb Simon Horman:
> > > vchiq_platform_init() is only used in this file so it can be static.
> > >
> > > clang-16 with W=1 reports:
> > >
> > >   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for function 'vchiq_platform_init' [-Werror,-Wmissing-prototypes]
> > >   int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> > >       ^
> > >   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
> > >
> > > Signed-off-by: Simon Horman <horms@kernel.org>
> >
> > this is not the first attempt to fix this [1]. But maybe this has been fixed
> > in the meantime.
> >
> > [1] - https://lore.kernel.org/linux-staging/20221022043548.1671644-1-scottjcrouch@gmail.com/#t
>
> Thanks,
>
> I see in that thread that Greg notes that "this breaks the build in a very
> horrible and strange way" [2].

So Arnd had a Kconfig suggestion later in that thread:
https://lore.kernel.org/linux-staging/d64dc0b4-ba82-49f4-9a43-c50b735ad91d@app.fastmail.com/

Marking something as `depends on OF_DYNAMIC`. (Not sure whether Arnd
meant that for BCM2835_VCHIQ or BCM_VIDEOCORE).
Perhaps try that?

>
> I'm a bit surprised as I didn't observe such a thing.
> But I also don't dispute it - clearly my testing was inadequate.
>
> I'll try and dig a bit deeper into this one.
> Perhaps starting with the report from the kernel test robot [3]
>
> [2] https://lore.kernel.org/linux-staging/Y1Okpjbi2kKU2GFz@kroah.com/
> [3] https://lore.kernel.org/linux-staging/202210230127.p1Dp295m-lkp@intel.com/



-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
@ 2023-04-19 21:21       ` Nick Desaulniers
  0 siblings, 0 replies; 24+ messages in thread
From: Nick Desaulniers @ 2023-04-19 21:21 UTC (permalink / raw)
  To: Simon Horman, Arnd Bergmann
  Cc: Stefan Wahren, Broadcom internal kernel review list,
	Nathan Chancellor, Tom Rix, Greg Kroah-Hartman, linux-rpi-kernel,
	linux-arm-kernel, linux-staging, llvm, Florian Fainelli

+ Arnd

On Wed, Apr 19, 2023 at 1:51 AM Simon Horman <horms@kernel.org> wrote:
>
> On Wed, Apr 19, 2023 at 08:37:58AM +0200, Stefan Wahren wrote:
> > Hi Simon,
> >
> > Am 18.04.23 um 13:23 schrieb Simon Horman:
> > > vchiq_platform_init() is only used in this file so it can be static.
> > >
> > > clang-16 with W=1 reports:
> > >
> > >   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for function 'vchiq_platform_init' [-Werror,-Wmissing-prototypes]
> > >   int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> > >       ^
> > >   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
> > >
> > > Signed-off-by: Simon Horman <horms@kernel.org>
> >
> > this is not the first attempt to fix this [1]. But maybe this has been fixed
> > in the meantime.
> >
> > [1] - https://lore.kernel.org/linux-staging/20221022043548.1671644-1-scottjcrouch@gmail.com/#t
>
> Thanks,
>
> I see in that thread that Greg notes that "this breaks the build in a very
> horrible and strange way" [2].

So Arnd had a Kconfig suggestion later in that thread:
https://lore.kernel.org/linux-staging/d64dc0b4-ba82-49f4-9a43-c50b735ad91d@app.fastmail.com/

Marking something as `depends on OF_DYNAMIC`. (Not sure whether Arnd
meant that for BCM2835_VCHIQ or BCM_VIDEOCORE).
Perhaps try that?

>
> I'm a bit surprised as I didn't observe such a thing.
> But I also don't dispute it - clearly my testing was inadequate.
>
> I'll try and dig a bit deeper into this one.
> Perhaps starting with the report from the kernel test robot [3]
>
> [2] https://lore.kernel.org/linux-staging/Y1Okpjbi2kKU2GFz@kroah.com/
> [3] https://lore.kernel.org/linux-staging/202210230127.p1Dp295m-lkp@intel.com/



-- 
Thanks,
~Nick Desaulniers

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
  2023-04-19 21:21       ` Nick Desaulniers
@ 2023-04-19 21:35         ` Arnd Bergmann
  -1 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2023-04-19 21:35 UTC (permalink / raw)
  To: Nick Desaulniers, Simon Horman
  Cc: Stefan Wahren, Broadcom internal kernel review list,
	Nathan Chancellor, Tom Rix, Greg Kroah-Hartman, linux-rpi-kernel,
	linux-arm-kernel, linux-staging, llvm, Florian Fainelli

On Wed, Apr 19, 2023, at 23:21, Nick Desaulniers wrote:
> On Wed, Apr 19, 2023 at 1:51 AM Simon Horman <horms@kernel.org> wrote:

> Marking something as `depends on OF_DYNAMIC`. (Not sure whether Arnd
> meant that for BCM2835_VCHIQ or BCM_VIDEOCORE).
> Perhaps try that?
>
>>
>> I'm a bit surprised as I didn't observe such a thing.
>> But I also don't dispute it - clearly my testing was inadequate.
>>
>> I'll try and dig a bit deeper into this one.
>> Perhaps starting with the report from the kernel test robot [3]

I have this in my randconfig build tree, but it looks like I never
sent it out.

----
From bb01f96d7dd66fc437302785f623a3ab323d486e Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Mon, 20 Mar 2023 11:33:52 +0100
Subject: [PATCH] staging: vchiq_arm: mark vchiq_platform_init() static

This function has no callers from other files, and the declaration
was removed a while ago, causing a W=1 warning:

drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for 'vchiq_platform_init'

Marking it static solves this problem but introduces a new warning
since gcc determines that 'g_fragments_base' is never initialized
in some kernel configurations:

In file included from include/linux/string.h:254,
                 from include/linux/bitmap.h:11,
                 from include/linux/cpumask.h:12,
                 from include/linux/mm_types_task.h:14,
                 from include/linux/mm_types.h:5,
                 from include/linux/buildid.h:5,
                 from include/linux/module.h:14,
                 from drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:8:
In function 'memcpy_to_page',
    inlined from 'free_pagelist' at drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:433:4:
include/linux/fortify-string.h:57:33: error: argument 2 null where non-null expected [-Werror=nonnull]
include/linux/highmem.h:427:9: note: in expansion of macro 'memcpy'
  427 |         memcpy(to + offset, from, len);
      |         ^~~~~~

Add a NULL pointer check for this in addition to the static annotation
to avoid both.

Fixes: 89cc4218f640 ("staging: vchiq_arm: drop unnecessary declarations")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 90a3958d1f29..aa2313f3bcab 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -415,7 +415,7 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
 	pagelistinfo->scatterlist_mapped = 0;
 
 	/* Deal with any partial cache lines (fragments) */
-	if (pagelist->type >= PAGELIST_READ_WITH_FRAGMENTS) {
+	if (pagelist->type >= PAGELIST_READ_WITH_FRAGMENTS && g_fragments_base) {
 		char *fragments = g_fragments_base +
 			(pagelist->type - PAGELIST_READ_WITH_FRAGMENTS) *
 			g_fragments_size;
@@ -462,7 +462,7 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
 	cleanup_pagelistinfo(instance, pagelistinfo);
 }
 
-int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
+static int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
 {
 	struct device *dev = &pdev->dev;
 	struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
@ 2023-04-19 21:35         ` Arnd Bergmann
  0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2023-04-19 21:35 UTC (permalink / raw)
  To: Nick Desaulniers, Simon Horman
  Cc: Stefan Wahren, Broadcom internal kernel review list,
	Nathan Chancellor, Tom Rix, Greg Kroah-Hartman, linux-rpi-kernel,
	linux-arm-kernel, linux-staging, llvm, Florian Fainelli

On Wed, Apr 19, 2023, at 23:21, Nick Desaulniers wrote:
> On Wed, Apr 19, 2023 at 1:51 AM Simon Horman <horms@kernel.org> wrote:

> Marking something as `depends on OF_DYNAMIC`. (Not sure whether Arnd
> meant that for BCM2835_VCHIQ or BCM_VIDEOCORE).
> Perhaps try that?
>
>>
>> I'm a bit surprised as I didn't observe such a thing.
>> But I also don't dispute it - clearly my testing was inadequate.
>>
>> I'll try and dig a bit deeper into this one.
>> Perhaps starting with the report from the kernel test robot [3]

I have this in my randconfig build tree, but it looks like I never
sent it out.

----
From bb01f96d7dd66fc437302785f623a3ab323d486e Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Mon, 20 Mar 2023 11:33:52 +0100
Subject: [PATCH] staging: vchiq_arm: mark vchiq_platform_init() static

This function has no callers from other files, and the declaration
was removed a while ago, causing a W=1 warning:

drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for 'vchiq_platform_init'

Marking it static solves this problem but introduces a new warning
since gcc determines that 'g_fragments_base' is never initialized
in some kernel configurations:

In file included from include/linux/string.h:254,
                 from include/linux/bitmap.h:11,
                 from include/linux/cpumask.h:12,
                 from include/linux/mm_types_task.h:14,
                 from include/linux/mm_types.h:5,
                 from include/linux/buildid.h:5,
                 from include/linux/module.h:14,
                 from drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:8:
In function 'memcpy_to_page',
    inlined from 'free_pagelist' at drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:433:4:
include/linux/fortify-string.h:57:33: error: argument 2 null where non-null expected [-Werror=nonnull]
include/linux/highmem.h:427:9: note: in expansion of macro 'memcpy'
  427 |         memcpy(to + offset, from, len);
      |         ^~~~~~

Add a NULL pointer check for this in addition to the static annotation
to avoid both.

Fixes: 89cc4218f640 ("staging: vchiq_arm: drop unnecessary declarations")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 90a3958d1f29..aa2313f3bcab 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -415,7 +415,7 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
 	pagelistinfo->scatterlist_mapped = 0;
 
 	/* Deal with any partial cache lines (fragments) */
-	if (pagelist->type >= PAGELIST_READ_WITH_FRAGMENTS) {
+	if (pagelist->type >= PAGELIST_READ_WITH_FRAGMENTS && g_fragments_base) {
 		char *fragments = g_fragments_base +
 			(pagelist->type - PAGELIST_READ_WITH_FRAGMENTS) *
 			g_fragments_size;
@@ -462,7 +462,7 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
 	cleanup_pagelistinfo(instance, pagelistinfo);
 }
 
-int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
+static int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
 {
 	struct device *dev = &pdev->dev;
 	struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
  2023-04-19 21:35         ` Arnd Bergmann
@ 2023-04-19 21:38           ` Nick Desaulniers
  -1 siblings, 0 replies; 24+ messages in thread
From: Nick Desaulniers @ 2023-04-19 21:38 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Simon Horman, Stefan Wahren,
	Broadcom internal kernel review list, Nathan Chancellor, Tom Rix,
	Greg Kroah-Hartman, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, llvm, Florian Fainelli

On Wed, Apr 19, 2023 at 2:35 PM Arnd Bergmann <arnd@kernel.org> wrote:
>
> On Wed, Apr 19, 2023, at 23:21, Nick Desaulniers wrote:
> > On Wed, Apr 19, 2023 at 1:51 AM Simon Horman <horms@kernel.org> wrote:
>
> > Marking something as `depends on OF_DYNAMIC`. (Not sure whether Arnd
> > meant that for BCM2835_VCHIQ or BCM_VIDEOCORE).
> > Perhaps try that?
> >
> >>
> >> I'm a bit surprised as I didn't observe such a thing.
> >> But I also don't dispute it - clearly my testing was inadequate.
> >>
> >> I'll try and dig a bit deeper into this one.
> >> Perhaps starting with the report from the kernel test robot [3]
>
> I have this in my randconfig build tree, but it looks like I never
> sent it out.

What else have you got hidden up in there? ;)
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>


>
> ----
> From bb01f96d7dd66fc437302785f623a3ab323d486e Mon Sep 17 00:00:00 2001
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Mon, 20 Mar 2023 11:33:52 +0100
> Subject: [PATCH] staging: vchiq_arm: mark vchiq_platform_init() static
>
> This function has no callers from other files, and the declaration
> was removed a while ago, causing a W=1 warning:
>
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for 'vchiq_platform_init'
>
> Marking it static solves this problem but introduces a new warning
> since gcc determines that 'g_fragments_base' is never initialized
> in some kernel configurations:
>
> In file included from include/linux/string.h:254,
>                  from include/linux/bitmap.h:11,
>                  from include/linux/cpumask.h:12,
>                  from include/linux/mm_types_task.h:14,
>                  from include/linux/mm_types.h:5,
>                  from include/linux/buildid.h:5,
>                  from include/linux/module.h:14,
>                  from drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:8:
> In function 'memcpy_to_page',
>     inlined from 'free_pagelist' at drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:433:4:
> include/linux/fortify-string.h:57:33: error: argument 2 null where non-null expected [-Werror=nonnull]
> include/linux/highmem.h:427:9: note: in expansion of macro 'memcpy'
>   427 |         memcpy(to + offset, from, len);
>       |         ^~~~~~
>
> Add a NULL pointer check for this in addition to the static annotation
> to avoid both.
>
> Fixes: 89cc4218f640 ("staging: vchiq_arm: drop unnecessary declarations")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index 90a3958d1f29..aa2313f3bcab 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -415,7 +415,7 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
>         pagelistinfo->scatterlist_mapped = 0;
>
>         /* Deal with any partial cache lines (fragments) */
> -       if (pagelist->type >= PAGELIST_READ_WITH_FRAGMENTS) {
> +       if (pagelist->type >= PAGELIST_READ_WITH_FRAGMENTS && g_fragments_base) {
>                 char *fragments = g_fragments_base +
>                         (pagelist->type - PAGELIST_READ_WITH_FRAGMENTS) *
>                         g_fragments_size;
> @@ -462,7 +462,7 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
>         cleanup_pagelistinfo(instance, pagelistinfo);
>  }
>
> -int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> +static int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
>  {
>         struct device *dev = &pdev->dev;
>         struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
@ 2023-04-19 21:38           ` Nick Desaulniers
  0 siblings, 0 replies; 24+ messages in thread
From: Nick Desaulniers @ 2023-04-19 21:38 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Simon Horman, Stefan Wahren,
	Broadcom internal kernel review list, Nathan Chancellor, Tom Rix,
	Greg Kroah-Hartman, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, llvm, Florian Fainelli

On Wed, Apr 19, 2023 at 2:35 PM Arnd Bergmann <arnd@kernel.org> wrote:
>
> On Wed, Apr 19, 2023, at 23:21, Nick Desaulniers wrote:
> > On Wed, Apr 19, 2023 at 1:51 AM Simon Horman <horms@kernel.org> wrote:
>
> > Marking something as `depends on OF_DYNAMIC`. (Not sure whether Arnd
> > meant that for BCM2835_VCHIQ or BCM_VIDEOCORE).
> > Perhaps try that?
> >
> >>
> >> I'm a bit surprised as I didn't observe such a thing.
> >> But I also don't dispute it - clearly my testing was inadequate.
> >>
> >> I'll try and dig a bit deeper into this one.
> >> Perhaps starting with the report from the kernel test robot [3]
>
> I have this in my randconfig build tree, but it looks like I never
> sent it out.

What else have you got hidden up in there? ;)
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>


>
> ----
> From bb01f96d7dd66fc437302785f623a3ab323d486e Mon Sep 17 00:00:00 2001
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Mon, 20 Mar 2023 11:33:52 +0100
> Subject: [PATCH] staging: vchiq_arm: mark vchiq_platform_init() static
>
> This function has no callers from other files, and the declaration
> was removed a while ago, causing a W=1 warning:
>
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for 'vchiq_platform_init'
>
> Marking it static solves this problem but introduces a new warning
> since gcc determines that 'g_fragments_base' is never initialized
> in some kernel configurations:
>
> In file included from include/linux/string.h:254,
>                  from include/linux/bitmap.h:11,
>                  from include/linux/cpumask.h:12,
>                  from include/linux/mm_types_task.h:14,
>                  from include/linux/mm_types.h:5,
>                  from include/linux/buildid.h:5,
>                  from include/linux/module.h:14,
>                  from drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:8:
> In function 'memcpy_to_page',
>     inlined from 'free_pagelist' at drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:433:4:
> include/linux/fortify-string.h:57:33: error: argument 2 null where non-null expected [-Werror=nonnull]
> include/linux/highmem.h:427:9: note: in expansion of macro 'memcpy'
>   427 |         memcpy(to + offset, from, len);
>       |         ^~~~~~
>
> Add a NULL pointer check for this in addition to the static annotation
> to avoid both.
>
> Fixes: 89cc4218f640 ("staging: vchiq_arm: drop unnecessary declarations")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index 90a3958d1f29..aa2313f3bcab 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -415,7 +415,7 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
>         pagelistinfo->scatterlist_mapped = 0;
>
>         /* Deal with any partial cache lines (fragments) */
> -       if (pagelist->type >= PAGELIST_READ_WITH_FRAGMENTS) {
> +       if (pagelist->type >= PAGELIST_READ_WITH_FRAGMENTS && g_fragments_base) {
>                 char *fragments = g_fragments_base +
>                         (pagelist->type - PAGELIST_READ_WITH_FRAGMENTS) *
>                         g_fragments_size;
> @@ -462,7 +462,7 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
>         cleanup_pagelistinfo(instance, pagelistinfo);
>  }
>
> -int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> +static int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
>  {
>         struct device *dev = &pdev->dev;
>         struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);
>


-- 
Thanks,
~Nick Desaulniers

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
  2023-04-19 21:38           ` Nick Desaulniers
@ 2023-04-19 21:41             ` Arnd Bergmann
  -1 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2023-04-19 21:41 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Simon Horman, Stefan Wahren,
	Broadcom internal kernel review list, Nathan Chancellor, Tom Rix,
	Greg Kroah-Hartman, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, llvm, Florian Fainelli

On Wed, Apr 19, 2023, at 23:38, Nick Desaulniers wrote:
> On Wed, Apr 19, 2023 at 2:35 PM Arnd Bergmann <arnd@kernel.org> wrote:
>>
>> On Wed, Apr 19, 2023, at 23:21, Nick Desaulniers wrote:
>> > On Wed, Apr 19, 2023 at 1:51 AM Simon Horman <horms@kernel.org> wrote:
>>
>> > Marking something as `depends on OF_DYNAMIC`. (Not sure whether Arnd
>> > meant that for BCM2835_VCHIQ or BCM_VIDEOCORE).
>> > Perhaps try that?
>> >
>> >>
>> >> I'm a bit surprised as I didn't observe such a thing.
>> >> But I also don't dispute it - clearly my testing was inadequate.
>> >>
>> >> I'll try and dig a bit deeper into this one.
>> >> Perhaps starting with the report from the kernel test robot [3]
>>
>> I have this in my randconfig build tree, but it looks like I never
>> sent it out.
>
> What else have you got hidden up in there? ;)
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

Actually, I have fixes for all missing-prototype warnings
in my tree, just remembered that this one is part of my
longer series. I have about 150 patches that are already split
up and just need to be sent out, plus more that are currently
sitting in a combined changeset.

     Arnd

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
@ 2023-04-19 21:41             ` Arnd Bergmann
  0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2023-04-19 21:41 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Simon Horman, Stefan Wahren,
	Broadcom internal kernel review list, Nathan Chancellor, Tom Rix,
	Greg Kroah-Hartman, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, llvm, Florian Fainelli

On Wed, Apr 19, 2023, at 23:38, Nick Desaulniers wrote:
> On Wed, Apr 19, 2023 at 2:35 PM Arnd Bergmann <arnd@kernel.org> wrote:
>>
>> On Wed, Apr 19, 2023, at 23:21, Nick Desaulniers wrote:
>> > On Wed, Apr 19, 2023 at 1:51 AM Simon Horman <horms@kernel.org> wrote:
>>
>> > Marking something as `depends on OF_DYNAMIC`. (Not sure whether Arnd
>> > meant that for BCM2835_VCHIQ or BCM_VIDEOCORE).
>> > Perhaps try that?
>> >
>> >>
>> >> I'm a bit surprised as I didn't observe such a thing.
>> >> But I also don't dispute it - clearly my testing was inadequate.
>> >>
>> >> I'll try and dig a bit deeper into this one.
>> >> Perhaps starting with the report from the kernel test robot [3]
>>
>> I have this in my randconfig build tree, but it looks like I never
>> sent it out.
>
> What else have you got hidden up in there? ;)
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

Actually, I have fixes for all missing-prototype warnings
in my tree, just remembered that this one is part of my
longer series. I have about 150 patches that are already split
up and just need to be sent out, plus more that are currently
sitting in a combined changeset.

     Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
  2023-04-19 21:41             ` Arnd Bergmann
@ 2023-04-19 21:48               ` Nick Desaulniers
  -1 siblings, 0 replies; 24+ messages in thread
From: Nick Desaulniers @ 2023-04-19 21:48 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Simon Horman, Stefan Wahren,
	Broadcom internal kernel review list, Nathan Chancellor, Tom Rix,
	Greg Kroah-Hartman, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, llvm, Florian Fainelli

On Wed, Apr 19, 2023 at 2:42 PM Arnd Bergmann <arnd@kernel.org> wrote:
>
> On Wed, Apr 19, 2023, at 23:38, Nick Desaulniers wrote:
> > On Wed, Apr 19, 2023 at 2:35 PM Arnd Bergmann <arnd@kernel.org> wrote:
> >>
> >> On Wed, Apr 19, 2023, at 23:21, Nick Desaulniers wrote:
> >> > On Wed, Apr 19, 2023 at 1:51 AM Simon Horman <horms@kernel.org> wrote:
> >>
> >> > Marking something as `depends on OF_DYNAMIC`. (Not sure whether Arnd
> >> > meant that for BCM2835_VCHIQ or BCM_VIDEOCORE).
> >> > Perhaps try that?
> >> >
> >> >>
> >> >> I'm a bit surprised as I didn't observe such a thing.
> >> >> But I also don't dispute it - clearly my testing was inadequate.
> >> >>
> >> >> I'll try and dig a bit deeper into this one.
> >> >> Perhaps starting with the report from the kernel test robot [3]
> >>
> >> I have this in my randconfig build tree, but it looks like I never
> >> sent it out.
> >
> > What else have you got hidden up in there? ;)
> > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> Actually, I have fixes for all missing-prototype warnings
> in my tree, just remembered that this one is part of my
> longer series. I have about 150 patches that are already split
> up and just need to be sent out, plus more that are currently
> sitting in a combined changeset.

All? Wanna bet we can introduce new instances of that warning faster
than you can fix all of them? Actually, no I would not take that bet.

I'm just having fun, thanks for all of that cleanup work!
-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
@ 2023-04-19 21:48               ` Nick Desaulniers
  0 siblings, 0 replies; 24+ messages in thread
From: Nick Desaulniers @ 2023-04-19 21:48 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Simon Horman, Stefan Wahren,
	Broadcom internal kernel review list, Nathan Chancellor, Tom Rix,
	Greg Kroah-Hartman, linux-rpi-kernel, linux-arm-kernel,
	linux-staging, llvm, Florian Fainelli

On Wed, Apr 19, 2023 at 2:42 PM Arnd Bergmann <arnd@kernel.org> wrote:
>
> On Wed, Apr 19, 2023, at 23:38, Nick Desaulniers wrote:
> > On Wed, Apr 19, 2023 at 2:35 PM Arnd Bergmann <arnd@kernel.org> wrote:
> >>
> >> On Wed, Apr 19, 2023, at 23:21, Nick Desaulniers wrote:
> >> > On Wed, Apr 19, 2023 at 1:51 AM Simon Horman <horms@kernel.org> wrote:
> >>
> >> > Marking something as `depends on OF_DYNAMIC`. (Not sure whether Arnd
> >> > meant that for BCM2835_VCHIQ or BCM_VIDEOCORE).
> >> > Perhaps try that?
> >> >
> >> >>
> >> >> I'm a bit surprised as I didn't observe such a thing.
> >> >> But I also don't dispute it - clearly my testing was inadequate.
> >> >>
> >> >> I'll try and dig a bit deeper into this one.
> >> >> Perhaps starting with the report from the kernel test robot [3]
> >>
> >> I have this in my randconfig build tree, but it looks like I never
> >> sent it out.
> >
> > What else have you got hidden up in there? ;)
> > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> Actually, I have fixes for all missing-prototype warnings
> in my tree, just remembered that this one is part of my
> longer series. I have about 150 patches that are already split
> up and just need to be sent out, plus more that are currently
> sitting in a combined changeset.

All? Wanna bet we can introduce new instances of that warning faster
than you can fix all of them? Actually, no I would not take that bet.

I'm just having fun, thanks for all of that cleanup work!
-- 
Thanks,
~Nick Desaulniers

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2023-04-19 21:49 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-18 11:23 [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static Simon Horman
2023-04-18 11:23 ` Simon Horman
2023-04-18 18:39 ` Nick Desaulniers
2023-04-18 18:39   ` Nick Desaulniers
2023-04-19  5:39   ` Simon Horman
2023-04-19  5:39     ` Simon Horman
2023-04-18 18:44 ` Nathan Chancellor
2023-04-18 18:44   ` Nathan Chancellor
2023-04-19  5:42   ` Simon Horman
2023-04-19  5:42     ` Simon Horman
2023-04-19  6:37 ` Stefan Wahren
2023-04-19  6:37   ` Stefan Wahren
2023-04-19  8:51   ` Simon Horman
2023-04-19  8:51     ` Simon Horman
2023-04-19 21:21     ` Nick Desaulniers
2023-04-19 21:21       ` Nick Desaulniers
2023-04-19 21:35       ` Arnd Bergmann
2023-04-19 21:35         ` Arnd Bergmann
2023-04-19 21:38         ` Nick Desaulniers
2023-04-19 21:38           ` Nick Desaulniers
2023-04-19 21:41           ` Arnd Bergmann
2023-04-19 21:41             ` Arnd Bergmann
2023-04-19 21:48             ` Nick Desaulniers
2023-04-19 21:48               ` Nick Desaulniers

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.