linux-kernel - Re: [RFC PATCH 3/4] PM / devfreq: rockchip: add devfreq driver for rk3399 dmc

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, 1 Jun 2016 19:47:10 +0900
From:	MyungJoo Ham <myungjoo.ham@...sung.com>
To:	Lin Huang <hl@...k-chips.com>
Cc:	Heiko Stübner <heiko@...ech.de>,
	mark.yao@...k-chips.com, airlied@...ux.ie, mturquette@...libre.com,
	dbasehore@...omium.org, Stephen Boyd <sboyd@...eaurora.org>,
	LKML <linux-kernel@...r.kernel.org>,
	dri-devel@...ts.freedesktop.org, dianders@...omium.org,
	linux-rockchip@...ts.infradead.org,
	Kyungmin Park <kyungmin.park@...sung.com>,
	linux-clk@...r.kernel.org,
	linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [RFC PATCH 3/4] PM / devfreq: rockchip: add devfreq driver for
 rk3399 dmc

On Wed, Jun 1, 2016 at 6:35 PM, Lin Huang <hl@...k-chips.com> wrote:
> there is dfi controller on rk3399 platform, it can monitor
> ddr load, register this controller to devfreq framework, and
> default to use simple_ondeamnd policy, and do ddr frequency
> scaling base on this result.
>
> Signed-off-by: Lin Huang <hl@...k-chips.com>
> ---
>  drivers/devfreq/Kconfig                 |   2 +-
>  drivers/devfreq/Makefile                |   1 +
>  drivers/devfreq/rockchip/Kconfig        |  14 +
>  drivers/devfreq/rockchip/Makefile       |   2 +
>  drivers/devfreq/rockchip/rk3399_dmc.c   | 438 ++++++++++++++++++++++++++++++++
>  drivers/devfreq/rockchip/rockchip_dmc.c | 132 ++++++++++
>  include/soc/rockchip/rockchip_dmc.h     |  44 ++++
>  7 files changed, 632 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/devfreq/rockchip/Kconfig
>  create mode 100644 drivers/devfreq/rockchip/Makefile
>  create mode 100644 drivers/devfreq/rockchip/rk3399_dmc.c
>  create mode 100644 drivers/devfreq/rockchip/rockchip_dmc.c
>  create mode 100644 include/soc/rockchip/rockchip_dmc.h
>
> +
> +       /* check the rate we get whether correct */
> +       dmcfreq->rate = clk_get_rate(dmcfreq->dmc_clk);
> +       if (dmcfreq->rate != target_rate) {
> +               dev_err(dev, "get wrong ddr frequency, Request freq %lu,\
> +                       Current freq %lu\n", target_rate, dmcfreq->rate);
> +               regulator_set_voltage(dmcfreq->vdd_center, dmcfreq->volt,
> +                                     dmcfreq->volt);

Why do you need to check this and
more importantly, why do you assume that dmvfreq->volt
will be safe in this case? (what if the new dmcfreq->rate is
larger than the old dmcfreq->rate after clk_get_rate?)
Note that you didn't update dmcfreq->volt after clk_get_rate()


> +EXPORT_SYMBOL_GPL(dmc_event);
> +EXPORT_SYMBOL_GPL(rockchip_dmc_enabled);
> +EXPORT_SYMBOL_GPL(rockchip_dmc_enable);
> +EXPORT_SYMBOL_GPL(rockchip_dmc_disable);
> +EXPORT_SYMBOL_GPL(dmc_register_notifier);
> +EXPORT_SYMBOL_GPL(dmc_unregister_notifier);
> +EXPORT_SYMBOL_GPL(rockchip_dmc_get);
> +EXPORT_SYMBOL_GPL(rockchip_dmc_put);

Do you really need to export all these device driver specific
functions? Looks like a design flaw here.



Cheers,
MyungJoo


-- 
MyungJoo Ham, Ph.D.
S/W R&D Center, Samsung Electronics