From: Stephen Boyd <sboyd@kernel.org>
To: Damien Le Moal <damien.lemoal@wdc.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
linux-riscv@lists.infradead.org
Cc: Atish Patra <atish.patra@wdc.com>,
Michael Turquette <mturquette@baylibre.com>,
Anup Patel <anup.patel@wdc.com>,
linux-clk@vger.kernel.org, Sean Anderson <seanga2@gmail.com>
Subject: Re: [PATCH v19 01/17] clk: Add RISC-V Canaan Kendryte K210 clock driver
Date: Wed, 10 Feb 2021 18:34:33 -0800 [thread overview]
Message-ID: <161301087300.1254594.18438542581140302342@swboyd.mtv.corp.google.com> (raw)
In-Reply-To: <20210210050230.131281-2-damien.lemoal@wdc.com>
Quoting Damien Le Moal (2021-02-09 21:02:14)
> Add a clock provider driver for the Canaan Kendryte K210 RISC-V SoC.
> This new driver with the compatible string "canaan,k210-clk" implements
> support for the full clock structure of the K210 SoC. Since it is
> required for the correct operation of the SoC, this driver is
> selected by default for compilation when the SOC_CANAAN option is
> selected.
>
> With this change, the k210-sysctl driver is turned into a simple
> platform driver which enables its power bus clock and triggers
> populating its child nodes. The sysctl driver retains the SOC early
> initialization code, but the implementation now relies on the new
> function k210_clk_early_init() provided by the new clk-k210 driver.
>
> The clock structure implemented and many of the coding ideas for the
> driver come from the work by Sean Anderson on the K210 support for the
> U-Boot project.
>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: linux-clk@vger.kernel.org
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
No change log :/
Minor nitpicks but otherwise
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> drivers/clk/Kconfig | 7 +
> drivers/clk/Makefile | 1 +
> drivers/clk/clk-k210.c | 1007 ++++++++++++++++++++++++++++++
> drivers/soc/canaan/Kconfig | 18 +-
> drivers/soc/canaan/Makefile | 2 +-
> drivers/soc/canaan/k210-sysctl.c | 205 ++----
> include/soc/canaan/k210-sysctl.h | 2 +
> 7 files changed, 1064 insertions(+), 178 deletions(-)
> create mode 100644 drivers/clk/clk-k210.c
>
> diff --git a/drivers/clk/clk-k210.c b/drivers/clk/clk-k210.c
> new file mode 100644
> index 000000000000..6c84abf5b2e3
> --- /dev/null
> +++ b/drivers/clk/clk-k210.c
> @@ -0,0 +1,1007 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2019-20 Sean Anderson <seanga2@gmail.com>
> + * Copyright (c) 2019 Western Digital Corporation or its affiliates.
> + */
> +#define pr_fmt(fmt) "k210-clk: " fmt
> +
> +#include <linux/io.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>
> +#include <linux/of_clk.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_address.h>
> +#include <linux/clk-provider.h>
> +#include <linux/bitfield.h>
> +#include <linux/delay.h>
Some people sort this alphabetically, up to you.
> +#include <soc/canaan/k210-sysctl.h>
> +
> +#include <dt-bindings/clock/k210-clk.h>
> +
[...]
> +
> +/*
> + * PLL control register bits.
> + */
> +#define K210_PLL_CLKR GENMASK(3, 0)
> +#define K210_PLL_CLKF GENMASK(9, 4)
> +#define K210_PLL_CLKOD GENMASK(13, 10)
> +#define K210_PLL_BWADJ GENMASK(19, 14)
> +#define K210_PLL_RESET (1 << 20)
> +#define K210_PLL_PWRD (1 << 21)
> +#define K210_PLL_INTFB (1 << 22)
> +#define K210_PLL_BYPASS (1 << 23)
> +#define K210_PLL_TEST (1 << 24)
> +#define K210_PLL_EN (1 << 25)
> +#define K210_PLL_SEL GENMASK(27, 26) /* PLL2 only */
> +
> +/*
> + * PLL lock register bits.
> + */
> +#define K210_PLL_LOCK 0
> +#define K210_PLL_CLEAR_SLIP 2
> +#define K210_PLL_TEST_OUT 3
> +
> +/*
> + * Clock selector register bits.
> + */
> +#define K210_ACLK_SEL BIT(0)
> +#define K210_ACLK_DIV GENMASK(2, 1)
> +
> +/*
> + * PLLs.
> + */
> +enum k210_pll_id {
> + K210_PLL0, K210_PLL1, K210_PLL2, K210_PLL_NUM
> +};
> +
> +struct k210_pll {
> + enum k210_pll_id id;
> + struct k210_sysclk *ksc;
> + void __iomem *base;
> + void __iomem *reg;
> + void __iomem *lock;
> + u8 lock_shift;
> + u8 lock_width;
> + struct clk_hw hw;
> +};
> +#define to_k210_pll(_hw) container_of(_hw, struct k210_pll, hw)
> +
> +/*
> + * PLLs configuration: by default PLL0 runs at 780 MHz and PLL1 at 299 MHz.
> + * The first 2 SRAM banks depend on ACLK/CPU clock which is by default PLL0
> + * rate divided by 2. Set PLL1 to 390 MHz so that the third SRAM bank has the
> + * same clock as the first 2.
> + */
> +struct k210_pll_cfg {
> + u32 reg;
> + u8 lock_shift;
> + u8 lock_width;
> + u32 r;
> + u32 f;
> + u32 od;
> + u32 bwadj;
> +};
> +
> +static struct k210_pll_cfg k210_plls_cfg[] = {
const?
> + { K210_SYSCTL_PLL0, 0, 2, 0, 59, 1, 59 }, /* 780 MHz */
> + { K210_SYSCTL_PLL1, 8, 1, 0, 59, 3, 59 }, /* 390 MHz */
> + { K210_SYSCTL_PLL2, 16, 1, 0, 22, 1, 22 }, /* 299 MHz */
> +};
> +
> +/**
> + * struct k210_sysclk - sysclk driver data
> + * @regs: system controller registers start address
> + * @clk_lock: clock setting spinlock
> + * @plls: SoC PLLs descriptors
> + * @aclk: ACLK clock
> + * @clks: All other clocks
> + */
> +struct k210_sysclk {
> + void __iomem *regs;
> + spinlock_t clk_lock;
> + struct k210_pll plls[K210_PLL_NUM];
> + struct clk_hw aclk;
> + struct k210_clk clks[K210_NUM_CLKS];
> +};
> +
> +#define to_k210_sysclk(_hw) container_of(_hw, struct k210_sysclk, aclk)
> +
> +/*
> + * Set ACLK parent selector: 0 for IN0, 1 for PLL0.
> + */
> +static void k210_aclk_set_selector(void __iomem *regs, u8 sel)
> +{
> + u32 reg = readl(regs + K210_SYSCTL_SEL0);
> +
> + if (sel)
> + reg |= K210_ACLK_SEL;
> + else
> + reg &= K210_ACLK_SEL;
> + writel(reg, regs + K210_SYSCTL_SEL0);
> +}
> +
> +static void k210_init_pll(void __iomem *regs, enum k210_pll_id pllid,
Should this have __init too?
> + struct k210_pll *pll)
> +{
> + pll->id = pllid;
> + pll->reg = regs + k210_plls_cfg[pllid].reg;
> + pll->lock = regs + K210_SYSCTL_PLL_LOCK;
> + pll->lock_shift = k210_plls_cfg[pllid].lock_shift;
> + pll->lock_width = k210_plls_cfg[pllid].lock_width;
> +}
> +
> +static void k210_pll_wait_for_lock(struct k210_pll *pll)
[....]
> +
> + init.name = k210_clk_cfgs[id].name;
> + init.flags = flags;
> + init.parent_data = parent_data;
> + init.num_parents = num_parents;
> + if (num_parents > 1)
> + init.ops = &k210_clk_mux_ops;
> + else
> + init.ops = &k210_clk_ops;
> +
> + kclk->id = id;
> + kclk->ksc = ksc;
> + kclk->hw.init = &init;
> +
> + ret = of_clk_hw_register(np, &kclk->hw);
> + if (ret) {
> + pr_err("%pOFP: register clock %s failed\n",
> + np, k210_clk_cfgs[id].name);
> + kclk->id = -1;
> + }
> +}
> +
> +/*
> + * All muxed clocks have IN0 and PLL0 as parents.
> + */
> +static inline void __init k210_register_mux_clk(struct device_node *np,
> + struct k210_sysclk *ksc, int id)
> +{
> + const struct clk_parent_data parent_data[2] = {
> + { /* .index = 0 for in0 */ },
It could be explicitly written instead of a comment, but OK.
> + { .hw = &ksc->plls[K210_PLL0].hw }
> + };
> +
> + k210_register_clk(np, ksc, id, parent_data, 2, 0);
> +}
> +
> +static inline void __init k210_register_in0_child(struct device_node *np,
Maybe dropping inline would help to make shorter lines because it will
probably be inlined by the compiler anyway.
> + struct k210_sysclk *ksc, int id)
> +{
> + const struct clk_parent_data parent_data = {
> + /* .index = 0 for in0 */
> + };
> +
> + k210_register_clk(np, ksc, id, &parent_data, 1, 0);
> +}
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2021-02-11 2:34 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-10 5:02 [PATCH v19 00/17] RISC-V Kendryte K210 support improvements Damien Le Moal
2021-02-10 5:02 ` [PATCH v19 01/17] clk: Add RISC-V Canaan Kendryte K210 clock driver Damien Le Moal
2021-02-11 2:34 ` Stephen Boyd [this message]
2021-02-11 2:41 ` Damien Le Moal
[not found] ` <161301220412.1254594.16210039566881279371@swboyd.mtv.corp.google.com>
2021-02-11 3:00 ` Damien Le Moal
2021-02-10 5:02 ` [PATCH v19 02/17] dt-bindings: update MAINTAINERS file Damien Le Moal
2021-02-10 5:02 ` [PATCH v19 03/17] dt-bindings: add Canaan boards compatible strings Damien Le Moal
2021-02-10 5:02 ` [PATCH v19 04/17] dt-bindings: update risc-v cpu properties Damien Le Moal
2021-02-10 5:02 ` [PATCH v19 05/17] dt-bindings: update sifive plic compatible string Damien Le Moal
2021-02-10 5:02 ` [PATCH v19 06/17] dt-bindings: update sifive clint " Damien Le Moal
2021-02-10 5:02 ` [PATCH v19 07/17] dt-bindings: update sifive uart " Damien Le Moal
2021-02-10 5:02 ` [PATCH v19 08/17] dt-bindings: fix sifive gpio properties Damien Le Moal
2021-02-10 16:56 ` Rob Herring
2021-02-10 5:02 ` [PATCH v19 09/17] dt-bindings: add resets property to dw-apb-timer Damien Le Moal
2021-02-10 5:02 ` [PATCH v19 10/17] riscv: Update Canaan Kendryte K210 device tree Damien Le Moal
2021-02-10 5:02 ` [PATCH v19 11/17] riscv: Add SiPeed MAIX BiT board " Damien Le Moal
2021-02-10 5:02 ` [PATCH v19 12/17] riscv: Add SiPeed MAIX DOCK " Damien Le Moal
2021-02-10 5:02 ` [PATCH v19 13/17] riscv: Add SiPeed MAIX GO " Damien Le Moal
2021-02-10 5:02 ` [PATCH v19 14/17] riscv: Add SiPeed MAIXDUINO " Damien Le Moal
2021-02-10 5:02 ` [PATCH v19 15/17] riscv: Add Kendryte KD233 " Damien Le Moal
2021-02-10 5:02 ` [PATCH v19 16/17] riscv: Update Canaan Kendryte K210 defconfig Damien Le Moal
2021-02-10 5:02 ` [PATCH v19 17/17] riscv: Add Canaan Kendryte K210 SD card defconfig Damien Le Moal
2021-02-15 5:16 ` [PATCH v19 00/17] RISC-V Kendryte K210 support improvements Damien Le Moal
2021-02-19 6:28 ` Palmer Dabbelt
2021-02-19 6:54 ` Damien Le Moal
2021-02-19 7:20 ` Palmer Dabbelt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=161301087300.1254594.18438542581140302342@swboyd.mtv.corp.google.com \
--to=sboyd@kernel.org \
--cc=anup.patel@wdc.com \
--cc=atish.patra@wdc.com \
--cc=damien.lemoal@wdc.com \
--cc=linux-clk@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=mturquette@baylibre.com \
--cc=palmer@dabbelt.com \
--cc=seanga2@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).