public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: Daejun Park <daejun7.park@samsung.com>
To: Bart Van Assche <bvanassche@acm.org>,
	"Martin K . Petersen" <martin.petersen@oracle.com>
Cc: "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	"James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>,
	Avri Altman <avri.altman@wdc.com>,
	Peter Wang <peter.wang@mediatek.com>,
	Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
	Bean Huo <beanhuo@micron.com>, Minwoo Im <minwoo.im@samsung.com>,
	Maramaina Naresh <quic_mnaresh@quicinc.com>,
	Akinobu Mita <akinobu.mita@gmail.com>,
	Daejun Park <daejun7.park@samsung.com>
Subject: RE: [PATCH 1/8] scsi: ufs: Initialize struct uic_command once
Date: Tue, 18 Jun 2024 10:25:10 +0900	[thread overview]
Message-ID: <1891546521.01718675102603.JavaMail.epsvc@epcpadp3> (raw)
In-Reply-To: <20240617210844.337476-2-bvanassche@acm.org>

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

  reply	other threads:[~2024-06-18  1:45 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-17 21:07 [PATCH 0/8] UFS patches for kernel 6.11 Bart Van Assche
2024-06-17 21:07 ` [PATCH 1/8] scsi: ufs: Initialize struct uic_command once Bart Van Assche
2024-06-18  1:25   ` Daejun Park [this message]
2024-06-18 16:15     ` Bart Van Assche
2024-06-18  6:18   ` Avri Altman
2024-06-19  6:55   ` Manivannan Sadhasivam
2024-06-17 21:07 ` [PATCH 2/8] scsi: ufs: Remove two constants Bart Van Assche
2024-06-19  6:58   ` Manivannan Sadhasivam
2024-06-17 21:07 ` [PATCH 3/8] scsi: ufs: Inline ufshcd_mcq_vops_get_hba_mac() Bart Van Assche
2024-06-18  6:23   ` Avri Altman
2024-06-18 16:14     ` Bart Van Assche
2024-06-17 21:07 ` [PATCH 4/8] scsi: ufs: Make .get_hba_mac() optional Bart Van Assche
2024-06-18  1:28   ` Daejun Park
2024-06-18 16:17     ` Bart Van Assche
2024-06-19  7:13   ` Manivannan Sadhasivam
2024-06-19  7:57     ` Manivannan Sadhasivam
2024-06-21  3:32       ` Peter Wang (王信友)
2024-06-23 13:33         ` manivannan.sadhasivam
2024-06-24  8:39           ` Peter Wang (王信友)
2024-06-24 17:30             ` Bart Van Assche
2024-06-17 21:07 ` [PATCH 5/8] scsi: ufs: Declare ufshcd_mcq_poll_cqe_lock() once Bart Van Assche
2024-06-18 11:01   ` Avri Altman
2024-06-17 21:07 ` [PATCH 6/8] scsi: ufs: Make ufshcd_poll() complain about unsupported arguments Bart Van Assche
2024-06-19  7:32   ` Manivannan Sadhasivam
2024-06-20 20:13     ` Bart Van Assche
2024-06-23 13:39       ` Manivannan Sadhasivam
2024-06-17 21:07 ` [PATCH 7/8] scsi: ufs: Make the polling code report which command has been completed Bart Van Assche
2024-06-17 21:07 ` [PATCH 8/8] scsi: ufs: Check for completion from the timeout handler Bart Van Assche
2024-06-21  6:54   ` Peter Wang (王信友)
2024-06-21 17:23     ` Bart Van Assche
2024-06-24  8:54       ` Peter Wang (王信友)
2024-06-24 18:12         ` Bart Van Assche
2024-06-25 10:04           ` Peter Wang (王信友)
2024-06-25 16:33             ` Bart Van Assche
2024-06-26  3:54               ` Peter Wang (王信友)
2024-06-26 21:54                 ` Bart Van Assche
2024-06-27 10:56                   ` Peter Wang (王信友)
2024-06-27 16:33                     ` Bart Van Assche
2024-06-27  3:50   ` Wenchao Hao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1891546521.01718675102603.JavaMail.epsvc@epcpadp3 \
    --to=daejun7.park@samsung.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=akinobu.mita@gmail.com \
    --cc=avri.altman@wdc.com \
    --cc=beanhuo@micron.com \
    --cc=bvanassche@acm.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=manivannan.sadhasivam@linaro.org \
    --cc=martin.petersen@oracle.com \
    --cc=minwoo.im@samsung.com \
    --cc=peter.wang@mediatek.com \
    --cc=quic_mnaresh@quicinc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox