RE: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware

["Kweh, Hock Leong" <hock.leong.kweh@intel.com>]

> -----Original Message-----
> From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> Sent: Tuesday, April 14, 2015 10:09 PM
> 
> On Tue, Apr 14, 2015 at 05:44:56PM +0800, Kweh, Hock Leong wrote:
> > From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> >
> > Introducing a kernel module to expose capsule loader interface
> > for user to upload capsule binaries. This module leverage the
> > request_firmware_direct_full_path() to obtain the binary at a
> > specific path input by user.
> >
> > Example method to load the capsule binary:
> > echo -n "/path/to/capsule/binary" >
> /sys/devices/platform/efi_capsule_loader/capsule_loader
> 
> Ick, why not just have the firmware file location present, and copy it
> to the sysfs file directly from userspace, instead of this two-step
> process?

Err .... I may not catch your meaning correctly. Are you trying to say
that you would prefer the user to perform:

cat file.bin > /sys/.../capsule_loader

instead of

echo -n "/path/to/binary" > /sys/..../capsule_laoder


The reason we stick with the firmware_class is because we don't
want to replicate a code which already mature and has open API
for developer to use.

> 
> > +/*
> > + * To remove this kernel module, just perform:
> > + * rmmod efi_capsule_loader.ko
> 
> This comment is not needed.

Okay, I will remove this.

> 
> 
> > + */
> > +static void __exit efi_capsule_loader_exit(void)
> > +{
> > +	platform_device_unregister(efi_capsule_pdev);
> 
> This is not a platform device, don't abuse that interface please.
> 
> greg k-h

Okay, so you would recommend to use device_register() for this case?
Or you would think that this is more suitable to use class_register()?


Thanks & Regards,
Wilson