All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.