diff for duplicates of <20130802233009.6450.21915@quantum> diff --git a/a/1.txt b/N1/1.txt index 9861d20..1e237e5 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,18 +1,14 @@ Quoting Gerhard Sittig (2013-07-23 06:14:06) > [ summary: "shared gate" support desirable? approach acceptable? ] -> = - +> > On Mon, Jul 22, 2013 at 14:14 +0200, Gerhard Sittig wrote: -> > = - +> > > > this change implements a clock driver for the MPC512x PowerPC platform > > which follows the COMMON_CLK approach and uses common clock drivers > > shared with other platforms -> > = - +> > > > [ ... ] -> > = - +> > > > some of the clock items get pre-enabled in the clock driver to not have > > them automatically disabled by the underlying clock subsystem because of > > their being unused -- this approach is desirable because @@ -21,8 +17,7 @@ Quoting Gerhard Sittig (2013-07-23 06:14:06) > > infrastructure, while more appropriate support for specific hardware > > constraints isn't available yet (remaining changes are strictly > > internal to the clock driver and won't affect peripheral drivers) -> = - +> > This remark was related to the CAN clocks of the MPC512x SoC. Gerhard, @@ -38,15 +33,13 @@ correct? Regards, Mike -> = - +> > The clock subtrees which are involved in generating CAN bitrates > include one path from the XTAL to an internal MCLK (this is part > of the CCF support for the platform), and another path from the > MCLK or yet another IP bus clock to the actual bitrate on the > wire (this is taken care of within the mscan(4) driver). -> = - +> > The MCLK generation for CAN is documented in the MPC5121e > Reference Manual, chapter 5, section 5.2.5 "MSCAN Clock > Generation". SYS, REF (both internal), PSC_MCLK_IN, and SPDIF_TX @@ -54,14 +47,12 @@ Mike > muxed with IP. The result is fed into the MSCAN component and > gets muxed with IP again (can't tell why, maybe for backwards > compatibility). -> = - +> > In parallel to this MCLK block there is SCCR2[25], the "BDLC and > MSCAN clock enable", documented in section 5.3.1.3 "System Clock > Control Register 2". So there is a gate that "somehow needs to > get setup" yet isn't part of the visible MCLK chain. -> = - +> > The series up to and including v3 approaches the problem by > - adding a gate after the second MCLK mux, which gets exported > for client lookups and is the MCLK input for the mscan(4) @@ -73,12 +64,10 @@ Mike > thus avoid having the clock disabled from the common > infrastructure, because disabling one of these clocks had > closed the shared gate and thus had broken all other clock uses -> = - +> > > clkdev registration provides "alias names" for few clock items > > [ ... ] -> > = - +> > > [ ... ] > > + > > +/* setup the MCLK clock subtree of an individual PSC/MSCAN/SPDIF */ @@ -86,43 +75,41 @@ Mike > > +{ > > + size_t clks_idx_pub, clks_idx_int; > > + u32 __iomem *mccr_reg; /* MCLK control register (mux, en, div) */ -> > + u32 __iomem *sccr_reg; /* system clock control register (enable)= - */ +> > + u32 __iomem *sccr_reg; /* system clock control register (enable) */ > > + int sccr_bit; > > + int div; > > + > > + /* derive a few parameters from the component type and index */ > > + switch (entry->type) { > > + case MCLK_TYPE_PSC: -> > + clks_idx_pub =3D MPC512x_CLK_PSC0_MCLK + entry->comp_idx; -> > + clks_idx_int =3D MPC512x_CLK_MCLKS_FIRST +> > + clks_idx_pub = MPC512x_CLK_PSC0_MCLK + entry->comp_idx; +> > + clks_idx_int = MPC512x_CLK_MCLKS_FIRST > > + + (entry->comp_idx) * MCLK_MAX_IDX; -> > + mccr_reg =3D &clkregs->psc_ccr[entry->comp_idx]; +> > + mccr_reg = &clkregs->psc_ccr[entry->comp_idx]; > > + break; > > + case MCLK_TYPE_MSCAN: -> > + clks_idx_pub =3D MPC512x_CLK_MSCAN0_MCLK + entry->comp_id= -x; -> > + clks_idx_int =3D MPC512x_CLK_MCLKS_FIRST +> > + clks_idx_pub = MPC512x_CLK_MSCAN0_MCLK + entry->comp_idx; +> > + clks_idx_int = MPC512x_CLK_MCLKS_FIRST > > + + (NR_PSCS + entry->comp_idx) * MCLK_MAX_IDX; -> > + mccr_reg =3D &clkregs->mscan_ccr[entry->comp_idx]; +> > + mccr_reg = &clkregs->mscan_ccr[entry->comp_idx]; > > + break; > > + case MCLK_TYPE_SPDIF: -> > + clks_idx_pub =3D MPC512x_CLK_SPDIF_MCLK; -> > + clks_idx_int =3D MPC512x_CLK_MCLKS_FIRST +> > + clks_idx_pub = MPC512x_CLK_SPDIF_MCLK; +> > + clks_idx_int = MPC512x_CLK_MCLKS_FIRST > > + + (NR_PSCS + NR_MSCANS) * MCLK_MAX_IDX; -> > + mccr_reg =3D &clkregs->spccr; +> > + mccr_reg = &clkregs->spccr; > > + break; > > + default: > > + return; > > + } -> > + if (entry->bit_sccr1 >=3D 0) { -> > + sccr_reg =3D &clkregs->sccr1; -> > + sccr_bit =3D entry->bit_sccr1; -> > + } else if (entry->bit_sccr2 >=3D 0) { -> > + sccr_reg =3D &clkregs->sccr2; -> > + sccr_bit =3D entry->bit_sccr2; +> > + if (entry->bit_sccr1 >= 0) { +> > + sccr_reg = &clkregs->sccr1; +> > + sccr_bit = entry->bit_sccr1; +> > + } else if (entry->bit_sccr2 >= 0) { +> > + sccr_reg = &clkregs->sccr2; +> > + sccr_bit = entry->bit_sccr2; > > + } else { -> > + sccr_reg =3D NULL; +> > + sccr_reg = NULL; > > + } > > + > > + /* @@ -131,8 +118,8 @@ x; > > + * during setup (that's a documented hardware requirement) > > + * > > + * the PPC_CLOCK implementation might even have violated the -> > + * "MCLK <=3D IPS" constraint, the fixed divider value of 1 -> > + * results in a divider of 2 and thus MCLK =3D SYS/2 which equals +> > + * "MCLK <= IPS" constraint, the fixed divider value of 1 +> > + * results in a divider of 2 and thus MCLK = SYS/2 which equals > > + * CSB which is greater than IPS; the serial port setup may have > > + * adjusted the divider which the clock setup might have left in > > + * an undesirable state @@ -143,8 +130,8 @@ x; > > + * - MCLK 0 enabled > > + * - MCLK 1 from MCLK DIV > > + */ -> > + div =3D clk_get_rate(clks[MPC512x_CLK_SYS]); -> > + div /=3D clk_get_rate(clks[MPC512x_CLK_IPS]); +> > + div = clk_get_rate(clks[MPC512x_CLK_SYS]); +> > + div /= clk_get_rate(clks[MPC512x_CLK_IPS]); > > + out_be32(mccr_reg, (0 << 16)); > > + out_be32(mccr_reg, (0 << 16) | ((div - 1) << 17)); > > + out_be32(mccr_reg, (1 << 16) | ((div - 1) << 17)); @@ -167,36 +154,34 @@ x; > > + * clock items even for those muxers which actually are NOPs > > + * (those with two inputs of which one is reserved) > > + */ -> > + clks[clks_idx_int + MCLK_IDX_MUX0] =3D mpc512x_clk_muxed( +> > + clks[clks_idx_int + MCLK_IDX_MUX0] = mpc512x_clk_muxed( > > + entry->name_mux0, -> > + &parent_names_mux0[0], ARRAY_SIZE(parent_names_mu= -x0), +> > + &parent_names_mux0[0], ARRAY_SIZE(parent_names_mux0), > > + mccr_reg, 14, 2); -> > + clks[clks_idx_int + MCLK_IDX_EN0] =3D mpc512x_clk_gated( +> > + clks[clks_idx_int + MCLK_IDX_EN0] = mpc512x_clk_gated( > > + entry->name_en0, entry->name_mux0, > > + mccr_reg, 16); -> > + clks[clks_idx_int + MCLK_IDX_DIV0] =3D mpc512x_clk_divider( +> > + clks[clks_idx_int + MCLK_IDX_DIV0] = mpc512x_clk_divider( > > + entry->name_div0, > > + entry->name_en0, CLK_SET_RATE_GATE, > > + mccr_reg, 17, 15, 0); > > + if (entry->has_mclk1) { -> > + clks[clks_idx_int + MCLK_IDX_MUX1] =3D mpc512x_clk_muxed( +> > + clks[clks_idx_int + MCLK_IDX_MUX1] = mpc512x_clk_muxed( > > + entry->name_mux1, > > + &entry->parent_names_mux1[0], > > + ARRAY_SIZE(entry->parent_names_mux1), > > + mccr_reg, 7, 1); > > + } else { -> > + clks[clks_idx_int + MCLK_IDX_MUX1] =3D mpc512x_clk_factor( -> > + entry->name_mux1, entry->parent_names_mux= -1[0], +> > + clks[clks_idx_int + MCLK_IDX_MUX1] = mpc512x_clk_factor( +> > + entry->name_mux1, entry->parent_names_mux1[0], > > + 1, 1); > > + } > > + if (sccr_reg) { -> > + clks[clks_idx_pub] =3D mpc512x_clk_gated( +> > + clks[clks_idx_pub] = mpc512x_clk_gated( > > + entry->name_mclk, > > + entry->name_mux1, sccr_reg, sccr_bit); > > + } else { -> > + clks[clks_idx_pub] =3D mpc512x_clk_factor( +> > + clks[clks_idx_pub] = mpc512x_clk_factor( > > + entry->name_mclk, > > + entry->name_mux1, 1, 1); > > + } @@ -213,30 +198,24 @@ x0), > > + clk_register_clkdev(clks[clks_idx_pub], entry->name_mclk, NULL); > > +} > > [ ... ] -> = - +> > This was the routine which sets up _one_ MCLK block, note the > assignment at the routine's end to the "published" clock item > that's the gate's output after the second mux stage. -> = - +> > > [ ... ] -> > + clks[MPC512x_CLK_I2C] =3D mpc512x_clk_gated("i2c", "ips", +> > + clks[MPC512x_CLK_I2C] = mpc512x_clk_gated("i2c", "ips", > > + &clkregs->sccr2, 26); -> > + mpc512x_clk_setup_mclks(mclk_mscan_data, ARRAY_SIZE(mclk_mscan_da= -ta)); -> > + clks[MPC512x_CLK_SDHC] =3D mpc512x_clk_gated("sdhc", "sdhc-ug", +> > + mpc512x_clk_setup_mclks(mclk_mscan_data, ARRAY_SIZE(mclk_mscan_data)); +> > + clks[MPC512x_CLK_SDHC] = mpc512x_clk_gated("sdhc", "sdhc-ug", > > + &clkregs->sccr2, 24); -> > + mpc512x_clk_setup_mclks(mclk_spdif_data, ARRAY_SIZE(mclk_spdif_da= -ta)); +> > + mpc512x_clk_setup_mclks(mclk_spdif_data, ARRAY_SIZE(mclk_spdif_data)); > > [ ... ] -> = - +> > This is the invocation of the routine which sets up four MCLK > blocks for the MSCAN components, while all of them refer to bit > 25 of SCCR2. -> = - +> > > [ ... ] > > + > > + /* enable some of the clocks here unconditionally because ... */ @@ -251,8 +230,7 @@ ta)); > > + /* some are required yet no dependencies were declared */ > > + clk_prepare_enable(clks[MPC512x_CLK_PSC_FIFO]); > > + /* some are not yet acquired by their respective drivers */ -> > + clk_prepare_enable(clks[MPC512x_CLK_PSC3_MCLK]);/* serial console= - */ +> > + clk_prepare_enable(clks[MPC512x_CLK_PSC3_MCLK]);/* serial console */ > > + clk_prepare_enable(clks[MPC512x_CLK_FEC]); /* network, NFS */ > > + clk_prepare_enable(clks[MPC512x_CLK_DIU]); /* display */ > > + clk_prepare_enable(clks[MPC512x_CLK_I2C]); @@ -272,52 +250,42 @@ ta)); > > + clk_prepare_enable(clks[MPC512x_CLK_MSCAN2_MCLK]); > > + clk_prepare_enable(clks[MPC512x_CLK_MSCAN3_MCLK]); > > +} -> = - +> > This is the pre-enable workaround for the MSCAN0 to MSCAN3 clock > items. -> = - +> > The above approach does work in that it introduces complete > support for common clock on the MPC512x platform, with the CAN > component being operational, and the clock driver using shared > logic across platforms. -> = - +> > The remaining issue is that regardless of whether CAN is used, > the (chip internal) clock is enabled. This may not be a problem > when bitrates aren't generated and the wire isn't driven. -> = - -> = - +> +> > The question now is how to correctly support the situation where > a gate is shared between subtrees yet isn't really part of any > path within the subtrees. I really cannot find a single spot > where to introduce the gate such that it's not duplicated. -> = - +> > The appropriate solution would not be to pre-enable those clocks, > but to either introduce another gate clock type which supports a > shared reference, or to add support for the shared reference to > the existing gate code. -> = - -> = - +> +> > I'd rather not duplicate most or all of the code of clk-gate.c, > instead I looked into how to add "shared gate" support to the > existing driver. -> = - +> > My question is whether the approach is acceptable. It adds > minimal overhead and shall be OK for the enable/disable path from > a technical POV. And it doesn't feel like too much of a stretch. > But there may be non-technical reasons to reject the approach. > I'd like to learn whether to follow that path before preparing > another version of the patch series. -> = - +> > The diffs were taken with the '-w -b' options to demonstrate > their essence and not drown it in whitespace changes. The > implementation assumes that the caller which registers the gate @@ -325,8 +293,7 @@ ta)); > the lock. And that all gates with a "shared use counter" use the > same lock (which is satisfied as they all get registered from the > same spot in the platform's clock driver). -> = - +> > The CLK_IGNORE_UNUSED flag addresses a different problem. The > SoC has four MSCAN components, while two of them are enabled in > the device tree (the other two are present but disabled). So @@ -338,63 +305,52 @@ ta)); > the common gate logic would implement a separate disable_unused() > routine, but I guess this isn't necessary and the use of the flag > is appropriate. -> = - +> > That the example use creates a field for just one counter is to > better demonstrate the use and potential extension as need > arises. Reducing this to a mere integer variable would be a > micro optimization. -> = - -> = - +> +> > The extension of the existing clk_gate implementation: -> = - +> > --- a/drivers/clk/clk-gate.c > +++ b/drivers/clk/clk-gate.c -> @@ -46,6 +46,7 @@ static void clk_gate_endisable(struct clk_hw *hw, int e= -nable) -> struct clk_gate *gate =3D to_clk_gate(hw); -> int set =3D gate->flags & CLK_GATE_SET_TO_DISABLE ? 1 : 0; -> unsigned long flags =3D 0; +> @@ -46,6 +46,7 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable) +> struct clk_gate *gate = to_clk_gate(hw); +> int set = gate->flags & CLK_GATE_SET_TO_DISABLE ? 1 : 0; +> unsigned long flags = 0; > + int need_reg_access; > u32 reg; -> = - -> set ^=3D enable; -> @@ -53,6 +54,20 @@ static void clk_gate_endisable(struct clk_hw *hw, int = -enable) +> +> set ^= enable; +> @@ -53,6 +54,20 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable) > if (gate->lock) > spin_lock_irqsave(gate->lock, flags); -> = - +> > + /* > + * if a "shared use counter" was specified, keep track of enable > + * and disable calls and only access hardware registers upon the > + * very first enable or very last disable call > + */ > + if (!gate->share_count) { -> + need_reg_access =3D 1; +> + need_reg_access = 1; > + } else if (enable) { -> + need_reg_access =3D (*gate->share_count)++ =3D=3D 0; +> + need_reg_access = (*gate->share_count)++ == 0; > + } else { -> + need_reg_access =3D --(*gate->share_count) =3D=3D 0; +> + need_reg_access = --(*gate->share_count) == 0; > + } > + > + if (need_reg_access) { > if (gate->flags & CLK_GATE_HIWORD_MASK) { -> reg =3D BIT(gate->bit_idx + 16); +> reg = BIT(gate->bit_idx + 16); > if (set) -> @@ -67,6 +82,7 @@ static void clk_gate_endisable(struct clk_hw *hw, int e= -nable) +> @@ -67,6 +82,7 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable) > } -> = - +> > clk_writel(reg, gate->reg); > + } -> = - +> > if (gate->lock) > spin_unlock_irqrestore(gate->lock, flags); > @@ -118,10 +134,11 @@ EXPORT_SYMBOL_GPL(clk_gate_ops); @@ -402,8 +358,7 @@ nable) > * @lock: shared register lock for this clock > */ > -struct clk *clk_register_gate(struct device *dev, const char *name, -> +struct clk *clk_register_gate_shared(struct device *dev, const char *nam= -e, +> +struct clk *clk_register_gate_shared(struct device *dev, const char *name, > const char *parent_name, unsigned long flags, > void __iomem *reg, u8 bit_idx, > - u8 clk_gate_flags, spinlock_t *lock) @@ -412,20 +367,16 @@ e, > { > struct clk_gate *gate; > struct clk *clk; -> @@ -152,6 +169,7 @@ struct clk *clk_register_gate(struct device *dev, con= -st char *name, -> gate->bit_idx =3D bit_idx; -> gate->flags =3D clk_gate_flags; -> gate->lock =3D lock; -> + gate->share_count =3D share_count; -> gate->hw.init =3D &init; -> = - -> clk =3D clk_register(dev, &gate->hw); -> @@ -161,3 +179,14 @@ struct clk *clk_register_gate(struct device *dev, co= -nst char *name, -> = - +> @@ -152,6 +169,7 @@ struct clk *clk_register_gate(struct device *dev, const char *name, +> gate->bit_idx = bit_idx; +> gate->flags = clk_gate_flags; +> gate->lock = lock; +> + gate->share_count = share_count; +> gate->hw.init = &init; +> +> clk = clk_register(dev, &gate->hw); +> @@ -161,3 +179,14 @@ struct clk *clk_register_gate(struct device *dev, const char *name, +> > return clk; > } > + @@ -447,38 +398,30 @@ nst char *name, > spinlock_t *lock; > + int *share_count; > }; -> = - +> > #define CLK_GATE_SET_TO_DISABLE BIT(0) -> @@ -232,6 +233,11 @@ struct clk *clk_register_gate(struct device *dev, co= -nst char *name, +> @@ -232,6 +233,11 @@ struct clk *clk_register_gate(struct device *dev, const char *name, > const char *parent_name, unsigned long flags, > void __iomem *reg, u8 bit_idx, > u8 clk_gate_flags, spinlock_t *lock); -> +struct clk *clk_register_gate_shared(struct device *dev, const char *nam= -e, +> +struct clk *clk_register_gate_shared(struct device *dev, const char *name, > + const char *parent_name, unsigned long flags, > + void __iomem *reg, u8 bit_idx, > + u8 clk_gate_flags, spinlock_t *lock, > + int *share_count); -> = - +> > struct clk_div_table { > unsigned int val; -> = - -> = - +> +> > How to use these shared gates: -> = - +> > --- a/arch/powerpc/platforms/512x/clock-commonclk.c > +++ b/arch/powerpc/platforms/512x/clock-commonclk.c > @@ -123,6 +123,39 @@ static inline struct clk *mpc512x_clk_gated( > reg, pos, 0, &clklock); > } -> = - +> > +enum mpc512x_clk_shared_gate_id_t { > + MPC512x_CLK_SHARED_GATE_MSCAN, > + MPC512x_CLK_SHARED_GATE_MAX, @@ -505,46 +448,40 @@ e, > +{ > + int clkflags; > + -> + clkflags =3D CLK_SET_RATE_PARENT; -> + clkflags |=3D CLK_IGNORE_UNUSED; +> + clkflags = CLK_SET_RATE_PARENT; +> + clkflags |= CLK_IGNORE_UNUSED; > + return clk_register_gate_shared(NULL, name, parent_name, clkflags, > + reg, pos, 0, &clklock, -> + &mpc512x_clk_gate_counters[share_= -id]); +> + &mpc512x_clk_gate_counters[share_id]); > +} > + > static inline struct clk *mpc512x_clk_muxed(const char *name, > const char **parent_names, int parent_count, > u32 __iomem *reg, u8 pos, u8 len) -> @@ -520,9 +553,16 @@ static void mpc512x_clk_setup_mclk(struct mclk_setup= -_data *entry) +> @@ -520,9 +553,16 @@ static void mpc512x_clk_setup_mclk(struct mclk_setup_data *entry) > 1, 1); > } > if (sccr_reg) { -> + if (entry->type =3D=3D MCLK_TYPE_MSCAN) { -> + clks[clks_idx_pub] =3D mpc512x_clk_gated_shared( +> + if (entry->type == MCLK_TYPE_MSCAN) { +> + clks[clks_idx_pub] = mpc512x_clk_gated_shared( > + entry->name_mclk, -> + entry->name_mux1, sccr_reg, sccr_= -bit, +> + entry->name_mux1, sccr_reg, sccr_bit, > + MPC512x_CLK_SHARED_GATE_MSCAN); > + } else { -> clks[clks_idx_pub] =3D mpc512x_clk_gated( +> clks[clks_idx_pub] = mpc512x_clk_gated( > entry->name_mclk, -> entry->name_mux1, sccr_reg, sccr_= -bit); +> entry->name_mux1, sccr_reg, sccr_bit); > + } > } else { -> clks[clks_idx_pub] =3D mpc512x_clk_factor( +> clks[clks_idx_pub] = mpc512x_clk_factor( > entry->name_mclk, -> = - +> > Local tests have shown that the extension solves the problem of > how to satisfy the SoC's constraints on the MPC512x platform. > The MSCAN clocks no longer need to get pre-enabled, instead they > get setup and enabled only as the mscan(4) driver probes devices > according to how it was instructed (device tree nodes). -> = - +> > What do you think? Is the "shared gate" support in the common > logic appropriate? I'd rather not duplicate all of this code > just to introduce the specific gate I need, while most of the @@ -552,16 +489,12 @@ bit); > desire isn't to override the gate's operations, but to wrap them > and to consult a counter in addition, while the register access > still applies. -> = - -> = - -> = - +> +> +> > virtually yours > Gerhard Sittig -> -- = - +> -- > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany -> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de +> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de diff --git a/a/content_digest b/N1/content_digest index a056199..215e2e1 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -2,40 +2,23 @@ "ref\01374495298-22019-1-git-send-email-gsi@denx.de\0" "ref\01374495298-22019-18-git-send-email-gsi@denx.de\0" "ref\020130723131406.GI19071@book.gsilab.sittig.org\0" - "From\0Mike Turquette <mturquette@linaro.org>\0" - "Subject\0Re: [PATCH v3 17/31] clk: mpc512x: introduce COMMON_CLK for MPC512x\0" + "From\0mturquette@linaro.org (Mike Turquette)\0" + "Subject\0[PATCH v3 17/31] clk: mpc512x: introduce COMMON_CLK for MPC512x\0" "Date\0Fri, 02 Aug 2013 16:30:09 -0700\0" - "To\0Gerhard Sittig <gsi@denx.de>" - linuxppc-dev@lists.ozlabs.org - Anatolij Gustschin <agust@denx.de> - linux-arm-kernel@lists.infradead.org - " devicetree@vger.kernel.org\0" - "Cc\0Detlev Zundel <dzu@denx.de>" - Wolfram Sang <wsa@the-dreams.de> - Greg Kroah-Hartman <gregkh@linuxfoundation.org> - Rob Herring <rob.herring@calxeda.com> - Mark Brown <broonie@kernel.org> - Marc Kleine-Budde <mkl@pengutronix.de> - David Woodhouse <dwmw2@infradead.org> - Wolfgang Grandegger <wg@grandegger.com> - " Mauro Carvalho Chehab <m.chehab@samsung.com>\0" + "To\0linux-arm-kernel@lists.infradead.org\0" "\00:1\0" "b\0" "Quoting Gerhard Sittig (2013-07-23 06:14:06)\n" "> [ summary: \"shared gate\" support desirable? approach acceptable? ]\n" - "> =\n" - "\n" + "> \n" "> On Mon, Jul 22, 2013 at 14:14 +0200, Gerhard Sittig wrote:\n" - "> > =\n" - "\n" + "> > \n" "> > this change implements a clock driver for the MPC512x PowerPC platform\n" "> > which follows the COMMON_CLK approach and uses common clock drivers\n" "> > shared with other platforms\n" - "> > =\n" - "\n" + "> > \n" "> > [ ... ]\n" - "> > =\n" - "\n" + "> > \n" "> > some of the clock items get pre-enabled in the clock driver to not have\n" "> > them automatically disabled by the underlying clock subsystem because of\n" "> > their being unused -- this approach is desirable because\n" @@ -44,8 +27,7 @@ "> > infrastructure, while more appropriate support for specific hardware\n" "> > constraints isn't available yet (remaining changes are strictly\n" "> > internal to the clock driver and won't affect peripheral drivers)\n" - "> =\n" - "\n" + "> \n" "> This remark was related to the CAN clocks of the MPC512x SoC.\n" "\n" "Gerhard,\n" @@ -61,15 +43,13 @@ "Regards,\n" "Mike\n" "\n" - "> =\n" - "\n" + "> \n" "> The clock subtrees which are involved in generating CAN bitrates\n" "> include one path from the XTAL to an internal MCLK (this is part\n" "> of the CCF support for the platform), and another path from the\n" "> MCLK or yet another IP bus clock to the actual bitrate on the\n" "> wire (this is taken care of within the mscan(4) driver).\n" - "> =\n" - "\n" + "> \n" "> The MCLK generation for CAN is documented in the MPC5121e\n" "> Reference Manual, chapter 5, section 5.2.5 \"MSCAN Clock\n" "> Generation\". SYS, REF (both internal), PSC_MCLK_IN, and SPDIF_TX\n" @@ -77,14 +57,12 @@ "> muxed with IP. The result is fed into the MSCAN component and\n" "> gets muxed with IP again (can't tell why, maybe for backwards\n" "> compatibility).\n" - "> =\n" - "\n" + "> \n" "> In parallel to this MCLK block there is SCCR2[25], the \"BDLC and\n" "> MSCAN clock enable\", documented in section 5.3.1.3 \"System Clock\n" "> Control Register 2\". So there is a gate that \"somehow needs to\n" "> get setup\" yet isn't part of the visible MCLK chain.\n" - "> =\n" - "\n" + "> \n" "> The series up to and including v3 approaches the problem by\n" "> - adding a gate after the second MCLK mux, which gets exported\n" "> for client lookups and is the MCLK input for the mscan(4)\n" @@ -96,12 +74,10 @@ "> thus avoid having the clock disabled from the common\n" "> infrastructure, because disabling one of these clocks had\n" "> closed the shared gate and thus had broken all other clock uses\n" - "> =\n" - "\n" + "> \n" "> > clkdev registration provides \"alias names\" for few clock items\n" "> > [ ... ]\n" - "> > =\n" - "\n" + "> > \n" "> [ ... ]\n" "> > +\n" "> > +/* setup the MCLK clock subtree of an individual PSC/MSCAN/SPDIF */\n" @@ -109,43 +85,41 @@ "> > +{\n" "> > + size_t clks_idx_pub, clks_idx_int;\n" "> > + u32 __iomem *mccr_reg; /* MCLK control register (mux, en, div) */\n" - "> > + u32 __iomem *sccr_reg; /* system clock control register (enable)=\n" - " */\n" + "> > + u32 __iomem *sccr_reg; /* system clock control register (enable) */\n" "> > + int sccr_bit;\n" "> > + int div;\n" "> > +\n" "> > + /* derive a few parameters from the component type and index */\n" "> > + switch (entry->type) {\n" "> > + case MCLK_TYPE_PSC:\n" - "> > + clks_idx_pub =3D MPC512x_CLK_PSC0_MCLK + entry->comp_idx;\n" - "> > + clks_idx_int =3D MPC512x_CLK_MCLKS_FIRST\n" + "> > + clks_idx_pub = MPC512x_CLK_PSC0_MCLK + entry->comp_idx;\n" + "> > + clks_idx_int = MPC512x_CLK_MCLKS_FIRST\n" "> > + + (entry->comp_idx) * MCLK_MAX_IDX;\n" - "> > + mccr_reg =3D &clkregs->psc_ccr[entry->comp_idx];\n" + "> > + mccr_reg = &clkregs->psc_ccr[entry->comp_idx];\n" "> > + break;\n" "> > + case MCLK_TYPE_MSCAN:\n" - "> > + clks_idx_pub =3D MPC512x_CLK_MSCAN0_MCLK + entry->comp_id=\n" - "x;\n" - "> > + clks_idx_int =3D MPC512x_CLK_MCLKS_FIRST\n" + "> > + clks_idx_pub = MPC512x_CLK_MSCAN0_MCLK + entry->comp_idx;\n" + "> > + clks_idx_int = MPC512x_CLK_MCLKS_FIRST\n" "> > + + (NR_PSCS + entry->comp_idx) * MCLK_MAX_IDX;\n" - "> > + mccr_reg =3D &clkregs->mscan_ccr[entry->comp_idx];\n" + "> > + mccr_reg = &clkregs->mscan_ccr[entry->comp_idx];\n" "> > + break;\n" "> > + case MCLK_TYPE_SPDIF:\n" - "> > + clks_idx_pub =3D MPC512x_CLK_SPDIF_MCLK;\n" - "> > + clks_idx_int =3D MPC512x_CLK_MCLKS_FIRST\n" + "> > + clks_idx_pub = MPC512x_CLK_SPDIF_MCLK;\n" + "> > + clks_idx_int = MPC512x_CLK_MCLKS_FIRST\n" "> > + + (NR_PSCS + NR_MSCANS) * MCLK_MAX_IDX;\n" - "> > + mccr_reg =3D &clkregs->spccr;\n" + "> > + mccr_reg = &clkregs->spccr;\n" "> > + break;\n" "> > + default:\n" "> > + return;\n" "> > + }\n" - "> > + if (entry->bit_sccr1 >=3D 0) {\n" - "> > + sccr_reg =3D &clkregs->sccr1;\n" - "> > + sccr_bit =3D entry->bit_sccr1;\n" - "> > + } else if (entry->bit_sccr2 >=3D 0) {\n" - "> > + sccr_reg =3D &clkregs->sccr2;\n" - "> > + sccr_bit =3D entry->bit_sccr2;\n" + "> > + if (entry->bit_sccr1 >= 0) {\n" + "> > + sccr_reg = &clkregs->sccr1;\n" + "> > + sccr_bit = entry->bit_sccr1;\n" + "> > + } else if (entry->bit_sccr2 >= 0) {\n" + "> > + sccr_reg = &clkregs->sccr2;\n" + "> > + sccr_bit = entry->bit_sccr2;\n" "> > + } else {\n" - "> > + sccr_reg =3D NULL;\n" + "> > + sccr_reg = NULL;\n" "> > + }\n" "> > +\n" "> > + /*\n" @@ -154,8 +128,8 @@ "> > + * during setup (that's a documented hardware requirement)\n" "> > + *\n" "> > + * the PPC_CLOCK implementation might even have violated the\n" - "> > + * \"MCLK <=3D IPS\" constraint, the fixed divider value of 1\n" - "> > + * results in a divider of 2 and thus MCLK =3D SYS/2 which equals\n" + "> > + * \"MCLK <= IPS\" constraint, the fixed divider value of 1\n" + "> > + * results in a divider of 2 and thus MCLK = SYS/2 which equals\n" "> > + * CSB which is greater than IPS; the serial port setup may have\n" "> > + * adjusted the divider which the clock setup might have left in\n" "> > + * an undesirable state\n" @@ -166,8 +140,8 @@ "> > + * - MCLK 0 enabled\n" "> > + * - MCLK 1 from MCLK DIV\n" "> > + */\n" - "> > + div =3D clk_get_rate(clks[MPC512x_CLK_SYS]);\n" - "> > + div /=3D clk_get_rate(clks[MPC512x_CLK_IPS]);\n" + "> > + div = clk_get_rate(clks[MPC512x_CLK_SYS]);\n" + "> > + div /= clk_get_rate(clks[MPC512x_CLK_IPS]);\n" "> > + out_be32(mccr_reg, (0 << 16));\n" "> > + out_be32(mccr_reg, (0 << 16) | ((div - 1) << 17));\n" "> > + out_be32(mccr_reg, (1 << 16) | ((div - 1) << 17));\n" @@ -190,36 +164,34 @@ "> > + * clock items even for those muxers which actually are NOPs\n" "> > + * (those with two inputs of which one is reserved)\n" "> > + */\n" - "> > + clks[clks_idx_int + MCLK_IDX_MUX0] =3D mpc512x_clk_muxed(\n" + "> > + clks[clks_idx_int + MCLK_IDX_MUX0] = mpc512x_clk_muxed(\n" "> > + entry->name_mux0,\n" - "> > + &parent_names_mux0[0], ARRAY_SIZE(parent_names_mu=\n" - "x0),\n" + "> > + &parent_names_mux0[0], ARRAY_SIZE(parent_names_mux0),\n" "> > + mccr_reg, 14, 2);\n" - "> > + clks[clks_idx_int + MCLK_IDX_EN0] =3D mpc512x_clk_gated(\n" + "> > + clks[clks_idx_int + MCLK_IDX_EN0] = mpc512x_clk_gated(\n" "> > + entry->name_en0, entry->name_mux0,\n" "> > + mccr_reg, 16);\n" - "> > + clks[clks_idx_int + MCLK_IDX_DIV0] =3D mpc512x_clk_divider(\n" + "> > + clks[clks_idx_int + MCLK_IDX_DIV0] = mpc512x_clk_divider(\n" "> > + entry->name_div0,\n" "> > + entry->name_en0, CLK_SET_RATE_GATE,\n" "> > + mccr_reg, 17, 15, 0);\n" "> > + if (entry->has_mclk1) {\n" - "> > + clks[clks_idx_int + MCLK_IDX_MUX1] =3D mpc512x_clk_muxed(\n" + "> > + clks[clks_idx_int + MCLK_IDX_MUX1] = mpc512x_clk_muxed(\n" "> > + entry->name_mux1,\n" "> > + &entry->parent_names_mux1[0],\n" "> > + ARRAY_SIZE(entry->parent_names_mux1),\n" "> > + mccr_reg, 7, 1);\n" "> > + } else {\n" - "> > + clks[clks_idx_int + MCLK_IDX_MUX1] =3D mpc512x_clk_factor(\n" - "> > + entry->name_mux1, entry->parent_names_mux=\n" - "1[0],\n" + "> > + clks[clks_idx_int + MCLK_IDX_MUX1] = mpc512x_clk_factor(\n" + "> > + entry->name_mux1, entry->parent_names_mux1[0],\n" "> > + 1, 1);\n" "> > + }\n" "> > + if (sccr_reg) {\n" - "> > + clks[clks_idx_pub] =3D mpc512x_clk_gated(\n" + "> > + clks[clks_idx_pub] = mpc512x_clk_gated(\n" "> > + entry->name_mclk,\n" "> > + entry->name_mux1, sccr_reg, sccr_bit);\n" "> > + } else {\n" - "> > + clks[clks_idx_pub] =3D mpc512x_clk_factor(\n" + "> > + clks[clks_idx_pub] = mpc512x_clk_factor(\n" "> > + entry->name_mclk,\n" "> > + entry->name_mux1, 1, 1);\n" "> > + }\n" @@ -236,30 +208,24 @@ "> > + clk_register_clkdev(clks[clks_idx_pub], entry->name_mclk, NULL);\n" "> > +}\n" "> > [ ... ]\n" - "> =\n" - "\n" + "> \n" "> This was the routine which sets up _one_ MCLK block, note the\n" "> assignment at the routine's end to the \"published\" clock item\n" "> that's the gate's output after the second mux stage.\n" - "> =\n" - "\n" + "> \n" "> > [ ... ]\n" - "> > + clks[MPC512x_CLK_I2C] =3D mpc512x_clk_gated(\"i2c\", \"ips\",\n" + "> > + clks[MPC512x_CLK_I2C] = mpc512x_clk_gated(\"i2c\", \"ips\",\n" "> > + &clkregs->sccr2, 26);\n" - "> > + mpc512x_clk_setup_mclks(mclk_mscan_data, ARRAY_SIZE(mclk_mscan_da=\n" - "ta));\n" - "> > + clks[MPC512x_CLK_SDHC] =3D mpc512x_clk_gated(\"sdhc\", \"sdhc-ug\",\n" + "> > + mpc512x_clk_setup_mclks(mclk_mscan_data, ARRAY_SIZE(mclk_mscan_data));\n" + "> > + clks[MPC512x_CLK_SDHC] = mpc512x_clk_gated(\"sdhc\", \"sdhc-ug\",\n" "> > + &clkregs->sccr2, 24);\n" - "> > + mpc512x_clk_setup_mclks(mclk_spdif_data, ARRAY_SIZE(mclk_spdif_da=\n" - "ta));\n" + "> > + mpc512x_clk_setup_mclks(mclk_spdif_data, ARRAY_SIZE(mclk_spdif_data));\n" "> > [ ... ]\n" - "> =\n" - "\n" + "> \n" "> This is the invocation of the routine which sets up four MCLK\n" "> blocks for the MSCAN components, while all of them refer to bit\n" "> 25 of SCCR2.\n" - "> =\n" - "\n" + "> \n" "> > [ ... ]\n" "> > +\n" "> > + /* enable some of the clocks here unconditionally because ... */\n" @@ -274,8 +240,7 @@ "> > + /* some are required yet no dependencies were declared */\n" "> > + clk_prepare_enable(clks[MPC512x_CLK_PSC_FIFO]);\n" "> > + /* some are not yet acquired by their respective drivers */\n" - "> > + clk_prepare_enable(clks[MPC512x_CLK_PSC3_MCLK]);/* serial console=\n" - " */\n" + "> > + clk_prepare_enable(clks[MPC512x_CLK_PSC3_MCLK]);/* serial console */\n" "> > + clk_prepare_enable(clks[MPC512x_CLK_FEC]); /* network, NFS */\n" "> > + clk_prepare_enable(clks[MPC512x_CLK_DIU]); /* display */\n" "> > + clk_prepare_enable(clks[MPC512x_CLK_I2C]);\n" @@ -295,52 +260,42 @@ "> > + clk_prepare_enable(clks[MPC512x_CLK_MSCAN2_MCLK]);\n" "> > + clk_prepare_enable(clks[MPC512x_CLK_MSCAN3_MCLK]);\n" "> > +}\n" - "> =\n" - "\n" + "> \n" "> This is the pre-enable workaround for the MSCAN0 to MSCAN3 clock\n" "> items.\n" - "> =\n" - "\n" + "> \n" "> The above approach does work in that it introduces complete\n" "> support for common clock on the MPC512x platform, with the CAN\n" "> component being operational, and the clock driver using shared\n" "> logic across platforms.\n" - "> =\n" - "\n" + "> \n" "> The remaining issue is that regardless of whether CAN is used,\n" "> the (chip internal) clock is enabled. This may not be a problem\n" "> when bitrates aren't generated and the wire isn't driven.\n" - "> =\n" - "\n" - "> =\n" - "\n" + "> \n" + "> \n" "> The question now is how to correctly support the situation where\n" "> a gate is shared between subtrees yet isn't really part of any\n" "> path within the subtrees. I really cannot find a single spot\n" "> where to introduce the gate such that it's not duplicated.\n" - "> =\n" - "\n" + "> \n" "> The appropriate solution would not be to pre-enable those clocks,\n" "> but to either introduce another gate clock type which supports a\n" "> shared reference, or to add support for the shared reference to\n" "> the existing gate code.\n" - "> =\n" - "\n" - "> =\n" - "\n" + "> \n" + "> \n" "> I'd rather not duplicate most or all of the code of clk-gate.c,\n" "> instead I looked into how to add \"shared gate\" support to the\n" "> existing driver.\n" - "> =\n" - "\n" + "> \n" "> My question is whether the approach is acceptable. It adds\n" "> minimal overhead and shall be OK for the enable/disable path from\n" "> a technical POV. And it doesn't feel like too much of a stretch.\n" "> But there may be non-technical reasons to reject the approach.\n" "> I'd like to learn whether to follow that path before preparing\n" "> another version of the patch series.\n" - "> =\n" - "\n" + "> \n" "> The diffs were taken with the '-w -b' options to demonstrate\n" "> their essence and not drown it in whitespace changes. The\n" "> implementation assumes that the caller which registers the gate\n" @@ -348,8 +303,7 @@ "> the lock. And that all gates with a \"shared use counter\" use the\n" "> same lock (which is satisfied as they all get registered from the\n" "> same spot in the platform's clock driver).\n" - "> =\n" - "\n" + "> \n" "> The CLK_IGNORE_UNUSED flag addresses a different problem. The\n" "> SoC has four MSCAN components, while two of them are enabled in\n" "> the device tree (the other two are present but disabled). So\n" @@ -361,63 +315,52 @@ "> the common gate logic would implement a separate disable_unused()\n" "> routine, but I guess this isn't necessary and the use of the flag\n" "> is appropriate.\n" - "> =\n" - "\n" + "> \n" "> That the example use creates a field for just one counter is to\n" "> better demonstrate the use and potential extension as need\n" "> arises. Reducing this to a mere integer variable would be a\n" "> micro optimization.\n" - "> =\n" - "\n" - "> =\n" - "\n" + "> \n" + "> \n" "> The extension of the existing clk_gate implementation:\n" - "> =\n" - "\n" + "> \n" "> --- a/drivers/clk/clk-gate.c\n" "> +++ b/drivers/clk/clk-gate.c\n" - "> @@ -46,6 +46,7 @@ static void clk_gate_endisable(struct clk_hw *hw, int e=\n" - "nable)\n" - "> struct clk_gate *gate =3D to_clk_gate(hw);\n" - "> int set =3D gate->flags & CLK_GATE_SET_TO_DISABLE ? 1 : 0;\n" - "> unsigned long flags =3D 0;\n" + "> @@ -46,6 +46,7 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable)\n" + "> struct clk_gate *gate = to_clk_gate(hw);\n" + "> int set = gate->flags & CLK_GATE_SET_TO_DISABLE ? 1 : 0;\n" + "> unsigned long flags = 0;\n" "> + int need_reg_access;\n" "> u32 reg;\n" - "> =\n" - "\n" - "> set ^=3D enable;\n" - "> @@ -53,6 +54,20 @@ static void clk_gate_endisable(struct clk_hw *hw, int =\n" - "enable)\n" + "> \n" + "> set ^= enable;\n" + "> @@ -53,6 +54,20 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable)\n" "> if (gate->lock)\n" "> spin_lock_irqsave(gate->lock, flags);\n" - "> =\n" - "\n" + "> \n" "> + /*\n" "> + * if a \"shared use counter\" was specified, keep track of enable\n" "> + * and disable calls and only access hardware registers upon the\n" "> + * very first enable or very last disable call\n" "> + */\n" "> + if (!gate->share_count) {\n" - "> + need_reg_access =3D 1;\n" + "> + need_reg_access = 1;\n" "> + } else if (enable) {\n" - "> + need_reg_access =3D (*gate->share_count)++ =3D=3D 0;\n" + "> + need_reg_access = (*gate->share_count)++ == 0;\n" "> + } else {\n" - "> + need_reg_access =3D --(*gate->share_count) =3D=3D 0;\n" + "> + need_reg_access = --(*gate->share_count) == 0;\n" "> + }\n" "> +\n" "> + if (need_reg_access) {\n" "> if (gate->flags & CLK_GATE_HIWORD_MASK) {\n" - "> reg =3D BIT(gate->bit_idx + 16);\n" + "> reg = BIT(gate->bit_idx + 16);\n" "> if (set)\n" - "> @@ -67,6 +82,7 @@ static void clk_gate_endisable(struct clk_hw *hw, int e=\n" - "nable)\n" + "> @@ -67,6 +82,7 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable)\n" "> }\n" - "> =\n" - "\n" + "> \n" "> clk_writel(reg, gate->reg);\n" "> + }\n" - "> =\n" - "\n" + "> \n" "> if (gate->lock)\n" "> spin_unlock_irqrestore(gate->lock, flags);\n" "> @@ -118,10 +134,11 @@ EXPORT_SYMBOL_GPL(clk_gate_ops);\n" @@ -425,8 +368,7 @@ "> * @lock: shared register lock for this clock\n" "> */\n" "> -struct clk *clk_register_gate(struct device *dev, const char *name,\n" - "> +struct clk *clk_register_gate_shared(struct device *dev, const char *nam=\n" - "e,\n" + "> +struct clk *clk_register_gate_shared(struct device *dev, const char *name,\n" "> const char *parent_name, unsigned long flags,\n" "> void __iomem *reg, u8 bit_idx,\n" "> - u8 clk_gate_flags, spinlock_t *lock)\n" @@ -435,20 +377,16 @@ "> {\n" "> struct clk_gate *gate;\n" "> struct clk *clk;\n" - "> @@ -152,6 +169,7 @@ struct clk *clk_register_gate(struct device *dev, con=\n" - "st char *name,\n" - "> gate->bit_idx =3D bit_idx;\n" - "> gate->flags =3D clk_gate_flags;\n" - "> gate->lock =3D lock;\n" - "> + gate->share_count =3D share_count;\n" - "> gate->hw.init =3D &init;\n" - "> =\n" - "\n" - "> clk =3D clk_register(dev, &gate->hw);\n" - "> @@ -161,3 +179,14 @@ struct clk *clk_register_gate(struct device *dev, co=\n" - "nst char *name,\n" - "> =\n" - "\n" + "> @@ -152,6 +169,7 @@ struct clk *clk_register_gate(struct device *dev, const char *name,\n" + "> gate->bit_idx = bit_idx;\n" + "> gate->flags = clk_gate_flags;\n" + "> gate->lock = lock;\n" + "> + gate->share_count = share_count;\n" + "> gate->hw.init = &init;\n" + "> \n" + "> clk = clk_register(dev, &gate->hw);\n" + "> @@ -161,3 +179,14 @@ struct clk *clk_register_gate(struct device *dev, const char *name,\n" + "> \n" "> return clk;\n" "> }\n" "> +\n" @@ -470,38 +408,30 @@ "> spinlock_t *lock;\n" "> + int *share_count;\n" "> };\n" - "> =\n" - "\n" + "> \n" "> #define CLK_GATE_SET_TO_DISABLE BIT(0)\n" - "> @@ -232,6 +233,11 @@ struct clk *clk_register_gate(struct device *dev, co=\n" - "nst char *name,\n" + "> @@ -232,6 +233,11 @@ struct clk *clk_register_gate(struct device *dev, const char *name,\n" "> const char *parent_name, unsigned long flags,\n" "> void __iomem *reg, u8 bit_idx,\n" "> u8 clk_gate_flags, spinlock_t *lock);\n" - "> +struct clk *clk_register_gate_shared(struct device *dev, const char *nam=\n" - "e,\n" + "> +struct clk *clk_register_gate_shared(struct device *dev, const char *name,\n" "> + const char *parent_name, unsigned long flags,\n" "> + void __iomem *reg, u8 bit_idx,\n" "> + u8 clk_gate_flags, spinlock_t *lock,\n" "> + int *share_count);\n" - "> =\n" - "\n" + "> \n" "> struct clk_div_table {\n" "> unsigned int val;\n" - "> =\n" - "\n" - "> =\n" - "\n" + "> \n" + "> \n" "> How to use these shared gates:\n" - "> =\n" - "\n" + "> \n" "> --- a/arch/powerpc/platforms/512x/clock-commonclk.c\n" "> +++ b/arch/powerpc/platforms/512x/clock-commonclk.c\n" "> @@ -123,6 +123,39 @@ static inline struct clk *mpc512x_clk_gated(\n" "> reg, pos, 0, &clklock);\n" "> }\n" - "> =\n" - "\n" + "> \n" "> +enum mpc512x_clk_shared_gate_id_t {\n" "> + MPC512x_CLK_SHARED_GATE_MSCAN,\n" "> + MPC512x_CLK_SHARED_GATE_MAX,\n" @@ -528,46 +458,40 @@ "> +{\n" "> + int clkflags;\n" "> +\n" - "> + clkflags =3D CLK_SET_RATE_PARENT;\n" - "> + clkflags |=3D CLK_IGNORE_UNUSED;\n" + "> + clkflags = CLK_SET_RATE_PARENT;\n" + "> + clkflags |= CLK_IGNORE_UNUSED;\n" "> + return clk_register_gate_shared(NULL, name, parent_name, clkflags,\n" "> + reg, pos, 0, &clklock,\n" - "> + &mpc512x_clk_gate_counters[share_=\n" - "id]);\n" + "> + &mpc512x_clk_gate_counters[share_id]);\n" "> +}\n" "> +\n" "> static inline struct clk *mpc512x_clk_muxed(const char *name,\n" "> const char **parent_names, int parent_count,\n" "> u32 __iomem *reg, u8 pos, u8 len)\n" - "> @@ -520,9 +553,16 @@ static void mpc512x_clk_setup_mclk(struct mclk_setup=\n" - "_data *entry)\n" + "> @@ -520,9 +553,16 @@ static void mpc512x_clk_setup_mclk(struct mclk_setup_data *entry)\n" "> 1, 1);\n" "> }\n" "> if (sccr_reg) {\n" - "> + if (entry->type =3D=3D MCLK_TYPE_MSCAN) {\n" - "> + clks[clks_idx_pub] =3D mpc512x_clk_gated_shared(\n" + "> + if (entry->type == MCLK_TYPE_MSCAN) {\n" + "> + clks[clks_idx_pub] = mpc512x_clk_gated_shared(\n" "> + entry->name_mclk,\n" - "> + entry->name_mux1, sccr_reg, sccr_=\n" - "bit,\n" + "> + entry->name_mux1, sccr_reg, sccr_bit,\n" "> + MPC512x_CLK_SHARED_GATE_MSCAN);\n" "> + } else {\n" - "> clks[clks_idx_pub] =3D mpc512x_clk_gated(\n" + "> clks[clks_idx_pub] = mpc512x_clk_gated(\n" "> entry->name_mclk,\n" - "> entry->name_mux1, sccr_reg, sccr_=\n" - "bit);\n" + "> entry->name_mux1, sccr_reg, sccr_bit);\n" "> + }\n" "> } else {\n" - "> clks[clks_idx_pub] =3D mpc512x_clk_factor(\n" + "> clks[clks_idx_pub] = mpc512x_clk_factor(\n" "> entry->name_mclk,\n" - "> =\n" - "\n" + "> \n" "> Local tests have shown that the extension solves the problem of\n" "> how to satisfy the SoC's constraints on the MPC512x platform.\n" "> The MSCAN clocks no longer need to get pre-enabled, instead they\n" "> get setup and enabled only as the mscan(4) driver probes devices\n" "> according to how it was instructed (device tree nodes).\n" - "> =\n" - "\n" + "> \n" "> What do you think? Is the \"shared gate\" support in the common\n" "> logic appropriate? I'd rather not duplicate all of this code\n" "> just to introduce the specific gate I need, while most of the\n" @@ -575,18 +499,14 @@ "> desire isn't to override the gate's operations, but to wrap them\n" "> and to consult a counter in addition, while the register access\n" "> still applies.\n" - "> =\n" - "\n" - "> =\n" - "\n" - "> =\n" - "\n" + "> \n" + "> \n" + "> \n" "> virtually yours\n" "> Gerhard Sittig\n" - "> -- =\n" - "\n" + "> -- \n" "> DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel\n" "> HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany\n" - > Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de + > Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de -6dfe6303b82a73af88e2174bd1fc2e75c2cf3e92ba2bdbaca8ec7116a23a48a6 +0854d90b714711dbf45ea12a673d1a3685a0af914f69ab6738650e5779a516ec
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.