[PATCH 7/9] ARM: zynq: clk: add pll type
Signed-off-by: Steffen Trumtrar --- arch/arm/mach-zynq/clk-zynq7000.c | 33 - 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/arch/arm/mach-zynq/clk-zynq7000.c b/arch/arm/mach-zynq/clk-zynq7000.c index 5a8a12a..0d3c3a8 100644 --- a/arch/arm/mach-zynq/clk-zynq7000.c +++ b/arch/arm/mach-zynq/clk-zynq7000.c @@ -33,10 +33,21 @@ enum zynq_clks { cpu_clk, cpu_6x4x, cpu_3x2x, cpu_2x, cpu_1x, clks_max }; +enum zynq_pll_type { + ZYNQ_PLL_ARM, + ZYNQ_PLL_DDR, + ZYNQ_PLL_IO, +}; + +#define PLL_ARM_LOCK (1 << 0) +#define PLL_DDR_LOCK (1 << 1) +#define PLL_IO_LOCK(1 << 2) + static struct clk *clks[clks_max]; struct zynq_pll_clk { struct clk clk; + u32 pll_lock; void __iomem*pll_ctrl; }; @@ -51,11 +62,19 @@ static unsigned long zynq_pll_recalc_rate(struct clk *clk, return parent_rate * PLL_CTRL_FDIV(readl(pll->pll_ctrl)); } +static int zynq_pll_enable(struct clk *clk) +{ + return 0; +} + static struct clk_ops zynq_pll_clk_ops = { .recalc_rate = zynq_pll_recalc_rate, + .enable = zynq_pll_enable, }; -static inline struct clk *zynq_pll_clk(const char *name, void __iomem *pll_ctrl) +static inline struct clk *zynq_pll_clk(enum zynq_pll_type type, + const char *name, + void __iomem *pll_ctrl) { static const char *pll_parent = "ps_clk"; struct zynq_pll_clk *pll; @@ -68,6 +87,18 @@ static inline struct clk *zynq_pll_clk(const char *name, void __iomem *pll_ctrl) pll->clk.parent_names = &pll_parent; pll->clk.num_parents= 1; + switch(type) { + case ZYNQ_PLL_ARM: + pll->pll_lock = PLL_ARM_LOCK; + break; + case ZYNQ_PLL_DDR: + pll->pll_lock = PLL_DDR_LOCK; + break; + case ZYNQ_PLL_IO: + pll->pll_lock = PLL_IO_LOCK; + break; + } + ret = clk_register(&pll->clk); if (ret) { free(pll); -- 1.8.2.rc2 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 7/9] ARM: zynq: clk: add pll type
On Mon, Mar 11, 2013 at 10:15:04AM +0100, Steffen Trumtrar wrote: > Signed-off-by: Steffen Trumtrar > --- > arch/arm/mach-zynq/clk-zynq7000.c | 33 - > 1 file changed, 32 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/mach-zynq/clk-zynq7000.c > b/arch/arm/mach-zynq/clk-zynq7000.c > index 5a8a12a..0d3c3a8 100644 > --- a/arch/arm/mach-zynq/clk-zynq7000.c > +++ b/arch/arm/mach-zynq/clk-zynq7000.c > @@ -33,10 +33,21 @@ enum zynq_clks { > cpu_clk, cpu_6x4x, cpu_3x2x, cpu_2x, cpu_1x, clks_max > }; > > +enum zynq_pll_type { > + ZYNQ_PLL_ARM, > + ZYNQ_PLL_DDR, > + ZYNQ_PLL_IO, > +}; > + > +#define PLL_ARM_LOCK (1 << 0) > +#define PLL_DDR_LOCK (1 << 1) > +#define PLL_IO_LOCK (1 << 2) Having both an enum and the #define's seem like an unnecessary indirection. I'd suggest just: enum zynq_pll_lockbit { PLL_ARM_LOCK= (1 << 0), PLL_DDR_LOCK= (1 << 1), PLL_IO_LOCK = (1 << 2), }; struct zynq_pll_clk { /* ... */ enum zynq_pll_lockbit lockbit; }; static inline struct clk *zynq_pll_clk(enum zynq_pll_lockbit lockbit, const char *name, void __iomem *pll_ctrl) { /* ... */ pll->lockbit = lockbit; /* ... */ } > + > static struct clk *clks[clks_max]; > > struct zynq_pll_clk { > struct clk clk; > + u32 pll_lock; > void __iomem*pll_ctrl; > }; > > @@ -51,11 +62,19 @@ static unsigned long zynq_pll_recalc_rate(struct clk *clk, > return parent_rate * PLL_CTRL_FDIV(readl(pll->pll_ctrl)); > } > > +static int zynq_pll_enable(struct clk *clk) > +{ > + return 0; > +} > + > static struct clk_ops zynq_pll_clk_ops = { > .recalc_rate = zynq_pll_recalc_rate, > + .enable = zynq_pll_enable, > }; > > -static inline struct clk *zynq_pll_clk(const char *name, void __iomem > *pll_ctrl) > +static inline struct clk *zynq_pll_clk(enum zynq_pll_type type, > +const char *name, > +void __iomem *pll_ctrl) > { > static const char *pll_parent = "ps_clk"; > struct zynq_pll_clk *pll; > @@ -68,6 +87,18 @@ static inline struct clk *zynq_pll_clk(const char *name, > void __iomem *pll_ctrl) > pll->clk.parent_names = &pll_parent; > pll->clk.num_parents= 1; > > + switch(type) { > + case ZYNQ_PLL_ARM: > + pll->pll_lock = PLL_ARM_LOCK; > + break; > + case ZYNQ_PLL_DDR: > + pll->pll_lock = PLL_DDR_LOCK; > + break; > + case ZYNQ_PLL_IO: > + pll->pll_lock = PLL_IO_LOCK; Actually, maybe I've gotten a little ahead of myself...you add bits for the lock, but you never use it! So, what's the point! (If it's to be used in the future, it'd be nice to see that in the commit description). Josh ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 7/9] ARM: zynq: clk: add pll type
On Mon, Mar 11, 2013 at 01:28:09PM -0500, Josh Cartwright wrote: > On Mon, Mar 11, 2013 at 10:15:04AM +0100, Steffen Trumtrar wrote: > > Signed-off-by: Steffen Trumtrar > > --- > > arch/arm/mach-zynq/clk-zynq7000.c | 33 - > > 1 file changed, 32 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm/mach-zynq/clk-zynq7000.c > > b/arch/arm/mach-zynq/clk-zynq7000.c > > index 5a8a12a..0d3c3a8 100644 > > --- a/arch/arm/mach-zynq/clk-zynq7000.c > > +++ b/arch/arm/mach-zynq/clk-zynq7000.c > > @@ -33,10 +33,21 @@ enum zynq_clks { > > cpu_clk, cpu_6x4x, cpu_3x2x, cpu_2x, cpu_1x, clks_max > > }; > > > > +enum zynq_pll_type { > > + ZYNQ_PLL_ARM, > > + ZYNQ_PLL_DDR, > > + ZYNQ_PLL_IO, > > +}; > > + > > +#define PLL_ARM_LOCK (1 << 0) > > +#define PLL_DDR_LOCK (1 << 1) > > +#define PLL_IO_LOCK(1 << 2) > > Having both an enum and the #define's seem like an unnecessary > indirection. I'd suggest just: > > enum zynq_pll_lockbit { > PLL_ARM_LOCK= (1 << 0), > PLL_DDR_LOCK= (1 << 1), > PLL_IO_LOCK = (1 << 2), > }; > > struct zynq_pll_clk { > /* ... */ > enum zynq_pll_lockbit lockbit; > }; > > static inline struct clk *zynq_pll_clk(enum zynq_pll_lockbit lockbit, > const char *name, > void __iomem *pll_ctrl) > { > /* ... */ > pll->lockbit = lockbit; > /* ... */ > } > > > + > > static struct clk *clks[clks_max]; > > > > struct zynq_pll_clk { > > struct clk clk; > > + u32 pll_lock; > > void __iomem*pll_ctrl; > > }; > > > > @@ -51,11 +62,19 @@ static unsigned long zynq_pll_recalc_rate(struct clk > > *clk, > > return parent_rate * PLL_CTRL_FDIV(readl(pll->pll_ctrl)); > > } > > > > +static int zynq_pll_enable(struct clk *clk) > > +{ > > + return 0; > > +} > > + > > static struct clk_ops zynq_pll_clk_ops = { > > .recalc_rate = zynq_pll_recalc_rate, > > + .enable = zynq_pll_enable, > > }; > > > > -static inline struct clk *zynq_pll_clk(const char *name, void __iomem > > *pll_ctrl) > > +static inline struct clk *zynq_pll_clk(enum zynq_pll_type type, > > + const char *name, > > + void __iomem *pll_ctrl) > > { > > static const char *pll_parent = "ps_clk"; > > struct zynq_pll_clk *pll; > > @@ -68,6 +87,18 @@ static inline struct clk *zynq_pll_clk(const char *name, > > void __iomem *pll_ctrl) > > pll->clk.parent_names = &pll_parent; > > pll->clk.num_parents= 1; > > > > + switch(type) { > > + case ZYNQ_PLL_ARM: > > + pll->pll_lock = PLL_ARM_LOCK; > > + break; > > + case ZYNQ_PLL_DDR: > > + pll->pll_lock = PLL_DDR_LOCK; > > + break; > > + case ZYNQ_PLL_IO: > > + pll->pll_lock = PLL_IO_LOCK; > > Actually, maybe I've gotten a little ahead of myself...you add bits for > the lock, but you never use it! So, what's the point! (If it's to be > used in the future, it'd be nice to see that in the commit description). I will remove this from this series. This only makes sense, when the enable function is filled. str -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox