[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <AHgABAClCOKoeJL-ajjSxaqg.3.1586688730769.Hmail.wenhu.wang@vivo.com>
Date: Sun, 12 Apr 2020 18:52:10 +0800 (GMT+08:00)
From: 王文虎 <wenhu.wang@...o.com>
To: Randy Dunlap <rdunlap@...radead.org>
Cc: gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org
Subject: Re:Re: [PATCH 1/3] driver: rpmon: new driver Remote Processor Monitor
Hi,
From: Randy Dunlap <rdunlap@...radead.org>
Date: 2020-04-12 02:08:09
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
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@...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
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@...o.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@...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:
>
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
Powered by blists - more mailing lists