| Message ID | 42d3ab9227ac3d299abcedbbdd68c86e1dd6acce.1663604826.git.quic_gokukris@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/1] remoteproc: qcom: Add sysfs entry to detect device shutdown | expand |
Hi Gokul,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on remoteproc/rproc-next]
[also build test WARNING on linus/master v6.0-rc6 next-20220921]
[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/Gokul-krishna-Krishnakumar/remoteproc-qcom-Add-sysfs-entry-to-detect-device-shutdown/20220920-004947
base: git://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git rproc-next
config: arm-defconfig (https://download.01.org/0day-ci/archive/20220922/202209220842.14LbOlkM-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 791a7ae1ba3efd6bca96338e10ffde557ba83920)
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
# install arm cross compiling tool for clang build
# apt-get install binutils-arm-linux-gnueabi
# https://github.com/intel-lab-lkp/linux/commit/dcf1afc28aee2a1770cdabe1f52d7e90532018c7
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Gokul-krishna-Krishnakumar/remoteproc-qcom-Add-sysfs-entry-to-detect-device-shutdown/20220920-004947
git checkout dcf1afc28aee2a1770cdabe1f52d7e90532018c7
# 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=arm SHELL=/bin/bash drivers/remoteproc/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/remoteproc/qcom_common.c:97:27: warning: unused variable 'ssr_timeout_msg' [-Wunused-const-variable]
static const char * const ssr_timeout_msg = "srcu notifier chain for %s:%s taking too long";
^
1 warning generated.
vim +/ssr_timeout_msg +97 drivers/remoteproc/qcom_common.c
96
> 97 static const char * const ssr_timeout_msg = "srcu notifier chain for %s:%s taking too long";
98
Hi Gokul, > Subject: [PATCH v1 1/1] remoteproc: qcom: Add sysfs entry to detect device > shutdown > > This change adds a sysfs entry which indicates whether the device is being > shutdown to the qcom remoteproc drivers. This is going to be used in the > following patches. I have no knowledge of qcom platform, just a few generic comments: I think it would be better if you post a link to give people a full picture on how this going to work. > > Signed-off-by: Gokul krishna Krishnakumar <quic_gokukris@quicinc.com> > --- > drivers/remoteproc/qcom_common.c | 58 > ++++++++++++++++++++++++++++++++++++++++ > drivers/remoteproc/qcom_common.h | 1 + > 2 files changed, 59 insertions(+) > > diff --git a/drivers/remoteproc/qcom_common.c > b/drivers/remoteproc/qcom_common.c > index 020349f..7959e96 100644 > --- a/drivers/remoteproc/qcom_common.c > +++ b/drivers/remoteproc/qcom_common.c > @@ -87,9 +87,32 @@ struct qcom_ssr_subsystem { > struct list_head list; > }; > > +static struct kobject *sysfs_kobject; > +bool qcom_device_shutdown_in_progress; > +EXPORT_SYMBOL(qcom_device_shutdown_in_progress); > + > static LIST_HEAD(qcom_ssr_subsystem_list); > static DEFINE_MUTEX(qcom_ssr_subsys_lock); > > +static const char * const ssr_timeout_msg = "srcu notifier chain for > +%s:%s taking too long"; I not see this variable is being used anywhere. > + > +static ssize_t qcom_rproc_shutdown_request_store(struct kobject *kobj, > struct kobj_attribute *attr, > + const char *buf, size_t > count) > +{ > + bool val; > + int ret; > + > + ret = kstrtobool(buf, &val); > + if (ret) > + return ret; > + > + qcom_device_shutdown_in_progress = val; > + pr_info("qcom rproc: Device shutdown requested: %s\n", val ? > "true" : "false"); This is a sysfs write operation, how does it matter with device shutdown in progress? > + return count; > +} > +static struct kobj_attribute shutdown_requested_attr = > __ATTR(shutdown_in_progress, 0220, NULL, How about DEVICE_ATTR_WO, but seems you use 0220, the generic marco use 0200. > + > qcom_rproc_shutdown_request_store); > + > static void qcom_minidump_cleanup(struct rproc *rproc) { > struct rproc_dump_segment *entry, *tmp; @@ -505,5 +528,40 @@ > void qcom_remove_ssr_subdev(struct rproc *rproc, struct qcom_rproc_ssr > *ssr) } EXPORT_SYMBOL_GPL(qcom_remove_ssr_subdev); > > +static int __init qcom_common_init(void) { > + int ret = 0; > + > + qcom_device_shutdown_in_progress = false; > + > + sysfs_kobject = kobject_create_and_add("qcom_rproc", > kernel_kobj); > + if (!sysfs_kobject) { > + pr_err("qcom rproc: failed to create sysfs kobject\n"); > + return -EINVAL; > + } > + > + ret = sysfs_create_file(sysfs_kobject, > &shutdown_requested_attr.attr); > + if (ret) { > + pr_err("qcom rproc: failed to create sysfs file\n"); > + goto remove_kobject; > + } > + > + return 0; > + > + sysfs_remove_file(sysfs_kobject, &shutdown_requested_attr.attr); > +remove_kobject: > + kobject_put(sysfs_kobject); > + return ret; > + > +} > +module_init(qcom_common_init); > + > +static void __exit qcom_common_exit(void) { > + sysfs_remove_file(sysfs_kobject, &shutdown_requested_attr.attr); > + kobject_put(sysfs_kobject); > +} > +module_exit(qcom_common_exit); > + > MODULE_DESCRIPTION("Qualcomm Remoteproc helper driver"); > MODULE_LICENSE("GPL v2"); As I recall, checkpatch would report GPL is enough, no need v2. Regards, Peng. diff --git > a/drivers/remoteproc/qcom_common.h > b/drivers/remoteproc/qcom_common.h > index c35adf7..90b79ce 100644 > --- a/drivers/remoteproc/qcom_common.h > +++ b/drivers/remoteproc/qcom_common.h > @@ -34,6 +34,7 @@ struct qcom_rproc_ssr { }; > > void qcom_minidump(struct rproc *rproc, unsigned int minidump_id); > +extern bool qcom_device_shutdown_in_progress; > > void qcom_add_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink > *glink, > const char *ssr_name); > -- > 2.7.4
diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c index 020349f..7959e96 100644 --- a/drivers/remoteproc/qcom_common.c +++ b/drivers/remoteproc/qcom_common.c @@ -87,9 +87,32 @@ struct qcom_ssr_subsystem { struct list_head list; }; +static struct kobject *sysfs_kobject; +bool qcom_device_shutdown_in_progress; +EXPORT_SYMBOL(qcom_device_shutdown_in_progress); + static LIST_HEAD(qcom_ssr_subsystem_list); static DEFINE_MUTEX(qcom_ssr_subsys_lock); +static const char * const ssr_timeout_msg = "srcu notifier chain for %s:%s taking too long"; + +static ssize_t qcom_rproc_shutdown_request_store(struct kobject *kobj, struct kobj_attribute *attr, + const char *buf, size_t count) +{ + bool val; + int ret; + + ret = kstrtobool(buf, &val); + if (ret) + return ret; + + qcom_device_shutdown_in_progress = val; + pr_info("qcom rproc: Device shutdown requested: %s\n", val ? "true" : "false"); + return count; +} +static struct kobj_attribute shutdown_requested_attr = __ATTR(shutdown_in_progress, 0220, NULL, + qcom_rproc_shutdown_request_store); + static void qcom_minidump_cleanup(struct rproc *rproc) { struct rproc_dump_segment *entry, *tmp; @@ -505,5 +528,40 @@ void qcom_remove_ssr_subdev(struct rproc *rproc, struct qcom_rproc_ssr *ssr) } EXPORT_SYMBOL_GPL(qcom_remove_ssr_subdev); +static int __init qcom_common_init(void) +{ + int ret = 0; + + qcom_device_shutdown_in_progress = false; + + sysfs_kobject = kobject_create_and_add("qcom_rproc", kernel_kobj); + if (!sysfs_kobject) { + pr_err("qcom rproc: failed to create sysfs kobject\n"); + return -EINVAL; + } + + ret = sysfs_create_file(sysfs_kobject, &shutdown_requested_attr.attr); + if (ret) { + pr_err("qcom rproc: failed to create sysfs file\n"); + goto remove_kobject; + } + + return 0; + + sysfs_remove_file(sysfs_kobject, &shutdown_requested_attr.attr); +remove_kobject: + kobject_put(sysfs_kobject); + return ret; + +} +module_init(qcom_common_init); + +static void __exit qcom_common_exit(void) +{ + sysfs_remove_file(sysfs_kobject, &shutdown_requested_attr.attr); + kobject_put(sysfs_kobject); +} +module_exit(qcom_common_exit); + MODULE_DESCRIPTION("Qualcomm Remoteproc helper driver"); MODULE_LICENSE("GPL v2"); diff --git a/drivers/remoteproc/qcom_common.h b/drivers/remoteproc/qcom_common.h index c35adf7..90b79ce 100644 --- a/drivers/remoteproc/qcom_common.h +++ b/drivers/remoteproc/qcom_common.h @@ -34,6 +34,7 @@ struct qcom_rproc_ssr { }; void qcom_minidump(struct rproc *rproc, unsigned int minidump_id); +extern bool qcom_device_shutdown_in_progress; void qcom_add_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink *glink, const char *ssr_name);
This change adds a sysfs entry which indicates whether the device is being shutdown to the qcom remoteproc drivers. This is going to be used in the following patches. Signed-off-by: Gokul krishna Krishnakumar <quic_gokukris@quicinc.com> --- drivers/remoteproc/qcom_common.c | 58 ++++++++++++++++++++++++++++++++++++++++ drivers/remoteproc/qcom_common.h | 1 + 2 files changed, 59 insertions(+)