Re: [PATCH 1/8] scsi: ufs: Initialize struct uic_command once

Manivannan Sadhasivam
2024-06-19T06:55:27Z

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;

--
மணிவண்ணன் சதாசிவம்