linux-kernel - Re: [PATCH v4 4/4] ARM: EXYNOS: add Exynos Dual Cluster Support

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.10.1311271246010.9667@knanqh.ubzr>
Date:	Wed, 27 Nov 2013 13:00:33 -0500 (EST)
From:	Nicolas Pitre <nicolas.pitre@...aro.org>
To:	Vyacheslav Tyrtov <v.tyrtov@...sung.com>
cc:	linux-kernel@...r.kernel.org,
	Rob Herring <rob.herring@...xeda.com>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Stephen Warren <swarren@...dotorg.org>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Rob Landley <rob@...dley.net>,
	Kukjin Kim <kgene.kim@...sung.com>,
	Russell King <linux@....linux.org.uk>,
	Ben Dooks <ben-linux@...ff.org>,
	Mike Turquette <mturquette@...aro.org>,
	Daniel Lezcano <daniel.lezcano@...aro.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Heiko Stuebner <heiko@...ech.de>,
	Naour Romain <romain.naour@...nwide.fr>,
	devicetree@...r.kernel.org, linux-doc@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	linux-samsung-soc@...r.kernel.org,
	Tarek Dakhran <t.dakhran@...sung.com>,
	Dave Martin <Dave.Martin@....com>, tomasz.figa@...il.com
Subject: Re: [PATCH v4 4/4] ARM: EXYNOS: add Exynos Dual Cluster Support

On Tue, 26 Nov 2013, Vyacheslav Tyrtov wrote:

> From: Tarek Dakhran <t.dakhran@...sung.com>
> 
> Add EDCS(Exynos Dual Cluster Support) for Samsung Exynos5410 SoC.
> This enables all 8 cores, 4 x A7 and 4 x A15 run at the same time.
> 
> Signed-off-by: Tarek Dakhran <t.dakhran@...sung.com>
> Signed-off-by: Vyacheslav Tyrtov <v.tyrtov@...sung.com>

Just some minor comments in addition to those Dave already provided.

[...]
> +/*
> + * Enable cluster-level coherency, in preparation for turning on the MMU.
> + */
> +static void __naked edcs_power_up_setup(unsigned int affinity_level)
> +{
> +	asm volatile ("\n"
> +	"b	cci_enable_port_for_self");
> +}

You should need to turn on the CCI port only for the first CPU to power 
up a cluster.  This is indicated by the affinity level passed into r0.  
See tc2_pm_power_up_setup for example.

> +static int __init edcs_init(void)
> +{
> +	int ret;
> +	struct device_node *node;
> +
> +	node = of_find_compatible_node(NULL, NULL, "samsung,exynos5410");
> +	if (!node)
> +		return -ENODEV;
> +
> +	if (!cci_probed())
> +		return -ENODEV;
> +
> +	/*
> +	 * Future entries into the kernel can now go
> +	 * through the cluster entry vectors.
> +	 */
> +	writel_relaxed(virt_to_phys(mcpm_entry_point), REG_ENTRY_ADDR);
> +
> +	edcs_data_init();
> +	mcpm_smp_set_ops();
> +
> +	ret = mcpm_platform_register(&edcs_power_ops);
> +	if (!ret) {
> +		mcpm_sync_init(edcs_power_up_setup);
> +		pr_info("EDCS power management initialized\n");
> +	}
> +	return ret;
> +}

Here I'd suggest initializing EG_ENTRY_ADDR only after mcpm_sync_init() 
is done.  If ever a secondary CPU, seeing that the address is no longer 
zero, decides to enter the kernel while the state machine is not 
initialized yet, might lead to all sorts of funny behaviors.


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/