On Fri, Jun 30, 2023 at 8:25 AM Pavan Kondeti <quic_pkondeti@xxxxxxxxxxx> wrote: > On Wed, Jun 28, 2023 at 06:04:47PM +0530, Mukesh Ojha wrote: ... > > +static int set_download_mode(const char *val, const struct kernel_param *kp) > > +{ > > + u32 old = download_mode; > > + int ret; > > + > > + ret = sysfs_match_string(download_mode_name, val); > > + if (ret < 0) { > > + download_mode = old; Why is this old variable needed at all? > > + pr_err("qcom_scm: unknown download mode: %s\n", val); > > + return -EINVAL; > > + } > > minor nit: %s/-EINVAL/ret > > > + download_mode = ret; > > + if (__scm) > > + qcom_scm_set_download_mode(download_mode); > > + > > + return 0; > > +} -- With Best Regards, Andy Shevchenko