[PATCH 02/16] clk: sunxi-ng: Add common infrastructure
Chen-Yu Tsai
wens at csie.org
Mon May 16 00:02:13 PDT 2016
On Mon, May 16, 2016 at 2:31 AM, Maxime Ripard
<maxime.ripard at free-electrons.com> wrote:
> 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?
Either u16 or u32 will do. I suspect u16 will get bumped up to u32 due to
alignment issues of the struct.
ChenYu
>>
>> > +
>> > + 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
More information about the linux-arm-kernel
mailing list