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

Date:	Tue, 12 Jul 2016 10:58:29 +0200
From:	Krzysztof Kozlowski <k.kozlowski@...sung.com>
To:	Sylwester Nawrocki <s.nawrocki@...sung.com>,
	Abhilash Kesavan <kesavan.abhilash@...il.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 06:38 PM, Sylwester Nawrocki wrote:
> 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.

Do the clk-exynos7 driver really needs the second region? If not then it
would be sufficient just to reduce the mapping range for CMU_ATLAS.

Best regards,
Krzysztof

> 
> 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.
>