From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753354AbbLJHSr (ORCPT ); Thu, 10 Dec 2015 02:18:47 -0500 Received: from mailout2.samsung.com ([203.254.224.25]:33989 "EHLO mailout2.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750975AbbLJHSp (ORCPT ); Thu, 10 Dec 2015 02:18:45 -0500 MIME-version: 1.0 Content-type: text/plain; charset=utf-8 X-AuditID: cbfee690-f79646d000001316-c1-56692751eb96 Content-transfer-encoding: 8BIT Message-id: <56692750.4060905@samsung.com> Date: Thu, 10 Dec 2015 16:18:40 +0900 From: Chanwoo Choi User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 To: Krzysztof Kozlowski , myungjoo.ham@samsung.com, kgene@kernel.org 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 Subject: Re: [PATCH v2 14/19] ARM: dts: Add bus nodes using VDD_INT for Exynos4x12 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> <566925CD.4040205@samsung.com> In-reply-to: <566925CD.4040205@samsung.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrHIsWRmVeSWpSXmKPExsWyRsSkWDdIPTPM4M4NJov5R86xWvS/Wchq ce7VSkaL1y8MLfofv2a2ONv0ht3i8q45bBafe48wWsw4v4/JYt3GW+wWty/zWiy9fpHJ4nbj CjaLCdPXsli07j3CbtG2+gOrg4DHmnlrGD1amnvYPC739TJ57Jx1l91j5fIvbB6bVnWyefw7 xu7Rt2UVo8fnTXIBnFFcNimpOZllqUX6dglcGVs2vGAp+GRQceX3FJYGxjUaXYycHBICJhLr 37SwQNhiEhfurWfrYuTiEBJYwShxpH0lG0zR9x8nmCESsxglmm9uZwJJ8AoISvyYfA+om4OD WUBe4silbAhTXWLKlFyI8geMEn0vtjBDlGtJfPhwHMxmEVCV+LmjGWwMG1B8/4sbbCC9ogIR Et0nKkHCIgLxEpteXAW7h1ngLpPExfdPwO4RFgiR2DRvDSvEgvtMEvObdrODJDgFtCU+NV1l hDh6LYfE+n/6EMsEJL5NPgR2p4SArMSmA8wQJZISB1fcYJnAKDYLyTezEL6ZhfDNAkbmVYyi qQXJBcVJ6UUmesWJucWleel6yfm5mxiB0X7637MJOxjvHbA+xCjAwajEw/tCOjNMiDWxrLgy 9xCjKdANE5mlRJPzgSklryTe0NjMyMLUxNTYyNzSTEmc97XUz2AhgfTEktTs1NSC1KL4otKc 1OJDjEwcnFINjMmxG2y3dws/0vBvEbuvL378kFBaysyc/t03zAxWvPW2XTGjbXXLt5/rWRWk +0T+35fu2vT23YTrmyZGc6/4N99d4mpHfPGD7vKgIzoyat9/vnkbOef2tuybxupbhH1OBy5I dnu5vU9h8metP+4O1rvPV+yRvnczMOayJ9ev2umB+y1PHnktcFiJpTgj0VCLuag4EQBT7dhO 8QIAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrDKsWRmVeSWpSXmKPExsVy+t9jQd1A9cwwgynnLCzmHznHatH/ZiGr xblXKxktXr8wtOh//JrZ4mzTG3aLy7vmsFl87j3CaDHj/D4mi3Ubb7Fb3L7Ma7H0+kUmi9uN K9gsJkxfy2LRuvcIu0Xb6g+sDgIea+atYfRoae5h87jc18vksXPWXXaPlcu/sHlsWtXJ5vHv GLtH35ZVjB6fN8kFcEY1MNpkpCampBYppOYl56dk5qXbKnkHxzvHm5oZGOoaWlqYKynkJeam 2iq5+AToumXmAH2hpFCWmFMKFApILC5W0rfDNCE0xE3XAqYxQtc3JAiux8gADSSsYczYsuEF S8Eng4orv6ewNDCu0ehi5OSQEDCR+P7jBDOELSZx4d56ti5GLg4hgVmMEs03tzOBJHgFBCV+ TL7H0sXIwcEsIC9x5FI2hKkuMWVKLkT5A0aJvhdbmCHKtSQ+fDgOZrMIqEr83NEMNoYNKL7/ xQ02kF5RgQiJ7hOVIGERgXiJTS+ugq1lFrjLJHHx/RM2kISwQIjEpnlrWCEW3GeSmN+0mx0k wSmgLfGp6SrjBEagKxHOm4Vw3iyE8xYwMq9ilEgtSC4oTkrPNcpLLdcrTswtLs1L10vOz93E CE4pz6R3MB7e5X6IUYCDUYmH94V0ZpgQa2JZcWXuIUYJDmYlEd6jPzPChHhTEiurUovy44tK c1KLDzGaAv03kVlKNDkfmO7ySuINjU3MjCyNzA0tjIzNlcR5912KDBMSSE8sSc1OTS1ILYLp Y+LglGpg3Px3k7jAyfdHXz0/bPd+aWf9Bs3OnhUrVq5uWbqlX0618UxElLWBcrKNiXbkYxm2 m7Ol8k/Yi2854+wZu+lo+YT7E7N9Vll6xPYFCrddm6b99KhVZNDkF++3uUx5tvJF0wtxdk0d 7eN56rIS8j55DXdmCCiIL477/2Tr+61aclt2Vfr6TzS5ocRSnJFoqMVcVJwIALwj9Ow/AwAA DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2015년 12월 10일 16:12, Krzysztof Kozlowski wrote: > 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. Thanks for your review. Best Regards, Chanwoo Choi