[PATCH V2] scsi: ufs: core: store min and max clk freq from OPP table

[PATCH V2] scsi: ufs: core: store min and max clk freq from OPP table

[Nitin Rawat]

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>
---
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

Re: [PATCH V2] scsi: ufs: core: store min and max clk freq from OPP table

[Manivannan Sadhasivam]

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>

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
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH V2] scsi: ufs: core: store min and max clk freq from OPP table

[Nitin Rawat]

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
>>
> 

Re: [PATCH V2] scsi: ufs: core: store min and max clk freq from OPP table

[Christoph Hellwig]

On Wed, Dec 06, 2023 at 05:16:59PM +0530, Nitin Rawat wrote:
> But these values are used by the vendor host drivers internally for

There is no such thing as a "vendor" driver.  Please be precise with
your wording.