From: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>
To: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Ohad Ben-Cohen <ohad@wizery.com>,
Mathieu Poirier <mathieu.poirier@linaro.org>,
<linux-remoteproc@vger.kernel.org>,
<linux-kernel@vger.kernel.org>,
<linux-stm32@st-md-mailman.stormreply.com>,
<julien.massot@iot.bzh>
Subject: Re: [PATCH v6 03/10] rpmsg: Move the rpmsg control device from rpmsg_char to rpmsg_ctrl
Date: Tue, 2 Nov 2021 16:42:28 +0100 [thread overview]
Message-ID: <37c37910-9cc0-2caa-de48-614d3b2f41a8@foss.st.com> (raw)
In-Reply-To: <YYAe1tUR+aCZ8cw0@builder.lan>
On 11/1/21 6:07 PM, Bjorn Andersson wrote:
> On Fri 22 Oct 07:54 CDT 2021, Arnaud Pouliquen wrote:
>
>> Create the rpmsg_ctrl.c module and move the code related to the
>> rpmsg_ctrldev device in this new module.
>>
>> Add the dependency between rpmsg_char and rpmsg_ctrl in the
>> kconfig file.
>>
>> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
>> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>>
>> ---
>> update vs previous version:
>> - set the ctrl device class with new rpmsg_get_class API for legacy support
>> ---
>> drivers/rpmsg/Kconfig | 9 ++
>> drivers/rpmsg/Makefile | 1 +
>> drivers/rpmsg/rpmsg_char.c | 169 +----------------------------
>> drivers/rpmsg/rpmsg_char.h | 2 +
>> drivers/rpmsg/rpmsg_ctrl.c | 216 +++++++++++++++++++++++++++++++++++++
>> 5 files changed, 230 insertions(+), 167 deletions(-)
>> create mode 100644 drivers/rpmsg/rpmsg_ctrl.c
>>
>> diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig
>> index 0b4407abdf13..d822ec9ec692 100644
>> --- a/drivers/rpmsg/Kconfig
>> +++ b/drivers/rpmsg/Kconfig
>> @@ -10,11 +10,20 @@ config RPMSG_CHAR
>> tristate "RPMSG device interface"
>> depends on RPMSG
>> depends on NET
>> + select RPMSG_CTRL
>
> We don't want select of user selectable config options.
>
>> help
>> Say Y here to export rpmsg endpoints as device files, usually found
>> in /dev. They make it possible for user-space programs to send and
>> receive rpmsg packets.
>>
>> +config RPMSG_CTRL
>
> I still don't like the introduction of more Kconfig options - search the
> list for the number of patches that has corrected Kconfig dependency
> issues.
>
> That said, if you get it right...
>
>> + tristate "RPMSG control interface"
>> + depends on RPMSG
>> + help
>> + Say Y here to enable the support of the /dev/rpmsg_ctrlX API. This API
>> + allows user-space programs to create endpoints with specific service name,
>> + source and destination addresses.
>> +
>> config RPMSG_NS
>> tristate "RPMSG name service announcement"
>> depends on RPMSG
> [..]
>> diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
> [..]
>> diff --git a/drivers/rpmsg/rpmsg_char.h b/drivers/rpmsg/rpmsg_char.h
>> index 109c2c43005f..ff1acc42628a 100644
>> --- a/drivers/rpmsg/rpmsg_char.h
>> +++ b/drivers/rpmsg/rpmsg_char.h
>> @@ -12,6 +12,8 @@
>> * In such case a kernel warning is printed to help develloper to fix the issue.
>> */
>>
>> +#define RPMSG_DEV_MAX (MINORMASK + 1)
>
> This was used to define the minors of the rpmsg chdev, now you're
> splitting that range in one for the ctrl and one for the char.
>
> Moving this define to a common place gives an impression that there's a
> relationship between the two, but I don't see any. So I think you should
> duplicate this in the two files - just like the other stuff.
That's true, I will fix this
>
>> +
>> #if IS_REACHABLE(CONFIG_RPMSG_CHAR)
>> /**
>> * rpmsg_chrdev_eptdev_create() - register char device based on an endpoint
>> diff --git a/drivers/rpmsg/rpmsg_ctrl.c b/drivers/rpmsg/rpmsg_ctrl.c
>> new file mode 100644
>> index 000000000000..1d3c12e5cdcf
>> --- /dev/null
>> +++ b/drivers/rpmsg/rpmsg_ctrl.c
>> @@ -0,0 +1,216 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2021, STMicroelectronics
>
> Did you actually change anything that warrant the explicit copyright
> claim?
>
Mainly because i created a new file here, but right it is a copy-past, so
I will remove it.
Thanks,
Arnaud
>> + * Copyright (c) 2016, Linaro Ltd.
>> + * Copyright (c) 2012, Michal Simek <monstr@monstr.eu>
>> + * Copyright (c) 2012, PetaLogix
>> + * Copyright (c) 2011, Texas Instruments, Inc.
>> + * Copyright (c) 2011, Google, Inc.
>> + *
>> + * Based on rpmsg performance statistics driver by Michal Simek, which in turn
>> + * was based on TI & Google OMX rpmsg driver.
>> + */
> [..]
>> +static int rpmsg_ctrldev_probe(struct rpmsg_device *rpdev)
>> +{
>> + struct rpmsg_ctrldev *ctrldev;
>> + struct device *dev;
>> + int ret;
>> +
>> + ctrldev = kzalloc(sizeof(*ctrldev), GFP_KERNEL);
>> + if (!ctrldev)
>> + return -ENOMEM;
>> +
>> + ctrldev->rpdev = rpdev;
>> +
>> + dev = &ctrldev->dev;
>> + device_initialize(dev);
>> + dev->parent = &rpdev->dev;
>> + dev->class = rpmsg_get_class();
>
> Thank you.
>
> Regards,
> Bjorn
>
next prev parent reply other threads:[~2021-11-02 15:42 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-22 12:54 [PATCH v6 00/10] Restructure the rpmsg_char driver and introduce rpmsg_ctrl driver Arnaud Pouliquen
2021-10-22 12:54 ` [PATCH v6 01/10] rpmsg: char: Export eptdev create an destroy functions Arnaud Pouliquen
2021-11-01 16:55 ` Bjorn Andersson
2021-11-02 14:52 ` Arnaud POULIQUEN
2021-10-22 12:54 ` [PATCH v6 02/10] rpmsg: create the rpmsg class in core instead of in rpmsg char Arnaud Pouliquen
2021-11-01 16:57 ` Bjorn Andersson
2021-11-02 15:23 ` Arnaud POULIQUEN
2021-11-02 16:38 ` Bjorn Andersson
2021-11-02 17:11 ` Arnaud POULIQUEN
2021-11-03 17:48 ` Bjorn Andersson
2021-10-22 12:54 ` [PATCH v6 03/10] rpmsg: Move the rpmsg control device from rpmsg_char to rpmsg_ctrl Arnaud Pouliquen
2021-11-01 17:07 ` Bjorn Andersson
2021-11-02 15:42 ` Arnaud POULIQUEN [this message]
2021-10-22 12:54 ` [PATCH v6 04/10] rpmsg: Update rpmsg_chrdev_register_device function Arnaud Pouliquen
2021-11-01 17:14 ` Bjorn Andersson
2021-10-22 12:54 ` [PATCH v6 05/10] rpmsg: char: Refactor rpmsg_chrdev_eptdev_create function Arnaud Pouliquen
2021-10-22 12:54 ` [PATCH v6 06/10] rpmsg: Introduce rpmsg_create_default_ept function Arnaud Pouliquen
2021-11-01 17:37 ` Bjorn Andersson
2021-11-02 16:56 ` Arnaud POULIQUEN
2021-11-03 16:57 ` Bjorn Andersson
2021-11-03 17:35 ` Arnaud POULIQUEN
2021-10-22 12:54 ` [PATCH v6 07/10] rpmsg: char: Add possibility to use default endpoint of the rpmsg device Arnaud Pouliquen
2021-10-22 12:54 ` [PATCH v6 08/10] rpmsg: char: Introduce the "rpmsg-raw" channel Arnaud Pouliquen
2021-10-22 12:54 ` [PATCH v6 09/10] rpmsg: ctrl: Introduce new RPMSG_CREATE/RELEASE_DEV_IOCTL controls Arnaud Pouliquen
2021-10-22 12:54 ` [PATCH v6 10/10] rpmsg: core: send a ns announcement when a default endpoint is created Arnaud Pouliquen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=37c37910-9cc0-2caa-de48-614d3b2f41a8@foss.st.com \
--to=arnaud.pouliquen@foss.st.com \
--cc=bjorn.andersson@linaro.org \
--cc=julien.massot@iot.bzh \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-remoteproc@vger.kernel.org \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=mathieu.poirier@linaro.org \
--cc=ohad@wizery.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).