Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Message-ID: <5fe56e62-e1ab-1267-ddaa-576e880851c8@quicinc.com>
Date:   Wed, 6 Dec 2023 17:39:38 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.3.2
Subject: Re: [PATCH V2] scsi: ufs: core: store min and max clk freq from OPP
 table
Content-Language: en-US
To:     Manivannan Sadhasivam <mani@kernel.org>
CC:     "James E.J. Bottomley" <jejb@linux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@oracle.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        <linux-scsi@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <quic_cang@quicinc.com>, Manish Pandey <quic_mapa@quicinc.com>
References: <20231206114659.13009-1-quic_nitirawa@quicinc.com>
 <20231206120246.GB12802@thinkpad>
From:   Nitin Rawat <quic_nitirawa@quicinc.com>
In-Reply-To: <20231206120246.GB12802@thinkpad>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 12/6/2023 5:32 PM, Manivannan Sadhasivam wrote:
> On Wed, Dec 06, 2023 at 05:16:59PM +0530, Nitin Rawat wrote:
>> OPP support added by commit 72208ebe181e ("scsi: ufs: core: Add support
>> for parsing OPP") doesn't update the min_freq and max_freq of each clocks
>> in 'struct ufs_clk_info'.
>>
>> But these values are used by the vendor host drivers internally for
>> controller configuration. When the OPP support is enabled in devicetree,
>> these values will be 0, causing boot issues on the respective platforms.
>>
>> So add support to parse the min_freq and max_freq of all clocks while
>> parsing the OPP table.
>>
>> Fixes: 72208ebe181e ("scsi: ufs: core: Add support for parsing OPP")
>> Co-developed-by: Manish Pandey <quic_mapa@quicinc.com>
>> Signed-off-by: Manish Pandey <quic_mapa@quicinc.com>
>> Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
> 
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>


Hi Mani,
Pushed new patchset v3 with minor change.
Please can you review that.

Regards,
Nitin

> 
> Martin, please queue this patch for 6.7-rcS.
> 
> - Mani
> 
>> ---
>> Changes from v1:
>> As per Manivannan's comment:
>> - Updated commmit description
>> - Sort include file alphabetically
>> - Added missing dev_pm_opp_put
>> - updated function name and documention
>> - removed ret variable
>> ---
>>   drivers/ufs/host/ufshcd-pltfrm.c | 53 ++++++++++++++++++++++++++++++++
>>   1 file changed, 53 insertions(+)
>>
>> diff --git a/drivers/ufs/host/ufshcd-pltfrm.c b/drivers/ufs/host/ufshcd-pltfrm.c
>> index da2558e274b4..409efa0db8fa 100644
>> --- a/drivers/ufs/host/ufshcd-pltfrm.c
>> +++ b/drivers/ufs/host/ufshcd-pltfrm.c
>> @@ -8,6 +8,7 @@
>>    *	Vinayak Holikatti <h.vinayak@samsung.com>
>>    */
>>
>> +#include <linux/clk.h>
>>   #include <linux/module.h>
>>   #include <linux/platform_device.h>
>>   #include <linux/pm_opp.h>
>> @@ -213,6 +214,54 @@ static void ufshcd_init_lanes_per_dir(struct ufs_hba *hba)
>>   	}
>>   }
>>
>> +/**
>> + * ufshcd_parse_clock_min_max_freq  - Parse MIN and MAX clocks freq
>> + * @hba: per adapter instance
>> + *
>> + * This function parses MIN and MAX frequencies of all clocks required
>> + * by the vendor host drivers.
>> + *
>> + * Returns 0 for success and non-zero for failure
>> + */
>> +static int ufshcd_parse_clock_min_max_freq(struct ufs_hba *hba)
>> +{
>> +	struct list_head *head = &hba->clk_list_head;
>> +	struct ufs_clk_info *clki;
>> +	struct dev_pm_opp *opp;
>> +	unsigned long freq;
>> +	u8 idx = 0;
>> +
>> +	list_for_each_entry(clki, head, list) {
>> +		if (!clki->name)
>> +			continue;
>> +
>> +		clki->clk = devm_clk_get(hba->dev, clki->name);
>> +		if (!IS_ERR(clki->clk)) {
>> +			/* Find Max Freq */
>> +			freq = ULONG_MAX;
>> +			opp = dev_pm_opp_find_freq_floor_indexed(hba->dev, &freq, idx);
>> +			if (IS_ERR(opp)) {
>> +				dev_err(hba->dev, "Failed to find OPP for MAX frequency\n");
>> +				return PTR_ERR(opp);
>> +			}
>> +			clki->max_freq = dev_pm_opp_get_freq_indexed(opp, idx);
>> +			dev_pm_opp_put(opp);
>> +
>> +			/* Find Min Freq */
>> +			freq = 0;
>> +			opp = dev_pm_opp_find_freq_ceil_indexed(hba->dev, &freq, idx++);
>> +			if (IS_ERR(opp)) {
>> +				dev_err(hba->dev, "Failed to find OPP for MIN frequency\n");
>> +				return PTR_ERR(opp);
>> +			}
>> +			clki->min_freq = dev_pm_opp_get_freq_indexed(opp, idx);
>> +			dev_pm_opp_put(opp);
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>   static int ufshcd_parse_operating_points(struct ufs_hba *hba)
>>   {
>>   	struct device *dev = hba->dev;
>> @@ -279,6 +328,10 @@ static int ufshcd_parse_operating_points(struct ufs_hba *hba)
>>   		return ret;
>>   	}
>>
>> +	ret = ufshcd_parse_clock_min_max_freq(hba);
>> +	if (ret)
>> +		return ret;
>> +
>>   	hba->use_pm_opp = true;
>>
>>   	return 0;
>> --
>> 2.17.1
>>
>