[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <64b5f77a-7bc1-ee43-0f83-1ff323e4ac51@infradead.org>
Date: Sat, 11 Apr 2020 11:08:09 -0700
From: Randy Dunlap <rdunlap@...radead.org>
To: Wang Wenhu <wenhu.wang@...o.com>, gregkh@...uxfoundation.org,
linux-kernel@...r.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
> 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
> 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@...o.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
> + 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
> + 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@...o.com>
> + * All rights reserved.
> + *
> + * RPMON: An implementation of remote processor monitor freamwork
framework
> + * for platforms that multi-processors exists. RPMON is implemented
confusing wording above ^^^^^^^^^^^^^^^^^
> + * 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,
> + * 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
> + 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/
> + *
> + * returns zero on success or a negative error code.
use kernel-doc notation:
* return: zero on success or a negative error code.
> + */
> +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.
> +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@...o.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:
> +#define RPMON_EVENT(x) (1 << (x))
> +#define RPMON_ACTION(x) (1 << (x))
so that there cannot be any operator precedence problems.
thanks.
--
~Randy
Powered by blists - more mailing lists