| Message ID | 20161128230428.6872-3-grygorii.strashko@ti.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Mon, Nov 28, 2016 at 05:04:24PM -0600, Grygorii Strashko wrote: > Some CPTS instances, which can be found on KeyStone 2 1/10G Ethernet > Switch Subsystems, can control an external multiplexer that selects > one of up to 32 clocks for time sync reference (RFTCLK). This feature > can be configured through CPTS_RFTCLK_SEL register (offset: x08). > > Hence, introduce optional DT cpts_rftclk_sel poperty wich, if present, > will specify CPTS reference clock. The cpts_rftclk_sel should be > omitted in DT if HW doesn't support this feature. The external fixed > rate clocks can be defined in board files as "fixed-clock". Can't you implement this using the clock tree, rather than an ad-hoc DT property? Thanks, Richard -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/30/2016 03:56 AM, Richard Cochran wrote: > On Mon, Nov 28, 2016 at 05:04:24PM -0600, Grygorii Strashko wrote: >> Some CPTS instances, which can be found on KeyStone 2 1/10G Ethernet >> Switch Subsystems, can control an external multiplexer that selects >> one of up to 32 clocks for time sync reference (RFTCLK). This feature >> can be configured through CPTS_RFTCLK_SEL register (offset: x08). >> >> Hence, introduce optional DT cpts_rftclk_sel poperty wich, if present, >> will specify CPTS reference clock. The cpts_rftclk_sel should be >> omitted in DT if HW doesn't support this feature. The external fixed >> rate clocks can be defined in board files as "fixed-clock". > > Can't you implement this using the clock tree, rather than an ad-hoc > DT property? > I've thought about this, but decided to move forward with this impl which is pretty simple. I will try.
On Mon, Nov 28, 2016 at 05:04:24PM -0600, Grygorii Strashko wrote: > Some CPTS instances, which can be found on KeyStone 2 1/10G Ethernet > Switch Subsystems, can control an external multiplexer that selects > one of up to 32 clocks for time sync reference (RFTCLK). This feature > can be configured through CPTS_RFTCLK_SEL register (offset: x08). > > Hence, introduce optional DT cpts_rftclk_sel poperty wich, if present, > will specify CPTS reference clock. The cpts_rftclk_sel should be > omitted in DT if HW doesn't support this feature. The external fixed > rate clocks can be defined in board files as "fixed-clock". > > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> > --- > Documentation/devicetree/bindings/net/keystone-netcp.txt | 2 ++ Please group binding changes into a single patch. > drivers/net/ethernet/ti/cpts.c | 12 ++++++++++++ > drivers/net/ethernet/ti/cpts.h | 8 +++++++- > 3 files changed, 21 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/net/keystone-netcp.txt b/Documentation/devicetree/bindings/net/keystone-netcp.txt > index c37b54e..ec4a241 100644 > --- a/Documentation/devicetree/bindings/net/keystone-netcp.txt > +++ b/Documentation/devicetree/bindings/net/keystone-netcp.txt > @@ -114,6 +114,8 @@ Optional properties: > driver to them if needed. > > Properties related to cpts configurations. > + - cpts-rftclk-sel: selects one of up to 32 clocks for time sync > + reference. Default = 0. Vendor prefix. > - cpts_clock_mult/cpts_clock_shift: > used for converting time counter cycles to ns as in > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
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/Documentation/devicetree/bindings/net/keystone-netcp.txt b/Documentation/devicetree/bindings/net/keystone-netcp.txt index c37b54e..ec4a241 100644 --- a/Documentation/devicetree/bindings/net/keystone-netcp.txt +++ b/Documentation/devicetree/bindings/net/keystone-netcp.txt @@ -114,6 +114,8 @@ Optional properties: driver to them if needed. Properties related to cpts configurations. + - cpts-rftclk-sel: selects one of up to 32 clocks for time sync + reference. Default = 0. - cpts_clock_mult/cpts_clock_shift: used for converting time counter cycles to ns as in diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c index c96a94a..9c5b835 100644 --- a/drivers/net/ethernet/ti/cpts.c +++ b/drivers/net/ethernet/ti/cpts.c @@ -459,6 +459,15 @@ static int cpts_of_parse(struct cpts *cpts, struct device_node *node) (!cpts->cc_mult && cpts->cc.shift)) goto of_error; + if (!of_property_read_u32(node, "cpts-rftclk-sel", &prop)) { + if (prop & ~CPTS_RFTCLK_SEL_MASK) { + dev_err(cpts->dev, "cpts: invalid cpts_rftclk_sel.\n"); + goto of_error; + } + cpts->caps |= CPTS_CAP_RFTCLK_SEL; + cpts->rftclk_sel = prop & CPTS_RFTCLK_SEL_MASK; + } + return 0; of_error: @@ -496,6 +505,9 @@ struct cpts *cpts_create(struct device *dev, void __iomem *regs, clk_prepare(cpts->refclk); + if (cpts->caps & CPTS_CAP_RFTCLK_SEL) + cpts_write32(cpts, cpts->rftclk_sel, rftclk_sel); + cpts->cc.read = cpts_systim_read; cpts->cc.mask = CLOCKSOURCE_MASK(32); cpts->info = cpts_info; diff --git a/drivers/net/ethernet/ti/cpts.h b/drivers/net/ethernet/ti/cpts.h index c96eca2..c934b61 100644 --- a/drivers/net/ethernet/ti/cpts.h +++ b/drivers/net/ethernet/ti/cpts.h @@ -35,7 +35,7 @@ struct cpsw_cpts { u32 idver; /* Identification and version */ u32 control; /* Time sync control */ - u32 res1; + u32 rftclk_sel; /* Reference Clock Select Register */ u32 ts_push; /* Time stamp event push */ u32 ts_load_val; /* Time stamp load value */ u32 ts_load_en; /* Time stamp load enable */ @@ -67,6 +67,8 @@ struct cpsw_cpts { #define INT_TEST (1<<1) /* Interrupt Test */ #define CPTS_EN (1<<0) /* Time Sync Enable */ +#define CPTS_RFTCLK_SEL_MASK 0x1f + /* * Definitions for the single bit resisters: * TS_PUSH TS_LOAD_EN INTSTAT_RAW INTSTAT_MASKED INT_ENABLE EVENT_POP @@ -107,6 +109,8 @@ struct cpts_event { u32 low; }; +#define CPTS_CAP_RFTCLK_SEL BIT(0) + struct cpts { struct device *dev; struct cpsw_cpts __iomem *reg; @@ -125,6 +129,8 @@ struct cpts { struct list_head pool; struct cpts_event pool_data[CPTS_MAX_EVENTS]; unsigned long ov_check_period; + u32 rftclk_sel; + u32 caps; }; void cpts_rx_timestamp(struct cpts *cpts, struct sk_buff *skb);
Some CPTS instances, which can be found on KeyStone 2 1/10G Ethernet Switch Subsystems, can control an external multiplexer that selects one of up to 32 clocks for time sync reference (RFTCLK). This feature can be configured through CPTS_RFTCLK_SEL register (offset: x08). Hence, introduce optional DT cpts_rftclk_sel poperty wich, if present, will specify CPTS reference clock. The cpts_rftclk_sel should be omitted in DT if HW doesn't support this feature. The external fixed rate clocks can be defined in board files as "fixed-clock". Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> --- Documentation/devicetree/bindings/net/keystone-netcp.txt | 2 ++ drivers/net/ethernet/ti/cpts.c | 12 ++++++++++++ drivers/net/ethernet/ti/cpts.h | 8 +++++++- 3 files changed, 21 insertions(+), 1 deletion(-)