All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] clk: meson8b: updates for video clocks / resets
@ 2020-04-14 20:00 ` Martin Blumenstingl
  0 siblings, 0 replies; 30+ messages in thread
From: Martin Blumenstingl @ 2020-04-14 20:00 UTC (permalink / raw)
  To: jbrunet, linux-amlogic, linux-clk
  Cc: narmstrong, mturquette, sboyd, linux-arm-kernel, linux-kernel,
	Martin Blumenstingl

This is the first batch of fixes and updates for the Meson8/8b/8m2
clock controller driver.

The first patch fixes the video clock hierarchy. Special thanks to
Neil for providing a lot of details about the video clock tree!

The second and third came up while testing video output on my EC-100
(Endless Mini). This board is special because u-boot does not enable
the video outputs like most other u-boot versions do. However, this
is very useful for development because it shows (the hard way ;))
where the existing code is buggy.

The last patch is a small improvement for the VPU clock so we
utilize the glitch-free mux (on SoCs which support it) and avoid
problems by changing the "live" clock tree at runtime (with the mali
clock this resulted in system hangs/freezes).

In my opinion all of these patches - including the fixes - can go to
"next" because the relevant clock trees are still read-only.


Martin Blumenstingl (4):
  clk: meson: meson8b: Fix the first parent of vid_pll_in_sel
  clk: meson: meson8b: Fix the polarity of the RESET_N lines
  clk: meson: meson8b: Fix the vclk_div{1,2,4,6,12}_en gate bits
  clk: meson: meson8b: Make the CCF use the glitch-free VPU mux

 drivers/clk/meson/meson8b.c | 107 +++++++++++++++++++++++++-----------
 1 file changed, 75 insertions(+), 32 deletions(-)

-- 
2.26.0


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 0/4] clk: meson8b: updates for video clocks / resets
@ 2020-04-14 20:00 ` Martin Blumenstingl
  0 siblings, 0 replies; 30+ messages in thread
From: Martin Blumenstingl @ 2020-04-14 20:00 UTC (permalink / raw)
  To: jbrunet, linux-amlogic, linux-clk
  Cc: narmstrong, sboyd, mturquette, linux-kernel, Martin Blumenstingl,
	linux-arm-kernel

This is the first batch of fixes and updates for the Meson8/8b/8m2
clock controller driver.

The first patch fixes the video clock hierarchy. Special thanks to
Neil for providing a lot of details about the video clock tree!

The second and third came up while testing video output on my EC-100
(Endless Mini). This board is special because u-boot does not enable
the video outputs like most other u-boot versions do. However, this
is very useful for development because it shows (the hard way ;))
where the existing code is buggy.

The last patch is a small improvement for the VPU clock so we
utilize the glitch-free mux (on SoCs which support it) and avoid
problems by changing the "live" clock tree at runtime (with the mali
clock this resulted in system hangs/freezes).

In my opinion all of these patches - including the fixes - can go to
"next" because the relevant clock trees are still read-only.


Martin Blumenstingl (4):
  clk: meson: meson8b: Fix the first parent of vid_pll_in_sel
  clk: meson: meson8b: Fix the polarity of the RESET_N lines
  clk: meson: meson8b: Fix the vclk_div{1,2,4,6,12}_en gate bits
  clk: meson: meson8b: Make the CCF use the glitch-free VPU mux

 drivers/clk/meson/meson8b.c | 107 +++++++++++++++++++++++++-----------
 1 file changed, 75 insertions(+), 32 deletions(-)

-- 
2.26.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 0/4] clk: meson8b: updates for video clocks / resets
@ 2020-04-14 20:00 ` Martin Blumenstingl
  0 siblings, 0 replies; 30+ messages in thread
From: Martin Blumenstingl @ 2020-04-14 20:00 UTC (permalink / raw)
  To: jbrunet, linux-amlogic, linux-clk
  Cc: narmstrong, sboyd, mturquette, linux-kernel, Martin Blumenstingl,
	linux-arm-kernel

This is the first batch of fixes and updates for the Meson8/8b/8m2
clock controller driver.

The first patch fixes the video clock hierarchy. Special thanks to
Neil for providing a lot of details about the video clock tree!

The second and third came up while testing video output on my EC-100
(Endless Mini). This board is special because u-boot does not enable
the video outputs like most other u-boot versions do. However, this
is very useful for development because it shows (the hard way ;))
where the existing code is buggy.

The last patch is a small improvement for the VPU clock so we
utilize the glitch-free mux (on SoCs which support it) and avoid
problems by changing the "live" clock tree at runtime (with the mali
clock this resulted in system hangs/freezes).

In my opinion all of these patches - including the fixes - can go to
"next" because the relevant clock trees are still read-only.


Martin Blumenstingl (4):
  clk: meson: meson8b: Fix the first parent of vid_pll_in_sel
  clk: meson: meson8b: Fix the polarity of the RESET_N lines
  clk: meson: meson8b: Fix the vclk_div{1,2,4,6,12}_en gate bits
  clk: meson: meson8b: Make the CCF use the glitch-free VPU mux

 drivers/clk/meson/meson8b.c | 107 +++++++++++++++++++++++++-----------
 1 file changed, 75 insertions(+), 32 deletions(-)

-- 
2.26.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1/4] clk: meson: meson8b: Fix the first parent of vid_pll_in_sel
  2020-04-14 20:00 ` Martin Blumenstingl
  (?)
@ 2020-04-14 20:00   ` Martin Blumenstingl
  -1 siblings, 0 replies; 30+ messages in thread
From: Martin Blumenstingl @ 2020-04-14 20:00 UTC (permalink / raw)
  To: jbrunet, linux-amlogic, linux-clk
  Cc: narmstrong, mturquette, sboyd, linux-arm-kernel, linux-kernel,
	Martin Blumenstingl

Use hdmi_pll_lvds_out as parent of the vid_pll_in_sel clock. It's not
easy to see that the vendor kernel does the same, but it actually does.
meson_clk_pll_ops in mainline still cannot fully recalculate all rates
from the HDMI PLL registers because some register bits (at the time of
writing it's unknown which bits are used for this) double the HDMI PLL
output rate (compared to simply considering M, N and FRAC).

Update the vid_pll_in_sel parent so our clock calculation works for
simple clock settings like the CVBS output (where no rate doubling is
going on). The PLL ops need to be fixed later on for more complex clock
settings (all HDMI rates).

Fixes: 6cb57c678bb70 ("clk: meson: meson8b: add the read-only video clock trees")
Suggested-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/clk/meson/meson8b.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index 7c55c695cbae..90d284ffc780 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -1077,7 +1077,7 @@ static struct clk_regmap meson8b_vid_pll_in_sel = {
 		 * Meson8m2: vid2_pll
 		 */
 		.parent_hws = (const struct clk_hw *[]) {
-			&meson8b_hdmi_pll_dco.hw
+			&meson8b_hdmi_pll_lvds_out.hw
 		},
 		.num_parents = 1,
 		.flags = CLK_SET_RATE_PARENT,
-- 
2.26.0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 1/4] clk: meson: meson8b: Fix the first parent of vid_pll_in_sel
@ 2020-04-14 20:00   ` Martin Blumenstingl
  0 siblings, 0 replies; 30+ messages in thread
From: Martin Blumenstingl @ 2020-04-14 20:00 UTC (permalink / raw)
  To: jbrunet, linux-amlogic, linux-clk
  Cc: narmstrong, sboyd, mturquette, linux-kernel, Martin Blumenstingl,
	linux-arm-kernel

Use hdmi_pll_lvds_out as parent of the vid_pll_in_sel clock. It's not
easy to see that the vendor kernel does the same, but it actually does.
meson_clk_pll_ops in mainline still cannot fully recalculate all rates
from the HDMI PLL registers because some register bits (at the time of
writing it's unknown which bits are used for this) double the HDMI PLL
output rate (compared to simply considering M, N and FRAC).

Update the vid_pll_in_sel parent so our clock calculation works for
simple clock settings like the CVBS output (where no rate doubling is
going on). The PLL ops need to be fixed later on for more complex clock
settings (all HDMI rates).

Fixes: 6cb57c678bb70 ("clk: meson: meson8b: add the read-only video clock trees")
Suggested-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/clk/meson/meson8b.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index 7c55c695cbae..90d284ffc780 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -1077,7 +1077,7 @@ static struct clk_regmap meson8b_vid_pll_in_sel = {
 		 * Meson8m2: vid2_pll
 		 */
 		.parent_hws = (const struct clk_hw *[]) {
-			&meson8b_hdmi_pll_dco.hw
+			&meson8b_hdmi_pll_lvds_out.hw
 		},
 		.num_parents = 1,
 		.flags = CLK_SET_RATE_PARENT,
-- 
2.26.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 1/4] clk: meson: meson8b: Fix the first parent of vid_pll_in_sel
@ 2020-04-14 20:00   ` Martin Blumenstingl
  0 siblings, 0 replies; 30+ messages in thread
From: Martin Blumenstingl @ 2020-04-14 20:00 UTC (permalink / raw)
  To: jbrunet, linux-amlogic, linux-clk
  Cc: narmstrong, sboyd, mturquette, linux-kernel, Martin Blumenstingl,
	linux-arm-kernel

Use hdmi_pll_lvds_out as parent of the vid_pll_in_sel clock. It's not
easy to see that the vendor kernel does the same, but it actually does.
meson_clk_pll_ops in mainline still cannot fully recalculate all rates
from the HDMI PLL registers because some register bits (at the time of
writing it's unknown which bits are used for this) double the HDMI PLL
output rate (compared to simply considering M, N and FRAC).

Update the vid_pll_in_sel parent so our clock calculation works for
simple clock settings like the CVBS output (where no rate doubling is
going on). The PLL ops need to be fixed later on for more complex clock
settings (all HDMI rates).

Fixes: 6cb57c678bb70 ("clk: meson: meson8b: add the read-only video clock trees")
Suggested-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/clk/meson/meson8b.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index 7c55c695cbae..90d284ffc780 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -1077,7 +1077,7 @@ static struct clk_regmap meson8b_vid_pll_in_sel = {
 		 * Meson8m2: vid2_pll
 		 */
 		.parent_hws = (const struct clk_hw *[]) {
-			&meson8b_hdmi_pll_dco.hw
+			&meson8b_hdmi_pll_lvds_out.hw
 		},
 		.num_parents = 1,
 		.flags = CLK_SET_RATE_PARENT,
-- 
2.26.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 2/4] clk: meson: meson8b: Fix the polarity of the RESET_N lines
  2020-04-14 20:00 ` Martin Blumenstingl
  (?)
@ 2020-04-14 20:00   ` Martin Blumenstingl
  -1 siblings, 0 replies; 30+ messages in thread
From: Martin Blumenstingl @ 2020-04-14 20:00 UTC (permalink / raw)
  To: jbrunet, linux-amlogic, linux-clk
  Cc: narmstrong, mturquette, sboyd, linux-arm-kernel, linux-kernel,
	Martin Blumenstingl

CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_POST and
CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_PRE are active low. This means:
- asserting them requires setting the register value to 0
- de-asserting them requires setting the register value to 1

Set the register value accordingly for these two reset lines by setting
the inverted the register value compared to all other reset lines.

Fixes: 189621726bc2f6 ("clk: meson: meson8b: register the built-in reset controller")
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/clk/meson/meson8b.c | 81 ++++++++++++++++++++++++++-----------
 1 file changed, 58 insertions(+), 23 deletions(-)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index 90d284ffc780..fa251e45e208 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -3506,54 +3506,87 @@ static struct clk_regmap *const meson8b_clk_regmaps[] = {
 static const struct meson8b_clk_reset_line {
 	u32 reg;
 	u8 bit_idx;
+	bool active_low;
 } meson8b_clk_reset_bits[] = {
 	[CLKC_RESET_L2_CACHE_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 30
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 30,
+		.active_low = false,
 	},
 	[CLKC_RESET_AXI_64_TO_128_BRIDGE_A5_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 29
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 29,
+		.active_low = false,
 	},
 	[CLKC_RESET_SCU_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 28
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 28,
+		.active_low = false,
 	},
 	[CLKC_RESET_CPU3_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 27
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 27,
+		.active_low = false,
 	},
 	[CLKC_RESET_CPU2_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 26
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 26,
+		.active_low = false,
 	},
 	[CLKC_RESET_CPU1_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 25
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 25,
+		.active_low = false,
 	},
 	[CLKC_RESET_CPU0_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 24
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 24,
+		.active_low = false,
 	},
 	[CLKC_RESET_A5_GLOBAL_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 18
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 18,
+		.active_low = false,
 	},
 	[CLKC_RESET_A5_AXI_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 17
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 17,
+		.active_low = false,
 	},
 	[CLKC_RESET_A5_ABP_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 16
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 16,
+		.active_low = false,
 	},
 	[CLKC_RESET_AXI_64_TO_128_BRIDGE_MMC_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL1, .bit_idx = 30
+		.reg = HHI_SYS_CPU_CLK_CNTL1,
+		.bit_idx = 30,
+		.active_low = false,
 	},
 	[CLKC_RESET_VID_CLK_CNTL_SOFT_RESET] = {
-		.reg = HHI_VID_CLK_CNTL, .bit_idx = 15
+		.reg = HHI_VID_CLK_CNTL,
+		.bit_idx = 15,
+		.active_low = false,
 	},
 	[CLKC_RESET_VID_DIVIDER_CNTL_SOFT_RESET_POST] = {
-		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 7
+		.reg = HHI_VID_DIVIDER_CNTL,
+		.bit_idx = 7,
+		.active_low = false,
 	},
 	[CLKC_RESET_VID_DIVIDER_CNTL_SOFT_RESET_PRE] = {
-		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 3
+		.reg = HHI_VID_DIVIDER_CNTL,
+		.bit_idx = 3,
+		.active_low = false,
 	},
 	[CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_POST] = {
-		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 1
+		.reg = HHI_VID_DIVIDER_CNTL,
+		.bit_idx = 1,
+		.active_low = true,
 	},
 	[CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_PRE] = {
-		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 0
+		.reg = HHI_VID_DIVIDER_CNTL,
+		.bit_idx = 0,
+		.active_low = true,
 	},
 };
 
@@ -3562,22 +3595,24 @@ static int meson8b_clk_reset_update(struct reset_controller_dev *rcdev,
 {
 	struct meson8b_clk_reset *meson8b_clk_reset =
 		container_of(rcdev, struct meson8b_clk_reset, reset);
-	unsigned long flags;
 	const struct meson8b_clk_reset_line *reset;
+	unsigned long flags;
+	unsigned int value;
 
 	if (id >= ARRAY_SIZE(meson8b_clk_reset_bits))
 		return -EINVAL;
 
 	reset = &meson8b_clk_reset_bits[id];
 
+	if (assert == reset->active_low)
+		value = 0;
+	else
+		value = BIT(reset->bit_idx);
+
 	spin_lock_irqsave(&meson_clk_lock, flags);
 
-	if (assert)
-		regmap_update_bits(meson8b_clk_reset->regmap, reset->reg,
-				   BIT(reset->bit_idx), BIT(reset->bit_idx));
-	else
-		regmap_update_bits(meson8b_clk_reset->regmap, reset->reg,
-				   BIT(reset->bit_idx), 0);
+	regmap_update_bits(meson8b_clk_reset->regmap, reset->reg,
+			   BIT(reset->bit_idx), value);
 
 	spin_unlock_irqrestore(&meson_clk_lock, flags);
 
-- 
2.26.0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 2/4] clk: meson: meson8b: Fix the polarity of the RESET_N lines
@ 2020-04-14 20:00   ` Martin Blumenstingl
  0 siblings, 0 replies; 30+ messages in thread
From: Martin Blumenstingl @ 2020-04-14 20:00 UTC (permalink / raw)
  To: jbrunet, linux-amlogic, linux-clk
  Cc: narmstrong, sboyd, mturquette, linux-kernel, Martin Blumenstingl,
	linux-arm-kernel

CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_POST and
CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_PRE are active low. This means:
- asserting them requires setting the register value to 0
- de-asserting them requires setting the register value to 1

Set the register value accordingly for these two reset lines by setting
the inverted the register value compared to all other reset lines.

Fixes: 189621726bc2f6 ("clk: meson: meson8b: register the built-in reset controller")
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/clk/meson/meson8b.c | 81 ++++++++++++++++++++++++++-----------
 1 file changed, 58 insertions(+), 23 deletions(-)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index 90d284ffc780..fa251e45e208 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -3506,54 +3506,87 @@ static struct clk_regmap *const meson8b_clk_regmaps[] = {
 static const struct meson8b_clk_reset_line {
 	u32 reg;
 	u8 bit_idx;
+	bool active_low;
 } meson8b_clk_reset_bits[] = {
 	[CLKC_RESET_L2_CACHE_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 30
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 30,
+		.active_low = false,
 	},
 	[CLKC_RESET_AXI_64_TO_128_BRIDGE_A5_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 29
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 29,
+		.active_low = false,
 	},
 	[CLKC_RESET_SCU_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 28
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 28,
+		.active_low = false,
 	},
 	[CLKC_RESET_CPU3_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 27
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 27,
+		.active_low = false,
 	},
 	[CLKC_RESET_CPU2_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 26
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 26,
+		.active_low = false,
 	},
 	[CLKC_RESET_CPU1_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 25
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 25,
+		.active_low = false,
 	},
 	[CLKC_RESET_CPU0_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 24
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 24,
+		.active_low = false,
 	},
 	[CLKC_RESET_A5_GLOBAL_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 18
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 18,
+		.active_low = false,
 	},
 	[CLKC_RESET_A5_AXI_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 17
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 17,
+		.active_low = false,
 	},
 	[CLKC_RESET_A5_ABP_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 16
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 16,
+		.active_low = false,
 	},
 	[CLKC_RESET_AXI_64_TO_128_BRIDGE_MMC_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL1, .bit_idx = 30
+		.reg = HHI_SYS_CPU_CLK_CNTL1,
+		.bit_idx = 30,
+		.active_low = false,
 	},
 	[CLKC_RESET_VID_CLK_CNTL_SOFT_RESET] = {
-		.reg = HHI_VID_CLK_CNTL, .bit_idx = 15
+		.reg = HHI_VID_CLK_CNTL,
+		.bit_idx = 15,
+		.active_low = false,
 	},
 	[CLKC_RESET_VID_DIVIDER_CNTL_SOFT_RESET_POST] = {
-		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 7
+		.reg = HHI_VID_DIVIDER_CNTL,
+		.bit_idx = 7,
+		.active_low = false,
 	},
 	[CLKC_RESET_VID_DIVIDER_CNTL_SOFT_RESET_PRE] = {
-		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 3
+		.reg = HHI_VID_DIVIDER_CNTL,
+		.bit_idx = 3,
+		.active_low = false,
 	},
 	[CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_POST] = {
-		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 1
+		.reg = HHI_VID_DIVIDER_CNTL,
+		.bit_idx = 1,
+		.active_low = true,
 	},
 	[CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_PRE] = {
-		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 0
+		.reg = HHI_VID_DIVIDER_CNTL,
+		.bit_idx = 0,
+		.active_low = true,
 	},
 };
 
@@ -3562,22 +3595,24 @@ static int meson8b_clk_reset_update(struct reset_controller_dev *rcdev,
 {
 	struct meson8b_clk_reset *meson8b_clk_reset =
 		container_of(rcdev, struct meson8b_clk_reset, reset);
-	unsigned long flags;
 	const struct meson8b_clk_reset_line *reset;
+	unsigned long flags;
+	unsigned int value;
 
 	if (id >= ARRAY_SIZE(meson8b_clk_reset_bits))
 		return -EINVAL;
 
 	reset = &meson8b_clk_reset_bits[id];
 
+	if (assert == reset->active_low)
+		value = 0;
+	else
+		value = BIT(reset->bit_idx);
+
 	spin_lock_irqsave(&meson_clk_lock, flags);
 
-	if (assert)
-		regmap_update_bits(meson8b_clk_reset->regmap, reset->reg,
-				   BIT(reset->bit_idx), BIT(reset->bit_idx));
-	else
-		regmap_update_bits(meson8b_clk_reset->regmap, reset->reg,
-				   BIT(reset->bit_idx), 0);
+	regmap_update_bits(meson8b_clk_reset->regmap, reset->reg,
+			   BIT(reset->bit_idx), value);
 
 	spin_unlock_irqrestore(&meson_clk_lock, flags);
 
-- 
2.26.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 2/4] clk: meson: meson8b: Fix the polarity of the RESET_N lines
@ 2020-04-14 20:00   ` Martin Blumenstingl
  0 siblings, 0 replies; 30+ messages in thread
From: Martin Blumenstingl @ 2020-04-14 20:00 UTC (permalink / raw)
  To: jbrunet, linux-amlogic, linux-clk
  Cc: narmstrong, sboyd, mturquette, linux-kernel, Martin Blumenstingl,
	linux-arm-kernel

CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_POST and
CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_PRE are active low. This means:
- asserting them requires setting the register value to 0
- de-asserting them requires setting the register value to 1

Set the register value accordingly for these two reset lines by setting
the inverted the register value compared to all other reset lines.

Fixes: 189621726bc2f6 ("clk: meson: meson8b: register the built-in reset controller")
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/clk/meson/meson8b.c | 81 ++++++++++++++++++++++++++-----------
 1 file changed, 58 insertions(+), 23 deletions(-)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index 90d284ffc780..fa251e45e208 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -3506,54 +3506,87 @@ static struct clk_regmap *const meson8b_clk_regmaps[] = {
 static const struct meson8b_clk_reset_line {
 	u32 reg;
 	u8 bit_idx;
+	bool active_low;
 } meson8b_clk_reset_bits[] = {
 	[CLKC_RESET_L2_CACHE_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 30
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 30,
+		.active_low = false,
 	},
 	[CLKC_RESET_AXI_64_TO_128_BRIDGE_A5_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 29
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 29,
+		.active_low = false,
 	},
 	[CLKC_RESET_SCU_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 28
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 28,
+		.active_low = false,
 	},
 	[CLKC_RESET_CPU3_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 27
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 27,
+		.active_low = false,
 	},
 	[CLKC_RESET_CPU2_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 26
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 26,
+		.active_low = false,
 	},
 	[CLKC_RESET_CPU1_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 25
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 25,
+		.active_low = false,
 	},
 	[CLKC_RESET_CPU0_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 24
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 24,
+		.active_low = false,
 	},
 	[CLKC_RESET_A5_GLOBAL_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 18
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 18,
+		.active_low = false,
 	},
 	[CLKC_RESET_A5_AXI_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 17
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 17,
+		.active_low = false,
 	},
 	[CLKC_RESET_A5_ABP_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 16
+		.reg = HHI_SYS_CPU_CLK_CNTL0,
+		.bit_idx = 16,
+		.active_low = false,
 	},
 	[CLKC_RESET_AXI_64_TO_128_BRIDGE_MMC_SOFT_RESET] = {
-		.reg = HHI_SYS_CPU_CLK_CNTL1, .bit_idx = 30
+		.reg = HHI_SYS_CPU_CLK_CNTL1,
+		.bit_idx = 30,
+		.active_low = false,
 	},
 	[CLKC_RESET_VID_CLK_CNTL_SOFT_RESET] = {
-		.reg = HHI_VID_CLK_CNTL, .bit_idx = 15
+		.reg = HHI_VID_CLK_CNTL,
+		.bit_idx = 15,
+		.active_low = false,
 	},
 	[CLKC_RESET_VID_DIVIDER_CNTL_SOFT_RESET_POST] = {
-		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 7
+		.reg = HHI_VID_DIVIDER_CNTL,
+		.bit_idx = 7,
+		.active_low = false,
 	},
 	[CLKC_RESET_VID_DIVIDER_CNTL_SOFT_RESET_PRE] = {
-		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 3
+		.reg = HHI_VID_DIVIDER_CNTL,
+		.bit_idx = 3,
+		.active_low = false,
 	},
 	[CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_POST] = {
-		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 1
+		.reg = HHI_VID_DIVIDER_CNTL,
+		.bit_idx = 1,
+		.active_low = true,
 	},
 	[CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_PRE] = {
-		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 0
+		.reg = HHI_VID_DIVIDER_CNTL,
+		.bit_idx = 0,
+		.active_low = true,
 	},
 };
 
@@ -3562,22 +3595,24 @@ static int meson8b_clk_reset_update(struct reset_controller_dev *rcdev,
 {
 	struct meson8b_clk_reset *meson8b_clk_reset =
 		container_of(rcdev, struct meson8b_clk_reset, reset);
-	unsigned long flags;
 	const struct meson8b_clk_reset_line *reset;
+	unsigned long flags;
+	unsigned int value;
 
 	if (id >= ARRAY_SIZE(meson8b_clk_reset_bits))
 		return -EINVAL;
 
 	reset = &meson8b_clk_reset_bits[id];
 
+	if (assert == reset->active_low)
+		value = 0;
+	else
+		value = BIT(reset->bit_idx);
+
 	spin_lock_irqsave(&meson_clk_lock, flags);
 
-	if (assert)
-		regmap_update_bits(meson8b_clk_reset->regmap, reset->reg,
-				   BIT(reset->bit_idx), BIT(reset->bit_idx));
-	else
-		regmap_update_bits(meson8b_clk_reset->regmap, reset->reg,
-				   BIT(reset->bit_idx), 0);
+	regmap_update_bits(meson8b_clk_reset->regmap, reset->reg,
+			   BIT(reset->bit_idx), value);
 
 	spin_unlock_irqrestore(&meson_clk_lock, flags);
 
-- 
2.26.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 3/4] clk: meson: meson8b: Fix the vclk_div{1,2,4,6,12}_en gate bits
  2020-04-14 20:00 ` Martin Blumenstingl
  (?)
@ 2020-04-14 20:00   ` Martin Blumenstingl
  -1 siblings, 0 replies; 30+ messages in thread
From: Martin Blumenstingl @ 2020-04-14 20:00 UTC (permalink / raw)
  To: jbrunet, linux-amlogic, linux-clk
  Cc: narmstrong, mturquette, sboyd, linux-arm-kernel, linux-kernel,
	Martin Blumenstingl

The DIV{1,2,4,6,12}_EN bits are actually located in HHI_VID_CLK_CNTL
register:
- HHI_VID_CLK_CNTL[0] = DIV1_EN
- HHI_VID_CLK_CNTL[1] = DIV2_EN
- HHI_VID_CLK_CNTL[2] = DIV4_EN
- HHI_VID_CLK_CNTL[3] = DIV6_EN
- HHI_VID_CLK_CNTL[4] = DIV12_EN

Update the bits accordingly so we will enable the bits in the correct
register once we switch these clocks to be mutable.

Fixes: 6cb57c678bb70e ("clk: meson: meson8b: add the read-only video clock trees")
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/clk/meson/meson8b.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index fa251e45e208..ed4b70c2d4bd 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -1213,7 +1213,7 @@ static struct clk_regmap meson8b_vclk_in_en = {
 
 static struct clk_regmap meson8b_vclk_div1_gate = {
 	.data = &(struct clk_regmap_gate_data){
-		.offset = HHI_VID_CLK_DIV,
+		.offset = HHI_VID_CLK_CNTL,
 		.bit_idx = 0,
 	},
 	.hw.init = &(struct clk_init_data){
@@ -1243,7 +1243,7 @@ static struct clk_fixed_factor meson8b_vclk_div2_div = {
 
 static struct clk_regmap meson8b_vclk_div2_div_gate = {
 	.data = &(struct clk_regmap_gate_data){
-		.offset = HHI_VID_CLK_DIV,
+		.offset = HHI_VID_CLK_CNTL,
 		.bit_idx = 1,
 	},
 	.hw.init = &(struct clk_init_data){
@@ -1273,7 +1273,7 @@ static struct clk_fixed_factor meson8b_vclk_div4_div = {
 
 static struct clk_regmap meson8b_vclk_div4_div_gate = {
 	.data = &(struct clk_regmap_gate_data){
-		.offset = HHI_VID_CLK_DIV,
+		.offset = HHI_VID_CLK_CNTL,
 		.bit_idx = 2,
 	},
 	.hw.init = &(struct clk_init_data){
@@ -1303,7 +1303,7 @@ static struct clk_fixed_factor meson8b_vclk_div6_div = {
 
 static struct clk_regmap meson8b_vclk_div6_div_gate = {
 	.data = &(struct clk_regmap_gate_data){
-		.offset = HHI_VID_CLK_DIV,
+		.offset = HHI_VID_CLK_CNTL,
 		.bit_idx = 3,
 	},
 	.hw.init = &(struct clk_init_data){
@@ -1333,7 +1333,7 @@ static struct clk_fixed_factor meson8b_vclk_div12_div = {
 
 static struct clk_regmap meson8b_vclk_div12_div_gate = {
 	.data = &(struct clk_regmap_gate_data){
-		.offset = HHI_VID_CLK_DIV,
+		.offset = HHI_VID_CLK_CNTL,
 		.bit_idx = 4,
 	},
 	.hw.init = &(struct clk_init_data){
-- 
2.26.0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 3/4] clk: meson: meson8b: Fix the vclk_div{1, 2, 4, 6, 12}_en gate bits
@ 2020-04-14 20:00   ` Martin Blumenstingl
  0 siblings, 0 replies; 30+ messages in thread
From: Martin Blumenstingl @ 2020-04-14 20:00 UTC (permalink / raw)
  To: jbrunet, linux-amlogic, linux-clk
  Cc: narmstrong, sboyd, mturquette, linux-kernel, Martin Blumenstingl,
	linux-arm-kernel

The DIV{1,2,4,6,12}_EN bits are actually located in HHI_VID_CLK_CNTL
register:
- HHI_VID_CLK_CNTL[0] = DIV1_EN
- HHI_VID_CLK_CNTL[1] = DIV2_EN
- HHI_VID_CLK_CNTL[2] = DIV4_EN
- HHI_VID_CLK_CNTL[3] = DIV6_EN
- HHI_VID_CLK_CNTL[4] = DIV12_EN

Update the bits accordingly so we will enable the bits in the correct
register once we switch these clocks to be mutable.

Fixes: 6cb57c678bb70e ("clk: meson: meson8b: add the read-only video clock trees")
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/clk/meson/meson8b.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index fa251e45e208..ed4b70c2d4bd 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -1213,7 +1213,7 @@ static struct clk_regmap meson8b_vclk_in_en = {
 
 static struct clk_regmap meson8b_vclk_div1_gate = {
 	.data = &(struct clk_regmap_gate_data){
-		.offset = HHI_VID_CLK_DIV,
+		.offset = HHI_VID_CLK_CNTL,
 		.bit_idx = 0,
 	},
 	.hw.init = &(struct clk_init_data){
@@ -1243,7 +1243,7 @@ static struct clk_fixed_factor meson8b_vclk_div2_div = {
 
 static struct clk_regmap meson8b_vclk_div2_div_gate = {
 	.data = &(struct clk_regmap_gate_data){
-		.offset = HHI_VID_CLK_DIV,
+		.offset = HHI_VID_CLK_CNTL,
 		.bit_idx = 1,
 	},
 	.hw.init = &(struct clk_init_data){
@@ -1273,7 +1273,7 @@ static struct clk_fixed_factor meson8b_vclk_div4_div = {
 
 static struct clk_regmap meson8b_vclk_div4_div_gate = {
 	.data = &(struct clk_regmap_gate_data){
-		.offset = HHI_VID_CLK_DIV,
+		.offset = HHI_VID_CLK_CNTL,
 		.bit_idx = 2,
 	},
 	.hw.init = &(struct clk_init_data){
@@ -1303,7 +1303,7 @@ static struct clk_fixed_factor meson8b_vclk_div6_div = {
 
 static struct clk_regmap meson8b_vclk_div6_div_gate = {
 	.data = &(struct clk_regmap_gate_data){
-		.offset = HHI_VID_CLK_DIV,
+		.offset = HHI_VID_CLK_CNTL,
 		.bit_idx = 3,
 	},
 	.hw.init = &(struct clk_init_data){
@@ -1333,7 +1333,7 @@ static struct clk_fixed_factor meson8b_vclk_div12_div = {
 
 static struct clk_regmap meson8b_vclk_div12_div_gate = {
 	.data = &(struct clk_regmap_gate_data){
-		.offset = HHI_VID_CLK_DIV,
+		.offset = HHI_VID_CLK_CNTL,
 		.bit_idx = 4,
 	},
 	.hw.init = &(struct clk_init_data){
-- 
2.26.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 3/4] clk: meson: meson8b: Fix the vclk_div{1, 2, 4, 6, 12}_en gate bits
@ 2020-04-14 20:00   ` Martin Blumenstingl
  0 siblings, 0 replies; 30+ messages in thread
From: Martin Blumenstingl @ 2020-04-14 20:00 UTC (permalink / raw)
  To: jbrunet, linux-amlogic, linux-clk
  Cc: narmstrong, sboyd, mturquette, linux-kernel, Martin Blumenstingl,
	linux-arm-kernel

The DIV{1,2,4,6,12}_EN bits are actually located in HHI_VID_CLK_CNTL
register:
- HHI_VID_CLK_CNTL[0] = DIV1_EN
- HHI_VID_CLK_CNTL[1] = DIV2_EN
- HHI_VID_CLK_CNTL[2] = DIV4_EN
- HHI_VID_CLK_CNTL[3] = DIV6_EN
- HHI_VID_CLK_CNTL[4] = DIV12_EN

Update the bits accordingly so we will enable the bits in the correct
register once we switch these clocks to be mutable.

Fixes: 6cb57c678bb70e ("clk: meson: meson8b: add the read-only video clock trees")
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/clk/meson/meson8b.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index fa251e45e208..ed4b70c2d4bd 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -1213,7 +1213,7 @@ static struct clk_regmap meson8b_vclk_in_en = {
 
 static struct clk_regmap meson8b_vclk_div1_gate = {
 	.data = &(struct clk_regmap_gate_data){
-		.offset = HHI_VID_CLK_DIV,
+		.offset = HHI_VID_CLK_CNTL,
 		.bit_idx = 0,
 	},
 	.hw.init = &(struct clk_init_data){
@@ -1243,7 +1243,7 @@ static struct clk_fixed_factor meson8b_vclk_div2_div = {
 
 static struct clk_regmap meson8b_vclk_div2_div_gate = {
 	.data = &(struct clk_regmap_gate_data){
-		.offset = HHI_VID_CLK_DIV,
+		.offset = HHI_VID_CLK_CNTL,
 		.bit_idx = 1,
 	},
 	.hw.init = &(struct clk_init_data){
@@ -1273,7 +1273,7 @@ static struct clk_fixed_factor meson8b_vclk_div4_div = {
 
 static struct clk_regmap meson8b_vclk_div4_div_gate = {
 	.data = &(struct clk_regmap_gate_data){
-		.offset = HHI_VID_CLK_DIV,
+		.offset = HHI_VID_CLK_CNTL,
 		.bit_idx = 2,
 	},
 	.hw.init = &(struct clk_init_data){
@@ -1303,7 +1303,7 @@ static struct clk_fixed_factor meson8b_vclk_div6_div = {
 
 static struct clk_regmap meson8b_vclk_div6_div_gate = {
 	.data = &(struct clk_regmap_gate_data){
-		.offset = HHI_VID_CLK_DIV,
+		.offset = HHI_VID_CLK_CNTL,
 		.bit_idx = 3,
 	},
 	.hw.init = &(struct clk_init_data){
@@ -1333,7 +1333,7 @@ static struct clk_fixed_factor meson8b_vclk_div12_div = {
 
 static struct clk_regmap meson8b_vclk_div12_div_gate = {
 	.data = &(struct clk_regmap_gate_data){
-		.offset = HHI_VID_CLK_DIV,
+		.offset = HHI_VID_CLK_CNTL,
 		.bit_idx = 4,
 	},
 	.hw.init = &(struct clk_init_data){
-- 
2.26.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 4/4] clk: meson: meson8b: Make the CCF use the glitch-free VPU mux
  2020-04-14 20:00 ` Martin Blumenstingl
  (?)
@ 2020-04-14 20:00   ` Martin Blumenstingl
  -1 siblings, 0 replies; 30+ messages in thread
From: Martin Blumenstingl @ 2020-04-14 20:00 UTC (permalink / raw)
  To: jbrunet, linux-amlogic, linux-clk
  Cc: narmstrong, mturquette, sboyd, linux-arm-kernel, linux-kernel,
	Martin Blumenstingl

The "vpu_0" or "vpu_1" clock trees should not be updated while the
clock is running. Enforce this by setting CLK_SET_RATE_GATE on the
"vpu_0" and "vpu_1" gates. This makes the CCF switch to the "vpu_1"
tree when "vpu_0" is currently active and vice versa, which is exactly
what the vendor driver does when updating the frequency of the VPU
clock.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/clk/meson/meson8b.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index ed4b70c2d4bd..427392678fec 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -2063,7 +2063,7 @@ static struct clk_regmap meson8b_vpu_0 = {
 			&meson8b_vpu_0_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_GATE | CLK_SET_RATE_PARENT,
 	},
 };
 
@@ -2134,10 +2134,18 @@ static struct clk_regmap meson8b_vpu_1 = {
 			&meson8b_vpu_1_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_GATE | CLK_SET_RATE_PARENT,
 	},
 };
 
+/*
+ * The VPU clock has two two identical clock trees (vpu_0 and vpu_1)
+ * muxed by a glitch-free switch on Meson8b and Meson8m2. The CCF can
+ * actually manage this glitch-free mux because it does top-to-bottom
+ * updates the each clock tree and switches to the "inactive" one when
+ * CLK_SET_RATE_GATE is set.
+ * Meson8 only has vpu_0 and no glitch-free mux.
+ */
 static struct clk_regmap meson8b_vpu = {
 	.data = &(struct clk_regmap_mux_data){
 		.offset = HHI_VPU_CLK_CNTL,
@@ -2152,7 +2160,7 @@ static struct clk_regmap meson8b_vpu = {
 			&meson8b_vpu_1.hw,
 		},
 		.num_parents = 2,
-		.flags = CLK_SET_RATE_NO_REPARENT,
+		.flags = CLK_SET_RATE_PARENT,
 	},
 };
 
-- 
2.26.0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 4/4] clk: meson: meson8b: Make the CCF use the glitch-free VPU mux
@ 2020-04-14 20:00   ` Martin Blumenstingl
  0 siblings, 0 replies; 30+ messages in thread
From: Martin Blumenstingl @ 2020-04-14 20:00 UTC (permalink / raw)
  To: jbrunet, linux-amlogic, linux-clk
  Cc: narmstrong, sboyd, mturquette, linux-kernel, Martin Blumenstingl,
	linux-arm-kernel

The "vpu_0" or "vpu_1" clock trees should not be updated while the
clock is running. Enforce this by setting CLK_SET_RATE_GATE on the
"vpu_0" and "vpu_1" gates. This makes the CCF switch to the "vpu_1"
tree when "vpu_0" is currently active and vice versa, which is exactly
what the vendor driver does when updating the frequency of the VPU
clock.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/clk/meson/meson8b.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index ed4b70c2d4bd..427392678fec 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -2063,7 +2063,7 @@ static struct clk_regmap meson8b_vpu_0 = {
 			&meson8b_vpu_0_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_GATE | CLK_SET_RATE_PARENT,
 	},
 };
 
@@ -2134,10 +2134,18 @@ static struct clk_regmap meson8b_vpu_1 = {
 			&meson8b_vpu_1_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_GATE | CLK_SET_RATE_PARENT,
 	},
 };
 
+/*
+ * The VPU clock has two two identical clock trees (vpu_0 and vpu_1)
+ * muxed by a glitch-free switch on Meson8b and Meson8m2. The CCF can
+ * actually manage this glitch-free mux because it does top-to-bottom
+ * updates the each clock tree and switches to the "inactive" one when
+ * CLK_SET_RATE_GATE is set.
+ * Meson8 only has vpu_0 and no glitch-free mux.
+ */
 static struct clk_regmap meson8b_vpu = {
 	.data = &(struct clk_regmap_mux_data){
 		.offset = HHI_VPU_CLK_CNTL,
@@ -2152,7 +2160,7 @@ static struct clk_regmap meson8b_vpu = {
 			&meson8b_vpu_1.hw,
 		},
 		.num_parents = 2,
-		.flags = CLK_SET_RATE_NO_REPARENT,
+		.flags = CLK_SET_RATE_PARENT,
 	},
 };
 
-- 
2.26.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 4/4] clk: meson: meson8b: Make the CCF use the glitch-free VPU mux
@ 2020-04-14 20:00   ` Martin Blumenstingl
  0 siblings, 0 replies; 30+ messages in thread
From: Martin Blumenstingl @ 2020-04-14 20:00 UTC (permalink / raw)
  To: jbrunet, linux-amlogic, linux-clk
  Cc: narmstrong, sboyd, mturquette, linux-kernel, Martin Blumenstingl,
	linux-arm-kernel

The "vpu_0" or "vpu_1" clock trees should not be updated while the
clock is running. Enforce this by setting CLK_SET_RATE_GATE on the
"vpu_0" and "vpu_1" gates. This makes the CCF switch to the "vpu_1"
tree when "vpu_0" is currently active and vice versa, which is exactly
what the vendor driver does when updating the frequency of the VPU
clock.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/clk/meson/meson8b.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index ed4b70c2d4bd..427392678fec 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -2063,7 +2063,7 @@ static struct clk_regmap meson8b_vpu_0 = {
 			&meson8b_vpu_0_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_GATE | CLK_SET_RATE_PARENT,
 	},
 };
 
@@ -2134,10 +2134,18 @@ static struct clk_regmap meson8b_vpu_1 = {
 			&meson8b_vpu_1_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_GATE | CLK_SET_RATE_PARENT,
 	},
 };
 
+/*
+ * The VPU clock has two two identical clock trees (vpu_0 and vpu_1)
+ * muxed by a glitch-free switch on Meson8b and Meson8m2. The CCF can
+ * actually manage this glitch-free mux because it does top-to-bottom
+ * updates the each clock tree and switches to the "inactive" one when
+ * CLK_SET_RATE_GATE is set.
+ * Meson8 only has vpu_0 and no glitch-free mux.
+ */
 static struct clk_regmap meson8b_vpu = {
 	.data = &(struct clk_regmap_mux_data){
 		.offset = HHI_VPU_CLK_CNTL,
@@ -2152,7 +2160,7 @@ static struct clk_regmap meson8b_vpu = {
 			&meson8b_vpu_1.hw,
 		},
 		.num_parents = 2,
-		.flags = CLK_SET_RATE_NO_REPARENT,
+		.flags = CLK_SET_RATE_PARENT,
 	},
 };
 
-- 
2.26.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/4] clk: meson: meson8b: Fix the first parent of vid_pll_in_sel
  2020-04-14 20:00   ` Martin Blumenstingl
  (?)
@ 2020-04-16 10:32     ` Jerome Brunet
  -1 siblings, 0 replies; 30+ messages in thread
From: Jerome Brunet @ 2020-04-16 10:32 UTC (permalink / raw)
  To: Martin Blumenstingl, linux-amlogic, linux-clk
  Cc: narmstrong, mturquette, sboyd, linux-arm-kernel, linux-kernel


On Tue 14 Apr 2020 at 22:00, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> Use hdmi_pll_lvds_out as parent of the vid_pll_in_sel clock. It's not
> easy to see that the vendor kernel does the same, but it actually does.
> meson_clk_pll_ops in mainline still cannot fully recalculate all rates
> from the HDMI PLL registers because some register bits (at the time of
> writing it's unknown which bits are used for this) double the HDMI PLL
> output rate (compared to simply considering M, N and FRAC).

Have you considered adding a fixed_factor pre-multiplier, like in the
gxbb driver ?

Seems to be the same thing

>
> Update the vid_pll_in_sel parent so our clock calculation works for
> simple clock settings like the CVBS output (where no rate doubling is
> going on). The PLL ops need to be fixed later on for more complex clock
> settings (all HDMI rates).
>
> Fixes: 6cb57c678bb70 ("clk: meson: meson8b: add the read-only video clock trees")
> Suggested-by: Neil Armstrong <narmstrong@baylibre.com>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  drivers/clk/meson/meson8b.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
> index 7c55c695cbae..90d284ffc780 100644
> --- a/drivers/clk/meson/meson8b.c
> +++ b/drivers/clk/meson/meson8b.c
> @@ -1077,7 +1077,7 @@ static struct clk_regmap meson8b_vid_pll_in_sel = {
>  		 * Meson8m2: vid2_pll
>  		 */
>  		.parent_hws = (const struct clk_hw *[]) {
> -			&meson8b_hdmi_pll_dco.hw
> +			&meson8b_hdmi_pll_lvds_out.hw
>  		},
>  		.num_parents = 1,
>  		.flags = CLK_SET_RATE_PARENT,


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/4] clk: meson: meson8b: Fix the first parent of vid_pll_in_sel
@ 2020-04-16 10:32     ` Jerome Brunet
  0 siblings, 0 replies; 30+ messages in thread
From: Jerome Brunet @ 2020-04-16 10:32 UTC (permalink / raw)
  To: Martin Blumenstingl, linux-amlogic, linux-clk
  Cc: sboyd, mturquette, linux-kernel, linux-arm-kernel, narmstrong


On Tue 14 Apr 2020 at 22:00, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> Use hdmi_pll_lvds_out as parent of the vid_pll_in_sel clock. It's not
> easy to see that the vendor kernel does the same, but it actually does.
> meson_clk_pll_ops in mainline still cannot fully recalculate all rates
> from the HDMI PLL registers because some register bits (at the time of
> writing it's unknown which bits are used for this) double the HDMI PLL
> output rate (compared to simply considering M, N and FRAC).

Have you considered adding a fixed_factor pre-multiplier, like in the
gxbb driver ?

Seems to be the same thing

>
> Update the vid_pll_in_sel parent so our clock calculation works for
> simple clock settings like the CVBS output (where no rate doubling is
> going on). The PLL ops need to be fixed later on for more complex clock
> settings (all HDMI rates).
>
> Fixes: 6cb57c678bb70 ("clk: meson: meson8b: add the read-only video clock trees")
> Suggested-by: Neil Armstrong <narmstrong@baylibre.com>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  drivers/clk/meson/meson8b.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
> index 7c55c695cbae..90d284ffc780 100644
> --- a/drivers/clk/meson/meson8b.c
> +++ b/drivers/clk/meson/meson8b.c
> @@ -1077,7 +1077,7 @@ static struct clk_regmap meson8b_vid_pll_in_sel = {
>  		 * Meson8m2: vid2_pll
>  		 */
>  		.parent_hws = (const struct clk_hw *[]) {
> -			&meson8b_hdmi_pll_dco.hw
> +			&meson8b_hdmi_pll_lvds_out.hw
>  		},
>  		.num_parents = 1,
>  		.flags = CLK_SET_RATE_PARENT,


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/4] clk: meson: meson8b: Fix the first parent of vid_pll_in_sel
@ 2020-04-16 10:32     ` Jerome Brunet
  0 siblings, 0 replies; 30+ messages in thread
From: Jerome Brunet @ 2020-04-16 10:32 UTC (permalink / raw)
  To: Martin Blumenstingl, linux-amlogic, linux-clk
  Cc: sboyd, mturquette, linux-kernel, linux-arm-kernel, narmstrong


On Tue 14 Apr 2020 at 22:00, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> Use hdmi_pll_lvds_out as parent of the vid_pll_in_sel clock. It's not
> easy to see that the vendor kernel does the same, but it actually does.
> meson_clk_pll_ops in mainline still cannot fully recalculate all rates
> from the HDMI PLL registers because some register bits (at the time of
> writing it's unknown which bits are used for this) double the HDMI PLL
> output rate (compared to simply considering M, N and FRAC).

Have you considered adding a fixed_factor pre-multiplier, like in the
gxbb driver ?

Seems to be the same thing

>
> Update the vid_pll_in_sel parent so our clock calculation works for
> simple clock settings like the CVBS output (where no rate doubling is
> going on). The PLL ops need to be fixed later on for more complex clock
> settings (all HDMI rates).
>
> Fixes: 6cb57c678bb70 ("clk: meson: meson8b: add the read-only video clock trees")
> Suggested-by: Neil Armstrong <narmstrong@baylibre.com>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  drivers/clk/meson/meson8b.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
> index 7c55c695cbae..90d284ffc780 100644
> --- a/drivers/clk/meson/meson8b.c
> +++ b/drivers/clk/meson/meson8b.c
> @@ -1077,7 +1077,7 @@ static struct clk_regmap meson8b_vid_pll_in_sel = {
>  		 * Meson8m2: vid2_pll
>  		 */
>  		.parent_hws = (const struct clk_hw *[]) {
> -			&meson8b_hdmi_pll_dco.hw
> +			&meson8b_hdmi_pll_lvds_out.hw
>  		},
>  		.num_parents = 1,
>  		.flags = CLK_SET_RATE_PARENT,


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/4] clk: meson: meson8b: Fix the polarity of the RESET_N lines
  2020-04-14 20:00   ` Martin Blumenstingl
  (?)
@ 2020-04-16 10:38     ` Jerome Brunet
  -1 siblings, 0 replies; 30+ messages in thread
From: Jerome Brunet @ 2020-04-16 10:38 UTC (permalink / raw)
  To: Martin Blumenstingl, linux-amlogic, linux-clk
  Cc: narmstrong, mturquette, sboyd, linux-arm-kernel, linux-kernel


On Tue 14 Apr 2020 at 22:00, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_POST and
> CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_PRE are active low. This means:
> - asserting them requires setting the register value to 0
> - de-asserting them requires setting the register value to 1
>
> Set the register value accordingly for these two reset lines by setting
> the inverted the register value compared to all other reset lines.
>
> Fixes: 189621726bc2f6 ("clk: meson: meson8b: register the built-in reset controller")
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  drivers/clk/meson/meson8b.c | 81 ++++++++++++++++++++++++++-----------
>  1 file changed, 58 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
> index 90d284ffc780..fa251e45e208 100644
> --- a/drivers/clk/meson/meson8b.c
> +++ b/drivers/clk/meson/meson8b.c
> @@ -3506,54 +3506,87 @@ static struct clk_regmap *const meson8b_clk_regmaps[] = {
>  static const struct meson8b_clk_reset_line {
>  	u32 reg;
>  	u8 bit_idx;
> +	bool active_low;
>  } meson8b_clk_reset_bits[] = {
>  	[CLKC_RESET_L2_CACHE_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 30
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 30,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_AXI_64_TO_128_BRIDGE_A5_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 29
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 29,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_SCU_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 28
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 28,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_CPU3_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 27
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 27,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_CPU2_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 26
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 26,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_CPU1_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 25
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 25,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_CPU0_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 24
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 24,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_A5_GLOBAL_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 18
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 18,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_A5_AXI_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 17
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 17,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_A5_ABP_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 16
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 16,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_AXI_64_TO_128_BRIDGE_MMC_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL1, .bit_idx = 30
> +		.reg = HHI_SYS_CPU_CLK_CNTL1,
> +		.bit_idx = 30,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_VID_CLK_CNTL_SOFT_RESET] = {
> -		.reg = HHI_VID_CLK_CNTL, .bit_idx = 15
> +		.reg = HHI_VID_CLK_CNTL,
> +		.bit_idx = 15,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_VID_DIVIDER_CNTL_SOFT_RESET_POST] = {
> -		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 7
> +		.reg = HHI_VID_DIVIDER_CNTL,
> +		.bit_idx = 7,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_VID_DIVIDER_CNTL_SOFT_RESET_PRE] = {
> -		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 3
> +		.reg = HHI_VID_DIVIDER_CNTL,
> +		.bit_idx = 3,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_POST] = {
> -		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 1
> +		.reg = HHI_VID_DIVIDER_CNTL,
> +		.bit_idx = 1,
> +		.active_low = true,
>  	},
>  	[CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_PRE] = {
> -		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 0
> +		.reg = HHI_VID_DIVIDER_CNTL,
> +		.bit_idx = 0,
> +		.active_low = true,
>  	},
>  };
>  
> @@ -3562,22 +3595,24 @@ static int meson8b_clk_reset_update(struct reset_controller_dev *rcdev,
>  {
>  	struct meson8b_clk_reset *meson8b_clk_reset =
>  		container_of(rcdev, struct meson8b_clk_reset, reset);
> -	unsigned long flags;
>  	const struct meson8b_clk_reset_line *reset;
> +	unsigned long flags;
> +	unsigned int value;

suggestion:

unsigned int value = 0;

>  
>  	if (id >= ARRAY_SIZE(meson8b_clk_reset_bits))
>  		return -EINVAL;
>  
>  	reset = &meson8b_clk_reset_bits[id];
>  
> +	if (assert == reset->active_low)
> +		value = 0;
> +	else
> +		value = BIT(reset->bit_idx);

if (assert ^ reset->active_low)
	value = BIT(reset->bit_idx);

?

> +
>  	spin_lock_irqsave(&meson_clk_lock, flags);
>  
> -	if (assert)
> -		regmap_update_bits(meson8b_clk_reset->regmap, reset->reg,
> -				   BIT(reset->bit_idx), BIT(reset->bit_idx));
> -	else
> -		regmap_update_bits(meson8b_clk_reset->regmap, reset->reg,
> -				   BIT(reset->bit_idx), 0);
> +	regmap_update_bits(meson8b_clk_reset->regmap, reset->reg,
> +			   BIT(reset->bit_idx), value);
>  
>  	spin_unlock_irqrestore(&meson_clk_lock, flags);


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/4] clk: meson: meson8b: Fix the polarity of the RESET_N lines
@ 2020-04-16 10:38     ` Jerome Brunet
  0 siblings, 0 replies; 30+ messages in thread
From: Jerome Brunet @ 2020-04-16 10:38 UTC (permalink / raw)
  To: Martin Blumenstingl, linux-amlogic, linux-clk
  Cc: sboyd, mturquette, linux-kernel, linux-arm-kernel, narmstrong


On Tue 14 Apr 2020 at 22:00, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_POST and
> CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_PRE are active low. This means:
> - asserting them requires setting the register value to 0
> - de-asserting them requires setting the register value to 1
>
> Set the register value accordingly for these two reset lines by setting
> the inverted the register value compared to all other reset lines.
>
> Fixes: 189621726bc2f6 ("clk: meson: meson8b: register the built-in reset controller")
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  drivers/clk/meson/meson8b.c | 81 ++++++++++++++++++++++++++-----------
>  1 file changed, 58 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
> index 90d284ffc780..fa251e45e208 100644
> --- a/drivers/clk/meson/meson8b.c
> +++ b/drivers/clk/meson/meson8b.c
> @@ -3506,54 +3506,87 @@ static struct clk_regmap *const meson8b_clk_regmaps[] = {
>  static const struct meson8b_clk_reset_line {
>  	u32 reg;
>  	u8 bit_idx;
> +	bool active_low;
>  } meson8b_clk_reset_bits[] = {
>  	[CLKC_RESET_L2_CACHE_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 30
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 30,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_AXI_64_TO_128_BRIDGE_A5_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 29
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 29,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_SCU_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 28
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 28,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_CPU3_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 27
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 27,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_CPU2_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 26
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 26,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_CPU1_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 25
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 25,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_CPU0_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 24
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 24,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_A5_GLOBAL_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 18
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 18,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_A5_AXI_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 17
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 17,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_A5_ABP_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 16
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 16,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_AXI_64_TO_128_BRIDGE_MMC_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL1, .bit_idx = 30
> +		.reg = HHI_SYS_CPU_CLK_CNTL1,
> +		.bit_idx = 30,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_VID_CLK_CNTL_SOFT_RESET] = {
> -		.reg = HHI_VID_CLK_CNTL, .bit_idx = 15
> +		.reg = HHI_VID_CLK_CNTL,
> +		.bit_idx = 15,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_VID_DIVIDER_CNTL_SOFT_RESET_POST] = {
> -		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 7
> +		.reg = HHI_VID_DIVIDER_CNTL,
> +		.bit_idx = 7,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_VID_DIVIDER_CNTL_SOFT_RESET_PRE] = {
> -		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 3
> +		.reg = HHI_VID_DIVIDER_CNTL,
> +		.bit_idx = 3,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_POST] = {
> -		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 1
> +		.reg = HHI_VID_DIVIDER_CNTL,
> +		.bit_idx = 1,
> +		.active_low = true,
>  	},
>  	[CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_PRE] = {
> -		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 0
> +		.reg = HHI_VID_DIVIDER_CNTL,
> +		.bit_idx = 0,
> +		.active_low = true,
>  	},
>  };
>  
> @@ -3562,22 +3595,24 @@ static int meson8b_clk_reset_update(struct reset_controller_dev *rcdev,
>  {
>  	struct meson8b_clk_reset *meson8b_clk_reset =
>  		container_of(rcdev, struct meson8b_clk_reset, reset);
> -	unsigned long flags;
>  	const struct meson8b_clk_reset_line *reset;
> +	unsigned long flags;
> +	unsigned int value;

suggestion:

unsigned int value = 0;

>  
>  	if (id >= ARRAY_SIZE(meson8b_clk_reset_bits))
>  		return -EINVAL;
>  
>  	reset = &meson8b_clk_reset_bits[id];
>  
> +	if (assert == reset->active_low)
> +		value = 0;
> +	else
> +		value = BIT(reset->bit_idx);

if (assert ^ reset->active_low)
	value = BIT(reset->bit_idx);

?

> +
>  	spin_lock_irqsave(&meson_clk_lock, flags);
>  
> -	if (assert)
> -		regmap_update_bits(meson8b_clk_reset->regmap, reset->reg,
> -				   BIT(reset->bit_idx), BIT(reset->bit_idx));
> -	else
> -		regmap_update_bits(meson8b_clk_reset->regmap, reset->reg,
> -				   BIT(reset->bit_idx), 0);
> +	regmap_update_bits(meson8b_clk_reset->regmap, reset->reg,
> +			   BIT(reset->bit_idx), value);
>  
>  	spin_unlock_irqrestore(&meson_clk_lock, flags);


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/4] clk: meson: meson8b: Fix the polarity of the RESET_N lines
@ 2020-04-16 10:38     ` Jerome Brunet
  0 siblings, 0 replies; 30+ messages in thread
From: Jerome Brunet @ 2020-04-16 10:38 UTC (permalink / raw)
  To: Martin Blumenstingl, linux-amlogic, linux-clk
  Cc: sboyd, mturquette, linux-kernel, linux-arm-kernel, narmstrong


On Tue 14 Apr 2020 at 22:00, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_POST and
> CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_PRE are active low. This means:
> - asserting them requires setting the register value to 0
> - de-asserting them requires setting the register value to 1
>
> Set the register value accordingly for these two reset lines by setting
> the inverted the register value compared to all other reset lines.
>
> Fixes: 189621726bc2f6 ("clk: meson: meson8b: register the built-in reset controller")
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  drivers/clk/meson/meson8b.c | 81 ++++++++++++++++++++++++++-----------
>  1 file changed, 58 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
> index 90d284ffc780..fa251e45e208 100644
> --- a/drivers/clk/meson/meson8b.c
> +++ b/drivers/clk/meson/meson8b.c
> @@ -3506,54 +3506,87 @@ static struct clk_regmap *const meson8b_clk_regmaps[] = {
>  static const struct meson8b_clk_reset_line {
>  	u32 reg;
>  	u8 bit_idx;
> +	bool active_low;
>  } meson8b_clk_reset_bits[] = {
>  	[CLKC_RESET_L2_CACHE_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 30
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 30,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_AXI_64_TO_128_BRIDGE_A5_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 29
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 29,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_SCU_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 28
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 28,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_CPU3_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 27
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 27,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_CPU2_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 26
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 26,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_CPU1_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 25
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 25,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_CPU0_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 24
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 24,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_A5_GLOBAL_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 18
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 18,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_A5_AXI_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 17
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 17,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_A5_ABP_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 16
> +		.reg = HHI_SYS_CPU_CLK_CNTL0,
> +		.bit_idx = 16,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_AXI_64_TO_128_BRIDGE_MMC_SOFT_RESET] = {
> -		.reg = HHI_SYS_CPU_CLK_CNTL1, .bit_idx = 30
> +		.reg = HHI_SYS_CPU_CLK_CNTL1,
> +		.bit_idx = 30,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_VID_CLK_CNTL_SOFT_RESET] = {
> -		.reg = HHI_VID_CLK_CNTL, .bit_idx = 15
> +		.reg = HHI_VID_CLK_CNTL,
> +		.bit_idx = 15,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_VID_DIVIDER_CNTL_SOFT_RESET_POST] = {
> -		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 7
> +		.reg = HHI_VID_DIVIDER_CNTL,
> +		.bit_idx = 7,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_VID_DIVIDER_CNTL_SOFT_RESET_PRE] = {
> -		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 3
> +		.reg = HHI_VID_DIVIDER_CNTL,
> +		.bit_idx = 3,
> +		.active_low = false,
>  	},
>  	[CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_POST] = {
> -		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 1
> +		.reg = HHI_VID_DIVIDER_CNTL,
> +		.bit_idx = 1,
> +		.active_low = true,
>  	},
>  	[CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_PRE] = {
> -		.reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 0
> +		.reg = HHI_VID_DIVIDER_CNTL,
> +		.bit_idx = 0,
> +		.active_low = true,
>  	},
>  };
>  
> @@ -3562,22 +3595,24 @@ static int meson8b_clk_reset_update(struct reset_controller_dev *rcdev,
>  {
>  	struct meson8b_clk_reset *meson8b_clk_reset =
>  		container_of(rcdev, struct meson8b_clk_reset, reset);
> -	unsigned long flags;
>  	const struct meson8b_clk_reset_line *reset;
> +	unsigned long flags;
> +	unsigned int value;

suggestion:

unsigned int value = 0;

>  
>  	if (id >= ARRAY_SIZE(meson8b_clk_reset_bits))
>  		return -EINVAL;
>  
>  	reset = &meson8b_clk_reset_bits[id];
>  
> +	if (assert == reset->active_low)
> +		value = 0;
> +	else
> +		value = BIT(reset->bit_idx);

if (assert ^ reset->active_low)
	value = BIT(reset->bit_idx);

?

> +
>  	spin_lock_irqsave(&meson_clk_lock, flags);
>  
> -	if (assert)
> -		regmap_update_bits(meson8b_clk_reset->regmap, reset->reg,
> -				   BIT(reset->bit_idx), BIT(reset->bit_idx));
> -	else
> -		regmap_update_bits(meson8b_clk_reset->regmap, reset->reg,
> -				   BIT(reset->bit_idx), 0);
> +	regmap_update_bits(meson8b_clk_reset->regmap, reset->reg,
> +			   BIT(reset->bit_idx), value);
>  
>  	spin_unlock_irqrestore(&meson_clk_lock, flags);


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/4] clk: meson: meson8b: Fix the first parent of vid_pll_in_sel
  2020-04-16 10:32     ` Jerome Brunet
  (?)
@ 2020-04-16 17:49       ` Martin Blumenstingl
  -1 siblings, 0 replies; 30+ messages in thread
From: Martin Blumenstingl @ 2020-04-16 17:49 UTC (permalink / raw)
  To: Jerome Brunet
  Cc: linux-amlogic, linux-clk, Neil Armstrong, mturquette, sboyd,
	linux-arm-kernel, linux-kernel

Hi Jerome,

On Thu, Apr 16, 2020 at 12:32 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
>
>
> On Tue 14 Apr 2020 at 22:00, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:
>
> > Use hdmi_pll_lvds_out as parent of the vid_pll_in_sel clock. It's not
> > easy to see that the vendor kernel does the same, but it actually does.
> > meson_clk_pll_ops in mainline still cannot fully recalculate all rates
> > from the HDMI PLL registers because some register bits (at the time of
> > writing it's unknown which bits are used for this) double the HDMI PLL
> > output rate (compared to simply considering M, N and FRAC).
>
> Have you considered adding a fixed_factor pre-multiplier, like in the
> gxbb driver ?
>
> Seems to be the same thing
it seems like I haven't been clear enough here: the doubling only
happens for some - but not all - PLL settings.

Let me give you two examples with values from the 3.10 vendor kernel:
1. the CVBS modes use a hdmi_pll_dco freq of 1296MHz
it uses: M = 54, N = 1 and FRAC = 0
with our existing clock tree this works out perfectly: 24MHz * 54 / 1 = 1296MHz

2. HDMI 1080P mode uses hdmi_pll_dco freq of 2970MHz
it uses M = 61, N = 1 and FRAC = 3584
with our existing clock tree this doesn't end up right: (24MHz * 61 /
1)  + (24MHz * 3584 / 4095) = 1485MHz

I did play with the registers and our clock-measurer.
it *seems* that the HHI_VID_PLL_CNTL3 and/or HHI_VID_PLL_CNTL4 are
related to this doubling, but I don't know for sure
my assumption is that there's either a fixed pre-multiplier like you
suggested and then a configurable "divide by 2" somewhere or there's
simply a configurable "multiply by 2" somewhere
Either way, I want to fix that at some point but since I don't know
the related bits I want to do that later on (in separate patches once
I have figured it out)


Regards
Martin

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/4] clk: meson: meson8b: Fix the first parent of vid_pll_in_sel
@ 2020-04-16 17:49       ` Martin Blumenstingl
  0 siblings, 0 replies; 30+ messages in thread
From: Martin Blumenstingl @ 2020-04-16 17:49 UTC (permalink / raw)
  To: Jerome Brunet
  Cc: Neil Armstrong, sboyd, mturquette, linux-kernel, linux-amlogic,
	linux-clk, linux-arm-kernel

Hi Jerome,

On Thu, Apr 16, 2020 at 12:32 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
>
>
> On Tue 14 Apr 2020 at 22:00, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:
>
> > Use hdmi_pll_lvds_out as parent of the vid_pll_in_sel clock. It's not
> > easy to see that the vendor kernel does the same, but it actually does.
> > meson_clk_pll_ops in mainline still cannot fully recalculate all rates
> > from the HDMI PLL registers because some register bits (at the time of
> > writing it's unknown which bits are used for this) double the HDMI PLL
> > output rate (compared to simply considering M, N and FRAC).
>
> Have you considered adding a fixed_factor pre-multiplier, like in the
> gxbb driver ?
>
> Seems to be the same thing
it seems like I haven't been clear enough here: the doubling only
happens for some - but not all - PLL settings.

Let me give you two examples with values from the 3.10 vendor kernel:
1. the CVBS modes use a hdmi_pll_dco freq of 1296MHz
it uses: M = 54, N = 1 and FRAC = 0
with our existing clock tree this works out perfectly: 24MHz * 54 / 1 = 1296MHz

2. HDMI 1080P mode uses hdmi_pll_dco freq of 2970MHz
it uses M = 61, N = 1 and FRAC = 3584
with our existing clock tree this doesn't end up right: (24MHz * 61 /
1)  + (24MHz * 3584 / 4095) = 1485MHz

I did play with the registers and our clock-measurer.
it *seems* that the HHI_VID_PLL_CNTL3 and/or HHI_VID_PLL_CNTL4 are
related to this doubling, but I don't know for sure
my assumption is that there's either a fixed pre-multiplier like you
suggested and then a configurable "divide by 2" somewhere or there's
simply a configurable "multiply by 2" somewhere
Either way, I want to fix that at some point but since I don't know
the related bits I want to do that later on (in separate patches once
I have figured it out)


Regards
Martin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/4] clk: meson: meson8b: Fix the first parent of vid_pll_in_sel
@ 2020-04-16 17:49       ` Martin Blumenstingl
  0 siblings, 0 replies; 30+ messages in thread
From: Martin Blumenstingl @ 2020-04-16 17:49 UTC (permalink / raw)
  To: Jerome Brunet
  Cc: Neil Armstrong, sboyd, mturquette, linux-kernel, linux-amlogic,
	linux-clk, linux-arm-kernel

Hi Jerome,

On Thu, Apr 16, 2020 at 12:32 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
>
>
> On Tue 14 Apr 2020 at 22:00, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:
>
> > Use hdmi_pll_lvds_out as parent of the vid_pll_in_sel clock. It's not
> > easy to see that the vendor kernel does the same, but it actually does.
> > meson_clk_pll_ops in mainline still cannot fully recalculate all rates
> > from the HDMI PLL registers because some register bits (at the time of
> > writing it's unknown which bits are used for this) double the HDMI PLL
> > output rate (compared to simply considering M, N and FRAC).
>
> Have you considered adding a fixed_factor pre-multiplier, like in the
> gxbb driver ?
>
> Seems to be the same thing
it seems like I haven't been clear enough here: the doubling only
happens for some - but not all - PLL settings.

Let me give you two examples with values from the 3.10 vendor kernel:
1. the CVBS modes use a hdmi_pll_dco freq of 1296MHz
it uses: M = 54, N = 1 and FRAC = 0
with our existing clock tree this works out perfectly: 24MHz * 54 / 1 = 1296MHz

2. HDMI 1080P mode uses hdmi_pll_dco freq of 2970MHz
it uses M = 61, N = 1 and FRAC = 3584
with our existing clock tree this doesn't end up right: (24MHz * 61 /
1)  + (24MHz * 3584 / 4095) = 1485MHz

I did play with the registers and our clock-measurer.
it *seems* that the HHI_VID_PLL_CNTL3 and/or HHI_VID_PLL_CNTL4 are
related to this doubling, but I don't know for sure
my assumption is that there's either a fixed pre-multiplier like you
suggested and then a configurable "divide by 2" somewhere or there's
simply a configurable "multiply by 2" somewhere
Either way, I want to fix that at some point but since I don't know
the related bits I want to do that later on (in separate patches once
I have figured it out)


Regards
Martin

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/4] clk: meson: meson8b: Fix the polarity of the RESET_N lines
  2020-04-16 10:38     ` Jerome Brunet
  (?)
@ 2020-04-16 18:12       ` Martin Blumenstingl
  -1 siblings, 0 replies; 30+ messages in thread
From: Martin Blumenstingl @ 2020-04-16 18:12 UTC (permalink / raw)
  To: Jerome Brunet
  Cc: linux-amlogic, linux-clk, Neil Armstrong, mturquette, sboyd,
	linux-arm-kernel, linux-kernel

Hi Jerome,

On Thu, Apr 16, 2020 at 12:38 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
[...]
> >
> >       if (id >= ARRAY_SIZE(meson8b_clk_reset_bits))
> >               return -EINVAL;
> >
> >       reset = &meson8b_clk_reset_bits[id];
> >
> > +     if (assert == reset->active_low)
> > +             value = 0;
> > +     else
> > +             value = BIT(reset->bit_idx);
>
> if (assert ^ reset->active_low)
>         value = BIT(reset->bit_idx);
I can do that, but I prefer "!=" over "^" because the result is
expected to be a bool (and because I'm not used to reading "^" for
logical comparisons)
will this work for you as well?


Martin

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/4] clk: meson: meson8b: Fix the polarity of the RESET_N lines
@ 2020-04-16 18:12       ` Martin Blumenstingl
  0 siblings, 0 replies; 30+ messages in thread
From: Martin Blumenstingl @ 2020-04-16 18:12 UTC (permalink / raw)
  To: Jerome Brunet
  Cc: Neil Armstrong, sboyd, mturquette, linux-kernel, linux-amlogic,
	linux-clk, linux-arm-kernel

Hi Jerome,

On Thu, Apr 16, 2020 at 12:38 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
[...]
> >
> >       if (id >= ARRAY_SIZE(meson8b_clk_reset_bits))
> >               return -EINVAL;
> >
> >       reset = &meson8b_clk_reset_bits[id];
> >
> > +     if (assert == reset->active_low)
> > +             value = 0;
> > +     else
> > +             value = BIT(reset->bit_idx);
>
> if (assert ^ reset->active_low)
>         value = BIT(reset->bit_idx);
I can do that, but I prefer "!=" over "^" because the result is
expected to be a bool (and because I'm not used to reading "^" for
logical comparisons)
will this work for you as well?


Martin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/4] clk: meson: meson8b: Fix the polarity of the RESET_N lines
@ 2020-04-16 18:12       ` Martin Blumenstingl
  0 siblings, 0 replies; 30+ messages in thread
From: Martin Blumenstingl @ 2020-04-16 18:12 UTC (permalink / raw)
  To: Jerome Brunet
  Cc: Neil Armstrong, sboyd, mturquette, linux-kernel, linux-amlogic,
	linux-clk, linux-arm-kernel

Hi Jerome,

On Thu, Apr 16, 2020 at 12:38 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
[...]
> >
> >       if (id >= ARRAY_SIZE(meson8b_clk_reset_bits))
> >               return -EINVAL;
> >
> >       reset = &meson8b_clk_reset_bits[id];
> >
> > +     if (assert == reset->active_low)
> > +             value = 0;
> > +     else
> > +             value = BIT(reset->bit_idx);
>
> if (assert ^ reset->active_low)
>         value = BIT(reset->bit_idx);
I can do that, but I prefer "!=" over "^" because the result is
expected to be a bool (and because I'm not used to reading "^" for
logical comparisons)
will this work for you as well?


Martin

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/4] clk: meson: meson8b: Fix the polarity of the RESET_N lines
  2020-04-16 18:12       ` Martin Blumenstingl
  (?)
@ 2020-04-17  7:43         ` Jerome Brunet
  -1 siblings, 0 replies; 30+ messages in thread
From: Jerome Brunet @ 2020-04-17  7:43 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: linux-amlogic, linux-clk, Neil Armstrong, mturquette, sboyd,
	linux-arm-kernel, linux-kernel


On Thu 16 Apr 2020 at 20:12, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> Hi Jerome,
>
> On Thu, Apr 16, 2020 at 12:38 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
> [...]
>> >
>> >       if (id >= ARRAY_SIZE(meson8b_clk_reset_bits))
>> >               return -EINVAL;
>> >
>> >       reset = &meson8b_clk_reset_bits[id];
>> >
>> > +     if (assert == reset->active_low)
>> > +             value = 0;
>> > +     else
>> > +             value = BIT(reset->bit_idx);
>>
>> if (assert ^ reset->active_low)
>>         value = BIT(reset->bit_idx);
> I can do that, but I prefer "!=" over "^" because the result is
> expected to be a bool (and because I'm not used to reading "^" for
> logical comparisons)
> will this work for you as well?

yes

>
>
> Martin


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/4] clk: meson: meson8b: Fix the polarity of the RESET_N lines
@ 2020-04-17  7:43         ` Jerome Brunet
  0 siblings, 0 replies; 30+ messages in thread
From: Jerome Brunet @ 2020-04-17  7:43 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: Neil Armstrong, sboyd, mturquette, linux-kernel, linux-amlogic,
	linux-clk, linux-arm-kernel


On Thu 16 Apr 2020 at 20:12, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> Hi Jerome,
>
> On Thu, Apr 16, 2020 at 12:38 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
> [...]
>> >
>> >       if (id >= ARRAY_SIZE(meson8b_clk_reset_bits))
>> >               return -EINVAL;
>> >
>> >       reset = &meson8b_clk_reset_bits[id];
>> >
>> > +     if (assert == reset->active_low)
>> > +             value = 0;
>> > +     else
>> > +             value = BIT(reset->bit_idx);
>>
>> if (assert ^ reset->active_low)
>>         value = BIT(reset->bit_idx);
> I can do that, but I prefer "!=" over "^" because the result is
> expected to be a bool (and because I'm not used to reading "^" for
> logical comparisons)
> will this work for you as well?

yes

>
>
> Martin


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/4] clk: meson: meson8b: Fix the polarity of the RESET_N lines
@ 2020-04-17  7:43         ` Jerome Brunet
  0 siblings, 0 replies; 30+ messages in thread
From: Jerome Brunet @ 2020-04-17  7:43 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: Neil Armstrong, sboyd, mturquette, linux-kernel, linux-amlogic,
	linux-clk, linux-arm-kernel


On Thu 16 Apr 2020 at 20:12, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> Hi Jerome,
>
> On Thu, Apr 16, 2020 at 12:38 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
> [...]
>> >
>> >       if (id >= ARRAY_SIZE(meson8b_clk_reset_bits))
>> >               return -EINVAL;
>> >
>> >       reset = &meson8b_clk_reset_bits[id];
>> >
>> > +     if (assert == reset->active_low)
>> > +             value = 0;
>> > +     else
>> > +             value = BIT(reset->bit_idx);
>>
>> if (assert ^ reset->active_low)
>>         value = BIT(reset->bit_idx);
> I can do that, but I prefer "!=" over "^" because the result is
> expected to be a bool (and because I'm not used to reading "^" for
> logical comparisons)
> will this work for you as well?

yes

>
>
> Martin


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2020-04-17  7:43 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-14 20:00 [PATCH 0/4] clk: meson8b: updates for video clocks / resets Martin Blumenstingl
2020-04-14 20:00 ` Martin Blumenstingl
2020-04-14 20:00 ` Martin Blumenstingl
2020-04-14 20:00 ` [PATCH 1/4] clk: meson: meson8b: Fix the first parent of vid_pll_in_sel Martin Blumenstingl
2020-04-14 20:00   ` Martin Blumenstingl
2020-04-14 20:00   ` Martin Blumenstingl
2020-04-16 10:32   ` Jerome Brunet
2020-04-16 10:32     ` Jerome Brunet
2020-04-16 10:32     ` Jerome Brunet
2020-04-16 17:49     ` Martin Blumenstingl
2020-04-16 17:49       ` Martin Blumenstingl
2020-04-16 17:49       ` Martin Blumenstingl
2020-04-14 20:00 ` [PATCH 2/4] clk: meson: meson8b: Fix the polarity of the RESET_N lines Martin Blumenstingl
2020-04-14 20:00   ` Martin Blumenstingl
2020-04-14 20:00   ` Martin Blumenstingl
2020-04-16 10:38   ` Jerome Brunet
2020-04-16 10:38     ` Jerome Brunet
2020-04-16 10:38     ` Jerome Brunet
2020-04-16 18:12     ` Martin Blumenstingl
2020-04-16 18:12       ` Martin Blumenstingl
2020-04-16 18:12       ` Martin Blumenstingl
2020-04-17  7:43       ` Jerome Brunet
2020-04-17  7:43         ` Jerome Brunet
2020-04-17  7:43         ` Jerome Brunet
2020-04-14 20:00 ` [PATCH 3/4] clk: meson: meson8b: Fix the vclk_div{1,2,4,6,12}_en gate bits Martin Blumenstingl
2020-04-14 20:00   ` [PATCH 3/4] clk: meson: meson8b: Fix the vclk_div{1, 2, 4, 6, 12}_en " Martin Blumenstingl
2020-04-14 20:00   ` Martin Blumenstingl
2020-04-14 20:00 ` [PATCH 4/4] clk: meson: meson8b: Make the CCF use the glitch-free VPU mux Martin Blumenstingl
2020-04-14 20:00   ` Martin Blumenstingl
2020-04-14 20:00   ` Martin Blumenstingl

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.