| Message ID | 1476193185-32107-4-git-send-email-matt.redfearn@imgtec.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
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 <matt.redfearn@imgtec.com> > > --- > > 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 <matt.redfearn@imgtec.com> > +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 <matt.redfearn@imgtec.com> > +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 <linux/remoteproc.h> > + > +#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); > +} > -- To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Loic, On 13/10/16 14:56, loic pallardy wrote: > > > 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 <matt.redfearn@imgtec.com> >> >> --- >> >> 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 <matt.redfearn@imgtec.com> >> +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 <matt.redfearn@imgtec.com> >> +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. It would be nice if that were possible, but since the class has to associated with the devices created within remoteproc_core, as it stands we must have visibility of this symbol there, see above the change to drivers/remoteproc/remoteproc_core.c line 1455. The alternative would be a utility function in remoteproc_sysfs.c to associate the class with an rproc device, it depends what the preference would be. Thanks, Matt > >> +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 <linux/remoteproc.h> >> + >> +#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); >> +} >> -- To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/13/2016 04:25 PM, Matt Redfearn wrote: > Hi Loic, > > > On 13/10/16 14:56, loic pallardy wrote: >> >> >> 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 <matt.redfearn@imgtec.com> >>> >>> --- >>> >>> 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 <matt.redfearn@imgtec.com> >>> +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 <matt.redfearn@imgtec.com> >>> +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. > > It would be nice if that were possible, but since the class has to > associated with the devices created within remoteproc_core, as it stands > we must have visibility of this symbol there, see above the change to > drivers/remoteproc/remoteproc_core.c line 1455. > The alternative would be a utility function in remoteproc_sysfs.c to > associate the class with an rproc device, it depends what the preference > would be. Yes it will be my preference. remoteproc_sysfs.c file to provide a new service like: void rproc_register_sysfs(struct rproc *rproc) { rproc->dev.class = &rproc_class; } and to call this function from rproc_alloc /Loic > > Thanks, > Matt > >> >>> +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 <linux/remoteproc.h> >>> + >>> +#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); >>> +} >>> > -- To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Loic On 13/10/16 15:39, loic pallardy wrote: > > > On 10/13/2016 04:25 PM, Matt Redfearn wrote: >> Hi Loic, >> >> >> On 13/10/16 14:56, loic pallardy wrote: >>> >>> >>> 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 <matt.redfearn@imgtec.com> >>>> >>>> --- >>>> >>>> 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 <matt.redfearn@imgtec.com> >>>> +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 <matt.redfearn@imgtec.com> >>>> +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. >> >> It would be nice if that were possible, but since the class has to >> associated with the devices created within remoteproc_core, as it stands >> we must have visibility of this symbol there, see above the change to >> drivers/remoteproc/remoteproc_core.c line 1455. >> The alternative would be a utility function in remoteproc_sysfs.c to >> associate the class with an rproc device, it depends what the preference >> would be. > > Yes it will be my preference. remoteproc_sysfs.c file to provide a new > service like: > void rproc_register_sysfs(struct rproc *rproc) { > rproc->dev.class = &rproc_class; > } > > and to call this function from rproc_alloc > > /Loic OK, no problem - that is probably neater :-) Thanks, Matt >> >> Thanks, >> Matt >> >>> >>>> +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 <linux/remoteproc.h> >>>> + >>>> +#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); >>>> +} >>>> >> -- To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue 11 Oct 06:39 PDT 2016, 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 <matt.redfearn@imgtec.com> > > --- > > 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 <matt.redfearn@imgtec.com> > +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 <matt.redfearn@imgtec.com> > +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; > +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 <linux/remoteproc.h> > + > +#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"); Per current and future logic, rproc->firmware can't be NULL. > +} > + > +/* 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); Rather than introducing rproc_set_firmware_name() and rproc_change_firmware() - which does not have a use case beyond this caller you should be able to keep rproc_alloc() more or less as is, and then inline a simplified version (as we don't have to deal with the templated case here). > + return err ? err : count; > +} > +static DEVICE_ATTR_RW(firmware); > + Extra newline... > + > +/* > + * 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); The documentation above does not describe the "($state)" part, which I'm fine leaving out. We know the strings are below PAGE_SIZE, so you can use sprintf(). > +} > + > +/* 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)) { Please use sysfs_streq() instead, it will handle the \n for you. > + 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 = { > + .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); > +} Besides these nits, I think this looks good. Thanks, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu 13 Oct 07:39 PDT 2016, loic pallardy wrote: > > > On 10/13/2016 04:25 PM, Matt Redfearn wrote: > >Hi Loic, > > > > > >On 13/10/16 14:56, loic pallardy wrote: > >> > >> > >>On 10/11/2016 03:39 PM, Matt Redfearn wrote: [..] > >>>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. > > > >It would be nice if that were possible, but since the class has to > >associated with the devices created within remoteproc_core, as it stands > >we must have visibility of this symbol there, see above the change to > >drivers/remoteproc/remoteproc_core.c line 1455. > >The alternative would be a utility function in remoteproc_sysfs.c to > >associate the class with an rproc device, it depends what the preference > >would be. > > Yes it will be my preference. remoteproc_sysfs.c file to provide a new > service like: > void rproc_register_sysfs(struct rproc *rproc) { > rproc->dev.class = &rproc_class; > } > > and to call this function from rproc_alloc > Compare: rproc->dev.class = &rproc_class; to: rproc_register_sysfs(rproc); The meaning of the prior is obvious, the latter require the jump to a different file to follow. And we would only be trading one global variable for a global function. A viable alternative would be to keep a local pointer to the class in core.c, that we assign during the call to rproc_init_sysfs(). But I'm fine with the way it's written. Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <matt.redfearn@imgtec.com> +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 <matt.redfearn@imgtec.com> +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; +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 <linux/remoteproc.h> + +#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 = { + .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); +}
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 <matt.redfearn@imgtec.com> --- 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