All of lore.kernel.org
 help / color / mirror / Atom feed
From: sebastian.hesselbarth@gmail.com (Sebastian Hesselbarth)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/5] clk: berlin: add support for berlin plls
Date: Fri, 21 Mar 2014 13:49:32 +0100	[thread overview]
Message-ID: <532C355C.3090606@gmail.com> (raw)
In-Reply-To: <1395402220-23503-2-git-send-email-alexandre.belloni@free-electrons.com>

On 03/21/2014 12:43 PM, Alexandre Belloni wrote:
> This drivers allows to provide DT clocks for the cpu and system PLLs found on
> Marvell Berlin SoCs.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
>   drivers/clk/Makefile              |   1 +
>   drivers/clk/berlin/Makefile       |   1 +
>   drivers/clk/berlin/clk.h          |  35 +++++++++++++
>   drivers/clk/berlin/pll-berlin2.c  |  41 ++++++++++++++++
>   drivers/clk/berlin/pll-berlin2q.c |  41 ++++++++++++++++
>   drivers/clk/berlin/pll.c          | 100 ++++++++++++++++++++++++++++++++++++++
>   6 files changed, 219 insertions(+)
>   create mode 100644 drivers/clk/berlin/Makefile
>   create mode 100644 drivers/clk/berlin/clk.h
>   create mode 100644 drivers/clk/berlin/pll-berlin2.c
>   create mode 100644 drivers/clk/berlin/pll-berlin2q.c
>   create mode 100644 drivers/clk/berlin/pll.c
>
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index a367a9831717..4a2602737c27 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_ARCH_VT8500)		+= clk-vt8500.o
>   obj-$(CONFIG_COMMON_CLK_WM831X)		+= clk-wm831x.o
>   obj-$(CONFIG_COMMON_CLK_XGENE)		+= clk-xgene.o
>   obj-$(CONFIG_COMMON_CLK_AT91)		+= at91/
> +obj-$(CONFIG_ARCH_BERLIN)		+= berlin/
>   obj-$(CONFIG_ARCH_HI3xxx)		+= hisilicon/
>   obj-$(CONFIG_COMMON_CLK_KEYSTONE)	+= keystone/

I'll have a look at clk/Makefile later myself, but alphabetic ordering
seems to be broken by ARCH_HI3xxx already. Anyway, we shouldn't just
add another violator.

>   ifeq ($(CONFIG_COMMON_CLK), y)
> diff --git a/drivers/clk/berlin/Makefile b/drivers/clk/berlin/Makefile
> new file mode 100644
> index 000000000000..4f6580e2a3ee
> --- /dev/null
> +++ b/drivers/clk/berlin/Makefile
> @@ -0,0 +1 @@
> +obj-y += pll.o pll-berlin2.o pll-berlin2q.o

obj-y += pll.o
obj-$(CONFIG_MACH_BERLIN_BG2)	+= pll-berlin2.o
obj-$(CONFIG_MACH_BERLIN_BG2CD)	+= pll-berlin2.o
obj-$(CONFIG_MACH_BERLIN_BG2Q)	+= pll-berlin2q.o

Which reminds me, that we forgot to add MACH_BERLIN_BG2Q to
arch/arm/mach-berlin/Kconfig. Can you spin a patch?

> diff --git a/drivers/clk/berlin/clk.h b/drivers/clk/berlin/clk.h

I prefer to have this include named "common.h" as I guess we will
dump all common declarations in here.

> new file mode 100644
> index 000000000000..41459db31a02
> --- /dev/null
> +++ b/drivers/clk/berlin/clk.h
> @@ -0,0 +1,35 @@
> +/*
> + * Copyright (c) 2013 Marvell Technology Group Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#ifndef __BERLIN_CLK_H
> +#define __BERLIN_CLK_H
> +
> +#include <linux/clk-provider.h>

What is this include good for in here?

You will need struct device_node declared, but a simple

struct device_node;

should be sufficient.

> +struct berlin_pllmap {
> +	const u8	*vcodiv;
> +	u32		fbdiv_mask;
> +	u32		rfdiv_mask;
> +	u32		divsel_mask;
> +	u8		fbdiv_shift;
> +	u8		rfdiv_shift;
> +	u8		divsel_shift;
> +	u8		mult;
> +};
> +
> +extern void __init berlin_pll_setup(struct device_node *np,
> +				struct berlin_pllmap *map);
> +
> +#endif /* BERLIN_CLK_H */
> diff --git a/drivers/clk/berlin/pll-berlin2.c b/drivers/clk/berlin/pll-berlin2.c
> new file mode 100644
> index 000000000000..5f5b4674f811
> --- /dev/null
> +++ b/drivers/clk/berlin/pll-berlin2.c
> @@ -0,0 +1,41 @@
> +/*
> + * Copyright (c) 2014 Marvell Technology Group Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/kernel.h>
> +#include <linux/clk-provider.h>

Please sort includes alphabetically, it will save us some pain
in the future.

> +#include "clk.h"

#include "common.h"

> +static const u8 vcodiv_berlin2[] = {10, 15, 20, 25, 30, 40, 50, 60, 80};
> +
> +static struct berlin_pllmap berlin_pll_map = {
> +	.vcodiv = vcodiv_berlin2,
> +	.fbdiv_mask = 0x1FF,
> +	.rfdiv_mask = 0x1F,
> +	.divsel_mask = 0xF,

divsel_mask allows 16 possible values, vcodiv_berlin2[] only provides
9, and common pll driver below does not know how many valid vcodiv
values are passed. That can become dangerous..

I'd rather extend vcodiv_berlin2 to full divsel range and provide
safe (=1) divisiors. This way wrong/new register values will only
break clock frequency derived.

> +	.fbdiv_shift = 6,
> +	.rfdiv_shift = 1,
> +	.divsel_shift = 7,

Have .foo_mask and .foo_shift together?

> +	.mult = 10,
> +};
> +
> +static void __init berlin2_pll_setup(struct device_node *np)
> +{
> +	berlin_pll_setup(np, &berlin_pll_map);
> +}
> +CLK_OF_DECLARE(berlin2q_pll, "marvell,berlin2-pll", berlin2_pll_setup);
> +
> diff --git a/drivers/clk/berlin/pll-berlin2q.c b/drivers/clk/berlin/pll-berlin2q.c
> new file mode 100644
> index 000000000000..8ddfa69cf8c4
> --- /dev/null
> +++ b/drivers/clk/berlin/pll-berlin2q.c
> @@ -0,0 +1,41 @@
> +/*
> + * Copyright (c) 2013 Marvell Technology Group Ltd.

nit: 2014?

> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/kernel.h>
> +#include <linux/clk-provider.h>

same as above.

> +#include "clk.h"

ditto.

> +static const u8 vcodiv_berlin2q[] = {1, 0, 2, 0, 3, 4, 0, 6, 8};
> +
> +static struct berlin_pllmap berlin2q_pll_map = {
> +	.vcodiv = vcodiv_berlin2q,
> +	.fbdiv_mask = 0x1FF,
> +	.rfdiv_mask = 0x1F,
> +	.divsel_mask = 0xF,
> +	.fbdiv_shift = 7,
> +	.rfdiv_shift = 2,
> +	.divsel_shift = 9,
> +	.mult = 1,
> +};

ditto.

> +static void __init berlin2q_pll_setup(struct device_node *np)
> +{
> +	berlin_pll_setup(np, &berlin2q_pll_map);
> +}
> +CLK_OF_DECLARE(berlin2q_pll, "marvell,berlin2q-pll", berlin2q_pll_setup);
> +
> diff --git a/drivers/clk/berlin/pll.c b/drivers/clk/berlin/pll.c
> new file mode 100644
> index 000000000000..188b385af436
> --- /dev/null
> +++ b/drivers/clk/berlin/pll.c
> @@ -0,0 +1,100 @@
> +/*
> + * Copyright (c) 2013 Marvell Technology Group Ltd.

nit: 2014?

> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +#include <linux/kernel.h>
> +#include <linux/clk-provider.h>

same as above.

> +#include "clk.h"

ditto.

> +struct berlin_pll {
> +	struct clk_hw	hw;
> +	void __iomem	*base;
> +	void		*data;

struct berlin2_pllmap *map instead?

> +};
> +
> +#define to_berlin_pll(hw)	container_of(hw, struct berlin_pll, hw)
> +
> +#define PLL_CTRL0	0x00
> +#define PLL_CTRL1	0x04
> +
> +static inline u32 berlin_pll_read(struct berlin_pll *pll, unsigned long offset)
> +{
> +	return readl_relaxed(pll->base + offset);
> +}
> +
> +static unsigned long berlin_pll_recalc_rate(struct clk_hw *hw,
> +					unsigned long parent_rate)

Maybe have a nice little comment on how pll output frequency is
calculated out of the 5 variables?

> +{
> +	struct berlin_pll *pll = to_berlin_pll(hw);
> +	struct berlin_pllmap *map = pll->data;
> +	u32 val, fbdiv, rfdiv, vcodivsel;
> +
> +	val = berlin_pll_read(pll, PLL_CTRL0);
> +	fbdiv = (val >> map->fbdiv_shift) & map->fbdiv_mask;
> +	rfdiv = (val >> map->rfdiv_shift) & map->rfdiv_mask;
> +	if (rfdiv == 0)
> +		rfdiv = 1;
> +
> +	val = berlin_pll_read(pll, PLL_CTRL1);
> +	vcodivsel = (val >> map->divsel_shift) & map->divsel_mask;
> +
> +	return parent_rate * fbdiv * map->mult / rfdiv /
> +		map->vcodiv[vcodivsel];

IMHO

parent_rate *= fbdiv * map->mult;
parent_rate /= rfdiv;
return parent_rate / map->vcodiv[vcodivsel];

makes it slightly more readable, but that is up to you.

> +}
> +
> +static const struct clk_ops berlin_pll_ops = {
> +	.recalc_rate	= berlin_pll_recalc_rate,
> +};
> +
> +void __init berlin_pll_setup(struct device_node *np,
> +			struct berlin_pllmap *map)
> +{
> +	struct clk_init_data init;
> +	struct berlin_pll *pll;
> +	const char *parent_name;
> +	struct clk *clk;
> +	int ret;
> +
> +	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> +	if (WARN_ON(!pll))
> +		return;
> +
> +	pll->base = of_iomap(np, 0);
> +	if (WARN_ON(!pll->base))
> +		return;
> +
> +	pll->data = map;

pll->map = map;

> +
> +	init.name = np->name;
> +	init.ops = &berlin_pll_ops;
> +	parent_name = of_clk_get_parent_name(np, 0);
> +	init.parent_names = &parent_name;
> +	init.num_parents = 1;
> +
> +	pll->hw.init = &init;
> +
> +	clk = clk_register(NULL, &pll->hw);
> +	if (WARN_ON(IS_ERR(clk)))
> +		return;
> +
> +	ret = of_clk_add_provider(np, of_clk_src_simple_get, clk);
> +	if (WARN_ON(ret))
> +		return;
> +}
> +
>

WARNING: multiple messages have this Message-ID (diff)
From: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
To: Alexandre Belloni <alexandre.belloni@free-electrons.com>,
	Mike Turquette <mturquette@linaro.org>
Cc: linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	"Antoine Ténart" <antoine.tenart@free-electrons.com>
Subject: Re: [PATCH 1/5] clk: berlin: add support for berlin plls
Date: Fri, 21 Mar 2014 13:49:32 +0100	[thread overview]
Message-ID: <532C355C.3090606@gmail.com> (raw)
In-Reply-To: <1395402220-23503-2-git-send-email-alexandre.belloni@free-electrons.com>

On 03/21/2014 12:43 PM, Alexandre Belloni wrote:
> This drivers allows to provide DT clocks for the cpu and system PLLs found on
> Marvell Berlin SoCs.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
>   drivers/clk/Makefile              |   1 +
>   drivers/clk/berlin/Makefile       |   1 +
>   drivers/clk/berlin/clk.h          |  35 +++++++++++++
>   drivers/clk/berlin/pll-berlin2.c  |  41 ++++++++++++++++
>   drivers/clk/berlin/pll-berlin2q.c |  41 ++++++++++++++++
>   drivers/clk/berlin/pll.c          | 100 ++++++++++++++++++++++++++++++++++++++
>   6 files changed, 219 insertions(+)
>   create mode 100644 drivers/clk/berlin/Makefile
>   create mode 100644 drivers/clk/berlin/clk.h
>   create mode 100644 drivers/clk/berlin/pll-berlin2.c
>   create mode 100644 drivers/clk/berlin/pll-berlin2q.c
>   create mode 100644 drivers/clk/berlin/pll.c
>
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index a367a9831717..4a2602737c27 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_ARCH_VT8500)		+= clk-vt8500.o
>   obj-$(CONFIG_COMMON_CLK_WM831X)		+= clk-wm831x.o
>   obj-$(CONFIG_COMMON_CLK_XGENE)		+= clk-xgene.o
>   obj-$(CONFIG_COMMON_CLK_AT91)		+= at91/
> +obj-$(CONFIG_ARCH_BERLIN)		+= berlin/
>   obj-$(CONFIG_ARCH_HI3xxx)		+= hisilicon/
>   obj-$(CONFIG_COMMON_CLK_KEYSTONE)	+= keystone/

I'll have a look at clk/Makefile later myself, but alphabetic ordering
seems to be broken by ARCH_HI3xxx already. Anyway, we shouldn't just
add another violator.

>   ifeq ($(CONFIG_COMMON_CLK), y)
> diff --git a/drivers/clk/berlin/Makefile b/drivers/clk/berlin/Makefile
> new file mode 100644
> index 000000000000..4f6580e2a3ee
> --- /dev/null
> +++ b/drivers/clk/berlin/Makefile
> @@ -0,0 +1 @@
> +obj-y += pll.o pll-berlin2.o pll-berlin2q.o

obj-y += pll.o
obj-$(CONFIG_MACH_BERLIN_BG2)	+= pll-berlin2.o
obj-$(CONFIG_MACH_BERLIN_BG2CD)	+= pll-berlin2.o
obj-$(CONFIG_MACH_BERLIN_BG2Q)	+= pll-berlin2q.o

Which reminds me, that we forgot to add MACH_BERLIN_BG2Q to
arch/arm/mach-berlin/Kconfig. Can you spin a patch?

> diff --git a/drivers/clk/berlin/clk.h b/drivers/clk/berlin/clk.h

I prefer to have this include named "common.h" as I guess we will
dump all common declarations in here.

> new file mode 100644
> index 000000000000..41459db31a02
> --- /dev/null
> +++ b/drivers/clk/berlin/clk.h
> @@ -0,0 +1,35 @@
> +/*
> + * Copyright (c) 2013 Marvell Technology Group Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#ifndef __BERLIN_CLK_H
> +#define __BERLIN_CLK_H
> +
> +#include <linux/clk-provider.h>

What is this include good for in here?

You will need struct device_node declared, but a simple

struct device_node;

should be sufficient.

> +struct berlin_pllmap {
> +	const u8	*vcodiv;
> +	u32		fbdiv_mask;
> +	u32		rfdiv_mask;
> +	u32		divsel_mask;
> +	u8		fbdiv_shift;
> +	u8		rfdiv_shift;
> +	u8		divsel_shift;
> +	u8		mult;
> +};
> +
> +extern void __init berlin_pll_setup(struct device_node *np,
> +				struct berlin_pllmap *map);
> +
> +#endif /* BERLIN_CLK_H */
> diff --git a/drivers/clk/berlin/pll-berlin2.c b/drivers/clk/berlin/pll-berlin2.c
> new file mode 100644
> index 000000000000..5f5b4674f811
> --- /dev/null
> +++ b/drivers/clk/berlin/pll-berlin2.c
> @@ -0,0 +1,41 @@
> +/*
> + * Copyright (c) 2014 Marvell Technology Group Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/kernel.h>
> +#include <linux/clk-provider.h>

Please sort includes alphabetically, it will save us some pain
in the future.

> +#include "clk.h"

#include "common.h"

> +static const u8 vcodiv_berlin2[] = {10, 15, 20, 25, 30, 40, 50, 60, 80};
> +
> +static struct berlin_pllmap berlin_pll_map = {
> +	.vcodiv = vcodiv_berlin2,
> +	.fbdiv_mask = 0x1FF,
> +	.rfdiv_mask = 0x1F,
> +	.divsel_mask = 0xF,

divsel_mask allows 16 possible values, vcodiv_berlin2[] only provides
9, and common pll driver below does not know how many valid vcodiv
values are passed. That can become dangerous..

I'd rather extend vcodiv_berlin2 to full divsel range and provide
safe (=1) divisiors. This way wrong/new register values will only
break clock frequency derived.

> +	.fbdiv_shift = 6,
> +	.rfdiv_shift = 1,
> +	.divsel_shift = 7,

Have .foo_mask and .foo_shift together?

> +	.mult = 10,
> +};
> +
> +static void __init berlin2_pll_setup(struct device_node *np)
> +{
> +	berlin_pll_setup(np, &berlin_pll_map);
> +}
> +CLK_OF_DECLARE(berlin2q_pll, "marvell,berlin2-pll", berlin2_pll_setup);
> +
> diff --git a/drivers/clk/berlin/pll-berlin2q.c b/drivers/clk/berlin/pll-berlin2q.c
> new file mode 100644
> index 000000000000..8ddfa69cf8c4
> --- /dev/null
> +++ b/drivers/clk/berlin/pll-berlin2q.c
> @@ -0,0 +1,41 @@
> +/*
> + * Copyright (c) 2013 Marvell Technology Group Ltd.

nit: 2014?

> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/kernel.h>
> +#include <linux/clk-provider.h>

same as above.

> +#include "clk.h"

ditto.

> +static const u8 vcodiv_berlin2q[] = {1, 0, 2, 0, 3, 4, 0, 6, 8};
> +
> +static struct berlin_pllmap berlin2q_pll_map = {
> +	.vcodiv = vcodiv_berlin2q,
> +	.fbdiv_mask = 0x1FF,
> +	.rfdiv_mask = 0x1F,
> +	.divsel_mask = 0xF,
> +	.fbdiv_shift = 7,
> +	.rfdiv_shift = 2,
> +	.divsel_shift = 9,
> +	.mult = 1,
> +};

ditto.

> +static void __init berlin2q_pll_setup(struct device_node *np)
> +{
> +	berlin_pll_setup(np, &berlin2q_pll_map);
> +}
> +CLK_OF_DECLARE(berlin2q_pll, "marvell,berlin2q-pll", berlin2q_pll_setup);
> +
> diff --git a/drivers/clk/berlin/pll.c b/drivers/clk/berlin/pll.c
> new file mode 100644
> index 000000000000..188b385af436
> --- /dev/null
> +++ b/drivers/clk/berlin/pll.c
> @@ -0,0 +1,100 @@
> +/*
> + * Copyright (c) 2013 Marvell Technology Group Ltd.

nit: 2014?

> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +#include <linux/kernel.h>
> +#include <linux/clk-provider.h>

same as above.

> +#include "clk.h"

ditto.

> +struct berlin_pll {
> +	struct clk_hw	hw;
> +	void __iomem	*base;
> +	void		*data;

struct berlin2_pllmap *map instead?

> +};
> +
> +#define to_berlin_pll(hw)	container_of(hw, struct berlin_pll, hw)
> +
> +#define PLL_CTRL0	0x00
> +#define PLL_CTRL1	0x04
> +
> +static inline u32 berlin_pll_read(struct berlin_pll *pll, unsigned long offset)
> +{
> +	return readl_relaxed(pll->base + offset);
> +}
> +
> +static unsigned long berlin_pll_recalc_rate(struct clk_hw *hw,
> +					unsigned long parent_rate)

Maybe have a nice little comment on how pll output frequency is
calculated out of the 5 variables?

> +{
> +	struct berlin_pll *pll = to_berlin_pll(hw);
> +	struct berlin_pllmap *map = pll->data;
> +	u32 val, fbdiv, rfdiv, vcodivsel;
> +
> +	val = berlin_pll_read(pll, PLL_CTRL0);
> +	fbdiv = (val >> map->fbdiv_shift) & map->fbdiv_mask;
> +	rfdiv = (val >> map->rfdiv_shift) & map->rfdiv_mask;
> +	if (rfdiv == 0)
> +		rfdiv = 1;
> +
> +	val = berlin_pll_read(pll, PLL_CTRL1);
> +	vcodivsel = (val >> map->divsel_shift) & map->divsel_mask;
> +
> +	return parent_rate * fbdiv * map->mult / rfdiv /
> +		map->vcodiv[vcodivsel];

IMHO

parent_rate *= fbdiv * map->mult;
parent_rate /= rfdiv;
return parent_rate / map->vcodiv[vcodivsel];

makes it slightly more readable, but that is up to you.

> +}
> +
> +static const struct clk_ops berlin_pll_ops = {
> +	.recalc_rate	= berlin_pll_recalc_rate,
> +};
> +
> +void __init berlin_pll_setup(struct device_node *np,
> +			struct berlin_pllmap *map)
> +{
> +	struct clk_init_data init;
> +	struct berlin_pll *pll;
> +	const char *parent_name;
> +	struct clk *clk;
> +	int ret;
> +
> +	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> +	if (WARN_ON(!pll))
> +		return;
> +
> +	pll->base = of_iomap(np, 0);
> +	if (WARN_ON(!pll->base))
> +		return;
> +
> +	pll->data = map;

pll->map = map;

> +
> +	init.name = np->name;
> +	init.ops = &berlin_pll_ops;
> +	parent_name = of_clk_get_parent_name(np, 0);
> +	init.parent_names = &parent_name;
> +	init.num_parents = 1;
> +
> +	pll->hw.init = &init;
> +
> +	clk = clk_register(NULL, &pll->hw);
> +	if (WARN_ON(IS_ERR(clk)))
> +		return;
> +
> +	ret = of_clk_add_provider(np, of_clk_src_simple_get, clk);
> +	if (WARN_ON(ret))
> +		return;
> +}
> +
>


  reply	other threads:[~2014-03-21 12:49 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-21 11:43 [PATCH 0/5] berlin: initial support for the clocks Alexandre Belloni
2014-03-21 11:43 ` Alexandre Belloni
2014-03-21 11:43 ` [PATCH 1/5] clk: berlin: add support for berlin plls Alexandre Belloni
2014-03-21 11:43   ` Alexandre Belloni
2014-03-21 12:49   ` Sebastian Hesselbarth [this message]
2014-03-21 12:49     ` Sebastian Hesselbarth
2014-03-21 15:45     ` Alexandre Belloni
2014-03-21 15:45       ` Alexandre Belloni
2014-03-21 15:58       ` Alexandre Belloni
2014-03-21 15:58         ` Alexandre Belloni
2014-03-21 16:03       ` Sebastian Hesselbarth
2014-03-21 16:03         ` Sebastian Hesselbarth
2014-03-21 16:01         ` Alexandre Belloni
2014-03-21 16:01           ` Alexandre Belloni
2014-03-21 15:56     ` Alexandre Belloni
2014-03-21 15:56       ` Alexandre Belloni
2014-03-21 16:04       ` Sebastian Hesselbarth
2014-03-21 16:04         ` Sebastian Hesselbarth
2014-03-21 11:43 ` [PATCH 2/5] clk: berlin: add berlin clocks DT bindings documentation Alexandre Belloni
2014-03-21 11:43   ` Alexandre Belloni
2014-03-21 11:43   ` Alexandre Belloni
2014-03-21 11:53   ` Mark Rutland
2014-03-21 11:53     ` Mark Rutland
2014-03-21 12:16   ` Sebastian Hesselbarth
2014-03-21 12:16     ` Sebastian Hesselbarth
2014-03-21 11:43 ` [PATCH 3/5] ARM: berlin/dt: add cpupll and syspll support to BG2Q Alexandre Belloni
2014-03-21 11:43   ` Alexandre Belloni
2014-03-21 12:11   ` Sebastian Hesselbarth
2014-03-21 12:11     ` Sebastian Hesselbarth
2014-03-21 12:17     ` Alexandre Belloni
2014-03-21 12:17       ` Alexandre Belloni
2014-03-21 12:29       ` Sebastian Hesselbarth
2014-03-21 12:29         ` Sebastian Hesselbarth
2014-03-21 14:54         ` Alexandre Belloni
2014-03-21 14:54           ` Alexandre Belloni
2014-03-21 11:43 ` [PATCH 4/5] ARM: berlin/dt: add cpupll and syspll support to BG2CD Alexandre Belloni
2014-03-21 11:43   ` Alexandre Belloni
2014-03-21 12:13   ` Sebastian Hesselbarth
2014-03-21 12:13     ` Sebastian Hesselbarth
2014-03-21 11:43 ` [PATCH 5/5] ARM: berlin/dt: add cpupll and syspll support to BG2 Alexandre Belloni
2014-03-21 11:43   ` Alexandre Belloni
2014-03-21 12:13   ` Sebastian Hesselbarth
2014-03-21 12:13     ` Sebastian Hesselbarth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=532C355C.3090606@gmail.com \
    --to=sebastian.hesselbarth@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.