Re: [PATCH v5 07/16] ufs: core: mcq: Calculate queue depth

[Asutosh Das]

On Mon, Nov 28 2022 at 07:15 -0800, Manivannan Sadhasivam wrote:
> On Tue, Nov 22, 2022 at 08:10:20PM -0800, Asutosh Das wrote:
> > The ufs device defines the supported queuedepth by
> > bqueuedepth which has a max value of 256.
> > The HC defines MAC (Max Active Commands) that define
> > the max number of commands that in flight to the ufs
> > device.
> > Calculate and configure the nutrs based on both these
> > values.
> >
> > Co-developed-by: Can Guo <quic_cang@xxxxxxxxxxx>
> > Signed-off-by: Can Guo <quic_cang@xxxxxxxxxxx>
> > Signed-off-by: Asutosh Das <quic_asutoshd@xxxxxxxxxxx>
> > ---
> >  drivers/ufs/core/ufs-mcq.c     | 32 ++++++++++++++++++++++++++++++++
> >  drivers/ufs/core/ufshcd-priv.h |  9 +++++++++
> >  drivers/ufs/core/ufshcd.c      | 17 ++++++++++++++++-
> >  drivers/ufs/host/ufs-qcom.c    |  8 ++++++++
> >  include/ufs/ufs.h              |  2 ++
> >  include/ufs/ufshcd.h           |  2 ++
> >  include/ufs/ufshci.h           |  1 +
> >  7 files changed, 70 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
> > index 4aaa6aa..e95f748 100644
> > --- a/drivers/ufs/core/ufs-mcq.c
> > +++ b/drivers/ufs/core/ufs-mcq.c
> > @@ -18,6 +18,8 @@
> >  #define UFS_MCQ_NUM_DEV_CMD_QUEUES 1
> >  #define UFS_MCQ_MIN_POLL_QUEUES 0
> >
> > +#define MAX_DEV_CMD_ENTRIES	2
> > +#define MCQ_CFG_MAC_MASK	GENMASK(16, 8)
> >  #define MCQ_QCFGPTR_MASK	GENMASK(7, 0)
> >  #define MCQ_QCFGPTR_UNIT	0x200
> >  #define MCQ_SQATTR_OFFSET(c) \
> > @@ -88,6 +90,36 @@ static const struct ufshcd_res_info ufs_res_info[RES_MAX] = {
> >  	{.name = "mcq_vs",},
> >  };
[...]

Hello Mani,
Thanks for taking a look.

> > +	WARN_ON(!hba->dev_info.bqueuedepth);
>
> Instead of panic, you could just print and return an error.
I'd make it WARN_ON_ONCE();

> > +	/*
> > +	 * max. value of bqueuedepth = 256, mac is host dependent.
> > +	 * It is mandatory for UFS device to define bQueueDepth if
> > +	 * shared queuing architecture is enabled.
> > +	 */
> > +	return min_t(int, mac, hba->dev_info.bqueuedepth);
> > +}
> > +
> >  static int ufshcd_mcq_config_resource(struct ufs_hba *hba)
> >  {
> >  	struct platform_device *pdev = to_platform_device(hba->dev);
> > diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
> > index 9368ba2..9f40fa5 100644
> > --- a/drivers/ufs/core/ufshcd-priv.h
> > +++ b/drivers/ufs/core/ufshcd-priv.h
> > @@ -62,6 +62,7 @@ int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode,
> >  	enum flag_idn idn, u8 index, bool *flag_res);
> >  void ufshcd_auto_hibern8_update(struct ufs_hba *hba, u32 ahit);
> >  int ufshcd_mcq_init(struct ufs_hba *hba);
> > +int ufshcd_mcq_decide_queue_depth(struct ufs_hba *hba);
> >
> >  #define SD_ASCII_STD true
> >  #define SD_RAW false
> > @@ -227,6 +228,14 @@ static inline void ufshcd_vops_config_scaling_param(struct ufs_hba *hba,
> >  		hba->vops->config_scaling_param(hba, p, data);
> >  }
> >
> > +static inline int ufshcd_mcq_vops_get_hba_mac(struct ufs_hba *hba)
>
> Again, no inline please.
It spits out the following warning for all files that include this header, when
inline is removed:
warning: 'ufshcd_mcq_vops_get_hba_mac' defined but not used [-Wunused-function]

[...]
> > +#define MAX_SUPP_MAC 64
>
> Similar definitions are part of ufs-qcom.h.
>
> Thanks,
> Mani
>
>
> --
> மணிவண்ணன் சதாசிவம்