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;
Message-ID: <0fb3f8ae-5ed7-9057-0d2b-8866f36c2441@acm.org>
Date:   Thu, 27 Oct 2022 14:52:03 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.3.0
Subject: Re: [PATCH v3 07/17] ufs: core: mcq: Calculate queue depth
Content-Language: en-US
To:     Asutosh Das <quic_asutoshd@quicinc.com>, quic_cang@quicinc.com,
        martin.petersen@oracle.com, linux-scsi@vger.kernel.org
Cc:     quic_nguyenb@quicinc.com, quic_xiaosenh@quicinc.com,
        stanley.chu@mediatek.com, eddie.huang@mediatek.com,
        daejun7.park@samsung.com, avri.altman@wdc.com, mani@kernel.org,
        beanhuo@micron.com, quic_richardp@quicinc.com,
        linux-arm-msm@vger.kernel.org,
        Alim Akhtar <alim.akhtar@samsung.com>,
        "James E.J. Bottomley" <jejb@linux.ibm.com>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        Jinyoung Choi <j-young.choi@samsung.com>,
        Kiwoong Kim <kwmad.kim@samsung.com>,
        open list <linux-kernel@vger.kernel.org>
References: <cover.1666288432.git.quic_asutoshd@quicinc.com>
 <1987fbada1d33c04c9598614ef712e0a48fe065e.1666288432.git.quic_asutoshd@quicinc.com>
From:   Bart Van Assche <bvanassche@acm.org>
In-Reply-To: <1987fbada1d33c04c9598614ef712e0a48fe065e.1666288432.git.quic_asutoshd@quicinc.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 10/20/22 11:03, Asutosh Das wrote:
> +u32 ufshcd_mcq_decide_queue_depth(struct ufs_hba *hba)
> +{
> +	u32 qd, val;
> +	int mac;
> +
> +	mac = ufshcd_mcq_vops_get_hba_mac(hba);
> +	if (mac < 0) {
> +		val = ufshcd_readl(hba, REG_UFS_MCQ_CFG);
> +		mac = FIELD_GET(MCQ_CFG_MAC_MASK, val);
> +	}

According to the UFSHCI 4.0 specification the MAC value is set by the 
host. Can the above code read the MAC value from the host controller 
before it has been set by the host? If so, how about leaving out the 
code that reads the MAC value from the controller and making it 
mandatory to implement the new get_hba_mac vop?

> +
> +	/*  MAC is a 0 based value. */
> +	mac += 1;
> +	/* max. value of bqueuedepth = 256, mac is host dependent */

host dependent -> defined by the host controller?

> +	qd = min_t(u32, mac, hba->dev_info.bqueuedepth);
> +	if (!qd)
> +		qd = mac;

How about using min_not_zero() instead of open-coding it?

Thanks,

Bart.