Re: [PATCH 1/2] mfd: ab8500-debugfs: add function declaration

[Lee Jones]

On Sat, 24 Sep 2016, Baoyou Xie wrote:

> We get 1 warning when building kernel with W=1:
> drivers/mfd/ab8500-debugfs.c:1587:28: warning: no previous prototype for 
> 'suspend_test_wake_cause_interrupt_is_mine' [-Wmissing-prototypes]
> 
> In fact, this function need be declared in a header files.
> 
> So this patch adds function declaration in
> include/linux/mfd/abx500/ab8500.h.
> 
> Signed-off-by: Baoyou Xie 
> ---
>  include/linux/mfd/abx500/ab8500.h | 1 +
>  1 file changed, 1 insertion(+)

I already fixed this: https://patchwork.kernel.org/patch/9333097/

> diff --git a/include/linux/mfd/abx500/ab8500.h 
> b/include/linux/mfd/abx500/ab8500.h
> index 9475fee..a496120 100644
> --- a/include/linux/mfd/abx500/ab8500.h
> +++ b/include/linux/mfd/abx500/ab8500.h
> @@ -508,6 +508,7 @@ void ab8500_override_turn_on_stat(u8 mask, u8 set);
>  extern int prcmu_abb_read(u8 slave, u8 reg, u8 *value, u8 size);
>  void ab8500_dump_all_banks(struct device *dev);
>  void ab8500_debug_register_interrupt(int line);
> +bool suspend_test_wake_cause_interrupt_is_mine(u32 my_int);
>  #else
>  static inline void ab8500_dump_all_banks(struct device *dev) {}
>  static inline void ab8500_debug_register_interrupt(int line) {}

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 1/2] mfd: ab8500-debugfs: add function declaration

[Lee Jones]

On Sat, 24 Sep 2016, Baoyou Xie wrote:

> We get 1 warning when building kernel with W=1:
> drivers/mfd/ab8500-debugfs.c:1587:28: warning: no previous prototype for 
> 'suspend_test_wake_cause_interrupt_is_mine' [-Wmissing-prototypes]
> 
> In fact, this function need be declared in a header files.
> 
> So this patch adds function declaration in
> include/linux/mfd/abx500/ab8500.h.
> 
> Signed-off-by: Baoyou Xie 
> ---
>  include/linux/mfd/abx500/ab8500.h | 1 +
>  1 file changed, 1 insertion(+)

I already fixed this: https://patchwork.kernel.org/patch/9333097/

> diff --git a/include/linux/mfd/abx500/ab8500.h 
> b/include/linux/mfd/abx500/ab8500.h
> index 9475fee..a496120 100644
> --- a/include/linux/mfd/abx500/ab8500.h
> +++ b/include/linux/mfd/abx500/ab8500.h
> @@ -508,6 +508,7 @@ void ab8500_override_turn_on_stat(u8 mask, u8 set);
>  extern int prcmu_abb_read(u8 slave, u8 reg, u8 *value, u8 size);
>  void ab8500_dump_all_banks(struct device *dev);
>  void ab8500_debug_register_interrupt(int line);
> +bool suspend_test_wake_cause_interrupt_is_mine(u32 my_int);
>  #else
>  static inline void ab8500_dump_all_banks(struct device *dev) {}
>  static inline void ab8500_debug_register_interrupt(int line) {}

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 1/2] mfd: ab8500-debugfs: add function declaration

[Arnd Bergmann]

On Sunday 25 September 2016, Baoyou Xie wrote:
> On 24 September 2016 at 16:30, Arnd Bergmann  wrote:
> >
> > This can't be right for two reasons:
> >
> > - the suspend_test_wake_cause_interrupt_is_mine function again
> >   is only used in the file it is defined in
> >
> 
> but, it is declared as weak, so other modules can implement it.

In theory that is correct, but doing that would be very unusual
and bad style. As nobody does this at the moment, and the driver is for
obsolete hardware, just remove it now, and if we ever need want to
override it, that should be done using a runtime callback handler
or something completely different.

Arnd

Re: [PATCH 1/2] mfd: ab8500-debugfs: add function declaration

[Arnd Bergmann]

On Sunday 25 September 2016, Baoyou Xie wrote:
> On 24 September 2016 at 16:30, Arnd Bergmann  wrote:
> >
> > This can't be right for two reasons:
> >
> > - the suspend_test_wake_cause_interrupt_is_mine function again
> >   is only used in the file it is defined in
> >
> 
> but, it is declared as weak, so other modules can implement it.

In theory that is correct, but doing that would be very unusual
and bad style. As nobody does this at the moment, and the driver is for
obsolete hardware, just remove it now, and if we ever need want to
override it, that should be done using a runtime callback handler
or something completely different.

Arnd

Re: [PATCH 1/2] mfd: ab8500-debugfs: add function declaration

[Arnd Bergmann]

On Saturday, September 24, 2016 1:34:50 PM CEST Baoyou Xie wrote:
> We get 1 warning when building kernel with W=1:
> drivers/mfd/ab8500-debugfs.c:1587:28: warning: no previous prototype for 
> 'suspend_test_wake_cause_interrupt_is_mine' [-Wmissing-prototypes]
> 
> In fact, this function need be declared in a header files.
> 
> So this patch adds function declaration in
> include/linux/mfd/abx500/ab8500.h.
> 
> Signed-off-by: Baoyou Xie 
> ---
>  include/linux/mfd/abx500/ab8500.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/mfd/abx500/ab8500.h 
> b/include/linux/mfd/abx500/ab8500.h
> index 9475fee..a496120 100644
> --- a/include/linux/mfd/abx500/ab8500.h
> +++ b/include/linux/mfd/abx500/ab8500.h
> @@ -508,6 +508,7 @@ void ab8500_override_turn_on_stat(u8 mask, u8 set);
>  extern int prcmu_abb_read(u8 slave, u8 reg, u8 *value, u8 size);
>  void ab8500_dump_all_banks(struct device *dev);
>  void ab8500_debug_register_interrupt(int line);
> +bool suspend_test_wake_cause_interrupt_is_mine(u32 my_int);
>  #else
>  static inline void ab8500_dump_all_banks(struct device *dev) {}
>  static inline void ab8500_debug_register_interrupt(int line) {}
> 

This can't be right for two reasons:

- the suspend_test_wake_cause_interrupt_is_mine function again
  is only used in the file it is defined in

- the name of the function does not have a prefix with the name of
  the driver

I think the right fix is to make the function 'static inline'
and add a comment about the intent.

Arnd

Re: [PATCH 1/2] mfd: ab8500-debugfs: add function declaration

[Arnd Bergmann]

On Saturday, September 24, 2016 1:34:50 PM CEST Baoyou Xie wrote:
> We get 1 warning when building kernel with W=1:
> drivers/mfd/ab8500-debugfs.c:1587:28: warning: no previous prototype for 
> 'suspend_test_wake_cause_interrupt_is_mine' [-Wmissing-prototypes]
> 
> In fact, this function need be declared in a header files.
> 
> So this patch adds function declaration in
> include/linux/mfd/abx500/ab8500.h.
> 
> Signed-off-by: Baoyou Xie 
> ---
>  include/linux/mfd/abx500/ab8500.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/mfd/abx500/ab8500.h 
> b/include/linux/mfd/abx500/ab8500.h
> index 9475fee..a496120 100644
> --- a/include/linux/mfd/abx500/ab8500.h
> +++ b/include/linux/mfd/abx500/ab8500.h
> @@ -508,6 +508,7 @@ void ab8500_override_turn_on_stat(u8 mask, u8 set);
>  extern int prcmu_abb_read(u8 slave, u8 reg, u8 *value, u8 size);
>  void ab8500_dump_all_banks(struct device *dev);
>  void ab8500_debug_register_interrupt(int line);
> +bool suspend_test_wake_cause_interrupt_is_mine(u32 my_int);
>  #else
>  static inline void ab8500_dump_all_banks(struct device *dev) {}
>  static inline void ab8500_debug_register_interrupt(int line) {}
> 

This can't be right for two reasons:

- the suspend_test_wake_cause_interrupt_is_mine function again
  is only used in the file it is defined in

- the name of the function does not have a prefix with the name of
  the driver

I think the right fix is to make the function 'static inline'
and add a comment about the intent.

Arnd

[PATCH 1/2] mfd: ab8500-debugfs: add function declaration

[Baoyou Xie]

We get 1 warning when building kernel with W=1:
drivers/mfd/ab8500-debugfs.c:1587:28: warning: no previous prototype for 
'suspend_test_wake_cause_interrupt_is_mine' [-Wmissing-prototypes]

In fact, this function need be declared in a header files.

So this patch adds function declaration in
include/linux/mfd/abx500/ab8500.h.

Signed-off-by: Baoyou Xie 
---
 include/linux/mfd/abx500/ab8500.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/mfd/abx500/ab8500.h 
b/include/linux/mfd/abx500/ab8500.h
index 9475fee..a496120 100644
--- a/include/linux/mfd/abx500/ab8500.h
+++ b/include/linux/mfd/abx500/ab8500.h
@@ -508,6 +508,7 @@ void ab8500_override_turn_on_stat(u8 mask, u8 set);
 extern int prcmu_abb_read(u8 slave, u8 reg, u8 *value, u8 size);
 void ab8500_dump_all_banks(struct device *dev);
 void ab8500_debug_register_interrupt(int line);
+bool suspend_test_wake_cause_interrupt_is_mine(u32 my_int);
 #else
 static inline void ab8500_dump_all_banks(struct device *dev) {}
 static inline void ab8500_debug_register_interrupt(int line) {}
-- 
2.7.4

[PATCH 1/2] mfd: ab8500-debugfs: add function declaration

[Baoyou Xie]

We get 1 warning when building kernel with W=1:
drivers/mfd/ab8500-debugfs.c:1587:28: warning: no previous prototype for 
'suspend_test_wake_cause_interrupt_is_mine' [-Wmissing-prototypes]

In fact, this function need be declared in a header files.

So this patch adds function declaration in
include/linux/mfd/abx500/ab8500.h.

Signed-off-by: Baoyou Xie 
---
 include/linux/mfd/abx500/ab8500.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/mfd/abx500/ab8500.h 
b/include/linux/mfd/abx500/ab8500.h
index 9475fee..a496120 100644
--- a/include/linux/mfd/abx500/ab8500.h
+++ b/include/linux/mfd/abx500/ab8500.h
@@ -508,6 +508,7 @@ void ab8500_override_turn_on_stat(u8 mask, u8 set);
 extern int prcmu_abb_read(u8 slave, u8 reg, u8 *value, u8 size);
 void ab8500_dump_all_banks(struct device *dev);
 void ab8500_debug_register_interrupt(int line);
+bool suspend_test_wake_cause_interrupt_is_mine(u32 my_int);
 #else
 static inline void ab8500_dump_all_banks(struct device *dev) {}
 static inline void ab8500_debug_register_interrupt(int line) {}
-- 
2.7.4