| Message ID | 1554150230-24335-5-git-send-email-eajames@linux.ibm.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Enable video engine | expand |
On 4/1/2019 1:23 PM, Eddie James wrote: > Add eclk mux and clock divider table. > > Signed-off-by: Eddie James <eajames@linux.ibm.com> > --- > drivers/clk/clk-aspeed.c | 40 ++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 38 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c > index 5961367..133e80c 100644 > --- a/drivers/clk/clk-aspeed.c > +++ b/drivers/clk/clk-aspeed.c > @@ -87,7 +87,7 @@ struct aspeed_clk_gate { > /* TODO: ask Aspeed about the actual parent data */ > static const struct aspeed_gate_data aspeed_gates[] = { > /* clk rst name parent flags */ > - [ASPEED_CLK_GATE_ECLK] = { 0, -1, "eclk-gate", "eclk", 0 }, /* Video Engine */ > + [ASPEED_CLK_GATE_ECLK] = { 0, 6, "eclk-gate", "eclk", 0 }, /* Video Engine */ > [ASPEED_CLK_GATE_GCLK] = { 1, 7, "gclk-gate", NULL, 0 }, /* 2D engine */ > [ASPEED_CLK_GATE_MCLK] = { 2, -1, "mclk-gate", "mpll", CLK_IS_CRITICAL }, /* SDRAM */ > [ASPEED_CLK_GATE_VCLK] = { 3, 6, "vclk-gate", NULL, 0 }, /* Video Capture */ Hi Eddie, I already left a comment on it but you probably didn't see that. With this change, both eclk and vclk will be coupled with reset bit '6'. Actually, the reset bit 6 is for video engine so this change seems correct, but you should replace reset bit of vclk with -1 instead otherwise the video engine reset will be triggered twice and it will take at least total 20ms of delay time which is a bad idea in case if aspeed_clk_enable is called from interrupt context. Test it again after changing the vclk rst setting to '-1' on top of this patch series. It worked well in my H/W. Cheers, Jae > @@ -113,6 +113,24 @@ struct aspeed_clk_gate { > [ASPEED_CLK_GATE_LHCCLK] = { 28, -1, "lhclk-gate", "lhclk", 0 }, /* LPC master/LPC+ */ > }; > > +static const char * const eclk_parent_names[] = { > + "mpll", > + "hpll", > + "dpll", > +}; > + > +static const struct clk_div_table ast2500_eclk_div_table[] = { > + { 0x0, 2 }, > + { 0x1, 2 }, > + { 0x2, 3 }, > + { 0x3, 4 }, > + { 0x4, 5 }, > + { 0x5, 6 }, > + { 0x6, 7 }, > + { 0x7, 8 }, > + { 0 } > +}; > + > static const struct clk_div_table ast2500_mac_div_table[] = { > { 0x0, 4 }, /* Yep, really. Aspeed confirmed this is correct */ > { 0x1, 4 }, > @@ -192,18 +210,21 @@ static struct clk_hw *aspeed_ast2500_calc_pll(const char *name, u32 val) > > struct aspeed_clk_soc_data { > const struct clk_div_table *div_table; > + const struct clk_div_table *eclk_div_table; > const struct clk_div_table *mac_div_table; > struct clk_hw *(*calc_pll)(const char *name, u32 val); > }; > > static const struct aspeed_clk_soc_data ast2500_data = { > .div_table = ast2500_div_table, > + .eclk_div_table = ast2500_eclk_div_table, > .mac_div_table = ast2500_mac_div_table, > .calc_pll = aspeed_ast2500_calc_pll, > }; > > static const struct aspeed_clk_soc_data ast2400_data = { > .div_table = ast2400_div_table, > + .eclk_div_table = ast2400_div_table, > .mac_div_table = ast2400_div_table, > .calc_pll = aspeed_ast2400_calc_pll, > }; > @@ -522,6 +543,22 @@ static int aspeed_clk_probe(struct platform_device *pdev) > return PTR_ERR(hw); > aspeed_clk_data->hws[ASPEED_CLK_24M] = hw; > > + hw = clk_hw_register_mux(dev, "eclk-mux", eclk_parent_names, > + ARRAY_SIZE(eclk_parent_names), 0, > + scu_base + ASPEED_CLK_SELECTION, 2, 0x3, 0, > + &aspeed_clk_lock); > + if (IS_ERR(hw)) > + return PTR_ERR(hw); > + aspeed_clk_data->hws[ASPEED_CLK_ECLK_MUX] = hw; > + > + hw = clk_hw_register_divider_table(dev, "eclk", "eclk-mux", 0, > + scu_base + ASPEED_CLK_SELECTION, 28, > + 3, 0, soc_data->eclk_div_table, > + &aspeed_clk_lock); > + if (IS_ERR(hw)) > + return PTR_ERR(hw); > + aspeed_clk_data->hws[ASPEED_CLK_ECLK] = hw; > + > /* > * TODO: There are a number of clocks that not included in this driver > * as more information is required: > @@ -531,7 +568,6 @@ static int aspeed_clk_probe(struct platform_device *pdev) > * RGMII > * RMII > * UART[1..5] clock source mux > - * Video Engine (ECLK) mux and clock divider > */ > > for (i = 0; i < ARRAY_SIZE(aspeed_gates); i++) { >
On 4/1/19 3:48 PM, Jae Hyun Yoo wrote: > On 4/1/2019 1:23 PM, Eddie James wrote: >> Add eclk mux and clock divider table. >> >> Signed-off-by: Eddie James <eajames@linux.ibm.com> >> --- >> drivers/clk/clk-aspeed.c | 40 ++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 38 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c >> index 5961367..133e80c 100644 >> --- a/drivers/clk/clk-aspeed.c >> +++ b/drivers/clk/clk-aspeed.c >> @@ -87,7 +87,7 @@ struct aspeed_clk_gate { >> /* TODO: ask Aspeed about the actual parent data */ >> static const struct aspeed_gate_data aspeed_gates[] = { >> /* clk rst name parent flags */ >> - [ASPEED_CLK_GATE_ECLK] = { 0, -1, "eclk-gate", "eclk", 0 >> }, /* Video Engine */ >> + [ASPEED_CLK_GATE_ECLK] = { 0, 6, "eclk-gate", "eclk", 0 >> }, /* Video Engine */ >> [ASPEED_CLK_GATE_GCLK] = { 1, 7, "gclk-gate", NULL, 0 >> }, /* 2D engine */ >> [ASPEED_CLK_GATE_MCLK] = { 2, -1, "mclk-gate", "mpll", >> CLK_IS_CRITICAL }, /* SDRAM */ >> [ASPEED_CLK_GATE_VCLK] = { 3, 6, "vclk-gate", NULL, 0 >> }, /* Video Capture */ > > Hi Eddie, > > I already left a comment on it but you probably didn't see that. > With this change, both eclk and vclk will be coupled with reset bit '6'. > Actually, the reset bit 6 is for video engine so this change seems > correct, but you should replace reset bit of vclk with -1 instead > otherwise the video engine reset will be triggered twice and it will > take at least total 20ms of delay time which is a bad idea in case if > aspeed_clk_enable is called from interrupt context. > > Test it again after changing the vclk rst setting to '-1' on top of this > patch series. It worked well in my H/W. Sorry, I had seen it and then forgotten about it... thanks Jae, will include. Eddie > > Cheers, > Jae > >> @@ -113,6 +113,24 @@ struct aspeed_clk_gate { >> [ASPEED_CLK_GATE_LHCCLK] = { 28, -1, "lhclk-gate", >> "lhclk", 0 }, /* LPC master/LPC+ */ >> }; >> +static const char * const eclk_parent_names[] = { >> + "mpll", >> + "hpll", >> + "dpll", >> +}; >> + >> +static const struct clk_div_table ast2500_eclk_div_table[] = { >> + { 0x0, 2 }, >> + { 0x1, 2 }, >> + { 0x2, 3 }, >> + { 0x3, 4 }, >> + { 0x4, 5 }, >> + { 0x5, 6 }, >> + { 0x6, 7 }, >> + { 0x7, 8 }, >> + { 0 } >> +}; >> + >> static const struct clk_div_table ast2500_mac_div_table[] = { >> { 0x0, 4 }, /* Yep, really. Aspeed confirmed this is correct */ >> { 0x1, 4 }, >> @@ -192,18 +210,21 @@ static struct clk_hw >> *aspeed_ast2500_calc_pll(const char *name, u32 val) >> struct aspeed_clk_soc_data { >> const struct clk_div_table *div_table; >> + const struct clk_div_table *eclk_div_table; >> const struct clk_div_table *mac_div_table; >> struct clk_hw *(*calc_pll)(const char *name, u32 val); >> }; >> static const struct aspeed_clk_soc_data ast2500_data = { >> .div_table = ast2500_div_table, >> + .eclk_div_table = ast2500_eclk_div_table, >> .mac_div_table = ast2500_mac_div_table, >> .calc_pll = aspeed_ast2500_calc_pll, >> }; >> static const struct aspeed_clk_soc_data ast2400_data = { >> .div_table = ast2400_div_table, >> + .eclk_div_table = ast2400_div_table, >> .mac_div_table = ast2400_div_table, >> .calc_pll = aspeed_ast2400_calc_pll, >> }; >> @@ -522,6 +543,22 @@ static int aspeed_clk_probe(struct >> platform_device *pdev) >> return PTR_ERR(hw); >> aspeed_clk_data->hws[ASPEED_CLK_24M] = hw; >> + hw = clk_hw_register_mux(dev, "eclk-mux", eclk_parent_names, >> + ARRAY_SIZE(eclk_parent_names), 0, >> + scu_base + ASPEED_CLK_SELECTION, 2, 0x3, 0, >> + &aspeed_clk_lock); >> + if (IS_ERR(hw)) >> + return PTR_ERR(hw); >> + aspeed_clk_data->hws[ASPEED_CLK_ECLK_MUX] = hw; >> + >> + hw = clk_hw_register_divider_table(dev, "eclk", "eclk-mux", 0, >> + scu_base + ASPEED_CLK_SELECTION, 28, >> + 3, 0, soc_data->eclk_div_table, >> + &aspeed_clk_lock); >> + if (IS_ERR(hw)) >> + return PTR_ERR(hw); >> + aspeed_clk_data->hws[ASPEED_CLK_ECLK] = hw; >> + >> /* >> * TODO: There are a number of clocks that not included in this >> driver >> * as more information is required: >> @@ -531,7 +568,6 @@ static int aspeed_clk_probe(struct >> platform_device *pdev) >> * RGMII >> * RMII >> * UART[1..5] clock source mux >> - * Video Engine (ECLK) mux and clock divider >> */ >> for (i = 0; i < ARRAY_SIZE(aspeed_gates); i++) { >> >
diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c index 5961367..133e80c 100644 --- a/drivers/clk/clk-aspeed.c +++ b/drivers/clk/clk-aspeed.c @@ -87,7 +87,7 @@ struct aspeed_clk_gate { /* TODO: ask Aspeed about the actual parent data */ static const struct aspeed_gate_data aspeed_gates[] = { /* clk rst name parent flags */ - [ASPEED_CLK_GATE_ECLK] = { 0, -1, "eclk-gate", "eclk", 0 }, /* Video Engine */ + [ASPEED_CLK_GATE_ECLK] = { 0, 6, "eclk-gate", "eclk", 0 }, /* Video Engine */ [ASPEED_CLK_GATE_GCLK] = { 1, 7, "gclk-gate", NULL, 0 }, /* 2D engine */ [ASPEED_CLK_GATE_MCLK] = { 2, -1, "mclk-gate", "mpll", CLK_IS_CRITICAL }, /* SDRAM */ [ASPEED_CLK_GATE_VCLK] = { 3, 6, "vclk-gate", NULL, 0 }, /* Video Capture */ @@ -113,6 +113,24 @@ struct aspeed_clk_gate { [ASPEED_CLK_GATE_LHCCLK] = { 28, -1, "lhclk-gate", "lhclk", 0 }, /* LPC master/LPC+ */ }; +static const char * const eclk_parent_names[] = { + "mpll", + "hpll", + "dpll", +}; + +static const struct clk_div_table ast2500_eclk_div_table[] = { + { 0x0, 2 }, + { 0x1, 2 }, + { 0x2, 3 }, + { 0x3, 4 }, + { 0x4, 5 }, + { 0x5, 6 }, + { 0x6, 7 }, + { 0x7, 8 }, + { 0 } +}; + static const struct clk_div_table ast2500_mac_div_table[] = { { 0x0, 4 }, /* Yep, really. Aspeed confirmed this is correct */ { 0x1, 4 }, @@ -192,18 +210,21 @@ static struct clk_hw *aspeed_ast2500_calc_pll(const char *name, u32 val) struct aspeed_clk_soc_data { const struct clk_div_table *div_table; + const struct clk_div_table *eclk_div_table; const struct clk_div_table *mac_div_table; struct clk_hw *(*calc_pll)(const char *name, u32 val); }; static const struct aspeed_clk_soc_data ast2500_data = { .div_table = ast2500_div_table, + .eclk_div_table = ast2500_eclk_div_table, .mac_div_table = ast2500_mac_div_table, .calc_pll = aspeed_ast2500_calc_pll, }; static const struct aspeed_clk_soc_data ast2400_data = { .div_table = ast2400_div_table, + .eclk_div_table = ast2400_div_table, .mac_div_table = ast2400_div_table, .calc_pll = aspeed_ast2400_calc_pll, }; @@ -522,6 +543,22 @@ static int aspeed_clk_probe(struct platform_device *pdev) return PTR_ERR(hw); aspeed_clk_data->hws[ASPEED_CLK_24M] = hw; + hw = clk_hw_register_mux(dev, "eclk-mux", eclk_parent_names, + ARRAY_SIZE(eclk_parent_names), 0, + scu_base + ASPEED_CLK_SELECTION, 2, 0x3, 0, + &aspeed_clk_lock); + if (IS_ERR(hw)) + return PTR_ERR(hw); + aspeed_clk_data->hws[ASPEED_CLK_ECLK_MUX] = hw; + + hw = clk_hw_register_divider_table(dev, "eclk", "eclk-mux", 0, + scu_base + ASPEED_CLK_SELECTION, 28, + 3, 0, soc_data->eclk_div_table, + &aspeed_clk_lock); + if (IS_ERR(hw)) + return PTR_ERR(hw); + aspeed_clk_data->hws[ASPEED_CLK_ECLK] = hw; + /* * TODO: There are a number of clocks that not included in this driver * as more information is required: @@ -531,7 +568,6 @@ static int aspeed_clk_probe(struct platform_device *pdev) * RGMII * RMII * UART[1..5] clock source mux - * Video Engine (ECLK) mux and clock divider */ for (i = 0; i < ARRAY_SIZE(aspeed_gates); i++) {
Add eclk mux and clock divider table. Signed-off-by: Eddie James <eajames@linux.ibm.com> --- drivers/clk/clk-aspeed.c | 40 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-)