Re: [PATCH 2/6] ufs: Make ufshcd_add_command_trace() easier to read

Alim Akhtar
2019-12-27T01:18:13Z

On Wed, Dec 25, 2019 at 3:34 AM Bart Van Assche <bvanassche@acm.org> wrote:
>
> Since the lrbp->cmd expression occurs multiple times, introduce a new
> local variable to hold that pointer. This patch does not change any
> functionality.
>
FYMI, Any Advantage of doing this? or it just we don't want to fetch
cmd from lrbp every time? I mean in terms of execution speed.

> Cc: Bean Huo <beanhuo@micron.com>
> Cc: Can Guo <cang@codeaurora.org>
> Cc: Avri Altman <avri.altman@wdc.com>
> Cc: Stanley Chu <stanley.chu@mediatek.com>
> Cc: Tomas Winkler <tomas.winkler@intel.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
> drivers/scsi/ufs/ufshcd.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 48f2f94d51bc..acc84e964e8f 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -327,27 +327,27 @@ static void ufshcd_add_command_trace(struct ufs_hba *hba,
> u8 opcode = 0;
> u32 intr, doorbell;
> struct ufshcd_lrb *lrbp = &hba->lrb[tag];
> + struct scsi_cmnd *cmd = lrbp->cmd;
> int transfer_len = -1;
>
> if (!trace_ufshcd_command_enabled()) {
> /* trace UPIU W/O tracing command */
> - if (lrbp->cmd)
> + if (cmd)
> ufshcd_add_cmd_upiu_trace(hba, tag, str);
> return;
> }
>
> - if (lrbp->cmd) { /* data phase exists */
> + if (cmd) { /* data phase exists */
> /* trace UPIU also */
> ufshcd_add_cmd_upiu_trace(hba, tag, str);
> - opcode = (u8)(*lrbp->cmd->cmnd);
> + opcode = cmd->cmnd[0];
> if ((opcode == READ_10) || (opcode == WRITE_10)) {
> /*
> * Currently we only fully trace read(10) and write(10)
> * commands
> */
> - if (lrbp->cmd->request && lrbp->cmd->request->bio)
> - lba =
> - lrbp->cmd->request->bio->bi_iter.bi_sector;
> + if (cmd->request && cmd->request->bio)
> + lba = cmd->request->bio->bi_iter.bi_sector;
> transfer_len = be32_to_cpu(
> lrbp->ucd_req_ptr->sc.exp_data_transfer_len);
> }

--
Regards,
Alim