linux-kernel - Re: [RFC 2/2] phy: exynos5-usbdrd: remove disable and enable of phy clk

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANAwSgSMFBS8n48=_OnNNs_=Di9Oqq2tNeiwZM9u1qaLevLP4w@mail.gmail.com>
Date:   Mon, 9 Oct 2017 02:37:14 +0530
From:   Anand Moon <linux.amoon@...il.com>
To:     Krzysztof Kozlowski <krzk@...nel.org>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Russell King <linux@...linux.org.uk>,
        Kukjin Kim <kgene@...nel.org>,
        Kishon Vijay Abraham I <kishon@...com>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
        Lee Jones <lee.jones@...aro.org>,
        Chunfeng Yun <chunfeng.yun@...iatek.com>,
        Vivek Gautam <vivek.gautam@...eaurora.org>,
        Andrzej Pietrasiewicz <andrzej.p@...sung.com>,
        devicetree <devicetree@...r.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        "linux-samsung-soc@...r.kernel.org" 
        <linux-samsung-soc@...r.kernel.org>,
        Linux Kernel <linux-kernel@...r.kernel.org>
Subject: Re: [RFC 2/2] phy: exynos5-usbdrd: remove disable and enable of phy clk

Hi Krzysztof,

On 8 October 2017 at 21:20, Krzysztof Kozlowski <krzk@...nel.org> wrote:
> On Sun, Oct 08, 2017 at 06:11:12PM +0530, Anand Moon wrote:
>> Hi Krzysztof,
>>
>> On 6 October 2017 at 12:12, Krzysztof Kozlowski <krzk@...nel.org> wrote:
>> > On Fri, Oct 6, 2017 at 6:36 AM, Anand Moon <linux.amoon@...il.com> wrote:
>> >> remove the disable and enable of phy clk.
>> >> phy clk is needed to tune the phy controller.
>> >
>> > Drivers should in general enable and disable the clocks they use. Just
>> > like in patch #1 please describe why you are doing this, what kind of
>> > problem are you trying to solve and what exactly are you trying to do
>> > here.
>> >
>> > BR,
>> > Krzysztof
>> >
>>
>> [snip]
>>
>> Usually we would disable the clk on error patch and return with failed.
>> but in the current code we disable the clk in init routine and
>> enable the clk in disable routine which seem incorrect.
>
> No. For example for exynos5_usbdrd_phy_exit() the driver enables the
> clock only for the access to PHY block (PHY registers).
>

But I dont get it why we disable code phy clk, which could be used in
future phy tune or link control as it part of CLK_FSYS.

>>
>> [0] https://github.com/torvalds/linux/blob/master/drivers/phy/samsung/phy-exynos5-usbdrd.c#L362-L412
>> [1] https://github.com/torvalds/linux/blob/master/drivers/phy/samsung/phy-exynos5-usbdrd.c#L424-L446
>>
>> On 3.10.x kernel clk_summary all the clk are enables.
>>
>>     mout_usbdrd300              3           3            24000000
>>        dout_usbdrd300           2           2            24000000
>>           sclk_usbdrd300        1           1            24000000
>>        dout_usbphy300           2           2            24000000
>>           sclk_usbphy300        1           1            24000000
>>     mout_usbdrd301              3           3            24000000
>>        dout_usbdrd301           2           2            24000000
>>           sclk_usbdrd301        1           1            24000000
>>        dout_usbphy301           2           2            24000000
>>           sclk_usbphy301        1           1            24000000
>>
>> Some more changes in clk driver might be required to stabilize the usb module.
>
> Please describe the issues with stability first.
>
> Best regards,
> Krzysztof
>
On stability:

USB 3.0 read / write performance is not up to mark with moderate data
read / write speed,
If you give long compilation of kernel or any source code it would
likely be slow
It work pretty good with USB 3.0 SSD but generally failed with 2.5' inc HDD.

If needed I will share you some performance test result with and
without these patches.

I have some more changes related to usbdrd phy change queued along this on.

Best Regards
-Anand