clk: sirf: add CSR atlas7 clk and reset support
Subsystems (2)
PATCH v2 1/1 clk: sirf: add CSR atlas7 clk and reset support
From: Zhiwu Song <[email protected]>
the hardware node includes both clock and reset support, so it is named as "car".
this patch implements Flexible clocks(mux, divider, gate), Selectable clock(mux, divider, gate), root clock(gate),leaf clock(gate), others. it also implements the reset controller functionality.
---
-v2: add example for clock/reset controller node in dt-binding doc
.../devicetree/bindings/clock/csr,atlas7-car.txt | 55 + drivers/clk/sirf/Makefile | 2 +- drivers/clk/sirf/clk-atlas7.c | 1652 ++++++++++++++++++++ 3 files changed, 1708 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/clock/csr,atlas7-car.txt create mode 100644 drivers/clk/sirf/clk-atlas7.c
Documentation/devicetree/bindings/clock/csr,atlas7-car.txt new file +55 -0
@@ -0,0 +1,55 @@ +* Clock and reset bindings for CSR atlas7 + +Required properties: +- compatible: Should be "sirf,atlas7-car" +- reg: Address and length of the register set +- #clock-cells: Should be <1> +- #reset-cells: Should be <1> + +The clock consumer should specify the desired clock by having the clock +ID in its "clocks" phandle cell. +The ID list atlas7_clks defined in drivers/clk/sirf/clk-atlas7.c + +The reset consumer should specify the desired reset by having the reset +ID in its "reset" phandle cell. +The ID list atlas7_reset_unit defined in drivers/clk/sirf/clk-atlas7.c + +Examples: Clock and reset controller node: + +car: clock-controller at 18620000 { + compatible = "sirf,atlas7-car"; + reg = <0x18620000 0x1000>; + #clock-cells = <1>; + #reset-cells = <1>; +}; + +Examples: Consumers using clock or reset: + +timer at 10dc0000 { + compatible = "sirf,macro-tick"; + reg = <0x10dc0000 0x1000>; + clocks = <&car 54>; + interrupts = <0 0 0>, + <0 1 0>, + <0 2 0>, + <0 49 0>, + <0 50 0>, + <0 51 0>; +}; + +uart1: uart at 18020000 { + cell-index = <1>; + compatible = "sirf,macro-uart"; + reg = <0x18020000 0x1000>; + clocks = <&clks 95>; + interrupts = <0 18 0>; + fifosize = <32>; +}; + +vpp at 13110000 { + compatible = "sirf,prima2-vpp"; + reg = <0x13110000 0x10000>; + interrupts = <0 31 0>; + clocks = <&car 85>; + resets = <&car 29>; +};
drivers/clk/sirf/Makefile +1 -1
@@ -2,4 +2,4 @@ # Makefile for sirf specific clk # -obj-$(CONFIG_ARCH_SIRF) += clk-prima2.o clk-atlas6.o +obj-$(CONFIG_ARCH_SIRF) += clk-prima2.o clk-atlas6.o clk-atlas7.o
drivers/clk/sirf/clk-atlas7.c new file +1652 -0
1,652 lines not on this page · raw message
Signature · 1 line
1.9.1
PATCH v2 1/1 clk: sirf: add CSR atlas7 clk and reset support
Quoted 11 lines · Barry Song
From: Zhiwu Song <[email protected]>
the hardware node includes both clock and reset support, so it is named as "car".
this patch implements Flexibleclocks(mux, divider, gate), Selectableclock(mux, divider, gate), root clock(gate),leaf clock(gate), others. it also implements the reset controller functionality.Signed-off-by Zhiwu Song <[email protected]>Signed-off-by Barry Song <[email protected]>---
Sorry for late review. I wasn't Cc'ed on the patch.
drivers/clk/sirf/clk-atlas7.c line 212
Quoted 3 lines
drivers/clk/sirf/clk-atlas7.c new file +3
-0+ + +static void *sirfsoc_clk_vbase, *sirfsoc_clk_vbase;
Missing __iomem and duplicate sirfsoc_clk_vbase.
Quoted 11 lines
drivers/clk/sirf/clk-atlas7.c new file +11
-0+static struct clk_onecell_data clk_data; + +static const struct clk_div_table pll_div_table[] = { + { .val =0 , .div =1 }, + { .val =1 , .div =2 }, + { .val =2 , .div =4 }, + { .val =3 , .div =8 }, + { .val =4 , .div =16 }, + { .val =5 , .div =32 }, +}; +
drivers/clk/sirf/clk-atlas7.c line 233
Quoted 10 lines
drivers/clk/sirf/clk-atlas7.c new file +10
-0+struct clk_pll { +struct clk_hw hw; +unsigned short regofs;/* register offset */ +}; +#define to_pllclk (_hw) container_of(_hw, struct clk_pll, hw) + +struct clk_dto { +struct clk_hw hw; +unsigned short inc_offset;/* dto increment offset */ +unsigned short src_offset;/* dto src offset */
Why not use u16 instead of unsigned short?
Quoted 3 lines
drivers/clk/sirf/clk-atlas7.c new file +3
-0+}; +#define to_dtoclk (_hw) container_of(_hw, struct clk_dto, hw) +
[...]
drivers/clk/sirf/clk-atlas7.c line 337
Quoted 4 lines
drivers/clk/sirf/clk-atlas7.c new file +4
-0+ +/* +ABPLL +interger_n mode: Fvco = Fin * 2 * NF / NR
s/interger_n/integer_n/ ?
Quoted 5 lines
drivers/clk/sirf/clk-atlas7.c new file +5
-0+Spread Spectrum mode: Fvco = Fin * SSN / NR +SSN = 2^24 / (256 * ((ssdiv >> ssdepth) << ssdepth) + (ssmod << ssdepth)) +*/ +static unsigned long pll_clk_recalc_rate (struct clk_hw *hw, +unsigned long parent_rate)
[...]
drivers/clk/sirf/clk-atlas7.c line 470
Quoted 6 lines
drivers/clk/sirf/clk-atlas7.c new file +6
-0+ +/* +DTO in clkc, default enable double resolution mode +double resolution mode:fout = fin * finc / 2^29 +normal mode:fout = fin * finc / 2^28 +*/
Nitpick: Comments should have a '*' at the beginning of each line.
Quoted 9 lines
drivers/clk/sirf/clk-atlas7.c new file +9
-0+static int dto_clk_is_enabled (struct clk_hw *hw) +{ +struct clk_dto *clk =to_dtoclk (hw); +int reg; + + reg = clk->src_offset +SIRFSOC_CLKC_AUDIO_DTO_ENA -SIRFSOC_CLKC_AUDIO_DTO_SRC ; + +return !!(clkc_readl (reg) &BIT (0 )); +
[...]
drivers/clk/sirf/clk-atlas7.c line 598
Quoted 9 lines
drivers/clk/sirf/clk-atlas7.c new file +9
-0+static struct clk_dto clk_audio_dto = { + .inc_offset =SIRFSOC_CLKC_AUDIO_DTO_INC , + .src_offset =SIRFSOC_CLKC_AUDIO_DTO_SRC , + .hw = { + .init = &clk_audiodto_init, + }, +}; + +static const char *disp0dto_clk_parents[] = {
If you rebase onto clk-next you can make all these parent string arrays const char * const so that checkpatch doesn't complain.
Quoted 5 lines
drivers/clk/sirf/clk-atlas7.c new file +5
-0+"sys0pll_clk1" , +"sys1pll_clk1" , + "sys3pll_clk1", +}; +
[...]
drivers/clk/sirf/clk-atlas7.c line 1198
Quoted 6 lines
drivers/clk/sirf/clk-atlas7.c new file +6
-0+ +static int unit_clk_enable (struct clk_hw *hw) +{ +u32 reg; +struct clk_unit *clk =to_unitclk (hw); +unsigned long flags =0 ;
Unnecessary initialization.
Quoted 4 lines
drivers/clk/sirf/clk-atlas7.c new file +4
-0+ + reg = clk->regofs; + +spin_lock_irqsave (clk->lock, flags);
drivers/clk/sirf/clk-atlas7.c line 1212
Quoted 10 lines
drivers/clk/sirf/clk-atlas7.c new file +10
-0+clkc_writel (BIT (clk->bit), reg); +spin_unlock_irqrestore (clk->lock, flags); +return 0 ; +} + +static void unit_clk_disable (struct clk_hw *hw) +{ +u32 reg; +struct clk_unit *clk =to_unitclk (hw); +unsigned long flags =0 ;
Unnecessary initialization.
Quoted 17 lines
drivers/clk/sirf/clk-atlas7.c new file +17
-0+ + reg = clk->regofs +SIRFSOC_CLKC_ROOT_CLK_EN0_CLR -SIRFSOC_CLKC_ROOT_CLK_EN0_SET ; + +spin_lock_irqsave (clk->lock, flags); +clkc_writel (BIT (clk->bit), reg); +spin_unlock_irqrestore (clk->lock, flags); +} + +static struct clk_ops unit_clk_ops = { + .is_enabled = unit_clk_is_enabled, + .enable = unit_clk_enable, + .disable = unit_clk_disable, +}; + +static struct clk *__init +atlas7_unit_clk_register(struct device *dev, const char *name, + const char *parent_name, unsigned long flags,
drivers/clk/sirf/clk-atlas7.c line 1239
Quoted 10 lines
drivers/clk/sirf/clk-atlas7.c new file +10
-0+ u32 regofs, u8 bit, spinlock_t *lock) +{ + struct clk *clk; +struct clk_unit *unit; +struct clk_init_data init; + + unit =kzalloc (sizeof (*unit),GFP_KERNEL ); +if (!unit) { +pr_err ("could not alloc unit clock %s\n" , + name);
Useless error message. Please run checkpatch.
drivers/clk/sirf/clk-atlas7.c line 1244
Quoted 5 lines
drivers/clk/sirf/clk-atlas7.c new file +5
-0+return ERR_PTR (-ENOMEM ); + } + init.name = name; + init.parent_names = (parent_name ? &parent_name :NULL ); + init.num_parents = (parent_name ?1 :0 );
Why the parenthesis?
Quoted 7 lines
drivers/clk/sirf/clk-atlas7.c new file +7
-0+ init.ops = &unit_clk_ops; + init.flags = flags; + + unit->hw.init = &init; + unit->regofs = regofs; + unit->bit = bit; + unit->lock = lock;
drivers/clk/sirf/clk-atlas7.c line 1261
Quoted 10 lines
drivers/clk/sirf/clk-atlas7.c new file +10
-0+ + clk =clk_register (dev, &unit->hw); +if (IS_ERR (clk)) +kfree (unit); + +return clk; +} + +static struct atlas7_reset_desc atlas7_reset_unit[] = { + {"PWM" ,0x0244 ,0 ,0x0320 ,0 , &leaf0_gate_lock},/*0-5*/
Style nitpick: Please put some space around the braces here.
Quoted 4 lines
drivers/clk/sirf/clk-atlas7.c new file +4
-0+ {"THCGUM", 0x0244, 3, 0x0320, 1, &leaf0_gate_lock}, + {"CVD", 0x04A0, 0, 0x032C, 0, &leaf1_gate_lock}, + {"TIMER", 0x04A0, 1, 0x032C, 1, &leaf1_gate_lock}, + {"PULSEC", 0x04A0, 2, 0x032C, 2, &leaf1_gate_lock},
[...]
drivers/clk/sirf/clk-atlas7.c line 1348
Quoted 6 lines
drivers/clk/sirf/clk-atlas7.c new file +6
-0+ +static int atlas7_reset_module (struct reset_controller_dev *rcdev, +unsigned long reset_idx) +{ +struct atlas7_reset_desc *reset = &atlas7_reset_unit[reset_idx]; +unsigned long flags =0 ;
Unnecessary initialization.
drivers/clk/sirf/clk-atlas7.c line 1351
Quoted 3 lines
drivers/clk/sirf/clk-atlas7.c new file +3
-0+ +if (reset_idx >= rcdev->nr_resets) +return -EINVAL ;
Does this actually happen? Seems like the reset framework should be checking this.
Quoted 22 lines
drivers/clk/sirf/clk-atlas7.c new file +22
-0+/* +* HW suggest unit reset sequence: +* assert sw reset (0) +* setting sw clk_en to if the clock was disabled before reset +* delay 16 clocks +* disable clock (sw clk_en = 0) +* de-assert reset (1) +* after this sequence, restore clock or not is decided by SW +*/ + +spin_lock_irqsave (reset->lock, flags); +/* clock enable or not */ +if (clkc_readl (reset->clk_ofs +8 ) & (1 << reset->clk_bit)) { +clkc_writel (1 << reset->rst_bit, reset->rst_ofs +4 ); +udelay (2 ); +clkc_writel (1 << reset->clk_bit, reset->clk_ofs +4 ); +clkc_writel (1 << reset->rst_bit, reset->rst_ofs); +/* restore clock enable */ +clkc_writel (1 << reset->clk_bit, reset->clk_ofs); + }else { +clkc_writel (1 << reset->rst_bit, reset->rst_ofs +4 ); +clkc_writel (1 << reset->clk_bit, reset->clk_ofs);
drivers/clk/sirf/clk-atlas7.c line 1383
Quoted 10 lines
drivers/clk/sirf/clk-atlas7.c new file +10
-0+udelay (2 ); +clkc_writel (1 << reset->clk_bit, reset->clk_ofs +4 ); +clkc_writel (1 << reset->rst_bit, reset->rst_ofs); + } +spin_unlock_irqrestore (reset->lock, flags); + +return 0 ; +} + +static struct reset_control_ops atlas7_rst_ops = {
const?
drivers/clk/sirf/clk-atlas7.c line 1399
Quoted 5 lines
drivers/clk/sirf/clk-atlas7.c new file +5
-0+ .reset = atlas7_reset_module, +}; + + +void __init atlas7_clk_init (struct device_node *np)
static
Quoted 8 lines
drivers/clk/sirf/clk-atlas7.c new file +8
-0+{ +struct clk *clk; +struct atlas7_div_init_data *div; +struct atlas7_mux_init_data *mux; +struct atlas7_unit_init_data *unit; +int i; +int ret; +
drivers/clk/sirf/clk-atlas7.c line 1417
Quoted 10 lines
drivers/clk/sirf/clk-atlas7.c new file +10
-0+ sirfsoc_clk_vbase =of_iomap (np,0 ); +if (!sirfsoc_clk_vbase) +panic ("unable to map clkc registers\n" ); + +of_node_put (np); +/* These are always available (32k osc xinw and 26MHz osc xin) */ +clk_register_fixed_rate (NULL ,"xinw" ,NULL , +CLK_IS_ROOT ,32768 ); +clk_register_fixed_rate (NULL ,"xin" ,NULL , +CLK_IS_ROOT ,26000000 );
Should these be in DT? I don't imagine they're part of the SoC?
Quoted 31 lines
drivers/clk/sirf/clk-atlas7.c new file +31
-0+ + +for (i =0 ; i <ARRAY_SIZE (divider_list); i++) { + div = ÷r_list[i]; + clk =clk_register_divider (NULL , div->div_name, + div->parent_name, div->divider_flags, sirfsoc_clk_vbase + div->div_offset, + div->shift, div->width,0 , div->lock); +BUG_ON (!clk); + clk =clk_register_gate (NULL , div->gate_name, div->div_name, + div->gate_flags, sirfsoc_clk_vbase + div->gate_offset, + div->gate_bit,0 , div->lock); +BUG_ON (!clk); + } +/* ignore selector status register check */ +for (i =0 ; i <ARRAY_SIZE (mux_list); i++) { + mux = &mux_list[i]; + clk =clk_register_mux (NULL , mux->mux_name, mux->parent_names, + mux->parent_num, mux->flags, + sirfsoc_clk_vbase + mux->mux_offset, + mux->shift, mux->width, + mux->mux_flags,NULL ); +BUG_ON (!clk); + } + +for (i =0 ; i <ARRAY_SIZE (unit_list); i++) { + unit = &unit_list[i]; + atlas7_clks[i] =atlas7_unit_clk_register (NULL , unit->unit_name, unit->parent_name, + unit->flags, unit->regofs, unit->bit, unit->lock); +BUG_ON (!atlas7_clks[i]); + } +
drivers/clk/sirf/clk-atlas7.c line 1649
Quoted 10 lines
drivers/clk/sirf/clk-atlas7.c new file +10
-0+ clk_data.clks = atlas7_clks; + clk_data.clk_num =ARRAY_SIZE (unit_list); + + ret =of_clk_add_provider (np, of_clk_src_onecell_get, &clk_data); +BUG_ON (ret); + + atlas7_rst_ctlr.of_node = np; + atlas7_rst_ctlr.nr_resets =ARRAY_SIZE (atlas7_reset_unit); +reset_controller_register (&atlas7_rst_ctlr); + arm_pm_restart = atlas7_restart;
arm_pm_restart can be hooked generically with register_restart_handler(). Can you please use that instead?
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v2 1/1 clk: sirf: add CSR atlas7 clk and reset support
2015-05-15 7:41 GMT+08:00 Stephen Boyd <[email protected]>:
Quoted 10 lines · Barry Song
Quoted 8 lines
From: Zhiwu Song <[email protected]>
the hardware node includes both clock and reset support, so it is named as "car".
this patch implements Flexibleclocks(mux, divider, gate), Selectableclock(mux, divider, gate), root clock(gate),leaf clock(gate), others. it also implements the reset controller functionality.
Quoted 6 lines
Quoted 3 lines
Signed-off-by Zhiwu Song <[email protected]>Signed-off-by Barry Song <[email protected]>---
Sorry for late review. I wasn't Cc'ed on the patch.
Stephen, thanks for very careful and high-quality review. i realized i missed you after i sent this one. sorry.
Quoted 11 lines
drivers/clk/sirf/clk-atlas7.c line 1261
Quoted 2 lines
drivers/clk/sirf/clk-atlas7.c new file +2
-0+static struct atlas7_reset_desc atlas7_reset_unit[] = { + {"PWM" ,0x0244 ,0 ,0x0320 ,0 , &leaf0_gate_lock},/*0-5*/ Style nitpick: Please put some space around the braces here.
Quoted 4 lines
drivers/clk/sirf/clk-atlas7.c new file +4
-0+ {"THCGUM", 0x0244, 3, 0x0320, 1, &leaf0_gate_lock}, + {"CVD", 0x04A0, 0, 0x032C, 0, &leaf1_gate_lock}, + {"TIMER", 0x04A0, 1, 0x032C, 1, &leaf1_gate_lock}, + {"PULSEC", 0x04A0, 2, 0x032C, 2, &leaf1_gate_lock},[...]
Quoted 1 line
+
all other issues have been cleared in v3 i sent minutes ago. for this one, where do you want the spaces added?
-barry
PATCH v2 1/1 clk: sirf: add CSR atlas7 clk and reset support
Quoted 9 lines · Barry Song
Quoted 9 lines
drivers/clk/sirf/clk-atlas7.c line 1261
Quoted 3 lines
drivers/clk/sirf/clk-atlas7.c new file +2
-0+static struct atlas7_reset_desc atlas7_reset_unit[] = { + {"PWM" ,0x0244 ,0 ,0x0320 ,0 , &leaf0_gate_lock},/*0-5*/ Style nitpick: Please put some space around the braces here.
Quoted 4 lines
drivers/clk/sirf/clk-atlas7.c new file +4
-0+ {"THCGUM", 0x0244, 3, 0x0320, 1, &leaf0_gate_lock}, + {"CVD", 0x04A0, 0, 0x032C, 0, &leaf1_gate_lock}, + {"TIMER", 0x04A0, 1, 0x032C, 1, &leaf1_gate_lock}, + {"PULSEC", 0x04A0, 2, 0x032C, 2, &leaf1_gate_lock},
Quoted 6 lines
Quoted 2 lines
[...]
Quoted 1 line
+
all other issues have been cleared in v3 i sent minutes ago. for this one, where do you want the spaces added?
I'd prefer it look like:
{ "THCGUM", 0x0244, ..., &leaf0_gate_lock },
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project