[PATCH 0/3] Three small UFS patches

[PATCH 0/3] Three small UFS patches

[Bart Van Assche]

Hi Martin,

These three patches are what I came up with after having reviewed the compiler warnings
triggered by the UFS driver and after having reviewed its source code. Please consider
these patches for kernel version v5.5.

Thanks,

Bart.

Bart Van Assche (3):
  ufs: Fix kernel-doc warnings
  ufs: Use enum dev_cmd_type where appropriate
  ufs: Remove .setup_xfer_req()

 drivers/scsi/ufs/ufs_bsg.c |  1 +
 drivers/scsi/ufs/ufshcd.c  |  8 +++-----
 drivers/scsi/ufs/ufshcd.h  | 10 ----------
 3 files changed, 4 insertions(+), 15 deletions(-)

-- 
2.24.0.rc0.303.g954a862665-goog

[PATCH 1/3] ufs: Fix kernel-doc warnings

[Bart Van Assche]

Fix the following three kernel-doc warnings:

drivers/scsi/ufs/ufs_bsg.c:165: warning: Function parameter or member 'hba' not described in 'ufs_bsg_remove'
drivers/scsi/ufs/ufshcd.c:5789: warning: Function parameter or member 'cmd_type' not described in 'ufshcd_issue_devman_upiu_cmd'
drivers/scsi/ufs/ufshcd.c:5789: warning: Excess function parameter 'msgcode' description in 'ufshcd_issue_devman_upiu_cmd'

Cc: Yaniv Gardi <ygardi@codeaurora.org>
Cc: Subhash Jadavani <subhashj@codeaurora.org>
Cc: Stanley Chu <stanley.chu@mediatek.com>
Cc: Avri Altman <avri.altman@wdc.com>
Cc: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/ufs/ufs_bsg.c | 1 +
 drivers/scsi/ufs/ufshcd.c  | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufs_bsg.c b/drivers/scsi/ufs/ufs_bsg.c
index a9344eb4e047..3a2e68f1ad42 100644
--- a/drivers/scsi/ufs/ufs_bsg.c
+++ b/drivers/scsi/ufs/ufs_bsg.c
@@ -158,6 +158,7 @@ static int ufs_bsg_request(struct bsg_job *job)
 
 /**
  * ufs_bsg_remove - detach and remove the added ufs-bsg node
+ * @hba: per adapter object
  *
  * Should be called when unloading the driver.
  */
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index c28c144d9b4a..7ced77c8cc4d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5768,9 +5768,9 @@ static int ufshcd_issue_tm_cmd(struct ufs_hba *hba, int lun_id, int task_id,
  * @hba:	per-adapter instance
  * @req_upiu:	upiu request
  * @rsp_upiu:	upiu reply
- * @msgcode:	message code, one of UPIU Transaction Codes Initiator to Target
  * @desc_buff:	pointer to descriptor buffer, NULL if NA
  * @buff_len:	descriptor size, 0 if NA
+ * @cmd_type:	specifies the type (NOP, Query...)
  * @desc_op:	descriptor operation
  *
  * Those type of requests uses UTP Transfer Request Descriptor - utrd.
-- 
2.24.0.rc0.303.g954a862665-goog

[PATCH 2/3] ufs: Use enum dev_cmd_type where appropriate

[Bart Van Assche]

Declare all variables that hold dev_cmd_type values as an enum instead of
as an int.

Cc: Yaniv Gardi <ygardi@codeaurora.org>
Cc: Subhash Jadavani <subhashj@codeaurora.org>
Cc: Stanley Chu <stanley.chu@mediatek.com>
Cc: Avri Altman <avri.altman@wdc.com>
Cc: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/ufs/ufshcd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 7ced77c8cc4d..180033b4b515 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5784,7 +5784,7 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba,
 					struct utp_upiu_req *req_upiu,
 					struct utp_upiu_req *rsp_upiu,
 					u8 *desc_buff, int *buff_len,
-					int cmd_type,
+					enum dev_cmd_type cmd_type,
 					enum query_opcode desc_op)
 {
 	struct ufshcd_lrb *lrbp;
@@ -5899,7 +5899,7 @@ int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba,
 			     enum query_opcode desc_op)
 {
 	int err;
-	int cmd_type = DEV_CMD_TYPE_QUERY;
+	enum dev_cmd_type cmd_type = DEV_CMD_TYPE_QUERY;
 	struct utp_task_req_desc treq = { { 0 }, };
 	int ocs_value;
 	u8 tm_f = be32_to_cpu(req_upiu->header.dword_1) >> 16 & MASK_TM_FUNC;
-- 
2.24.0.rc0.303.g954a862665-goog

[PATCH 3/3] ufs: Remove .setup_xfer_req()

[Bart Van Assche]

Since the function ufshcd_vops_setup_xfer_req() is the only user of the
setup_xfer_req function pointer and since that function pointer is always
zero, remove both this function and the function pointer. This patch
does not change any functionality.

Cc: Yaniv Gardi <ygardi@codeaurora.org>
Cc: Subhash Jadavani <subhashj@codeaurora.org>
Cc: Stanley Chu <stanley.chu@mediatek.com>
Cc: Avri Altman <avri.altman@wdc.com>
Cc: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/ufs/ufshcd.c |  2 --
 drivers/scsi/ufs/ufshcd.h | 10 ----------
 2 files changed, 12 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 180033b4b515..9fc05a535624 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2487,7 +2487,6 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
 
 	/* issue command to the controller */
 	spin_lock_irqsave(hba->host->host_lock, flags);
-	ufshcd_vops_setup_xfer_req(hba, tag, (lrbp->cmd ? true : false));
 	ufshcd_send_command(hba, tag);
 out_unlock:
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
@@ -2704,7 +2703,6 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba,
 	/* Make sure descriptors are ready before ringing the doorbell */
 	wmb();
 	spin_lock_irqsave(hba->host->host_lock, flags);
-	ufshcd_vops_setup_xfer_req(hba, tag, (lrbp->cmd ? true : false));
 	ufshcd_send_command(hba, tag);
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
 
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index e0fe247c719e..e3593cce23c1 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -288,8 +288,6 @@ struct ufs_pwr_mode_info {
  * @pwr_change_notify: called before and after a power mode change
  *			is carried out to allow vendor spesific capabilities
  *			to be set.
- * @setup_xfer_req: called before any transfer request is issued
- *                  to set some things
  * @setup_task_mgmt: called before any task management request is issued
  *                  to set some things
  * @hibern8_notify: called around hibern8 enter/exit
@@ -318,7 +316,6 @@ struct ufs_hba_variant_ops {
 					enum ufs_notify_change_status status,
 					struct ufs_pa_layer_attr *,
 					struct ufs_pa_layer_attr *);
-	void	(*setup_xfer_req)(struct ufs_hba *, int, bool);
 	void	(*setup_task_mgmt)(struct ufs_hba *, int, u8);
 	void    (*hibern8_notify)(struct ufs_hba *, enum uic_cmd_dme,
 					enum ufs_notify_change_status);
@@ -1031,13 +1028,6 @@ static inline int ufshcd_vops_pwr_change_notify(struct ufs_hba *hba,
 	return -ENOTSUPP;
 }
 
-static inline void ufshcd_vops_setup_xfer_req(struct ufs_hba *hba, int tag,
-					bool is_scsi_cmd)
-{
-	if (hba->vops && hba->vops->setup_xfer_req)
-		return hba->vops->setup_xfer_req(hba, tag, is_scsi_cmd);
-}
-
 static inline void ufshcd_vops_setup_task_mgmt(struct ufs_hba *hba,
 					int tag, u8 tm_function)
 {
-- 
2.24.0.rc0.303.g954a862665-goog

RE: [PATCH 3/3] ufs: Remove .setup_xfer_req()

[Avri Altman]

+ Kiwoong Kim

> 
> Since the function ufshcd_vops_setup_xfer_req() is the only user of the
> setup_xfer_req function pointer and since that function pointer is always zero,
> remove both this function and the function pointer. This patch does not change
> any functionality.
> 
> Cc: Yaniv Gardi <ygardi@codeaurora.org>
> Cc: Subhash Jadavani <subhashj@codeaurora.org>
> Cc: Stanley Chu <stanley.chu@mediatek.com>
> Cc: Avri Altman <avri.altman@wdc.com>
> Cc: Tomas Winkler <tomas.winkler@intel.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Since this was introduced only a couple of years ago, 
Maybe better to CC the author Kiwoong Kim <kwmad.kim@samsung.com>
Before removing this altogether.

RE: [PATCH 1/3] ufs: Fix kernel-doc warnings

[Avri Altman]

 
> 
> Fix the following three kernel-doc warnings:
> 
> drivers/scsi/ufs/ufs_bsg.c:165: warning: Function parameter or member 'hba'
> not described in 'ufs_bsg_remove'
> drivers/scsi/ufs/ufshcd.c:5789: warning: Function parameter or member
> 'cmd_type' not described in 'ufshcd_issue_devman_upiu_cmd'
> drivers/scsi/ufs/ufshcd.c:5789: warning: Excess function parameter 'msgcode'
> description in 'ufshcd_issue_devman_upiu_cmd'
> 
> Cc: Yaniv Gardi <ygardi@codeaurora.org>
> Cc: Subhash Jadavani <subhashj@codeaurora.org>
> Cc: Stanley Chu <stanley.chu@mediatek.com>
> Cc: Avri Altman <avri.altman@wdc.com>
> Cc: Tomas Winkler <tomas.winkler@intel.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Avri Altman <avri.altman@wdc.com>

RE: [PATCH 2/3] ufs: Use enum dev_cmd_type where appropriate

[Avri Altman]

> 
> Declare all variables that hold dev_cmd_type values as an enum instead of as an
> int.
> 
> Cc: Yaniv Gardi <ygardi@codeaurora.org>
> Cc: Subhash Jadavani <subhashj@codeaurora.org>
> Cc: Stanley Chu <stanley.chu@mediatek.com>
> Cc: Avri Altman <avri.altman@wdc.com>
> Cc: Tomas Winkler <tomas.winkler@intel.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Avri Altman <avri.altman@wdc.com>

Re: [PATCH 1/3] ufs: Fix kernel-doc warnings

[Martin K. Petersen]

Bart,

> Fix the following three kernel-doc warnings:
>
> drivers/scsi/ufs/ufs_bsg.c:165: warning: Function parameter or member 'hba' not described in 'ufs_bsg_remove'
> drivers/scsi/ufs/ufshcd.c:5789: warning: Function parameter or member 'cmd_type' not described in 'ufshcd_issue_devman_upiu_cmd'
> drivers/scsi/ufs/ufshcd.c:5789: warning: Excess function parameter 'msgcode' description in 'ufshcd_issue_devman_upiu_cmd'

Applied to 5.5/scsi-queue, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 2/3] ufs: Use enum dev_cmd_type where appropriate

[Martin K. Petersen]

Bart,

> Declare all variables that hold dev_cmd_type values as an enum instead
> of as an int.

Applied to 5.5/scsi-queue, thank you!

-- 
Martin K. Petersen	Oracle Linux Engineering

RE: [PATCH 3/3] ufs: Remove .setup_xfer_req()

[Avri Altman]

As no response from Kiwoong Kim:

> 
> 
> + Kiwoong Kim
> 
> >
> > Since the function ufshcd_vops_setup_xfer_req() is the only user of
> > the setup_xfer_req function pointer and since that function pointer is
> > always zero, remove both this function and the function pointer. This
> > patch does not change any functionality.
> >
> > Cc: Yaniv Gardi <ygardi@codeaurora.org>
> > Cc: Subhash Jadavani <subhashj@codeaurora.org>
> > Cc: Stanley Chu <stanley.chu@mediatek.com>
> > Cc: Avri Altman <avri.altman@wdc.com>
> > Cc: Tomas Winkler <tomas.winkler@intel.com>
> > Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> Since this was introduced only a couple of years ago, Maybe better to CC the
> author Kiwoong Kim <kwmad.kim@samsung.com> Before removing this
> altogether.
Reviewed-by: Avri Altman <avri.altman@wdc.com>

Re: [PATCH 3/3] ufs: Remove .setup_xfer_req()

[Martin K. Petersen]

Bart,

> Since the function ufshcd_vops_setup_xfer_req() is the only user of
> the setup_xfer_req function pointer and since that function pointer is
> always zero, remove both this function and the function pointer. This
> patch does not change any functionality.

Applied to 5.5/scsi-queue, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 3/3] ufs: Remove .setup_xfer_req()

[Alim Akhtar]

Hi

On Mon, Nov 4, 2019 at 6:29 PM Avri Altman <Avri.Altman@wdc.com> wrote:
>
> As no response from Kiwoong Kim:
>
> >
> >
> > + Kiwoong Kim
Looks like he is not active here.
> >
> > >
> > > Since the function ufshcd_vops_setup_xfer_req() is the only user of
> > > the setup_xfer_req function pointer and since that function pointer is
> > > always zero, remove both this function and the function pointer. This
> > > patch does not change any functionality.
> > >
> > > Cc: Yaniv Gardi <ygardi@codeaurora.org>
> > > Cc: Subhash Jadavani <subhashj@codeaurora.org>
> > > Cc: Stanley Chu <stanley.chu@mediatek.com>
> > > Cc: Avri Altman <avri.altman@wdc.com>
> > > Cc: Tomas Winkler <tomas.winkler@intel.com>
> > > Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> > Since this was introduced only a couple of years ago, Maybe better to CC the
> > author Kiwoong Kim <kwmad.kim@samsung.com> Before removing this
> > altogether.
> Reviewed-by: Avri Altman <avri.altman@wdc.com>

Let me check and reconfirm this, give a day or two.
It will be good if am copied to the ufs patch (I hope
get_maintainer.pl still pointout my email)
thanks

-- 
Regards,
Alim

Re: [PATCH 3/3] ufs: Remove .setup_xfer_req()

[Alim Akhtar]

Hi Bart / Avri

On Tue, Nov 5, 2019 at 9:47 PM Alim Akhtar <alim.akhtar@gmail.com> wrote:
>
> Hi
>
> On Mon, Nov 4, 2019 at 6:29 PM Avri Altman <Avri.Altman@wdc.com> wrote:
> >
> > As no response from Kiwoong Kim:
> >
> > >
> > >
> > > + Kiwoong Kim
> Looks like he is not active here.
> > >
> > > >
> > > > Since the function ufshcd_vops_setup_xfer_req() is the only user of
> > > > the setup_xfer_req function pointer and since that function pointer is
> > > > always zero, remove both this function and the function pointer. This
> > > > patch does not change any functionality.
> > > >
> > > > Cc: Yaniv Gardi <ygardi@codeaurora.org>
> > > > Cc: Subhash Jadavani <subhashj@codeaurora.org>
> > > > Cc: Stanley Chu <stanley.chu@mediatek.com>
> > > > Cc: Avri Altman <avri.altman@wdc.com>
> > > > Cc: Tomas Winkler <tomas.winkler@intel.com>
> > > > Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> > > Since this was introduced only a couple of years ago, Maybe better to CC the
> > > author Kiwoong Kim <kwmad.kim@samsung.com> Before removing this
> > > altogether.
> > Reviewed-by: Avri Altman <avri.altman@wdc.com>
>
> Let me check and reconfirm this, give a day or two.
> It will be good if am copied to the ufs patch (I hope
> get_maintainer.pl still pointout my email)
> thanks
>
I checked the brief history of this adding  "setup_xfer_req" to
support Samsung UFSHCI (this was the ground work done)
We need this to support vendor specific NEXUS_TYPE settings.
The Samsung UFSHCI driver will be up for review in near future
For usecase of the function pointer please see an older version of the
patch https://patchwork.kernel.org/patch/7306321/

> --
> Regards,
> Alim



-- 
Regards,
Alim

Re: [PATCH 3/3] ufs: Remove .setup_xfer_req()

[Bart Van Assche]

On 2019-11-05 20:09, Alim Akhtar wrote:
> I checked the brief history of this adding  "setup_xfer_req" to
> support Samsung UFSHCI (this was the ground work done)
> We need this to support vendor specific NEXUS_TYPE settings.
> The Samsung UFSHCI driver will be up for review in near future
> For usecase of the function pointer please see an older version of the
> patch https://patchwork.kernel.org/patch/7306321/

Thanks Alim for having looked up this information. Let's drop this patch.

Bart.

Re: [PATCH 3/3] ufs: Remove .setup_xfer_req()

[Martin K. Petersen]

Bart,

> Thanks Alim for having looked up this information. Let's drop this
> patch.

Dropped from 5.5/scsi-queue.

-- 
Martin K. Petersen	Oracle Linux Engineering