| Message ID | 11994fbc-3713-6ef7-8a44-8a2442106dfc@ti.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, Dec 06, 2016 at 01:39:40PM -0600, Grygorii Strashko wrote:
> I come with below RFC patch. if no objection I'll move forward with it.
Thanks for following through with this!
The am335x will also need the MUX in its clock tree, won't it?
Thanks,
Richard
On 12/06/2016 02:25 PM, Richard Cochran wrote: > On Tue, Dec 06, 2016 at 01:39:40PM -0600, Grygorii Strashko wrote: >> I come with below RFC patch. if no objection I'll move forward with it. > > Thanks for following through with this! > > The am335x will also need the MUX in its clock tree, won't it? > Not exactly. I do not see CPTS_RFTCLK_SEL register in trm, but looks like it's already implemented in am335 clock tree: cpsw_cpts_rft_clk: cpsw_cpts_rft_clk@520 { #clock-cells = <0>; compatible = "ti,mux-clock"; clocks = <&dpll_core_m5_ck>, <&dpll_core_m4_ck>; reg = <0x0520>; }; and ssigned-clock-xx can be used to change parent in board file: &cpsw_cpts_rft_clk { assigned-clocks = <&cpsw_cpts_rft_clk>; assigned-clock-parents = <&dpll_core_m4_ck>; };
On 12/06, Grygorii Strashko wrote: > Subject: [PATCH] cpts refclk sel > > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> > --- > arch/arm/boot/dts/keystone-k2e-netcp.dtsi | 10 +++++- > drivers/net/ethernet/ti/cpts.c | 52 ++++++++++++++++++++++++++++++- > 2 files changed, 60 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/boot/dts/keystone-k2e-netcp.dtsi b/arch/arm/boot/dts/keystone-k2e-netcp.dtsi > index 919e655..b27aa22 100644 > --- a/arch/arm/boot/dts/keystone-k2e-netcp.dtsi > +++ b/arch/arm/boot/dts/keystone-k2e-netcp.dtsi > @@ -138,7 +138,7 @@ netcp: netcp@24000000 { > /* NetCP address range */ > ranges = <0 0x24000000 0x1000000>; > > - clocks = <&clkpa>, <&clkcpgmac>, <&chipclk12>; > + clocks = <&clkpa>, <&clkcpgmac>, <&cpts_mux>; > clock-names = "pa_clk", "ethss_clk", "cpts"; > dma-coherent; > > @@ -162,6 +162,14 @@ netcp: netcp@24000000 { > cpts-ext-ts-inputs = <6>; > cpts-ts-comp-length; > > + cpts_mux: cpts_refclk_mux { > + #clock-cells = <0>; > + clocks = <&chipclk12>, <&chipclk13>; > + cpts-mux-tbl = <0>, <1>; > + assigned-clocks = <&cpts_mux>; > + assigned-clock-parents = <&chipclk12>; Is there a binding update? Why the subnode? Why not have it as part of the netcp node? Does the cpts-mux-tbl property change? > + }; > + > interfaces { > gbe0: interface-0 { > slave-port = <0>; > diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c > index 938de22..ef94316 100644 > --- a/drivers/net/ethernet/ti/cpts.c > +++ b/drivers/net/ethernet/ti/cpts.c > @@ -17,6 +17,7 @@ > * along with this program; if not, write to the Free Software > * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > */ > +#include <linux/clk-provider.h> > #include <linux/err.h> > #include <linux/if.h> > #include <linux/hrtimer.h> > @@ -672,6 +673,7 @@ int cpts_register(struct cpts *cpts) > cpts->phc_index = ptp_clock_index(cpts->clock); > > schedule_delayed_work(&cpts->overflow_work, cpts->ov_check_period); > + Maybe in another patch. > return 0; > > err_ptp: > @@ -741,6 +743,54 @@ static void cpts_calc_mult_shift(struct cpts *cpts) > freq, cpts->cc_mult, cpts->cc.shift, (ns - NSEC_PER_SEC)); > } > > +static int cpts_of_mux_clk_setup(struct cpts *cpts, struct device_node *node) > +{ > + unsigned int num_parents; > + const char **parent_names; > + struct device_node *refclk_np; > + void __iomem *reg; > + struct clk *clk; > + u32 *mux_table; > + int ret; > + > + refclk_np = of_get_child_by_name(node, "cpts_refclk_mux"); > + if (!refclk_np) > + return -EINVAL; > + > + num_parents = of_clk_get_parent_count(refclk_np); > + if (num_parents < 1) { > + dev_err(cpts->dev, "mux-clock %s must have parents\n", > + refclk_np->name); > + return -EINVAL; > + } > + parent_names = devm_kzalloc(cpts->dev, (sizeof(char *) * num_parents), > + GFP_KERNEL); > + if (!parent_names) > + return -ENOMEM; > + > + of_clk_parent_fill(refclk_np, parent_names, num_parents); > + > + mux_table = devm_kzalloc(cpts->dev, sizeof(*mux_table) * (32 + 1), > + GFP_KERNEL); > + if (!mux_table) > + return -ENOMEM; > + > + ret = of_property_read_variable_u32_array(refclk_np, "cpts-mux-tbl", > + mux_table, 1, 32); > + if (ret < 0) > + return ret; > + > + reg = &cpts->reg->rftclk_sel; > + > + clk = clk_register_mux_table(cpts->dev, refclk_np->name, > + parent_names, num_parents, > + 0, reg, 0, 0x1F, 0, mux_table, NULL); > + if (IS_ERR(clk)) > + return PTR_ERR(clk); > + > + return of_clk_add_provider(refclk_np, of_clk_src_simple_get, clk); Can you please use the clk_hw APIs instead?
On 12/08/2016 06:47 PM, Stephen Boyd wrote: > On 12/06, Grygorii Strashko wrote: >> Subject: [PATCH] cpts refclk sel >> >> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> >> --- >> arch/arm/boot/dts/keystone-k2e-netcp.dtsi | 10 +++++- >> drivers/net/ethernet/ti/cpts.c | 52 ++++++++++++++++++++++++++++++- >> 2 files changed, 60 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm/boot/dts/keystone-k2e-netcp.dtsi b/arch/arm/boot/dts/keystone-k2e-netcp.dtsi >> index 919e655..b27aa22 100644 >> --- a/arch/arm/boot/dts/keystone-k2e-netcp.dtsi >> +++ b/arch/arm/boot/dts/keystone-k2e-netcp.dtsi >> @@ -138,7 +138,7 @@ netcp: netcp@24000000 { >> /* NetCP address range */ >> ranges = <0 0x24000000 0x1000000>; >> >> - clocks = <&clkpa>, <&clkcpgmac>, <&chipclk12>; >> + clocks = <&clkpa>, <&clkcpgmac>, <&cpts_mux>; ^^ mux clock used here >> clock-names = "pa_clk", "ethss_clk", "cpts"; >> dma-coherent; >> >> @@ -162,6 +162,14 @@ netcp: netcp@24000000 { >> cpts-ext-ts-inputs = <6>; >> cpts-ts-comp-length; >> >> + cpts_mux: cpts_refclk_mux { >> + #clock-cells = <0>; >> + clocks = <&chipclk12>, <&chipclk13>; >> + cpts-mux-tbl = <0>, <1>; >> + assigned-clocks = <&cpts_mux>; >> + assigned-clock-parents = <&chipclk12>; > > Is there a binding update? this was pure RFC-DEV patch just to check the possibility of modeling CPTS_RFTCLK_SEL register as mux clock. Original patch: https://lkml.org/lkml/2016/11/28/780 I've plan to resend it using clk framework. Why the subnode? Sry, I did not get this question - is there another way to pas phandle on clock in clocks list property? Am I missing smth.? Sry, this is my first clock :) > Why not have it as part of the netcp node? cpts is part of gbe ethss, which is part of netcp. Only netcp is modeled as DD - cpts and gbe ethss implemented without using DD model, so generic resources acquired by netcp and then passed to cpts and gbe ethss. CPTS has register to control an external multiplexer that selects one of up to 32 clocks for time sync reference (RFTCLK) > Does the cpts-mux-tbl property change? On Keystone 2 66AK2e (as example) the following list of clocks can be selected as ref clocks (list is different for other SoCs): 0000 = SYSCLK2 0001 = SYSCLK3 0010 = TIMI0 0011 = TIMI1 0100 = TSIPCLKA 1000 = TSREFCLK 1100 = TSIPCLKB Others = Reserved and only 0 and 1 are internal, other external and board specific (parameters unknown and corresponding inputs can be used for other purposes), so I can't define all parent clocks, only internal: clocks = <&chipclk12>, <&chipclk13>; cpts-mux-tbl = <0>, <1>; to use another, external, clock - it should be explicitly defined in board file the board file timi1clk: timi1clk { #clock-cells = <0>; compatible = "fixed-clock"; ... &cpts_mux { clocks = <&chipclk12>, <&chipclk13>, <timi1clk>; ^^^ i can't predict value here cpts-mux-tbl = <0>, <1>, <3>; ^^i can't predict value here assigned-clocks = <&cpts_mux>; assigned-clock-parents = <&timi1clk>; }; or I understood your question wrongly? > >> + }; >> + >> interfaces { >> gbe0: interface-0 { >> slave-port = <0>; >> diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c >> index 938de22..ef94316 100644 >> --- a/drivers/net/ethernet/ti/cpts.c >> +++ b/drivers/net/ethernet/ti/cpts.c >> @@ -17,6 +17,7 @@ >> * along with this program; if not, write to the Free Software >> * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA >> */ >> +#include <linux/clk-provider.h> >> #include <linux/err.h> >> #include <linux/if.h> >> #include <linux/hrtimer.h> >> @@ -672,6 +673,7 @@ int cpts_register(struct cpts *cpts) >> cpts->phc_index = ptp_clock_index(cpts->clock); >> >> schedule_delayed_work(&cpts->overflow_work, cpts->ov_check_period); >> + > > Maybe in another patch. > sure >> return 0; >> >> err_ptp: >> @@ -741,6 +743,54 @@ static void cpts_calc_mult_shift(struct cpts *cpts) >> freq, cpts->cc_mult, cpts->cc.shift, (ns - NSEC_PER_SEC)); >> } >> ... >> + >> + reg = &cpts->reg->rftclk_sel; >> + >> + clk = clk_register_mux_table(cpts->dev, refclk_np->name, >> + parent_names, num_parents, >> + 0, reg, 0, 0x1F, 0, mux_table, NULL); >> + if (IS_ERR(clk)) >> + return PTR_ERR(clk); >> + >> + return of_clk_add_provider(refclk_np, of_clk_src_simple_get, clk); > > Can you please use the clk_hw APIs instead? > ok
diff --git a/arch/arm/boot/dts/keystone-k2e-netcp.dtsi b/arch/arm/boot/dts/keystone-k2e-netcp.dtsi index 919e655..b27aa22 100644 --- a/arch/arm/boot/dts/keystone-k2e-netcp.dtsi +++ b/arch/arm/boot/dts/keystone-k2e-netcp.dtsi @@ -138,7 +138,7 @@ netcp: netcp@24000000 { /* NetCP address range */ ranges = <0 0x24000000 0x1000000>; - clocks = <&clkpa>, <&clkcpgmac>, <&chipclk12>; + clocks = <&clkpa>, <&clkcpgmac>, <&cpts_mux>; clock-names = "pa_clk", "ethss_clk", "cpts"; dma-coherent; @@ -162,6 +162,14 @@ netcp: netcp@24000000 { cpts-ext-ts-inputs = <6>; cpts-ts-comp-length; + cpts_mux: cpts_refclk_mux { + #clock-cells = <0>; + clocks = <&chipclk12>, <&chipclk13>; + cpts-mux-tbl = <0>, <1>; + assigned-clocks = <&cpts_mux>; + assigned-clock-parents = <&chipclk12>; + }; + interfaces { gbe0: interface-0 { slave-port = <0>; diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c index 938de22..ef94316 100644 --- a/drivers/net/ethernet/ti/cpts.c +++ b/drivers/net/ethernet/ti/cpts.c @@ -17,6 +17,7 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */ +#include <linux/clk-provider.h> #include <linux/err.h> #include <linux/if.h> #include <linux/hrtimer.h> @@ -672,6 +673,7 @@ int cpts_register(struct cpts *cpts) cpts->phc_index = ptp_clock_index(cpts->clock); schedule_delayed_work(&cpts->overflow_work, cpts->ov_check_period); + return 0; err_ptp: @@ -741,6 +743,54 @@ static void cpts_calc_mult_shift(struct cpts *cpts) freq, cpts->cc_mult, cpts->cc.shift, (ns - NSEC_PER_SEC)); } +static int cpts_of_mux_clk_setup(struct cpts *cpts, struct device_node *node) +{ + unsigned int num_parents; + const char **parent_names; + struct device_node *refclk_np; + void __iomem *reg; + struct clk *clk; + u32 *mux_table; + int ret; + + refclk_np = of_get_child_by_name(node, "cpts_refclk_mux"); + if (!refclk_np) + return -EINVAL; + + num_parents = of_clk_get_parent_count(refclk_np); + if (num_parents < 1) { + dev_err(cpts->dev, "mux-clock %s must have parents\n", + refclk_np->name); + return -EINVAL; + } + parent_names = devm_kzalloc(cpts->dev, (sizeof(char *) * num_parents), + GFP_KERNEL); + if (!parent_names) + return -ENOMEM; + + of_clk_parent_fill(refclk_np, parent_names, num_parents); + + mux_table = devm_kzalloc(cpts->dev, sizeof(*mux_table) * (32 + 1), + GFP_KERNEL); + if (!mux_table) + return -ENOMEM; + + ret = of_property_read_variable_u32_array(refclk_np, "cpts-mux-tbl", + mux_table, 1, 32); + if (ret < 0) + return ret; + + reg = &cpts->reg->rftclk_sel; + + clk = clk_register_mux_table(cpts->dev, refclk_np->name, + parent_names, num_parents, + 0, reg, 0, 0x1F, 0, mux_table, NULL); + if (IS_ERR(clk)) + return PTR_ERR(clk); + + return of_clk_add_provider(refclk_np, of_clk_src_simple_get, clk); +} + static int cpts_of_parse(struct cpts *cpts, struct device_node *node) { int ret = -EINVAL; @@ -787,7 +837,7 @@ static int cpts_of_parse(struct cpts *cpts, struct device_node *node) if (!of_property_read_u32(node, "cpts-ext-ts-inputs", &prop)) cpts->ext_ts_inputs = prop; - return 0; + return cpts_of_mux_clk_setup(cpts, node); of_error: dev_err(cpts->dev, "CPTS: Missing property in the DT.\n");