Re: [PATCH v1 1/4] clk: tegra20: Add DEV1/DEV2 OSC dividers

[Dmitry Osipenko]

On 03.05.2018 15:30, Marcel Ziswiler wrote:
> On Thu, 2018-05-03 at 15:02 +0300, Dmitry Osipenko wrote:
>> ...
>>> Marcel, you previously mentioned that reverting of your DT patch
>>> works for the
>>> Colibri now. Does that reverting also work for the 4.17 kernel? If
>>> yes, I may
>>> add stable tag to the revert-patch to get back paz00 working with
>>> 4.17. If it's
>>> not working, we should figure out why pll_p_out4 is getting
>>> disabled as it
>>> should be a distinct issue.
>>
>> Oh, I just found that Thierry already asked to get this patches
>> included into
>> 4.17. Anyway would be nice to know if a sole DT revert works with
>> 4.17 for you.
> 
> Yes, 4.17.0-rc3 with just 4c9a27a6c66d4427f3cba4019d4ba738fe99fa87
> reverted works fine as well.

Okay, thank you very much. So pll_p_out4 disabling was either an intermediate
issue that got fixed or it is masked by something else now.

Re: [PATCH v1 1/4] clk: tegra20: Add DEV1/DEV2 OSC dividers

[Dmitry Osipenko]

On 03.05.2018 15:30, Marcel Ziswiler wrote:
> On Thu, 2018-05-03 at 15:02 +0300, Dmitry Osipenko wrote:
>> ...
>>> Marcel, you previously mentioned that reverting of your DT patch
>>> works for the
>>> Colibri now. Does that reverting also work for the 4.17 kernel? If
>>> yes, I may
>>> add stable tag to the revert-patch to get back paz00 working with
>>> 4.17. If it's
>>> not working, we should figure out why pll_p_out4 is getting
>>> disabled as it
>>> should be a distinct issue.
>>
>> Oh, I just found that Thierry already asked to get this patches
>> included into
>> 4.17. Anyway would be nice to know if a sole DT revert works with
>> 4.17 for you.
> 
> Yes, 4.17.0-rc3 with just 4c9a27a6c66d4427f3cba4019d4ba738fe99fa87
> reverted works fine as well.

Okay, thank you very much. So pll_p_out4 disabling was either an intermediate
issue that got fixed or it is masked by something else now.

Re: [PATCH v1 1/4] clk: tegra20: Add DEV1/DEV2 OSC dividers

[Marcel Ziswiler]

On Thu, 2018-05-03 at 15:02 +0300, Dmitry Osipenko wrote:
> ...
> > Marcel, you previously mentioned that reverting of your DT patch
> > works for the
> > Colibri now. Does that reverting also work for the 4.17 kernel? If
> > yes, I may
> > add stable tag to the revert-patch to get back paz00 working with
> > 4.17. If it's
> > not working, we should figure out why pll_p_out4 is getting
> > disabled as it
> > should be a distinct issue.
> 
> Oh, I just found that Thierry already asked to get this patches
> included into
> 4.17. Anyway would be nice to know if a sole DT revert works with
> 4.17 for you.

Yes, 4.17.0-rc3 with just 4c9a27a6c66d4427f3cba4019d4ba738fe99fa87
reverted works fine as well.

Re: [PATCH v1 1/4] clk: tegra20: Add DEV1/DEV2 OSC dividers

[Marcel Ziswiler]

On Thu, 2018-05-03 at 15:02 +0300, Dmitry Osipenko wrote:
> ...
> > Marcel, you previously mentioned that reverting of your DT patch
> > works for the
> > Colibri now. Does that reverting also work for the 4.17 kernel? If
> > yes, I may
> > add stable tag to the revert-patch to get back paz00 working with
> > 4.17. If it's
> > not working, we should figure out why pll_p_out4 is getting
> > disabled as it
> > should be a distinct issue.
> 
> Oh, I just found that Thierry already asked to get this patches
> included into
> 4.17. Anyway would be nice to know if a sole DT revert works with
> 4.17 for you.

Yes, 4.17.0-rc3 with just 4c9a27a6c66d4427f3cba4019d4ba738fe99fa87
reverted works fine as well.

Re: [PATCH v1 1/4] clk: tegra20: Add DEV1/DEV2 OSC dividers

[Dmitry Osipenko]

On 03.05.2018 14:59, Dmitry Osipenko wrote:
> On 27.04.2018 16:00, Marcel Ziswiler wrote:
>> On Fri, 2018-04-27 at 15:54 +0300, Dmitry Osipenko wrote:
>>> Hi Marcel,
>>>
>>> On 27.04.2018 15:33,  Ziswiler wrote:
 Hi Dmitry

 Isn't the CLK_RST_CONTROLLER_MISC_CLK_ENB_0 the other way around
 e.g.
 DEV1_OSC_DIV_SEL at bit 23:22 and DEV2_OSC_DIV_SEL at 21:20?

 On Fri, 2018-04-27 at 02:58 +0300, Dmitry Osipenko wrote:
> CDEV1/CDEV2 clocks could have corresponding oscillator clock
> divider
> as
> a parent. Add these dividers in order to be able to provide that
> parent
> option.
>
> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/clk/tegra/clk-tegra20.c | 12 
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/clk/tegra/clk-tegra20.c
> b/drivers/clk/tegra/clk-
> tegra20.c
> index 0ee56dd04cec..16cf4108f2ff 100644
> --- a/drivers/clk/tegra/clk-tegra20.c
> +++ b/drivers/clk/tegra/clk-tegra20.c
> @@ -26,6 +26,8 @@
>  #include "clk.h"
>  #include "clk-id.h"
>  
> +#define MISC_CLK_ENB 0x48
> +
>  #define OSC_CTRL 0x50
>  #define OSC_CTRL_OSC_FREQ_MASK (3<<30)
>  #define OSC_CTRL_OSC_FREQ_13MHZ (0<<30)
> @@ -831,6 +833,16 @@ static void __init
> tegra20_periph_clk_init(void)
>   periph_clk_enb_refcnt);
>   clks[TEGRA20_CLK_PEX] = clk;
>  
> + /* cdev1 OSC divider */
> + clk_register_divider(NULL, "cdev1_osc_div", "clk_m",
> +  0, clk_base + MISC_CLK_ENB, 20, 2,

 So it would be:

 +   0, clk_base + MISC_CLK_ENB, 22, 2,

> +  CLK_DIVIDER_POWER_OF_TWO, NULL);
> +
> + /* cdev2 OSC divider */
> + clk_register_divider(NULL, "cdev2_osc_div", "clk_m",
> +  0, clk_base + MISC_CLK_ENB, 22, 2,

 And:

 +   0, clk_base + MISC_CLK_ENB, 20, 2,

> +  CLK_DIVIDER_POWER_OF_TWO, NULL);
> +
>   /* cdev1 */
>   clk = clk_register_fixed_rate(NULL, "cdev1_fixed", NULL,
> 0,
> 2600);
>   clk = tegra_clk_register_periph_gate("cdev1",
> "cdev1_fixed",
> 0,
>>>
>>> Indeed, good catch! I'll wait for more comments and then re-spin
>>> patchset with
>>> the fix. Thank you.
>>
>> You are very welcome. Thank you!
>>
>> Other than that it all looks proper and works fine at least in the
>> configuration we use on Colibri T20. So you may add my reviewed and
>> tested bys to the whole series:
>>
>> Reviewed-by: Marcel Ziswiler 
>> Tested-by: Marcel Ziswiler 
> 
> Marcel, you previously mentioned that reverting of your DT patch works for the
> Colibri now. Does that reverting also work for the 4.17 kernel? If yes, I may
> add stable tag to the revert-patch to get back paz00 working with 4.17. If 
> it's
> not working, we should figure out why pll_p_out4 is getting disabled as it
> should be a distinct issue.

Oh, I just found that Thierry already asked to get this patches included into
4.17. Anyway would be nice to know if a sole DT revert works with 4.17 for you.

Re: [PATCH v1 1/4] clk: tegra20: Add DEV1/DEV2 OSC dividers

[Dmitry Osipenko]

On 03.05.2018 14:59, Dmitry Osipenko wrote:
> On 27.04.2018 16:00, Marcel Ziswiler wrote:
>> On Fri, 2018-04-27 at 15:54 +0300, Dmitry Osipenko wrote:
>>> Hi Marcel,
>>>
>>> On 27.04.2018 15:33,  Ziswiler wrote:
 Hi Dmitry

 Isn't the CLK_RST_CONTROLLER_MISC_CLK_ENB_0 the other way around
 e.g.
 DEV1_OSC_DIV_SEL at bit 23:22 and DEV2_OSC_DIV_SEL at 21:20?

 On Fri, 2018-04-27 at 02:58 +0300, Dmitry Osipenko wrote:
> CDEV1/CDEV2 clocks could have corresponding oscillator clock
> divider
> as
> a parent. Add these dividers in order to be able to provide that
> parent
> option.
>
> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/clk/tegra/clk-tegra20.c | 12 
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/clk/tegra/clk-tegra20.c
> b/drivers/clk/tegra/clk-
> tegra20.c
> index 0ee56dd04cec..16cf4108f2ff 100644
> --- a/drivers/clk/tegra/clk-tegra20.c
> +++ b/drivers/clk/tegra/clk-tegra20.c
> @@ -26,6 +26,8 @@
>  #include "clk.h"
>  #include "clk-id.h"
>  
> +#define MISC_CLK_ENB 0x48
> +
>  #define OSC_CTRL 0x50
>  #define OSC_CTRL_OSC_FREQ_MASK (3<<30)
>  #define OSC_CTRL_OSC_FREQ_13MHZ (0<<30)
> @@ -831,6 +833,16 @@ static void __init
> tegra20_periph_clk_init(void)
>   periph_clk_enb_refcnt);
>   clks[TEGRA20_CLK_PEX] = clk;
>  
> + /* cdev1 OSC divider */
> + clk_register_divider(NULL, "cdev1_osc_div", "clk_m",
> +  0, clk_base + MISC_CLK_ENB, 20, 2,

 So it would be:

 +   0, clk_base + MISC_CLK_ENB, 22, 2,

> +  CLK_DIVIDER_POWER_OF_TWO, NULL);
> +
> + /* cdev2 OSC divider */
> + clk_register_divider(NULL, "cdev2_osc_div", "clk_m",
> +  0, clk_base + MISC_CLK_ENB, 22, 2,

 And:

 +   0, clk_base + MISC_CLK_ENB, 20, 2,

> +  CLK_DIVIDER_POWER_OF_TWO, NULL);
> +
>   /* cdev1 */
>   clk = clk_register_fixed_rate(NULL, "cdev1_fixed", NULL,
> 0,
> 2600);
>   clk = tegra_clk_register_periph_gate("cdev1",
> "cdev1_fixed",
> 0,
>>>
>>> Indeed, good catch! I'll wait for more comments and then re-spin
>>> patchset with
>>> the fix. Thank you.
>>
>> You are very welcome. Thank you!
>>
>> Other than that it all looks proper and works fine at least in the
>> configuration we use on Colibri T20. So you may add my reviewed and
>> tested bys to the whole series:
>>
>> Reviewed-by: Marcel Ziswiler 
>> Tested-by: Marcel Ziswiler 
> 
> Marcel, you previously mentioned that reverting of your DT patch works for the
> Colibri now. Does that reverting also work for the 4.17 kernel? If yes, I may
> add stable tag to the revert-patch to get back paz00 working with 4.17. If 
> it's
> not working, we should figure out why pll_p_out4 is getting disabled as it
> should be a distinct issue.

Oh, I just found that Thierry already asked to get this patches included into
4.17. Anyway would be nice to know if a sole DT revert works with 4.17 for you.

Re: [PATCH v1 1/4] clk: tegra20: Add DEV1/DEV2 OSC dividers

[Dmitry Osipenko]

On 27.04.2018 16:00, Marcel Ziswiler wrote:
> On Fri, 2018-04-27 at 15:54 +0300, Dmitry Osipenko wrote:
>> Hi Marcel,
>>
>> On 27.04.2018 15:33,  Ziswiler wrote:
>>> Hi Dmitry
>>>
>>> Isn't the CLK_RST_CONTROLLER_MISC_CLK_ENB_0 the other way around
>>> e.g.
>>> DEV1_OSC_DIV_SEL at bit 23:22 and DEV2_OSC_DIV_SEL at 21:20?
>>>
>>> On Fri, 2018-04-27 at 02:58 +0300, Dmitry Osipenko wrote:
 CDEV1/CDEV2 clocks could have corresponding oscillator clock
 divider
 as
 a parent. Add these dividers in order to be able to provide that
 parent
 option.

 Signed-off-by: Dmitry Osipenko 
 ---
  drivers/clk/tegra/clk-tegra20.c | 12 
  1 file changed, 12 insertions(+)

 diff --git a/drivers/clk/tegra/clk-tegra20.c
 b/drivers/clk/tegra/clk-
 tegra20.c
 index 0ee56dd04cec..16cf4108f2ff 100644
 --- a/drivers/clk/tegra/clk-tegra20.c
 +++ b/drivers/clk/tegra/clk-tegra20.c
 @@ -26,6 +26,8 @@
  #include "clk.h"
  #include "clk-id.h"
  
 +#define MISC_CLK_ENB 0x48
 +
  #define OSC_CTRL 0x50
  #define OSC_CTRL_OSC_FREQ_MASK (3<<30)
  #define OSC_CTRL_OSC_FREQ_13MHZ (0<<30)
 @@ -831,6 +833,16 @@ static void __init
 tegra20_periph_clk_init(void)
periph_clk_enb_refcnt);
clks[TEGRA20_CLK_PEX] = clk;
  
 +  /* cdev1 OSC divider */
 +  clk_register_divider(NULL, "cdev1_osc_div", "clk_m",
 +   0, clk_base + MISC_CLK_ENB, 20, 2,
>>>
>>> So it would be:
>>>
>>> +0, clk_base + MISC_CLK_ENB, 22, 2,
>>>
 +   CLK_DIVIDER_POWER_OF_TWO, NULL);
 +
 +  /* cdev2 OSC divider */
 +  clk_register_divider(NULL, "cdev2_osc_div", "clk_m",
 +   0, clk_base + MISC_CLK_ENB, 22, 2,
>>>
>>> And:
>>>
>>> +0, clk_base + MISC_CLK_ENB, 20, 2,
>>>
 +   CLK_DIVIDER_POWER_OF_TWO, NULL);
 +
/* cdev1 */
clk = clk_register_fixed_rate(NULL, "cdev1_fixed", NULL,
 0,
 2600);
clk = tegra_clk_register_periph_gate("cdev1",
 "cdev1_fixed",
 0,
>>
>> Indeed, good catch! I'll wait for more comments and then re-spin
>> patchset with
>> the fix. Thank you.
> 
> You are very welcome. Thank you!
> 
> Other than that it all looks proper and works fine at least in the
> configuration we use on Colibri T20. So you may add my reviewed and
> tested bys to the whole series:
> 
> Reviewed-by: Marcel Ziswiler 
> Tested-by: Marcel Ziswiler 

Marcel, you previously mentioned that reverting of your DT patch works for the
Colibri now. Does that reverting also work for the 4.17 kernel? If yes, I may
add stable tag to the revert-patch to get back paz00 working with 4.17. If it's
not working, we should figure out why pll_p_out4 is getting disabled as it
should be a distinct issue.

Re: [PATCH v1 1/4] clk: tegra20: Add DEV1/DEV2 OSC dividers

[Dmitry Osipenko]

On 27.04.2018 16:00, Marcel Ziswiler wrote:
> On Fri, 2018-04-27 at 15:54 +0300, Dmitry Osipenko wrote:
>> Hi Marcel,
>>
>> On 27.04.2018 15:33,  Ziswiler wrote:
>>> Hi Dmitry
>>>
>>> Isn't the CLK_RST_CONTROLLER_MISC_CLK_ENB_0 the other way around
>>> e.g.
>>> DEV1_OSC_DIV_SEL at bit 23:22 and DEV2_OSC_DIV_SEL at 21:20?
>>>
>>> On Fri, 2018-04-27 at 02:58 +0300, Dmitry Osipenko wrote:
 CDEV1/CDEV2 clocks could have corresponding oscillator clock
 divider
 as
 a parent. Add these dividers in order to be able to provide that
 parent
 option.

 Signed-off-by: Dmitry Osipenko 
 ---
  drivers/clk/tegra/clk-tegra20.c | 12 
  1 file changed, 12 insertions(+)

 diff --git a/drivers/clk/tegra/clk-tegra20.c
 b/drivers/clk/tegra/clk-
 tegra20.c
 index 0ee56dd04cec..16cf4108f2ff 100644
 --- a/drivers/clk/tegra/clk-tegra20.c
 +++ b/drivers/clk/tegra/clk-tegra20.c
 @@ -26,6 +26,8 @@
  #include "clk.h"
  #include "clk-id.h"
  
 +#define MISC_CLK_ENB 0x48
 +
  #define OSC_CTRL 0x50
  #define OSC_CTRL_OSC_FREQ_MASK (3<<30)
  #define OSC_CTRL_OSC_FREQ_13MHZ (0<<30)
 @@ -831,6 +833,16 @@ static void __init
 tegra20_periph_clk_init(void)
periph_clk_enb_refcnt);
clks[TEGRA20_CLK_PEX] = clk;
  
 +  /* cdev1 OSC divider */
 +  clk_register_divider(NULL, "cdev1_osc_div", "clk_m",
 +   0, clk_base + MISC_CLK_ENB, 20, 2,
>>>
>>> So it would be:
>>>
>>> +0, clk_base + MISC_CLK_ENB, 22, 2,
>>>
 +   CLK_DIVIDER_POWER_OF_TWO, NULL);
 +
 +  /* cdev2 OSC divider */
 +  clk_register_divider(NULL, "cdev2_osc_div", "clk_m",
 +   0, clk_base + MISC_CLK_ENB, 22, 2,
>>>
>>> And:
>>>
>>> +0, clk_base + MISC_CLK_ENB, 20, 2,
>>>
 +   CLK_DIVIDER_POWER_OF_TWO, NULL);
 +
/* cdev1 */
clk = clk_register_fixed_rate(NULL, "cdev1_fixed", NULL,
 0,
 2600);
clk = tegra_clk_register_periph_gate("cdev1",
 "cdev1_fixed",
 0,
>>
>> Indeed, good catch! I'll wait for more comments and then re-spin
>> patchset with
>> the fix. Thank you.
> 
> You are very welcome. Thank you!
> 
> Other than that it all looks proper and works fine at least in the
> configuration we use on Colibri T20. So you may add my reviewed and
> tested bys to the whole series:
> 
> Reviewed-by: Marcel Ziswiler 
> Tested-by: Marcel Ziswiler 

Marcel, you previously mentioned that reverting of your DT patch works for the
Colibri now. Does that reverting also work for the 4.17 kernel? If yes, I may
add stable tag to the revert-patch to get back paz00 working with 4.17. If it's
not working, we should figure out why pll_p_out4 is getting disabled as it
should be a distinct issue.

Re: [PATCH v1 1/4] clk: tegra20: Add DEV1/DEV2 OSC dividers

[Peter De Schrijver]

On Fri, Apr 27, 2018 at 02:58:15AM +0300, Dmitry Osipenko wrote:
> CDEV1/CDEV2 clocks could have corresponding oscillator clock divider as
> a parent. Add these dividers in order to be able to provide that parent
> option.
> 
> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/clk/tegra/clk-tegra20.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
> index 0ee56dd04cec..16cf4108f2ff 100644
> --- a/drivers/clk/tegra/clk-tegra20.c
> +++ b/drivers/clk/tegra/clk-tegra20.c
> @@ -26,6 +26,8 @@
>  #include "clk.h"
>  #include "clk-id.h"
>  
> +#define MISC_CLK_ENB 0x48
> +
>  #define OSC_CTRL 0x50
>  #define OSC_CTRL_OSC_FREQ_MASK (3<<30)
>  #define OSC_CTRL_OSC_FREQ_13MHZ (0<<30)
> @@ -831,6 +833,16 @@ static void __init tegra20_periph_clk_init(void)
>   periph_clk_enb_refcnt);
>   clks[TEGRA20_CLK_PEX] = clk;
>  
> + /* cdev1 OSC divider */
> + clk_register_divider(NULL, "cdev1_osc_div", "clk_m",
> +  0, clk_base + MISC_CLK_ENB, 20, 2,
> +  CLK_DIVIDER_POWER_OF_TWO, NULL);
> +

I don't know if this divider can be changed glitchlessly so to be safe,
I would mark this readonly, so add the CLK_DIVIDER_READ_ONLY flag.

> + /* cdev2 OSC divider */
> + clk_register_divider(NULL, "cdev2_osc_div", "clk_m",
> +  0, clk_base + MISC_CLK_ENB, 22, 2,
> +  CLK_DIVIDER_POWER_OF_TWO, NULL);
> +
>   /* cdev1 */
>   clk = clk_register_fixed_rate(NULL, "cdev1_fixed", NULL, 0, 2600);
>   clk = tegra_clk_register_periph_gate("cdev1", "cdev1_fixed", 0,
> -- 
> 2.17.0
> 

Peter.

Re: [PATCH v1 1/4] clk: tegra20: Add DEV1/DEV2 OSC dividers

[Peter De Schrijver]

On Fri, Apr 27, 2018 at 02:58:15AM +0300, Dmitry Osipenko wrote:
> CDEV1/CDEV2 clocks could have corresponding oscillator clock divider as
> a parent. Add these dividers in order to be able to provide that parent
> option.
> 
> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/clk/tegra/clk-tegra20.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
> index 0ee56dd04cec..16cf4108f2ff 100644
> --- a/drivers/clk/tegra/clk-tegra20.c
> +++ b/drivers/clk/tegra/clk-tegra20.c
> @@ -26,6 +26,8 @@
>  #include "clk.h"
>  #include "clk-id.h"
>  
> +#define MISC_CLK_ENB 0x48
> +
>  #define OSC_CTRL 0x50
>  #define OSC_CTRL_OSC_FREQ_MASK (3<<30)
>  #define OSC_CTRL_OSC_FREQ_13MHZ (0<<30)
> @@ -831,6 +833,16 @@ static void __init tegra20_periph_clk_init(void)
>   periph_clk_enb_refcnt);
>   clks[TEGRA20_CLK_PEX] = clk;
>  
> + /* cdev1 OSC divider */
> + clk_register_divider(NULL, "cdev1_osc_div", "clk_m",
> +  0, clk_base + MISC_CLK_ENB, 20, 2,
> +  CLK_DIVIDER_POWER_OF_TWO, NULL);
> +

I don't know if this divider can be changed glitchlessly so to be safe,
I would mark this readonly, so add the CLK_DIVIDER_READ_ONLY flag.

> + /* cdev2 OSC divider */
> + clk_register_divider(NULL, "cdev2_osc_div", "clk_m",
> +  0, clk_base + MISC_CLK_ENB, 22, 2,
> +  CLK_DIVIDER_POWER_OF_TWO, NULL);
> +
>   /* cdev1 */
>   clk = clk_register_fixed_rate(NULL, "cdev1_fixed", NULL, 0, 2600);
>   clk = tegra_clk_register_periph_gate("cdev1", "cdev1_fixed", 0,
> -- 
> 2.17.0
> 

Peter.

Re: [PATCH v1 1/4] clk: tegra20: Add DEV1/DEV2 OSC dividers

[Marcel Ziswiler]

On Fri, 2018-04-27 at 15:54 +0300, Dmitry Osipenko wrote:
> Hi Marcel,
> 
> On 27.04.2018 15:33,  Ziswiler wrote:
> > Hi Dmitry
> > 
> > Isn't the CLK_RST_CONTROLLER_MISC_CLK_ENB_0 the other way around
> > e.g.
> > DEV1_OSC_DIV_SEL at bit 23:22 and DEV2_OSC_DIV_SEL at 21:20?
> > 
> > On Fri, 2018-04-27 at 02:58 +0300, Dmitry Osipenko wrote:
> > > CDEV1/CDEV2 clocks could have corresponding oscillator clock
> > > divider
> > > as
> > > a parent. Add these dividers in order to be able to provide that
> > > parent
> > > option.
> > > 
> > > Signed-off-by: Dmitry Osipenko 
> > > ---
> > >  drivers/clk/tegra/clk-tegra20.c | 12 
> > >  1 file changed, 12 insertions(+)
> > > 
> > > diff --git a/drivers/clk/tegra/clk-tegra20.c
> > > b/drivers/clk/tegra/clk-
> > > tegra20.c
> > > index 0ee56dd04cec..16cf4108f2ff 100644
> > > --- a/drivers/clk/tegra/clk-tegra20.c
> > > +++ b/drivers/clk/tegra/clk-tegra20.c
> > > @@ -26,6 +26,8 @@
> > >  #include "clk.h"
> > >  #include "clk-id.h"
> > >  
> > > +#define MISC_CLK_ENB 0x48
> > > +
> > >  #define OSC_CTRL 0x50
> > >  #define OSC_CTRL_OSC_FREQ_MASK (3<<30)
> > >  #define OSC_CTRL_OSC_FREQ_13MHZ (0<<30)
> > > @@ -831,6 +833,16 @@ static void __init
> > > tegra20_periph_clk_init(void)
> > >   periph_clk_enb_refcnt);
> > >   clks[TEGRA20_CLK_PEX] = clk;
> > >  
> > > + /* cdev1 OSC divider */
> > > + clk_register_divider(NULL, "cdev1_osc_div", "clk_m",
> > > +  0, clk_base + MISC_CLK_ENB, 20, 2,
> > 
> > So it would be:
> > 
> > +0, clk_base + MISC_CLK_ENB, 22, 2,
> > 
> > > +  CLK_DIVIDER_POWER_OF_TWO, NULL);
> > > +
> > > + /* cdev2 OSC divider */
> > > + clk_register_divider(NULL, "cdev2_osc_div", "clk_m",
> > > +  0, clk_base + MISC_CLK_ENB, 22, 2,
> > 
> > And:
> > 
> > +0, clk_base + MISC_CLK_ENB, 20, 2,
> > 
> > > +  CLK_DIVIDER_POWER_OF_TWO, NULL);
> > > +
> > >   /* cdev1 */
> > >   clk = clk_register_fixed_rate(NULL, "cdev1_fixed", NULL,
> > > 0,
> > > 2600);
> > >   clk = tegra_clk_register_periph_gate("cdev1",
> > > "cdev1_fixed",
> > > 0,
> 
> Indeed, good catch! I'll wait for more comments and then re-spin
> patchset with
> the fix. Thank you.

You are very welcome. Thank you!

Other than that it all looks proper and works fine at least in the
configuration we use on Colibri T20. So you may add my reviewed and
tested bys to the whole series:

Reviewed-by: Marcel Ziswiler 
Tested-by: Marcel Ziswiler 

Re: [PATCH v1 1/4] clk: tegra20: Add DEV1/DEV2 OSC dividers

[Marcel Ziswiler]

On Fri, 2018-04-27 at 15:54 +0300, Dmitry Osipenko wrote:
> Hi Marcel,
> 
> On 27.04.2018 15:33,  Ziswiler wrote:
> > Hi Dmitry
> > 
> > Isn't the CLK_RST_CONTROLLER_MISC_CLK_ENB_0 the other way around
> > e.g.
> > DEV1_OSC_DIV_SEL at bit 23:22 and DEV2_OSC_DIV_SEL at 21:20?
> > 
> > On Fri, 2018-04-27 at 02:58 +0300, Dmitry Osipenko wrote:
> > > CDEV1/CDEV2 clocks could have corresponding oscillator clock
> > > divider
> > > as
> > > a parent. Add these dividers in order to be able to provide that
> > > parent
> > > option.
> > > 
> > > Signed-off-by: Dmitry Osipenko 
> > > ---
> > >  drivers/clk/tegra/clk-tegra20.c | 12 
> > >  1 file changed, 12 insertions(+)
> > > 
> > > diff --git a/drivers/clk/tegra/clk-tegra20.c
> > > b/drivers/clk/tegra/clk-
> > > tegra20.c
> > > index 0ee56dd04cec..16cf4108f2ff 100644
> > > --- a/drivers/clk/tegra/clk-tegra20.c
> > > +++ b/drivers/clk/tegra/clk-tegra20.c
> > > @@ -26,6 +26,8 @@
> > >  #include "clk.h"
> > >  #include "clk-id.h"
> > >  
> > > +#define MISC_CLK_ENB 0x48
> > > +
> > >  #define OSC_CTRL 0x50
> > >  #define OSC_CTRL_OSC_FREQ_MASK (3<<30)
> > >  #define OSC_CTRL_OSC_FREQ_13MHZ (0<<30)
> > > @@ -831,6 +833,16 @@ static void __init
> > > tegra20_periph_clk_init(void)
> > >   periph_clk_enb_refcnt);
> > >   clks[TEGRA20_CLK_PEX] = clk;
> > >  
> > > + /* cdev1 OSC divider */
> > > + clk_register_divider(NULL, "cdev1_osc_div", "clk_m",
> > > +  0, clk_base + MISC_CLK_ENB, 20, 2,
> > 
> > So it would be:
> > 
> > +0, clk_base + MISC_CLK_ENB, 22, 2,
> > 
> > > +  CLK_DIVIDER_POWER_OF_TWO, NULL);
> > > +
> > > + /* cdev2 OSC divider */
> > > + clk_register_divider(NULL, "cdev2_osc_div", "clk_m",
> > > +  0, clk_base + MISC_CLK_ENB, 22, 2,
> > 
> > And:
> > 
> > +0, clk_base + MISC_CLK_ENB, 20, 2,
> > 
> > > +  CLK_DIVIDER_POWER_OF_TWO, NULL);
> > > +
> > >   /* cdev1 */
> > >   clk = clk_register_fixed_rate(NULL, "cdev1_fixed", NULL,
> > > 0,
> > > 2600);
> > >   clk = tegra_clk_register_periph_gate("cdev1",
> > > "cdev1_fixed",
> > > 0,
> 
> Indeed, good catch! I'll wait for more comments and then re-spin
> patchset with
> the fix. Thank you.

You are very welcome. Thank you!

Other than that it all looks proper and works fine at least in the
configuration we use on Colibri T20. So you may add my reviewed and
tested bys to the whole series:

Reviewed-by: Marcel Ziswiler 
Tested-by: Marcel Ziswiler 

Re: [PATCH v1 1/4] clk: tegra20: Add DEV1/DEV2 OSC dividers

[Dmitry Osipenko]

Hi Marcel,

On 27.04.2018 15:33,  Ziswiler wrote:
> Hi Dmitry
> 
> Isn't the CLK_RST_CONTROLLER_MISC_CLK_ENB_0 the other way around e.g.
> DEV1_OSC_DIV_SEL at bit 23:22 and DEV2_OSC_DIV_SEL at 21:20?
> 
> On Fri, 2018-04-27 at 02:58 +0300, Dmitry Osipenko wrote:
>> CDEV1/CDEV2 clocks could have corresponding oscillator clock divider
>> as
>> a parent. Add these dividers in order to be able to provide that
>> parent
>> option.
>>
>> Signed-off-by: Dmitry Osipenko 
>> ---
>>  drivers/clk/tegra/clk-tegra20.c | 12 
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-
>> tegra20.c
>> index 0ee56dd04cec..16cf4108f2ff 100644
>> --- a/drivers/clk/tegra/clk-tegra20.c
>> +++ b/drivers/clk/tegra/clk-tegra20.c
>> @@ -26,6 +26,8 @@
>>  #include "clk.h"
>>  #include "clk-id.h"
>>  
>> +#define MISC_CLK_ENB 0x48
>> +
>>  #define OSC_CTRL 0x50
>>  #define OSC_CTRL_OSC_FREQ_MASK (3<<30)
>>  #define OSC_CTRL_OSC_FREQ_13MHZ (0<<30)
>> @@ -831,6 +833,16 @@ static void __init tegra20_periph_clk_init(void)
>>  periph_clk_enb_refcnt);
>>  clks[TEGRA20_CLK_PEX] = clk;
>>  
>> +/* cdev1 OSC divider */
>> +clk_register_divider(NULL, "cdev1_osc_div", "clk_m",
>> + 0, clk_base + MISC_CLK_ENB, 20, 2,
> 
> So it would be:
> 
> +  0, clk_base + MISC_CLK_ENB, 22, 2,
> 
>> + CLK_DIVIDER_POWER_OF_TWO, NULL);
>> +
>> +/* cdev2 OSC divider */
>> +clk_register_divider(NULL, "cdev2_osc_div", "clk_m",
>> + 0, clk_base + MISC_CLK_ENB, 22, 2,
> 
> And:
> 
> +  0, clk_base + MISC_CLK_ENB, 20, 2,
> 
>> + CLK_DIVIDER_POWER_OF_TWO, NULL);
>> +
>>  /* cdev1 */
>>  clk = clk_register_fixed_rate(NULL, "cdev1_fixed", NULL, 0,
>> 2600);
>>  clk = tegra_clk_register_periph_gate("cdev1", "cdev1_fixed",
>> 0,
Indeed, good catch! I'll wait for more comments and then re-spin patchset with
the fix. Thank you.

Re: [PATCH v1 1/4] clk: tegra20: Add DEV1/DEV2 OSC dividers

[Dmitry Osipenko]

Hi Marcel,

On 27.04.2018 15:33,  Ziswiler wrote:
> Hi Dmitry
> 
> Isn't the CLK_RST_CONTROLLER_MISC_CLK_ENB_0 the other way around e.g.
> DEV1_OSC_DIV_SEL at bit 23:22 and DEV2_OSC_DIV_SEL at 21:20?
> 
> On Fri, 2018-04-27 at 02:58 +0300, Dmitry Osipenko wrote:
>> CDEV1/CDEV2 clocks could have corresponding oscillator clock divider
>> as
>> a parent. Add these dividers in order to be able to provide that
>> parent
>> option.
>>
>> Signed-off-by: Dmitry Osipenko 
>> ---
>>  drivers/clk/tegra/clk-tegra20.c | 12 
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-
>> tegra20.c
>> index 0ee56dd04cec..16cf4108f2ff 100644
>> --- a/drivers/clk/tegra/clk-tegra20.c
>> +++ b/drivers/clk/tegra/clk-tegra20.c
>> @@ -26,6 +26,8 @@
>>  #include "clk.h"
>>  #include "clk-id.h"
>>  
>> +#define MISC_CLK_ENB 0x48
>> +
>>  #define OSC_CTRL 0x50
>>  #define OSC_CTRL_OSC_FREQ_MASK (3<<30)
>>  #define OSC_CTRL_OSC_FREQ_13MHZ (0<<30)
>> @@ -831,6 +833,16 @@ static void __init tegra20_periph_clk_init(void)
>>  periph_clk_enb_refcnt);
>>  clks[TEGRA20_CLK_PEX] = clk;
>>  
>> +/* cdev1 OSC divider */
>> +clk_register_divider(NULL, "cdev1_osc_div", "clk_m",
>> + 0, clk_base + MISC_CLK_ENB, 20, 2,
> 
> So it would be:
> 
> +  0, clk_base + MISC_CLK_ENB, 22, 2,
> 
>> + CLK_DIVIDER_POWER_OF_TWO, NULL);
>> +
>> +/* cdev2 OSC divider */
>> +clk_register_divider(NULL, "cdev2_osc_div", "clk_m",
>> + 0, clk_base + MISC_CLK_ENB, 22, 2,
> 
> And:
> 
> +  0, clk_base + MISC_CLK_ENB, 20, 2,
> 
>> + CLK_DIVIDER_POWER_OF_TWO, NULL);
>> +
>>  /* cdev1 */
>>  clk = clk_register_fixed_rate(NULL, "cdev1_fixed", NULL, 0,
>> 2600);
>>  clk = tegra_clk_register_periph_gate("cdev1", "cdev1_fixed",
>> 0,
Indeed, good catch! I'll wait for more comments and then re-spin patchset with
the fix. Thank you.

Re: [PATCH v1 1/4] clk: tegra20: Add DEV1/DEV2 OSC dividers

[Marcel Ziswiler]

Hi Dmitry

Isn't the CLK_RST_CONTROLLER_MISC_CLK_ENB_0 the other way around e.g.
DEV1_OSC_DIV_SEL at bit 23:22 and DEV2_OSC_DIV_SEL at 21:20?

On Fri, 2018-04-27 at 02:58 +0300, Dmitry Osipenko wrote:
> CDEV1/CDEV2 clocks could have corresponding oscillator clock divider
> as
> a parent. Add these dividers in order to be able to provide that
> parent
> option.
> 
> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/clk/tegra/clk-tegra20.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-
> tegra20.c
> index 0ee56dd04cec..16cf4108f2ff 100644
> --- a/drivers/clk/tegra/clk-tegra20.c
> +++ b/drivers/clk/tegra/clk-tegra20.c
> @@ -26,6 +26,8 @@
>  #include "clk.h"
>  #include "clk-id.h"
>  
> +#define MISC_CLK_ENB 0x48
> +
>  #define OSC_CTRL 0x50
>  #define OSC_CTRL_OSC_FREQ_MASK (3<<30)
>  #define OSC_CTRL_OSC_FREQ_13MHZ (0<<30)
> @@ -831,6 +833,16 @@ static void __init tegra20_periph_clk_init(void)
>   periph_clk_enb_refcnt);
>   clks[TEGRA20_CLK_PEX] = clk;
>  
> + /* cdev1 OSC divider */
> + clk_register_divider(NULL, "cdev1_osc_div", "clk_m",
> +  0, clk_base + MISC_CLK_ENB, 20, 2,

So it would be:

+0, clk_base + MISC_CLK_ENB, 22, 2,

> +  CLK_DIVIDER_POWER_OF_TWO, NULL);
> +
> + /* cdev2 OSC divider */
> + clk_register_divider(NULL, "cdev2_osc_div", "clk_m",
> +  0, clk_base + MISC_CLK_ENB, 22, 2,

And:

+0, clk_base + MISC_CLK_ENB, 20, 2,

> +  CLK_DIVIDER_POWER_OF_TWO, NULL);
> +
>   /* cdev1 */
>   clk = clk_register_fixed_rate(NULL, "cdev1_fixed", NULL, 0,
> 2600);
>   clk = tegra_clk_register_periph_gate("cdev1", "cdev1_fixed",
> 0,

Cheers

Marcel

Re: [PATCH v1 1/4] clk: tegra20: Add DEV1/DEV2 OSC dividers

[Marcel Ziswiler]

Hi Dmitry

Isn't the CLK_RST_CONTROLLER_MISC_CLK_ENB_0 the other way around e.g.
DEV1_OSC_DIV_SEL at bit 23:22 and DEV2_OSC_DIV_SEL at 21:20?

On Fri, 2018-04-27 at 02:58 +0300, Dmitry Osipenko wrote:
> CDEV1/CDEV2 clocks could have corresponding oscillator clock divider
> as
> a parent. Add these dividers in order to be able to provide that
> parent
> option.
> 
> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/clk/tegra/clk-tegra20.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-
> tegra20.c
> index 0ee56dd04cec..16cf4108f2ff 100644
> --- a/drivers/clk/tegra/clk-tegra20.c
> +++ b/drivers/clk/tegra/clk-tegra20.c
> @@ -26,6 +26,8 @@
>  #include "clk.h"
>  #include "clk-id.h"
>  
> +#define MISC_CLK_ENB 0x48
> +
>  #define OSC_CTRL 0x50
>  #define OSC_CTRL_OSC_FREQ_MASK (3<<30)
>  #define OSC_CTRL_OSC_FREQ_13MHZ (0<<30)
> @@ -831,6 +833,16 @@ static void __init tegra20_periph_clk_init(void)
>   periph_clk_enb_refcnt);
>   clks[TEGRA20_CLK_PEX] = clk;
>  
> + /* cdev1 OSC divider */
> + clk_register_divider(NULL, "cdev1_osc_div", "clk_m",
> +  0, clk_base + MISC_CLK_ENB, 20, 2,

So it would be:

+0, clk_base + MISC_CLK_ENB, 22, 2,

> +  CLK_DIVIDER_POWER_OF_TWO, NULL);
> +
> + /* cdev2 OSC divider */
> + clk_register_divider(NULL, "cdev2_osc_div", "clk_m",
> +  0, clk_base + MISC_CLK_ENB, 22, 2,

And:

+0, clk_base + MISC_CLK_ENB, 20, 2,

> +  CLK_DIVIDER_POWER_OF_TWO, NULL);
> +
>   /* cdev1 */
>   clk = clk_register_fixed_rate(NULL, "cdev1_fixed", NULL, 0,
> 2600);
>   clk = tegra_clk_register_periph_gate("cdev1", "cdev1_fixed",
> 0,

Cheers

Marcel

[PATCH v1 1/4] clk: tegra20: Add DEV1/DEV2 OSC dividers

[Dmitry Osipenko]

CDEV1/CDEV2 clocks could have corresponding oscillator clock divider as
a parent. Add these dividers in order to be able to provide that parent
option.

Signed-off-by: Dmitry Osipenko 
---
 drivers/clk/tegra/clk-tegra20.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index 0ee56dd04cec..16cf4108f2ff 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -26,6 +26,8 @@
 #include "clk.h"
 #include "clk-id.h"
 
+#define MISC_CLK_ENB 0x48
+
 #define OSC_CTRL 0x50
 #define OSC_CTRL_OSC_FREQ_MASK (3<<30)
 #define OSC_CTRL_OSC_FREQ_13MHZ (0<<30)
@@ -831,6 +833,16 @@ static void __init tegra20_periph_clk_init(void)
periph_clk_enb_refcnt);
clks[TEGRA20_CLK_PEX] = clk;
 
+   /* cdev1 OSC divider */
+   clk_register_divider(NULL, "cdev1_osc_div", "clk_m",
+0, clk_base + MISC_CLK_ENB, 20, 2,
+CLK_DIVIDER_POWER_OF_TWO, NULL);
+
+   /* cdev2 OSC divider */
+   clk_register_divider(NULL, "cdev2_osc_div", "clk_m",
+0, clk_base + MISC_CLK_ENB, 22, 2,
+CLK_DIVIDER_POWER_OF_TWO, NULL);
+
/* cdev1 */
clk = clk_register_fixed_rate(NULL, "cdev1_fixed", NULL, 0, 2600);
clk = tegra_clk_register_periph_gate("cdev1", "cdev1_fixed", 0,
-- 
2.17.0

[PATCH v1 1/4] clk: tegra20: Add DEV1/DEV2 OSC dividers

[Dmitry Osipenko]

CDEV1/CDEV2 clocks could have corresponding oscillator clock divider as
a parent. Add these dividers in order to be able to provide that parent
option.

Signed-off-by: Dmitry Osipenko 
---
 drivers/clk/tegra/clk-tegra20.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index 0ee56dd04cec..16cf4108f2ff 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -26,6 +26,8 @@
 #include "clk.h"
 #include "clk-id.h"
 
+#define MISC_CLK_ENB 0x48
+
 #define OSC_CTRL 0x50
 #define OSC_CTRL_OSC_FREQ_MASK (3<<30)
 #define OSC_CTRL_OSC_FREQ_13MHZ (0<<30)
@@ -831,6 +833,16 @@ static void __init tegra20_periph_clk_init(void)
periph_clk_enb_refcnt);
clks[TEGRA20_CLK_PEX] = clk;
 
+   /* cdev1 OSC divider */
+   clk_register_divider(NULL, "cdev1_osc_div", "clk_m",
+0, clk_base + MISC_CLK_ENB, 20, 2,
+CLK_DIVIDER_POWER_OF_TWO, NULL);
+
+   /* cdev2 OSC divider */
+   clk_register_divider(NULL, "cdev2_osc_div", "clk_m",
+0, clk_base + MISC_CLK_ENB, 22, 2,
+CLK_DIVIDER_POWER_OF_TWO, NULL);
+
/* cdev1 */
clk = clk_register_fixed_rate(NULL, "cdev1_fixed", NULL, 0, 2600);
clk = tegra_clk_register_periph_gate("cdev1", "cdev1_fixed", 0,
-- 
2.17.0