| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-id: <566925CD.4040205@samsung.com>
Date: Thu, 10 Dec 2015 16:12:13 +0900
From: Krzysztof Kozlowski <k.kozlowski@...sung.com>
To: Chanwoo Choi <cw00.choi@...sung.com>, myungjoo.ham@...sung.com,
kgene@...nel.org
Cc: kyungmin.park@...sung.com, robh+dt@...nel.org, pawel.moll@....com,
mark.rutland@....com, ijc+devicetree@...lion.org.uk,
galak@...eaurora.org, linux@....linux.org.uk,
tjakobi@...h.uni-bielefeld.de, linux.amoon@...il.com,
linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
linux-samsung-soc@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v2 14/19] ARM: dts: Add bus nodes using VDD_INT for
Exynos4x12
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 <cw00.choi@...sung.com>
>>>>>>> ---
>>>>>>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists