| Message ID | 20240617210844.337476-2-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | UFS patches for kernel 6.11 | expand |
Hi Bart, > Instead of first zero-initializing struct uic_command and next initializing > it memberwise, initialize all members at once. > > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > drivers/ufs/core/ufshcd.c 62 ++++++++++++++++++++------------------- > include/ufs/ufshcd.h 4 +-- > 2 files changed, 34 insertions(+), 32 deletions(-) > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > index 41bf2e249c83..5d784876513e 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -3993,11 +3993,11 @@ static void ufshcd_host_memory_configure(struct ufs_hba *hba) > */ > static int ufshcd_dme_link_startup(struct ufs_hba *hba) > { > - struct uic_command uic_cmd = {0}; > + struct uic_command uic_cmd = { > + .command = UIC_CMD_DME_LINK_STARTUP, > + }; > int ret; > > - uic_cmd.command = UIC_CMD_DME_LINK_STARTUP; > - > ret = ufshcd_send_uic_cmd(hba, &uic_cmd); > if (ret) > dev_dbg(hba->dev, > @@ -4015,11 +4015,11 @@ static int ufshcd_dme_link_startup(struct ufs_hba *hba) > */ > static int ufshcd_dme_reset(struct ufs_hba *hba) > { > - struct uic_command uic_cmd = {0}; > + struct uic_command uic_cmd = { > + .command = UIC_CMD_DME_RESET, > + }; > int ret; > > - uic_cmd.command = UIC_CMD_DME_RESET; > - > ret = ufshcd_send_uic_cmd(hba, &uic_cmd); > if (ret) > dev_err(hba->dev, > @@ -4054,11 +4054,11 @@ EXPORT_SYMBOL_GPL(ufshcd_dme_configure_adapt); > */ > static int ufshcd_dme_enable(struct ufs_hba *hba) > { > - struct uic_command uic_cmd = {0}; > + struct uic_command uic_cmd = { > + .command = UIC_CMD_DME_ENABLE, > + }; > int ret; > > - uic_cmd.command = UIC_CMD_DME_ENABLE; > - > ret = ufshcd_send_uic_cmd(hba, &uic_cmd); > if (ret) > dev_err(hba->dev, > @@ -4111,7 +4111,12 @@ static inline void ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba) > int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel, > u8 attr_set, u32 mib_val, u8 peer) > { > - struct uic_command uic_cmd = {0}; > + struct uic_command uic_cmd = { > + .command = peer ? UIC_CMD_DME_PEER_SET : UIC_CMD_DME_SET, > + .argument1 = attr_sel, > + .argument2 = UIC_ARG_ATTR_TYPE(attr_set), > + .argument3 = mib_val, > + }; > static const char *const action[] = { > "dme-set", > "dme-peer-set" > @@ -4120,12 +4125,6 @@ int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel, > int ret; > int retries = UFS_UIC_COMMAND_RETRIES; > > - uic_cmd.command = peer ? > - UIC_CMD_DME_PEER_SET : UIC_CMD_DME_SET; > - uic_cmd.argument1 = attr_sel; > - uic_cmd.argument2 = UIC_ARG_ATTR_TYPE(attr_set); > - uic_cmd.argument3 = mib_val; > - > do { > /* for peer attributes we retry upon failure */ > ret = ufshcd_send_uic_cmd(hba, &uic_cmd); > @@ -4155,7 +4154,11 @@ EXPORT_SYMBOL_GPL(ufshcd_dme_set_attr); > int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel, > u32 *mib_val, u8 peer) > { > - struct uic_command uic_cmd = {0}; > + struct uic_command uic_cmd = { > + .command = peer ? UIC_CMD_DME_PEER_GET : UIC_CMD_DME_GET, > + .argument1 = attr_sel, > + Empty line. > + }; > static const char *const action[] = { > "dme-get", > "dme-peer-get" > @@ -4189,10 +4192,6 @@ int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel, > } > } > > - uic_cmd.command = peer ? > - UIC_CMD_DME_PEER_GET : UIC_CMD_DME_GET; > - uic_cmd.argument1 = attr_sel; > - > do { > /* for peer attributes we retry upon failure */ > ret = ufshcd_send_uic_cmd(hba, &uic_cmd); > @@ -4325,7 +4324,11 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba, struct uic_command *cmd) > */ > int ufshcd_uic_change_pwr_mode(struct ufs_hba *hba, u8 mode) > { > - struct uic_command uic_cmd = {0}; > + struct uic_command uic_cmd = { > + .command = UIC_CMD_DME_SET, > + .argument1 = UIC_ARG_MIB(PA_PWRMODE), > + .argument3 = mode, > + }; > int ret; > > if (hba->quirks & UFSHCD_QUIRK_BROKEN_PA_RXHSUNTERMCAP) { > @@ -4338,9 +4341,6 @@ int ufshcd_uic_change_pwr_mode(struct ufs_hba *hba, u8 mode) > } > } > > - uic_cmd.command = UIC_CMD_DME_SET; > - uic_cmd.argument1 = UIC_ARG_MIB(PA_PWRMODE); > - uic_cmd.argument3 = mode; > ufshcd_hold(hba); > ret = ufshcd_uic_pwr_ctrl(hba, &uic_cmd); > ufshcd_release(hba); > @@ -4381,13 +4381,14 @@ EXPORT_SYMBOL_GPL(ufshcd_link_recovery); > > int ufshcd_uic_hibern8_enter(struct ufs_hba *hba) > { > - int ret; > - struct uic_command uic_cmd = {0}; > + struct uic_command uic_cmd = { > + .command = UIC_CMD_DME_HIBER_ENTER, > + }; > ktime_t start = ktime_get(); > + int ret; > > ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_ENTER, PRE_CHANGE); > > - uic_cmd.command = UIC_CMD_DME_HIBER_ENTER; > ret = ufshcd_uic_pwr_ctrl(hba, &uic_cmd); > trace_ufshcd_profile_hibern8(dev_name(hba->dev), "enter", > ktime_to_us(ktime_sub(ktime_get(), start)), ret); > @@ -4405,13 +4406,14 @@ EXPORT_SYMBOL_GPL(ufshcd_uic_hibern8_enter); > > int ufshcd_uic_hibern8_exit(struct ufs_hba *hba) > { > - struct uic_command uic_cmd = {0}; > + struct uic_command uic_cmd = { > + .command = UIC_CMD_DME_HIBER_EXIT, > + }; > int ret; > ktime_t start = ktime_get(); > > ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_EXIT, PRE_CHANGE); > > - uic_cmd.command = UIC_CMD_DME_HIBER_EXIT; > ret = ufshcd_uic_pwr_ctrl(hba, &uic_cmd); > trace_ufshcd_profile_hibern8(dev_name(hba->dev), "exit", > ktime_to_us(ktime_sub(ktime_get(), start)), ret); > diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h > index 9e0581115b34..d4d63507d090 100644 > --- a/include/ufs/ufshcd.h > +++ b/include/ufs/ufshcd.h > @@ -73,8 +73,8 @@ enum ufs_event_type { > * @done: UIC command completion > */ > struct uic_command { > - u32 command; > - u32 argument1; > + const u32 command; > + const u32 argument1; > u32 argument2; > u32 argument3; > int cmd_active; Reviewed-by: Daejun Park <daejun7.park@samsung.com> Thanks, Daejun
> Instead of first zero-initializing struct uic_command and next initializing > it memberwise, initialize all members at once. > > Signed-off-by: Bart Van Assche <bvanassche@acm.org> Reviewed-by: Avri Altman <avri.altman@wdc.com>
On 6/17/24 6:25 PM, Daejun Park wrote: >> @@ -4155,7 +4154,11 @@ EXPORT_SYMBOL_GPL(ufshcd_dme_set_attr); >> int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel, >> u32 *mib_val, u8 peer) >> { >> - struct uic_command uic_cmd = {0}; >> + struct uic_command uic_cmd = { >> + .command = peer ? UIC_CMD_DME_PEER_GET : UIC_CMD_DME_GET, >> + .argument1 = attr_sel, >> + > Empty line. Thanks for the feedback. I will remove this empty line before I repost this patch series. Bart.
On Mon, Jun 17, 2024 at 02:07:40PM -0700, Bart Van Assche wrote: > Instead of first zero-initializing struct uic_command and next initializing > it memberwise, initialize all members at once. > > Signed-off-by: Bart Van Assche <bvanassche@acm.org> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> - Mani > --- > drivers/ufs/core/ufshcd.c | 62 ++++++++++++++++++++------------------- > include/ufs/ufshcd.h | 4 +-- > 2 files changed, 34 insertions(+), 32 deletions(-) > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > index 41bf2e249c83..5d784876513e 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -3993,11 +3993,11 @@ static void ufshcd_host_memory_configure(struct ufs_hba *hba) > */ > static int ufshcd_dme_link_startup(struct ufs_hba *hba) > { > - struct uic_command uic_cmd = {0}; > + struct uic_command uic_cmd = { > + .command = UIC_CMD_DME_LINK_STARTUP, > + }; > int ret; > > - uic_cmd.command = UIC_CMD_DME_LINK_STARTUP; > - > ret = ufshcd_send_uic_cmd(hba, &uic_cmd); > if (ret) > dev_dbg(hba->dev, > @@ -4015,11 +4015,11 @@ static int ufshcd_dme_link_startup(struct ufs_hba *hba) > */ > static int ufshcd_dme_reset(struct ufs_hba *hba) > { > - struct uic_command uic_cmd = {0}; > + struct uic_command uic_cmd = { > + .command = UIC_CMD_DME_RESET, > + }; > int ret; > > - uic_cmd.command = UIC_CMD_DME_RESET; > - > ret = ufshcd_send_uic_cmd(hba, &uic_cmd); > if (ret) > dev_err(hba->dev, > @@ -4054,11 +4054,11 @@ EXPORT_SYMBOL_GPL(ufshcd_dme_configure_adapt); > */ > static int ufshcd_dme_enable(struct ufs_hba *hba) > { > - struct uic_command uic_cmd = {0}; > + struct uic_command uic_cmd = { > + .command = UIC_CMD_DME_ENABLE, > + }; > int ret; > > - uic_cmd.command = UIC_CMD_DME_ENABLE; > - > ret = ufshcd_send_uic_cmd(hba, &uic_cmd); > if (ret) > dev_err(hba->dev, > @@ -4111,7 +4111,12 @@ static inline void ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba) > int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel, > u8 attr_set, u32 mib_val, u8 peer) > { > - struct uic_command uic_cmd = {0}; > + struct uic_command uic_cmd = { > + .command = peer ? UIC_CMD_DME_PEER_SET : UIC_CMD_DME_SET, > + .argument1 = attr_sel, > + .argument2 = UIC_ARG_ATTR_TYPE(attr_set), > + .argument3 = mib_val, > + }; > static const char *const action[] = { > "dme-set", > "dme-peer-set" > @@ -4120,12 +4125,6 @@ int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel, > int ret; > int retries = UFS_UIC_COMMAND_RETRIES; > > - uic_cmd.command = peer ? > - UIC_CMD_DME_PEER_SET : UIC_CMD_DME_SET; > - uic_cmd.argument1 = attr_sel; > - uic_cmd.argument2 = UIC_ARG_ATTR_TYPE(attr_set); > - uic_cmd.argument3 = mib_val; > - > do { > /* for peer attributes we retry upon failure */ > ret = ufshcd_send_uic_cmd(hba, &uic_cmd); > @@ -4155,7 +4154,11 @@ EXPORT_SYMBOL_GPL(ufshcd_dme_set_attr); > int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel, > u32 *mib_val, u8 peer) > { > - struct uic_command uic_cmd = {0}; > + struct uic_command uic_cmd = { > + .command = peer ? UIC_CMD_DME_PEER_GET : UIC_CMD_DME_GET, > + .argument1 = attr_sel, > + > + }; > static const char *const action[] = { > "dme-get", > "dme-peer-get" > @@ -4189,10 +4192,6 @@ int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel, > } > } > > - uic_cmd.command = peer ? > - UIC_CMD_DME_PEER_GET : UIC_CMD_DME_GET; > - uic_cmd.argument1 = attr_sel; > - > do { > /* for peer attributes we retry upon failure */ > ret = ufshcd_send_uic_cmd(hba, &uic_cmd); > @@ -4325,7 +4324,11 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba, struct uic_command *cmd) > */ > int ufshcd_uic_change_pwr_mode(struct ufs_hba *hba, u8 mode) > { > - struct uic_command uic_cmd = {0}; > + struct uic_command uic_cmd = { > + .command = UIC_CMD_DME_SET, > + .argument1 = UIC_ARG_MIB(PA_PWRMODE), > + .argument3 = mode, > + }; > int ret; > > if (hba->quirks & UFSHCD_QUIRK_BROKEN_PA_RXHSUNTERMCAP) { > @@ -4338,9 +4341,6 @@ int ufshcd_uic_change_pwr_mode(struct ufs_hba *hba, u8 mode) > } > } > > - uic_cmd.command = UIC_CMD_DME_SET; > - uic_cmd.argument1 = UIC_ARG_MIB(PA_PWRMODE); > - uic_cmd.argument3 = mode; > ufshcd_hold(hba); > ret = ufshcd_uic_pwr_ctrl(hba, &uic_cmd); > ufshcd_release(hba); > @@ -4381,13 +4381,14 @@ EXPORT_SYMBOL_GPL(ufshcd_link_recovery); > > int ufshcd_uic_hibern8_enter(struct ufs_hba *hba) > { > - int ret; > - struct uic_command uic_cmd = {0}; > + struct uic_command uic_cmd = { > + .command = UIC_CMD_DME_HIBER_ENTER, > + }; > ktime_t start = ktime_get(); > + int ret; > > ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_ENTER, PRE_CHANGE); > > - uic_cmd.command = UIC_CMD_DME_HIBER_ENTER; > ret = ufshcd_uic_pwr_ctrl(hba, &uic_cmd); > trace_ufshcd_profile_hibern8(dev_name(hba->dev), "enter", > ktime_to_us(ktime_sub(ktime_get(), start)), ret); > @@ -4405,13 +4406,14 @@ EXPORT_SYMBOL_GPL(ufshcd_uic_hibern8_enter); > > int ufshcd_uic_hibern8_exit(struct ufs_hba *hba) > { > - struct uic_command uic_cmd = {0}; > + struct uic_command uic_cmd = { > + .command = UIC_CMD_DME_HIBER_EXIT, > + }; > int ret; > ktime_t start = ktime_get(); > > ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_EXIT, PRE_CHANGE); > > - uic_cmd.command = UIC_CMD_DME_HIBER_EXIT; > ret = ufshcd_uic_pwr_ctrl(hba, &uic_cmd); > trace_ufshcd_profile_hibern8(dev_name(hba->dev), "exit", > ktime_to_us(ktime_sub(ktime_get(), start)), ret); > diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h > index 9e0581115b34..d4d63507d090 100644 > --- a/include/ufs/ufshcd.h > +++ b/include/ufs/ufshcd.h > @@ -73,8 +73,8 @@ enum ufs_event_type { > * @done: UIC command completion > */ > struct uic_command { > - u32 command; > - u32 argument1; > + const u32 command; > + const u32 argument1; > u32 argument2; > u32 argument3; > int cmd_active;
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 41bf2e249c83..5d784876513e 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -3993,11 +3993,11 @@ static void ufshcd_host_memory_configure(struct ufs_hba *hba) */ static int ufshcd_dme_link_startup(struct ufs_hba *hba) { - struct uic_command uic_cmd = {0}; + struct uic_command uic_cmd = { + .command = UIC_CMD_DME_LINK_STARTUP, + }; int ret; - uic_cmd.command = UIC_CMD_DME_LINK_STARTUP; - ret = ufshcd_send_uic_cmd(hba, &uic_cmd); if (ret) dev_dbg(hba->dev, @@ -4015,11 +4015,11 @@ static int ufshcd_dme_link_startup(struct ufs_hba *hba) */ static int ufshcd_dme_reset(struct ufs_hba *hba) { - struct uic_command uic_cmd = {0}; + struct uic_command uic_cmd = { + .command = UIC_CMD_DME_RESET, + }; int ret; - uic_cmd.command = UIC_CMD_DME_RESET; - ret = ufshcd_send_uic_cmd(hba, &uic_cmd); if (ret) dev_err(hba->dev, @@ -4054,11 +4054,11 @@ EXPORT_SYMBOL_GPL(ufshcd_dme_configure_adapt); */ static int ufshcd_dme_enable(struct ufs_hba *hba) { - struct uic_command uic_cmd = {0}; + struct uic_command uic_cmd = { + .command = UIC_CMD_DME_ENABLE, + }; int ret; - uic_cmd.command = UIC_CMD_DME_ENABLE; - ret = ufshcd_send_uic_cmd(hba, &uic_cmd); if (ret) dev_err(hba->dev, @@ -4111,7 +4111,12 @@ static inline void ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba) int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel, u8 attr_set, u32 mib_val, u8 peer) { - struct uic_command uic_cmd = {0}; + struct uic_command uic_cmd = { + .command = peer ? UIC_CMD_DME_PEER_SET : UIC_CMD_DME_SET, + .argument1 = attr_sel, + .argument2 = UIC_ARG_ATTR_TYPE(attr_set), + .argument3 = mib_val, + }; static const char *const action[] = { "dme-set", "dme-peer-set" @@ -4120,12 +4125,6 @@ int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel, int ret; int retries = UFS_UIC_COMMAND_RETRIES; - uic_cmd.command = peer ? - UIC_CMD_DME_PEER_SET : UIC_CMD_DME_SET; - uic_cmd.argument1 = attr_sel; - uic_cmd.argument2 = UIC_ARG_ATTR_TYPE(attr_set); - uic_cmd.argument3 = mib_val; - do { /* for peer attributes we retry upon failure */ ret = ufshcd_send_uic_cmd(hba, &uic_cmd); @@ -4155,7 +4154,11 @@ EXPORT_SYMBOL_GPL(ufshcd_dme_set_attr); int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel, u32 *mib_val, u8 peer) { - struct uic_command uic_cmd = {0}; + struct uic_command uic_cmd = { + .command = peer ? UIC_CMD_DME_PEER_GET : UIC_CMD_DME_GET, + .argument1 = attr_sel, + + }; static const char *const action[] = { "dme-get", "dme-peer-get" @@ -4189,10 +4192,6 @@ int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel, } } - uic_cmd.command = peer ? - UIC_CMD_DME_PEER_GET : UIC_CMD_DME_GET; - uic_cmd.argument1 = attr_sel; - do { /* for peer attributes we retry upon failure */ ret = ufshcd_send_uic_cmd(hba, &uic_cmd); @@ -4325,7 +4324,11 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba, struct uic_command *cmd) */ int ufshcd_uic_change_pwr_mode(struct ufs_hba *hba, u8 mode) { - struct uic_command uic_cmd = {0}; + struct uic_command uic_cmd = { + .command = UIC_CMD_DME_SET, + .argument1 = UIC_ARG_MIB(PA_PWRMODE), + .argument3 = mode, + }; int ret; if (hba->quirks & UFSHCD_QUIRK_BROKEN_PA_RXHSUNTERMCAP) { @@ -4338,9 +4341,6 @@ int ufshcd_uic_change_pwr_mode(struct ufs_hba *hba, u8 mode) } } - uic_cmd.command = UIC_CMD_DME_SET; - uic_cmd.argument1 = UIC_ARG_MIB(PA_PWRMODE); - uic_cmd.argument3 = mode; ufshcd_hold(hba); ret = ufshcd_uic_pwr_ctrl(hba, &uic_cmd); ufshcd_release(hba); @@ -4381,13 +4381,14 @@ EXPORT_SYMBOL_GPL(ufshcd_link_recovery); int ufshcd_uic_hibern8_enter(struct ufs_hba *hba) { - int ret; - struct uic_command uic_cmd = {0}; + struct uic_command uic_cmd = { + .command = UIC_CMD_DME_HIBER_ENTER, + }; ktime_t start = ktime_get(); + int ret; ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_ENTER, PRE_CHANGE); - uic_cmd.command = UIC_CMD_DME_HIBER_ENTER; ret = ufshcd_uic_pwr_ctrl(hba, &uic_cmd); trace_ufshcd_profile_hibern8(dev_name(hba->dev), "enter", ktime_to_us(ktime_sub(ktime_get(), start)), ret); @@ -4405,13 +4406,14 @@ EXPORT_SYMBOL_GPL(ufshcd_uic_hibern8_enter); int ufshcd_uic_hibern8_exit(struct ufs_hba *hba) { - struct uic_command uic_cmd = {0}; + struct uic_command uic_cmd = { + .command = UIC_CMD_DME_HIBER_EXIT, + }; int ret; ktime_t start = ktime_get(); ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_EXIT, PRE_CHANGE); - uic_cmd.command = UIC_CMD_DME_HIBER_EXIT; ret = ufshcd_uic_pwr_ctrl(hba, &uic_cmd); trace_ufshcd_profile_hibern8(dev_name(hba->dev), "exit", ktime_to_us(ktime_sub(ktime_get(), start)), ret); diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h index 9e0581115b34..d4d63507d090 100644 --- a/include/ufs/ufshcd.h +++ b/include/ufs/ufshcd.h @@ -73,8 +73,8 @@ enum ufs_event_type { * @done: UIC command completion */ struct uic_command { - u32 command; - u32 argument1; + const u32 command; + const u32 argument1; u32 argument2; u32 argument3; int cmd_active;
Instead of first zero-initializing struct uic_command and next initializing it memberwise, initialize all members at once. Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- drivers/ufs/core/ufshcd.c | 62 ++++++++++++++++++++------------------- include/ufs/ufshcd.h | 4 +-- 2 files changed, 34 insertions(+), 32 deletions(-)