macintosh/via-pmu: Fix compiler warnings when CONFIG_PROC_FS is disabled

[PATCH] macintosh/via-pmu: Fix compiler warnings when CONFIG_PROC_FS is disabled

Posted by Finn Thain 4 years, 3 months ago

drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show' defined but not used [-Wunused-function]
 static int pmu_battery_proc_show(struct seq_file *m, void *v)
            ^~~~~~~~~~~~~~~~~~~~~
drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show' defined but not used [-Wunused-function]
 static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
            ^~~~~~~~~~~~~~~~~~~~~~
drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined but not used [-Wunused-function]
 static int pmu_info_proc_show(struct seq_file *m, void *v)
            ^~~~~~~~~~~~~~~~~~

Rearrange some code and add some #ifdefs to avoid unused code warnings
when CONFIG_PROC_FS is disabled.

Reported-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
---
 drivers/macintosh/via-pmu.c | 61 ++++++++++++++++++++++---------------
 1 file changed, 36 insertions(+), 25 deletions(-)

diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
index 55afa6dfa263..5ffebf29b630 100644
--- a/drivers/macintosh/via-pmu.c
+++ b/drivers/macintosh/via-pmu.c
@@ -173,10 +173,15 @@ static unsigned long async_req_locks;
 #define NUM_IRQ_STATS 13
 static unsigned int pmu_irq_stats[NUM_IRQ_STATS];
 
+#ifdef CONFIG_PROC_FS
 static struct proc_dir_entry *proc_pmu_root;
 static struct proc_dir_entry *proc_pmu_info;
 static struct proc_dir_entry *proc_pmu_irqstats;
 static struct proc_dir_entry *proc_pmu_options;
+static struct proc_dir_entry *proc_pmu_batt[PMU_MAX_BATTERIES];
+static void pmu_proc_setup(void);
+#endif
+
 static int option_server_mode;
 
 int pmu_battery_count;
@@ -185,7 +190,6 @@ unsigned int pmu_power_flags = PMU_PWR_AC_PRESENT;
 struct pmu_battery_info pmu_batteries[PMU_MAX_BATTERIES];
 static int query_batt_timer = BATTERY_POLLING_COUNT;
 static struct adb_request batt_req;
-static struct proc_dir_entry *proc_pmu_batt[PMU_MAX_BATTERIES];
 
 int asleep;
 
@@ -204,11 +208,7 @@ static int init_pmu(void);
 static void pmu_start(void);
 static irqreturn_t via_pmu_interrupt(int irq, void *arg);
 static irqreturn_t gpio1_interrupt(int irq, void *arg);
-static int pmu_info_proc_show(struct seq_file *m, void *v);
-static int pmu_irqstats_proc_show(struct seq_file *m, void *v);
-static int pmu_battery_proc_show(struct seq_file *m, void *v);
 static void pmu_pass_intr(unsigned char *data, int len);
-static const struct proc_ops pmu_options_proc_ops;
 
 #ifdef CONFIG_ADB
 const struct adb_driver via_pmu_driver = {
@@ -551,26 +551,9 @@ static int __init via_pmu_dev_init(void)
 	}
 #endif /* CONFIG_PPC32 */
 
-	/* Create /proc/pmu */
-	proc_pmu_root = proc_mkdir("pmu", NULL);
-	if (proc_pmu_root) {
-		long i;
-
-		for (i=0; i<pmu_battery_count; i++) {
-			char title[16];
-			sprintf(title, "battery_%ld", i);
-			proc_pmu_batt[i] = proc_create_single_data(title, 0,
-					proc_pmu_root, pmu_battery_proc_show,
-					(void *)i);
-		}
-
-		proc_pmu_info = proc_create_single("info", 0, proc_pmu_root,
-				pmu_info_proc_show);
-		proc_pmu_irqstats = proc_create_single("interrupts", 0,
-				proc_pmu_root, pmu_irqstats_proc_show);
-		proc_pmu_options = proc_create("options", 0600, proc_pmu_root,
-						&pmu_options_proc_ops);
-	}
+#ifdef CONFIG_PROC_FS
+	pmu_proc_setup();
+#endif
 	return 0;
 }
 
@@ -857,6 +840,7 @@ query_battery_state(void)
 			2, PMU_SMART_BATTERY_STATE, pmu_cur_battery+1);
 }
 
+#ifdef CONFIG_PROC_FS
 static int pmu_info_proc_show(struct seq_file *m, void *v)
 {
 	seq_printf(m, "PMU driver version     : %d\n", PMU_DRIVER_VERSION);
@@ -978,6 +962,33 @@ static const struct proc_ops pmu_options_proc_ops = {
 	.proc_write	= pmu_options_proc_write,
 };
 
+static void pmu_proc_setup(void)
+{
+	long i;
+
+	/* Create /proc/pmu */
+	proc_pmu_root = proc_mkdir("pmu", NULL);
+	if (!proc_pmu_root)
+		return;
+
+	for (i = 0; i < pmu_battery_count; i++) {
+		char title[16];
+
+		sprintf(title, "battery_%ld", i);
+		proc_pmu_batt[i] =
+			proc_create_single_data(title, 0, proc_pmu_root,
+						pmu_battery_proc_show, (void *)i);
+	}
+
+	proc_pmu_info = proc_create_single("info", 0, proc_pmu_root,
+					   pmu_info_proc_show);
+	proc_pmu_irqstats = proc_create_single("interrupts", 0, proc_pmu_root,
+					       pmu_irqstats_proc_show);
+	proc_pmu_options = proc_create("options", 0600, proc_pmu_root,
+				       &pmu_options_proc_ops);
+}
+#endif /* CONFIG_PROC_FS */
+
 #ifdef CONFIG_ADB
 /* Send an ADB command */
 static int pmu_send_request(struct adb_request *req, int sync)
-- 
2.32.0

Re: [PATCH] macintosh/via-pmu: Fix compiler warnings when CONFIG_PROC_FS is disabled

Posted by Randy Dunlap 4 years, 3 months ago


On 3/19/22 00:20, Finn Thain wrote:
> drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show' defined but not used [-Wunused-function]
>  static int pmu_battery_proc_show(struct seq_file *m, void *v)
>             ^~~~~~~~~~~~~~~~~~~~~
> drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show' defined but not used [-Wunused-function]
>  static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
>             ^~~~~~~~~~~~~~~~~~~~~~
> drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined but not used [-Wunused-function]
>  static int pmu_info_proc_show(struct seq_file *m, void *v)
>             ^~~~~~~~~~~~~~~~~~
> 
> Rearrange some code and add some #ifdefs to avoid unused code warnings
> when CONFIG_PROC_FS is disabled.
> 
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Finn Thain <fthain@linux-m68k.org>

Acked-by: Randy Dunlap <rdunlap@infradead.org>
Tested-by: Randy Dunlap <rdunlap@infradead.org>

Thanks.

> ---
>  drivers/macintosh/via-pmu.c | 61 ++++++++++++++++++++++---------------
>  1 file changed, 36 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
> index 55afa6dfa263..5ffebf29b630 100644
> --- a/drivers/macintosh/via-pmu.c
> +++ b/drivers/macintosh/via-pmu.c
> @@ -173,10 +173,15 @@ static unsigned long async_req_locks;
>  #define NUM_IRQ_STATS 13
>  static unsigned int pmu_irq_stats[NUM_IRQ_STATS];
>  
> +#ifdef CONFIG_PROC_FS
>  static struct proc_dir_entry *proc_pmu_root;
>  static struct proc_dir_entry *proc_pmu_info;
>  static struct proc_dir_entry *proc_pmu_irqstats;
>  static struct proc_dir_entry *proc_pmu_options;
> +static struct proc_dir_entry *proc_pmu_batt[PMU_MAX_BATTERIES];
> +static void pmu_proc_setup(void);
> +#endif
> +
>  static int option_server_mode;
>  
>  int pmu_battery_count;
> @@ -185,7 +190,6 @@ unsigned int pmu_power_flags = PMU_PWR_AC_PRESENT;
>  struct pmu_battery_info pmu_batteries[PMU_MAX_BATTERIES];
>  static int query_batt_timer = BATTERY_POLLING_COUNT;
>  static struct adb_request batt_req;
> -static struct proc_dir_entry *proc_pmu_batt[PMU_MAX_BATTERIES];
>  
>  int asleep;
>  
> @@ -204,11 +208,7 @@ static int init_pmu(void);
>  static void pmu_start(void);
>  static irqreturn_t via_pmu_interrupt(int irq, void *arg);
>  static irqreturn_t gpio1_interrupt(int irq, void *arg);
> -static int pmu_info_proc_show(struct seq_file *m, void *v);
> -static int pmu_irqstats_proc_show(struct seq_file *m, void *v);
> -static int pmu_battery_proc_show(struct seq_file *m, void *v);
>  static void pmu_pass_intr(unsigned char *data, int len);
> -static const struct proc_ops pmu_options_proc_ops;
>  
>  #ifdef CONFIG_ADB
>  const struct adb_driver via_pmu_driver = {
> @@ -551,26 +551,9 @@ static int __init via_pmu_dev_init(void)
>  	}
>  #endif /* CONFIG_PPC32 */
>  
> -	/* Create /proc/pmu */
> -	proc_pmu_root = proc_mkdir("pmu", NULL);
> -	if (proc_pmu_root) {
> -		long i;
> -
> -		for (i=0; i<pmu_battery_count; i++) {
> -			char title[16];
> -			sprintf(title, "battery_%ld", i);
> -			proc_pmu_batt[i] = proc_create_single_data(title, 0,
> -					proc_pmu_root, pmu_battery_proc_show,
> -					(void *)i);
> -		}
> -
> -		proc_pmu_info = proc_create_single("info", 0, proc_pmu_root,
> -				pmu_info_proc_show);
> -		proc_pmu_irqstats = proc_create_single("interrupts", 0,
> -				proc_pmu_root, pmu_irqstats_proc_show);
> -		proc_pmu_options = proc_create("options", 0600, proc_pmu_root,
> -						&pmu_options_proc_ops);
> -	}
> +#ifdef CONFIG_PROC_FS
> +	pmu_proc_setup();
> +#endif
>  	return 0;
>  }
>  
> @@ -857,6 +840,7 @@ query_battery_state(void)
>  			2, PMU_SMART_BATTERY_STATE, pmu_cur_battery+1);
>  }
>  
> +#ifdef CONFIG_PROC_FS
>  static int pmu_info_proc_show(struct seq_file *m, void *v)
>  {
>  	seq_printf(m, "PMU driver version     : %d\n", PMU_DRIVER_VERSION);
> @@ -978,6 +962,33 @@ static const struct proc_ops pmu_options_proc_ops = {
>  	.proc_write	= pmu_options_proc_write,
>  };
>  
> +static void pmu_proc_setup(void)
> +{
> +	long i;
> +
> +	/* Create /proc/pmu */
> +	proc_pmu_root = proc_mkdir("pmu", NULL);
> +	if (!proc_pmu_root)
> +		return;
> +
> +	for (i = 0; i < pmu_battery_count; i++) {
> +		char title[16];
> +
> +		sprintf(title, "battery_%ld", i);
> +		proc_pmu_batt[i] =
> +			proc_create_single_data(title, 0, proc_pmu_root,
> +						pmu_battery_proc_show, (void *)i);
> +	}
> +
> +	proc_pmu_info = proc_create_single("info", 0, proc_pmu_root,
> +					   pmu_info_proc_show);
> +	proc_pmu_irqstats = proc_create_single("interrupts", 0, proc_pmu_root,
> +					       pmu_irqstats_proc_show);
> +	proc_pmu_options = proc_create("options", 0600, proc_pmu_root,
> +				       &pmu_options_proc_ops);
> +}
> +#endif /* CONFIG_PROC_FS */
> +
>  #ifdef CONFIG_ADB
>  /* Send an ADB command */
>  static int pmu_send_request(struct adb_request *req, int sync)

-- 
~Randy

Re: [PATCH] macintosh/via-pmu: Fix compiler warnings when CONFIG_PROC_FS is disabled

Posted by Christophe Leroy 4 years, 3 months ago


Le 19/03/2022 à 08:20, Finn Thain a écrit :
> drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show' defined but not used [-Wunused-function]
>   static int pmu_battery_proc_show(struct seq_file *m, void *v)
>              ^~~~~~~~~~~~~~~~~~~~~
> drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show' defined but not used [-Wunused-function]
>   static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
>              ^~~~~~~~~~~~~~~~~~~~~~
> drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined but not used [-Wunused-function]
>   static int pmu_info_proc_show(struct seq_file *m, void *v)
>              ^~~~~~~~~~~~~~~~~~
> 
> Rearrange some code and add some #ifdefs to avoid unused code warnings
> when CONFIG_PROC_FS is disabled.

Why not just put those three functions inside an #ifdef CONFIG_PROC_FS ?

Christophe


> 
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
> ---
>   drivers/macintosh/via-pmu.c | 61 ++++++++++++++++++++++---------------
>   1 file changed, 36 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
> index 55afa6dfa263..5ffebf29b630 100644
> --- a/drivers/macintosh/via-pmu.c
> +++ b/drivers/macintosh/via-pmu.c
> @@ -173,10 +173,15 @@ static unsigned long async_req_locks;
>   #define NUM_IRQ_STATS 13
>   static unsigned int pmu_irq_stats[NUM_IRQ_STATS];
>   
> +#ifdef CONFIG_PROC_FS
>   static struct proc_dir_entry *proc_pmu_root;
>   static struct proc_dir_entry *proc_pmu_info;
>   static struct proc_dir_entry *proc_pmu_irqstats;
>   static struct proc_dir_entry *proc_pmu_options;
> +static struct proc_dir_entry *proc_pmu_batt[PMU_MAX_BATTERIES];
> +static void pmu_proc_setup(void);
> +#endif
> +
>   static int option_server_mode;
>   
>   int pmu_battery_count;
> @@ -185,7 +190,6 @@ unsigned int pmu_power_flags = PMU_PWR_AC_PRESENT;
>   struct pmu_battery_info pmu_batteries[PMU_MAX_BATTERIES];
>   static int query_batt_timer = BATTERY_POLLING_COUNT;
>   static struct adb_request batt_req;
> -static struct proc_dir_entry *proc_pmu_batt[PMU_MAX_BATTERIES];
>   
>   int asleep;
>   
> @@ -204,11 +208,7 @@ static int init_pmu(void);
>   static void pmu_start(void);
>   static irqreturn_t via_pmu_interrupt(int irq, void *arg);
>   static irqreturn_t gpio1_interrupt(int irq, void *arg);
> -static int pmu_info_proc_show(struct seq_file *m, void *v);
> -static int pmu_irqstats_proc_show(struct seq_file *m, void *v);
> -static int pmu_battery_proc_show(struct seq_file *m, void *v);
>   static void pmu_pass_intr(unsigned char *data, int len);
> -static const struct proc_ops pmu_options_proc_ops;
>   
>   #ifdef CONFIG_ADB
>   const struct adb_driver via_pmu_driver = {
> @@ -551,26 +551,9 @@ static int __init via_pmu_dev_init(void)
>   	}
>   #endif /* CONFIG_PPC32 */
>   
> -	/* Create /proc/pmu */
> -	proc_pmu_root = proc_mkdir("pmu", NULL);
> -	if (proc_pmu_root) {
> -		long i;
> -
> -		for (i=0; i<pmu_battery_count; i++) {
> -			char title[16];
> -			sprintf(title, "battery_%ld", i);
> -			proc_pmu_batt[i] = proc_create_single_data(title, 0,
> -					proc_pmu_root, pmu_battery_proc_show,
> -					(void *)i);
> -		}
> -
> -		proc_pmu_info = proc_create_single("info", 0, proc_pmu_root,
> -				pmu_info_proc_show);
> -		proc_pmu_irqstats = proc_create_single("interrupts", 0,
> -				proc_pmu_root, pmu_irqstats_proc_show);
> -		proc_pmu_options = proc_create("options", 0600, proc_pmu_root,
> -						&pmu_options_proc_ops);
> -	}
> +#ifdef CONFIG_PROC_FS
> +	pmu_proc_setup();
> +#endif
>   	return 0;
>   }
>   
> @@ -857,6 +840,7 @@ query_battery_state(void)
>   			2, PMU_SMART_BATTERY_STATE, pmu_cur_battery+1);
>   }
>   
> +#ifdef CONFIG_PROC_FS
>   static int pmu_info_proc_show(struct seq_file *m, void *v)
>   {
>   	seq_printf(m, "PMU driver version     : %d\n", PMU_DRIVER_VERSION);
> @@ -978,6 +962,33 @@ static const struct proc_ops pmu_options_proc_ops = {
>   	.proc_write	= pmu_options_proc_write,
>   };
>   
> +static void pmu_proc_setup(void)
> +{
> +	long i;
> +
> +	/* Create /proc/pmu */
> +	proc_pmu_root = proc_mkdir("pmu", NULL);
> +	if (!proc_pmu_root)
> +		return;
> +
> +	for (i = 0; i < pmu_battery_count; i++) {
> +		char title[16];
> +
> +		sprintf(title, "battery_%ld", i);
> +		proc_pmu_batt[i] =
> +			proc_create_single_data(title, 0, proc_pmu_root,
> +						pmu_battery_proc_show, (void *)i);
> +	}
> +
> +	proc_pmu_info = proc_create_single("info", 0, proc_pmu_root,
> +					   pmu_info_proc_show);
> +	proc_pmu_irqstats = proc_create_single("interrupts", 0, proc_pmu_root,
> +					       pmu_irqstats_proc_show);
> +	proc_pmu_options = proc_create("options", 0600, proc_pmu_root,
> +				       &pmu_options_proc_ops);
> +}
> +#endif /* CONFIG_PROC_FS */
> +
>   #ifdef CONFIG_ADB
>   /* Send an ADB command */
>   static int pmu_send_request(struct adb_request *req, int sync)

Re: [PATCH] macintosh/via-pmu: Fix compiler warnings when CONFIG_PROC_FS is disabled

Posted by Finn Thain 4 years, 3 months ago

On Mon, 21 Mar 2022, Christophe Leroy wrote:

> Le 19/03/2022 à 08:20, Finn Thain a écrit :
> > drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show' defined but not used [-Wunused-function]
> >   static int pmu_battery_proc_show(struct seq_file *m, void *v)
> >              ^~~~~~~~~~~~~~~~~~~~~
> > drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show' defined but not used [-Wunused-function]
> >   static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
> >              ^~~~~~~~~~~~~~~~~~~~~~
> > drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined but not used [-Wunused-function]
> >   static int pmu_info_proc_show(struct seq_file *m, void *v)
> >              ^~~~~~~~~~~~~~~~~~
> > 
> > Rearrange some code and add some #ifdefs to avoid unused code warnings
> > when CONFIG_PROC_FS is disabled.
> 
> Why not just put those three functions inside an #ifdef CONFIG_PROC_FS ?
> 

You'd get a warning about the prototypes ("declared static but never 
defined"). Rather than add an ifdef around the prototypes as well, I just 
reordered things a little.

Re: [PATCH] macintosh/via-pmu: Fix compiler warnings when CONFIG_PROC_FS is disabled

Posted by Finn Thain 4 years, 3 months ago

Hi Christophe,

On Mon, 21 Mar 2022, Finn Thain wrote:

> On Mon, 21 Mar 2022, Christophe Leroy wrote:
> 
> > Le 19/03/2022 à 08:20, Finn Thain a écrit :
> > > drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show' defined but not used [-Wunused-function]
> > >   static int pmu_battery_proc_show(struct seq_file *m, void *v)
> > >              ^~~~~~~~~~~~~~~~~~~~~
> > > drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show' defined but not used [-Wunused-function]
> > >   static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
> > >              ^~~~~~~~~~~~~~~~~~~~~~
> > > drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined but not used [-Wunused-function]
> > >   static int pmu_info_proc_show(struct seq_file *m, void *v)
> > >              ^~~~~~~~~~~~~~~~~~
> > > 
> > > Rearrange some code and add some #ifdefs to avoid unused code warnings
> > > when CONFIG_PROC_FS is disabled.
> > 
> > Why not just put those three functions inside an #ifdef CONFIG_PROC_FS ?
> > 
> 
> You'd get a warning about the prototypes ("declared static but never 
> defined"). Rather than add an ifdef around the prototypes as well, I 
> just reordered things a little.

Oops, I was forgetting that I also added an ifdef around the new 
prototype.

The simplest solution is probably the patch below, as it better exploits 
the stubbed-out proc_* API in include/linux/proc_fs.h.

Was this what you had in mind?

diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
index 2109129ea1bb..495fd35b11de 100644
--- a/drivers/macintosh/via-pmu.c
+++ b/drivers/macintosh/via-pmu.c
@@ -204,9 +204,11 @@ static int init_pmu(void);
 static void pmu_start(void);
 static irqreturn_t via_pmu_interrupt(int irq, void *arg);
 static irqreturn_t gpio1_interrupt(int irq, void *arg);
+#ifdef CONFIG_PROC_FS
 static int pmu_info_proc_show(struct seq_file *m, void *v);
 static int pmu_irqstats_proc_show(struct seq_file *m, void *v);
 static int pmu_battery_proc_show(struct seq_file *m, void *v);
+#endif
 static void pmu_pass_intr(unsigned char *data, int len);
 static const struct proc_ops pmu_options_proc_ops;
 
@@ -857,6 +859,7 @@ query_battery_state(void)
 			2, PMU_SMART_BATTERY_STATE, pmu_cur_battery+1);
 }
 
+#ifdef CONFIG_PROC_FS
 static int pmu_info_proc_show(struct seq_file *m, void *v)
 {
 	seq_printf(m, "PMU driver version     : %d\n", PMU_DRIVER_VERSION);
@@ -977,6 +980,7 @@ static const struct proc_ops pmu_options_proc_ops = {
 	.proc_release	= single_release,
 	.proc_write	= pmu_options_proc_write,
 };
+#endif
 
 #ifdef CONFIG_ADB
 /* Send an ADB command */

Re: [PATCH] macintosh/via-pmu: Fix compiler warnings when CONFIG_PROC_FS is disabled

Posted by Christophe Leroy 4 years, 3 months ago


Le 21/03/2022 à 09:50, Finn Thain a écrit :
> Hi Christophe,
> 
> On Mon, 21 Mar 2022, Finn Thain wrote:
> 
>> On Mon, 21 Mar 2022, Christophe Leroy wrote:
>>
>>> Le 19/03/2022 à 08:20, Finn Thain a écrit :
>>>> drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show' defined but not used [-Wunused-function]
>>>>    static int pmu_battery_proc_show(struct seq_file *m, void *v)
>>>>               ^~~~~~~~~~~~~~~~~~~~~
>>>> drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show' defined but not used [-Wunused-function]
>>>>    static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
>>>>               ^~~~~~~~~~~~~~~~~~~~~~
>>>> drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined but not used [-Wunused-function]
>>>>    static int pmu_info_proc_show(struct seq_file *m, void *v)
>>>>               ^~~~~~~~~~~~~~~~~~
>>>>
>>>> Rearrange some code and add some #ifdefs to avoid unused code warnings
>>>> when CONFIG_PROC_FS is disabled.
>>>
>>> Why not just put those three functions inside an #ifdef CONFIG_PROC_FS ?
>>>
>>
>> You'd get a warning about the prototypes ("declared static but never
>> defined"). Rather than add an ifdef around the prototypes as well, I
>> just reordered things a little.
> 
> Oops, I was forgetting that I also added an ifdef around the new
> prototype.
> 
> The simplest solution is probably the patch below, as it better exploits
> the stubbed-out proc_* API in include/linux/proc_fs.h.
> 
> Was this what you had in mind?

Yes that's exactly what I had in mind.

Thanks
Christophe

Re: [PATCH] macintosh/via-pmu: Fix compiler warnings when CONFIG_PROC_FS is disabled

Posted by Finn Thain 4 years, 3 months ago

On Mon, 21 Mar 2022, Christophe Leroy wrote:

> Le 21/03/2022 à 09:50, Finn Thain a écrit :
> > Hi Christophe,
> > 
> > On Mon, 21 Mar 2022, Finn Thain wrote:
> > 
> >> On Mon, 21 Mar 2022, Christophe Leroy wrote:
> >>
> >>> Le 19/03/2022 à 08:20, Finn Thain a écrit :
> >>>> drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show' defined but not used [-Wunused-function]
> >>>>    static int pmu_battery_proc_show(struct seq_file *m, void *v)
> >>>>               ^~~~~~~~~~~~~~~~~~~~~
> >>>> drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show' defined but not used [-Wunused-function]
> >>>>    static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
> >>>>               ^~~~~~~~~~~~~~~~~~~~~~
> >>>> drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined but not used [-Wunused-function]
> >>>>    static int pmu_info_proc_show(struct seq_file *m, void *v)
> >>>>               ^~~~~~~~~~~~~~~~~~
> >>>>
> >>>> Rearrange some code and add some #ifdefs to avoid unused code warnings
> >>>> when CONFIG_PROC_FS is disabled.
> >>>
> >>> Why not just put those three functions inside an #ifdef CONFIG_PROC_FS ?
> >>>
> >>
> >> You'd get a warning about the prototypes ("declared static but never
> >> defined"). Rather than add an ifdef around the prototypes as well, I
> >> just reordered things a little.
> > 
> > Oops, I was forgetting that I also added an ifdef around the new
> > prototype.
> > 
> > The simplest solution is probably the patch below, as it better exploits
> > the stubbed-out proc_* API in include/linux/proc_fs.h.
> > 
> > Was this what you had in mind?
> 
> Yes that's exactly what I had in mind.
> 

OK, I'll submit it formally.

Thanks for your review.

Re: [PATCH] macintosh/via-pmu: Fix compiler warnings when CONFIG_PROC_FS is disabled

Posted by Christophe Leroy 4 years, 3 months ago


Le 21/03/2022 à 09:33, Finn Thain a écrit :
> On Mon, 21 Mar 2022, Christophe Leroy wrote:
> 
>> Le 19/03/2022 à 08:20, Finn Thain a écrit :
>>> drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show' defined but not used [-Wunused-function]
>>>    static int pmu_battery_proc_show(struct seq_file *m, void *v)
>>>               ^~~~~~~~~~~~~~~~~~~~~
>>> drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show' defined but not used [-Wunused-function]
>>>    static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
>>>               ^~~~~~~~~~~~~~~~~~~~~~
>>> drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined but not used [-Wunused-function]
>>>    static int pmu_info_proc_show(struct seq_file *m, void *v)
>>>               ^~~~~~~~~~~~~~~~~~
>>>
>>> Rearrange some code and add some #ifdefs to avoid unused code warnings
>>> when CONFIG_PROC_FS is disabled.
>>
>> Why not just put those three functions inside an #ifdef CONFIG_PROC_FS ?
>>
> 
> You'd get a warning about the prototypes ("declared static but never
> defined"). Rather than add an ifdef around the prototypes as well, I just
> reordered things a little.

Then now you have callers of proc_create_single_data() inside the ifdefs 
and then you have an additional #ifdef in the middle of 
via_pmu_dev_init() and also have to ifdef out all proc_pmu_xxxx.

I thing ifdefing out the prototypes would be less churn.

Christophe