linux-kernel - Re: [PATCH v4 1/9] devfreq: exynos: Add generic exynos memory bus frequency driver

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-next>] [day] [month] [year] [list]

Date:	Mon, 19 Jan 2015 09:20:17 +0000 (GMT)
From:	MyungJoo Ham <myungjoo.ham@...sung.com>
To:	최찬우 <cw00.choi@...sung.com>,
	"kgene@...nel.org" <kgene@...nel.org>
Cc:	박경민 <kyungmin.park@...sung.com>,
	"rafael.j.wysocki@...el.com" <rafael.j.wysocki@...el.com>,
	"mark.rutland@....com" <mark.rutland@....com>,
	ABHILASH KESAVAN <a.kesavan@...sung.com>,
	"tomasz.figa@...il.com" <tomasz.figa@...il.com>,
	Krzysztof Kozlowski <k.kozlowski@...sung.com>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
	"robh+dt@...nel.org" <robh+dt@...nel.org>,
	대인기 <inki.dae@...sung.com>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-samsung-soc@...r.kernel.org" 
	<linux-samsung-soc@...r.kernel.org>
Subject: Re: [PATCH v4 1/9] devfreq: exynos: Add generic exynos memory bus
 frequency driver

>   
>  This patch adds the generic exynos bus frequency driver for memory bus
> with DEVFREQ framework. The Samsung Exynos SoCs have the common architecture
> for memory bus between DRAM memory and MMC/sub IP in SoC. This driver can
> support the memory bus frequency driver for Exynos SoCs.
> 
> Each memory bus block has a clock for memory bus speed and frequency
> table which is changed according to the utilization of memory bus on runtime.
> And then each memory bus group has the one more memory bus blocks and
> OPP table (including frequency and voltage), regulator, devfreq-event
> devices.
> 
> There are a little difference about the number of memory bus because each Exynos
> SoC have the different sub-IP and different memory bus speed. In spite of this
> difference among Exynos SoCs, we can support almost Exynos SoC by adding
> unique data of memory bus to devicetree file.
> 
> Cc: Myungjoo Ham <myungjoo.ham@...sung.com>
> Cc: Kyungmin Park <kyungmin.park@...sung.com>
> Cc: Kukjin Kim <kgene@...nel.org>
> Signed-off-by: Chanwoo Choi <cw00.choi@...sung.com>
> ---
>  drivers/devfreq/Kconfig             |  15 +
>  drivers/devfreq/Makefile            |   1 +
>  drivers/devfreq/exynos/Makefile     |   1 +
>  drivers/devfreq/exynos/exynos-bus.c | 598 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 615 insertions(+)
>  create mode 100644 drivers/devfreq/exynos/exynos-bus.c
> 

[]

> +static void exynos_bus_exit(struct device *dev)
> +{
> +	struct exynos_memory_bus *bus = dev_get_drvdata(dev);
> +	int i, ret;
> +
> +	ret = exynos_bus_disable_edev(bus);
> +	if (ret < 0)
> +		dev_warn(dev, "failed to disable the devfreq-event devices\n");
> +
> +	for (i = 0; i < bus->block_count; i++)
> +		clk_disable_unprepare(bus->block[i].clk);
> +
> +	if (regulator_is_enabled(bus->regulator))
> +		regulator_disable(bus->regulator);

This is_enabled check is itchy.

Why do you need this here?
Please let me know what kind of errors here.
(note that this may simply hide errors made by other drivers)

Adding this condition does not introduce additional error, but
could you please let me know why it is here? This is supposed to be
paired with probe.


Except this point (valid if addressed or {explained and understood}),
Acked-by: MyungJoo Ham <myungjoo.ham@...sung.com>


Cheers,
MyungJoo

> +
> +	of_free_opp_table(dev);
> +}
> +
> +static struct devfreq_dev_profile exynos_memory_bus_profile = {
> +	.polling_ms	= 100,
> +	.target		= exynos_bus_target,
> +	.get_dev_status	= exynos_bus_get_dev_status,
> +	.exit		= exynos_bus_exit,
> +};
> +

[]