Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752967AbbH1RUD (ORCPT ); Fri, 28 Aug 2015 13:20:03 -0400 Received: from mail-pa0-f51.google.com ([209.85.220.51]:36412 "EHLO mail-pa0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752820AbbH1RUA (ORCPT ); Fri, 28 Aug 2015 13:20:00 -0400 Message-ID: <55E097A8.4080305@gmail.com> Date: Fri, 28 Aug 2015 10:17:28 -0700 From: Florian Fainelli User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.8.0 MIME-Version: 1.0 To: Lee Jones , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org CC: ohad@wizery.com, devicetree@vger.kernel.org, kernel@stlinux.com, Nathan_Lynch@mentor.com, Ludovic Barre Subject: Re: [PATCH v2 4/4] remoteproc: debugfs: Add ability to boot remote processor using debugfs References: <1440757911-9120-1-git-send-email-lee.jones@linaro.org> <1440757911-9120-5-git-send-email-lee.jones@linaro.org> In-Reply-To: <1440757911-9120-5-git-send-email-lee.jones@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2658 Lines: 77 On 28/08/15 03:31, Lee Jones wrote: > This functionality is especially useful during the testing phase. When > used in conjunction with Mailbox's Test Framework we can trivially conduct > end-to-end testing i.e. boot co-processor, send and receive messages to > the co-processor, then shut it down again (repeat as required). This does not strike me as a particularly well defined nor suitable interface for controlling a remote processor's state. I know you are just extending the existing debugfs interface here, but someone ought to remove that piece of code and make it a proper character device or netlink or whatever that allows someone to get proper signaling of what's going on with the remote processor state by polling or listening to a socket. What's the intended use case behind debugfs for this after all? Is your application expected to keep reading the state file in a loop until it is happy and reads "running", how is that not racy by nature? > > Signed-off-by: Ludovic Barre > Signed-off-by: Lee Jones > --- > drivers/remoteproc/remoteproc_debugfs.c | 29 +++++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/drivers/remoteproc/remoteproc_debugfs.c b/drivers/remoteproc/remoteproc_debugfs.c > index 9d30809..464470d 100644 > --- a/drivers/remoteproc/remoteproc_debugfs.c > +++ b/drivers/remoteproc/remoteproc_debugfs.c > @@ -88,8 +88,37 @@ static ssize_t rproc_state_read(struct file *filp, char __user *userbuf, > return simple_read_from_buffer(userbuf, count, ppos, buf, i); > } > > +static ssize_t rproc_state_write(struct file *filp, const char __user *userbuf, > + size_t count, loff_t *ppos) > +{ > + struct rproc *rproc = filp->private_data; > + char buf[2]; > + int ret; > + > + ret = copy_from_user(buf, userbuf, 1); > + if (ret) > + return -EFAULT; > + > + switch (buf[0]) { > + case '0': > + rproc_shutdown(rproc); > + break; > + case '1': > + ret = rproc_boot(rproc); > + if (ret) > + dev_warn(&rproc->dev, "Boot failed: %d\n", ret); > + break; > + default: > + dev_err(&rproc->dev, "Unrecognised option: %x\n", buf[1]); > + return -EINVAL; > + } > + > + return count; > +} > + > static const struct file_operations rproc_state_ops = { > .read = rproc_state_read, > + .write = rproc_state_write, > .open = simple_open, > .llseek = generic_file_llseek, > }; > -- Florian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/