clk: sunxi: introduce "modern" clock support

The critical clock handling in __clk_core_init isn't taking the enable lock before calling clk_core_enable, which in turns triggers the warning in the lockdep_assert_held call in that function when lockep is enabled.

Add the calls to clk_enable_lock/unlock to make sure it doesn't happen.

Fixes 32b9b1096186 ("clk: Allow clocks to be marked as CRITICAL")

Signed-off-by Maxime Ripard <[email protected]>

---

 drivers/clk/clk.c | 7 +++++++
 1 file changed, 7 insertions(+)

drivers/clk/clk.c +0 -0

index ce39add5a258..16a38df3c688 100644

Quoted 4 lines · Maxime Ripard

The critical clock handling in __clk_core_init isn't taking the enable lock before calling clk_core_enable, which in turns triggers the warning in the lockdep_assert_held call in that function when lockep is enabled.

Quoted 5 lines

Add the calls to clk_enable_lock/unlock to make sure it doesn't happen.

Fixes 32b9b1096186 ("clk: Allow clocks to be marked as CRITICAL")

Signed-off-by Maxime Ripard <[email protected]>

---

Why is this patch hiding in this series?

Quoted 11 lines

 drivers/clk/clk.c | 7 +++++++
 1 file changed, 7 insertions(+)

drivers/clk/clk.c +0 -0

index ce39add5a258..16a38df3c688 100644
@@ -2404,8 +2404,15 @@ static int __clk_core_init(struct clk_core *core)
 		core->ops->init(core->hw);
 

drivers/clk/clk.c static int __clk_core_init(struct clk_core *core) · line 2411

Quoted 6 lines

drivers/clk/clk.c +4 -0

 	if (core->flags & CLK_IS_CRITICAL) {
+		unsigned long flags;
+
+		clk_prepare_lock();
 		clk_core_prepare(core);
+		clk_prepare_unlock();

It looks like we already hold the prepare lock at this point.

Start our new clock infrastructure by adding the registration code, common structure and common code.

Signed-off-by Maxime Ripard <[email protected]>

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

drivers/clk/Makefile +1 -0

index 4ef71a13ab37..83a93cd9e21d 100644
@@ -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/

drivers/clk/sunxi-ng/Makefile new file +2 -0

new file mode 100644
index 000000000000..bd3461b0f38c
@@ -0,0 +1,2 @@
+obj-y += ccu_common.o
+obj-y += ccu_reset.o

drivers/clk/sunxi-ng/ccu_common.c new file +25 -0

new file mode 100644
index 000000000000..1d9242566fbd
@@ -0,0 +1,108 @@
+/*
+ * Copyright 2016 Maxime Ripard
+ *
+ * Maxime Ripard <[email protected]>
+ *
+ * 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);

drivers/clk/sunxi-ng/ccu_common.c new file +4 -0

+}
+
+int sunxi_ccu_probe(struct device_node *node,
+                   const struct sunxi_ccu_desc *desc)

drivers/clk/sunxi-ng/ccu_common.c new file +12 -0

+               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;

drivers/clk/sunxi-ng/ccu_common.c new file +36 -0

+                       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;
+}

drivers/clk/sunxi-ng/ccu_common.h new file +11 -0

new file mode 100644
index 000000000000..e8b477fcd320
@@ -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.

drivers/clk/sunxi-ng/ccu_common.h new file +18 -0

+#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) {                                       \

drivers/clk/sunxi-ng/ccu_mux.h new file +10 -0

+};
+
+#define SUNXI_CLK_MUX(_shift, _width, _map)    \
+       {                                       \
+               .map    = _map,                 \
+               .shift  = _shift,               \
+               .width  = _width,               \
+       }
+
+#endif /* _CCU_MUX_H_ */

drivers/clk/sunxi-ng/ccu_reset.c new file +55 -0

new file mode 100644
index 000000000000..6c31d48783a7
@@ -0,0 +1,55 @@
+/*
+ * Copyright (C) 2016 Maxime Ripard
+ * Maxime Ripard <[email protected]>
+ *
+ * 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,
+};

drivers/clk/sunxi-ng/ccu_reset.h new file +30 -0

new file mode 100644
index 000000000000..36a4679210bd
@@ -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;

--
2.8.2

Quoted 77 lines · Maxime Ripard

Start our new clock infrastructure by adding the registration code, common structure and common code.

Signed-off-by Maxime Ripard <[email protected]>

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

drivers/clk/Makefile +1 -0

index 4ef71a13ab37..83a93cd9e21d 100644
@@ -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/

drivers/clk/sunxi-ng/Makefile new file +2 -0

new file mode 100644
index 000000000000..bd3461b0f38c
@@ -0,0 +1,2 @@
+obj-y += ccu_common.o
+obj-y += ccu_reset.o

drivers/clk/sunxi-ng/ccu_common.c new file +29 -0

new file mode 100644
index 000000000000..1d9242566fbd
@@ -0,0 +1,108 @@
+/*
+ * Copyright 2016 Maxime Ripard
+ *
+ * Maxime Ripard <[email protected]>
+ *
+ * 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;

drivers/clk/sunxi-ng/ccu_common.c line 35

Quoted 6 lines

drivers/clk/sunxi-ng/ccu_common.c new file +6 -0

+
+       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? ^

drivers/clk/sunxi-ng/ccu_common.c line 49

Quoted 13 lines

drivers/clk/sunxi-ng/ccu_common.c new file +12 -0

+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?

drivers/clk/sunxi-ng/ccu_common.c line 50

Quoted 2 lines

drivers/clk/sunxi-ng/ccu_common.c new file +1 -0

+       if (IS_ERR(reg)) {

And of_iomap returns NULL on error. This is for of_io_request_and_map.

Quoted 16 lines

drivers/clk/sunxi-ng/ccu_common.c new file +16 -0

+               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;
+

drivers/clk/sunxi-ng/ccu_common.c line 72

Quoted 6 lines

drivers/clk/sunxi-ng/ccu_common.c new file +6 -0

+       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?

Quoted 59 lines

drivers/clk/sunxi-ng/ccu_common.c new file +36 -0

+                       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;
+}

drivers/clk/sunxi-ng/ccu_common.h new file +16 -0

new file mode 100644
index 000000000000..e8b477fcd320
@@ -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_
+

drivers/clk/sunxi-ng/ccu_common.h line 21

Quoted 5 lines

drivers/clk/sunxi-ng/ccu_common.h new file +5 -0

+#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.

Quoted 24 lines

drivers/clk/sunxi-ng/ccu_common.h new file +23 -0

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

drivers/clk/sunxi-ng/ccu_common.h line 49

Quoted 5 lines

drivers/clk/sunxi-ng/ccu_common.h new file +5 -0

+       }
+
+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.

Quoted 48 lines

drivers/clk/sunxi-ng/ccu_common.h new file +25 -0

+
+       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_ */

drivers/clk/sunxi-ng/ccu_factor.h new file +15 -0

new file mode 100644
index 000000000000..e7cc564aaea0
@@ -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_ */

drivers/clk/sunxi-ng/ccu_mux.h +0 -0

drivers/clk/sunxi-ng/ccu_mux.h

Quoted 4 lines

new file mode 100644
index 000000000000..17cedad4e433

drivers/clk/sunxi-ng/ccu_mux.h new file +0 -0

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.

drivers/clk/sunxi-ng/ccu_mux.h line 10

Quoted 12 lines

drivers/clk/sunxi-ng/ccu_mux.h new file +10 -0

@@ -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?

Quoted 111 lines

drivers/clk/sunxi-ng/ccu_mux.h new file +10 -0

+};
+
+#define SUNXI_CLK_MUX(_shift, _width, _map)    \
+       {                                       \
+               .map    = _map,                 \
+               .shift  = _shift,               \
+               .width  = _width,               \
+       }
+
+#endif /* _CCU_MUX_H_ */

drivers/clk/sunxi-ng/ccu_reset.c new file +55 -0

new file mode 100644
index 000000000000..6c31d48783a7
@@ -0,0 +1,55 @@
+/*
+ * Copyright (C) 2016 Maxime Ripard
+ * Maxime Ripard <[email protected]>
+ *
+ * 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,
+};

drivers/clk/sunxi-ng/ccu_reset.h new file +34 -0

new file mode 100644
index 000000000000..36a4679210bd
@@ -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)
+{

drivers/clk/sunxi-ng/ccu_reset.h line 40

Quoted 6 lines

drivers/clk/sunxi-ng/ccu_reset.h new file +6 -0

+       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.

Hi,

Quoted 79 lines · Chen-Yu Tsai

Quoted 77 lines · Maxime Ripard

Start our new clock infrastructure by adding the registration code, common structure and common code.

Signed-off-by Maxime Ripard <[email protected]>

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

drivers/clk/Makefile +1 -0

index 4ef71a13ab37..83a93cd9e21d 100644
@@ -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/

drivers/clk/sunxi-ng/Makefile new file +2 -0

new file mode 100644
index 000000000000..bd3461b0f38c
@@ -0,0 +1,2 @@
+obj-y += ccu_common.o
+obj-y += ccu_reset.o

drivers/clk/sunxi-ng/ccu_common.c new file +29 -0

new file mode 100644
index 000000000000..1d9242566fbd
@@ -0,0 +1,108 @@
+/*
+ * Copyright 2016 Maxime Ripard
+ *
+ * Maxime Ripard <[email protected]>
+ *
+ * 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;

Quoted 8 lines

drivers/clk/sunxi-ng/ccu_common.c line 35

Quoted 6 lines

drivers/clk/sunxi-ng/ccu_common.c new file +6 -0

+
+       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?

Quoted 15 lines

drivers/clk/sunxi-ng/ccu_common.c line 49

Quoted 13 lines

drivers/clk/sunxi-ng/ccu_common.c new file +12 -0

+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.

Quoted 21 lines

drivers/clk/sunxi-ng/ccu_common.c line 50

Quoted 2 lines

drivers/clk/sunxi-ng/ccu_common.c new file +1 -0

+       if (IS_ERR(reg)) {

And of_iomap returns NULL on error. This is for of_io_request_and_map.

Quoted 16 lines

drivers/clk/sunxi-ng/ccu_common.c new file +16 -0

+               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;
+

Quoted 8 lines

drivers/clk/sunxi-ng/ccu_common.c line 72

Quoted 6 lines

drivers/clk/sunxi-ng/ccu_common.c new file +6 -0

+       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.

Quoted 59 lines

Quoted 59 lines

drivers/clk/sunxi-ng/ccu_common.c new file +36 -0

+                       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;
+}

drivers/clk/sunxi-ng/ccu_common.h new file +16 -0

new file mode 100644
index 000000000000..e8b477fcd320
@@ -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_
+

Quoted 8 lines

drivers/clk/sunxi-ng/ccu_common.h line 21

Quoted 5 lines

drivers/clk/sunxi-ng/ccu_common.h new file +5 -0

+#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

Quoted 24 lines

Quoted 24 lines

drivers/clk/sunxi-ng/ccu_common.h new file +23 -0

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

Quoted 8 lines

drivers/clk/sunxi-ng/ccu_common.h line 49

Quoted 5 lines

drivers/clk/sunxi-ng/ccu_common.h new file +5 -0

+       }
+
+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?

Quoted 48 lines

Quoted 48 lines

drivers/clk/sunxi-ng/ccu_common.h new file +25 -0

+
+       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_ */

drivers/clk/sunxi-ng/ccu_factor.h new file +15 -0

new file mode 100644
index 000000000000..e7cc564aaea0
@@ -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_ */

drivers/clk/sunxi-ng/ccu_mux.h +0 -0

Quoted 8 lines

drivers/clk/sunxi-ng/ccu_mux.h

Quoted 4 lines

new file mode 100644
index 000000000000..17cedad4e433

drivers/clk/sunxi-ng/ccu_mux.h new file +0 -0

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.

Quoted 126 lines

drivers/clk/sunxi-ng/ccu_mux.h line 10

Quoted 12 lines

drivers/clk/sunxi-ng/ccu_mux.h new file +10 -0

@@ -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?

Quoted 111 lines

drivers/clk/sunxi-ng/ccu_mux.h new file +10 -0

+};
+
+#define SUNXI_CLK_MUX(_shift, _width, _map)    \
+       {                                       \
+               .map    = _map,                 \
+               .shift  = _shift,               \
+               .width  = _width,               \
+       }
+
+#endif /* _CCU_MUX_H_ */

drivers/clk/sunxi-ng/ccu_reset.c new file +55 -0

new file mode 100644
index 000000000000..6c31d48783a7
@@ -0,0 +1,55 @@
+/*
+ * Copyright (C) 2016 Maxime Ripard
+ * Maxime Ripard <[email protected]>
+ *
+ * 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,
+};

drivers/clk/sunxi-ng/ccu_reset.h new file +34 -0

new file mode 100644
index 000000000000..36a4679210bd
@@ -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)
+{

Quoted 8 lines

drivers/clk/sunxi-ng/ccu_reset.h line 40

Quoted 6 lines

drivers/clk/sunxi-ng/ccu_reset.h new file +6 -0

+       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

Quoted 11 lines

drivers/clk/sunxi-ng/ccu_common.c line 35

Quoted 9 lines

drivers/clk/sunxi-ng/ccu_common.c new file +9 -0

+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?

Quoted 12 lines · On Sun, 15 May 2016 20:31:22 +0200 Maxime Ripard…

Quoted 11 lines

drivers/clk/sunxi-ng/ccu_common.c line 35

Quoted 9 lines

drivers/clk/sunxi-ng/ccu_common.c new file +9 -0

+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? ^

Quoted 3 lines

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?

I have a Soc in which the rate of the audio clock is stable after a good second.

Quoted 6 lines

Quoted 3 lines

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?

I have a Soc in which the rate of the audio clock is stable after a good second.

You mean before the clock is actually stable, or before the lock bit is cleared?

Which SoC is it? As far as I've seen, only the H3 allows to configure the stable time, and while by default it will take 16us, you can configure as high as 66ms (which is still way higher than the current limit).

Quoted 7 lines · On Mon, 16 May 2016 22:15:09 +0200 Maxime Ripard…

Quoted 6 lines

Quoted 3 lines

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?

I have a Soc in which the rate of the audio clock is stable after a good second.

Quoted 7 lines

You mean before the clock is actually stable, or before the lock bit is cleared?

Which SoC is it? As far as I've seen, only the H3 allows to configure the stable time, and while by default it will take 16us, you can configure as high as 66ms (which is still way higher than the current limit).

Hi Maxime,

Well, it is not a SoC, but an external chip, the SI5351 (in the Dove Cubox). There is no lock bit, but as it is used for audio, and as I did not set any delay at streaming start time, I can hear when the clock is stable.

Some clocks in the Allwinner SoCs clock units are direct, fixed, multipliers or dividers from their parent.

Add support for such clocks.

Signed-off-by Maxime Ripard <[email protected]>

---
 drivers/clk/sunxi-ng/Makefile           |  2 ++
 drivers/clk/sunxi-ng/ccu_fixed_factor.c | 42 +++++++++++++++++++++++++++
 drivers/clk/sunxi-ng/ccu_fixed_factor.h | 50 +++++++++++++++++++++++++++++++++
 3 files changed, 94 insertions(+)
 create mode 100644 drivers/clk/sunxi-ng/ccu_fixed_factor.c
 create mode 100644 drivers/clk/sunxi-ng/ccu_fixed_factor.h

drivers/clk/sunxi-ng/Makefile +2 -0

index bd3461b0f38c..d76276736765 100644
@@ -1,2 +1,4 @@
 obj-y += ccu_common.o
 obj-y += ccu_reset.o
+
+obj-y += ccu_fixed_factor.o

drivers/clk/sunxi-ng/ccu_fixed_factor.c new file +10 -0

new file mode 100644
index 000000000000..75df8a854db5
@@ -0,0 +1,42 @@
+/*
+ * Copyright (C) 2016 Maxime Ripard
+ * Maxime Ripard <[email protected]>
+ *
+ * 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.
+ */
+

drivers/clk/sunxi-ng/ccu_fixed_factor.c new file +13 -0

+}
+
+static int ccu_fixed_factor_set_rate(struct clk_hw *hw, unsigned long rate,
+                                    unsigned long parent_rate)
+{
+       return 0;
+}
+
+const struct clk_ops ccu_fixed_factor_ops = {
+       .recalc_rate    = ccu_fixed_factor_recalc_rate,
+       .round_rate     = ccu_fixed_factor_round_rate,
+       .set_rate       = ccu_fixed_factor_set_rate,
+};

drivers/clk/sunxi-ng/ccu_fixed_factor.h new file +50 -1

new file mode 100644
index 000000000000..4e53dbc9d10b
@@ -0,0 +1,50 @@
+/*
+ * 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_FIXED_FACTOR_H_
+#define _CCU_FIXED_FACTOR_H_
+
+#include <linux/clk-provider.h>
+
+#include "ccu_common.h"
+
+struct ccu_fixed_factor {
+       u16                     div;
+       u16                     mult;
+
+       struct ccu_common       common;
+};
+
+#define SUNXI_CCU_FIXED_FACTOR(_struct, _name, _parent,                        \
+                              _div, _mult, _flags)                     \
+       struct ccu_fixed_factor _struct = {                             \
+               .div    = _div,                                         \
+               .mult   = _mult,                                        \
+               .common = {                                             \
+                       .hw.init        = SUNXI_HW_INIT(_name,          \
+                                                       _parent,        \
+                                                       &ccu_fixed_factor_ops, \
+                                                       _flags),        \
+               },                                                      \
+       }
+
+static inline struct ccu_fixed_factor *hw_to_ccu_fixed_factor(struct clk_hw *hw)
+{
+       struct ccu_common *common = hw_to_ccu_common(hw);
+
+       return container_of(common, struct ccu_fixed_factor, common);
+}
+
+extern const struct clk_ops ccu_fixed_factor_ops;
+
+#endif /* _CCU_FIXED_FACTOR_H_ */
--

2.8.2

drivers/clk/sunxi-ng/ccu_fixed_factor.c line 29

Quoted 9 lines

drivers/clk/sunxi-ng/ccu_fixed_factor.c new file +9 -0

+}
+
+static long ccu_fixed_factor_round_rate(struct clk_hw *hw,
+                                       unsigned long rate,
+                                       unsigned long *parent_rate)
+{
+       struct ccu_fixed_factor *fix = hw_to_ccu_fixed_factor(hw);
+
+       return *parent_rate / fix->div * fix->mult;

Why is this the other way around? With integer math it shouldn't be interchangeable. (Though it seems clk_fixed_factor does the same...)

Also, clk_fixed_factor handles CLK_SET_RATE_PARENT. Do we need to do the same here?

Hi,

Quoted 44 lines · Maxime Ripard

Some clocks in the Allwinner SoCs clock units are direct, fixed, multipliers or dividers from their parent.

Add support for such clocks.

Signed-off-by Maxime Ripard <[email protected]>

---
 drivers/clk/sunxi-ng/Makefile           |  2 ++
 drivers/clk/sunxi-ng/ccu_fixed_factor.c | 42 +++++++++++++++++++++++++++
 drivers/clk/sunxi-ng/ccu_fixed_factor.h | 50 +++++++++++++++++++++++++++++++++
 3 files changed, 94 insertions(+)
 create mode 100644 drivers/clk/sunxi-ng/ccu_fixed_factor.c
 create mode 100644 drivers/clk/sunxi-ng/ccu_fixed_factor.h

drivers/clk/sunxi-ng/Makefile +2 -0

index bd3461b0f38c..d76276736765 100644
@@ -1,2 +1,4 @@
 obj-y += ccu_common.o
 obj-y += ccu_reset.o
+
+obj-y += ccu_fixed_factor.o

drivers/clk/sunxi-ng/ccu_fixed_factor.c new file +15 -0

new file mode 100644
index 000000000000..75df8a854db5
@@ -0,0 +1,42 @@
+/*
+ * Copyright (C) 2016 Maxime Ripard
+ * Maxime Ripard <[email protected]>
+ *
+ * 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/clk-provider.h>
+
+#include "ccu_fixed_factor.h"
+
+static unsigned long ccu_fixed_factor_recalc_rate(struct clk_hw *hw,

drivers/clk/sunxi-ng/ccu_fixed_factor.c line 20

Quoted 5 lines

drivers/clk/sunxi-ng/ccu_fixed_factor.c new file +5 -0

+                                                 unsigned long parent_rate)
+{
+       struct ccu_fixed_factor *fix = hw_to_ccu_fixed_factor(hw);
+
+       return parent_rate * fix->mult / fix->div;

do_div (from include/asm-generic/div64.h) is better, since this is an 64 bit value.

drivers/clk/sunxi-ng/ccu_fixed_factor.c line 29

Quoted 10 lines

drivers/clk/sunxi-ng/ccu_fixed_factor.c new file +9 -0

+}
+
+static long ccu_fixed_factor_round_rate(struct clk_hw *hw,
+                                       unsigned long rate,
+                                       unsigned long *parent_rate)
+{
+       struct ccu_fixed_factor *fix = hw_to_ccu_fixed_factor(hw);
+
+       return *parent_rate / fix->div * fix->mult;

Why is this the other way around? With integer math it shouldn't be interchangeable. (Though it seems clk_fixed_factor does the same...)

Also, clk_fixed_factor handles CLK_SET_RATE_PARENT. Do we need to do the same here?

Some clocks in the Allwinner SoCs clocks unit are just muxes.

However, those muxes might also be found in some other complicated clocks that would benefit from the code in there to deal with "advanced" features, like pre-dividers.

Introduce a set of helpers to reduce the code duplication in such cases.

Signed-off-by Maxime Ripard <[email protected]>

---
 drivers/clk/sunxi-ng/Makefile  |   1 +
 drivers/clk/sunxi-ng/ccu_mux.c | 187 +++++++++++++++++++++++++++++++++++++++++
 drivers/clk/sunxi-ng/ccu_mux.h |  80 +++++++++++++++++-
 3 files changed, 264 insertions(+), 4 deletions(-)
 create mode 100644 drivers/clk/sunxi-ng/ccu_mux.c

drivers/clk/sunxi-ng/Makefile +1 -0

index fc01127b3b45..aa5c411ff8ea 100644
@@ -3,3 +3,4 @@ obj-y += ccu_reset.o
 
 obj-y += ccu_fixed_factor.o
 obj-y += ccu_gate.o
+obj-y += ccu_mux.o

drivers/clk/sunxi-ng/ccu_mux.c new file +61 -0

new file mode 100644
index 000000000000..cb54a8931de3
@@ -0,0 +1,187 @@
+/*
+ * Copyright (C) 2016 Maxime Ripard
+ * Maxime Ripard <[email protected]>
+ *
+ * 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/clk-provider.h>
+
+#include "ccu_gate.h"
+#include "ccu_mux.h"
+
+void ccu_mux_helper_adjust_parent_for_prediv(struct ccu_common *common,
+                                            struct ccu_mux_internal *cm,
+                                            int parent_index,
+                                            unsigned long *parent_rate)
+{
+       u8 prediv = 1;
+       u32 reg;
+
+       if (!((common->features & CCU_FEATURE_FIXED_PREDIV) ||
+             (common->features & CCU_FEATURE_VARIABLE_PREDIV)))
+               return;
+
+       reg = readl(common->base + common->reg);
+       if (parent_index < 0) {
+               parent_index = reg >> cm->shift;
+               parent_index &= (1 << cm->width) - 1;
+       }
+
+       if (common->features & CCU_FEATURE_FIXED_PREDIV)
+               if (parent_index == cm->fixed_prediv.index)
+                       prediv = cm->fixed_prediv.div;
+
+       if (common->features & CCU_FEATURE_VARIABLE_PREDIV)
+               if (parent_index == cm->variable_prediv.index) {
+                       u8 div;
+
+                       div = reg >> cm->variable_prediv.shift;
+                       div &= (1 << cm->variable_prediv.width) - 1;
+                       prediv = div + 1;
+               }
+
+       *parent_rate = *parent_rate / prediv;
+}
+
+int ccu_mux_helper_determine_rate(struct ccu_common *common,
+                                 struct ccu_mux_internal *cm,
+                                 struct clk_rate_request *req,
+                                 unsigned long (*round)(struct ccu_mux_internal *,
+                                                        unsigned long,
+                                                        unsigned long,
+                                                        void *),
+                                 void *data)
+{
+       unsigned long best_parent_rate = 0, best_rate = 0;
+       struct clk_hw *best_parent, *hw = &common->hw;
+       unsigned int i;

drivers/clk/sunxi-ng/ccu_mux.c new file +109 -0

+                       best_rate = tmp_rate;
+                       goto out;
+               }
+
+               if ((req->rate - tmp_rate) < (req->rate - best_rate)) {
+                       best_rate = tmp_rate;
+                       best_parent_rate = parent_rate;
+                       best_parent = parent;
+               }
+       }
+
+       if (best_rate == 0)
+               return -EINVAL;
+
+out:
+       req->best_parent_hw = best_parent;
+       req->best_parent_rate = best_parent_rate;
+       req->rate = best_rate;
+       return 0;
+}
+
+u8 ccu_mux_helper_get_parent(struct ccu_common *common,
+                            struct ccu_mux_internal *cm)
+{
+       u32 reg;
+       u8 parent;
+
+       reg = readl(common->base + common->reg);
+       parent = reg >> cm->shift;
+       parent &= (1 << cm->width) - 1;
+
+       return parent;
+}
+
+int ccu_mux_helper_set_parent(struct ccu_common *common,
+                             struct ccu_mux_internal *cm,
+                             u8 index)
+{
+       unsigned long flags;
+       u32 reg;
+
+       spin_lock_irqsave(common->lock, flags);
+
+       reg = readl(common->base + common->reg);
+       reg &= ~GENMASK(cm->width + cm->shift, cm->shift);
+       writel(reg | (index << cm->shift), common->base + common->reg);
+
+       spin_unlock_irqrestore(common->lock, flags);
+
+       return 0;
+}
+
+static void ccu_mux_disable(struct clk_hw *hw)
+{
+       struct ccu_mux *cm = hw_to_ccu_mux(hw);
+
+       return ccu_gate_helper_disable(&cm->common, cm->enable);
+}
+
+static int ccu_mux_enable(struct clk_hw *hw)
+{
+       struct ccu_mux *cm = hw_to_ccu_mux(hw);
+
+       return ccu_gate_helper_enable(&cm->common, cm->enable);
+}
+
+static int ccu_mux_is_enabled(struct clk_hw *hw)
+{
+       struct ccu_mux *cm = hw_to_ccu_mux(hw);
+
+       return ccu_gate_helper_is_enabled(&cm->common, cm->enable);
+}
+
+static u8 ccu_mux_get_parent(struct clk_hw *hw)
+{
+       struct ccu_mux *cm = hw_to_ccu_mux(hw);
+
+       return ccu_mux_helper_get_parent(&cm->common, &cm->mux);
+}
+
+static int ccu_mux_set_parent(struct clk_hw *hw, u8 index)
+{
+       struct ccu_mux *cm = hw_to_ccu_mux(hw);
+
+       return ccu_mux_helper_set_parent(&cm->common, &cm->mux, index);
+}
+
+static unsigned long ccu_mux_recalc_rate(struct clk_hw *hw,
+                                        unsigned long parent_rate)
+{
+       struct ccu_mux *cm = hw_to_ccu_mux(hw);
+
+       ccu_mux_helper_adjust_parent_for_prediv(&cm->common, &cm->mux, -1,
+                                               &parent_rate);
+
+       return parent_rate;
+}
+
+const struct clk_ops ccu_mux_ops = {
+       .disable        = ccu_mux_disable,
+       .enable         = ccu_mux_enable,
+       .is_enabled     = ccu_mux_is_enabled,
+
+       .get_parent     = ccu_mux_get_parent,
+       .set_parent     = ccu_mux_set_parent,
+
+       .determine_rate = __clk_mux_determine_rate,
+       .recalc_rate    = ccu_mux_recalc_rate,
+};

drivers/clk/sunxi-ng/ccu_mux.h +76 -5

index 17cedad4e433..c85707f80f68 100644
@@ -1,20 +1,92 @@
 #ifndef _CCU_MUX_H_
 #define _CCU_MUX_H_
 
-#include "common.h"
+#include <linux/clk-provider.h>
+
+#include "ccu_common.h"
 
 struct ccu_mux_internal {
        u8      shift;
        u8      width;
 
-       u8      *map;
+       struct {
+               u8      index;
+               u8      div;
+       } fixed_prediv;
+
+       struct {
+               u8      index;
+               u8      shift;
+               u8      width;
+       } variable_prediv;
 };
 
-#define SUNXI_CLK_MUX(_shift, _width, _map)    \
+#define SUNXI_CLK_MUX(_shift, _width)  \
        {                                       \
-               .map    = _map,                 \
                .shift  = _shift,               \
                .width  = _width,               \
        }
 
+struct ccu_mux {
+       u16                     reg;
+       u32                     enable;
+
+       struct ccu_mux_internal mux;
+       struct ccu_common       common;
+};
+
+#define SUNXI_CCU_MUX(_struct, _name, _parents, _reg, _shift, _width, _flags) \
+       struct ccu_mux _struct = {                                      \
+               .mux    = SUNXI_CLK_MUX(_shift, _width),                \
+               .common = {                                             \
+                       .reg            = _reg,                         \
+                       .hw.init        = SUNXI_HW_INIT_PARENTS(_name,  \
+                                                               _parents, \
+                                                               &ccu_mux_ops, \
+                                                               _flags), \
+               }                                                       \
+       }
+
+#define SUNXI_CCU_MUX_WITH_GATE(_struct, _name, _parents, _reg,                \
+                               _shift, _width, _gate, _flags)          \
+       struct ccu_mux _struct = {                                      \
+               .enable = _gate,                                        \
+               .mux    = SUNXI_CLK_MUX(_shift, _width),                \
+               .common = {                                             \
+                       .reg            = _reg,                         \
+                       .features       = CCU_FEATURE_GATE,             \
+                       .hw.init        = SUNXI_HW_INIT_PARENTS(_name,  \
+                                                               _parents, \
+                                                               &ccu_mux_ops, \
+                                                               _flags), \
+               }                                                       \
+       }
+
+static inline struct ccu_mux *hw_to_ccu_mux(struct clk_hw *hw)
+{
+       struct ccu_common *common = hw_to_ccu_common(hw);
+
+       return container_of(common, struct ccu_mux, common);
+}
+
+extern const struct clk_ops ccu_mux_ops;
+
+void ccu_mux_helper_adjust_parent_for_prediv(struct ccu_common *common,
+                                            struct ccu_mux_internal *cm,
+                                            int parent_index,
+                                            unsigned long *parent_rate);
+int ccu_mux_helper_determine_rate(struct ccu_common *common,
+                                 struct ccu_mux_internal *cm,
+                                 struct clk_rate_request *req,
+                                 unsigned long (*round)(struct ccu_mux_internal *,
+                                                        unsigned long,
+                                                        unsigned long,
+                                                        void *),
+                                 void *data);
+u8 ccu_mux_helper_get_parent(struct ccu_common *common,
+                            struct ccu_mux_internal *cm);
+int ccu_mux_helper_set_parent(struct ccu_common *common,
+                             struct ccu_mux_internal *cm,
+                             u8 index);
+
 #endif /* _CCU_MUX_H_ */
--

2.8.2

Hi,

Quoted 92 lines · Maxime Ripard

Some clocks in the Allwinner SoCs clocks unit are just muxes.

However, those muxes might also be found in some other complicated clocks that would benefit from the code in there to deal with "advanced" features, like pre-dividers.

Introduce a set of helpers to reduce the code duplication in such cases.

Signed-off-by Maxime Ripard <[email protected]>

---
 drivers/clk/sunxi-ng/Makefile  |   1 +
 drivers/clk/sunxi-ng/ccu_mux.c | 187 +++++++++++++++++++++++++++++++++++++++++
 drivers/clk/sunxi-ng/ccu_mux.h |  80 +++++++++++++++++-
 3 files changed, 264 insertions(+), 4 deletions(-)
 create mode 100644 drivers/clk/sunxi-ng/ccu_mux.c

drivers/clk/sunxi-ng/Makefile +1 -0

index fc01127b3b45..aa5c411ff8ea 100644
@@ -3,3 +3,4 @@ obj-y += ccu_reset.o
 
 obj-y += ccu_fixed_factor.o
 obj-y += ccu_gate.o
+obj-y += ccu_mux.o

drivers/clk/sunxi-ng/ccu_mux.c new file +61 -0

new file mode 100644
index 000000000000..cb54a8931de3
@@ -0,0 +1,187 @@
+/*
+ * Copyright (C) 2016 Maxime Ripard
+ * Maxime Ripard <[email protected]>
+ *
+ * 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/clk-provider.h>
+
+#include "ccu_gate.h"
+#include "ccu_mux.h"
+
+void ccu_mux_helper_adjust_parent_for_prediv(struct ccu_common *common,
+                                            struct ccu_mux_internal *cm,
+                                            int parent_index,
+                                            unsigned long *parent_rate)
+{
+       u8 prediv = 1;
+       u32 reg;
+
+       if (!((common->features & CCU_FEATURE_FIXED_PREDIV) ||
+             (common->features & CCU_FEATURE_VARIABLE_PREDIV)))
+               return;
+
+       reg = readl(common->base + common->reg);
+       if (parent_index < 0) {
+               parent_index = reg >> cm->shift;
+               parent_index &= (1 << cm->width) - 1;
+       }
+
+       if (common->features & CCU_FEATURE_FIXED_PREDIV)
+               if (parent_index == cm->fixed_prediv.index)
+                       prediv = cm->fixed_prediv.div;
+
+       if (common->features & CCU_FEATURE_VARIABLE_PREDIV)
+               if (parent_index == cm->variable_prediv.index) {
+                       u8 div;
+
+                       div = reg >> cm->variable_prediv.shift;
+                       div &= (1 << cm->variable_prediv.width) - 1;
+                       prediv = div + 1;
+               }
+
+       *parent_rate = *parent_rate / prediv;
+}
+
+int ccu_mux_helper_determine_rate(struct ccu_common *common,
+                                 struct ccu_mux_internal *cm,
+                                 struct clk_rate_request *req,
+                                 unsigned long (*round)(struct ccu_mux_internal *,
+                                                        unsigned long,
+                                                        unsigned long,
+                                                        void *),
+                                 void *data)
+{
+       unsigned long best_parent_rate = 0, best_rate = 0;
+       struct clk_hw *best_parent, *hw = &common->hw;
+       unsigned int i;

drivers/clk/sunxi-ng/ccu_mux.c line 71

Quoted 10 lines

drivers/clk/sunxi-ng/ccu_mux.c new file +10 -0

+
+       for (i = 0; i < clk_hw_get_num_parents(hw); i++) {
+               unsigned long tmp_rate, parent_rate;
+               struct clk_hw *parent;
+
+               parent = clk_hw_get_parent_by_index(hw, i);
+               if (!parent)
+                       continue;
+
+               parent_rate = clk_hw_get_rate(parent);

Using clk-mux.c as a reference, you should honor CLK_SET_RATE_PARENT here.

drivers/clk/sunxi-ng/ccu_mux.c line 73

Quoted 2 lines

drivers/clk/sunxi-ng/ccu_mux.c new file +2 -0

+               ccu_mux_helper_adjust_parent_for_prediv(common, cm, i,
+                                                       &parent_rate);

ccu_mux_helper_adjust_parent_for_prediv can modify parent_rate. You should probably save a copy...

drivers/clk/sunxi-ng/ccu_mux.c line 78

Quoted 5 lines

drivers/clk/sunxi-ng/ccu_mux.c new file +5 -0

+
+               tmp_rate = round(cm, clk_hw_get_rate(parent), req->rate, data);
+               if (tmp_rate == req->rate) {
+                       best_parent = parent;
+                       best_parent_rate = parent_rate;

... to assign to best_parent_rate. The returned best_parent_rate is used to change the parent clock rate. This happens if CLK_SET_RATE_PARENT is set. The CCF doesn't know about our predivs, so you should pass back the original rate, not the one after the prediv.

I suppose you didn't run into problems as CLK_SET_RATE_PARENT was not used anywhere?

Add support for clocks based on a divider tables.

Signed-off-by Maxime Ripard <[email protected]>

---
 drivers/clk/sunxi-ng/Makefile        |   1 +
 drivers/clk/sunxi-ng/ccu_div_table.c | 117 +++++++++++++++++++++++++++++++++++
 drivers/clk/sunxi-ng/ccu_div_table.h |  75 ++++++++++++++++++++++
 3 files changed, 193 insertions(+)
 create mode 100644 drivers/clk/sunxi-ng/ccu_div_table.c
 create mode 100644 drivers/clk/sunxi-ng/ccu_div_table.h

drivers/clk/sunxi-ng/Makefile +1 -0

index aa5c411ff8ea..f20c6c8f217c 100644
@@ -1,6 +1,7 @@
 obj-y += ccu_common.o
 obj-y += ccu_reset.o
 
+obj-y += ccu_div_table.o
 obj-y += ccu_fixed_factor.o
 obj-y += ccu_gate.o
 obj-y += ccu_mux.o

drivers/clk/sunxi-ng/ccu_div_table.c new file +27 -0

new file mode 100644
index 000000000000..cbfff0bd47e3
@@ -0,0 +1,117 @@
+/*
+ * Copyright (C) 2016 Maxime Ripard
+ * Maxime Ripard <[email protected]>
+ *
+ * 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/clk-provider.h>
+
+#include "ccu_div_table.h"
+#include "ccu_gate.h"
+
+static void ccu_div_table_find_best(unsigned long parent, unsigned long rate,
+                                   struct ccu_div_table *ct,
+                                   unsigned int *index)
+{
+       unsigned long best_rate = 0;
+       unsigned int best_index = 0;
+       unsigned int _index;
+
+       for (_index = 0; _index <= ct->num_divs; _index++) {
+               unsigned long tmp_rate = parent / ct->table[_index];
+
+               if (tmp_rate > rate)

drivers/clk/sunxi-ng/ccu_div_table.c new file +25 -0

+
+static void ccu_div_table_disable(struct clk_hw *hw)
+{
+       struct ccu_div_table *ct = hw_to_ccu_div_table(hw);
+
+       return ccu_gate_helper_disable(&ct->common, ct->enable);
+}
+
+static int ccu_div_table_enable(struct clk_hw *hw)
+{
+       struct ccu_div_table *ct = hw_to_ccu_div_table(hw);
+
+       return ccu_gate_helper_enable(&ct->common, ct->enable);
+}
+
+static int ccu_div_table_is_enabled(struct clk_hw *hw)
+{
+       struct ccu_div_table *ct = hw_to_ccu_div_table(hw);
+
+       return ccu_gate_helper_is_enabled(&ct->common, ct->enable);
+}
+
+static unsigned long ccu_div_table_recalc_rate(struct clk_hw *hw,
+                                              unsigned long parent_rate)
+{

drivers/clk/sunxi-ng/ccu_div_table.c new file +22 -0

+
+       spin_lock_irqsave(ct->common.lock, flags);
+
+       reg = readl(ct->common.base + ct->common.reg);
+       reg &= ~GENMASK(ct->div.width + ct->div.shift, ct->div.shift);
+       writel(reg | ((ct->table[index]) << ct->div.shift),
+              ct->common.base + ct->common.reg);
+
+       spin_unlock_irqrestore(ct->common.lock, flags);
+
+       return 0;
+}
+
+const struct clk_ops ccu_div_table_ops = {
+       .disable        = ccu_div_table_disable,
+       .enable         = ccu_div_table_enable,
+       .is_enabled     = ccu_div_table_is_enabled,
+
+       .recalc_rate    = ccu_div_table_recalc_rate,
+       .round_rate     = ccu_div_table_round_rate,
+       .set_rate       = ccu_div_table_set_rate,
+};

drivers/clk/sunxi-ng/ccu_div_table.h new file +15 -0

new file mode 100644
index 000000000000..bd7da49087ed
@@ -0,0 +1,75 @@
+/*
+ * 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_DIV_TABLE_H_
+#define _CCU_DIV_TABLE_H_

drivers/clk/sunxi-ng/ccu_div_table.h new file +49 -1

+
+       struct ccu_factor       div;
+       struct ccu_common       common;
+};
+
+#define SUNXI_CCU_DIV_TABLE(_struct, _name, _parent, _reg,             \
+                           _shift, _width,                             \
+                           _table, _flags)                             \
+       struct ccu_div_table _struct = {                                \
+               .table          = _table,                               \
+               .num_divs       = ARRAY_SIZE(_table),                   \
+               .div            = SUNXI_CLK_FACTOR(_shift, _width),     \
+               .common = {                                             \
+                       .reg            = _reg,                         \
+                       .hw.init        = SUNXI_HW_INIT(_name,          \
+                                                       _parent,        \
+                                                       &ccu_div_table_ops, \
+                                                       _flags),        \
+               }                                                       \
+       }
+
+#define SUNXI_CCU_DIV_TABLE_WITH_GATE(_struct, _name, _parent, _reg,   \
+                                     _shift, _width,                   \
+                                     _table, _gate, _flags)            \
+       struct ccu_div_table _struct = {                                \
+               .enable         = _gate,                                \
+               .table          = _table,                               \
+               .num_divs       = ARRAY_SIZE(_table),                   \
+               .div            = SUNXI_CLK_FACTOR(_shift, _width),     \
+               .common = {                                             \
+                       .reg            = _reg,                         \
+                       .features       = CCU_FEATURE_GATE,             \
+                       .hw.init        = SUNXI_HW_INIT(_name,          \
+                                                       _parent,        \
+                                                       &ccu_div_table_ops, \
+                                                       _flags),        \
+               }                                                       \
+       }
+
+static inline struct ccu_div_table *hw_to_ccu_div_table(struct clk_hw *hw)
+{
+       struct ccu_common *common = hw_to_ccu_common(hw);
+
+       return container_of(common, struct ccu_div_table, common);
+}
+
+extern const struct clk_ops ccu_div_table_ops;
+
+#endif /* _CCU_DIV_H_ */
--

2.8.2

Add support for the clocks in the CCU that introduce a phase shift from their parent clock.

Signed-off-by Maxime Ripard <[email protected]>

---
 drivers/clk/sunxi-ng/Makefile    |   1 +
 drivers/clk/sunxi-ng/ccu_phase.c | 126 +++++++++++++++++++++++++++++++++++++++
 drivers/clk/sunxi-ng/ccu_phase.h |  50 ++++++++++++++++
 3 files changed, 177 insertions(+)
 create mode 100644 drivers/clk/sunxi-ng/ccu_phase.c
 create mode 100644 drivers/clk/sunxi-ng/ccu_phase.h

drivers/clk/sunxi-ng/Makefile +1 -0

index f20c6c8f217c..a47a3bbdf285 100644
@@ -5,3 +5,4 @@ obj-y += ccu_div_table.o
 obj-y += ccu_fixed_factor.o
 obj-y += ccu_gate.o
 obj-y += ccu_mux.o
+obj-y += ccu_phase.o

drivers/clk/sunxi-ng/ccu_phase.c new file +20 -0

new file mode 100644
index 000000000000..cf0f0b20115c
@@ -0,0 +1,126 @@
+/*
+ * Copyright (C) 2016 Maxime Ripard
+ * Maxime Ripard <[email protected]>
+ *
+ * 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/clk-provider.h>
+#include <linux/spinlock.h>
+
+#include "ccu_phase.h"
+
+static int ccu_phase_get_phase(struct clk_hw *hw)
+{
+       struct ccu_phase *phase = hw_to_ccu_phase(hw);
+       struct clk_hw *parent, *pparent;
+       unsigned int parent_rate, pparent_rate;

drivers/clk/sunxi-ng/ccu_phase.c new file +23 -0

+
+       /* Get our parent clock, it's the one that can adjust its rate */
+       parent = clk_hw_get_parent(hw);
+       if (!parent)
+               return -EINVAL;
+
+       /* And its rate */
+       parent_rate = clk_hw_get_rate(parent);
+       if (!parent_rate)
+               return -EINVAL;
+
+       /* Now, get our parent's parent (most likely some PLL) */
+       pparent = clk_hw_get_parent(parent);
+       if (!pparent)
+               return -EINVAL;
+
+       /* And its rate */
+       pparent_rate = clk_hw_get_rate(pparent);
+       if (!pparent_rate)
+               return -EINVAL;
+
+       /* Get our parent clock divider */
+       parent_div = pparent_rate / parent_rate;

drivers/clk/sunxi-ng/ccu_phase.c new file +35 -0

+       unsigned long flags;
+       u32 reg;
+       u8 delay;
+
+       /* Get our parent clock, it's the one that can adjust its rate */
+       parent = clk_hw_get_parent(hw);
+       if (!parent)
+               return -EINVAL;
+
+       /* And its rate */
+       parent_rate = clk_hw_get_rate(parent);
+       if (!parent_rate)
+               return -EINVAL;
+
+       /* Now, get our parent's parent (most likely some PLL) */
+       pparent = clk_hw_get_parent(parent);
+       if (!pparent)
+               return -EINVAL;
+
+       /* And its rate */
+       pparent_rate = clk_hw_get_rate(pparent);
+       if (!pparent_rate)
+               return -EINVAL;
+
+       if (degrees != 180) {
+               u16 step, parent_div;
+
+               /* Get our parent divider */
+               parent_div = pparent_rate / parent_rate;
+
+               /*
+                * We can only outphase the clocks by multiple of the
+                * PLL's period.
+                *
+                * Since our parent clock is only a divider, and the

drivers/clk/sunxi-ng/ccu_phase.c new file +18 -0

+       } else {
+               delay = 0;
+       }
+
+       spin_lock_irqsave(phase->common.lock, flags);
+       reg = readl(phase->common.base + phase->common.reg);
+       reg &= ~GENMASK(phase->width + phase->shift, phase->shift);
+       writel(reg | (delay << phase->shift),
+              phase->common.base + phase->common.reg);
+       spin_unlock_irqrestore(phase->common.lock, flags);
+
+       return 0;
+}
+
+const struct clk_ops ccu_phase_ops = {
+       .get_phase      = ccu_phase_get_phase,
+       .set_phase      = ccu_phase_set_phase,
+};

drivers/clk/sunxi-ng/ccu_phase.h new file +13 -0

new file mode 100644
index 000000000000..e28b4e58a819
@@ -0,0 +1,50 @@
+/*
+ * 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.
+ */
+

drivers/clk/sunxi-ng/ccu_phase.h new file +27 -1

+
+       struct ccu_common       common;
+};
+
+#define SUNXI_CCU_PHASE(_struct, _name, _parent, _reg, _shift, _width, _flags) \
+       struct ccu_phase _struct = {                                    \
+               .shift  = _shift,                                       \
+               .width  = _width,                                       \
+               .common = {                                             \
+                       .reg            = _reg,                         \
+                       .hw.init        = SUNXI_HW_INIT(_name,          \
+                                                       _parent,        \
+                                                       &ccu_phase_ops, \
+                                                       _flags),        \
+               }                                                       \
+       }
+
+static inline struct ccu_phase *hw_to_ccu_phase(struct clk_hw *hw)
+{
+       struct ccu_common *common = hw_to_ccu_common(hw);
+
+       return container_of(common, struct ccu_phase, common);
+}
+
+extern const struct clk_ops ccu_phase_ops;
+
+#endif /* _CCU_PHASE_H_ */
--

2.8.2

drivers/clk/sunxi-ng/ccu_phase.c line 63

Quoted 5 lines

drivers/clk/sunxi-ng/ccu_phase.c new file +5 -0

+static int ccu_phase_set_phase(struct clk_hw *hw, int degrees)
+{
+       struct ccu_phase *phase = hw_to_ccu_phase(hw);
+       struct clk_hw *parent, *pparent;
+       unsigned int parent_rate, pparent_rate;

grandparent(_rate) would be easier to understand.

Quoted 36 lines

drivers/clk/sunxi-ng/ccu_phase.c new file +35 -0

+       unsigned long flags;
+       u32 reg;
+       u8 delay;
+
+       /* Get our parent clock, it's the one that can adjust its rate */
+       parent = clk_hw_get_parent(hw);
+       if (!parent)
+               return -EINVAL;
+
+       /* And its rate */
+       parent_rate = clk_hw_get_rate(parent);
+       if (!parent_rate)
+               return -EINVAL;
+
+       /* Now, get our parent's parent (most likely some PLL) */
+       pparent = clk_hw_get_parent(parent);
+       if (!pparent)
+               return -EINVAL;
+
+       /* And its rate */
+       pparent_rate = clk_hw_get_rate(pparent);
+       if (!pparent_rate)
+               return -EINVAL;
+
+       if (degrees != 180) {
+               u16 step, parent_div;
+
+               /* Get our parent divider */
+               parent_div = pparent_rate / parent_rate;
+
+               /*
+                * We can only outphase the clocks by multiple of the
+                * PLL's period.
+                *
+                * Since our parent clock is only a divider, and the

drivers/clk/sunxi-ng/ccu_phase.c line 108

Quoted 10 lines

drivers/clk/sunxi-ng/ccu_phase.c new file +10 -0

+                * formula to get the outphasing in degrees is deg =
+                * 360 * delta / period
+                *
+                * If we simplify this formula, we can see that the
+                * only thing that we're concerned about is the number
+                * of period we want to outphase our clock from, and
+                * the divider set by our parent clock.
+                */
+               step = DIV_ROUND_CLOSEST(360, parent_div);
+               delay = DIV_ROUND_CLOSEST(degrees, step);

Doesn't this mean some delay values are impossible to set?

For instance, for PLL = 600 MHz and this clock = 50 MHz, div would be 12, and a step would be 30 degrees. This means we can't ask for a delay of 6, which is 180 degrees.

For PLL = 600 MHz, clock = 100 MHz, div would be 6, and a step is 60 degrees. Therefor we can't ask for a delay of 3.

drivers/clk/sunxi-ng/ccu_phase.h line 23

Quoted 3 lines

drivers/clk/sunxi-ng/ccu_phase.h new file +3 -0

+struct ccu_phase {
+       u8                      shift;
+       u8                      width;

Not sure why you used struct ccu_factor in the divider table clock, but separate fields directly in ccu_phase here.

Hi,

Quoted 7 lines · Chen-Yu Tsai

drivers/clk/sunxi-ng/ccu_phase.c line 63

Quoted 5 lines

drivers/clk/sunxi-ng/ccu_phase.c new file +5 -0

+static int ccu_phase_set_phase(struct clk_hw *hw, int degrees)
+{
+       struct ccu_phase *phase = hw_to_ccu_phase(hw);
+       struct clk_hw *parent, *pparent;
+       unsigned int parent_rate, pparent_rate;

grandparent(_rate) would be easier to understand.

Ack.

Quoted 49 lines

Quoted 36 lines

drivers/clk/sunxi-ng/ccu_phase.c new file +35 -0

+       unsigned long flags;
+       u32 reg;
+       u8 delay;
+
+       /* Get our parent clock, it's the one that can adjust its rate */
+       parent = clk_hw_get_parent(hw);
+       if (!parent)
+               return -EINVAL;
+
+       /* And its rate */
+       parent_rate = clk_hw_get_rate(parent);
+       if (!parent_rate)
+               return -EINVAL;
+
+       /* Now, get our parent's parent (most likely some PLL) */
+       pparent = clk_hw_get_parent(parent);
+       if (!pparent)
+               return -EINVAL;
+
+       /* And its rate */
+       pparent_rate = clk_hw_get_rate(pparent);
+       if (!pparent_rate)
+               return -EINVAL;
+
+       if (degrees != 180) {
+               u16 step, parent_div;
+
+               /* Get our parent divider */
+               parent_div = pparent_rate / parent_rate;
+
+               /*
+                * We can only outphase the clocks by multiple of the
+                * PLL's period.
+                *
+                * Since our parent clock is only a divider, and the

drivers/clk/sunxi-ng/ccu_phase.c line 108

Quoted 10 lines

drivers/clk/sunxi-ng/ccu_phase.c new file +10 -0

+                * formula to get the outphasing in degrees is deg =
+                * 360 * delta / period
+                *
+                * If we simplify this formula, we can see that the
+                * only thing that we're concerned about is the number
+                * of period we want to outphase our clock from, and
+                * the divider set by our parent clock.
+                */
+               step = DIV_ROUND_CLOSEST(360, parent_div);
+               delay = DIV_ROUND_CLOSEST(degrees, step);

Doesn't this mean some delay values are impossible to set?

Quoted 6 lines

For instance, for PLL = 600 MHz and this clock = 50 MHz, div would be 12, and a step would be 30 degrees. This means we can't ask for a delay of 6, which is 180 degrees.

For PLL = 600 MHz, clock = 100 MHz, div would be 6, and a step is 60 degrees. Therefor we can't ask for a delay of 3.

You don't ask for a delay, you ask for an outphasing in degrees. In the hardware, in the register 0 means an outphasing of 180 degrees (and this has been confirmed by Allwinner a while back). In the two cases you point out, we would have two ways of achieving the same thing, we prefer one over another, but I don't see how it's problematic.

It's also a direct copy of the current code we have, which didn't raise any objection, or had any known bugs.

Quoted 7 lines

drivers/clk/sunxi-ng/ccu_phase.h line 23

Quoted 4 lines

drivers/clk/sunxi-ng/ccu_phase.h new file +3 -0

+struct ccu_phase {
+       u8                      shift;
+       u8                      width;

Not sure why you used struct ccu_factor in the divider table clock, but separate fields directly in ccu_phase here.

Because this is not meant for the same thing. ccu_factor is probably going to go away anyway because of the dividers consolidation.

Thanks!
Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Introduce support for clocks that divide by a linear factor.

Signed-off-by Maxime Ripard <[email protected]>

---
 drivers/clk/sunxi-ng/Makefile |   1 +
 drivers/clk/sunxi-ng/ccu_m.c  | 135 ++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/sunxi-ng/ccu_m.h  | 101 +++++++++++++++++++++++++++++++
 3 files changed, 237 insertions(+)
 create mode 100644 drivers/clk/sunxi-ng/ccu_m.c
 create mode 100644 drivers/clk/sunxi-ng/ccu_m.h

drivers/clk/sunxi-ng/Makefile +1 -0

index a47a3bbdf285..f41de901c607 100644
@@ -4,5 +4,6 @@ obj-y += ccu_reset.o
 obj-y += ccu_div_table.o
 obj-y += ccu_fixed_factor.o
 obj-y += ccu_gate.o
+obj-y += ccu_m.o
 obj-y += ccu_mux.o
 obj-y += ccu_phase.o

drivers/clk/sunxi-ng/ccu_m.c new file +0 -0

new file mode 100644
index 000000000000..424eb6da0d5b

drivers/clk/sunxi-ng/ccu_m.c new file +4 -0

+static int ccu_m_set_rate(struct clk_hw *hw, unsigned long rate,
+			  unsigned long parent_rate)
+{
+	struct ccu_m *cm = hw_to_ccu_m(hw);

Quoted 28 lines · On Sun, 8 May 2016 22:01:43 +0200 Maxime Ripard…

Introduce support for clocks that divide by a linear factor.

Signed-off-by Maxime Ripard <[email protected]>

---
 drivers/clk/sunxi-ng/Makefile |   1 +
 drivers/clk/sunxi-ng/ccu_m.c  | 135 ++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/sunxi-ng/ccu_m.h  | 101 +++++++++++++++++++++++++++++++
 3 files changed, 237 insertions(+)
 create mode 100644 drivers/clk/sunxi-ng/ccu_m.c
 create mode 100644 drivers/clk/sunxi-ng/ccu_m.h

drivers/clk/sunxi-ng/Makefile +1 -0

index a47a3bbdf285..f41de901c607 100644
@@ -4,5 +4,6 @@ obj-y += ccu_reset.o
 obj-y += ccu_div_table.o
 obj-y += ccu_fixed_factor.o
 obj-y += ccu_gate.o
+obj-y += ccu_m.o
 obj-y += ccu_mux.o
 obj-y += ccu_phase.o

drivers/clk/sunxi-ng/ccu_m.c new file +0 -0

new file mode 100644
index 000000000000..424eb6da0d5b

drivers/clk/sunxi-ng/ccu_m.c line 9

Quoted 9 lines

drivers/clk/sunxi-ng/ccu_m.c new file +9 -0

@@ -0,0 +1,135 @@
+/*
+ * Copyright (C) 2016 Maxime Ripard
+ * Maxime Ripard <[email protected]>
+ *
+ * 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.
+ */

	[snip]

Quoted 4 lines

drivers/clk/sunxi-ng/ccu_m.c new file +4 -0

+static int ccu_m_set_rate(struct clk_hw *hw, unsigned long rate,
+			  unsigned long parent_rate)
+{
+	struct ccu_m *cm = hw_to_ccu_m(hw);

drivers/clk/sunxi-ng/ccu_m.c line 100

Quoted 10 lines

drivers/clk/sunxi-ng/ccu_m.c new file +10 -0

+	unsigned long flags;
+	unsigned int m;
+	u32 reg;
+
+	ccu_m_find_best(parent_rate, rate, 1 << cm->m.width, &m);
+
+	spin_lock_irqsave(cm->common.lock, flags);
+
+	reg = readl(cm->common.base + cm->common.reg);
+	reg &= ((1 << cm->m.width) - 1) << cm->m.shift;

Bug:

	reg &= ~GENMASK(cm->m.width + cm->m.shift, cm->m.shift);

Introduce support for clocks that divide by a linear factor.

Signed-off-by Maxime Ripard <[email protected]>

---
 drivers/clk/sunxi-ng/Makefile |   1 +
 drivers/clk/sunxi-ng/ccu_m.c  | 135 ++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/sunxi-ng/ccu_m.h  | 101 +++++++++++++++++++++++++++++++
 3 files changed, 237 insertions(+)
 create mode 100644 drivers/clk/sunxi-ng/ccu_m.c
 create mode 100644 drivers/clk/sunxi-ng/ccu_m.h

drivers/clk/sunxi-ng/Makefile +1 -0

index a47a3bbdf285..f41de901c607 100644
@@ -4,5 +4,6 @@ obj-y += ccu_reset.o
 obj-y += ccu_div_table.o
 obj-y += ccu_fixed_factor.o
 obj-y += ccu_gate.o
+obj-y += ccu_m.o
 obj-y += ccu_mux.o
 obj-y += ccu_phase.o

drivers/clk/sunxi-ng/ccu_m.c new file +16 -0

new file mode 100644
index 000000000000..424eb6da0d5b
@@ -0,0 +1,135 @@
+/*
+ * Copyright (C) 2016 Maxime Ripard
+ * Maxime Ripard <[email protected]>
+ *
+ * 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/clk-provider.h>
+
+#include "ccu_gate.h"
+#include "ccu_m.h"
+#include "ccu_mux.h"
+

drivers/clk/sunxi-ng/ccu_m.c new file +27 -0

+}
+
+
+static void ccu_m_disable(struct clk_hw *hw)
+{
+       struct ccu_m *cm = hw_to_ccu_m(hw);
+
+       return ccu_gate_helper_disable(&cm->common, cm->enable);
+}
+
+static int ccu_m_enable(struct clk_hw *hw)
+{
+       struct ccu_m *cm = hw_to_ccu_m(hw);
+
+       return ccu_gate_helper_enable(&cm->common, cm->enable);
+}
+
+static int ccu_m_is_enabled(struct clk_hw *hw)
+{
+       struct ccu_m *cm = hw_to_ccu_m(hw);
+
+       return ccu_gate_helper_is_enabled(&cm->common, cm->enable);
+}
+
+static unsigned long ccu_m_recalc_rate(struct clk_hw *hw,
+                                      unsigned long parent_rate)
+{

drivers/clk/sunxi-ng/ccu_m.c new file +10 -0

+}
+
+static int ccu_m_determine_rate(struct clk_hw *hw,
+                               struct clk_rate_request *req)
+{
+       struct ccu_m *cm = hw_to_ccu_m(hw);
+
+       return ccu_mux_helper_determine_rate(&cm->common, &cm->mux,
+                                            req, ccu_m_round_rate, cm);
+}

drivers/clk/sunxi-ng/ccu_m.c new file +40 -0

+
+       spin_lock_irqsave(cm->common.lock, flags);
+
+       reg = readl(cm->common.base + cm->common.reg);
+       reg &= ((1 << cm->m.width) - 1) << cm->m.shift;
+
+       writel(reg | ((m - 1) << cm->m.shift),
+              cm->common.base + cm->common.reg);
+
+       spin_unlock_irqrestore(cm->common.lock, flags);
+
+       return 0;
+}
+
+static u8 ccu_m_get_parent(struct clk_hw *hw)
+{
+       struct ccu_m *cm = hw_to_ccu_m(hw);
+
+       return ccu_mux_helper_get_parent(&cm->common, &cm->mux);
+}
+
+static int ccu_m_set_parent(struct clk_hw *hw, u8 index)
+{
+       struct ccu_m *cm = hw_to_ccu_m(hw);
+
+       return ccu_mux_helper_set_parent(&cm->common, &cm->mux, index);
+}
+
+const struct clk_ops ccu_m_ops = {
+       .disable        = ccu_m_disable,
+       .enable         = ccu_m_enable,
+       .is_enabled     = ccu_m_is_enabled,
+
+       .get_parent     = ccu_m_get_parent,
+       .set_parent     = ccu_m_set_parent,
+
+       .determine_rate = ccu_m_determine_rate,
+       .recalc_rate    = ccu_m_recalc_rate,
+       .set_rate       = ccu_m_set_rate,
+};

drivers/clk/sunxi-ng/ccu_m.h new file +14 -0

new file mode 100644
index 000000000000..625c0a7cef43
@@ -0,0 +1,101 @@
+/*
+ * 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_M_H_