[v1] scsi: ufs: ufs-mediatek: configure individual LU queue flags

[PATCH] scsi: ufs: ufs-mediatek: configure individual LU queue flags

Posted by ed.tsai@mediatek.com 1 month, 2 weeks ago

From: Ed Tsai <ed.tsai@mediatek.com>

Previously, ufs vops config_scsi_dev was removed because there were no
users. ufs-mediatek needs it to configure the queue flags for each LU
individually. Therefore, bring it back and customize the queue flag as
we required.

Signed-off-by: Ed Tsai <ed.tsai@mediatek.com>
---
 drivers/ufs/core/ufshcd.c       |  3 +++
 drivers/ufs/host/ufs-mediatek.c | 10 ++++++++++
 include/ufs/ufshcd.h            |  1 +
 3 files changed, 14 insertions(+)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 7cab103112e1..be50b86269bf 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -5253,6 +5253,9 @@ static int ufshcd_device_configure(struct scsi_device *sdev,
 	 */
 	sdev->silence_suspend = 1;
 
+	if (hba->vops && hba->vops->config_scsi_dev)
+		hba->vops->config_scsi_dev(sdev);
+
 	ufshcd_crypto_register(hba, q);
 
 	return 0;
diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
index 9a5919434c4e..0b57623edca5 100644
--- a/drivers/ufs/host/ufs-mediatek.c
+++ b/drivers/ufs/host/ufs-mediatek.c
@@ -1780,6 +1780,15 @@ static int ufs_mtk_config_esi(struct ufs_hba *hba)
 	return ufs_mtk_config_mcq(hba, true);
 }
 
+static void ufs_mtk_config_scsi_dev(struct scsi_device *sdev)
+{
+	struct ufs_hba *hba = shost_priv(sdev->host);
+
+	dev_dbg(hba->dev, "lu %llu scsi device configured", sdev->lun);
+	if (sdev->lun == 2)
+		blk_queue_flag_set(QUEUE_FLAG_SAME_FORCE, sdev->reqeust_queue);
+}
+
 /*
  * struct ufs_hba_mtk_vops - UFS MTK specific variant operations
  *
@@ -1809,6 +1818,7 @@ static const struct ufs_hba_variant_ops ufs_hba_mtk_vops = {
 	.op_runtime_config   = ufs_mtk_op_runtime_config,
 	.mcq_config_resource = ufs_mtk_mcq_config_resource,
 	.config_esi          = ufs_mtk_config_esi,
+	.config_scsi_dev     = ufs_mtk_config_scsi_dev,
 };
 
 /**
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index a95282b9f743..800d79dc91fc 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -383,6 +383,7 @@ struct ufs_hba_variant_ops {
 	int	(*get_outstanding_cqs)(struct ufs_hba *hba,
 				       unsigned long *ocqs);
 	int	(*config_esi)(struct ufs_hba *hba);
+	void	(*config_scsi_dev)(struct scsi_device *sdev);
 };
 
 /* clock gating state  */
-- 
2.45.2

Re: [PATCH] scsi: ufs: ufs-mediatek: configure individual LU queue flags

Posted by Martin K. Petersen 1 week, 6 days ago

On Tue, 08 Oct 2024 14:59:42 +0800, ed.tsai@mediatek.com wrote:

> Previously, ufs vops config_scsi_dev was removed because there were no
> users. ufs-mediatek needs it to configure the queue flags for each LU
> individually. Therefore, bring it back and customize the queue flag as
> we required.
> 
> 

Applied to 6.13/scsi-queue, thanks!

[1/1] scsi: ufs: ufs-mediatek: configure individual LU queue flags
      https://git.kernel.org/mkp/scsi/c/7670e74ff319

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] scsi: ufs: ufs-mediatek: configure individual LU queue flags

Posted by kernel test robot 1 month, 2 weeks ago

Hi,

kernel test robot noticed the following build errors:

[auto build test ERROR on mkp-scsi/for-next]
[also build test ERROR on jejb-scsi/for-next linus/master v6.12-rc2 next-20241008]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/ed-tsai-mediatek-com/scsi-ufs-ufs-mediatek-configure-individual-LU-queue-flags/20241008-153700
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
patch link:    https://lore.kernel.org/r/20241008065950.23431-1-ed.tsai%40mediatek.com
patch subject: [PATCH] scsi: ufs: ufs-mediatek: configure individual LU queue flags
config: arm-allyesconfig (https://download.01.org/0day-ci/archive/20241009/202410091257.SE04cckD-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241009/202410091257.SE04cckD-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410091257.SE04cckD-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/ufs/host/ufs-mediatek.c: In function 'ufs_mtk_config_scsi_dev':
>> drivers/ufs/host/ufs-mediatek.c:1789:65: error: 'struct scsi_device' has no member named 'reqeust_queue'; did you mean 'request_queue'?
    1789 |                 blk_queue_flag_set(QUEUE_FLAG_SAME_FORCE, sdev->reqeust_queue);
         |                                                                 ^~~~~~~~~~~~~
         |                                                                 request_queue

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for GET_FREE_REGION
   Depends on [n]: SPARSEMEM [=n]
   Selected by [y]:
   - RESOURCE_KUNIT_TEST [=y] && RUNTIME_TESTING_MENU [=y] && KUNIT [=y]


vim +1789 drivers/ufs/host/ufs-mediatek.c

  1782	
  1783	static void ufs_mtk_config_scsi_dev(struct scsi_device *sdev)
  1784	{
  1785		struct ufs_hba *hba = shost_priv(sdev->host);
  1786	
  1787		dev_dbg(hba->dev, "lu %llu scsi device configured", sdev->lun);
  1788		if (sdev->lun == 2)
> 1789			blk_queue_flag_set(QUEUE_FLAG_SAME_FORCE, sdev->reqeust_queue);
  1790	}
  1791	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] scsi: ufs: ufs-mediatek: configure individual LU queue flags

Posted by Bart Van Assche 1 month, 2 weeks ago

On 10/7/24 11:59 PM, ed.tsai@mediatek.com wrote:
> +static void ufs_mtk_config_scsi_dev(struct scsi_device *sdev)
> +{
> +	struct ufs_hba *hba = shost_priv(sdev->host);
> +
> +	dev_dbg(hba->dev, "lu %llu scsi device configured", sdev->lun);
> +	if (sdev->lun == 2)
> +		blk_queue_flag_set(QUEUE_FLAG_SAME_FORCE, sdev->reqeust_queue);
> +}

There are no block drivers in the upstream kernel that set
QUEUE_FLAG_SAME_FORCE. An explanation is missing from the patch
description why this flag is set from the UFS driver instead of by
writing the value "2" into /sys/class/block/$bdev/queue/rq_affinity.
Additionally, an explanation is missing why QUEUE_FLAG_SAME_FORCE is
set but QUEUE_FLAG_SAME_COMP not.

Bart.

Re: [PATCH] scsi: ufs: ufs-mediatek: configure individual LU queue flags

Posted by Ed Tsai (蔡宗軒) 1 month, 2 weeks ago

On Tue, 2024-10-08 at 10:38 -0700, Bart Van Assche wrote:
>  On 10/7/24 11:59 PM, ed.tsai@mediatek.com wrote:
> > +static void ufs_mtk_config_scsi_dev(struct scsi_device *sdev)
> > +{
> > +struct ufs_hba *hba = shost_priv(sdev->host);
> > +
> > +dev_dbg(hba->dev, "lu %llu scsi device configured", sdev->lun);
> > +if (sdev->lun == 2)
> > +blk_queue_flag_set(QUEUE_FLAG_SAME_FORCE, sdev->reqeust_queue);
> > +}
> 
> There are no block drivers in the upstream kernel that set
> QUEUE_FLAG_SAME_FORCE. An explanation is missing from the patch
> description why this flag is set from the UFS driver instead of by
> writing the value "2" into /sys/class/block/$bdev/queue/rq_affinity.

The LU probing is completed asynchronously by another thread, which
means that "sdc" cannot be guaranteed to be LU2. We do not need to
change the queue settings at runtime. Therefore, a simpler and more
intuitive approach is to set its flag once the SCSI device is confirmed
to be ready.

> Additionally, an explanation is missing why QUEUE_FLAG_SAME_FORCE is
> set but QUEUE_FLAG_SAME_COMP not.

QUEUE_FLAG_SAME_COMP is the default flag for blk mq queue. As this
stage, it should be set by default.

> 
> Bart.