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>
Subject: Re: [PATCH v5 2/4] rpmsg: char: Export eptdev create an destroy functions
Date: Mon, 11 Oct 2021 12:39:04 +0200 [thread overview]
Message-ID: <ea8084fa-b9b9-8e23-1adf-f5d22e94192e@foss.st.com> (raw)
In-Reply-To: <YWDURVTg1PpxRDEu@ripper>
On 10/9/21 1:29 AM, Bjorn Andersson wrote:
> On Mon 12 Jul 05:37 PDT 2021, Arnaud Pouliquen wrote:
> [..]
>> diff --git a/drivers/rpmsg/rpmsg_char.h b/drivers/rpmsg/rpmsg_char.h
>> new file mode 100644
>> index 000000000000..22573b60e008
>> --- /dev/null
>> +++ b/drivers/rpmsg/rpmsg_char.h
>> @@ -0,0 +1,49 @@
>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>> +/*
>> + * Copyright (C) STMicroelectronics 2021.
>> + */
>> +
>> +#ifndef __RPMSG_CHRDEV_H__
>> +#define __RPMSG_CHRDEV_H__
>> +
>> +#if IS_REACHABLE(CONFIG_RPMSG_CHAR)
>
> This does allow you to build a kernel with RPMSG_CHAR=m and RPMSG_CTRL=y
> without link failures. Any modules in the system will be able to call
> rpmsg_chrdev_eptdev_create(), but the rpmsg_ctrl driver will only end up
> in the stub.
>
> This sounds like a recipe for a terrible debugging session...
The RPMSG_CREATE_EPT_IOCTL control create a dependency between the rpmsg_ctrl
and the rpmsg_char. The build error option seems not a good alternative.
And in case of RPMSG_CHAR=m and RPMSG_CTRL=y, an error is returned so not only a
stub.
What about adding a WARN_ON(1) in rpmsg_chrdev_eptdev_create
with an associated comment to ease the debug?
Regards,
Arnaud
>
> Regards,
> Bjorn
>
>> +/**
>> + * rpmsg_chrdev_eptdev_create() - register char device based on an endpoint
>> + * @rpdev: prepared rpdev to be used for creating endpoints
>> + * @parent: parent device
>> + * @chinfo: associated endpoint channel information.
>> + *
>> + * This function create a new rpmsg char endpoint device to instantiate a new
>> + * endpoint based on chinfo information.
>> + */
>> +int rpmsg_chrdev_eptdev_create(struct rpmsg_device *rpdev, struct device *parent,
>> + struct rpmsg_channel_info chinfo);
>> +
>> +/**
>> + * rpmsg_chrdev_eptdev_destroy() - destroy created char device endpoint.
>> + * @data: private data associated to the endpoint device
>> + *
>> + * This function destroys a rpmsg char endpoint device created by the RPMSG_DESTROY_EPT_IOCTL
>> + * control.
>> + */
>> +int rpmsg_chrdev_eptdev_destroy(struct device *dev, void *data);
>> +
>> +#else /*IS_REACHABLE(CONFIG_RPMSG_CHAR) */
>> +
>> +static inline int rpmsg_chrdev_eptdev_create(struct rpmsg_device *rpdev, struct device *parent,
>> + struct rpmsg_channel_info chinfo)
>> +{
>> + return -EINVAL;
>> +}
>> +
>> +static inline int rpmsg_chrdev_eptdev_destroy(struct device *dev, void *data)
>> +{
>> + /* This shouldn't be possible */
>> + WARN_ON(1);
>> +
>> + return 0;
>> +}
>> +
>> +#endif /*IS_REACHABLE(CONFIG_RPMSG_CHAR) */
>> +
>> +#endif /*__RPMSG_CHRDEV_H__ */
>> --
>> 2.17.1
>>
next prev parent reply other threads:[~2021-10-11 10:39 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-12 12:37 [PATCH v5 0/4] Restructure the rpmsg char to decorrelate the control part Arnaud Pouliquen
2021-07-12 12:37 ` [PATCH v5 1/4] rpmsg: char: Remove useless include Arnaud Pouliquen
2021-10-09 0:30 ` (subset) " Bjorn Andersson
2021-07-12 12:37 ` [PATCH v5 2/4] rpmsg: char: Export eptdev create an destroy functions Arnaud Pouliquen
2021-10-08 23:29 ` Bjorn Andersson
2021-10-11 10:39 ` Arnaud POULIQUEN [this message]
2021-07-12 12:37 ` [PATCH v5 3/4] rpmsg: Move the rpmsg control device from rpmsg_char to rpmsg_ctrl Arnaud Pouliquen
2021-10-08 23:35 ` Bjorn Andersson
2021-10-11 10:46 ` Arnaud POULIQUEN
2021-10-16 4:46 ` Bjorn Andersson
2021-10-18 9:13 ` Arnaud POULIQUEN
2021-10-19 3:08 ` Bjorn Andersson
2021-07-12 12:37 ` [PATCH v5 4/4] rpmsg: Update rpmsg_chrdev_register_device function Arnaud Pouliquen
2021-10-08 23:21 ` [PATCH v5 0/4] Restructure the rpmsg char to decorrelate the control part Bjorn Andersson
2021-10-11 10:38 ` Arnaud POULIQUEN
2021-10-19 3:28 ` Bjorn Andersson
2021-10-19 12:54 ` 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=ea8084fa-b9b9-8e23-1adf-f5d22e94192e@foss.st.com \
--to=arnaud.pouliquen@foss.st.com \
--cc=bjorn.andersson@linaro.org \
--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).