Re: [PATCH v1] clk: tegra20/30: Add custom EMC clock implementation
12.04.2019 13:09, Dmitry Osipenko пишет:
> 12.04.2019 10:46, Peter De Schrijver пишет:
>> On Fri, Apr 12, 2019 at 01:47:08AM +0300, Dmitry Osipenko wrote:
>>> A proper External Memory Controller clock rounding and parent selection
>>> functionality is required by the EMC drivers. It is not available using
>>> the generic clock implementation, hence add a custom one. The clock rate
>>> rounding shall be done by the EMC drivers because they have information
>>> about available memory timings, so the drivers will have to register a
>>> callback that will round the requested rate. EMC clock users won't be able
>>> to request EMC clock by getting -EPROBE_DEFER until EMC driver is probed
>>> and the callback is set up. The functionality is somewhat similar to the
>>> clk-emc.c which serves Tegra124+ SoC's, the later HW generations support
>>> more parent clock sources and the HW configuration and integration with
>>> the EMC drivers differs a tad from the older gens, hence it's not really
>>> worth to try to squash everything into a single source file.
>>>
>> ..
>>
>>> + val = readl_relaxed(emc->ioaddr);
>>> + val &= ~CLK_SOURCE_EMC_2X_CLK_DIVISOR_MASK;
>>> + val |= div;
>>> +
>>> + parent = val >> CLK_SOURCE_EMC_2X_CLK_SRC_SHIFT;
>>> +
>>> + if (parent == EMC_SRC_PLL_M && div == 0 && emc->want_low_jitter)
>>> + val |= USE_PLLM_UD;
>>> + else
>>> + val &= ~USE_PLLM_UD;
>>> +
>>
>> Note that low jitter means the divider is bypassed, so you can only use
>> this when div == 1.
>>
>>> + writel_relaxed(val, emc->ioaddr);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int emc_set_rate_and_parent(struct clk_hw *hw,
>>> + unsigned long rate,
>>> + unsigned long parent_rate,
>>> + u8 index)
>>> +{
>>> + struct tegra_clk_emc *emc = to_tegra_clk_emc(hw);
>>> + u32 val, div;
>>> +
>>> + div = div_frac_get(rate, parent_rate, 8, 1, 0);
>>> +
>>> + val = readl_relaxed(emc->ioaddr);
>>> +
>>> + val &= ~CLK_SOURCE_EMC_2X_CLK_SRC_MASK;
>>> + val |= index << CLK_SOURCE_EMC_2X_CLK_SRC_SHIFT;
>>> +
>>> + val &= ~CLK_SOURCE_EMC_2X_CLK_DIVISOR_MASK;
>>> + val |= div;
>>> +
>>> + if (index == EMC_SRC_PLL_M && div == 0 && emc->want_low_jitter)
>>> + val |= USE_PLLM_UD;
>>> + else
>>> + val &= ~USE_PLLM_UD;
>>> +
>>
>> Same here ofcourse.
>
> Please note that hw_div = div + 1.0, hence writing div=0 means that HW
> divider is disabled and thus can be bypassed to get a low jitter clock. Also
> please note that div_frac_get() returns the "hw_div" value. Tegra30 doesn't
> work well if low jitter isn't enabled for the high clock rate (hangs after
> couple seconds) and Tegra20 is the opposite. Everything should be fine and it
> is working well, I don't see any problem.
>
Oh, div_frac_get() returns the "div" value of course.
Re: [PATCH v1] clk: tegra20/30: Add custom EMC clock implementation
12.04.2019 10:46, Peter De Schrijver пишет:
> On Fri, Apr 12, 2019 at 01:47:08AM +0300, Dmitry Osipenko wrote:
>> A proper External Memory Controller clock rounding and parent selection
>> functionality is required by the EMC drivers. It is not available using
>> the generic clock implementation, hence add a custom one. The clock rate
>> rounding shall be done by the EMC drivers because they have information
>> about available memory timings, so the drivers will have to register a
>> callback that will round the requested rate. EMC clock users won't be able
>> to request EMC clock by getting -EPROBE_DEFER until EMC driver is probed
>> and the callback is set up. The functionality is somewhat similar to the
>> clk-emc.c which serves Tegra124+ SoC's, the later HW generations support
>> more parent clock sources and the HW configuration and integration with
>> the EMC drivers differs a tad from the older gens, hence it's not really
>> worth to try to squash everything into a single source file.
>>
> ..
>
>> +val = readl_relaxed(emc->ioaddr);
>> +val &= ~CLK_SOURCE_EMC_2X_CLK_DIVISOR_MASK;
>> +val |= div;
>> +
>> +parent = val >> CLK_SOURCE_EMC_2X_CLK_SRC_SHIFT;
>> +
>> +if (parent == EMC_SRC_PLL_M && div == 0 && emc->want_low_jitter)
>> +val |= USE_PLLM_UD;
>> +else
>> +val &= ~USE_PLLM_UD;
>> +
>
> Note that low jitter means the divider is bypassed, so you can only use
> this when div == 1.
>
>> +writel_relaxed(val, emc->ioaddr);
>> +
>> +return 0;
>> +}
>> +
>> +static int emc_set_rate_and_parent(struct clk_hw *hw,
>> + unsigned long rate,
>> + unsigned long parent_rate,
>> + u8 index)
>> +{
>> +struct tegra_clk_emc *emc = to_tegra_clk_emc(hw);
>> +u32 val, div;
>> +
>> +div = div_frac_get(rate, parent_rate, 8, 1, 0);
>> +
>> +val = readl_relaxed(emc->ioaddr);
>> +
>> +val &= ~CLK_SOURCE_EMC_2X_CLK_SRC_MASK;
>> +val |= index << CLK_SOURCE_EMC_2X_CLK_SRC_SHIFT;
>> +
>> +val &= ~CLK_SOURCE_EMC_2X_CLK_DIVISOR_MASK;
>> +val |= div;
>> +
>> +if (index == EMC_SRC_PLL_M && div == 0 && emc->want_low_jitter)
>> +val |= USE_PLLM_UD;
>> +else
>> +val &= ~USE_PLLM_UD;
>> +
>
> Same here ofcourse.
Please note that hw_div = div + 1.0, hence writing div=0 means that HW divider
is disabled and thus can be bypassed to get a low jitter clock. Also please
note that div_frac_get() returns the "hw_div" value. Tegra30 doesn't work well
if low jitter isn't enabled for the high clock rate (hangs after couple
seconds) and Tegra20 is the opposite. Everything should be fine and it is
working well, I don't see any problem.
Re: [PATCH v1] clk: tegra20/30: Add custom EMC clock implementation
On Fri, Apr 12, 2019 at 01:47:08AM +0300, Dmitry Osipenko wrote:
> A proper External Memory Controller clock rounding and parent selection
> functionality is required by the EMC drivers. It is not available using
> the generic clock implementation, hence add a custom one. The clock rate
> rounding shall be done by the EMC drivers because they have information
> about available memory timings, so the drivers will have to register a
> callback that will round the requested rate. EMC clock users won't be able
> to request EMC clock by getting -EPROBE_DEFER until EMC driver is probed
> and the callback is set up. The functionality is somewhat similar to the
> clk-emc.c which serves Tegra124+ SoC's, the later HW generations support
> more parent clock sources and the HW configuration and integration with
> the EMC drivers differs a tad from the older gens, hence it's not really
> worth to try to squash everything into a single source file.
>
..
> + val = readl_relaxed(emc->ioaddr);
> + val &= ~CLK_SOURCE_EMC_2X_CLK_DIVISOR_MASK;
> + val |= div;
> +
> + parent = val >> CLK_SOURCE_EMC_2X_CLK_SRC_SHIFT;
> +
> + if (parent == EMC_SRC_PLL_M && div == 0 && emc->want_low_jitter)
> + val |= USE_PLLM_UD;
> + else
> + val &= ~USE_PLLM_UD;
> +
Note that low jitter means the divider is bypassed, so you can only use
this when div == 1.
> + writel_relaxed(val, emc->ioaddr);
> +
> + return 0;
> +}
> +
> +static int emc_set_rate_and_parent(struct clk_hw *hw,
> +unsigned long rate,
> +unsigned long parent_rate,
> +u8 index)
> +{
> + struct tegra_clk_emc *emc = to_tegra_clk_emc(hw);
> + u32 val, div;
> +
> + div = div_frac_get(rate, parent_rate, 8, 1, 0);
> +
> + val = readl_relaxed(emc->ioaddr);
> +
> + val &= ~CLK_SOURCE_EMC_2X_CLK_SRC_MASK;
> + val |= index << CLK_SOURCE_EMC_2X_CLK_SRC_SHIFT;
> +
> + val &= ~CLK_SOURCE_EMC_2X_CLK_DIVISOR_MASK;
> + val |= div;
> +
> + if (index == EMC_SRC_PLL_M && div == 0 && emc->want_low_jitter)
> + val |= USE_PLLM_UD;
> + else
> + val &= ~USE_PLLM_UD;
> +
Same here ofcourse.
Peter.
[PATCH v1] clk: tegra20/30: Add custom EMC clock implementation
A proper External Memory Controller clock rounding and parent selection
functionality is required by the EMC drivers. It is not available using
the generic clock implementation, hence add a custom one. The clock rate
rounding shall be done by the EMC drivers because they have information
about available memory timings, so the drivers will have to register a
callback that will round the requested rate. EMC clock users won't be able
to request EMC clock by getting -EPROBE_DEFER until EMC driver is probed
and the callback is set up. The functionality is somewhat similar to the
clk-emc.c which serves Tegra124+ SoC's, the later HW generations support
more parent clock sources and the HW configuration and integration with
the EMC drivers differs a tad from the older gens, hence it's not really
worth to try to squash everything into a single source file.
Signed-off-by: Dmitry Osipenko
---
drivers/clk/tegra/Makefile | 2 +
drivers/clk/tegra/clk-tegra20-emc.c | 274
drivers/clk/tegra/clk-tegra20.c | 51 ++
drivers/clk/tegra/clk-tegra30.c | 35 ++--
drivers/clk/tegra/clk.h | 6 +
include/linux/clk/tegra.h | 3 +
6 files changed, 321 insertions(+), 50 deletions(-)
create mode 100644 drivers/clk/tegra/clk-tegra20-emc.c
diff --git a/drivers/clk/tegra/Makefile b/drivers/clk/tegra/Makefile
index 4812e45c2214..df966ca06788 100644
--- a/drivers/clk/tegra/Makefile
+++ b/drivers/clk/tegra/Makefile
@@ -17,7 +17,9 @@ obj-y += clk-tegra-fixed.o
obj-y += clk-tegra-super-gen4.o
obj-$(CONFIG_TEGRA_CLK_EMC)+= clk-emc.o
obj-$(CONFIG_ARCH_TEGRA_2x_SOC) += clk-tegra20.o
+obj-$(CONFIG_ARCH_TEGRA_2x_SOC)+= clk-tegra20-emc.o
obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += clk-tegra30.o
+obj-$(CONFIG_ARCH_TEGRA_3x_SOC)+= clk-tegra20-emc.o
obj-$(CONFIG_ARCH_TEGRA_114_SOC) += clk-tegra114.o
obj-$(CONFIG_ARCH_TEGRA_124_SOC) += clk-tegra124.o
obj-$(CONFIG_TEGRA_CLK_DFLL) += clk-tegra124-dfll-fcpu.o
diff --git a/drivers/clk/tegra/clk-tegra20-emc.c
b/drivers/clk/tegra/clk-tegra20-emc.c
new file mode 100644
index ..2cc8f16cd136
--- /dev/null
+++ b/drivers/clk/tegra/clk-tegra20-emc.c
@@ -0,0 +1,274 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include
+#include
+#include
+#include
+#include
+
+#include "clk.h"
+
+#define CLK_SOURCE_EMC_2X_CLK_DIVISOR_MASK 0xff
+#define CLK_SOURCE_EMC_2X_CLK_SRC_SHIFT30
+#define CLK_SOURCE_EMC_2X_CLK_SRC_MASK 0xc000
+
+#define CLK_SOURCE_EMC 0x19c
+
+#define USE_PLLM_UDBIT(29)
+
+#define EMC_SRC_PLL_M 0
+#define EMC_SRC_PLL_C 1
+#define EMC_SRC_PLL_P 2
+#define EMC_SRC_CLK_M 3
+
+static const char * const emc_parent_clk_names[] = {
+ "pll_m", "pll_c", "pll_p", "clk_m",
+};
+
+struct tegra_clk_emc {
+ struct clk_hw hw;
+ void __iomem *ioaddr;
+ bool want_low_jitter;
+
+ long (*round_cb)(unsigned long, unsigned long, unsigned long, void *);
+ void *arg_cb;
+};
+
+static inline struct tegra_clk_emc *to_tegra_clk_emc(struct clk_hw *hw)
+{
+ return container_of(hw, struct tegra_clk_emc, hw);
+}
+
+static unsigned long emc_recalc_rate(struct clk_hw *hw,
+unsigned long parent_rate)
+{
+ struct tegra_clk_emc *emc = to_tegra_clk_emc(hw);
+ u32 val, div;
+
+ val = readl_relaxed(emc->ioaddr);
+ div = val & CLK_SOURCE_EMC_2X_CLK_DIVISOR_MASK;
+
+ return DIV_ROUND_UP(parent_rate * 2, div + 2);
+}
+
+static u8 emc_get_parent(struct clk_hw *hw)
+{
+ struct tegra_clk_emc *emc = to_tegra_clk_emc(hw);
+
+ return readl_relaxed(emc->ioaddr) >> CLK_SOURCE_EMC_2X_CLK_SRC_SHIFT;
+}
+
+static int emc_set_parent(struct clk_hw *hw, u8 index)
+{
+ struct tegra_clk_emc *emc = to_tegra_clk_emc(hw);
+ u32 val, div;
+
+ val = readl_relaxed(emc->ioaddr);
+ div = val & CLK_SOURCE_EMC_2X_CLK_DIVISOR_MASK;
+ val &= ~CLK_SOURCE_EMC_2X_CLK_SRC_MASK;
+ val |= index << CLK_SOURCE_EMC_2X_CLK_SRC_SHIFT;
+
+ if (index == EMC_SRC_PLL_M && div == 0 && emc->want_low_jitter)
+ val |= USE_PLLM_UD;
+ else
+ val &= ~USE_PLLM_UD;
+
+ writel_relaxed(val, emc->ioaddr);
+
+ return 0;
+}
+
+static int emc_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct tegra_clk_emc *emc = to_tegra_clk_emc(hw);
+ unsigned int parent;
+ u32 val, div;
+
+ div = div_frac_get(rate, parent_rate, 8, 1, 0);
+
+ val = readl_relaxed(emc->ioaddr);
+ val &= ~CLK_SOURCE_EMC_2X_CLK_DIVISOR_MASK;
+ val |= div;
+
+ parent = val >> CLK_SOURCE_EMC_2X_CLK_SRC_SHIFT;
+
+ if (parent == EMC_SRC_PLL_M && div == 0 && emc->want_low_jitter)
+ val |= USE_PLLM_UD;
+ else
+ val &=
