diff mbox

[v10,10/13] clk: mediatek: make dpi0_sel propagate rate changes

Message ID 1454527562-28154-11-git-send-email-p.zabel@pengutronix.de (mailing list archive)
State New, archived
Headers show

Commit Message

Philipp Zabel Feb. 3, 2016, 7:25 p.m. UTC
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(-)

Comments

Michael Turquette Feb. 15, 2016, 11:14 p.m. UTC | #1
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
>
Philipp Zabel Feb. 17, 2016, 9:19 a.m. UTC | #2
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 mbox

Patch

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,						\