linux-kernel - Re: [PATCH v8 09/26] memory: tegra30: Support interconnect framework

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]

Date:   Wed, 11 Nov 2020 11:23:13 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     Dmitry Osipenko <digetx@...il.com>
Cc:     Thierry Reding <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Georgi Djakov <georgi.djakov@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Peter De Schrijver <pdeschrijver@...dia.com>,
        MyungJoo Ham <myungjoo.ham@...sung.com>,
        Kyungmin Park <kyungmin.park@...sung.com>,
        Chanwoo Choi <cw00.choi@...sung.com>,
        Mikko Perttunen <cyndis@...si.fi>,
        Viresh Kumar <vireshk@...nel.org>,
        Peter Geis <pgwipeout@...il.com>,
        Nicolas Chauvet <kwizart@...il.com>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        linux-tegra@...r.kernel.org, linux-pm@...r.kernel.org,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v8 09/26] memory: tegra30: Support interconnect framework

On 11-11-20, 04:14, Dmitry Osipenko wrote:
> +static int tegra_emc_opp_table_init(struct tegra_emc *emc)
> +{
> +	struct opp_table *reg_opp_table = NULL, *clk_opp_table, *hw_opp_table;
> +	u32 hw_version = BIT(tegra_sku_info.soc_speedo_id);
> +	const char *rname = "core";
> +	int err;
> +
> +	/*
> +	 * Legacy device-trees don't have OPP table and EMC driver isn't
> +	 * useful in this case.
> +	 */
> +	if (!device_property_present(emc->dev, "operating-points-v2")) {

I don't understand why you want to check this ? The below call to
dev_pm_opp_of_add_table() will fail anyway and that should be good for
you.

> +		dev_err(emc->dev,
> +			"OPP table not found, please update your device tree\n");
> +		return -ENODEV;
> +	}
> +
> +	/* voltage scaling is optional */
> +	if (device_property_present(emc->dev, "core-supply")) {
> +		reg_opp_table = dev_pm_opp_set_regulators(emc->dev, &rname, 1);
> +		if (IS_ERR(reg_opp_table))
> +			return dev_err_probe(emc->dev, PTR_ERR(reg_opp_table),
> +					     "failed to set OPP regulator\n");
> +	}
> +
> +	clk_opp_table = dev_pm_opp_set_clkname(emc->dev, NULL);

I think there is still some misunderstanding here. This call with a
NULL connection id is useful only if the DT OPP table is optional for
your platform and you want the same driver to work with and without
the DT OPP table. Clearly in your case you want the OPP table in the
DT to be there and so this call is not required at all.

> +	err = PTR_ERR_OR_ZERO(clk_opp_table);
> +	if (err) {
> +		dev_err(emc->dev, "failed to set OPP clk: %d\n", err);
> +		goto put_reg_table;
> +	}
> +
> +	hw_opp_table = dev_pm_opp_set_supported_hw(emc->dev, &hw_version, 1);
> +	err = PTR_ERR_OR_ZERO(hw_opp_table);
> +	if (err) {
> +		dev_err(emc->dev, "failed to set OPP supported HW: %d\n", err);
> +		goto put_clk_table;
> +	}
> +
> +	err = dev_pm_opp_of_add_table(emc->dev);
> +	if (err) {
> +		dev_err(emc->dev, "failed to add OPP table: %d\n", err);
> +		goto put_hw_table;
> +	}
> +
> +	dev_info(emc->dev, "OPP HW ver. 0x%x, current clock rate %lu MHz\n",
> +		 hw_version, clk_get_rate(emc->clk) / 1000000);
> +
> +	/* first dummy rate-set initializes voltage state */
> +	err = dev_pm_opp_set_rate(emc->dev, clk_get_rate(emc->clk));
> +	if (err) {
> +		dev_err(emc->dev, "failed to initialize OPP clock: %d\n", err);
> +		goto remove_table;
> +	}
> +
> +	return 0;
> +
> +remove_table:
> +	dev_pm_opp_of_remove_table(emc->dev);
> +put_hw_table:
> +	dev_pm_opp_put_supported_hw(hw_opp_table);
> +put_clk_table:
> +	dev_pm_opp_put_clkname(clk_opp_table);
> +put_reg_table:
> +	if (reg_opp_table)
> +		dev_pm_opp_put_regulators(reg_opp_table);
> +
> +	return err;
> +}

-- 
viresh