Re: [PATCH v3 10/12] riscv: Add K210 clock support
Hi Sean,
> Due to the large number of clocks, I decided to use the CCF. The
> overall structure is modeled after the imx code. A common pattern is
> to create a composite clock composed of several component clocks. For
> these component clocks, the clk_register_* functions are not used,
> since they will be registered as part of the composite clock. To
> create these component clocks, several helper k210_clk_comp_*
> functions are used. This functionality seems like it would be useful
> to other drivers also creating composite clocks, so perhaps some
> general versions should be created. I am not particularly attached to
> the naming convention, suggestions are welcome.
>
> Signed-off-by: Sean Anderson
Reviewed-by: Lukasz Majewski
> ---
> Changes for v3:
> - Removed sysctl struct, replacing it with defines. This is to have
> the same interface to sysctl from C as from the device tree.
> - Fixed clocks having the same id
> - Fixed clocks not using the correct register/bits
> - Aligned the defines in headers
> Changes for v2:
> - Add clk.o to obj-y
> - Don't probe before relocation
>
> drivers/clk/kendryte/Kconfig| 2 +-
> drivers/clk/kendryte/Makefile | 2 +-
> drivers/clk/kendryte/clk.c | 390
> drivers/clk/kendryte/clk.h |
> 27 ++ include/dt-bindings/clock/k210-sysctl.h | 53
> include/dt-bindings/mfd/k210-sysctl.h | 38 +++
> 6 files changed, 510 insertions(+), 2 deletions(-)
> create mode 100644 drivers/clk/kendryte/clk.c
> create mode 100644 drivers/clk/kendryte/clk.h
> create mode 100644 include/dt-bindings/clock/k210-sysctl.h
> create mode 100644 include/dt-bindings/mfd/k210-sysctl.h
>
> diff --git a/drivers/clk/kendryte/Kconfig
> b/drivers/clk/kendryte/Kconfig index 7b69c8afaf..073fca0781 100644
> --- a/drivers/clk/kendryte/Kconfig
> +++ b/drivers/clk/kendryte/Kconfig
> @@ -1,6 +1,6 @@
> config CLK_K210
> bool "Clock support for Kendryte K210"
> - depends on CLK && CLK_CCF
> + depends on CLK && CLK_CCF && CLK_COMPOSITE_CCF
> help
> This enables support clock driver for Kendryte K210
> platforms.
> diff --git a/drivers/clk/kendryte/Makefile
> b/drivers/clk/kendryte/Makefile index c56d93ea1c..d26bce954f 100644
> --- a/drivers/clk/kendryte/Makefile
> +++ b/drivers/clk/kendryte/Makefile
> @@ -1 +1 @@
> -obj-y += pll.o
> +obj-y += clk.o pll.o
> diff --git a/drivers/clk/kendryte/clk.c b/drivers/clk/kendryte/clk.c
> new file mode 100644
> index 00..aaa7420c84
> --- /dev/null
> +++ b/drivers/clk/kendryte/clk.c
> @@ -0,0 +1,390 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019 Sean Anderson
> + */
> +#include "clk.h"
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "pll.h"
> +
> +static ulong k210_clk_get_rate(struct clk *clk)
> +{
> + struct clk *c;
> + int err = clk_get_by_id(clk->id, );
> +
> + if (err)
> + return err;
> + return clk_get_rate(c);
> +}
> +
> +static ulong k210_clk_set_rate(struct clk *clk, unsigned long rate)
> +{
> + struct clk *c;
> + int err = clk_get_by_id(clk->id, );
> +
> + if (err)
> + return err;
> + return clk_set_rate(c, rate);
> +}
> +
> +static int k210_clk_set_parent(struct clk *clk, struct clk *parent)
> +{
> + struct clk *c, *p;
> + int err = clk_get_by_id(clk->id, );
> +
> + if (err)
> + return err;
> +
> + err = clk_get_by_id(parent->id, );
> + if (err)
> + return err;
> +
> + return clk_set_parent(c, p);
> +}
> +
> +static int k210_clk_endisable(struct clk *clk, bool enable)
> +{
> + struct clk *c;
> + int err = clk_get_by_id(clk->id, );
> +
> + if (err)
> + return err;
> + return enable ? clk_enable(c) : clk_disable(c);
> +}
> +
> +static int k210_clk_enable(struct clk *clk)
> +{
> + return k210_clk_endisable(clk, true);
> +}
> +
> +static int k210_clk_disable(struct clk *clk)
> +{
> + return k210_clk_endisable(clk, false);
> +}
> +
> +static const struct clk_ops k210_clk_ops = {
> + .set_rate = k210_clk_set_rate,
> + .get_rate = k210_clk_get_rate,
> + .set_parent = k210_clk_set_parent,
> + .enable = k210_clk_enable,
> + .disable = k210_clk_disable,
> +};
> +
> +/* The first clock is in0, which is filled in by k210_clk_probe */
> +static const char *generic_sels[] = { NULL, "pll0", };
> +static const char *aclk_sels[] = { "in0_half", "pll0_half", };
> +static const char *pll2_sels[] = { NULL, "pll0", "pll1", };
> +
> +static struct clk_divider *k210_clk_comp_div_flags(void __iomem
> *reg, u8 shift,
> +u8 width, u8
> flags) +{
> + struct clk_divider *div;
> +
> + div = kzalloc(sizeof(*div), GFP_KERNEL);
> + if (!div)
> + return div;
> + div->reg = reg;
> + div->shift = shift;
> + div->width = width;
> +
[PATCH v3 10/12] riscv: Add K210 clock support
Due to the large number of clocks, I decided to use the CCF. The overall
structure is modeled after the imx code. A common pattern is to create a
composite clock composed of several component clocks. For these component
clocks, the clk_register_* functions are not used, since they will be registered
as part of the composite clock. To create these component clocks, several helper
k210_clk_comp_* functions are used. This functionality seems like it would be
useful to other drivers also creating composite clocks, so perhaps some general
versions should be created. I am not particularly attached to the naming
convention, suggestions are welcome.
Signed-off-by: Sean Anderson
---
Changes for v3:
- Removed sysctl struct, replacing it with defines. This is to have the same
interface to sysctl from C as from the device tree.
- Fixed clocks having the same id
- Fixed clocks not using the correct register/bits
- Aligned the defines in headers
Changes for v2:
- Add clk.o to obj-y
- Don't probe before relocation
drivers/clk/kendryte/Kconfig| 2 +-
drivers/clk/kendryte/Makefile | 2 +-
drivers/clk/kendryte/clk.c | 390
drivers/clk/kendryte/clk.h | 27 ++
include/dt-bindings/clock/k210-sysctl.h | 53
include/dt-bindings/mfd/k210-sysctl.h | 38 +++
6 files changed, 510 insertions(+), 2 deletions(-)
create mode 100644 drivers/clk/kendryte/clk.c
create mode 100644 drivers/clk/kendryte/clk.h
create mode 100644 include/dt-bindings/clock/k210-sysctl.h
create mode 100644 include/dt-bindings/mfd/k210-sysctl.h
diff --git a/drivers/clk/kendryte/Kconfig b/drivers/clk/kendryte/Kconfig
index 7b69c8afaf..073fca0781 100644
--- a/drivers/clk/kendryte/Kconfig
+++ b/drivers/clk/kendryte/Kconfig
@@ -1,6 +1,6 @@
config CLK_K210
bool "Clock support for Kendryte K210"
- depends on CLK && CLK_CCF
+ depends on CLK && CLK_CCF && CLK_COMPOSITE_CCF
help
This enables support clock driver for Kendryte K210 platforms.
diff --git a/drivers/clk/kendryte/Makefile b/drivers/clk/kendryte/Makefile
index c56d93ea1c..d26bce954f 100644
--- a/drivers/clk/kendryte/Makefile
+++ b/drivers/clk/kendryte/Makefile
@@ -1 +1 @@
-obj-y += pll.o
+obj-y += clk.o pll.o
diff --git a/drivers/clk/kendryte/clk.c b/drivers/clk/kendryte/clk.c
new file mode 100644
index 00..aaa7420c84
--- /dev/null
+++ b/drivers/clk/kendryte/clk.c
@@ -0,0 +1,390 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 Sean Anderson
+ */
+#include "clk.h"
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "pll.h"
+
+static ulong k210_clk_get_rate(struct clk *clk)
+{
+ struct clk *c;
+ int err = clk_get_by_id(clk->id, );
+
+ if (err)
+ return err;
+ return clk_get_rate(c);
+}
+
+static ulong k210_clk_set_rate(struct clk *clk, unsigned long rate)
+{
+ struct clk *c;
+ int err = clk_get_by_id(clk->id, );
+
+ if (err)
+ return err;
+ return clk_set_rate(c, rate);
+}
+
+static int k210_clk_set_parent(struct clk *clk, struct clk *parent)
+{
+ struct clk *c, *p;
+ int err = clk_get_by_id(clk->id, );
+
+ if (err)
+ return err;
+
+ err = clk_get_by_id(parent->id, );
+ if (err)
+ return err;
+
+ return clk_set_parent(c, p);
+}
+
+static int k210_clk_endisable(struct clk *clk, bool enable)
+{
+ struct clk *c;
+ int err = clk_get_by_id(clk->id, );
+
+ if (err)
+ return err;
+ return enable ? clk_enable(c) : clk_disable(c);
+}
+
+static int k210_clk_enable(struct clk *clk)
+{
+ return k210_clk_endisable(clk, true);
+}
+
+static int k210_clk_disable(struct clk *clk)
+{
+ return k210_clk_endisable(clk, false);
+}
+
+static const struct clk_ops k210_clk_ops = {
+ .set_rate = k210_clk_set_rate,
+ .get_rate = k210_clk_get_rate,
+ .set_parent = k210_clk_set_parent,
+ .enable = k210_clk_enable,
+ .disable = k210_clk_disable,
+};
+
+/* The first clock is in0, which is filled in by k210_clk_probe */
+static const char *generic_sels[] = { NULL, "pll0", };
+static const char *aclk_sels[] = { "in0_half", "pll0_half", };
+static const char *pll2_sels[] = { NULL, "pll0", "pll1", };
+
+static struct clk_divider *k210_clk_comp_div_flags(void __iomem *reg, u8 shift,
+ u8 width, u8 flags)
+{
+ struct clk_divider *div;
+
+ div = kzalloc(sizeof(*div), GFP_KERNEL);
+ if (!div)
+ return div;
+ div->reg = reg;
+ div->shift = shift;
+ div->width = width;
+ div->flags = flags;
+ return div;
+}
+
+static inline struct clk_divider *k210_clk_comp_div(void __iomem *reg, u8
shift,
+ u8 width)
+{
+ return k210_clk_comp_div_flags(reg, shift, width,
