Hi, this is a set of clock updates and fixes we did when developing
the tegra20/tegra30 auto support. The audio change is due to using
the tegra in clock-slave mode (this hasn't been done before)
We have also implemented the clock reset status callback as it is
used by some of the driver work we did.
The 2D and 3D clocks have an IDLE field in bits 15:8 so add these
clocks by making a 2D and 3D mux, and split the divider into the
standard 2D/3D ones and 2D/3D idle clocks.
Signed-off-by: Ben Dooks <[email protected]>
---
drivers/clk/tegra/clk-id.h | 4 ++++
drivers/clk/tegra/clk-tegra-periph.c | 23 +++++++++++++++++++++--
drivers/clk/tegra/clk-tegra30.c | 8 ++++++++
include/dt-bindings/clock/tegra30-car.h | 7 ++++++-
4 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/tegra/clk-id.h b/drivers/clk/tegra/clk-id.h
index b616e33c5255..0d202a70ce66 100644
--- a/drivers/clk/tegra/clk-id.h
+++ b/drivers/clk/tegra/clk-id.h
@@ -91,8 +91,12 @@ enum clk_id {
tegra_clk_fuse_burn,
tegra_clk_gpu,
tegra_clk_gr2d,
+ tegra_clk_gr2d_mux,
+ tegra_clk_gr2d_idle,
tegra_clk_gr2d_8,
tegra_clk_gr3d,
+ tegra_clk_gr3d_mux,
+ tegra_clk_gr3d_idle,
tegra_clk_gr3d_8,
tegra_clk_hclk,
tegra_clk_hda,
diff --git a/drivers/clk/tegra/clk-tegra-periph.c b/drivers/clk/tegra/clk-tegra-periph.c
index 47e5b1ac1a69..83967dac93f2 100644
--- a/drivers/clk/tegra/clk-tegra-periph.c
+++ b/drivers/clk/tegra/clk-tegra-periph.c
@@ -263,6 +263,21 @@
.flags = _flags, \
}
+#define GATE_DIV(_name, _parent_name, _offset, \
+ _div_shift, _div_width, _div_frac_width, _div_flags, \
+ _clk_num, _gate_flags, _clk_id, _flags) \
+ { \
+ .name = _name, \
+ .clk_id = _clk_id, \
+ .offset = _offset, \
+ .p.parent_name = _parent_name, \
+ .periph = TEGRA_CLK_PERIPH(0, 0, 0, \
+ _div_shift, _div_width, \
+ _div_frac_width, _div_flags, \
+ _clk_num, _gate_flags, NULL, NULL), \
+ .flags = _flags \
+ }
+
#define PLLP_BASE 0xa0
#define PLLP_MISC 0xac
#define PLLP_MISC1 0x680
@@ -646,8 +661,12 @@ static struct tegra_periph_init_data periph_clks[] = {
MUX("epp", mux_pllm_pllc_pllp_plla, CLK_SOURCE_EPP, 19, 0, tegra_clk_epp),
MUX("host1x", mux_pllm_pllc_pllp_plla, CLK_SOURCE_HOST1X, 28, 0, tegra_clk_host1x),
MUX("mpe", mux_pllm_pllc_pllp_plla, CLK_SOURCE_MPE, 60, 0, tegra_clk_mpe),
- MUX("2d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_2D, 21, 0, tegra_clk_gr2d),
- MUX("3d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_3D, 24, 0, tegra_clk_gr3d),
+ MUX("2d_mux", mux_pllm_pllc_pllp_plla, CLK_SOURCE_2D, 0, TEGRA_PERIPH_NO_DIV | TEGRA_PERIPH_NO_RESET | TEGRA_PERIPH_NO_GATE, tegra_clk_gr2d_mux),
+ GATE_DIV("2d", "2d_mux", CLK_SOURCE_2D, 0, 8, 1, TEGRA_DIVIDER_ROUND_UP,21, 0, tegra_clk_gr2d, 0),
+ GATE_DIV("2d_idle", "2d_mux", CLK_SOURCE_2D, 8, 8, 1, TEGRA_DIVIDER_ROUND_UP, 0, TEGRA_PERIPH_NO_GATE | TEGRA_PERIPH_NO_RESET, tegra_clk_gr2d_idle, 0),
+ MUX("3d_mux", mux_pllm_pllc_pllp_plla, CLK_SOURCE_3D, 0, TEGRA_PERIPH_NO_DIV | TEGRA_PERIPH_NO_RESET | TEGRA_PERIPH_NO_GATE, tegra_clk_gr3d_mux),
+ GATE_DIV("3d", "3d_mux", CLK_SOURCE_3D, 0, 8, 1, TEGRA_DIVIDER_ROUND_UP, 24, 0, tegra_clk_gr3d, 0),
+ GATE_DIV("3d_idle", "3d_mux", CLK_SOURCE_3D, 8, 8, 1, TEGRA_DIVIDER_ROUND_UP, 0, TEGRA_PERIPH_NO_GATE | TEGRA_PERIPH_NO_RESET, tegra_clk_gr3d_idle, 0),
INT8("vde", mux_pllp_pllc2_c_c3_pllm_clkm, CLK_SOURCE_VDE, 61, 0, tegra_clk_vde_8),
INT8("vi", mux_pllm_pllc2_c_c3_pllp_plla, CLK_SOURCE_VI, 20, 0, tegra_clk_vi_8),
INT8("vi", mux_pllm_pllc2_c_c3_pllp_plla_pllc4, CLK_SOURCE_VI, 20, 0, tegra_clk_vi_9),
diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index acfe661b2ae7..227d3643ecca 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -658,8 +658,12 @@ static struct tegra_devclk devclks[] __initdata = {
{ .dev_id = "mpe", .dt_id = TEGRA30_CLK_MPE },
{ .dev_id = "host1x", .dt_id = TEGRA30_CLK_HOST1X },
{ .dev_id = "3d", .dt_id = TEGRA30_CLK_GR3D },
+ { .dev_id = "3d", .con_id = "mux", .dt_id = TEGRA30_CLK_GR3D_MUX },
+ { .dev_id = "3d", .con_id = "idle", .dt_id = TEGRA30_CLK_GR3D_IDLE },
{ .dev_id = "3d2", .dt_id = TEGRA30_CLK_GR3D2 },
{ .dev_id = "2d", .dt_id = TEGRA30_CLK_GR2D },
+ { .dev_id = "2d", .con_id = "mux", .dt_id = TEGRA30_CLK_GR2D_MUX },
+ { .dev_id = "2d", .con_id = "idle", .dt_id = TEGRA30_CLK_GR2D_IDLE },
{ .dev_id = "se", .dt_id = TEGRA30_CLK_SE },
{ .dev_id = "mselect", .dt_id = TEGRA30_CLK_MSELECT },
{ .dev_id = "tegra-nor", .dt_id = TEGRA30_CLK_NOR },
@@ -762,6 +766,10 @@ static struct tegra_clk tegra30_clks[tegra_clk_max] __initdata = {
[tegra_clk_host1x] = { .dt_id = TEGRA30_CLK_HOST1X, .present = true },
[tegra_clk_gr2d] = { .dt_id = TEGRA30_CLK_GR2D, .present = true },
[tegra_clk_gr3d] = { .dt_id = TEGRA30_CLK_GR3D, .present = true },
+ [tegra_clk_gr2d_mux] = { .dt_id = TEGRA30_CLK_GR2D_MUX, .present = true },
+ [tegra_clk_gr3d_mux] = { .dt_id = TEGRA30_CLK_GR3D_MUX, .present = true },
+ [tegra_clk_gr2d_idle] = { .dt_id = TEGRA30_CLK_GR2D_IDLE, .present = true },
+ [tegra_clk_gr3d_idle] = { .dt_id = TEGRA30_CLK_GR3D_IDLE, .present = true },
[tegra_clk_mselect] = { .dt_id = TEGRA30_CLK_MSELECT, .present = true },
[tegra_clk_nor] = { .dt_id = TEGRA30_CLK_NOR, .present = true },
[tegra_clk_sdmmc1] = { .dt_id = TEGRA30_CLK_SDMMC1, .present = true },
diff --git a/include/dt-bindings/clock/tegra30-car.h b/include/dt-bindings/clock/tegra30-car.h
index 3c90f1535551..eda4ca60351e 100644
--- a/include/dt-bindings/clock/tegra30-car.h
+++ b/include/dt-bindings/clock/tegra30-car.h
@@ -269,6 +269,11 @@
#define TEGRA30_CLK_AUDIO3_MUX 306
#define TEGRA30_CLK_AUDIO4_MUX 307
#define TEGRA30_CLK_SPDIF_MUX 308
-#define TEGRA30_CLK_CLK_MAX 309
+
+#define TEGRA30_CLK_GR2D_MUX 309
+#define TEGRA30_CLK_GR3D_MUX 310
+#define TEGRA30_CLK_GR2D_IDLE 311
+#define TEGRA30_CLK_GR3D_IDLE 312
+#define TEGRA30_CLK_CLK_MAX 313
#endif /* _DT_BINDINGS_CLOCK_TEGRA30_CAR_H */
--
2.18.0
If both the TEGRA_PERIPH_NO_DIV and TEGRA_PERIPH_NO_GATE are set
as the clock is a mux only, then the clock code fails as it does
not handle both these at the same time. Add support for this by
adding new ops with just the parent get/set.
This is required to add the 2d and 3d idle clocks.
Signed-off-by: Ben Dooks <[email protected]>
---
drivers/clk/tegra/clk-periph.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/tegra/clk-periph.c b/drivers/clk/tegra/clk-periph.c
index 9475c00b7cf9..0c377d2dac43 100644
--- a/drivers/clk/tegra/clk-periph.c
+++ b/drivers/clk/tegra/clk-periph.c
@@ -137,6 +137,11 @@ static const struct clk_ops tegra_clk_periph_no_gate_ops = {
.set_rate = clk_periph_set_rate,
};
+static const struct clk_ops tegra_clk_periph_nodivgate_ops = {
+ .get_parent = clk_periph_get_parent,
+ .set_parent = clk_periph_set_parent,
+};
+
static struct clk *_tegra_clk_register_periph(const char *name,
const char * const *parent_names, int num_parents,
struct tegra_clk_periph *periph,
@@ -147,8 +152,11 @@ static struct clk *_tegra_clk_register_periph(const char *name,
struct clk_init_data init;
const struct tegra_clk_periph_regs *bank;
bool div = !(periph->gate.flags & TEGRA_PERIPH_NO_DIV);
+ bool gate = !(periph->gate.flags & TEGRA_PERIPH_NO_GATE);
- if (periph->gate.flags & TEGRA_PERIPH_NO_DIV) {
+ if (!div && !gate)
+ init.ops = &tegra_clk_periph_nodivgate_ops;
+ else if (periph->gate.flags & TEGRA_PERIPH_NO_DIV) {
flags |= CLK_SET_RATE_PARENT;
init.ops = &tegra_clk_periph_nodiv_ops;
} else if (periph->gate.flags & TEGRA_PERIPH_NO_GATE)
@@ -171,7 +179,7 @@ static struct clk *_tegra_clk_register_periph(const char *name,
periph->mux.reg = clk_base + offset;
periph->divider.reg = div ? (clk_base + offset) : NULL;
periph->gate.clk_base = clk_base;
- periph->gate.regs = bank;
+ periph->gate.regs = gate ? bank : NULL;
periph->gate.enable_refcnt = periph_clk_enb_refcnt;
clk = clk_register(NULL, &periph->hw);
@@ -180,7 +188,7 @@ static struct clk *_tegra_clk_register_periph(const char *name,
periph->mux.hw.clk = clk;
periph->divider.hw.clk = div ? clk : NULL;
- periph->gate.hw.clk = clk;
+ periph->gate.hw.clk = gate ? clk : NULL;
return clk;
}
--
2.18.0
The clocks vde, vi, epp, mpe, 2d and 3d are all fractional
divisors, and not integer divisors as setup in the current
kernel. This seems to be the same for tegra2 and tegra3.
Signed-off-by: Ben Dooks <[email protected]>
---
drivers/clk/tegra/clk-tegra-periph.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/clk/tegra/clk-tegra-periph.c b/drivers/clk/tegra/clk-tegra-periph.c
index 8fa1cecf18a0..ed70419f4ff9 100644
--- a/drivers/clk/tegra/clk-tegra-periph.c
+++ b/drivers/clk/tegra/clk-tegra-periph.c
@@ -641,13 +641,13 @@ static struct tegra_periph_init_data periph_clks[] = {
I2C("i2c4", mux_pllp_clkm, CLK_SOURCE_I2C4, 103, tegra_clk_i2c4),
I2C("i2c5", mux_pllp_clkm, CLK_SOURCE_I2C5, 47, tegra_clk_i2c5),
I2C("i2c6", mux_pllp_clkm, CLK_SOURCE_I2C6, 166, tegra_clk_i2c6),
- INT("vde", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_VDE, 61, 0, tegra_clk_vde),
- INT("vi", mux_pllm_pllc_pllp_plla, CLK_SOURCE_VI, 20, 0, tegra_clk_vi),
- INT("epp", mux_pllm_pllc_pllp_plla, CLK_SOURCE_EPP, 19, 0, tegra_clk_epp),
+ MUX("vde", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_VDE, 61, 0, tegra_clk_vde),
+ MUX("vi", mux_pllm_pllc_pllp_plla, CLK_SOURCE_VI, 20, 0, tegra_clk_vi),
+ MUX("epp", mux_pllm_pllc_pllp_plla, CLK_SOURCE_EPP, 19, 0, tegra_clk_epp),
MUX("host1x", mux_pllm_pllc_pllp_plla, CLK_SOURCE_HOST1X, 28, 0, tegra_clk_host1x),
- INT("mpe", mux_pllm_pllc_pllp_plla, CLK_SOURCE_MPE, 60, 0, tegra_clk_mpe),
- INT("2d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_2D, 21, 0, tegra_clk_gr2d),
- INT("3d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_3D, 24, 0, tegra_clk_gr3d),
+ MUX("mpe", mux_pllm_pllc_pllp_plla, CLK_SOURCE_MPE, 60, 0, tegra_clk_mpe),
+ MUX("2d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_2D, 21, 0, tegra_clk_gr2d),
+ MUX("3d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_3D, 24, 0, tegra_clk_gr3d),
INT8("vde", mux_pllp_pllc2_c_c3_pllm_clkm, CLK_SOURCE_VDE, 61, 0, tegra_clk_vde_8),
INT8("vi", mux_pllm_pllc2_c_c3_pllp_plla, CLK_SOURCE_VI, 20, 0, tegra_clk_vi_8),
INT8("vi", mux_pllm_pllc2_c_c3_pllp_plla_pllc4, CLK_SOURCE_VI, 20, 0, tegra_clk_vi_9),
--
2.18.0
The use of WARN_ON(1) is a bit extreme for something that is called from
very few places. Add a function to print the ID (and maybe more info if
people really want it).
This was done as during the development of the tegra-automotive branches
we got swamped with clock errors of not very useful data.
Signed-off-by: Ben Dooks <[email protected]>
---
drivers/clk/tegra/clk.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/clk/tegra/clk.c b/drivers/clk/tegra/clk.c
index a2cb3d0d38bf..3bba3509ca1f 100644
--- a/drivers/clk/tegra/clk.c
+++ b/drivers/clk/tegra/clk.c
@@ -258,6 +258,11 @@ void __init tegra_init_dup_clks(struct tegra_clk_duplicate *dup_list,
}
}
+static void tegra_clk_init_error(struct tegra_clk_init_table *tbl)
+{
+ pr_err("ERROR: Failed to initialise clock id %d\n", tbl->clk_id);
+}
+
void __init tegra_init_from_table(struct tegra_clk_init_table *tbl,
struct clk *clks[], int clk_max)
{
@@ -268,8 +273,7 @@ void __init tegra_init_from_table(struct tegra_clk_init_table *tbl,
if (IS_ERR_OR_NULL(clk)) {
pr_err("%s: invalid entry %ld in clks array for id %d\n",
__func__, PTR_ERR(clk), tbl->clk_id);
- WARN_ON(1);
-
+ tegra_clk_init_error(tbl);
continue;
}
@@ -279,7 +283,7 @@ void __init tegra_init_from_table(struct tegra_clk_init_table *tbl,
pr_err("%s: Failed to set parent %s of %s\n",
__func__, __clk_get_name(parent),
__clk_get_name(clk));
- WARN_ON(1);
+ tegra_clk_init_error(tbl);
}
}
@@ -288,14 +292,14 @@ void __init tegra_init_from_table(struct tegra_clk_init_table *tbl,
pr_err("%s: Failed to set rate %lu of %s\n",
__func__, tbl->rate,
__clk_get_name(clk));
- WARN_ON(1);
+ tegra_clk_init_error(tbl);
}
if (tbl->state)
if (clk_prepare_enable(clk)) {
pr_err("%s: Failed to enable %s\n", __func__,
__clk_get_name(clk));
- WARN_ON(1);
+ tegra_clk_init_error(tbl);
}
}
}
--
2.18.0
From: Thomas Preston <[email protected]>
The i2s clock mux should take audio_2x as a parent, but the
current macro misses the _2x suffix off the audio input clock
which means the audio_2x cannot be selected as a parent of i2s.
Fix the issue by appending the _2x prefix in the name array.
Signed-off-by: Thomas Preston <[email protected]>
---
drivers/clk/tegra/clk-tegra-periph.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/tegra/clk-tegra-periph.c b/drivers/clk/tegra/clk-tegra-periph.c
index ed70419f4ff9..47e5b1ac1a69 100644
--- a/drivers/clk/tegra/clk-tegra-periph.c
+++ b/drivers/clk/tegra/clk-tegra-periph.c
@@ -280,7 +280,7 @@ static DEFINE_SPINLOCK(sor0_lock);
#define MUX_I2S_SPDIF(_id) \
static const char *mux_pllaout0_##_id##_2x_pllp_clkm[] = { "pll_a_out0", \
- #_id, "pll_p",\
+ #_id"_2x", "pll_p",\
"clk_m"};
MUX_I2S_SPDIF(audio0)
MUX_I2S_SPDIF(audio1)
--
2.18.0
The host1x clock according to both tegra2 and tegra3 manuals is
an 8bit divider with lsb being fractional. This is running into
an issue where the host1x is being set on a tegra20a system to
266.4MHz but ends up at 222MHz instead.
Signed-off-by: Ben Dooks <[email protected]>
---
drivers/clk/tegra/clk-tegra-periph.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/tegra/clk-tegra-periph.c b/drivers/clk/tegra/clk-tegra-periph.c
index 2acba2986bc6..8fa1cecf18a0 100644
--- a/drivers/clk/tegra/clk-tegra-periph.c
+++ b/drivers/clk/tegra/clk-tegra-periph.c
@@ -644,7 +644,7 @@ static struct tegra_periph_init_data periph_clks[] = {
INT("vde", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_VDE, 61, 0, tegra_clk_vde),
INT("vi", mux_pllm_pllc_pllp_plla, CLK_SOURCE_VI, 20, 0, tegra_clk_vi),
INT("epp", mux_pllm_pllc_pllp_plla, CLK_SOURCE_EPP, 19, 0, tegra_clk_epp),
- INT("host1x", mux_pllm_pllc_pllp_plla, CLK_SOURCE_HOST1X, 28, 0, tegra_clk_host1x),
+ MUX("host1x", mux_pllm_pllc_pllp_plla, CLK_SOURCE_HOST1X, 28, 0, tegra_clk_host1x),
INT("mpe", mux_pllm_pllc_pllp_plla, CLK_SOURCE_MPE, 60, 0, tegra_clk_mpe),
INT("2d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_2D, 21, 0, tegra_clk_gr2d),
INT("3d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_3D, 24, 0, tegra_clk_gr3d),
--
2.18.0
Add a the status callback for the reset controller part of the clock
code in the tegra to allow drivers to query the status of a reset line.
Signed-off-by: Ben Dooks <[email protected]>
---
drivers/clk/tegra/clk.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/clk/tegra/clk.c b/drivers/clk/tegra/clk.c
index ffaf17f71860..a2cb3d0d38bf 100644
--- a/drivers/clk/tegra/clk.c
+++ b/drivers/clk/tegra/clk.c
@@ -146,6 +146,19 @@ static const struct tegra_clk_periph_regs periph_regs[] = {
static void __iomem *clk_base;
+static int tegra_clk_rst_status(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ void __iomem *reg;
+
+ if (id < periph_banks * 32) {
+ reg = clk_base + periph_regs[id / 32].rst_reg;
+ return readl_relaxed(reg) & BIT(id % 32) ? 1 : 0;
+ }
+
+ return -EINVAL;
+}
+
static int tegra_clk_rst_assert(struct reset_controller_dev *rcdev,
unsigned long id)
{
@@ -288,6 +301,7 @@ void __init tegra_init_from_table(struct tegra_clk_init_table *tbl,
}
static const struct reset_control_ops rst_ops = {
+ .status = tegra_clk_rst_status,
.assert = tegra_clk_rst_assert,
.deassert = tegra_clk_rst_deassert,
.reset = tegra_clk_rst_reset,
--
2.18.0
If the _calc_rate() function fails in the tegra clock code, print
the name of the clock that failed to allow debugging.
Signed-off-by: Ben Dooks <[email protected]>
---
drivers/clk/tegra/clk-pll.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
index 830d1c87fa7c..d1339a6139a1 100644
--- a/drivers/clk/tegra/clk-pll.c
+++ b/drivers/clk/tegra/clk-pll.c
@@ -576,8 +576,8 @@ static int _calc_rate(struct clk_hw *hw, struct tegra_clk_pll_freq_table *cfg,
cfreq = parent_rate / (parent_rate / 1000000);
break;
default:
- pr_err("%s Unexpected reference rate %lu\n",
- __func__, parent_rate);
+ pr_err("%s Unexpected reference rate %lu for %s\n",
+ __func__, parent_rate, __clk_get_name(hw->clk));
BUG();
}
--
2.18.0
On Friday, 20 July 2018 16:45:30 MSK Ben Dooks wrote:
> The 2D and 3D clocks have an IDLE field in bits 15:8 so add these
> clocks by making a 2D and 3D mux, and split the divider into the
> standard 2D/3D ones and 2D/3D idle clocks.
>
> Signed-off-by: Ben Dooks <[email protected]>
> ---
> drivers/clk/tegra/clk-id.h | 4 ++++
> drivers/clk/tegra/clk-tegra-periph.c | 23 +++++++++++++++++++++--
> drivers/clk/tegra/clk-tegra30.c | 8 ++++++++
> include/dt-bindings/clock/tegra30-car.h | 7 ++++++-
> 4 files changed, 39 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/tegra/clk-id.h b/drivers/clk/tegra/clk-id.h
> index b616e33c5255..0d202a70ce66 100644
> --- a/drivers/clk/tegra/clk-id.h
> +++ b/drivers/clk/tegra/clk-id.h
> @@ -91,8 +91,12 @@ enum clk_id {
> tegra_clk_fuse_burn,
> tegra_clk_gpu,
> tegra_clk_gr2d,
> + tegra_clk_gr2d_mux,
> + tegra_clk_gr2d_idle,
> tegra_clk_gr2d_8,
> tegra_clk_gr3d,
> + tegra_clk_gr3d_mux,
> + tegra_clk_gr3d_idle,
> tegra_clk_gr3d_8,
> tegra_clk_hclk,
> tegra_clk_hda,
> diff --git a/drivers/clk/tegra/clk-tegra-periph.c
> b/drivers/clk/tegra/clk-tegra-periph.c index 47e5b1ac1a69..83967dac93f2
> 100644
> --- a/drivers/clk/tegra/clk-tegra-periph.c
> +++ b/drivers/clk/tegra/clk-tegra-periph.c
> @@ -263,6 +263,21 @@
> .flags = _flags, \
> }
>
> +#define GATE_DIV(_name, _parent_name, _offset, \
> + _div_shift, _div_width, _div_frac_width, _div_flags, \
> + _clk_num, _gate_flags, _clk_id, _flags) \
> + { \
> + .name = _name, \
> + .clk_id = _clk_id, \
> + .offset = _offset, \
> + .p.parent_name = _parent_name, \
> + .periph = TEGRA_CLK_PERIPH(0, 0, 0, \
> + _div_shift, _div_width, \
> + _div_frac_width, _div_flags, \
> + _clk_num, _gate_flags, NULL, NULL), \
> + .flags = _flags \
> + }
> +
> #define PLLP_BASE 0xa0
> #define PLLP_MISC 0xac
> #define PLLP_MISC1 0x680
> @@ -646,8 +661,12 @@ static struct tegra_periph_init_data periph_clks[] = {
> MUX("epp", mux_pllm_pllc_pllp_plla, CLK_SOURCE_EPP, 19, 0,
tegra_clk_epp),
> MUX("host1x", mux_pllm_pllc_pllp_plla, CLK_SOURCE_HOST1X, 28, 0,
> tegra_clk_host1x), MUX("mpe", mux_pllm_pllc_pllp_plla, CLK_SOURCE_MPE, 60,
> 0, tegra_clk_mpe), - MUX("2d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_2D,
21,
> 0, tegra_clk_gr2d), - MUX("3d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_3D,
24,
> 0, tegra_clk_gr3d), + MUX("2d_mux", mux_pllm_pllc_pllp_plla,
CLK_SOURCE_2D,
> 0, TEGRA_PERIPH_NO_DIV | TEGRA_PERIPH_NO_RESET | TEGRA_PERIPH_NO_GATE,
> tegra_clk_gr2d_mux), + GATE_DIV("2d", "2d_mux", CLK_SOURCE_2D, 0, 8, 1,
> TEGRA_DIVIDER_ROUND_UP,21, 0, tegra_clk_gr2d, 0), + GATE_DIV("2d_idle",
> "2d_mux", CLK_SOURCE_2D, 8, 8, 1, TEGRA_DIVIDER_ROUND_UP, 0,
> TEGRA_PERIPH_NO_GATE | TEGRA_PERIPH_NO_RESET, tegra_clk_gr2d_idle, 0),
> + MUX("3d_mux", mux_pllm_pllc_pllp_plla, CLK_SOURCE_3D, 0,
> TEGRA_PERIPH_NO_DIV | TEGRA_PERIPH_NO_RESET | TEGRA_PERIPH_NO_GATE,
> tegra_clk_gr3d_mux), + GATE_DIV("3d", "3d_mux", CLK_SOURCE_3D, 0, 8, 1,
> TEGRA_DIVIDER_ROUND_UP, 24, 0, tegra_clk_gr3d, 0), + GATE_DIV("3d_idle",
> "3d_mux", CLK_SOURCE_3D, 8, 8, 1, TEGRA_DIVIDER_ROUND_UP, 0,
> TEGRA_PERIPH_NO_GATE | TEGRA_PERIPH_NO_RESET, tegra_clk_gr3d_idle, 0),
> INT8("vde", mux_pllp_pllc2_c_c3_pllm_clkm, CLK_SOURCE_VDE, 61, 0,
> tegra_clk_vde_8), INT8("vi", mux_pllm_pllc2_c_c3_pllp_plla, CLK_SOURCE_VI,
> 20, 0, tegra_clk_vi_8), INT8("vi", mux_pllm_pllc2_c_c3_pllp_plla_pllc4,
> CLK_SOURCE_VI, 20, 0, tegra_clk_vi_9), diff --git
> a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c index
> acfe661b2ae7..227d3643ecca 100644
> --- a/drivers/clk/tegra/clk-tegra30.c
> +++ b/drivers/clk/tegra/clk-tegra30.c
> @@ -658,8 +658,12 @@ static struct tegra_devclk devclks[] __initdata = {
> { .dev_id = "mpe", .dt_id = TEGRA30_CLK_MPE },
> { .dev_id = "host1x", .dt_id = TEGRA30_CLK_HOST1X },
> { .dev_id = "3d", .dt_id = TEGRA30_CLK_GR3D },
> + { .dev_id = "3d", .con_id = "mux", .dt_id = TEGRA30_CLK_GR3D_MUX },
> + { .dev_id = "3d", .con_id = "idle", .dt_id = TEGRA30_CLK_GR3D_IDLE },
> { .dev_id = "3d2", .dt_id = TEGRA30_CLK_GR3D2 },
> { .dev_id = "2d", .dt_id = TEGRA30_CLK_GR2D },
> + { .dev_id = "2d", .con_id = "mux", .dt_id = TEGRA30_CLK_GR2D_MUX },
> + { .dev_id = "2d", .con_id = "idle", .dt_id = TEGRA30_CLK_GR2D_IDLE },
> { .dev_id = "se", .dt_id = TEGRA30_CLK_SE },
> { .dev_id = "mselect", .dt_id = TEGRA30_CLK_MSELECT },
> { .dev_id = "tegra-nor", .dt_id = TEGRA30_CLK_NOR },
> @@ -762,6 +766,10 @@ static struct tegra_clk tegra30_clks[tegra_clk_max]
> __initdata = { [tegra_clk_host1x] = { .dt_id = TEGRA30_CLK_HOST1X, .present
> = true }, [tegra_clk_gr2d] = { .dt_id = TEGRA30_CLK_GR2D, .present = true
> }, [tegra_clk_gr3d] = { .dt_id = TEGRA30_CLK_GR3D, .present = true },
> + [tegra_clk_gr2d_mux] = { .dt_id = TEGRA30_CLK_GR2D_MUX, .present = true
> }, + [tegra_clk_gr3d_mux] = { .dt_id = TEGRA30_CLK_GR3D_MUX, .present =
> true }, + [tegra_clk_gr2d_idle] = { .dt_id = TEGRA30_CLK_GR2D_IDLE,
> .present = true }, + [tegra_clk_gr3d_idle] = { .dt_id =
> TEGRA30_CLK_GR3D_IDLE, .present = true }, [tegra_clk_mselect] = { .dt_id =
> TEGRA30_CLK_MSELECT, .present = true }, [tegra_clk_nor] = { .dt_id =
> TEGRA30_CLK_NOR, .present = true }, [tegra_clk_sdmmc1] = { .dt_id =
> TEGRA30_CLK_SDMMC1, .present = true }, diff --git
According to TRM, Tegra20 and Tegra114 have these "idle-mode" clock dividers
as well. Why only T30 should have them?
> a/include/dt-bindings/clock/tegra30-car.h
> b/include/dt-bindings/clock/tegra30-car.h index 3c90f1535551..eda4ca60351e
> 100644
> --- a/include/dt-bindings/clock/tegra30-car.h
> +++ b/include/dt-bindings/clock/tegra30-car.h
> @@ -269,6 +269,11 @@
> #define TEGRA30_CLK_AUDIO3_MUX 306
> #define TEGRA30_CLK_AUDIO4_MUX 307
> #define TEGRA30_CLK_SPDIF_MUX 308
> -#define TEGRA30_CLK_CLK_MAX 309
> +
> +#define TEGRA30_CLK_GR2D_MUX 309
> +#define TEGRA30_CLK_GR3D_MUX 310
> +#define TEGRA30_CLK_GR2D_IDLE 311
> +#define TEGRA30_CLK_GR3D_IDLE 312
> +#define TEGRA30_CLK_CLK_MAX 313
>
> #endif /* _DT_BINDINGS_CLOCK_TEGRA30_CAR_H */
IIUC, that "idle-mode" divisor is just some kind of power-safe feature, is
there any real use-case for these clocks? Why not to just pre-configure the
"idle-mode" bits during the clocks initialization?
On 2018-07-22 12:55, Dmitry Osipenko wrote:
> On Friday, 20 July 2018 16:45:30 MSK Ben Dooks wrote:
>> The 2D and 3D clocks have an IDLE field in bits 15:8 so add these
>> clocks by making a 2D and 3D mux, and split the divider into the
>> standard 2D/3D ones and 2D/3D idle clocks.
>>
>> Signed-off-by: Ben Dooks <[email protected]>
[snip]
>
> According to TRM, Tegra20 and Tegra114 have these "idle-mode" clock
> dividers
> as well. Why only T30 should have them?
I've got a separate series to sort t20 bits out, i've not used the
tegra114
>> a/include/dt-bindings/clock/tegra30-car.h
>> b/include/dt-bindings/clock/tegra30-car.h index
>> 3c90f1535551..eda4ca60351e
>> 100644
>> --- a/include/dt-bindings/clock/tegra30-car.h
>> +++ b/include/dt-bindings/clock/tegra30-car.h
>> @@ -269,6 +269,11 @@
>> #define TEGRA30_CLK_AUDIO3_MUX 306
>> #define TEGRA30_CLK_AUDIO4_MUX 307
>> #define TEGRA30_CLK_SPDIF_MUX 308
>> -#define TEGRA30_CLK_CLK_MAX 309
>> +
>> +#define TEGRA30_CLK_GR2D_MUX 309
>> +#define TEGRA30_CLK_GR3D_MUX 310
>> +#define TEGRA30_CLK_GR2D_IDLE 311
>> +#define TEGRA30_CLK_GR3D_IDLE 312
>> +#define TEGRA30_CLK_CLK_MAX 313
>>
>> #endif /* _DT_BINDINGS_CLOCK_TEGRA30_CAR_H */
>
> IIUC, that "idle-mode" divisor is just some kind of power-safe feature,
> is
> there any real use-case for these clocks? Why not to just pre-configure
> the
> "idle-mode" bits during the clocks initialization?
It is is nice to have it available after to check, other than that we're
not
using any drivers that currently dynamically change the values of this.
On Fri, Jul 20, 2018 at 02:45:26PM +0100, Ben Dooks wrote:
> The host1x clock according to both tegra2 and tegra3 manuals is
> an 8bit divider with lsb being fractional. This is running into
> an issue where the host1x is being set on a tegra20a system to
> 266.4MHz but ends up at 222MHz instead.
>
The fact the hw has a fractional divider, does not mean we're allowed to use
it. Due to the non 50% duty cycle of fractional divided clocks, they are not
allowed for certain peripherals. Do you have information indicating this is
ok for the host1x clock?
Peter.
> Signed-off-by: Ben Dooks <[email protected]>
> ---
> drivers/clk/tegra/clk-tegra-periph.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/tegra/clk-tegra-periph.c b/drivers/clk/tegra/clk-tegra-periph.c
> index 2acba2986bc6..8fa1cecf18a0 100644
> --- a/drivers/clk/tegra/clk-tegra-periph.c
> +++ b/drivers/clk/tegra/clk-tegra-periph.c
> @@ -644,7 +644,7 @@ static struct tegra_periph_init_data periph_clks[] = {
> INT("vde", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_VDE, 61, 0, tegra_clk_vde),
> INT("vi", mux_pllm_pllc_pllp_plla, CLK_SOURCE_VI, 20, 0, tegra_clk_vi),
> INT("epp", mux_pllm_pllc_pllp_plla, CLK_SOURCE_EPP, 19, 0, tegra_clk_epp),
> - INT("host1x", mux_pllm_pllc_pllp_plla, CLK_SOURCE_HOST1X, 28, 0, tegra_clk_host1x),
> + MUX("host1x", mux_pllm_pllc_pllp_plla, CLK_SOURCE_HOST1X, 28, 0, tegra_clk_host1x),
> INT("mpe", mux_pllm_pllc_pllp_plla, CLK_SOURCE_MPE, 60, 0, tegra_clk_mpe),
> INT("2d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_2D, 21, 0, tegra_clk_gr2d),
> INT("3d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_3D, 24, 0, tegra_clk_gr3d),
> --
> 2.18.0
>
On Fri, Jul 20, 2018 at 02:45:27PM +0100, Ben Dooks wrote:
> The clocks vde, vi, epp, mpe, 2d and 3d are all fractional
> divisors, and not integer divisors as setup in the current
> kernel. This seems to be the same for tegra2 and tegra3.
>
Same comment as the host1x clock patch.
> Signed-off-by: Ben Dooks <[email protected]>
> ---
> drivers/clk/tegra/clk-tegra-periph.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clk/tegra/clk-tegra-periph.c b/drivers/clk/tegra/clk-tegra-periph.c
> index 8fa1cecf18a0..ed70419f4ff9 100644
> --- a/drivers/clk/tegra/clk-tegra-periph.c
> +++ b/drivers/clk/tegra/clk-tegra-periph.c
> @@ -641,13 +641,13 @@ static struct tegra_periph_init_data periph_clks[] = {
> I2C("i2c4", mux_pllp_clkm, CLK_SOURCE_I2C4, 103, tegra_clk_i2c4),
> I2C("i2c5", mux_pllp_clkm, CLK_SOURCE_I2C5, 47, tegra_clk_i2c5),
> I2C("i2c6", mux_pllp_clkm, CLK_SOURCE_I2C6, 166, tegra_clk_i2c6),
> - INT("vde", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_VDE, 61, 0, tegra_clk_vde),
> - INT("vi", mux_pllm_pllc_pllp_plla, CLK_SOURCE_VI, 20, 0, tegra_clk_vi),
> - INT("epp", mux_pllm_pllc_pllp_plla, CLK_SOURCE_EPP, 19, 0, tegra_clk_epp),
> + MUX("vde", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_VDE, 61, 0, tegra_clk_vde),
> + MUX("vi", mux_pllm_pllc_pllp_plla, CLK_SOURCE_VI, 20, 0, tegra_clk_vi),
> + MUX("epp", mux_pllm_pllc_pllp_plla, CLK_SOURCE_EPP, 19, 0, tegra_clk_epp),
> MUX("host1x", mux_pllm_pllc_pllp_plla, CLK_SOURCE_HOST1X, 28, 0, tegra_clk_host1x),
> - INT("mpe", mux_pllm_pllc_pllp_plla, CLK_SOURCE_MPE, 60, 0, tegra_clk_mpe),
> - INT("2d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_2D, 21, 0, tegra_clk_gr2d),
> - INT("3d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_3D, 24, 0, tegra_clk_gr3d),
> + MUX("mpe", mux_pllm_pllc_pllp_plla, CLK_SOURCE_MPE, 60, 0, tegra_clk_mpe),
> + MUX("2d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_2D, 21, 0, tegra_clk_gr2d),
> + MUX("3d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_3D, 24, 0, tegra_clk_gr3d),
> INT8("vde", mux_pllp_pllc2_c_c3_pllm_clkm, CLK_SOURCE_VDE, 61, 0, tegra_clk_vde_8),
> INT8("vi", mux_pllm_pllc2_c_c3_pllp_plla, CLK_SOURCE_VI, 20, 0, tegra_clk_vi_8),
> INT8("vi", mux_pllm_pllc2_c_c3_pllp_plla_pllc4, CLK_SOURCE_VI, 20, 0, tegra_clk_vi_9),
> --
> 2.18.0
>
On 2018-07-23 09:50, Peter De Schrijver wrote:
> On Fri, Jul 20, 2018 at 02:45:26PM +0100, Ben Dooks wrote:
>> The host1x clock according to both tegra2 and tegra3 manuals is
>> an 8bit divider with lsb being fractional. This is running into
>> an issue where the host1x is being set on a tegra20a system to
>> 266.4MHz but ends up at 222MHz instead.
>>
>
> The fact the hw has a fractional divider, does not mean we're allowed
> to use
> it. Due to the non 50% duty cycle of fractional divided clocks, they
> are not
> allowed for certain peripherals. Do you have information indicating
> this is
> ok for the host1x clock?
Only that's what was setup for the systems we're using.
We couldn't match the 2.6 working system without these changes.
--
Ben
On Mon, Jul 23, 2018 at 10:32:58AM +0100, Ben Dooks wrote:
>
>
> On 2018-07-23 09:50, Peter De Schrijver wrote:
> >On Fri, Jul 20, 2018 at 02:45:26PM +0100, Ben Dooks wrote:
> >>The host1x clock according to both tegra2 and tegra3 manuals is
> >>an 8bit divider with lsb being fractional. This is running into
> >>an issue where the host1x is being set on a tegra20a system to
> >>266.4MHz but ends up at 222MHz instead.
> >>
> >
> >The fact the hw has a fractional divider, does not mean we're
> >allowed to use
> >it. Due to the non 50% duty cycle of fractional divided clocks,
> >they are not
> >allowed for certain peripherals. Do you have information
> >indicating this is
> >ok for the host1x clock?
>
> Only that's what was setup for the systems we're using.
> We couldn't match the 2.6 working system without these changes.
>
On Tegra20 or Tegra30?
Peter.
On 2018-07-23 12:12, Peter De Schrijver wrote:
> On Mon, Jul 23, 2018 at 10:32:58AM +0100, Ben Dooks wrote:
>>
>>
>> On 2018-07-23 09:50, Peter De Schrijver wrote:
>> >On Fri, Jul 20, 2018 at 02:45:26PM +0100, Ben Dooks wrote:
>> >>The host1x clock according to both tegra2 and tegra3 manuals is
>> >>an 8bit divider with lsb being fractional. This is running into
>> >>an issue where the host1x is being set on a tegra20a system to
>> >>266.4MHz but ends up at 222MHz instead.
>> >>
>> >
>> >The fact the hw has a fractional divider, does not mean we're
>> >allowed to use
>> >it. Due to the non 50% duty cycle of fractional divided clocks,
>> >they are not
>> >allowed for certain peripherals. Do you have information
>> >indicating this is
>> >ok for the host1x clock?
>>
>> Only that's what was setup for the systems we're using.
>> We couldn't match the 2.6 working system without these changes.
>>
>
> On Tegra20 or Tegra30?
I'll check tomorrow when I have access to all the hw involved.
--
Ben
On Monday, 23 July 2018 11:28:25 MSK Ben Dooks wrote:
> On 2018-07-22 12:55, Dmitry Osipenko wrote:
> > On Friday, 20 July 2018 16:45:30 MSK Ben Dooks wrote:
> >> The 2D and 3D clocks have an IDLE field in bits 15:8 so add these
> >> clocks by making a 2D and 3D mux, and split the divider into the
> >> standard 2D/3D ones and 2D/3D idle clocks.
> >>
> >> Signed-off-by: Ben Dooks <[email protected]>
>
> [snip]
> @@ -658,8 +658,12 @@ static struct tegra_devclk devclks[] __initdata = {
> { .dev_id = "mpe", .dt_id = TEGRA30_CLK_MPE },
> { .dev_id = "host1x", .dt_id = TEGRA30_CLK_HOST1X },
> { .dev_id = "3d", .dt_id = TEGRA30_CLK_GR3D },
> + { .dev_id = "3d", .con_id = "mux", .dt_id = TEGRA30_CLK_GR3D_MUX },
> + { .dev_id = "3d", .con_id = "idle", .dt_id = TEGRA30_CLK_GR3D_IDLE },
> { .dev_id = "3d2", .dt_id = TEGRA30_CLK_GR3D2 },
The "3d2" also has the "idle" divisor, why have you skipped it?
> { .dev_id = "2d", .dt_id = TEGRA30_CLK_GR2D },
> + { .dev_id = "2d", .con_id = "mux", .dt_id = TEGRA30_CLK_GR2D_MUX },
> + { .dev_id = "2d", .con_id = "idle", .dt_id = TEGRA30_CLK_GR2D_IDLE },
> { .dev_id = "se", .dt_id = TEGRA30_CLK_SE },
> { .dev_id = "mselect", .dt_id = TEGRA30_CLK_MSELECT },
> { .dev_id = "tegra-nor", .dt_id = TEGRA30_CLK_NOR },
[snip]
>
> > According to TRM, Tegra20 and Tegra114 have these "idle-mode" clock
> > dividers
> > as well. Why only T30 should have them?
>
> I've got a separate series to sort t20 bits out, i've not used the
> tegra114
>
This makes this series to look a bit inconsistent, please send out all the
patches to give a consistent view.
I don't see anything that could really stop you from adding the clocks for
T114, its 2d/3d clocks definition pretty matches to T20/30.
> >> a/include/dt-bindings/clock/tegra30-car.h
> >> b/include/dt-bindings/clock/tegra30-car.h index
> >> 3c90f1535551..eda4ca60351e
> >> 100644
> >> --- a/include/dt-bindings/clock/tegra30-car.h
> >> +++ b/include/dt-bindings/clock/tegra30-car.h
> >> @@ -269,6 +269,11 @@
> >>
> >> #define TEGRA30_CLK_AUDIO3_MUX 306
> >> #define TEGRA30_CLK_AUDIO4_MUX 307
> >> #define TEGRA30_CLK_SPDIF_MUX 308
> >>
> >> -#define TEGRA30_CLK_CLK_MAX 309
> >> +
> >> +#define TEGRA30_CLK_GR2D_MUX 309
> >> +#define TEGRA30_CLK_GR3D_MUX 310
> >> +#define TEGRA30_CLK_GR2D_IDLE 311
> >> +#define TEGRA30_CLK_GR3D_IDLE 312
> >> +#define TEGRA30_CLK_CLK_MAX 313
> >>
> >> #endif /* _DT_BINDINGS_CLOCK_TEGRA30_CAR_H */
> >
> > IIUC, that "idle-mode" divisor is just some kind of power-safe feature,
> > is
> > there any real use-case for these clocks? Why not to just pre-configure
> > the
> > "idle-mode" bits during the clocks initialization?
>
> It is is nice to have it available after to check,
Please initialize the "idle" clock rate via the tegra_clk_init_table in the
patch that adds the clock or in a followup patch within the same patchset.
> other than that we're
> not
> using any drivers that currently dynamically change the values of this.
All changes made to upstream kernel must be justified, the only acceptable
justification is that a change is required for the upstream driver.
On 2018-07-23 12:33, Dmitry Osipenko wrote:
> On Monday, 23 July 2018 11:28:25 MSK Ben Dooks wrote:
>> On 2018-07-22 12:55, Dmitry Osipenko wrote:
>> > On Friday, 20 July 2018 16:45:30 MSK Ben Dooks wrote:
>> >> The 2D and 3D clocks have an IDLE field in bits 15:8 so add these
>> >> clocks by making a 2D and 3D mux, and split the divider into the
>> >> standard 2D/3D ones and 2D/3D idle clocks.
>> >>
>> >> Signed-off-by: Ben Dooks <[email protected]>
>>
>> [snip]
>
>> @@ -658,8 +658,12 @@ static struct tegra_devclk devclks[] __initdata =
>> {
>> { .dev_id = "mpe", .dt_id = TEGRA30_CLK_MPE },
>> { .dev_id = "host1x", .dt_id = TEGRA30_CLK_HOST1X },
>> { .dev_id = "3d", .dt_id = TEGRA30_CLK_GR3D },
>> + { .dev_id = "3d", .con_id = "mux", .dt_id = TEGRA30_CLK_GR3D_MUX },
>> + { .dev_id = "3d", .con_id = "idle", .dt_id = TEGRA30_CLK_GR3D_IDLE
>> },
>> { .dev_id = "3d2", .dt_id = TEGRA30_CLK_GR3D2 },
>
> The "3d2" also has the "idle" divisor, why have you skipped it?
Thanks, missed this.
>> { .dev_id = "2d", .dt_id = TEGRA30_CLK_GR2D },
>> + { .dev_id = "2d", .con_id = "mux", .dt_id = TEGRA30_CLK_GR2D_MUX },
>> + { .dev_id = "2d", .con_id = "idle", .dt_id = TEGRA30_CLK_GR2D_IDLE
>> },
>> { .dev_id = "se", .dt_id = TEGRA30_CLK_SE },
>> { .dev_id = "mselect", .dt_id = TEGRA30_CLK_MSELECT },
>> { .dev_id = "tegra-nor", .dt_id = TEGRA30_CLK_NOR },
>
> [snip]
>
>>
>> > According to TRM, Tegra20 and Tegra114 have these "idle-mode" clock
>> > dividers
>> > as well. Why only T30 should have them?
>>
>> I've got a separate series to sort t20 bits out, i've not used the
>> tegra114
>>
>
> This makes this series to look a bit inconsistent, please send out all
> the
> patches to give a consistent view.
>
> I don't see anything that could really stop you from adding the clocks
> for
> T114, its 2d/3d clocks definition pretty matches to T20/30.
I'd prefer not to be touch architectures I don't have access to do.
> >> a/include/dt-bindings/clock/tegra30-car.h
>> >> b/include/dt-bindings/clock/tegra30-car.h index
>> >> 3c90f1535551..eda4ca60351e
>> >> 100644
>> >> --- a/include/dt-bindings/clock/tegra30-car.h
>> >> +++ b/include/dt-bindings/clock/tegra30-car.h
>> >> @@ -269,6 +269,11 @@
>> >>
>> >> #define TEGRA30_CLK_AUDIO3_MUX 306
>> >> #define TEGRA30_CLK_AUDIO4_MUX 307
>> >> #define TEGRA30_CLK_SPDIF_MUX 308
>> >>
>> >> -#define TEGRA30_CLK_CLK_MAX 309
>> >> +
>> >> +#define TEGRA30_CLK_GR2D_MUX 309
>> >> +#define TEGRA30_CLK_GR3D_MUX 310
>> >> +#define TEGRA30_CLK_GR2D_IDLE 311
>> >> +#define TEGRA30_CLK_GR3D_IDLE 312
>> >> +#define TEGRA30_CLK_CLK_MAX 313
>> >>
>> >> #endif /* _DT_BINDINGS_CLOCK_TEGRA30_CAR_H */
>> >
>> > IIUC, that "idle-mode" divisor is just some kind of power-safe feature,
>> > is
>> > there any real use-case for these clocks? Why not to just pre-configure
>> > the
>> > "idle-mode" bits during the clocks initialization?
>>
>> It is is nice to have it available after to check,
>
> Please initialize the "idle" clock rate via the tegra_clk_init_table in
> the
> patch that adds the clock or in a followup patch within the same
> patchset.
>
>> other than that we're
>> not
>> using any drivers that currently dynamically change the values of
>> this.
>
> All changes made to upstream kernel must be justified, the only
> acceptable
> justification is that a change is required for the upstream driver.
On Friday, 20 July 2018 16:45:30 MSK Ben Dooks wrote:
> The 2D and 3D clocks have an IDLE field in bits 15:8 so add these
> clocks by making a 2D and 3D mux, and split the divider into the
> standard 2D/3D ones and 2D/3D idle clocks.
>
> Signed-off-by: Ben Dooks <[email protected]>
> ---
> drivers/clk/tegra/clk-id.h | 4 ++++
> drivers/clk/tegra/clk-tegra-periph.c | 23 +++++++++++++++++++++--
> drivers/clk/tegra/clk-tegra30.c | 8 ++++++++
> include/dt-bindings/clock/tegra30-car.h | 7 ++++++-
> 4 files changed, 39 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/tegra/clk-id.h b/drivers/clk/tegra/clk-id.h
> index b616e33c5255..0d202a70ce66 100644
> --- a/drivers/clk/tegra/clk-id.h
> +++ b/drivers/clk/tegra/clk-id.h
> @@ -91,8 +91,12 @@ enum clk_id {
> tegra_clk_fuse_burn,
> tegra_clk_gpu,
> tegra_clk_gr2d,
> + tegra_clk_gr2d_mux,
> + tegra_clk_gr2d_idle,
> tegra_clk_gr2d_8,
> tegra_clk_gr3d,
> + tegra_clk_gr3d_mux,
> + tegra_clk_gr3d_idle,
> tegra_clk_gr3d_8,
> tegra_clk_hclk,
> tegra_clk_hda,
> diff --git a/drivers/clk/tegra/clk-tegra-periph.c
> b/drivers/clk/tegra/clk-tegra-periph.c index 47e5b1ac1a69..83967dac93f2
> 100644
> --- a/drivers/clk/tegra/clk-tegra-periph.c
> +++ b/drivers/clk/tegra/clk-tegra-periph.c
> @@ -263,6 +263,21 @@
> .flags = _flags, \
> }
>
> +#define GATE_DIV(_name, _parent_name, _offset, \
> + _div_shift, _div_width, _div_frac_width, _div_flags, \
> + _clk_num, _gate_flags, _clk_id, _flags) \
> + { \
> + .name = _name, \
> + .clk_id = _clk_id, \
> + .offset = _offset, \
> + .p.parent_name = _parent_name, \
> + .periph = TEGRA_CLK_PERIPH(0, 0, 0, \
> + _div_shift, _div_width, \
> + _div_frac_width, _div_flags, \
> + _clk_num, _gate_flags, NULL, NULL), \
> + .flags = _flags \
> + }
> +
> #define PLLP_BASE 0xa0
> #define PLLP_MISC 0xac
> #define PLLP_MISC1 0x680
> @@ -646,8 +661,12 @@ static struct tegra_periph_init_data periph_clks[] = {
> MUX("epp", mux_pllm_pllc_pllp_plla, CLK_SOURCE_EPP, 19, 0,
tegra_clk_epp),
> MUX("host1x", mux_pllm_pllc_pllp_plla, CLK_SOURCE_HOST1X, 28, 0,
> tegra_clk_host1x), MUX("mpe", mux_pllm_pllc_pllp_plla, CLK_SOURCE_MPE, 60,
> 0, tegra_clk_mpe), - MUX("2d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_2D,
21,
> 0, tegra_clk_gr2d), - MUX("3d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_3D,
24,
> 0, tegra_clk_gr3d), + MUX("2d_mux", mux_pllm_pllc_pllp_plla,
CLK_SOURCE_2D,
> 0, TEGRA_PERIPH_NO_DIV | TEGRA_PERIPH_NO_RESET | TEGRA_PERIPH_NO_GATE,
> tegra_clk_gr2d_mux), + GATE_DIV("2d", "2d_mux", CLK_SOURCE_2D, 0, 8, 1,
> TEGRA_DIVIDER_ROUND_UP,21, 0, tegra_clk_gr2d, 0), + GATE_DIV("2d_idle",
> "2d_mux", CLK_SOURCE_2D, 8, 8, 1, TEGRA_DIVIDER_ROUND_UP, 0,
> TEGRA_PERIPH_NO_GATE | TEGRA_PERIPH_NO_RESET, tegra_clk_gr2d_idle, 0),
> + MUX("3d_mux", mux_pllm_pllc_pllp_plla, CLK_SOURCE_3D, 0,
Now that the actual parent clock is specified by the "mux" clock, you have to
adjust the tegra_clk_init_table accordingly.
On Monday, 23 July 2018 16:05:21 MSK Dmitry Osipenko wrote:
> On Friday, 20 July 2018 16:45:30 MSK Ben Dooks wrote:
> > The 2D and 3D clocks have an IDLE field in bits 15:8 so add these
> > clocks by making a 2D and 3D mux, and split the divider into the
> > standard 2D/3D ones and 2D/3D idle clocks.
> >
> > Signed-off-by: Ben Dooks <[email protected]>
> > ---
> > drivers/clk/tegra/clk-id.h | 4 ++++
> > drivers/clk/tegra/clk-tegra-periph.c | 23 +++++++++++++++++++++--
> > drivers/clk/tegra/clk-tegra30.c | 8 ++++++++
> > include/dt-bindings/clock/tegra30-car.h | 7 ++++++-
> > 4 files changed, 39 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/clk/tegra/clk-id.h b/drivers/clk/tegra/clk-id.h
> > index b616e33c5255..0d202a70ce66 100644
> > --- a/drivers/clk/tegra/clk-id.h
> > +++ b/drivers/clk/tegra/clk-id.h
> > @@ -91,8 +91,12 @@ enum clk_id {
> > tegra_clk_fuse_burn,
> > tegra_clk_gpu,
> > tegra_clk_gr2d,
> > + tegra_clk_gr2d_mux,
> > + tegra_clk_gr2d_idle,
> > tegra_clk_gr2d_8,
> > tegra_clk_gr3d,
> > + tegra_clk_gr3d_mux,
> > + tegra_clk_gr3d_idle,
> > tegra_clk_gr3d_8,
> > tegra_clk_hclk,
> > tegra_clk_hda,
> > diff --git a/drivers/clk/tegra/clk-tegra-periph.c
> > b/drivers/clk/tegra/clk-tegra-periph.c index 47e5b1ac1a69..83967dac93f2
> > 100644
> > --- a/drivers/clk/tegra/clk-tegra-periph.c
> > +++ b/drivers/clk/tegra/clk-tegra-periph.c
> > @@ -263,6 +263,21 @@
> > .flags = _flags, \
> > }
> >
> > +#define GATE_DIV(_name, _parent_name, _offset, \
> > + _div_shift, _div_width, _div_frac_width, _div_flags, \
> > + _clk_num, _gate_flags, _clk_id, _flags) \
> > + { \
> > + .name = _name, \
> > + .clk_id = _clk_id, \
> > + .offset = _offset, \
> > + .p.parent_name = _parent_name, \
> > + .periph = TEGRA_CLK_PERIPH(0, 0, 0, \
> > + _div_shift, _div_width, \
> > + _div_frac_width, _div_flags, \
> > + _clk_num, _gate_flags, NULL, NULL), \
> > + .flags = _flags \
> > + }
> > +
> > #define PLLP_BASE 0xa0
> > #define PLLP_MISC 0xac
> > #define PLLP_MISC1 0x680
> > @@ -646,8 +661,12 @@ static struct tegra_periph_init_data periph_clks[] = {
> > MUX("epp", mux_pllm_pllc_pllp_plla, CLK_SOURCE_EPP, 19, 0,
> tegra_clk_epp),
> > MUX("host1x", mux_pllm_pllc_pllp_plla, CLK_SOURCE_HOST1X, 28, 0,
> > tegra_clk_host1x), MUX("mpe", mux_pllm_pllc_pllp_plla, CLK_SOURCE_MPE, 60,
> > 0, tegra_clk_mpe), - MUX("2d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_2D,
> 21,
> > 0, tegra_clk_gr2d), - MUX("3d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_3D,
> 24,
> > 0, tegra_clk_gr3d), + MUX("2d_mux", mux_pllm_pllc_pllp_plla,
> CLK_SOURCE_2D,
> > 0, TEGRA_PERIPH_NO_DIV | TEGRA_PERIPH_NO_RESET | TEGRA_PERIPH_NO_GATE,
> > tegra_clk_gr2d_mux), + GATE_DIV("2d", "2d_mux", CLK_SOURCE_2D, 0, 8, 1,
> > TEGRA_DIVIDER_ROUND_UP,21, 0, tegra_clk_gr2d, 0), + GATE_DIV("2d_idle",
> > "2d_mux", CLK_SOURCE_2D, 8, 8, 1, TEGRA_DIVIDER_ROUND_UP, 0,
> > TEGRA_PERIPH_NO_GATE | TEGRA_PERIPH_NO_RESET, tegra_clk_gr2d_idle, 0),
> > + MUX("3d_mux", mux_pllm_pllc_pllp_plla, CLK_SOURCE_3D, 0,
>
> Now that the actual parent clock is specified by the "mux" clock, you have to
> adjust the tegra_clk_init_table accordingly.
>
>
>
>
The 3d/2d clocks do not have a parent on T20 with these patches being applied.
# cat /sys/kernel/debug/clk/clk_summary
enable prepare protect duty
clock count count count rate accuracy phase cycle
---------------------------------------------------------------------------------------------
clock 0 0 0 32768 0 0 50000
3d 1 1 0 0 0 0 50000
2d 1 1 0 0 0 0 50000
clk_32k 2 2 0 32768 0 0 50000
blink_override 1 1 0 32768 0 0 50000
blink 2 2 0 32768 0 0 50000
kbc 0 0 0 32768 0 0 50000
rtc 2 2 0 32768 0 0 50000
...
Same on T30.
# cat /sys/kernel/debug/clk/clk_summary
enable prepare protect duty
clock count count count rate accuracy phase cycle
---------------------------------------------------------------------------------------------
...
3d_idle 0 0 0 0 0 0 50000
3d 1 1 0 0 0 0 50000
2d_idle 0 0 0 0 0 0 50000
2d 1 1 0 0 0 0 50000
clk_32k 2 2 0 32768 0 0 50000
blink_override 1 1 0 32768 0 0 50000
...
On 24/07/18 12:31, Dmitry Osipenko wrote:
> On Monday, 23 July 2018 16:05:21 MSK Dmitry Osipenko wrote:
>> On Friday, 20 July 2018 16:45:30 MSK Ben Dooks wrote:
>>> The 2D and 3D clocks have an IDLE field in bits 15:8 so add these
>>> clocks by making a 2D and 3D mux, and split the divider into the
>>> standard 2D/3D ones and 2D/3D idle clocks.
>>>
>>> Signed-off-by: Ben Dooks <[email protected]>
>>> ---
>>> drivers/clk/tegra/clk-id.h | 4 ++++
>>> drivers/clk/tegra/clk-tegra-periph.c | 23 +++++++++++++++++++++--
>>> drivers/clk/tegra/clk-tegra30.c | 8 ++++++++
>>> include/dt-bindings/clock/tegra30-car.h | 7 ++++++-
>>> 4 files changed, 39 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/clk/tegra/clk-id.h b/drivers/clk/tegra/clk-id.h
>>> index b616e33c5255..0d202a70ce66 100644
>>> --- a/drivers/clk/tegra/clk-id.h
>>> +++ b/drivers/clk/tegra/clk-id.h
>>> @@ -91,8 +91,12 @@ enum clk_id {
>>> tegra_clk_fuse_burn,
>>> tegra_clk_gpu,
>>> tegra_clk_gr2d,
>>> + tegra_clk_gr2d_mux,
>>> + tegra_clk_gr2d_idle,
>>> tegra_clk_gr2d_8,
>>> tegra_clk_gr3d,
>>> + tegra_clk_gr3d_mux,
>>> + tegra_clk_gr3d_idle,
>>> tegra_clk_gr3d_8,
>>> tegra_clk_hclk,
>>> tegra_clk_hda,
>>> diff --git a/drivers/clk/tegra/clk-tegra-periph.c
>>> b/drivers/clk/tegra/clk-tegra-periph.c index 47e5b1ac1a69..83967dac93f2
>>> 100644
>>> --- a/drivers/clk/tegra/clk-tegra-periph.c
>>> +++ b/drivers/clk/tegra/clk-tegra-periph.c
>>> @@ -263,6 +263,21 @@
>>> .flags = _flags, \
>>> }
>>>
>>> +#define GATE_DIV(_name, _parent_name, _offset, \
>>> + _div_shift, _div_width, _div_frac_width, _div_flags, \
>>> + _clk_num, _gate_flags, _clk_id, _flags) \
>>> + { \
>>> + .name = _name, \
>>> + .clk_id = _clk_id, \
>>> + .offset = _offset, \
>>> + .p.parent_name = _parent_name, \
>>> + .periph = TEGRA_CLK_PERIPH(0, 0, 0, \
>>> + _div_shift, _div_width, \
>>> + _div_frac_width, _div_flags, \
>>> + _clk_num, _gate_flags, NULL, NULL), \
>>> + .flags = _flags \
>>> + }
>>> +
>>> #define PLLP_BASE 0xa0
>>> #define PLLP_MISC 0xac
>>> #define PLLP_MISC1 0x680
>>> @@ -646,8 +661,12 @@ static struct tegra_periph_init_data periph_clks[] = {
>>> MUX("epp", mux_pllm_pllc_pllp_plla, CLK_SOURCE_EPP, 19, 0,
>> tegra_clk_epp),
>>> MUX("host1x", mux_pllm_pllc_pllp_plla, CLK_SOURCE_HOST1X, 28, 0,
>>> tegra_clk_host1x), MUX("mpe", mux_pllm_pllc_pllp_plla, CLK_SOURCE_MPE, 60,
>>> 0, tegra_clk_mpe), - MUX("2d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_2D,
>> 21,
>>> 0, tegra_clk_gr2d), - MUX("3d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_3D,
>> 24,
>>> 0, tegra_clk_gr3d), + MUX("2d_mux", mux_pllm_pllc_pllp_plla,
>> CLK_SOURCE_2D,
>>> 0, TEGRA_PERIPH_NO_DIV | TEGRA_PERIPH_NO_RESET | TEGRA_PERIPH_NO_GATE,
>>> tegra_clk_gr2d_mux), + GATE_DIV("2d", "2d_mux", CLK_SOURCE_2D, 0, 8, 1,
>>> TEGRA_DIVIDER_ROUND_UP,21, 0, tegra_clk_gr2d, 0), + GATE_DIV("2d_idle",
>>> "2d_mux", CLK_SOURCE_2D, 8, 8, 1, TEGRA_DIVIDER_ROUND_UP, 0,
>>> TEGRA_PERIPH_NO_GATE | TEGRA_PERIPH_NO_RESET, tegra_clk_gr2d_idle, 0),
>>> + MUX("3d_mux", mux_pllm_pllc_pllp_plla, CLK_SOURCE_3D, 0,
>>
>> Now that the actual parent clock is specified by the "mux" clock, you have to
>> adjust the tegra_clk_init_table accordingly.
>>
>>
>>
>>
>
> The 3d/2d clocks do not have a parent on T20 with these patches being applied.
Thanks, I need to extract the setup from some of the other changes that
we did for the tegra30a/20a devices.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
https://www.codethink.co.uk/privacy.html
On 2018-07-20 14:45, Ben Dooks wrote:
> Hi, this is a set of clock updates and fixes we did when developing
> the tegra20/tegra30 auto support. The audio change is due to using
> the tegra in clock-slave mode (this hasn't been done before)
>
> We have also implemented the clock reset status callback as it is
> used by some of the driver work we did.
Is it possible to look at applying the bits of the series that are
not currently under discussion? If so should I post a smaller series?
--
Ben
Quoting Ben Dooks (2018-07-26 06:32:29)
> On 2018-07-20 14:45, Ben Dooks wrote:
> > Hi, this is a set of clock updates and fixes we did when developing
> > the tegra20/tegra30 auto support. The audio change is due to using
> > the tegra in clock-slave mode (this hasn't been done before)
> >
> > We have also implemented the clock reset status callback as it is
> > used by some of the driver work we did.
>
> Is it possible to look at applying the bits of the series that are
> not currently under discussion? If so should I post a smaller series?
>
Just send it again please. I'd prefer to see some reviewed-by tags from
Nvidia folks on bits that are good to merge.
On 2018-07-26 16:22, Stephen Boyd wrote:
> Quoting Ben Dooks (2018-07-26 06:32:29)
>> On 2018-07-20 14:45, Ben Dooks wrote:
>> > Hi, this is a set of clock updates and fixes we did when developing
>> > the tegra20/tegra30 auto support. The audio change is due to using
>> > the tegra in clock-slave mode (this hasn't been done before)
>> >
>> > We have also implemented the clock reset status callback as it is
>> > used by some of the driver work we did.
>>
>> Is it possible to look at applying the bits of the series that are
>> not currently under discussion? If so should I post a smaller series?
>>
>
> Just send it again please. I'd prefer to see some reviewed-by tags from
> Nvidia folks on bits that are good to merge.
I've seen feedback on some of the tegra3 clock bits, however no one
has made any comments on other bits like the reset controller.
--
Ben