[PATCH 02/16] clk: sunxi-ng: Add common infrastructure
Maxime Ripard
maxime.ripard at free-electrons.com
Sun May 15 11:31:22 PDT 2016
Hi,
On Mon, May 09, 2016 at 06:01:45PM +0800, Chen-Yu Tsai wrote:
> On Mon, May 9, 2016 at 4:01 AM, Maxime Ripard
> <maxime.ripard at free-electrons.com> wrote:
> > Start our new clock infrastructure by adding the registration code, common
> > structure and common code.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard at free-electrons.com>
> > ---
> > drivers/clk/Makefile | 1 +
> > drivers/clk/sunxi-ng/Makefile | 2 +
> > drivers/clk/sunxi-ng/ccu_common.c | 108 ++++++++++++++++++++++++++++++++++++++
> > drivers/clk/sunxi-ng/ccu_common.h | 74 ++++++++++++++++++++++++++
> > drivers/clk/sunxi-ng/ccu_factor.h | 15 ++++++
> > drivers/clk/sunxi-ng/ccu_mux.h | 20 +++++++
> > drivers/clk/sunxi-ng/ccu_reset.c | 55 +++++++++++++++++++
> > drivers/clk/sunxi-ng/ccu_reset.h | 40 ++++++++++++++
> > 8 files changed, 315 insertions(+)
> > create mode 100644 drivers/clk/sunxi-ng/Makefile
> > create mode 100644 drivers/clk/sunxi-ng/ccu_common.c
> > create mode 100644 drivers/clk/sunxi-ng/ccu_common.h
> > create mode 100644 drivers/clk/sunxi-ng/ccu_factor.h
> > create mode 100644 drivers/clk/sunxi-ng/ccu_mux.h
> > create mode 100644 drivers/clk/sunxi-ng/ccu_reset.c
> > create mode 100644 drivers/clk/sunxi-ng/ccu_reset.h
> >
> > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> > index 4ef71a13ab37..83a93cd9e21d 100644
> > --- a/drivers/clk/Makefile
> > +++ b/drivers/clk/Makefile
> > @@ -78,6 +78,7 @@ obj-$(CONFIG_ARCH_SOCFPGA) += socfpga/
> > obj-$(CONFIG_PLAT_SPEAR) += spear/
> > obj-$(CONFIG_ARCH_STI) += st/
> > obj-$(CONFIG_ARCH_SUNXI) += sunxi/
> > +obj-$(CONFIG_ARCH_SUNXI) += sunxi-ng/
> > obj-$(CONFIG_ARCH_TEGRA) += tegra/
> > obj-y += ti/
> > obj-$(CONFIG_ARCH_U8500) += ux500/
> > diff --git a/drivers/clk/sunxi-ng/Makefile b/drivers/clk/sunxi-ng/Makefile
> > new file mode 100644
> > index 000000000000..bd3461b0f38c
> > --- /dev/null
> > +++ b/drivers/clk/sunxi-ng/Makefile
> > @@ -0,0 +1,2 @@
> > +obj-y += ccu_common.o
> > +obj-y += ccu_reset.o
> > diff --git a/drivers/clk/sunxi-ng/ccu_common.c b/drivers/clk/sunxi-ng/ccu_common.c
> > new file mode 100644
> > index 000000000000..1d9242566fbd
> > --- /dev/null
> > +++ b/drivers/clk/sunxi-ng/ccu_common.c
> > @@ -0,0 +1,108 @@
> > +/*
> > + * Copyright 2016 Maxime Ripard
> > + *
> > + * Maxime Ripard <maxime.ripard at free-electrons.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that 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.
> > + */
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/of_address.h>
> > +#include <linux/slab.h>
> > +
> > +#include "ccu_common.h"
> > +#include "ccu_reset.h"
> > +
> > +static DEFINE_SPINLOCK(ccu_lock);
> > +
> > +void ccu_helper_wait_for_lock(struct ccu_common *common, u32 lock)
> > +{
> > + u32 reg;
> > +
> > + if (!(common->features & CCU_FEATURE_LOCK))
> > + return;
> > +
> > + WARN_ON(readl_relaxed_poll_timeout(common->base + common->reg, reg,
> > + !(reg & lock), 0, 500));
>
> no delay between reads? ^
Yes, I intended it to be a simple busy waiting loop since I don't
expect it to be very long. Do yu have any more data on how much time
it usually takes?
>
> > +int sunxi_ccu_probe(struct device_node *node,
> > + const struct sunxi_ccu_desc *desc)
> > +{
> > + struct ccu_common **cclks = desc->clks;
> > + size_t num_clks = desc->num_clks;
> > + struct clk_onecell_data *data;
> > + struct ccu_reset *reset;
> > + struct clk **clks;
> > + void __iomem *reg;
> > + int i, ret;
> > +
> > + reg = of_iomap(node, 0);
>
> Why not of_io_request_and_map?
Because initially I still had some old clocks that were probing, which
was leading to some issues. This is obviously not the case anymore,
I'll switch to it.
>
> > + if (IS_ERR(reg)) {
>
> And of_iomap returns NULL on error. This is for of_io_request_and_map.
>
> > + pr_err("%s: Could not map the clock registers\n",
> > + of_node_full_name(node));
> > + return PTR_ERR(reg);
> > + }
> > +
> > + data = kzalloc(sizeof(*data), GFP_KERNEL);
> > + if (!data)
> > + return -ENOMEM;
> > +
> > + clks = kcalloc(num_clks, sizeof(struct clk *), GFP_KERNEL);
> > + if (!clks)
> > + return -ENOMEM;
> > +
> > + data->clks = clks;
> > + data->clk_num = num_clks;
> > +
> > + for (i = 0; i < num_clks; i++) {
> > + struct ccu_common *cclk = cclks[i];
> > + struct clk *clk;
> > +
> > + if (!cclk) {
> > + cclk = ERR_PTR(-ENOENT);
>
> This seems redundant, unless you intended to use it elsewhere?
Yeah, that was supposed to be clks[i] = ERR_PTR(..);
I'll fix it.
>
> > + continue;
> > + }
> > +
> > + cclk->base = reg;
> > + cclk->lock = &ccu_lock;
> > +
> > + clk = clk_register(NULL, &cclk->hw);
> > + if (IS_ERR(clk))
> > + continue;
> > +
> > + clks[i] = clk;
> > + }
> > +
> > + ret = of_clk_add_provider(node, of_clk_src_onecell_get, data);
> > + if (ret)
> > + goto err_clk_unreg;
> > +
> > + reset = kzalloc(sizeof(*reset), GFP_KERNEL);
> > + reset->rcdev.of_node = node;
> > + reset->rcdev.ops = &ccu_reset_ops;
> > + reset->rcdev.owner = THIS_MODULE;
> > + reset->rcdev.nr_resets = desc->num_resets;
> > + reset->base = reg;
> > + reset->lock = &ccu_lock;
> > + reset->reset_map = desc->resets;
> > +
> > + ret = reset_controller_register(&reset->rcdev);
> > + if (ret)
> > + goto err_of_clk_unreg;
> > +
> > + return 0;
> > +
> > +err_of_clk_unreg:
> > +err_clk_unreg:
> > + return ret;
> > +}
> > diff --git a/drivers/clk/sunxi-ng/ccu_common.h b/drivers/clk/sunxi-ng/ccu_common.h
> > new file mode 100644
> > index 000000000000..e8b477fcd320
> > --- /dev/null
> > +++ b/drivers/clk/sunxi-ng/ccu_common.h
> > @@ -0,0 +1,74 @@
> > +/*
> > + * Copyright (c) 2016 Maxime Ripard. All rights reserved.
> > + *
> > + * This software is licensed under the terms of the GNU General Public
> > + * License version 2, as published by the Free Software Foundation, and
> > + * may be copied, distributed, and modified under those terms.
> > + *
> > + * This program is distributed in the hope that 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.
> > + */
> > +
> > +#ifndef _COMMON_H_
> > +#define _COMMON_H_
> > +
> > +#include <linux/compiler.h>
> > +#include <linux/clk-provider.h>
> > +
> > +#define CCU_FEATURE_GATE BIT(0)
> > +#define CCU_FEATURE_LOCK BIT(1)
>
> *_PLL_LOCK would be clearer that this implements a PLL lock indicator.
> Or maybe a comment.
I'll change it for PLL_LOCK
>
> > +#define CCU_FEATURE_FRACTIONAL BIT(2)
> > +#define CCU_FEATURE_VARIABLE_PREDIV BIT(3)
> > +#define CCU_FEATURE_FIXED_PREDIV BIT(4)
> > +#define CCU_FEATURE_FIXED_POSTDIV BIT(5)
> > +
> > +struct device_node;
> > +
> > +#define SUNXI_HW_INIT(_name, _parent, _ops, _flags) \
> > + &(struct clk_init_data) { \
> > + .flags = _flags, \
> > + .name = _name, \
> > + .parent_names = (const char *[]) { _parent }, \
> > + .num_parents = 1, \
> > + .ops = _ops, \
> > + }
> > +
> > +#define SUNXI_HW_INIT_PARENTS(_name, _parents, _ops, _flags) \
> > + &(struct clk_init_data) { \
> > + .flags = _flags, \
> > + .name = _name, \
> > + .parent_names = _parents, \
> > + .num_parents = ARRAY_SIZE(_parents), \
> > + .ops = _ops, \
> > + }
> > +
> > +struct ccu_common {
> > + void __iomem *base;
> > + unsigned long reg;
>
> This seems quite large, considering the address space of the CCU,
> and you using u16 or u32 for the same thing on the reset control side.
Indeed, what about u16?
>
> > +
> > + unsigned long features;
> > + spinlock_t *lock;
> > + struct clk_hw hw;
> > +};
> > +
> > +static inline struct ccu_common *hw_to_ccu_common(struct clk_hw *hw)
> > +{
> > + return container_of(hw, struct ccu_common, hw);
> > +}
> > +
> > +struct sunxi_ccu_desc {
> > + struct ccu_common **clks;
> > + unsigned long num_clks;
> > +
> > + struct ccu_reset_map *resets;
> > + unsigned long num_resets;
> > +};
> > +
> > +void ccu_helper_wait_for_lock(struct ccu_common *common, u32 lock);
> > +
> > +int sunxi_ccu_probe(struct device_node *node,
> > + const struct sunxi_ccu_desc *desc);
> > +
> > +#endif /* _COMMON_H_ */
> > diff --git a/drivers/clk/sunxi-ng/ccu_factor.h b/drivers/clk/sunxi-ng/ccu_factor.h
> > new file mode 100644
> > index 000000000000..e7cc564aaea0
> > --- /dev/null
> > +++ b/drivers/clk/sunxi-ng/ccu_factor.h
> > @@ -0,0 +1,15 @@
> > +#ifndef _CLK_FACTOR_H_
> > +#define _CLK_FACTOR_H_
> > +
> > +struct ccu_factor {
> > + u8 shift;
> > + u8 width;
> > +};
> > +
> > +#define SUNXI_CLK_FACTOR(_shift, _width) \
> > + { \
> > + .shift = _shift, \
> > + .width = _width, \
> > + }
> > +
> > +#endif /* _CLK_FACTOR_H_ */
> > diff --git a/drivers/clk/sunxi-ng/ccu_mux.h b/drivers/clk/sunxi-ng/ccu_mux.h
> > new file mode 100644
> > index 000000000000..17cedad4e433
> > --- /dev/null
> > +++ b/drivers/clk/sunxi-ng/ccu_mux.h
>
> As far as I can tell there are no users of this file within this patch or the
> following patches before the mux clock support one. It'd be easier to understand
> if this part was moved to the mux clock patch.
Will do.
>
> > @@ -0,0 +1,20 @@
> > +#ifndef _CCU_MUX_H_
> > +#define _CCU_MUX_H_
> > +
> > +#include "common.h"
> > +
> > +struct ccu_mux_internal {
> > + u8 shift;
> > + u8 width;
> > +
> > + u8 *map;
>
> I assume map is a table?
>
> > +};
> > +
> > +#define SUNXI_CLK_MUX(_shift, _width, _map) \
> > + { \
> > + .map = _map, \
> > + .shift = _shift, \
> > + .width = _width, \
> > + }
> > +
> > +#endif /* _CCU_MUX_H_ */
> > diff --git a/drivers/clk/sunxi-ng/ccu_reset.c b/drivers/clk/sunxi-ng/ccu_reset.c
> > new file mode 100644
> > index 000000000000..6c31d48783a7
> > --- /dev/null
> > +++ b/drivers/clk/sunxi-ng/ccu_reset.c
> > @@ -0,0 +1,55 @@
> > +/*
> > + * Copyright (C) 2016 Maxime Ripard
> > + * Maxime Ripard <maxime.ripard at free-electrons.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation; either version 2 of
> > + * the License, or (at your option) any later version.
> > + */
> > +
> > +#include <linux/io.h>
> > +#include <linux/reset-controller.h>
> > +
> > +#include "ccu_reset.h"
> > +
> > +static int ccu_reset_assert(struct reset_controller_dev *rcdev,
> > + unsigned long id)
> > +{
> > + struct ccu_reset *ccu = rcdev_to_ccu_reset(rcdev);
> > + const struct ccu_reset_map *map = &ccu->reset_map[id];
> > + unsigned long flags;
> > + u32 reg;
> > +
> > + spin_lock_irqsave(ccu->lock, flags);
> > +
> > + reg = readl(ccu->base + map->reg);
> > + writel(reg & ~map->bit, ccu->base + map->reg);
> > +
> > + spin_unlock_irqrestore(ccu->lock, flags);
> > +
> > + return 0;
> > +}
> > +
> > +static int ccu_reset_deassert(struct reset_controller_dev *rcdev,
> > + unsigned long id)
> > +{
> > + struct ccu_reset *ccu = rcdev_to_ccu_reset(rcdev);
> > + const struct ccu_reset_map *map = &ccu->reset_map[id];
> > + unsigned long flags;
> > + u32 reg;
> > +
> > + spin_lock_irqsave(ccu->lock, flags);
> > +
> > + reg = readl(ccu->base + map->reg);
> > + writel(reg | map->bit, ccu->base + map->reg);
> > +
> > + spin_unlock_irqrestore(ccu->lock, flags);
> > +
> > + return 0;
> > +}
> > +
> > +const struct reset_control_ops ccu_reset_ops = {
> > + .assert = ccu_reset_assert,
> > + .deassert = ccu_reset_deassert,
> > +};
> > diff --git a/drivers/clk/sunxi-ng/ccu_reset.h b/drivers/clk/sunxi-ng/ccu_reset.h
> > new file mode 100644
> > index 000000000000..36a4679210bd
> > --- /dev/null
> > +++ b/drivers/clk/sunxi-ng/ccu_reset.h
> > @@ -0,0 +1,40 @@
> > +/*
> > + * Copyright (c) 2016 Maxime Ripard. All rights reserved.
> > + *
> > + * This software is licensed under the terms of the GNU General Public
> > + * License version 2, as published by the Free Software Foundation, and
> > + * may be copied, distributed, and modified under those terms.
> > + *
> > + * This program is distributed in the hope that 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.
> > + */
> > +
> > +#ifndef _CCU_RESET_H_
> > +#define _CCU_RESET_H_
> > +
> > +#include <linux/reset-controller.h>
> > +
> > +struct ccu_reset_map {
> > + u16 reg;
> > + u32 bit;
> > +};
> > +
> > +
> > +struct ccu_reset {
> > + void __iomem *base;
> > + struct ccu_reset_map *reset_map;
> > + spinlock_t *lock;
> > +
> > + struct reset_controller_dev rcdev;
> > +};
> > +
> > +static inline struct ccu_reset *rcdev_to_ccu_reset(struct reset_controller_dev *rcdev)
> > +{
> > + return container_of(rcdev, struct ccu_reset, rcdev);
> > +}
> > +
> > +extern const struct reset_control_ops ccu_reset_ops;
> > +
> > +#endif /* _CCU_RESET_H_ */
>
> The reset control code looks good.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160515/f96ad50b/attachment.sig>
More information about the linux-arm-kernel
mailing list