From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030907AbcGKQiU (ORCPT ); Mon, 11 Jul 2016 12:38:20 -0400 Received: from mailout3.w1.samsung.com ([210.118.77.13]:38575 "EHLO mailout3.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030609AbcGKQiS (ORCPT ); Mon, 11 Jul 2016 12:38:18 -0400 X-AuditID: cbfec7f5-f792a6d000001302-d1-5783cb76c417 Subject: Re: [PATCH 2/2] soc: samsung: Add support for Exynos7 PMU To: Abhilash Kesavan , Krzysztof Kozlowski References: <1467750507-13853-1-git-send-email-a.kesavan@samsung.com> <1467750507-13853-3-git-send-email-a.kesavan@samsung.com> Cc: linux-samsung-soc , linux-arm-kernel , Kukjin Kim , "linux-kernel@vger.kernel.org" From: Sylwester Nawrocki Message-id: <5783CB72.6030501@samsung.com> Date: Mon, 11 Jul 2016 18:38:10 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-version: 1.0 In-reply-to: Content-type: text/plain; charset=utf-8 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrHLMWRmVeSWpSXmKPExsVy+t/xK7plp5vDDfYcV7F4/cLQYs1fJYve BVfZLDY9vsZqcXnXHDaLGef3MTmweeycdZfdY/OSeo++LasYPT5vkgtgieKySUnNySxLLdK3 S+DKONDXxlJwSKDi0OmnzA2MXTxdjJwcEgImEjPvv2OGsMUkLtxbz9bFyMUhJLCUUWJ271x2 COc5o8S3J+0sIFXCAk4Sxx/8A+rg4BARSJKY8K8CoqafSeLkvdtMIA6zwG1GiVlrpjGBNLAJ GEr0Hu1jBLF5BbQkrk6eCxZnEVCV+PJqMRuILSoQIfFk7kmoGkGJH5PvgS3jFAiWWHnuHyvI MmYBdYkpU3JBwswC8hKb17xlnsAoMAtJxyyEqllIqhYwMq9iFE0tTS4oTkrPNdIrTswtLs1L 10vOz93ECAnlrzsYlx6zOsQowMGoxMPbcbI5XIg1say4MvcQowQHs5IIb9QRoBBvSmJlVWpR fnxRaU5q8SFGaQ4WJXHembvehwgJpCeWpGanphakFsFkmTg4pRoYRXPC9zNd5/pRu/TEgxhd Zlt7w4T49d2cK4/uYmT59pn3cn//lNniTl+Sk/kvFzBMO79SvHsRt1zHPMPa3QyXiy+3yluL xW+6MCP56875Ye2HNfSrV2904/X9G5j/WeGsmtc+k3dzf37X/vbzc4fmyUt1B1skDVb4nprn Liy60a0luXbX9EsHlViKMxINtZiLihMBIE8WlWECAAA= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: s.nawrocki@samsung.com (Sylwester Nawrocki) Date: Mon, 11 Jul 2016 18:38:10 +0200 Subject: [PATCH 2/2] soc: samsung: Add support for Exynos7 PMU In-Reply-To: References: <1467750507-13853-1-git-send-email-a.kesavan@samsung.com> <1467750507-13853-3-git-send-email-a.kesavan@samsung.com> Message-ID: <5783CB72.6030501@samsung.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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