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

2017-10-09 Thread Krzysztof Kozlowski 
On Mon, Oct 09, 2017 at 08:46:48PM +0530, Anand Moon wrote:
> hi Krzysztof,
> 
> On 9 October 2017 at 02:37, Anand Moon  wrote:
> > Hi Krzysztof,
> >
> > On 8 October 2017 at 21:20, Krzysztof Kozlowski  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  wrote:
> >>> > On Fri, Oct 6, 2017 at 6:36 AM, Anand Moon  
> >>> > 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.
> >>
> 
> disable of clk could affect the PMU used to control the phy driver.

Disabling device's clock affects the device but the PMU rather not...
but I might misunderstood you cause this sounds quite unprecise.

Best regards,
Krzysztof

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

2017-10-09 Thread Krzysztof Kozlowski 
On Mon, Oct 09, 2017 at 08:46:48PM +0530, Anand Moon wrote:
> hi Krzysztof,
> 
> On 9 October 2017 at 02:37, Anand Moon  wrote:
> > Hi Krzysztof,
> >
> > On 8 October 2017 at 21:20, Krzysztof Kozlowski  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  wrote:
> >>> > On Fri, Oct 6, 2017 at 6:36 AM, Anand Moon  
> >>> > 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.
> >>
> 
> disable of clk could affect the PMU used to control the phy driver.

Disabling device's clock affects the device but the PMU rather not...
but I might misunderstood you cause this sounds quite unprecise.

Best regards,
Krzysztof

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

2017-10-09 Thread Krzysztof Kozlowski 
On Mon, Oct 09, 2017 at 02:37:14AM +0530, Anand Moon wrote:
> Hi Krzysztof,
> 
> On 8 October 2017 at 21:20, Krzysztof Kozlowski  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  wrote:
> >> > On Fri, Oct 6, 2017 at 6:36 AM, Anand Moon  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.

Clock is being disabled when it is not used. AFAIU, current code follows
this logic.

> 
> >>
> >> [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   32400
> >>dout_usbdrd300   2   22400
> >>   sclk_usbdrd3001   12400
> >>dout_usbphy300   2   22400
> >>   sclk_usbphy3001   12400
> >> mout_usbdrd301  3   32400
> >>dout_usbdrd301   2   22400
> >>   sclk_usbdrd3011   12400
> >>dout_usbphy301   2   22400
> >>   sclk_usbphy3011   12400
> >>
> >> 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.

Yes, such results are essential for this (and any) commit.

Best regards,
Krzysztof


> 
> I have some more changes related to usbdrd phy change queued along this on.
> 
> Best Regards
> -Anand

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

2017-10-09 Thread Krzysztof Kozlowski 
On Mon, Oct 09, 2017 at 02:37:14AM +0530, Anand Moon wrote:
> Hi Krzysztof,
> 
> On 8 October 2017 at 21:20, Krzysztof Kozlowski  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  wrote:
> >> > On Fri, Oct 6, 2017 at 6:36 AM, Anand Moon  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.

Clock is being disabled when it is not used. AFAIU, current code follows
this logic.

> 
> >>
> >> [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   32400
> >>dout_usbdrd300   2   22400
> >>   sclk_usbdrd3001   12400
> >>dout_usbphy300   2   22400
> >>   sclk_usbphy3001   12400
> >> mout_usbdrd301  3   32400
> >>dout_usbdrd301   2   22400
> >>   sclk_usbdrd3011   12400
> >>dout_usbphy301   2   22400
> >>   sclk_usbphy3011   12400
> >>
> >> 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.

Yes, such results are essential for this (and any) commit.

Best regards,
Krzysztof


> 
> I have some more changes related to usbdrd phy change queued along this on.
> 
> Best Regards
> -Anand

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

2017-10-09 Thread Anand Moon 
hi Krzysztof,

On 9 October 2017 at 02:37, Anand Moon  wrote:
> Hi Krzysztof,
>
> On 8 October 2017 at 21:20, Krzysztof Kozlowski  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  wrote:
>>> > On Fri, Oct 6, 2017 at 6:36 AM, Anand Moon  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.
>>

disable of clk could affect the PMU used to control the phy driver.

>> 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   32400
>>>dout_usbdrd300   2   22400
>>>   sclk_usbdrd3001   12400
>>>dout_usbphy300   2   22400
>>>   sclk_usbphy3001   12400
>>> mout_usbdrd301  3   32400
>>>dout_usbdrd301   2   22400
>>>   sclk_usbdrd3011   12400
>>>dout_usbphy301   2   22400
>>>   sclk_usbphy3011   12400
>>>
>>> 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

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

2017-10-09 Thread Anand Moon 
hi Krzysztof,

On 9 October 2017 at 02:37, Anand Moon  wrote:
> Hi Krzysztof,
>
> On 8 October 2017 at 21:20, Krzysztof Kozlowski  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  wrote:
>>> > On Fri, Oct 6, 2017 at 6:36 AM, Anand Moon  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.
>>

disable of clk could affect the PMU used to control the phy driver.

>> 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   32400
>>>dout_usbdrd300   2   22400
>>>   sclk_usbdrd3001   12400
>>>dout_usbphy300   2   22400
>>>   sclk_usbphy3001   12400
>>> mout_usbdrd301  3   32400
>>>dout_usbdrd301   2   22400
>>>   sclk_usbdrd3011   12400
>>>dout_usbphy301   2   22400
>>>   sclk_usbphy3011   12400
>>>
>>> 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

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

2017-10-08 Thread Anand Moon 
Hi Krzysztof,

On 8 October 2017 at 21:20, Krzysztof Kozlowski  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  wrote:
>> > On Fri, Oct 6, 2017 at 6:36 AM, Anand Moon  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   32400
>>dout_usbdrd300   2   22400
>>   sclk_usbdrd3001   12400
>>dout_usbphy300   2   22400
>>   sclk_usbphy3001   12400
>> mout_usbdrd301  3   32400
>>dout_usbdrd301   2   22400
>>   sclk_usbdrd3011   12400
>>dout_usbphy301   2   22400
>>   sclk_usbphy3011   12400
>>
>> 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

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

2017-10-08 Thread Anand Moon 
Hi Krzysztof,

On 8 October 2017 at 21:20, Krzysztof Kozlowski  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  wrote:
>> > On Fri, Oct 6, 2017 at 6:36 AM, Anand Moon  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   32400
>>dout_usbdrd300   2   22400
>>   sclk_usbdrd3001   12400
>>dout_usbphy300   2   22400
>>   sclk_usbphy3001   12400
>> mout_usbdrd301  3   32400
>>dout_usbdrd301   2   22400
>>   sclk_usbdrd3011   12400
>>dout_usbphy301   2   22400
>>   sclk_usbphy3011   12400
>>
>> 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

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

2017-10-08 Thread Krzysztof Kozlowski 
On Sun, Oct 08, 2017 at 06:11:12PM +0530, Anand Moon wrote:
> Hi Krzysztof,
> 
> On 6 October 2017 at 12:12, Krzysztof Kozlowski  wrote:
> > On Fri, Oct 6, 2017 at 6:36 AM, Anand Moon  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).

> 
> [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   32400
>dout_usbdrd300   2   22400
>   sclk_usbdrd3001   12400
>dout_usbphy300   2   22400
>   sclk_usbphy3001   12400
> mout_usbdrd301  3   32400
>dout_usbdrd301   2   22400
>   sclk_usbdrd3011   12400
>dout_usbphy301   2   22400
>   sclk_usbphy3011   12400
> 
> Some more changes in clk driver might be required to stabilize the usb module.

Please describe the issues with stability first.

Best regards,
Krzysztof

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

2017-10-08 Thread Krzysztof Kozlowski 
On Sun, Oct 08, 2017 at 06:11:12PM +0530, Anand Moon wrote:
> Hi Krzysztof,
> 
> On 6 October 2017 at 12:12, Krzysztof Kozlowski  wrote:
> > On Fri, Oct 6, 2017 at 6:36 AM, Anand Moon  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).

> 
> [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   32400
>dout_usbdrd300   2   22400
>   sclk_usbdrd3001   12400
>dout_usbphy300   2   22400
>   sclk_usbphy3001   12400
> mout_usbdrd301  3   32400
>dout_usbdrd301   2   22400
>   sclk_usbdrd3011   12400
>dout_usbphy301   2   22400
>   sclk_usbphy3011   12400
> 
> Some more changes in clk driver might be required to stabilize the usb module.

Please describe the issues with stability first.

Best regards,
Krzysztof

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

2017-10-08 Thread Anand Moon 
Hi Krzysztof,

On 6 October 2017 at 12:12, Krzysztof Kozlowski  wrote:
> On Fri, Oct 6, 2017 at 6:36 AM, Anand Moon  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.

[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   32400
   dout_usbdrd300   2   22400
  sclk_usbdrd3001   12400
   dout_usbphy300   2   22400
  sclk_usbphy3001   12400
mout_usbdrd301  3   32400
   dout_usbdrd301   2   22400
  sclk_usbdrd3011   12400
   dout_usbphy301   2   22400
  sclk_usbphy3011   12400

Some more changes in clk driver might be required to stabilize the usb module.

Best Regards
-Anand

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

2017-10-08 Thread Anand Moon 
Hi Krzysztof,

On 6 October 2017 at 12:12, Krzysztof Kozlowski  wrote:
> On Fri, Oct 6, 2017 at 6:36 AM, Anand Moon  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.

[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   32400
   dout_usbdrd300   2   22400
  sclk_usbdrd3001   12400
   dout_usbphy300   2   22400
  sclk_usbphy3001   12400
mout_usbdrd301  3   32400
   dout_usbdrd301   2   22400
  sclk_usbdrd3011   12400
   dout_usbphy301   2   22400
  sclk_usbphy3011   12400

Some more changes in clk driver might be required to stabilize the usb module.

Best Regards
-Anand

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

2017-10-06 Thread Krzysztof Kozlowski 
On Fri, Oct 6, 2017 at 6:36 AM, Anand Moon  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

>
> Before:
> mout_usbd300  11 2400  0 0
>dout_usbd300   002400  
> 0 0
>   sclk_usbd300002400  
> 0 0
>dout_usbphy300 112400  
> 0 0
>   sclk_usbphy300  332400  
> 0 0
> mout_usbd301  112400  
> 0 0
>dout_usbd301   002400  
> 0 0
>   sclk_usbd301002400  
> 0 0
>dout_usbphy301 112400  
> 0 0
>   sclk_usbphy301  222400  
> 0 0
> After:
> mout_usbd300  222400  
> 0 0
>dout_usbd300   112400  
> 0 0
>   sclk_usbd300222400  
> 0 0
>dout_usbphy300 112400  
> 0 0
>   sclk_usbphy300  332400  
> 0 0
> mout_usbd301  222400  
> 0 0
>dout_usbd301   112400  
> 0 0
>   sclk_usbd301222400  
> 0 0
>dout_usbphy301 112400  
> 0 0
>   sclk_usbphy301  222400  
> 0 0
>
> Signed-off-by: Anand Moon 
> ---
>  drivers/phy/samsung/phy-exynos5-usbdrd.c | 7 ---
>  1 file changed, 7 deletions(-)
>
> diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c 
> b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> index 22c68f5..5e8054c 100644
> --- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
> +++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> @@ -409,22 +409,15 @@ static int exynos5_usbdrd_phy_init(struct phy *phy)
> reg &= ~PHYCLKRST_PORTRESET;
> writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYCLKRST);
>
> -   clk_disable_unprepare(phy_drd->clk);
> -
> return 0;
>  }
>
>  static int exynos5_usbdrd_phy_exit(struct phy *phy)
>  {
> -   int ret;
> u32 reg;
> struct phy_usb_instance *inst = phy_get_drvdata(phy);
> struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst);
>
> -   ret = clk_prepare_enable(phy_drd->clk);
> -   if (ret)
> -   return ret;
> -
> reg =   PHYUTMI_OTGDISABLE |
> PHYUTMI_FORCESUSPEND |
> PHYUTMI_FORCESLEEP;
> --
> 2.7.4
>

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

2017-10-06 Thread Krzysztof Kozlowski 
On Fri, Oct 6, 2017 at 6:36 AM, Anand Moon  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

>
> Before:
> mout_usbd300  11 2400  0 0
>dout_usbd300   002400  
> 0 0
>   sclk_usbd300002400  
> 0 0
>dout_usbphy300 112400  
> 0 0
>   sclk_usbphy300  332400  
> 0 0
> mout_usbd301  112400  
> 0 0
>dout_usbd301   002400  
> 0 0
>   sclk_usbd301002400  
> 0 0
>dout_usbphy301 112400  
> 0 0
>   sclk_usbphy301  222400  
> 0 0
> After:
> mout_usbd300  222400  
> 0 0
>dout_usbd300   112400  
> 0 0
>   sclk_usbd300222400  
> 0 0
>dout_usbphy300 112400  
> 0 0
>   sclk_usbphy300  332400  
> 0 0
> mout_usbd301  222400  
> 0 0
>dout_usbd301   112400  
> 0 0
>   sclk_usbd301222400  
> 0 0
>dout_usbphy301 112400  
> 0 0
>   sclk_usbphy301  222400  
> 0 0
>
> Signed-off-by: Anand Moon 
> ---
>  drivers/phy/samsung/phy-exynos5-usbdrd.c | 7 ---
>  1 file changed, 7 deletions(-)
>
> diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c 
> b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> index 22c68f5..5e8054c 100644
> --- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
> +++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> @@ -409,22 +409,15 @@ static int exynos5_usbdrd_phy_init(struct phy *phy)
> reg &= ~PHYCLKRST_PORTRESET;
> writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYCLKRST);
>
> -   clk_disable_unprepare(phy_drd->clk);
> -
> return 0;
>  }
>
>  static int exynos5_usbdrd_phy_exit(struct phy *phy)
>  {
> -   int ret;
> u32 reg;
> struct phy_usb_instance *inst = phy_get_drvdata(phy);
> struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst);
>
> -   ret = clk_prepare_enable(phy_drd->clk);
> -   if (ret)
> -   return ret;
> -
> reg =   PHYUTMI_OTGDISABLE |
> PHYUTMI_FORCESUSPEND |
> PHYUTMI_FORCESLEEP;
> --
> 2.7.4
>

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

2017-10-05 Thread Anand Moon 
remove the disable and enable of phy clk.
phy clk is needed to tune the phy controller.

Before:
mout_usbd300  112400  0 0
   dout_usbd300   002400  0 0
  sclk_usbd300002400  0 0
   dout_usbphy300 112400  0 0
  sclk_usbphy300  332400  0 0
mout_usbd301  112400  0 0
   dout_usbd301   002400  0 0
  sclk_usbd301002400  0 0
   dout_usbphy301 112400  0 0
  sclk_usbphy301  222400  0 0
After:
mout_usbd300  222400  0 0
   dout_usbd300   112400  0 0
  sclk_usbd300222400  0 0
   dout_usbphy300 112400  0 0
  sclk_usbphy300  332400  0 0
mout_usbd301  222400  0 0
   dout_usbd301   112400  0 0
  sclk_usbd301222400  0 0
   dout_usbphy301 112400  0 0
  sclk_usbphy301  222400  0 0

Signed-off-by: Anand Moon 
---
 drivers/phy/samsung/phy-exynos5-usbdrd.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c 
b/drivers/phy/samsung/phy-exynos5-usbdrd.c
index 22c68f5..5e8054c 100644
--- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
+++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
@@ -409,22 +409,15 @@ static int exynos5_usbdrd_phy_init(struct phy *phy)
reg &= ~PHYCLKRST_PORTRESET;
writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYCLKRST);
 
-   clk_disable_unprepare(phy_drd->clk);
-
return 0;
 }
 
 static int exynos5_usbdrd_phy_exit(struct phy *phy)
 {
-   int ret;
u32 reg;
struct phy_usb_instance *inst = phy_get_drvdata(phy);
struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst);
 
-   ret = clk_prepare_enable(phy_drd->clk);
-   if (ret)
-   return ret;
-
reg =   PHYUTMI_OTGDISABLE |
PHYUTMI_FORCESUSPEND |
PHYUTMI_FORCESLEEP;
-- 
2.7.4

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

2017-10-05 Thread Anand Moon 
remove the disable and enable of phy clk.
phy clk is needed to tune the phy controller.

Before:
mout_usbd300  112400  0 0
   dout_usbd300   002400  0 0
  sclk_usbd300002400  0 0
   dout_usbphy300 112400  0 0
  sclk_usbphy300  332400  0 0
mout_usbd301  112400  0 0
   dout_usbd301   002400  0 0
  sclk_usbd301002400  0 0
   dout_usbphy301 112400  0 0
  sclk_usbphy301  222400  0 0
After:
mout_usbd300  222400  0 0
   dout_usbd300   112400  0 0
  sclk_usbd300222400  0 0
   dout_usbphy300 112400  0 0
  sclk_usbphy300  332400  0 0
mout_usbd301  222400  0 0
   dout_usbd301   112400  0 0
  sclk_usbd301222400  0 0
   dout_usbphy301 112400  0 0
  sclk_usbphy301  222400  0 0

Signed-off-by: Anand Moon 
---
 drivers/phy/samsung/phy-exynos5-usbdrd.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c 
b/drivers/phy/samsung/phy-exynos5-usbdrd.c
index 22c68f5..5e8054c 100644
--- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
+++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
@@ -409,22 +409,15 @@ static int exynos5_usbdrd_phy_init(struct phy *phy)
reg &= ~PHYCLKRST_PORTRESET;
writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYCLKRST);
 
-   clk_disable_unprepare(phy_drd->clk);
-
return 0;
 }
 
 static int exynos5_usbdrd_phy_exit(struct phy *phy)
 {
-   int ret;
u32 reg;
struct phy_usb_instance *inst = phy_get_drvdata(phy);
struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst);
 
-   ret = clk_prepare_enable(phy_drd->clk);
-   if (ret)
-   return ret;
-
reg =   PHYUTMI_OTGDISABLE |
PHYUTMI_FORCESUSPEND |
PHYUTMI_FORCESLEEP;
-- 
2.7.4