On Wed, 2015-04-29 at 16:25 -0700, Andy Lutomirski wrote: > On Wed, Apr 29, 2015 at 4:12 PM, James Bottomley > <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote: > > From: James Bottomley <JBottomley@xxxxxxxx> > > > > The firmware update should be applied simply by doing > > > > cat fw_file > /sys/firmware/capsule/update > > > > With a properly formatted fw_file. Any error will be returned on close of > > stdout. util-linux returns errors correctly from closing stdout, but firmware > > shippers should check whatever utilities package they use correctly captures > > the error return on close. > > s/util-linux/coreutils/ > > This still makes my API sense itch. It's kind of an abuse of > open/write/close. It works ... and according to Alan, NFS is already doing it. I suppose we can have a do over of the whole debate again ... > > > > Signed-off-by: James Bottomley <JBottomley@xxxxxxxx> > > --- > > drivers/firmware/efi/Makefile | 2 +- > > drivers/firmware/efi/capsule.c | 78 ++++++++++++++++++++++++++++++++++++++++++ > > drivers/firmware/efi/capsule.h | 2 ++ > > drivers/firmware/efi/efi.c | 8 +++++ > > 4 files changed, 89 insertions(+), 1 deletion(-) > > create mode 100644 drivers/firmware/efi/capsule.c > > create mode 100644 drivers/firmware/efi/capsule.h > > > > diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile > > index d8be608..698846e 100644 > > --- a/drivers/firmware/efi/Makefile > > +++ b/drivers/firmware/efi/Makefile > > @@ -1,7 +1,7 @@ > > # > > # Makefile for linux kernel > > # > > -obj-$(CONFIG_EFI) += efi.o vars.o reboot.o > > +obj-$(CONFIG_EFI) += efi.o vars.o reboot.o capsule.o > > obj-$(CONFIG_EFI_VARS) += efivars.o > > obj-$(CONFIG_EFI_VARS_PSTORE) += efi-pstore.o > > obj-$(CONFIG_UEFI_CPER) += cper.o > > diff --git a/drivers/firmware/efi/capsule.c b/drivers/firmware/efi/capsule.c > > new file mode 100644 > > index 0000000..1fd78e7 > > --- /dev/null > > +++ b/drivers/firmware/efi/capsule.c > > @@ -0,0 +1,78 @@ > > +#include <linux/efi.h> > > +#include <linux/slab.h> > > +#include <linux/transaction_helper.h> > > + > > +#include "capsule.h" > > + > > +static struct kset *capsule_kset; > > +static struct transaction_buf *capsule_buf; > > + > > +static int capsule_data_write(struct file *file, struct kobject *kobj, > > + struct bin_attribute *attr, > > + char *buffer, loff_t offset, size_t count) > > +{ > > + if (!capsule_buf) { > > + capsule_buf = kmalloc(sizeof(*capsule_buf), GFP_KERNEL); > > + if (!capsule_buf) > > + return -ENOMEM; > > + transaction_init(capsule_buf); > > + } > > + > > + return transaction_write(capsule_buf, buffer, offset, count); > > +} > > This seems unlikely to have good effects if two struct files are > active at once. I thought of threading ->open and using that to make it exclusive. But then I thought caveat emptor. I think for multiple files, I need a mutex in the transaction code just to ensure orderly access. > Also, I think you crash if you open and close without calling write, yes there should be an if (!capsule_buf) return -EINVAL in flush > and I don't know what whether there can be spurious flushes (fsync?). It turns out that the bdi flusher and the fop->flush() operation are totally different things. ->flush() is used mostly just to do stuff on close (NFS uses it to tidy up for instance). James -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html