Messages in this thread | | | From | Andy Shevchenko <> | Date | Thu, 23 Apr 2020 13:21:06 +0300 | Subject | Re: [PATCH 1/1] platform/x86: Add Slim Bootloader firmware update signaling driver |
| |
On Thu, Apr 23, 2020 at 5:08 AM Jithu Joseph <jithu.joseph@intel.com> wrote:
...
> > > +static ssize_t firmware_update_request_store(struct device *dev, > > > + struct > > > device_attribute *attr, > > > + const char *buf, > > > size_t count) > > > +{ > > > + bool val; > > > + int ret; > > > + > > > + ret = kstrtobool(buf, &val); > > > + if (ret) > > > + return ret; > > > + > > > + ret = set_fwu_request(dev, val ? 1 : 0); > > > > Hmm... If you are going to extend this, why not to pass integer > > directly? (And thus take one from user) > > We have also been thinking about extensibility … > > So I will modify the code to allow any u32 input by the user to be > passed down via wmi_set_block(), though 0 and 1 will be the only > inputs documented in the ABI today.( Or did you still mean to error > out if the user input is something other than 0 or 1 ?)
I think the best approach is to allow to parse integer input (kstrtouint() is good enough) and return -ERANGE for values > 1. Also put a comment before that check why is done like this (some justification that interface may be extended in the future or so), and pass integer value directly to set_fwu_request().
-- With Best Regards, Andy Shevchenko
| |