Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932624AbcJMN5Y (ORCPT ); Thu, 13 Oct 2016 09:57:24 -0400 Received: from mx08-00178001.pphosted.com ([91.207.212.93]:60709 "EHLO mx07-00178001.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932518AbcJMN5A (ORCPT ); Thu, 13 Oct 2016 09:57:00 -0400 Subject: Re: [PATCH 3/4] remoteproc: Add a sysfs interface for firmware and state To: Matt Redfearn , Bjorn Andersson , Ohad Ben-Cohen References: <1476193185-32107-1-git-send-email-matt.redfearn@imgtec.com> <1476193185-32107-4-git-send-email-matt.redfearn@imgtec.com> CC: , From: loic pallardy Message-ID: Date: Thu, 13 Oct 2016 15:56:49 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <1476193185-32107-4-git-send-email-matt.redfearn@imgtec.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.201.23.23] X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-10-13_07:,, signatures=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8948 Lines: 283 On 10/11/2016 03:39 PM, Matt Redfearn wrote: > This patch adds a sysfs interface to rproc allowing the firmware name > and processor state to be changed dynamically. > > State was previously available in debugfs, and is replicated here. The > firmware file allows retrieval of the running firmware name, and a new > one to be specified at run time, so long as the remote processor has > been stopped. > > Signed-off-by: Matt Redfearn > > --- > > Documentation/ABI/testing/sysfs-class-remoteproc | 50 +++++++++ > drivers/remoteproc/Makefile | 1 + > drivers/remoteproc/remoteproc_core.c | 3 + > drivers/remoteproc/remoteproc_internal.h | 5 + > drivers/remoteproc/remoteproc_sysfs.c | 132 +++++++++++++++++++++++ > 5 files changed, 191 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-class-remoteproc > create mode 100644 drivers/remoteproc/remoteproc_sysfs.c > > diff --git a/Documentation/ABI/testing/sysfs-class-remoteproc b/Documentation/ABI/testing/sysfs-class-remoteproc > new file mode 100644 > index 000000000000..d188afebc8ba > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-class-remoteproc > @@ -0,0 +1,50 @@ > +What: /sys/class/remoteproc/.../firmware > +Date: October 2016 > +Contact: Matt Redfearn > +Description: Remote processor firmware > + > + Reports the name of the firmware currently loaded to the > + remote processor. > + > + To change the running firmware, ensure the remote processor is > + stopped (using /sys/class/remoteproc/.../state) and write a new filename. > + > +What: /sys/class/remoteproc/.../state > +Date: October 2016 > +Contact: Matt Redfearn > +Description: Remote processor state > + > + Reports the state of the remote processor, which will be one of: > + > + "offline" > + "suspended" > + "running" > + "crashed" > + "invalid" > + > + "offline" means the remote processor is powered off. > + > + "suspended" means that the remote processor is suspended and > + must be woken to receive messages. > + > + "running" is the normal state of an available remote processor > + > + "crashed" indicates that a problem/crash has been detected on > + the remote processor. > + > + "invalid" is returned if the remote processor is in an > + unknown state. > + > + Writing this file controls the state of the remote processor. > + The following states can be written: > + > + "start" > + "stop" > + > + Writing "start" will attempt to start the processor running the > + firmware indicated by, or written to, > + /sys/class/remoteproc/.../firmware. The remote processor should > + transition to "running" state. > + > + Writing "stop" will attempt to halt the remote processor and > + return it to the "offline" state. > diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile > index 92d3758bd15c..666258bd80aa 100644 > --- a/drivers/remoteproc/Makefile > +++ b/drivers/remoteproc/Makefile > @@ -5,6 +5,7 @@ > obj-$(CONFIG_REMOTEPROC) += remoteproc.o > remoteproc-y := remoteproc_core.o > remoteproc-y += remoteproc_debugfs.o > +remoteproc-y += remoteproc_sysfs.o > remoteproc-y += remoteproc_virtio.o > remoteproc-y += remoteproc_elf_loader.o > obj-$(CONFIG_OMAP_REMOTEPROC) += omap_remoteproc.o > diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c > index 2152b484f314..642c5b58fec5 100644 > --- a/drivers/remoteproc/remoteproc_core.c > +++ b/drivers/remoteproc/remoteproc_core.c > @@ -1455,6 +1455,7 @@ struct rproc *rproc_alloc(struct device *dev, const char *name, > device_initialize(&rproc->dev); > rproc->dev.parent = dev; > rproc->dev.type = &rproc_type; > + rproc->dev.class = &rproc_class; > > /* Assign a unique device index and name */ > rproc->index = ida_simple_get(&rproc_dev_index, 0, 0, GFP_KERNEL); > @@ -1580,6 +1581,7 @@ EXPORT_SYMBOL(rproc_report_crash); > > static int __init remoteproc_init(void) > { > + rproc_init_sysfs(); > rproc_init_debugfs(); > > return 0; > @@ -1591,6 +1593,7 @@ static void __exit remoteproc_exit(void) > ida_destroy(&rproc_dev_index); > > rproc_exit_debugfs(); > + rproc_exit_sysfs(); > } > module_exit(remoteproc_exit); > > diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h > index 837faf2677a6..2beb86ddfacc 100644 > --- a/drivers/remoteproc/remoteproc_internal.h > +++ b/drivers/remoteproc/remoteproc_internal.h > @@ -64,6 +64,11 @@ void rproc_create_debug_dir(struct rproc *rproc); > void rproc_init_debugfs(void); > void rproc_exit_debugfs(void); > > +/* from remoteproc_sysfs.c */ > +extern struct class rproc_class; struct class rproc_class should be static to remoteproc_sysfs.c file. It will be better to create a new interface like rproc_register_sysfs to associate rproc_class to rproc device. > +int rproc_init_sysfs(void); > +void rproc_exit_sysfs(void); > + > void rproc_free_vring(struct rproc_vring *rvring); > int rproc_alloc_vring(struct rproc_vdev *rvdev, int i); > > diff --git a/drivers/remoteproc/remoteproc_sysfs.c b/drivers/remoteproc/remoteproc_sysfs.c > new file mode 100644 > index 000000000000..892599863ca6 > --- /dev/null > +++ b/drivers/remoteproc/remoteproc_sysfs.c > @@ -0,0 +1,132 @@ > +/* > + * Remote Processor Framework > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include > + > +#include "remoteproc_internal.h" > + > +#define to_rproc(d) container_of(d, struct rproc, dev) > + > +/* Expose the loaded / running firmware name via sysfs */ > +static ssize_t firmware_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct rproc *rproc = to_rproc(dev); > + > + if (rproc->firmware) > + return sprintf(buf, "%s\n", rproc->firmware); > + else > + return sprintf(buf, "unknown"); > +} > + > +/* Change firmware name via sysfs */ > +static ssize_t firmware_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct rproc *rproc = to_rproc(dev); > + int err; > + > + err = rproc_change_firmware(rproc, buf); > + return err ? err : count; > +} > +static DEVICE_ATTR_RW(firmware); > + > + > +/* > + * A state-to-string lookup table, for exposing a human readable state > + * via sysfs. Always keep in sync with enum rproc_state > + */ > +static const char * const rproc_state_string[] = { > + "offline", > + "suspended", > + "running", > + "crashed", > + "invalid", > +}; > + > +/* Expose the state of the remote processor via sysfs */ > +static ssize_t state_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct rproc *rproc = to_rproc(dev); > + unsigned int state; > + > + state = rproc->state > RPROC_LAST ? RPROC_LAST : rproc->state; > + return scnprintf(buf, 30, "%.28s (%d)\n", > + rproc_state_string[state], rproc->state); > +} > + > +/* Change remote processor state via sysfs */ > +static ssize_t state_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct rproc *rproc = to_rproc(dev); > + int ret = 0; > + int len = count; > + > + if (buf[len - 1] == '\n') > + len--; > + > + if (!strncmp(buf, "start", len)) { > + if (rproc->state == RPROC_RUNNING) > + return -EBUSY; > + > + ret = rproc_boot(rproc); > + if (ret) > + dev_err(&rproc->dev, "Boot failed: %d\n", ret); > + } else if (!strncmp(buf, "stop", len)) { > + rproc_shutdown(rproc); > + } else { > + dev_err(&rproc->dev, "Unrecognised option: %s\n", buf); > + ret = -EINVAL; > + } > + return ret ? ret : count; > +} > +static DEVICE_ATTR_RW(state); > + > +static const struct attribute *rproc_attrs[] = { > + &dev_attr_firmware.attr, > + &dev_attr_state.attr, > + NULL > +}; > + > +static const struct attribute_group rproc_devgroup = { > + .attrs = rproc_attrs > +}; > + > +static const struct attribute_group *rproc_devgroups[] = { > + &rproc_devgroup, > + NULL > +}; > + > +struct class rproc_class = { static ? Regards, Loic > + .name = "remoteproc", > + .dev_groups = rproc_devgroups, > +}; > + > +int __init rproc_init_sysfs(void) > +{ > + /* create remoteproc device class for sysfs */ > + int err = class_register(&rproc_class); > + > + if (err) > + pr_err("remoteproc: unable to register class\n"); > + return err; > +} > + > +void __exit rproc_exit_sysfs(void) > +{ > + class_unregister(&rproc_class); > +} >