Re: [PATCH 15/39] acpi/battery: simplify procfs code

[Rafael J. Wysocki]

On Thu, Apr 19, 2018 at 2:41 PM, Christoph Hellwig  wrote:
> Use remove_proc_subtree to remove the whole subtree on cleanup, and
> unwind the registration loop into individual calls.  Switch to use
> proc_create_seq where applicable.
>
> Signed-off-by: Christoph Hellwig 

It is OK AFAICS.

Reviewed-by: Rafael J. Wysocki 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 15/39] acpi/battery: simplify procfs code

[Christoph Hellwig]

Use remove_proc_subtree to remove the whole subtree on cleanup, and
unwind the registration loop into individual calls.  Switch to use
proc_create_seq where applicable.

Signed-off-by: Christoph Hellwig 
---
 drivers/acpi/battery.c | 121 +
 1 file changed, 26 insertions(+), 95 deletions(-)

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index bdb24d636d9a..76550689ce10 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -81,14 +81,6 @@ MODULE_PARM_DESC(cache_time, "cache time in milliseconds");
 #ifdef CONFIG_ACPI_PROCFS_POWER
 extern struct proc_dir_entry *acpi_lock_battery_dir(void);
 extern void *acpi_unlock_battery_dir(struct proc_dir_entry *acpi_battery_dir);
-
-enum acpi_battery_files {
-   info_tag = 0,
-   state_tag,
-   alarm_tag,
-   ACPI_BATTERY_NUMFILES,
-};
-
 #endif
 
 static const struct acpi_device_id battery_device_ids[] = {
@@ -985,9 +977,10 @@ static const char *acpi_battery_units(const struct 
acpi_battery *battery)
"mA" : "mW";
 }
 
-static int acpi_battery_print_info(struct seq_file *seq, int result)
+static int acpi_battery_info_proc_show(struct seq_file *seq, void *offset)
 {
struct acpi_battery *battery = seq->private;
+   int result = acpi_battery_update(battery, false);
 
if (result)
goto end;
@@ -1041,9 +1034,10 @@ static int acpi_battery_print_info(struct seq_file *seq, 
int result)
return result;
 }
 
-static int acpi_battery_print_state(struct seq_file *seq, int result)
+static int acpi_battery_state_proc_show(struct seq_file *seq, void *offset)
 {
struct acpi_battery *battery = seq->private;
+   int result = acpi_battery_update(battery, false);
 
if (result)
goto end;
@@ -1088,9 +1082,10 @@ static int acpi_battery_print_state(struct seq_file 
*seq, int result)
return result;
 }
 
-static int acpi_battery_print_alarm(struct seq_file *seq, int result)
+static int acpi_battery_alarm_proc_show(struct seq_file *seq, void *offset)
 {
struct acpi_battery *battery = seq->private;
+   int result = acpi_battery_update(battery, false);
 
if (result)
goto end;
@@ -1142,82 +1137,22 @@ static ssize_t acpi_battery_write_alarm(struct file 
*file,
return result;
 }
 
-typedef int(*print_func)(struct seq_file *seq, int result);
-
-static print_func acpi_print_funcs[ACPI_BATTERY_NUMFILES] = {
-   acpi_battery_print_info,
-   acpi_battery_print_state,
-   acpi_battery_print_alarm,
-};
-
-static int acpi_battery_read(int fid, struct seq_file *seq)
+static int acpi_battery_alarm_proc_open(struct inode *inode, struct file *file)
 {
-   struct acpi_battery *battery = seq->private;
-   int result = acpi_battery_update(battery, false);
-   return acpi_print_funcs[fid](seq, result);
+   return single_open(file, acpi_battery_alarm_proc_show, PDE_DATA(inode));
 }
 
-#define DECLARE_FILE_FUNCTIONS(_name) \
-static int acpi_battery_read_##_name(struct seq_file *seq, void *offset) \
-{ \
-   return acpi_battery_read(_name##_tag, seq); \
-} \
-static int acpi_battery_##_name##_open_fs(struct inode *inode, struct file 
*file) \
-{ \
-   return single_open(file, acpi_battery_read_##_name, PDE_DATA(inode)); \
-}
-
-DECLARE_FILE_FUNCTIONS(info);
-DECLARE_FILE_FUNCTIONS(state);
-DECLARE_FILE_FUNCTIONS(alarm);
-
-#undef DECLARE_FILE_FUNCTIONS
-
-#define FILE_DESCRIPTION_RO(_name) \
-   { \
-   .name = __stringify(_name), \
-   .mode = S_IRUGO, \
-   .ops = { \
-   .open = acpi_battery_##_name##_open_fs, \
-   .read = seq_read, \
-   .llseek = seq_lseek, \
-   .release = single_release, \
-   .owner = THIS_MODULE, \
-   }, \
-   }
-
-#define FILE_DESCRIPTION_RW(_name) \
-   { \
-   .name = __stringify(_name), \
-   .mode = S_IFREG | S_IRUGO | S_IWUSR, \
-   .ops = { \
-   .open = acpi_battery_##_name##_open_fs, \
-   .read = seq_read, \
-   .llseek = seq_lseek, \
-   .write = acpi_battery_write_##_name, \
-   .release = single_release, \
-   .owner = THIS_MODULE, \
-   }, \
-   }
-
-static const struct battery_file {
-   struct file_operations ops;
-   umode_t mode;
-   const char *name;
-} acpi_battery_file[] = {
-   FILE_DESCRIPTION_RO(info),
-   FILE_DESCRIPTION_RO(state),
-   FILE_DESCRIPTION_RW(alarm),
+static const struct file_operations acpi_battery_alarm_fops = {
+   .owner  = THIS_MODULE,
+   .open   = acpi_battery_alarm_proc_open,
+   .read   = seq_read,
+   .write  = acpi_battery_write_alarm,
+   .llseek = seq_lseek,
+   .release= single_release,
 };
 
-#undef FILE_DESCRIPTION_RO
-#undef FILE_DESCRIPTION_RW
-
 static int