Re: [PATCH 6/6] clk: samsung: Introduce Exynos850 clock driver
From: Sam Protsenko <semen.protsenko@linaro.org>
Date: 2021-10-05 11:29:34
Also in:
linux-clk, linux-devicetree, linux-samsung-soc, lkml
On Wed, 15 Sept 2021 at 11:59, Krzysztof Kozlowski [off-list ref] wrote:
On 14/09/2021 17:56, Sam Protsenko wrote:quoted
This is the initial implementation adding only basic clocks like UART, MMC, I2C and corresponding parent clocks. Design is influenced by Exynos7 and Exynos5433 clock drivers. Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> --- drivers/clk/samsung/Makefile | 1 + drivers/clk/samsung/clk-exynos850.c | 700 ++++++++++++++++++++++++++++ 2 files changed, 701 insertions(+) create mode 100644 drivers/clk/samsung/clk-exynos850.cdiff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile index 028b2e27a37e..c46cf11e4d0b 100644 --- a/drivers/clk/samsung/Makefile +++ b/drivers/clk/samsung/Makefile@@ -17,6 +17,7 @@ obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK) += clk-exynos5433.o obj-$(CONFIG_EXYNOS_AUDSS_CLK_CON) += clk-exynos-audss.o obj-$(CONFIG_EXYNOS_CLKOUT) += clk-exynos-clkout.o obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK) += clk-exynos7.o +obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK) += clk-exynos850.o obj-$(CONFIG_S3C2410_COMMON_CLK)+= clk-s3c2410.o obj-$(CONFIG_S3C2410_COMMON_DCLK)+= clk-s3c2410-dclk.o obj-$(CONFIG_S3C2412_COMMON_CLK)+= clk-s3c2412.odiff --git a/drivers/clk/samsung/clk-exynos850.c b/drivers/clk/samsung/clk-exynos850.c new file mode 100644 index 000000000000..1028caa2102e --- /dev/null +++ b/drivers/clk/samsung/clk-exynos850.c@@ -0,0 +1,700 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2021 Linaro Ltd. + * Author: Sam Protsenko <semen.protsenko@linaro.org> + * + * Common Clock Framework support for Exynos850 SoC. + */ + +#include <linux/clk-provider.h> +#include <linux/of.h> +#include <linux/of_address.h> + +#include <dt-bindings/clock/exynos850.h> + +#include "clk.h" + +/* Gate register bits */ +#define GATE_MANUAL BIT(20) +#define GATE_ENABLE_HWACG BIT(28) + +/* Gate register offsets range */ +#define GATE_OFF_START 0x2000 +#define GATE_OFF_END 0x2fff + +/** + * exynos850_init_clocks - Set clocks initial configuration + * @np: CMU device tree node with "reg" property (CMU addr) + * @reg_offs: Register offsets array for clocks to init + * @reg_offs_len: Number of register offsets in reg_offs array + * + * Set manual control mode for all gate clocks. + */ +static void __init exynos850_init_clocks(struct device_node *np, + const unsigned long *reg_offs, size_t reg_offs_len) +{ + const __be32 *regaddr_p; + u64 regaddr; + u32 base; + size_t i; + + /* Get the base address ("reg" property in dts) */ + regaddr_p = of_get_address(np, 0, NULL, NULL); + if (!regaddr_p) + panic("%s: failed to get reg regaddr\n", __func__); + + regaddr = of_translate_address(np, regaddr_p); + if (regaddr == OF_BAD_ADDR || !regaddr) + panic("%s: bad reg regaddr\n", __func__); + + base = (u32)regaddr; + + for (i = 0; i < reg_offs_len; ++i) { + void __iomem *reg; + u32 val; + + /* Modify only gate clock registers */ + if (reg_offs[i] < GATE_OFF_START || reg_offs[i] > GATE_OFF_END) + continue; + + reg = ioremap(base + reg_offs[i], 4);You first translate the address to CPU physical address and then apply offset. This should be equivalent to one of_iomap() of entire range and iterate starting from the base pointer. IOW, I don't get why you have to map each register instead of mapping entire SFR/IO range?
Thanks, will do in v2.
quoted
+ val = ioread32(reg); + val |= GATE_MANUAL; + val &= ~GATE_ENABLE_HWACG; + iowrite32(val, reg);All other drivers use readl/writel, so how about keeping it consistent?
Ok. Though io* variants looks better to me (API names consistent with ioremap/iounmap) :)
Rest looks good but I did not verify the numbers :) Best regards, Krzysztof
_______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel