linux-kernel - Re: [PATCH 03/21] spi: spi-geni-qcom: Use OPP API to set clk/perf state

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: <20200409182021.GT199755@google.com>
Date:   Thu, 9 Apr 2020 11:20:21 -0700
From:   Matthias Kaehlcke <mka@...omium.org>
To:     Rajendra Nayak <rnayak@...eaurora.org>
Cc:     viresh.kumar@...aro.org, sboyd@...nel.org,
        bjorn.andersson@...aro.org, agross@...nel.org,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, Alok Chauhan <alokc@...eaurora.org>,
        Akash Asthana <akashast@...eaurora.org>,
        linux-spi@...r.kernel.org
Subject: Re: [PATCH 03/21] spi: spi-geni-qcom: Use OPP API to set clk/perf
 state

Hi Rajendra,

On Wed, Apr 08, 2020 at 07:16:29PM +0530, Rajendra Nayak wrote:
> geni spi needs to express a perforamnce state requirement on CX
> depending on the frequency of the clock rates. Use OPP table from
> DT to register with OPP framework and use dev_pm_opp_set_rate() to
> set the clk/perf state.
> 
> Signed-off-by: Rajendra Nayak <rnayak@...eaurora.org>
> Cc: Alok Chauhan <alokc@...eaurora.org>
> Cc: Akash Asthana <akashast@...eaurora.org>
> Cc: linux-spi@...r.kernel.org
> ---
>  drivers/spi/spi-geni-qcom.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index c397242..ce387dc 100644
> --- a/drivers/spi/spi-geni-qcom.c
> +++ b/drivers/spi/spi-geni-qcom.c
> @@ -7,6 +7,7 @@
>  #include <linux/log2.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_opp.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/qcom-geni-se.h>
>  #include <linux/spi/spi.h>
> @@ -95,7 +96,6 @@ static int get_spi_clk_cfg(unsigned int speed_hz,
>  {
>  	unsigned long sclk_freq;
>  	unsigned int actual_hz;
> -	struct geni_se *se = &mas->se;
>  	int ret;
>  
>  	ret = geni_se_clk_freq_match(&mas->se,
> @@ -112,9 +112,9 @@ static int get_spi_clk_cfg(unsigned int speed_hz,
>  
>  	dev_dbg(mas->dev, "req %u=>%u sclk %lu, idx %d, div %d\n", speed_hz,
>  				actual_hz, sclk_freq, *clk_idx, *clk_div);
> -	ret = clk_set_rate(se->clk, sclk_freq);
> +	ret = dev_pm_opp_set_rate(mas->dev, sclk_freq);
>  	if (ret)
> -		dev_err(mas->dev, "clk_set_rate failed %d\n", ret);
> +		dev_err(mas->dev, "dev_pm_opp_set_rate failed %d\n", ret);
>  	return ret;
>  }
>  
> @@ -553,6 +553,7 @@ static int spi_geni_probe(struct platform_device *pdev)
>  	if (!spi)
>  		return -ENOMEM;
>  
> +
>  	platform_set_drvdata(pdev, spi);
>  	mas = spi_master_get_devdata(spi);
>  	mas->irq = irq;
> @@ -561,6 +562,8 @@ static int spi_geni_probe(struct platform_device *pdev)
>  	mas->se.wrapper = dev_get_drvdata(dev->parent);
>  	mas->se.base = base;
>  	mas->se.clk = clk;
> +	mas->se.opp = dev_pm_opp_set_clkname(&pdev->dev, "se");

As commented on the serial patch, it seems an error check is needed
and the OPP table saved in 'struct geni_se' is never used.

> +	dev_pm_opp_of_add_table(&pdev->dev);

This function could also fail for multiple reasons, so the return value
should be checked.

>From patch "[01/21] opp: Manage empty OPP tables with clk handle" it seems
ignoring errors is intended to be able to operate when no OPP table is
specified. But even with that you want to return in case of certain errors,
like an invalid OPP table, out of memory or -EPROBE_DEFER.