From mboxrd@z Thu Jan 1 00:00:00 1970 From: lee.jones@linaro.org (Lee Jones) Date: Thu, 27 Aug 2015 07:44:52 +0100 Subject: [PATCH 4/4] remoteproc: debugfs: Add ability to boot remote processor using debugfs In-Reply-To: <55DDF2A0.5010404@mentor.com> References: <1440594483-29601-1-git-send-email-lee.jones@linaro.org> <1440594483-29601-5-git-send-email-lee.jones@linaro.org> <55DDF2A0.5010404@mentor.com> Message-ID: <20150827064452.GC19409@x1> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wed, 26 Aug 2015, Nathan Lynch wrote: > On 08/26/2015 08:08 AM, Lee Jones wrote: > > Signed-off-by: Lee Jones > > --- > > drivers/remoteproc/remoteproc_debugfs.c | 25 +++++++++++++++++++++++++ > > 1 file changed, 25 insertions(+) > > The commit message should describe why this is needed... > > > diff --git a/drivers/remoteproc/remoteproc_debugfs.c b/drivers/remoteproc/remoteproc_debugfs.c > > index 9d30809..9620962 100644 > > --- a/drivers/remoteproc/remoteproc_debugfs.c > > +++ b/drivers/remoteproc/remoteproc_debugfs.c > > @@ -88,8 +88,33 @@ 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 '1': > > + ret = rproc_boot(rproc); > > + if (ret) > > + dev_warn(&rproc->dev, "Boot failed: %d\n", ret); > > + break; > > + default: > > + rproc_shutdown(rproc); > > + } > > + > > + return count; > > +} > > ... and I suggest that the user interface be reconsidered. If '1' means > "boot" and literally anything else means "shut down" then you can't add > operations in the future without potentially breaking things. Good points, will fix. -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org ? Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lee Jones Subject: Re: [PATCH 4/4] remoteproc: debugfs: Add ability to boot remote processor using debugfs Date: Thu, 27 Aug 2015 07:44:52 +0100 Message-ID: <20150827064452.GC19409@x1> References: <1440594483-29601-1-git-send-email-lee.jones@linaro.org> <1440594483-29601-5-git-send-email-lee.jones@linaro.org> <55DDF2A0.5010404@mentor.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <55DDF2A0.5010404@mentor.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Nathan Lynch Cc: ohad@wizery.com, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kernel@stlinux.com List-Id: devicetree@vger.kernel.org T24gV2VkLCAyNiBBdWcgMjAxNSwgTmF0aGFuIEx5bmNoIHdyb3RlOgoKPiBPbiAwOC8yNi8yMDE1 IDA4OjA4IEFNLCBMZWUgSm9uZXMgd3JvdGU6Cj4gPiBTaWduZWQtb2ZmLWJ5OiBMZWUgSm9uZXMg PGxlZS5qb25lc0BsaW5hcm8ub3JnPgo+ID4gLS0tCj4gPiAgZHJpdmVycy9yZW1vdGVwcm9jL3Jl bW90ZXByb2NfZGVidWdmcy5jIHwgMjUgKysrKysrKysrKysrKysrKysrKysrKysrKwo+ID4gIDEg ZmlsZSBjaGFuZ2VkLCAyNSBpbnNlcnRpb25zKCspCj4gCj4gVGhlIGNvbW1pdCBtZXNzYWdlIHNo b3VsZCBkZXNjcmliZSB3aHkgdGhpcyBpcyBuZWVkZWQuLi4KPiAKPiA+IGRpZmYgLS1naXQgYS9k cml2ZXJzL3JlbW90ZXByb2MvcmVtb3RlcHJvY19kZWJ1Z2ZzLmMgYi9kcml2ZXJzL3JlbW90ZXBy b2MvcmVtb3RlcHJvY19kZWJ1Z2ZzLmMKPiA+IGluZGV4IDlkMzA4MDkuLjk2MjA5NjIgMTAwNjQ0 Cj4gPiAtLS0gYS9kcml2ZXJzL3JlbW90ZXByb2MvcmVtb3RlcHJvY19kZWJ1Z2ZzLmMKPiA+ICsr KyBiL2RyaXZlcnMvcmVtb3RlcHJvYy9yZW1vdGVwcm9jX2RlYnVnZnMuYwo+ID4gQEAgLTg4LDgg Kzg4LDMzIEBAIHN0YXRpYyBzc2l6ZV90IHJwcm9jX3N0YXRlX3JlYWQoc3RydWN0IGZpbGUgKmZp bHAsIGNoYXIgX191c2VyICp1c2VyYnVmLAo+ID4gIAlyZXR1cm4gc2ltcGxlX3JlYWRfZnJvbV9i dWZmZXIodXNlcmJ1ZiwgY291bnQsIHBwb3MsIGJ1ZiwgaSk7Cj4gPiAgfQo+ID4gIAo+ID4gK3N0 YXRpYyBzc2l6ZV90IHJwcm9jX3N0YXRlX3dyaXRlKHN0cnVjdCBmaWxlICpmaWxwLCBjb25zdCBj aGFyIF9fdXNlciAqdXNlcmJ1ZiwKPiA+ICsJCQkJIHNpemVfdCBjb3VudCwgbG9mZl90ICpwcG9z KQo+ID4gK3sKPiA+ICsJc3RydWN0IHJwcm9jICpycHJvYyA9IGZpbHAtPnByaXZhdGVfZGF0YTsK PiA+ICsJY2hhciBidWZbMl07Cj4gPiArCWludCByZXQ7Cj4gPiArCj4gPiArCXJldCA9IGNvcHlf ZnJvbV91c2VyKGJ1ZiwgdXNlcmJ1ZiwgMSk7Cj4gPiArCWlmIChyZXQpCj4gPiArCQlyZXR1cm4g LUVGQVVMVDsKPiA+ICsKPiA+ICsJc3dpdGNoIChidWZbMF0pIHsKPiA+ICsJY2FzZSAnMSc6Cj4g PiArCQlyZXQgPSBycHJvY19ib290KHJwcm9jKTsKPiA+ICsJCWlmIChyZXQpCj4gPiArCQkJZGV2 X3dhcm4oJnJwcm9jLT5kZXYsICJCb290IGZhaWxlZDogJWRcbiIsIHJldCk7Cj4gPiArCQlicmVh azsKPiA+ICsJZGVmYXVsdDoKPiA+ICsJCXJwcm9jX3NodXRkb3duKHJwcm9jKTsKPiA+ICsJfQo+ ID4gKwo+ID4gKwlyZXR1cm4gY291bnQ7Cj4gPiArfQo+IAo+IC4uLiBhbmQgSSBzdWdnZXN0IHRo YXQgdGhlIHVzZXIgaW50ZXJmYWNlIGJlIHJlY29uc2lkZXJlZC4gIElmICcxJyBtZWFucwo+ICJi b290IiBhbmQgbGl0ZXJhbGx5IGFueXRoaW5nIGVsc2UgbWVhbnMgInNodXQgZG93biIgdGhlbiB5 b3UgY2FuJ3QgYWRkCj4gb3BlcmF0aW9ucyBpbiB0aGUgZnV0dXJlIHdpdGhvdXQgcG90ZW50aWFs bHkgYnJlYWtpbmcgdGhpbmdzLgoKR29vZCBwb2ludHMsIHdpbGwgZml4LgoKLS0gCkxlZSBKb25l cwpMaW5hcm8gU1RNaWNyb2VsZWN0cm9uaWNzIExhbmRpbmcgVGVhbSBMZWFkCkxpbmFyby5vcmcg 4pSCIE9wZW4gc291cmNlIHNvZnR3YXJlIGZvciBBUk0gU29DcwpGb2xsb3cgTGluYXJvOiBGYWNl Ym9vayB8IFR3aXR0ZXIgfCBCbG9nCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fXwpsaW51eC1hcm0ta2VybmVsIG1haWxpbmcgbGlzdApsaW51eC1hcm0ta2Vy bmVsQGxpc3RzLmluZnJhZGVhZC5vcmcKaHR0cDovL2xpc3RzLmluZnJhZGVhZC5vcmcvbWFpbG1h bi9saXN0aW5mby9saW51eC1hcm0ta2VybmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932339AbbH0Gq1 (ORCPT ); Thu, 27 Aug 2015 02:46:27 -0400 Received: from mail-wi0-f176.google.com ([209.85.212.176]:38277 "EHLO mail-wi0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753549AbbH0Go4 (ORCPT ); Thu, 27 Aug 2015 02:44:56 -0400 Date: Thu, 27 Aug 2015 07:44:52 +0100 From: Lee Jones To: Nathan Lynch Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, ohad@wizery.com, devicetree@vger.kernel.org, kernel@stlinux.com Subject: Re: [PATCH 4/4] remoteproc: debugfs: Add ability to boot remote processor using debugfs Message-ID: <20150827064452.GC19409@x1> References: <1440594483-29601-1-git-send-email-lee.jones@linaro.org> <1440594483-29601-5-git-send-email-lee.jones@linaro.org> <55DDF2A0.5010404@mentor.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <55DDF2A0.5010404@mentor.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 26 Aug 2015, Nathan Lynch wrote: > On 08/26/2015 08:08 AM, Lee Jones wrote: > > Signed-off-by: Lee Jones > > --- > > drivers/remoteproc/remoteproc_debugfs.c | 25 +++++++++++++++++++++++++ > > 1 file changed, 25 insertions(+) > > The commit message should describe why this is needed... > > > diff --git a/drivers/remoteproc/remoteproc_debugfs.c b/drivers/remoteproc/remoteproc_debugfs.c > > index 9d30809..9620962 100644 > > --- a/drivers/remoteproc/remoteproc_debugfs.c > > +++ b/drivers/remoteproc/remoteproc_debugfs.c > > @@ -88,8 +88,33 @@ 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 '1': > > + ret = rproc_boot(rproc); > > + if (ret) > > + dev_warn(&rproc->dev, "Boot failed: %d\n", ret); > > + break; > > + default: > > + rproc_shutdown(rproc); > > + } > > + > > + return count; > > +} > > ... and I suggest that the user interface be reconsidered. If '1' means > "boot" and literally anything else means "shut down" then you can't add > operations in the future without potentially breaking things. Good points, will fix. -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog