Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 26 Sep 2022 14:32:51 -0700
From:   Asutosh Das <quic_asutoshd@quicinc.com>
To:     Manivannan Sadhasivam <mani@kernel.org>
CC:     <quic_nguyenb@quicinc.com>, <quic_xiaosenh@quicinc.com>,
        <quic_cang@quicinc.com>, <quic_nitirawa@quicinc.com>,
        <quic_rampraka@quicinc.com>, <quic_richardp@quicinc.com>,
        <stanley.chu@mediatek.com>, <adrian.hunter@intel.com>,
        <bvanassche@acm.org>, <avri.altman@wdc.com>, <beanhuo@micron.com>,
        <martin.petersen@oracle.com>, <linux-scsi@vger.kernel.org>,
        Alim Akhtar <alim.akhtar@samsung.com>,
        "James E.J. Bottomley" <jejb@linux.ibm.com>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        open list <linux-kernel@vger.kernel.org>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-arm-kernel@lists.infradead.org>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@lists.infradead.org>
Subject: Re: [PATCH v1 04/16] ufs: core: mcq: Introduce Multi Circular Queue
Message-ID: <20220926213251.GC15228@asutoshd-linux1.qualcomm.com>
References: <cover.1663894792.git.quic_asutoshd@quicinc.com>
 <e0ae0fccc579fa17b6cfcf3b8bcf963f5ebe67fa.1663894792.git.quic_asutoshd@quicinc.com>
 <20220926144957.GE101994@thinkpad>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; format=flowed
Content-Disposition: inline
In-Reply-To: <20220926144957.GE101994@thinkpad>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk

On Mon, Sep 26 2022 at 07:50 -0700, Manivannan Sadhasivam wrote:
>On Thu, Sep 22, 2022 at 06:05:11PM -0700, Asutosh Das wrote:

[...]

>> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
>> new file mode 100644
>> index 0000000..934556f
>> --- /dev/null
>> +++ b/drivers/ufs/core/ufs-mcq.c
[...]
>> +static int ufshcd_mcq_config_nr_queues(struct ufs_hba *hba)
>> +{
>> +	int i, rem;
>> +	u32 hbaq_cap, cmp, tot_queues;
>> +	struct Scsi_Host *host = hba->host;
>> +
>> +	hbaq_cap = hba->mcq_capabilities & 0xff;
>
>Define 0xff
>
>hbaq_cap should be named after the define used for 0xff. It is not quite
>understandable now.
>
Ok. I think you want the name hbaq_cap to be changed to what 0xff signifies.
Is that understanding correct?

>> +
>> +	if (!rw_queues)
>> +		rw_queues = num_possible_cpus();
>> +
>> +	tot_queues = dev_cmd_queue + read_queues + poll_queues + rw_queues;
>> +	if (hbaq_cap < tot_queues) {
>> +		dev_err(hba->dev, "Total queues (%d) exceeds HC capacity (%d)\n",
>> +			tot_queues, hbaq_cap);
>> +		return -EOPNOTSUPP;
>> +	}
>> +
>> +	rem = hbaq_cap - dev_cmd_queue;
>> +	cmp = rem;
>> +	hba->nr_queues[HCTX_TYPE_DEFAULT] = min3(cmp, rw_queues,
>> +						 num_possible_cpus());
>> +	rem -= hba->nr_queues[HCTX_TYPE_DEFAULT];
>> +	cmp = rem;
>> +	hba->nr_queues[HCTX_TYPE_POLL] = min(cmp, poll_queues);
>
>Hmm, so the driver is not using the number of queues set by the user?
>If the number varies, I don't think it should be configurable.
>
I think the driver would use the number of queues set by the user.
Because there's a check(hbaq_cap < tot_queues) above that ensures
that the HBA has enough queues for the given configuration.

-asd

>