lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e6a5129a-db07-977d-2ecd-328a52cbcdc0@linaro.org>
Date:   Wed, 12 Jul 2023 16:15:12 +0300
From:   Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To:     Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
        vireshk@...nel.org, nm@...com, sboyd@...nel.org,
        myungjoo.ham@...sung.com, kyungmin.park@...sung.com,
        cw00.choi@...sung.com, andersson@...nel.org,
        konrad.dybcio@...aro.org, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org
Cc:     linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        quic_asutoshd@...cinc.com, quic_cang@...cinc.com,
        quic_nitirawa@...cinc.com, quic_narepall@...cinc.com,
        quic_bhaskarv@...cinc.com, quic_richardp@...cinc.com,
        quic_nguyenb@...cinc.com, quic_ziqichen@...cinc.com,
        bmasney@...hat.com, krzysztof.kozlowski@...aro.org
Subject: Re: [PATCH 11/14] scsi: ufs: host: Add support for parsing OPP

On 12/07/2023 13:32, Manivannan Sadhasivam wrote:
> OPP framework can be used to scale the clocks along with other entities
> such as regulators, performance state etc... So let's add support for
> parsing OPP from devicetree. OPP support in devicetree is added through
> the "operating-points-v2" property which accepts the OPP table defining
> clock frequency, regulator voltage, power domain performance state etc...
> 
> Since the UFS controller requires multiple clocks to be controlled for
> proper working, devm_pm_opp_set_config() has been used which supports
> scaling multiple clocks through custom ufshcd_opp_config_clks() callback.
> 
> It should be noted that the OPP support is not compatible with the old
> "freq-table-hz" property. So only one can be used at a time even though
> the UFS core supports both.
> 
> Co-developed-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> ---
>   drivers/ufs/host/ufshcd-pltfrm.c | 116 +++++++++++++++++++++++++++++++
>   1 file changed, 116 insertions(+)
> 
> diff --git a/drivers/ufs/host/ufshcd-pltfrm.c b/drivers/ufs/host/ufshcd-pltfrm.c
> index 0b7430033047..068c22378c88 100644
> --- a/drivers/ufs/host/ufshcd-pltfrm.c
> +++ b/drivers/ufs/host/ufshcd-pltfrm.c
> @@ -8,8 +8,10 @@
>    *	Vinayak Holikatti <h.vinayak@...sung.com>
>    */
>   
> +#include <linux/clk.h>
>   #include <linux/module.h>
>   #include <linux/platform_device.h>
> +#include <linux/pm_opp.h>
>   #include <linux/pm_runtime.h>
>   #include <linux/of.h>
>   
> @@ -17,6 +19,8 @@
>   #include "ufshcd-pltfrm.h"
>   #include <ufs/unipro.h>
>   
> +#include <trace/events/ufs.h>
> +
>   #define UFSHCD_DEFAULT_LANES_PER_DIRECTION		2
>   
>   static int ufshcd_parse_clock_info(struct ufs_hba *hba)
> @@ -205,6 +209,112 @@ static void ufshcd_init_lanes_per_dir(struct ufs_hba *hba)
>   	}
>   }
>   
> +static int ufshcd_opp_config_clks(struct device *dev, struct opp_table *opp_table,
> +				  struct dev_pm_opp *opp, void *data,
> +				  bool scaling_down)
> +{
> +	struct ufs_hba *hba = dev_get_drvdata(dev);
> +	struct list_head *head = &hba->clk_list_head;
> +	struct ufs_clk_info *clki;
> +	unsigned long freq;
> +	u8 idx = 0;
> +	int ret;
> +
> +	list_for_each_entry(clki, head, list) {
> +		if (!IS_ERR_OR_NULL(clki->clk)) {
> +			freq = dev_pm_opp_get_freq_indexed(opp, idx++);
> +
> +			/* Do not set rate for clocks having frequency as 0 */
> +			if (!freq)
> +				continue;

Can we omit these clocks from the opp table? I don't think they serve 
any purpose.

Maybe it would even make sense to move this function to drivers/opp 
then, as it will be generic enough.

> +
> +			ret = clk_set_rate(clki->clk, freq);
> +			if (ret) {
> +				dev_err(dev, "%s: %s clk set rate(%ldHz) failed, %d\n",
> +					__func__, clki->name, freq, ret);
> +				return ret;
> +			}
> +
> +			trace_ufshcd_clk_scaling(dev_name(dev),
> +				(scaling_down ? "scaled down" : "scaled up"),
> +				clki->name, hba->clk_scaling.target_freq, freq);
> +		}
> +	}
> +
> +	return 0;
> +} > +
> +static int ufshcd_parse_operating_points(struct ufs_hba *hba)
> +{
> +	struct device *dev = hba->dev;
> +	struct device_node *np = dev->of_node;
> +	struct dev_pm_opp_config config = {};
> +	struct ufs_clk_info *clki;
> +	const char **clk_names;
> +	int cnt, i, ret;
> +
> +	if (!of_find_property(np, "operating-points-v2", NULL))
> +		return 0;
> +
> +	if (of_find_property(np, "freq-table-hz", NULL)) {
> +		dev_err(dev, "%s: operating-points and freq-table-hz are incompatible\n",
> +			 __func__);
> +		return -EINVAL;
> +	}
> +
> +	cnt = of_property_count_strings(np, "clock-names");
> +	if (cnt <= 0) {
> +		dev_err(dev, "%s: Missing clock-names\n",  __func__);
> +		return -ENODEV;
> +	}
> +
> +	/* OPP expects clk_names to be NULL terminated */
> +	clk_names = devm_kcalloc(dev, cnt + 1, sizeof(*clk_names), GFP_KERNEL);
> +	if (!clk_names)
> +		return -ENOMEM;
> +
> +	/*
> +	 * We still need to get reference to all clocks as the UFS core uses
> +	 * them separately.
> +	 */
> +	for (i = 0; i < cnt; i++) {
> +		ret = of_property_read_string_index(np, "clock-names", i,
> +						    &clk_names[i]);
> +		if (ret)
> +			return ret;
> +
> +		clki = devm_kzalloc(dev, sizeof(*clki), GFP_KERNEL);
> +		if (!clki)
> +			return -ENOMEM;
> +
> +		clki->name = devm_kstrdup(dev, clk_names[i], GFP_KERNEL);
> +		if (!clki->name)
> +			return -ENOMEM;
> +
> +		if (!strcmp(clk_names[i], "ref_clk"))
> +			clki->keep_link_active = true;
> +
> +		list_add_tail(&clki->list, &hba->clk_list_head);
> +	}
> +
> +	config.clk_names = clk_names,
> +	config.config_clks = ufshcd_opp_config_clks;
> +
> +	ret = devm_pm_opp_set_config(dev, &config);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_pm_opp_of_add_table(dev);
> +	if (ret) {
> +		dev_err(dev, "Failed to add OPP table: %d\n", ret);
> +		return ret;
> +	}
> +
> +	hba->use_pm_opp = true;
> +
> +	return 0;
> +}
> +
>   /**
>    * ufshcd_get_pwr_dev_param - get finally agreed attributes for
>    *                            power mode change
> @@ -371,6 +481,12 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
>   
>   	ufshcd_init_lanes_per_dir(hba);
>   
> +	err = ufshcd_parse_operating_points(hba);
> +	if (err) {
> +		dev_err(dev, "%s: OPP parse failed %d\n", __func__, err);
> +		goto dealloc_host;
> +	}
> +
>   	err = ufshcd_init(hba, mmio_base, irq);
>   	if (err) {
>   		dev_err(dev, "Initialization failed\n");

-- 
With best wishes
Dmitry

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ