From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752678AbcGLI6h (ORCPT ); Tue, 12 Jul 2016 04:58:37 -0400 Received: from mailout1.w1.samsung.com ([210.118.77.11]:27750 "EHLO mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750778AbcGLI6e (ORCPT ); Tue, 12 Jul 2016 04:58:34 -0400 X-AuditID: cbfec7f4-f796c6d000001486-a6-5784b13605f8 Subject: Re: [PATCH 2/2] soc: samsung: Add support for Exynos7 PMU To: Sylwester Nawrocki , Abhilash Kesavan References: <1467750507-13853-1-git-send-email-a.kesavan@samsung.com> <1467750507-13853-3-git-send-email-a.kesavan@samsung.com> <5783CB72.6030501@samsung.com> Cc: linux-samsung-soc , linux-arm-kernel , Kukjin Kim , "linux-kernel@vger.kernel.org" From: Krzysztof Kozlowski Message-id: <5784B135.2060209@samsung.com> Date: Tue, 12 Jul 2016 10:58:29 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-version: 1.0 In-reply-to: <5783CB72.6030501@samsung.com> Content-type: text/plain; charset=utf-8 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrMLMWRmVeSWpSXmKPExsVy+t/xy7pmG1vCDRa8NbN4/cLQYs1fJYve BVfZLDY9vsZqcXnXHDaLGef3MVkcftPO6sDusXPWXXaPzUvqPfq2rGL0+LxJLoAlissmJTUn syy1SN8ugSvj0/YtTAWbhCru7dvF2MB4i7eLkZNDQsBE4vKlBewQtpjEhXvr2boYuTiEBJYy Slw4+QnKecYocXXibyaQKmEBJ4njD/4xg9giAgkSq5a+ZIQo2sAk8eP3BhYQh1ngNqPErDXT wDrYBIwlNi9fAjSKg4NXQEvidJ8JSJhFQFVi8cU7rCC2qECExKztP8DKeQUEJX5MvscCYnMK aEvMurALrJVZQF1iypRckDCzgLzE5jVvmScwCsxC0jELoWoWkqoFjMyrGEVTS5MLipPScw31 ihNzi0vz0vWS83M3MUKC+ssOxsXHrA4xCnAwKvHwPjjdHC7EmlhWXJl7iFGCg1lJhHfN+pZw Id6UxMqq1KL8+KLSnNTiQ4zSHCxK4rxzd70PERJITyxJzU5NLUgtgskycXBKNTB2VKkfPxbf GV3RLuMy0fzk3+rqVb/E1rN1NHxm0r739+SJHRszuhwqDF19ei43vfzddtbedqXS11AXpsfh y4uKLl72OHnO843E8srPjxb/vHbgafZ/m5DVV2d3fNq5zl69Il7SmXHutNfeJ3xfFYUKfnVp mDSlJutw6Bm5AIaFOwW8/x9YOvuaEktxRqKhFnNRcSIA/CX2JmYCAAA= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. >