Message ID | 1454527562-28154-11-git-send-email-p.zabel@pengutronix.de (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Quoting Philipp Zabel (2016-02-03 11:25:59) > This mux is supposed to select a fitting divider after the PLL > is already set to the correct rate. > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> > Acked-by: James Liao <jamesjj.liao@mediatek.com> > --- > drivers/clk/mediatek/clk-mt8173.c | 2 +- > drivers/clk/mediatek/clk-mtk.h | 7 +++++-- > 2 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c > index 227e356..682b275 100644 > --- a/drivers/clk/mediatek/clk-mt8173.c > +++ b/drivers/clk/mediatek/clk-mt8173.c > @@ -558,7 +558,7 @@ static const struct mtk_composite top_muxes[] __initconst = { > MUX_GATE(CLK_TOP_ATB_SEL, "atb_sel", atb_parents, 0x0090, 16, 2, 23), > MUX_GATE(CLK_TOP_VENC_LT_SEL, "venclt_sel", venc_lt_parents, 0x0090, 24, 4, 31), > /* CLK_CFG_6 */ > - MUX_GATE(CLK_TOP_DPI0_SEL, "dpi0_sel", dpi0_parents, 0x00a0, 0, 3, 7), > + MUX_GATE_FLAGS(CLK_TOP_DPI0_SEL, "dpi0_sel", dpi0_parents, 0x00a0, 0, 3, 7, 0), So the only instance of MUX_GATE that should not propagate it's request up to the parent is dpi0_sel? Are you sure? I hope so because this patch changes all MUX_GATE clks to propagate their requests up to their parents, which is sort of a big change. Also, the name game is a bit confusing. I can see that you're trying to prevent having a huge patch that touches every MUX_GATE initialization. However it isn't obvious from the name "MUX_GATE()" that the macro will enable CLK_SET_RATE_PARENT functionality. Maybe put a comment above it just to make it extra clear? Regards, Mike > MUX_GATE(CLK_TOP_IRDA_SEL, "irda_sel", irda_parents, 0x00a0, 8, 2, 15), > MUX_GATE(CLK_TOP_CCI400_SEL, "cci400_sel", cci400_parents, 0x00a0, 16, 3, 23), > MUX_GATE(CLK_TOP_AUD_1_SEL, "aud_1_sel", aud_1_parents, 0x00a0, 24, 2, 31), > diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h > index 32d2e45..b607996 100644 > --- a/drivers/clk/mediatek/clk-mtk.h > +++ b/drivers/clk/mediatek/clk-mtk.h > @@ -83,7 +83,7 @@ struct mtk_composite { > signed char num_parents; > }; > > -#define MUX_GATE(_id, _name, _parents, _reg, _shift, _width, _gate) { \ > +#define MUX_GATE_FLAGS(_id, _name, _parents, _reg, _shift, _width, _gate, _flags) { \ > .id = _id, \ > .name = _name, \ > .mux_reg = _reg, \ > @@ -94,9 +94,12 @@ struct mtk_composite { > .divider_shift = -1, \ > .parent_names = _parents, \ > .num_parents = ARRAY_SIZE(_parents), \ > - .flags = CLK_SET_RATE_PARENT, \ > + .flags = _flags, \ > } > > +#define MUX_GATE(_id, _name, _parents, _reg, _shift, _width, _gate) \ > + MUX_GATE_FLAGS(_id, _name, _parents, _reg, _shift, _width, _gate, CLK_SET_RATE_PARENT) > + > #define MUX(_id, _name, _parents, _reg, _shift, _width) { \ > .id = _id, \ > .name = _name, \ > -- > 2.7.0.rc3 >
Hi Michael, thank you for the comments. Am Montag, den 15.02.2016, 15:14 -0800 schrieb Michael Turquette: > Quoting Philipp Zabel (2016-02-03 11:25:59) > > This mux is supposed to select a fitting divider after the PLL > > is already set to the correct rate. > > > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> > > Acked-by: James Liao <jamesjj.liao@mediatek.com> > > --- > > drivers/clk/mediatek/clk-mt8173.c | 2 +- > > drivers/clk/mediatek/clk-mtk.h | 7 +++++-- > > 2 files changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c > > index 227e356..682b275 100644 > > --- a/drivers/clk/mediatek/clk-mt8173.c > > +++ b/drivers/clk/mediatek/clk-mt8173.c > > @@ -558,7 +558,7 @@ static const struct mtk_composite top_muxes[] __initconst = { > > MUX_GATE(CLK_TOP_ATB_SEL, "atb_sel", atb_parents, 0x0090, 16, 2, 23), > > MUX_GATE(CLK_TOP_VENC_LT_SEL, "venclt_sel", venc_lt_parents, 0x0090, 24, 4, 31), > > /* CLK_CFG_6 */ > > - MUX_GATE(CLK_TOP_DPI0_SEL, "dpi0_sel", dpi0_parents, 0x00a0, 0, 3, 7), > > + MUX_GATE_FLAGS(CLK_TOP_DPI0_SEL, "dpi0_sel", dpi0_parents, 0x00a0, 0, 3, 7, 0), > > So the only instance of MUX_GATE that should not propagate it's request > up to the parent is dpi0_sel? Are you sure? The other way around. This is the only clock I'm sure of that it shouldn't propagate. For the others I don't know, so I want to leave them as they are. > I hope so because this patch changes all MUX_GATE clks to propagate > their requests up to their parents, which is sort of a big change. The MUX_GATE macro previously unconditionally set the CLK_SET_RATE_PARENT, so nothing should have changed there. See (way) below. > Also, the name game is a bit confusing. I can see that you're trying to > prevent having a huge patch that touches every MUX_GATE initialization. > However it isn't obvious from the name "MUX_GATE()" that the macro will > enable CLK_SET_RATE_PARENT functionality. Maybe put a comment above it > just to make it extra clear? I can do that. > > MUX_GATE(CLK_TOP_IRDA_SEL, "irda_sel", irda_parents, 0x00a0, 8, 2, 15), > > MUX_GATE(CLK_TOP_CCI400_SEL, "cci400_sel", cci400_parents, 0x00a0, 16, 3, 23), > > MUX_GATE(CLK_TOP_AUD_1_SEL, "aud_1_sel", aud_1_parents, 0x00a0, 24, 2, 31), > > diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h > > index 32d2e45..b607996 100644 > > --- a/drivers/clk/mediatek/clk-mtk.h > > +++ b/drivers/clk/mediatek/clk-mtk.h > > @@ -83,7 +83,7 @@ struct mtk_composite { > > signed char num_parents; > > }; > > > > -#define MUX_GATE(_id, _name, _parents, _reg, _shift, _width, _gate) { \ > > +#define MUX_GATE_FLAGS(_id, _name, _parents, _reg, _shift, _width, _gate, _flags) { \ > > .id = _id, \ > > .name = _name, \ > > .mux_reg = _reg, \ > > @@ -94,9 +94,12 @@ struct mtk_composite { > > .divider_shift = -1, \ > > .parent_names = _parents, \ > > .num_parents = ARRAY_SIZE(_parents), \ > > - .flags = CLK_SET_RATE_PARENT, \ > > + .flags = _flags, \ This. > > } > > > > +#define MUX_GATE(_id, _name, _parents, _reg, _shift, _width, _gate) \ > > + MUX_GATE_FLAGS(_id, _name, _parents, _reg, _shift, _width, _gate, CLK_SET_RATE_PARENT) > > + > > #define MUX(_id, _name, _parents, _reg, _shift, _width) { \ > > .id = _id, \ > > .name = _name, \ > > -- > > 2.7.0.rc3 best regards Philipp
diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c index 227e356..682b275 100644 --- a/drivers/clk/mediatek/clk-mt8173.c +++ b/drivers/clk/mediatek/clk-mt8173.c @@ -558,7 +558,7 @@ static const struct mtk_composite top_muxes[] __initconst = { MUX_GATE(CLK_TOP_ATB_SEL, "atb_sel", atb_parents, 0x0090, 16, 2, 23), MUX_GATE(CLK_TOP_VENC_LT_SEL, "venclt_sel", venc_lt_parents, 0x0090, 24, 4, 31), /* CLK_CFG_6 */ - MUX_GATE(CLK_TOP_DPI0_SEL, "dpi0_sel", dpi0_parents, 0x00a0, 0, 3, 7), + MUX_GATE_FLAGS(CLK_TOP_DPI0_SEL, "dpi0_sel", dpi0_parents, 0x00a0, 0, 3, 7, 0), MUX_GATE(CLK_TOP_IRDA_SEL, "irda_sel", irda_parents, 0x00a0, 8, 2, 15), MUX_GATE(CLK_TOP_CCI400_SEL, "cci400_sel", cci400_parents, 0x00a0, 16, 3, 23), MUX_GATE(CLK_TOP_AUD_1_SEL, "aud_1_sel", aud_1_parents, 0x00a0, 24, 2, 31), diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h index 32d2e45..b607996 100644 --- a/drivers/clk/mediatek/clk-mtk.h +++ b/drivers/clk/mediatek/clk-mtk.h @@ -83,7 +83,7 @@ struct mtk_composite { signed char num_parents; }; -#define MUX_GATE(_id, _name, _parents, _reg, _shift, _width, _gate) { \ +#define MUX_GATE_FLAGS(_id, _name, _parents, _reg, _shift, _width, _gate, _flags) { \ .id = _id, \ .name = _name, \ .mux_reg = _reg, \ @@ -94,9 +94,12 @@ struct mtk_composite { .divider_shift = -1, \ .parent_names = _parents, \ .num_parents = ARRAY_SIZE(_parents), \ - .flags = CLK_SET_RATE_PARENT, \ + .flags = _flags, \ } +#define MUX_GATE(_id, _name, _parents, _reg, _shift, _width, _gate) \ + MUX_GATE_FLAGS(_id, _name, _parents, _reg, _shift, _width, _gate, CLK_SET_RATE_PARENT) + #define MUX(_id, _name, _parents, _reg, _shift, _width) { \ .id = _id, \ .name = _name, \