From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> To: "Sam Protsenko" <semen.protsenko@linaro.org>, "Sylwester Nawrocki" <s.nawrocki@samsung.com>, "Paweł Chmiel" <pawel.mikolaj.chmiel@gmail.com>, "Chanwoo Choi" <cw00.choi@samsung.com>, "Tomasz Figa" <tomasz.figa@gmail.com>, "Rob Herring" <robh+dt@kernel.org>, "Stephen Boyd" <sboyd@kernel.org>, "Michael Turquette" <mturquette@baylibre.com> Cc: Ryu Euiyoul <ryu.real@samsung.com>, Tom Gall <tom.gall@linaro.org>, Sumit Semwal <sumit.semwal@linaro.org>, John Stultz <john.stultz@linaro.org>, Amit Pundir <amit.pundir@linaro.org>, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org Subject: Re: [PATCH 6/6] clk: samsung: Introduce Exynos850 clock driver Date: Wed, 15 Sep 2021 10:59:24 +0200 [thread overview] Message-ID: <3da75dbe-2f98-39db-c455-46adead7097b@canonical.com> (raw) In-Reply-To: <20210914155607.14122-7-semen.protsenko@linaro.org> On 14/09/2021 17:56, Sam Protsenko wrote: > 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.c > > diff --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.o > diff --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? > + 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? Rest looks good but I did not verify the numbers :) Best regards, Krzysztof
WARNING: multiple messages have this Message-ID (diff)
From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> To: "Sam Protsenko" <semen.protsenko@linaro.org>, "Sylwester Nawrocki" <s.nawrocki@samsung.com>, "Paweł Chmiel" <pawel.mikolaj.chmiel@gmail.com>, "Chanwoo Choi" <cw00.choi@samsung.com>, "Tomasz Figa" <tomasz.figa@gmail.com>, "Rob Herring" <robh+dt@kernel.org>, "Stephen Boyd" <sboyd@kernel.org>, "Michael Turquette" <mturquette@baylibre.com> Cc: Ryu Euiyoul <ryu.real@samsung.com>, Tom Gall <tom.gall@linaro.org>, Sumit Semwal <sumit.semwal@linaro.org>, John Stultz <john.stultz@linaro.org>, Amit Pundir <amit.pundir@linaro.org>, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org Subject: Re: [PATCH 6/6] clk: samsung: Introduce Exynos850 clock driver Date: Wed, 15 Sep 2021 10:59:24 +0200 [thread overview] Message-ID: <3da75dbe-2f98-39db-c455-46adead7097b@canonical.com> (raw) In-Reply-To: <20210914155607.14122-7-semen.protsenko@linaro.org> On 14/09/2021 17:56, Sam Protsenko wrote: > 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.c > > diff --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.o > diff --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? > + 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? 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
next prev parent reply other threads:[~2021-09-15 8:59 UTC|newest] Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-09-14 15:56 [PATCH 0/6] clk: samsung: Introduce Exynos850 SoC clock driver Sam Protsenko 2021-09-14 15:56 ` Sam Protsenko 2021-09-14 15:56 ` [PATCH 1/6] clk: samsung: Enable bus clock on init Sam Protsenko 2021-09-14 15:56 ` Sam Protsenko 2021-09-15 8:21 ` Krzysztof Kozlowski 2021-09-15 8:21 ` Krzysztof Kozlowski 2021-10-06 10:46 ` Sam Protsenko 2021-10-06 10:46 ` Sam Protsenko 2021-10-06 12:38 ` Krzysztof Kozlowski 2021-10-06 12:38 ` Krzysztof Kozlowski 2021-10-06 13:29 ` Sam Protsenko 2021-10-06 13:29 ` Sam Protsenko 2021-10-08 6:50 ` Krzysztof Kozlowski 2021-10-08 6:50 ` Krzysztof Kozlowski 2021-09-15 12:51 ` Sylwester Nawrocki 2021-09-15 12:51 ` Sylwester Nawrocki 2021-10-06 11:18 ` Sam Protsenko 2021-10-06 11:18 ` Sam Protsenko 2021-10-06 12:45 ` Krzysztof Kozlowski 2021-10-06 12:45 ` Krzysztof Kozlowski 2021-10-09 18:49 ` Sylwester Nawrocki 2021-10-09 18:49 ` Sylwester Nawrocki 2021-09-14 15:56 ` [PATCH 2/6] clk: samsung: clk-pll: Implement pll0822x PLL type Sam Protsenko 2021-09-14 15:56 ` Sam Protsenko 2021-09-15 8:24 ` Krzysztof Kozlowski 2021-09-15 8:24 ` Krzysztof Kozlowski 2021-09-15 15:59 ` Chanwoo Choi 2021-09-15 15:59 ` Chanwoo Choi 2021-09-14 15:56 ` [PATCH 3/6] clk: samsung: clk-pll: Implement pll0831x " Sam Protsenko 2021-09-14 15:56 ` Sam Protsenko 2021-09-15 8:26 ` Krzysztof Kozlowski 2021-09-15 8:26 ` Krzysztof Kozlowski 2021-09-15 16:11 ` Chanwoo Choi 2021-09-15 16:11 ` Chanwoo Choi 2021-09-14 15:56 ` [PATCH 4/6] dt-bindings: clock: Add bindings definitions for Exynos850 CMU Sam Protsenko 2021-09-14 15:56 ` Sam Protsenko 2021-09-15 8:27 ` Krzysztof Kozlowski 2021-09-15 8:27 ` Krzysztof Kozlowski 2021-09-15 16:37 ` Chanwoo Choi 2021-09-15 16:37 ` Chanwoo Choi 2021-10-05 10:28 ` Sam Protsenko 2021-10-05 10:28 ` Sam Protsenko 2021-10-06 10:49 ` Krzysztof Kozlowski 2021-10-06 10:49 ` Krzysztof Kozlowski 2021-10-06 13:31 ` Sam Protsenko 2021-10-06 13:31 ` Sam Protsenko 2021-09-21 21:10 ` Rob Herring 2021-09-21 21:10 ` Rob Herring 2021-09-14 15:56 ` [PATCH 5/6] dt-bindings: clock: Document Exynos850 CMU bindings Sam Protsenko 2021-09-14 15:56 ` Sam Protsenko 2021-09-14 21:35 ` Rob Herring 2021-09-14 21:35 ` Rob Herring 2021-09-15 8:28 ` Krzysztof Kozlowski 2021-09-15 8:28 ` Krzysztof Kozlowski 2021-10-05 11:48 ` Sam Protsenko 2021-10-05 11:48 ` Sam Protsenko 2021-09-15 16:47 ` Chanwoo Choi 2021-09-15 16:47 ` Chanwoo Choi 2021-09-14 15:56 ` [PATCH 6/6] clk: samsung: Introduce Exynos850 clock driver Sam Protsenko 2021-09-14 15:56 ` Sam Protsenko 2021-09-15 8:59 ` Krzysztof Kozlowski [this message] 2021-09-15 8:59 ` Krzysztof Kozlowski 2021-10-05 11:29 ` Sam Protsenko 2021-10-05 11:29 ` Sam Protsenko 2021-10-06 12:50 ` Krzysztof Kozlowski 2021-10-06 12:50 ` Krzysztof Kozlowski 2021-09-15 13:07 ` Sylwester Nawrocki 2021-09-15 13:07 ` Sylwester Nawrocki 2021-10-05 11:36 ` Sam Protsenko 2021-10-05 11:36 ` Sam Protsenko 2021-10-06 12:46 ` Krzysztof Kozlowski 2021-10-06 12:46 ` Krzysztof Kozlowski 2021-09-15 18:04 ` Chanwoo Choi 2021-09-15 18:04 ` Chanwoo Choi 2021-09-15 22:00 ` Sam Protsenko 2021-09-15 22:00 ` Sam Protsenko
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=3da75dbe-2f98-39db-c455-46adead7097b@canonical.com \ --to=krzysztof.kozlowski@canonical.com \ --cc=amit.pundir@linaro.org \ --cc=cw00.choi@samsung.com \ --cc=devicetree@vger.kernel.org \ --cc=john.stultz@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-clk@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-samsung-soc@vger.kernel.org \ --cc=mturquette@baylibre.com \ --cc=pawel.mikolaj.chmiel@gmail.com \ --cc=robh+dt@kernel.org \ --cc=ryu.real@samsung.com \ --cc=s.nawrocki@samsung.com \ --cc=sboyd@kernel.org \ --cc=semen.protsenko@linaro.org \ --cc=sumit.semwal@linaro.org \ --cc=tom.gall@linaro.org \ --cc=tomasz.figa@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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.