| Message ID | 20180426235818.10018-2-digetx@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Restore ULPI USB on Tegra20 | expand |
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 <digetx@gmail.com> > --- > 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, > 26000000); > clk = tegra_clk_register_periph_gate("cdev1", "cdev1_fixed", > 0, Cheers Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <digetx@gmail.com> >> --- >> 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, >> 26000000); >> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <digetx@gmail.com> > > > --- > > > 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, > > > 26000000); > > > 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 <marcel@ziswiler.com> Tested-by: Marcel Ziswiler <marcel@ziswiler.com> -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <digetx@gmail.com> > --- > 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, 26000000); > clk = tegra_clk_register_periph_gate("cdev1", "cdev1_fixed", 0, > -- > 2.17.0 > Peter. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <digetx@gmail.com> >>>> --- >>>> 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, >>>> 26000000); >>>> 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 <marcel@ziswiler.com> > Tested-by: Marcel Ziswiler <marcel@ziswiler.com> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <digetx@gmail.com> >>>>> --- >>>>> 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, >>>>> 26000000); >>>>> 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 <marcel@ziswiler.com> >> Tested-by: Marcel Ziswiler <marcel@ziswiler.com> > > 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. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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, 26000000); clk = tegra_clk_register_periph_gate("cdev1", "cdev1_fixed", 0,
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 <digetx@gmail.com> --- drivers/clk/tegra/clk-tegra20.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)