From: Krzysztof Kozlowski <k.kozlowski@samsung.com> To: Sylwester Nawrocki <s.nawrocki@samsung.com>, Abhilash Kesavan <kesavan.abhilash@gmail.com> Cc: linux-samsung-soc <linux-samsung-soc@vger.kernel.org>, linux-arm-kernel <linux-arm-kernel@lists.infradead.org>, Kukjin Kim <kgene.kim@samsung.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org> Subject: Re: [PATCH 2/2] soc: samsung: Add support for Exynos7 PMU Date: Tue, 12 Jul 2016 10:58:29 +0200 [thread overview] Message-ID: <5784B135.2060209@samsung.com> (raw) In-Reply-To: <5783CB72.6030501@samsung.com> 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. >
WARNING: multiple messages have this Message-ID (diff)
From: k.kozlowski@samsung.com (Krzysztof Kozlowski) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 2/2] soc: samsung: Add support for Exynos7 PMU Date: Tue, 12 Jul 2016 10:58:29 +0200 [thread overview] Message-ID: <5784B135.2060209@samsung.com> (raw) In-Reply-To: <5783CB72.6030501@samsung.com> 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. >
next prev parent reply other threads:[~2016-07-12 8:58 UTC|newest] Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-07-05 20:28 [PATCH 0/2] Add PMU support for Exynos7 Abhilash Kesavan 2016-07-05 20:28 ` Abhilash Kesavan 2016-07-05 20:28 ` [PATCH 1/2] soc: samsung: Change type of PMU configuration register value to u32 Abhilash Kesavan 2016-07-05 20:28 ` Abhilash Kesavan 2016-07-06 6:47 ` Krzysztof Kozlowski 2016-07-06 6:47 ` Krzysztof Kozlowski 2016-07-06 6:47 ` Krzysztof Kozlowski 2016-07-11 14:44 ` Abhilash Kesavan 2016-07-11 14:44 ` Abhilash Kesavan 2016-07-12 8:49 ` Krzysztof Kozlowski 2016-07-12 8:49 ` Krzysztof Kozlowski 2016-07-05 20:28 ` [PATCH 2/2] soc: samsung: Add support for Exynos7 PMU Abhilash Kesavan 2016-07-05 20:28 ` Abhilash Kesavan 2016-07-06 7:39 ` Krzysztof Kozlowski 2016-07-06 7:39 ` Krzysztof Kozlowski 2016-07-11 14:44 ` Abhilash Kesavan 2016-07-11 14:44 ` Abhilash Kesavan 2016-07-11 16:38 ` Sylwester Nawrocki 2016-07-11 16:38 ` Sylwester Nawrocki 2016-07-12 8:58 ` Krzysztof Kozlowski [this message] 2016-07-12 8:58 ` Krzysztof Kozlowski 2016-07-12 8:49 ` Krzysztof Kozlowski 2016-07-12 8:49 ` Krzysztof Kozlowski
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=5784B135.2060209@samsung.com \ --to=k.kozlowski@samsung.com \ --cc=kesavan.abhilash@gmail.com \ --cc=kgene.kim@samsung.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-samsung-soc@vger.kernel.org \ --cc=s.nawrocki@samsung.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.