| Message ID | 20230426052153epcms2p27d64a865f15bfd452d564f77d63605db@epcms2p2 (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | scsi: ufs: core: Simplify param_set_mcq_mode() | expand |
Keoseong Park <keosung.park@samsung.com> 於 2023年4月26日 週三 下午1:26寫道： > > This function does not require the "ret" variable because it returns > only the result of param_set_bool(). > > Remove unnecessary "ret" variable and simplify the code. > > Signed-off-by: Keoseong Park <keosung.park@samsung.com> > --- > drivers/ufs/core/ufshcd.c | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
> This function does not require the "ret" variable because it returns > only the result of param_set_bool(). > > Remove unnecessary "ret" variable and simplify the code. > > Signed-off-by: Keoseong Park <keosung.park@samsung.com> Reviewed-by: Avri Altman <avri.altman@wdc.com> > --- > drivers/ufs/core/ufshcd.c | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > index 9434328ba323..46c4ed478ad0 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -108,13 +108,7 @@ static bool is_mcq_supported(struct ufs_hba *hba) > > static int param_set_mcq_mode(const char *val, const struct kernel_param > *kp) > { > - int ret; > - > - ret = param_set_bool(val, kp); > - if (ret) > - return ret; > - > - return 0; > + return param_set_bool(val, kp); > } > > static const struct kernel_param_ops mcq_mode_ops = { > -- > 2.17.1
On 4/25/23 22:21, Keoseong Park wrote: > This function does not require the "ret" variable because it returns > only the result of param_set_bool(). > > Remove unnecessary "ret" variable and simplify the code. > > Signed-off-by: Keoseong Park <keosung.park@samsung.com> > --- > drivers/ufs/core/ufshcd.c | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > index 9434328ba323..46c4ed478ad0 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -108,13 +108,7 @@ static bool is_mcq_supported(struct ufs_hba *hba) > > static int param_set_mcq_mode(const char *val, const struct kernel_param *kp) > { > - int ret; > - > - ret = param_set_bool(val, kp); > - if (ret) > - return ret; > - > - return 0; > + return param_set_bool(val, kp); > } > > static const struct kernel_param_ops mcq_mode_ops = { Why do we even have the param_set_mcq_mode() callback? Has it been considered to remove mcq_mode_ops as in the untested patch below? Thanks, Bart. diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 7b1e7d7091ff..2b8c2613f7d7 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -98,7 +98,7 @@ /* Polling time to wait for fDeviceInit */ #define FDEVICEINIT_COMPL_TIMEOUT 1500 /* millisecs */ -/* UFSHC 4.0 compliant HC support this mode, refer param_set_mcq_mode() */ +/* UFSHC 4.0 compliant HC support this mode. */ static bool use_mcq_mode = true; static bool is_mcq_supported(struct ufs_hba *hba) @@ -106,23 +106,7 @@ static bool is_mcq_supported(struct ufs_hba *hba) return hba->mcq_sup && use_mcq_mode; } -static int param_set_mcq_mode(const char *val, const struct kernel_param *kp) -{ - int ret; - - ret = param_set_bool(val, kp); - if (ret) - return ret; - - return 0; -} - -static const struct kernel_param_ops mcq_mode_ops = { - .set = param_set_mcq_mode, - .get = param_get_bool, -}; - -module_param_cb(use_mcq_mode, &mcq_mode_ops, &use_mcq_mode, 0644); +module_param(use_mcq_mode, bool, 0644); MODULE_PARM_DESC(use_mcq_mode, "Control MCQ mode for controllers starting from UFSHCI 4.0. 1 - enable MCQ, 0 - disable MCQ. MCQ is enabled by default"); #define ufshcd_toggle_vreg(_dev, _vreg, _on) \
Hi Bart, Thank you for your review. >On 4/25/23 22:21, Keoseong Park wrote: >> This function does not require the "ret" variable because it returns >> only the result of param_set_bool(). >> >> Remove unnecessary "ret" variable and simplify the code. >> >> Signed-off-by: Keoseong Park <keosung.park@samsung.com> >> --- >> drivers/ufs/core/ufshcd.c | 8 +------- >> 1 file changed, 1 insertion(+), 7 deletions(-) >> >> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c >> index 9434328ba323..46c4ed478ad0 100644 >> --- a/drivers/ufs/core/ufshcd.c >> +++ b/drivers/ufs/core/ufshcd.c >> @@ -108,13 +108,7 @@ static bool is_mcq_supported(struct ufs_hba *hba) >> >> static int param_set_mcq_mode(const char *val, const struct kernel_param *kp) >> { >> - int ret; >> - >> - ret = param_set_bool(val, kp); >> - if (ret) >> - return ret; >> - >> - return 0; >> + return param_set_bool(val, kp); >> } >> >> static const struct kernel_param_ops mcq_mode_ops = { > >Why do we even have the param_set_mcq_mode() callback? Has it been considered >to remove mcq_mode_ops as in the untested patch below? I agree with you in that it only uses param_{set,get}_bool(). So, I'll test the patch below and send it. Best Regards, Keoseong > >Thanks, > >Bart. > >diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c >index 7b1e7d7091ff..2b8c2613f7d7 100644 >--- a/drivers/ufs/core/ufshcd.c >+++ b/drivers/ufs/core/ufshcd.c >@@ -98,7 +98,7 @@ > /* Polling time to wait for fDeviceInit */ > #define FDEVICEINIT_COMPL_TIMEOUT 1500 /* millisecs */ > >-/* UFSHC 4.0 compliant HC support this mode, refer param_set_mcq_mode() */ >+/* UFSHC 4.0 compliant HC support this mode. */ > static bool use_mcq_mode = true; > > static bool is_mcq_supported(struct ufs_hba *hba) >@@ -106,23 +106,7 @@ static bool is_mcq_supported(struct ufs_hba *hba) > return hba->mcq_sup && use_mcq_mode; > } > >-static int param_set_mcq_mode(const char *val, const struct kernel_param *kp) >-{ >- int ret; >- >- ret = param_set_bool(val, kp); >- if (ret) >- return ret; >- >- return 0; >-} >- >-static const struct kernel_param_ops mcq_mode_ops = { >- .set = param_set_mcq_mode, >- .get = param_get_bool, >-}; >- >-module_param_cb(use_mcq_mode, &mcq_mode_ops, &use_mcq_mode, 0644); >+module_param(use_mcq_mode, bool, 0644); > MODULE_PARM_DESC(use_mcq_mode, "Control MCQ mode for controllers starting from UFSHCI 4.0. 1 - enable MCQ, 0 - disable MCQ. MCQ is enabled by default"); > > #define ufshcd_toggle_vreg(_dev, _vreg, _on) \
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 9434328ba323..46c4ed478ad0 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -108,13 +108,7 @@ static bool is_mcq_supported(struct ufs_hba *hba) static int param_set_mcq_mode(const char *val, const struct kernel_param *kp) { - int ret; - - ret = param_set_bool(val, kp); - if (ret) - return ret; - - return 0; + return param_set_bool(val, kp); } static const struct kernel_param_ops mcq_mode_ops = {
This function does not require the "ret" variable because it returns only the result of param_set_bool(). Remove unnecessary "ret" variable and simplify the code. Signed-off-by: Keoseong Park <keosung.park@samsung.com> --- drivers/ufs/core/ufshcd.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)