Message ID | 20230225080458.1342359-4-W_Armin@gmx.de (mailing list archive) |
---|---|
State | Superseded, archived |
Headers | show |
Series | ACPI: SBS: Fix various issues | expand |
Hi Armin, I love your patch! Perhaps something to improve: [auto build test WARNING on rafael-pm/linux-next] [also build test WARNING on linus/master v6.2 next-20230225] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Armin-Wolf/ACPI-EC-Add-query-notifier-support/20230225-160641 base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next patch link: https://lore.kernel.org/r/20230225080458.1342359-4-W_Armin%40gmx.de patch subject: [PATCH 3/4] ACPI: EC: Make query handlers private config: x86_64-randconfig-a013 (https://download.01.org/0day-ci/archive/20230225/202302251812.I1FHOAnh-lkp@intel.com/config) compiler: gcc-11 (Debian 11.3.0-8) 11.3.0 reproduce (this is a W=1 build): # https://github.com/intel-lab-lkp/linux/commit/a62cb9e29bf040af617070fa775758720d2de12e git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Armin-Wolf/ACPI-EC-Add-query-notifier-support/20230225-160641 git checkout a62cb9e29bf040af617070fa775758720d2de12e # save the config file mkdir build_dir && cp config build_dir/.config make W=1 O=build_dir ARCH=x86_64 olddefconfig make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/acpi/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> | Link: https://lore.kernel.org/oe-kbuild-all/202302251812.I1FHOAnh-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/acpi/ec.c:1083:5: warning: no previous prototype for 'acpi_ec_add_query_handler' [-Wmissing-prototypes] 1083 | int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, acpi_handle handle) | ^~~~~~~~~~~~~~~~~~~~~~~~~ vim +/acpi_ec_add_query_handler +1083 drivers/acpi/ec.c 1082 > 1083 int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, acpi_handle handle) 1084 { 1085 struct acpi_ec_query_handler *handler = 1086 kzalloc(sizeof(struct acpi_ec_query_handler), GFP_KERNEL); 1087 1088 if (!handler) 1089 return -ENOMEM; 1090 1091 handler->query_bit = query_bit; 1092 handler->handle = handle; 1093 mutex_lock(&ec->mutex); 1094 kref_init(&handler->kref); 1095 list_add(&handler->node, &ec->list); 1096 mutex_unlock(&ec->mutex); 1097 return 0; 1098 } 1099
Hi Armin, I love your patch! Perhaps something to improve: [auto build test WARNING on rafael-pm/linux-next] [also build test WARNING on linus/master v6.2 next-20230225] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Armin-Wolf/ACPI-EC-Add-query-notifier-support/20230225-160641 base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next patch link: https://lore.kernel.org/r/20230225080458.1342359-4-W_Armin%40gmx.de patch subject: [PATCH 3/4] ACPI: EC: Make query handlers private config: x86_64-randconfig-a001 (https://download.01.org/0day-ci/archive/20230225/202302251843.r0u8s41s-lkp@intel.com/config) compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/a62cb9e29bf040af617070fa775758720d2de12e git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Armin-Wolf/ACPI-EC-Add-query-notifier-support/20230225-160641 git checkout a62cb9e29bf040af617070fa775758720d2de12e # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/acpi/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> | Link: https://lore.kernel.org/oe-kbuild-all/202302251843.r0u8s41s-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/acpi/ec.c:1083:5: warning: no previous prototype for function 'acpi_ec_add_query_handler' [-Wmissing-prototypes] int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, acpi_handle handle) ^ drivers/acpi/ec.c:1083:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, acpi_handle handle) ^ static 1 warning generated. vim +/acpi_ec_add_query_handler +1083 drivers/acpi/ec.c 1082 > 1083 int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, acpi_handle handle) 1084 { 1085 struct acpi_ec_query_handler *handler = 1086 kzalloc(sizeof(struct acpi_ec_query_handler), GFP_KERNEL); 1087 1088 if (!handler) 1089 return -ENOMEM; 1090 1091 handler->query_bit = query_bit; 1092 handler->handle = handle; 1093 mutex_lock(&ec->mutex); 1094 kref_init(&handler->kref); 1095 list_add(&handler->node, &ec->list); 1096 mutex_unlock(&ec->mutex); 1097 return 0; 1098 } 1099
Hi Armin, I love your patch! Perhaps something to improve: [auto build test WARNING on rafael-pm/linux-next] [also build test WARNING on linus/master v6.2 next-20230225] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Armin-Wolf/ACPI-EC-Add-query-notifier-support/20230225-160641 base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next patch link: https://lore.kernel.org/r/20230225080458.1342359-4-W_Armin%40gmx.de patch subject: [PATCH 3/4] ACPI: EC: Make query handlers private config: x86_64-randconfig-a012 (https://download.01.org/0day-ci/archive/20230225/202302251803.g3ubvi2K-lkp@intel.com/config) compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/a62cb9e29bf040af617070fa775758720d2de12e git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Armin-Wolf/ACPI-EC-Add-query-notifier-support/20230225-160641 git checkout a62cb9e29bf040af617070fa775758720d2de12e # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/acpi/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> | Link: https://lore.kernel.org/oe-kbuild-all/202302251803.g3ubvi2K-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/acpi/ec.c:1083:5: warning: no previous prototype for function 'acpi_ec_add_query_handler' [-Wmissing-prototypes] int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, acpi_handle handle) ^ drivers/acpi/ec.c:1083:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, acpi_handle handle) ^ static 1 warning generated. vim +/acpi_ec_add_query_handler +1083 drivers/acpi/ec.c 1082 > 1083 int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, acpi_handle handle) 1084 { 1085 struct acpi_ec_query_handler *handler = 1086 kzalloc(sizeof(struct acpi_ec_query_handler), GFP_KERNEL); 1087 1088 if (!handler) 1089 return -ENOMEM; 1090 1091 handler->query_bit = query_bit; 1092 handler->handle = handle; 1093 mutex_lock(&ec->mutex); 1094 kref_init(&handler->kref); 1095 list_add(&handler->node, &ec->list); 1096 mutex_unlock(&ec->mutex); 1097 return 0; 1098 } 1099
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index dc7860a825a0..94391a62a44c 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -143,9 +143,7 @@ MODULE_PARM_DESC(ec_no_wakeup, "Do not wake up from suspend-to-idle"); struct acpi_ec_query_handler { struct list_head node; - acpi_ec_query_func func; acpi_handle handle; - void *data; u8 query_bit; struct kref kref; }; @@ -1082,9 +1080,7 @@ static void acpi_ec_put_query_handler(struct acpi_ec_query_handler *handler) kref_put(&handler->kref, acpi_ec_query_handler_release); } -int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, - acpi_handle handle, acpi_ec_query_func func, - void *data) +int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, acpi_handle handle) { struct acpi_ec_query_handler *handler = kzalloc(sizeof(struct acpi_ec_query_handler), GFP_KERNEL); @@ -1094,40 +1090,28 @@ int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, handler->query_bit = query_bit; handler->handle = handle; - handler->func = func; - handler->data = data; mutex_lock(&ec->mutex); kref_init(&handler->kref); list_add(&handler->node, &ec->list); mutex_unlock(&ec->mutex); return 0; } -EXPORT_SYMBOL_GPL(acpi_ec_add_query_handler); -static void acpi_ec_remove_query_handlers(struct acpi_ec *ec, - bool remove_all, u8 query_bit) +static void acpi_ec_remove_query_handlers(struct acpi_ec *ec) { struct acpi_ec_query_handler *handler, *tmp; LIST_HEAD(free_list); mutex_lock(&ec->mutex); list_for_each_entry_safe(handler, tmp, &ec->list, node) { - if (remove_all || query_bit == handler->query_bit) { - list_del_init(&handler->node); - list_add(&handler->node, &free_list); - } + list_del_init(&handler->node); + list_add(&handler->node, &free_list); } mutex_unlock(&ec->mutex); list_for_each_entry_safe(handler, tmp, &free_list, node) acpi_ec_put_query_handler(handler); } -void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit) -{ - acpi_ec_remove_query_handlers(ec, false, query_bit); -} -EXPORT_SYMBOL_GPL(acpi_ec_remove_query_handler); - int register_acpi_ec_query_notifier(struct notifier_block *nb) { return blocking_notifier_chain_register(&acpi_ec_chain_head, nb); @@ -1151,12 +1135,8 @@ static void acpi_ec_event_processor(struct work_struct *work) /* Allow notifier handlers to override query handlers */ ret = blocking_notifier_call_chain(&acpi_ec_chain_head, handler->query_bit, ec); - if (ret != NOTIFY_BAD) { - if (handler->func) - handler->func(handler->data); - else if (handler->handle) - acpi_evaluate_object(handler->handle, NULL, NULL, NULL); - } + if (ret != NOTIFY_BAD && handler->handle) + acpi_evaluate_object(handler->handle, NULL, NULL, NULL); ec_dbg_evt("Query(0x%02x) stopped", handler->query_bit); @@ -1402,7 +1382,7 @@ acpi_ec_register_query_methods(acpi_handle handle, u32 level, status = acpi_get_name(handle, ACPI_SINGLE_NAME, &buffer); if (ACPI_SUCCESS(status) && sscanf(node_name, "_Q%x", &value) == 1) - acpi_ec_add_query_handler(ec, value, handle, NULL, NULL); + acpi_ec_add_query_handler(ec, value, handle); return AE_OK; } @@ -1587,7 +1567,7 @@ static void ec_remove_handlers(struct acpi_ec *ec) clear_bit(EC_FLAGS_EVENT_HANDLER_INSTALLED, &ec->flags); } if (test_bit(EC_FLAGS_QUERY_METHODS_INSTALLED, &ec->flags)) { - acpi_ec_remove_query_handlers(ec, true, 0); + acpi_ec_remove_query_handlers(ec); clear_bit(EC_FLAGS_QUERY_METHODS_INSTALLED, &ec->flags); } } diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 6f41d42375ab..72ce5a9ba57f 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -202,17 +202,12 @@ extern struct acpi_ec *first_ec; /* If we find an EC via the ECDT, we need to keep a ptr to its context */ /* External interfaces use first EC only, so remember */ -typedef int (*acpi_ec_query_func) (void *data); void acpi_ec_init(void); void acpi_ec_ecdt_probe(void); void acpi_ec_dsdt_probe(void); void acpi_ec_block_transactions(void); void acpi_ec_unblock_transactions(void); -int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, - acpi_handle handle, acpi_ec_query_func func, - void *data); -void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit); int register_acpi_ec_query_notifier(struct notifier_block *nb); int unregister_acpi_ec_query_notifier(struct notifier_block *nb);
The ability for external modules to register query handlers was broken for a long time due to having only a single user. With the only user (sbshc) having been converted to use the more robust query notifier call chain, the query handler functionality can be made private. This also allows for some cleanups. Tested on a Acer Travelmate 4000WLMi. Signed-off-by: Armin Wolf <W_Armin@gmx.de> --- drivers/acpi/ec.c | 36 ++++++++---------------------------- drivers/acpi/internal.h | 5 ----- 2 files changed, 8 insertions(+), 33 deletions(-) -- 2.30.2