From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753486AbbLJHM0 (ORCPT ); Thu, 10 Dec 2015 02:12:26 -0500 Received: from mailout2.w1.samsung.com ([210.118.77.12]:31823 "EHLO mailout2.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751358AbbLJHMY (ORCPT ); Thu, 10 Dec 2015 02:12:24 -0500 MIME-version: 1.0 Content-type: text/plain; charset=utf-8 X-AuditID: cbfec7f5-f79b16d000005389-0d-566925d5cc82 Content-transfer-encoding: 8BIT Subject: Re: [PATCH v2 14/19] ARM: dts: Add bus nodes using VDD_INT for Exynos4x12 To: Chanwoo Choi , myungjoo.ham@samsung.com, kgene@kernel.org References: <1449634091-1842-1-git-send-email-cw00.choi@samsung.com> <1449634091-1842-15-git-send-email-cw00.choi@samsung.com> <56691458.9000000@samsung.com> <566916E5.5060704@samsung.com> <56691C70.6080208@samsung.com> <56691F29.1020808@samsung.com> <5669215A.70104@samsung.com> <566924BA.2080103@samsung.com> Cc: kyungmin.park@samsung.com, robh+dt@kernel.org, pawel.moll@arm.com, mark.rutland@arm.com, ijc+devicetree@hellion.org.uk, galak@codeaurora.org, linux@arm.linux.org.uk, tjakobi@math.uni-bielefeld.de, linux.amoon@gmail.com, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, linux-samsung-soc@vger.kernel.org, devicetree@vger.kernel.org From: Krzysztof Kozlowski Message-id: <566925CD.4040205@samsung.com> Date: Thu, 10 Dec 2015 16:12:13 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 In-reply-to: <566924BA.2080103@samsung.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprDIsWRmVeSWpSXmKPExsVy+t/xq7pXVTPDDDr/m1pc//Kc1WL+kXOs Fv1vFrJanHu1ktHi9QtDi/7Hr5ktzja9Ybe4vGsOm8Xn3iOMFjPO72OyWLfxFrvF7cu8Fkuv X2SyuN24gs1iwvS1LBate4+wW7St/sDqIOixZt4aRo+W5h42j8t9vUweO2fdZfdYufwLm8em VZ1sHv+OsXv0bVnF6PF5k1wAZxSXTUpqTmZZapG+XQJXxr0vvWwF8/Qr7v+dwtbA+EKti5GT Q0LARKLn7m12CFtM4sK99WxdjFwcQgJLGSUubmxiBknwCghK/Jh8j6WLkYODWUBe4silbAhT XWLKlFyI8l+MEm83n2MEKRcWCJHYNG8NK4gtIhAmMXvGD2aIon1MEn9nd7GDOMwCd5kkLr5/ wgZSxSZgLLF5+RI2iGVaEnM+vgLrZhFQldi69ywbyDZRgQiJRTsyQcKcAtoSH/va2SYwCsxC ct4shPNmIZy3gJF5FaNoamlyQXFSeq6RXnFibnFpXrpecn7uJkZInH3dwbj0mNUhRgEORiUe 3hfSmWFCrIllxZW5hxglOJiVRHiP/swIE+JNSaysSi3Kjy8qzUktPsQozcGiJM47c9f7ECGB 9MSS1OzU1ILUIpgsEwenVANj9dHyO2vkOXaEKfZ9fbPnwSaR1pXvN900c88PmXLP4LnAjxKv 7ifG7/erHfjW6hEg8Xaq9cvm5w9vrti31k9xt+IqwW882zkXtNwTTNmbHzApxX7VWQlD47N2 wQUffzw+5zqBYeKFGcd/GHgu69L1+v//k5xSPqPljet5WucunDxpHOT5WYfXXYmlOCPRUIu5 qDgRAPpl23uvAgAA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10.12.2015 16:07, Chanwoo Choi wrote: > On 2015년 12월 10일 15:53, Krzysztof Kozlowski wrote: >> On 10.12.2015 15:43, Chanwoo Choi wrote: >>> On 2015년 12월 10일 15:32, Krzysztof Kozlowski wrote: >>>> On 10.12.2015 15:08, Chanwoo Choi wrote: >>>>> On 2015년 12월 10일 14:57, Krzysztof Kozlowski wrote: >>>>>> On 09.12.2015 13:08, Chanwoo Choi wrote: >>>>>>> This patch adds the bus noes using VDD_INT for Exynos4x12 SoC. >>>>>>> Exynos4x12 has the following AXI buses to translate data between >>>>>>> DRAM and sub-blocks. >>>>>>> >>>>>>> Following list specifies the detailed relation between DRAM and sub-blocks: >>>>>>> - ACLK100 clock for PERIL/PERIR/MFC(PCLK) >>>>>>> - ACLK160 clock for CAM/TV/LCD >>>>>>> : The minimum clock of ACLK160 should be over 160MHz. >>>>>>> When drop the clock under 160MHz, show the broken image. >>>>>>> - ACLK133 clock for FSYS >>>>>>> - GDL clock for LEFTBUS >>>>>>> - GDR clock for RIGHTBUS >>>>>>> - SCLK_MFC clock for MFC >>>>>>> >>>>>>> Signed-off-by: Chanwoo Choi >>>>>>> --- >>>>>>> arch/arm/boot/dts/exynos4x12.dtsi | 112 ++++++++++++++++++++++++++++++++++++++ >>>>>>> 1 file changed, 112 insertions(+) >>>>>>> >>>>>>> diff --git a/arch/arm/boot/dts/exynos4x12.dtsi b/arch/arm/boot/dts/exynos4x12.dtsi >>>>>>> index 3bcf0939755e..8bc4aee156b5 100644 >>>>>>> --- a/arch/arm/boot/dts/exynos4x12.dtsi >>>>>>> +++ b/arch/arm/boot/dts/exynos4x12.dtsi >>>>>>> @@ -354,6 +354,118 @@ >>>>>>> opp-microvolt = <950000>; >>>>>>> }; >>>>>>> }; >>>>>>> + >>>>>>> + bus_leftbus: bus_leftbus { >>>>>>> + compatible = "samsung,exynos-bus"; >>>>>>> + clocks = <&clock CLK_DIV_GDL>; >>>>>>> + clock-names = "bus"; >>>>>>> + operating-points-v2 = <&bus_leftbus_opp_table>; >>>>>>> + status = "disabled"; >>>>>>> + }; >>>>>>> + >>>>>>> + bus_rightbus: bus_rightbus { >>>>>>> + compatible = "samsung,exynos-bus"; >>>>>>> + clocks = <&clock CLK_DIV_GDR>; >>>>>>> + clock-names = "bus"; >>>>>>> + operating-points-v2 = <&bus_leftbus_opp_table>; >>>>>>> + status = "disabled"; >>>>>>> + }; >>>>>> >>>>>> These two nodes are symmetrical. The MFC below and other buses in other >>>>>> DTS share opps. How about changing the binding so multiple clocks could >>>>>> be specified at once ("bus0", "bus1")? I think there is no need for a >>>>>> bus device for each clock. >>>>> >>>>> The your commented method is possible. >>>>> >>>>> But, I focus on implementing the generic bus frequency driver. >>>>> >>>>> If specific bus device-tree node includes the one more clocks, >>>>> when adding the new Exynos SoC, the exynos-bus.c should be added >>>>> for new Exynos SoC. Because, each Exynos SoC has the different >>>>> set of bus device. >>>>> >>>>> If we use my approach, we don't need to modify the exynos-bus.c >>>>> driver to support for the bus frequency of new Exynos SoC. >>>> >>>> This won't change. The driver will just support from 1 to N clocks for >>>> given bus device and set the same OPP to all of them. This will only >>>> limit the number of duplicated entries. This won't affect the generic >>>> approach of driver itself. >>> >>> You're right aspect of only implementation of device driver. >>> >>> But, >>> If we use your commented approach, we can show the information >>> of only parent device through sysfs. We cannot see the information >>> of passive device. The some information includes the current >>> frequency and correlation of parent device. (But, current patchset >>> don' include the topology information between parent device and >>> passive device. I'll do it on later patches). >>> >>> For example, >>> We can see the following bus device through /sys/class/devfreq. >>> >>> drwxr-xr-x 2 root root 0 Dec 31 17:00 . >>> drwxr-xr-x 44 root root 0 Dec 31 17:00 .. >>> lrwxrwxrwx 1 root root 0 Dec 31 17:00 bus_display -> ../../devices/platform/bus_display/devfreq/bus_display >>> lrwxrwxrwx 1 root root 0 Dec 31 17:00 bus_fsys -> ../../devices/platform/bus_fsys/devfreq/bus_fsys >>> lrwxrwxrwx 1 root root 0 Dec 31 17:00 bus_leftbus -> ../../devices/platform/bus_leftbus/devfreq/bus_leftbus >>> lrwxrwxrwx 1 root root 0 Dec 31 17:00 bus_peri -> ../../devices/platform/bus_peri/devfreq/bus_peri >>> >>> >>> We don't see the following bus device because of following bus device >>> has the same frequency table with bus_leftbus device. >>> lrwxrwxrwx 1 root root 0 Dec 31 17:00 bus_mfc -> ../../devices/platform/bus_mfc/devfreq/bus_mfc >>> lrwxrwxrwx 1 root root 0 Dec 31 17:00 bus_rightbus -> ../../devices/platform/bus_rightbus/devfreq/bus_rightbus >> >> Right, but why do you want to see these bus devices? AFAIU, they will > > I think that the framework should show the accurate information of H/w device > through sysfs. On the exynos-bus.c driver, it is important the show the > accurate set of handled bus devices which are included in Exynos SoC. > >> always behave exactly the same as LEFTBUS. Their PPMU counters will be >> the same... or not? The MFC does not have its own PPMU counter. There >> are separate counters for left and right bus... but they are attached to >> the "&bus_leftbus" node. The "&bus_rightbus" does not use the PPMU >> counter and it follows the parent governor decisions... so this is >> purely an artificial creation just to handle one clock. >> >> I just can't see the benefit of such additional bus device. > > I agree about the behavior. Your description is right. > There is no difference and benefit about behavior both your and my approach. > > But, We can provide the accurate information of handled bus devices > to the user-space. I think that it is important information. > > Also, I have the plan that devfreq framework would show > the topology about the correlation of bus devices as following: Hmmm.... okay, fair enough. From hardware point of view these are separate AXI buses so I guess it is good to model them in DT. Best regards, Krzysztof