linux-kernel - Re: [PATCH 2/2] soc: samsung: Add support for Exynos7 PMU

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: <5783CB72.6030501@samsung.com>
Date:	Mon, 11 Jul 2016 18:38:10 +0200
From:	Sylwester Nawrocki <s.nawrocki@...sung.com>
To:	Abhilash Kesavan <kesavan.abhilash@...il.com>,
	Krzysztof Kozlowski <k.kozlowski@...sung.com>
Cc:	linux-samsung-soc <linux-samsung-soc@...r.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
	Kukjin Kim <kgene.kim@...sung.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] soc: samsung: Add support for Exynos7 PMU

On 07/11/2016 04:44 PM, Abhilash Kesavan wrote:
>>> +       /*
>>> >> +        * Set clock freeze cycle count to 0 before and after arm clamp or
>>> >> +        * reset signal transition
>>> >> +        */
>>> >> +       node = of_find_compatible_node(NULL, NULL,
>>> >> +                               "samsung,exynos7-clock-atlas");
>>> >> +       if (node) {
>>> >> +               atlas_cmu_base = of_iomap(node, 0);
>>> >> +               if (!atlas_cmu_base)
>>> >> +                       return;
>>> >> +
>>> >> +               __raw_writel(0x0,
>>> >> +                               atlas_cmu_base + EXYNOS7_CORE_ARMCLK_STOPCTRL);
>>> >> +               iounmap(atlas_cmu_base);
>> >
>> > Missing:
>> > of_node_put(node);
>> >
>> > ...but I think this creates unnecessary dependency on different
>> > compatible. I understand that disabling the EXTENDED_CLKSTOP is needed
>> > after configuring the PMU so this code belongs here. However
>> > everything you need is just a mapping of CMU address. The PMU driver
>> > should receive in bindings everything it needs to do its work. Either
>> > it is a phandle to something or an address for iomap. In this case the
>> > PMU should probably get two addresses: PMU and optionally CMU (part of
>> > CMU for example). Of course bindings would have to be updated.
>
> I will add an optional CMU phandle to the PMU bindings.

We could additionally split the CMU_ATLAS region into 2 regions in DT
(derived from exynos7420 documentation):

reg = <0x11800000 0xF08>, // offsets 0x0000...0x0F04
      <0x11801000 0x8C>,  // offsets 0x1000...0x1088

so that the first can be mapped by the clk driver and the second by 
the PMU driver? It seems the first region is strictly clock functionality
related, while the second contains power control related and other 
registers.

However I'm not sure it is a good idea, for consistency this would need 
to be done also for CMU_APOLLO, CMU_MIF{0...3}.  All these CMUs don't have 
DT bindings defined yet though and there is no corresponding dts entries.

-- 
Thanks,
Sylwester