Messages in this thread | | | Subject | Re:Re: [PATCH 1/3] driver: rpmon: new driver Remote Processor Monitor | From | 王文虎 <> | Date | Sun, 12 Apr 2020 18:52:10 +0800 (GMT+08:00) |
| |
Hi, From: Randy Dunlap <rdunlap@infradead.org> Date: 2020-04-12 02:08:09 To: Wang Wenhu <wenhu.wang@vivo.com>,gregkh@linuxfoundation.org,linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/3] driver: rpmon: new driver Remote Processor Monitor>Hi-- > >On 4/11/20 2:52 AM, Wang Wenhu wrote: >> RPMON is a driver framework. It supports remote processor monitor >> from user level. The baisc components are a character device > > basic
Addressed in v2
> >> with sysfs interfaces for user space communication and different >> kinds of message drivers introduced modularly, which are used to >> communicate with remote processors. >> >> As for user space, one can get notifications of different events >> of remote processors, like their registrations, through standard >> file read operation of the file discriptors related to the exported > > descriptors
Addressed in v2
> >> character devices. Actions can also be taken into account via >> standard write operations to the devices. Besides, the sysfs class >> attributes could be accessed conveniently. >> >> Message drivers act as engines to communicate with remote processors. >> Currently RPMON_QMI is available which uses QMI infrastructures >> on Qualcomm SoC Platforms. >> >> Signed-off-by: Wang Wenhu <wenhu.wang@vivo.com> >> --- >> drivers/Kconfig | 2 + >> drivers/Makefile | 1 + >> drivers/rpmon/Kconfig | 26 +++ >> drivers/rpmon/Makefile | 1 + >> drivers/rpmon/rpmon.c | 505 +++++++++++++++++++++++++++++++++++++++++ >> include/linux/rpmon.h | 68 ++++++ >> 6 files changed, 603 insertions(+) >> create mode 100644 drivers/rpmon/Kconfig >> create mode 100644 drivers/rpmon/Makefile >> create mode 100644 drivers/rpmon/rpmon.c >> create mode 100644 include/linux/rpmon.h >> >> diff --git a/drivers/rpmon/Kconfig b/drivers/rpmon/Kconfig >> new file mode 100644 >> index 000000000000..505d263e0867 >> --- /dev/null >> +++ b/drivers/rpmon/Kconfig >> @@ -0,0 +1,26 @@ >> +# >> +# Remote Processor Monitor Drivers >> +# >> +menu "Remote Processor Monitor Drivers" >> + >> +config RPMON >> + tristate "Remote Processor Monitor Core Framework" >> + help >> + RPMON is a driver framework. It supports remote processor monitor >> + from user level. The baisc components are a character device > > basic
Addressed in v2
> >> + with sysfs interfaces for user space communication and different >> + kinds of message drivers introduced modularly, which are used to >> + communicate with remote processors. >> + >> + As for user space, one can get notifications of different events >> + of remote processors, like their registrations, through standard >> + file read operation of the file discriptors related to the exported > > descriptors
Addressed in v2
> >> + character devices. Actions can also be taken into account via >> + standard write operations to the devices. Besides, the sysfs class >> + attributes could be accessed conveniently. >> + >> + Message drivers act as engines to communicate with remote processors. >> + Currently RPMON_QMI is available which uses QMI infrastructures >> + on Qualcomm SoC Platforms. >> + >> +endmenu > >> diff --git a/drivers/rpmon/rpmon.c b/drivers/rpmon/rpmon.c >> new file mode 100644 >> index 000000000000..65aab4de6733 >> --- /dev/null >> +++ b/drivers/rpmon/rpmon.c >> @@ -0,0 +1,505 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (C) 2020 Vivo Communication Technology Co. Ltd. >> + * Copyright (C) 2020 Wang Wenhu <wenhu.wang@vivo.com> >> + * All rights reserved. >> + * >> + * RPMON: An implementation of remote processor monitor freamwork > > framework
Addressed in v2
> >> + * for platforms that multi-processors exists. RPMON is implemented > > confusing wording above ^^^^^^^^^^^^^^^^^
Addressed in v2
> >> + * with chardev and sysfs class as interfaces to communicate with >> + * the user level. It supports different communication interfaces >> + * added modularly with remote processors. Currently QMI implementation >> + * is available. >> + * >> + * RPMON could be used to detect the stabilities of remote processors, >> + * collect any kinds of information you are interested with, take > > interested in,
Addressed in v2
> >> + * actions like connection status check, and so on. Enhancements can >> + * be made upon current implementation. >> + */ >> + >> +#include <linux/module.h> >> +#include <linux/cdev.h> >> +#include <linux/rpmon.h> >> + >> +#define RPMON_MAX_DEVICES (1U << MINORBITS) >> +#define RPMON_NAME "rpmon" >> + >> +static int rpmon_major; >> +static struct cdev *rpmon_cdev; >> +static DEFINE_IDR(rpmon_idr); >> +static const struct file_operations rpmon_fops; >> + >> +/* Protect idr accesses */ >> +static DEFINE_MUTEX(minor_lock); >> + > >[snip] > >> +/** >> + * rpmon_event_notify - trigger an notify event >> + * @info: RPMON device capabilities >> + * @event: RPMON event to be triggered >> + */ >> +void rpmon_event_notify(struct rpmon_info *info, u32 event) >> +{ >> + struct rpmon_device *rpmondev = info->rpmon_dev; >> + >> + if (event >= RPMON_EVENT_MAX) { >> + pr_err("Error un-supported rpmon event %d", event); > > unsupported
Addressed in v2
> >> + return; >> + } >> + >> + atomic_set(&rpmondev->event, RPMON_EVENT(event)); >> + wake_up_interruptible(&rpmondev->wait); >> + kill_fasync(&rpmondev->async_queue, SIGIO, POLL_IN); >> +} >> +EXPORT_SYMBOL_GPL(rpmon_event_notify); > >[snip] > >> +/** >> + * rpmon_register_device - register a new rpmon interface device >> + * @owner: module that creates the new device >> + * @parent: parent device >> + * @info: romon device capabilities > > s/romon/rpmon/
Addressed in v2
> >> + * >> + * returns zero on success or a negative error code. > >use kernel-doc notation: > > * return: zero on success or a negative error code. >
Addressed in v2
>> + */ >> +int __rpmon_register_device(struct module *owner, >> + struct device *parent, >> + struct rpmon_info *info) >> +{ >> + struct rpmon_device *rpmondev; >> + int ret = 0; >> + >> + if (!rpmon_class_registered) >> + return -EPROBE_DEFER; >> + >> + if (!parent || !info || !info->name || !info->version) >> + return -EINVAL; >> + >> + info->rpmon_dev = NULL; >> + >> + rpmondev = kzalloc(sizeof(*rpmondev), GFP_KERNEL); >> + if (!rpmondev) >> + return -ENOMEM; >> + >> + rpmondev->owner = owner; >> + rpmondev->info = info; >> + mutex_init(&rpmondev->info_lock); >> + init_waitqueue_head(&rpmondev->wait); >> + atomic_set(&rpmondev->event, 0); >> + >> + ret = rpmon_get_minor(rpmondev); >> + if (ret) { >> + kfree(rpmondev); >> + return ret; >> + } >> + >> + device_initialize(&rpmondev->dev); >> + rpmondev->dev.devt = MKDEV(rpmon_major, rpmondev->minor); >> + rpmondev->dev.class = &rpmon_class; >> + rpmondev->dev.parent = parent; >> + rpmondev->dev.release = rpmon_device_release; >> + dev_set_drvdata(&rpmondev->dev, rpmondev); >> + >> + ret = dev_set_name(&rpmondev->dev, RPMON_NAME"%d", rpmondev->minor); >> + if (ret) >> + goto err_device_create; >> + >> + ret = device_add(&rpmondev->dev); >> + if (ret) >> + goto err_device_create; >> + >> + if (rpmondev->info->rpmon_dev_add_attrs) { >> + ret = rpmondev->info->rpmon_dev_add_attrs(rpmondev); >> + if (ret) >> + goto err_dev_add_attrs; >> + } >> + >> + info->rpmon_dev = rpmondev; >> + >> + return 0; >> + >> +err_dev_add_attrs: >> + device_del(&rpmondev->dev); >> +err_device_create: >> + rpmon_free_minor(rpmondev); >> + put_device(&rpmondev->dev); >> + return ret; >> +} >> +EXPORT_SYMBOL_GPL(__rpmon_register_device); > >[snip] > >> +module_exit(rpmon_exit); >> + >> +MODULE_AUTHOR("Wang Wenhu"); > >Please add email address in the MODULE_AUTHOR() string. >About 3/4 of all uses of MODULE_AUTHOR() do so. >
Addressed in v2
>> +MODULE_DESCRIPTION("Remote Processor Monitor Core Framework"); >> +MODULE_LICENSE("GPL v2"); >> diff --git a/include/linux/rpmon.h b/include/linux/rpmon.h >> new file mode 100644 >> index 000000000000..40983a3b5655 >> --- /dev/null >> +++ b/include/linux/rpmon.h >> @@ -0,0 +1,68 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> +/* >> + * Copyright (C) 2020 Vivo Communication Technology Co. Ltd. >> + * Copyright (C) 2020 Wang Wenhu <wenhu.wang@vivo.com> >> + * All rights reserved. >> + */ >> + >> +#ifndef RPMON_H >> +#define RPMON_H >> + >> +#include <net/sock.h> >> + >> +/* RPMON action would be taken */ >> +enum rpmon_exec { >> + RPMON_EXEC_CHECK_CONN = 0, >> + RPMON_EXEC_MAX, >> +}; >> + >> +/* RPMON events that may be notified */ >> +enum rpmon_event { >> + RPMON_EVENT_CHKCONN_FAIL = 0, >> + RPMON_EVENT_REGISTER, >> + RPMON_EVENT_MAX, >> +}; >> + >> +#define RPMON_EVENT(x) (0x1 << x) >> +#define RPMON_ACTION(x) (0x1 << x) > >Unless you are very sure that 'x' above is never more than a simple >expression, you should put x in parentheses, like so: >
Addressed in v2
>> +#define RPMON_EVENT(x) (1 << (x)) >> +#define RPMON_ACTION(x) (1 << (x)) > >so that there cannot be any operator precedence problems. > > >thanks. >-- >~Randy >
thanks, Wenhu
| |