diff for duplicates of <20130802233009.6450.21915@quantum>
diff --git a/a/1.txt b/N1/1.txt
index 9704637..4c6bbc1 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
\ No newline at end of file
+> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
\ No newline at end of file
diff --git a/a/content_digest b/N1/content_digest
index 3254121..0dd0c3c 100644
--- a/a/content_digest
+++ b/N1/content_digest
@@ -11,31 +11,16 @@
"ref\00020130723131406.GI19071\@book.gsilab.sittig.org\0"
]
[
- "From\0Mike Turquette <mturquette\@linaro.org>\0"
+ "From\0mturquette\@linaro.org (Mike Turquette)\0"
]
[
- "Subject\0Re: [PATCH v3 17/31] clk: mpc512x: introduce COMMON_CLK for MPC512x\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"
]
[
"\0000:1\0"
@@ -46,19 +31,15 @@
[
"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",
@@ -67,8 +48,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",
@@ -84,15 +64,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",
@@ -100,14 +78,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",
@@ -119,12 +95,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",
@@ -132,43 +106,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",
@@ -177,8 +149,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",
@@ -189,8 +161,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",
@@ -213,36 +185,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",
@@ -259,30 +229,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",
@@ -297,8 +261,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",
@@ -318,52 +281,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",
@@ -371,8 +324,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",
@@ -384,63 +336,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",
@@ -448,8 +389,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",
@@ -458,20 +398,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",
@@ -493,38 +429,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",
@@ -551,46 +479,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",
@@ -598,19 +520,15 @@
"> 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.