[PATCHv1 0/3] Odroid N2 failes to boot using upstream kernel using microSD card

Anand Moon

author

2020-02-16 17:34 UTC #0 lore raw

cover Odroid N2 failes to boot using upstream kernel using microSD card

We am trying to build the upstream u-boot and upstream kernel, but it fails to pass the initialization of PWM_MESON driver. So these patches help boot the kernel on microSD card.

Fix the clk driver help booting of the kernel.
Any more suggestion or inputs are welcome.

Changes since RFCv1
[0] https://lore.kernel.org/linux-amlogic/[email protected]/
drop some patches and fix the clk driver as suggested by Neil.

-Anand

Anand Moon (3):
arm64: dts: meson: Add missing regulator linked to VDDAO_3V3 regulator
to FLASH_VDD
arm64: dts: meson: Add missing regulator linked to VCCV5 regulator to
VDDIO_C/TF_IO
clk: meson: g12a: set cpu clock divider flags too CLK_IS_CRITICAL

 arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts | 6 ++++++
 drivers/clk/meson/g12a.c                             | 3 ++-
 2 files changed, 8 insertions(+), 1 deletion(-)

Signature · 1 line

2.25.0

Anand Moon

author

2020-02-16 17:35 UTC #1 lore raw

PATCH 1/3 arm64: dts: meson: Add missing regulator linked to VDDAO_3V3 regulator to FLASH_VDD

As per schematics add missing VDDAO_3V3 power supply to FLASH_VDD regulator. Also add TFLASH_VDD_EN signal name to gpio pin.

Fixes c35f6dc5c377 (arm64: dts: meson: Add minimal support for Odroid-N2)

Cc Martin Blumenstingl <[email protected]>

Cc Jerome Brunet <[email protected]>

Cc Neil Armstrong <[email protected]>

Reviewed-by Neil Armstrong <[email protected]>

Signed-off-by Anand Moon <[email protected]>

---
 arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts | 3 +++
 1 file changed, 3 insertions(+)

arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts +3 -0

index 0e54c1dc2842..353db3b32cc4 100644
@@ -51,9 +51,12 @@ tflash_vdd: regulator-tflash_vdd {
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
 
+		/* TFLASH_VDD_EN */
 		gpio = <&gpio_ao GPIOAO_8 GPIO_ACTIVE_HIGH>;
 		enable-active-high;
 		regulator-always-on;
+		/* U18 FC8731-09VF05NRR */
+		vin-supply = <&vddao_3v3>;
 	};
 
 	tf_io: gpio-regulator-tf_io {

Signature · 1 line

2.25.0

Anand Moon

author

2020-02-16 17:35 UTC #2 lore raw

PATCH 2/3 arm64: dts: meson: Add missing regulator linked to VCCV5 regulator to VDDIO_C/TF_IO

In reply to Anand Moon

As per schematics add missing VCCV5 power supply to VDDIO_C/TF_IO regulator. Also add TF_3V3N_1V8_EN signal name to gpio pin.

Fixes c35f6dc5c377 (arm64: dts: meson: Add minimal support for Odroid-N2)

Cc Martin Blumenstingl <[email protected]>

Cc Jerome Brunet <[email protected]>

Cc Neil Armstrong <[email protected]>

Reviewed-by Neil Armstrong <[email protected]>

Signed-off-by Anand Moon <[email protected]>

---
 arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts | 3 +++
 1 file changed, 3 insertions(+)

arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts +3 -0

index 353db3b32cc4..23eddff85fe5 100644
@@ -66,11 +66,14 @@ tf_io: gpio-regulator-tf_io {
 		regulator-min-microvolt = <1800000>;
 		regulator-max-microvolt = <3300000>;
 
+		/* TF_3V3N_1V8_EN */
 		gpios = <&gpio_ao GPIOAO_9 GPIO_ACTIVE_HIGH>;
 		gpios-states = <0>;
 
 		states = <3300000 0>,
 			 <1800000 1>;
+		/* U16 RT9179GB */
+		vin-supply = <&vcc_5v>;
 	};
 
 	flash_1v8: regulator-flash_1v8 {

Signature · 1 line

2.25.0

Jerome Brunet

maintainer

2020-02-17 07:49 UTC #3 lore raw

Re: [PATCHv1 2/3] arm64: dts: meson: Add missing regulator linked to VCCV5 regulator to VDDIO_C/TF_IO

Quoted 2 lines · Anand Moon

As per schematics add missing VCCV5 power supply to VDDIO_C/TF_IO regulator. Also add TF_3V3N_1V8_EN signal name to gpio pin.

Why ? I don't see the connection with the cover letter here ...

Quoted 16 lines

Fixes c35f6dc5c377 (arm64: dts: meson: Add minimal support for Odroid-N2)
Cc Martin Blumenstingl <[email protected]>
Cc Jerome Brunet <[email protected]>
Cc Neil Armstrong <[email protected]>
Reviewed-by Neil Armstrong <[email protected]>
Signed-off-by Anand Moon <[email protected]>
---
 arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts | 3 +++
 1 file changed, 3 insertions(+)
arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts +0 -0
index 353db3b32cc4..23eddff85fe5 100644

arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts tf_io: gpio-regulator-tf_io { · line 69

Quoted 4 lines

arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts +1 -0

@@ -66,11 +66,14 @@ tf_io: gpio-regulator-tf_io {
 		regulator-min-microvolt = <1800000>;
 		regulator-max-microvolt = <3300000>;
 
+		/* TF_3V3N_1V8_EN */

This is not terribly useful ... same for the previous patch

arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts tf_io: gpio-regulator-tf_io { · line 76

Quoted 7 lines

arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts +2 -0

 		gpios = <&gpio_ao GPIOAO_9 GPIO_ACTIVE_HIGH>;
 		gpios-states = <0>;
 
 		states = <3300000 0>,
 			 <1800000 1>;
+		/* U16 RT9179GB */
+		vin-supply = <&vcc_5v>;

That is not parsed and not even part of the gpio regulator binding documentation. It won't make any difference.

Quoted 3 lines

arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts +0 -0
 	};
 
 	flash_1v8: regulator-flash_1v8 {

Anand Moon

author

2020-02-16 17:35 UTC #4 lore raw

PATCH 3/3 clk: meson: g12a: set cpu clock divider flags too CLK_IS_CRITICAL

In reply to Anand Moon

Odroid N2 would fail to boot using microSD unless we set cpu freq clk divider flags to CLK_IS_CRITICAL to avoid stalling of cpu when booting, most likely because of PWM module linked to the CPU for DVFS is getting disabled in between the late_init call, so gaiting the clock source shuts down the power to the codes. Setting clk divider flags to CLK_IS_CRITICAL help resolve the issue.

Cc Martin Blumenstingl <[email protected]>

Cc Jerome Brunet <[email protected]>

Cc Neil Armstrong <[email protected]>

Signed-off-by Anand Moon <[email protected]>

---

Following Neil's suggestion, I have prepared this patch.
https://patchwork.kernel.org/patch/11177441/#22964889

---
 drivers/clk/meson/g12a.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/clk/meson/g12a.c +2 -1

index d2760a021301..accae3695fe5 100644
@@ -283,6 +283,7 @@ static struct clk_fixed_factor g12a_fclk_div2_div = {
 		.ops = &clk_fixed_factor_ops,
 		.parent_hws = (const struct clk_hw *[]) { &g12a_fixed_pll.hw },
 		.num_parents = 1,
+		.flags = CLK_IS_CRITICAL,
 	},
 };

@@ -681,7 +682,7 @@ static struct clk_regmap g12b_cpub_clk = {
 			&g12a_sys_pll.hw
 		},
 		.num_parents = 2,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
 	},
 };

Signature · 1 line

2.25.0

Jerome Brunet

maintainer

2020-02-17 08:02 UTC #5 lore raw

Re: [PATCHv1 3/3] clk: meson: g12a: set cpu clock divider flags too CLK_IS_CRITICAL

Quoted 3 lines · Anand Moon

Odroid N2 would fail to boot using microSD unless we set cpu freq clk divider flags to CLK_IS_CRITICAL to avoid stalling of cpu when booting, most likely because of PWM module linked to

Where did you see a PWM ?

Quoted 1 line

the CPU for DVFS is getting disabled in between the late_init call,

between the late_init call and what ?

Quoted 1 line

so gaiting the clock source shuts down the power to the codes.

what code ?

Quoted 20 lines

Setting clk divider flags to CLK_IS_CRITICAL help resolve the issue.
Cc Martin Blumenstingl <[email protected]>
Cc Jerome Brunet <[email protected]>
Cc Neil Armstrong <[email protected]>
Suggested-by Neil Armstrong <[email protected]>
Signed-off-by Anand Moon <[email protected]>
---
Following Neil's suggestion, I have prepared this patch.
https://patchwork.kernel.org/patch/11177441/#22964889
---
 drivers/clk/meson/g12a.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/clk/meson/g12a.c +0 -0
index d2760a021301..accae3695fe5 100644

drivers/clk/meson/g12a.c static struct clk_fixed_factor g12a_fclk_div2_div = { · line 286

Quoted 4 lines

drivers/clk/meson/g12a.c +1 -0

@@ -283,6 +283,7 @@ static struct clk_fixed_factor g12a_fclk_div2_div = {
 		.ops = &clk_fixed_factor_ops,
 		.parent_hws = (const struct clk_hw *[]) { &g12a_fixed_pll.hw },
 		.num_parents = 1,
+		.flags = CLK_IS_CRITICAL,

This makes no sense for because:
* This clock cannot gate and none of its parents can either. IOW, the output of this clock is never disabled.
* I cannot guess the relation between fdiv2 and the commit description

drivers/clk/meson/g12a.c static struct clk_regmap g12b_cpub_clk = { · line 685

Quoted 9 lines

drivers/clk/meson/g12a.c +1 -1

 	},
 };

@@ -681,7 +682,7 @@ static struct clk_regmap g12b_cpub_clk = {
 			&g12a_sys_pll.hw
 		},
 		.num_parents = 2,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,

Why not. Neil what do you think of this ?
If nothing is claiming this clock and enabling it then I suppose it could make sense.

Quoted 2 lines

drivers/clk/meson/g12a.c +0 -0
 	},
 };

Anand Moon

author

2020-02-17 08:51 UTC #6 lore raw

Re: [PATCHv1 3/3] clk: meson: g12a: set cpu clock divider flags too CLK_IS_CRITICAL

Hi Jerome,

Thanks for your review comments.

Quoted 51 lines · Jerome Brunet

Quoted 3 lines · Anand Moon
Odroid N2 would fail to boot using microSD unless we set cpu freq clk divider flags to CLK_IS_CRITICAL to avoid stalling of cpu when booting, most likely because of PWM module linked to
Where did you see a PWM ?
Quoted 1 line
the CPU for DVFS is getting disabled in between the late_init call,
between the late_init call and what ?
Quoted 1 line
so gaiting the clock source shuts down the power to the codes.
what code ?
Quoted 20 lines
Setting clk divider flags to CLK_IS_CRITICAL help resolve the issue.
Cc Martin Blumenstingl <[email protected]>
Cc Jerome Brunet <[email protected]>
Cc Neil Armstrong <[email protected]>
Suggested-by Neil Armstrong <[email protected]>
Signed-off-by Anand Moon <[email protected]>
---
Following Neil's suggestion, I have prepared this patch.
https://patchwork.kernel.org/patch/11177441/#22964889
---
 drivers/clk/meson/g12a.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/clk/meson/g12a.c +0 -0
index d2760a021301..accae3695fe5 100644
drivers/clk/meson/g12a.c static struct clk_fixed_factor g12a_fclk_div2_div = { · line 286
Quoted 4 lines
drivers/clk/meson/g12a.c +1 -0
@@ -283,6 +283,7 @@ static struct clk_fixed_factor g12a_fclk_div2_div = {
              .ops = &clk_fixed_factor_ops,
              .parent_hws = (const struct clk_hw *[]) { &g12a_fixed_pll.hw },
              .num_parents = 1,
+             .flags = CLK_IS_CRITICAL,
This makes no sense for because:
* This clock cannot gate and none of its parents can either. IOW, the output of this clock is never disabled.
* I cannot guess the relation between fdiv2 and the commit description
Quoted 3 lines
drivers/clk/meson/g12a.c +0 -0
      },
 };
 

Quoted 15 lines

drivers/clk/meson/g12a.c static struct clk_regmap g12b_cpub_clk = { · line 685
Quoted 6 lines
drivers/clk/meson/g12a.c +1 -1
@@ -681,7 +682,7 @@ static struct clk_regmap g12b_cpub_clk = {
                      &g12a_sys_pll.hw
              },
              .num_parents = 2,
-             .flags = CLK_SET_RATE_PARENT,
+             .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
Why not. Neil what do you think of this ?
If nothing is claiming this clock and enabling it then I suppose it could make sense.
Quoted 2 lines
drivers/clk/meson/g12a.c +0 -0
      },
 };

Sorry for the noise, I should not have send this patch in first place.

-Anand

Anand Moon

author

2020-02-17 13:30 UTC #7 lore raw

Re: [PATCHv1 3/3] clk: meson: g12a: set cpu clock divider flags too CLK_IS_CRITICAL

In reply to Jerome Brunet

Hi Jeronme,

Thanks for your review comments.

Quoted 10 lines · Jerome Brunet

Quoted 3 lines · Anand Moon
Odroid N2 would fail to boot using microSD unless we set cpu freq clk divider flags to CLK_IS_CRITICAL to avoid stalling of cpu when booting, most likely because of PWM module linked to
Where did you see a PWM ?

Quoted 8 lines

Quoted 1 line
the CPU for DVFS is getting disabled in between the late_init call,
between the late_init call and what ?
Quoted 1 line
so gaiting the clock source shuts down the power to the codes.
what code ?

Sorry, I was really upset about my self.
I tried to improvise this commit message based on previous mails. sorry about that.

Quoted 15 lines

Quoted 15 lines
Setting clk divider flags to CLK_IS_CRITICAL help resolve the issue.
Cc Martin Blumenstingl <[email protected]>
Cc Jerome Brunet <[email protected]>
Cc Neil Armstrong <[email protected]>
Suggested-by Neil Armstrong <[email protected]>
Signed-off-by Anand Moon <[email protected]>
---
Following Neil's suggestion, I have prepared this patch.
https://patchwork.kernel.org/patch/11177441/#22964889
---
 drivers/clk/meson/g12a.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Quoted 15 lines

drivers/clk/meson/g12a.c static struct clk_fixed_factor g12a_fclk_div2_div = { · line 286
Quoted 9 lines
drivers/clk/meson/g12a.c +1 -0
index d2760a021301..accae3695fe5 100644
@@ -283,6 +283,7 @@ static struct clk_fixed_factor g12a_fclk_div2_div = {
              .ops = &clk_fixed_factor_ops,
              .parent_hws = (const struct clk_hw *[]) { &g12a_fixed_pll.hw },
              .num_parents = 1,
+             .flags = CLK_IS_CRITICAL,
This makes no sense for because:
* This clock cannot gate and none of its parents can either. IOW, the output of this clock is never disabled.
* I cannot guess the relation between fdiv2 and the commit description

Ok I check this code changes is not needed for this fix.

Quoted 14 lines

drivers/clk/meson/g12a.c static struct clk_regmap g12b_cpub_clk = { · line 685
Quoted 9 lines
drivers/clk/meson/g12a.c +1 -1
      },
 };
 
@@ -681,7 +682,7 @@ static struct clk_regmap g12b_cpub_clk = {
                      &g12a_sys_pll.hw
              },
              .num_parents = 2,
-             .flags = CLK_SET_RATE_PARENT,
+             .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
Why not. Neil what do you think of this ?
If nothing is claiming this clock and enabling it then I suppose it could make sense.

I would like core developers to handle this.
Sorry for the noise.

-Anand

Quoted 4 lines

Quoted 3 lines
drivers/clk/meson/g12a.c +0 -0
      },
 };

Martin Blumenstingl

reviewer

2020-02-20 21:15 UTC #8 lore raw

Re: [PATCHv1 3/3] clk: meson: g12a: set cpu clock divider flags too CLK_IS_CRITICAL

Hi Anand,

On Mon, Feb 17, 2020 at 2:30 PM Anand Moon <[email protected]> wrote:
[...]

Quoted 11 lines

Quoted 11 lines
drivers/clk/meson/g12a.c static struct clk_regmap g12b_cpub_clk = { · line 685
Quoted 6 lines
drivers/clk/meson/g12a.c +1 -1
@@ -681,7 +682,7 @@ static struct clk_regmap g12b_cpub_clk = {
                      &g12a_sys_pll.hw
              },
              .num_parents = 2,
-             .flags = CLK_SET_RATE_PARENT,
+             .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
Why not. Neil what do you think of this ?
If nothing is claiming this clock and enabling it then I suppose it could make sense.

Quoted 2 lines

I would like core developers to handle this.
Sorry for the noise.

can you please resend this patch with only the change to g12b_cpub_clk? I have no G12B board myself so it would be great if you could take care of this!

Martin

Anand Moon

author

2020-02-23 13:34 UTC #9 lore raw

Re: [PATCHv1 3/3] clk: meson: g12a: set cpu clock divider flags too CLK_IS_CRITICAL

Hi Martin / Jerome / Neil,

Quoted 16 lines · Martin Blumenstingl

Hi Anand,
On Mon, Feb 17, 2020 at 2:30 PM Anand Moon <[email protected]> wrote:
[...]
Quoted 11 lines
Quoted 11 lines
drivers/clk/meson/g12a.c static struct clk_regmap g12b_cpub_clk = { · line 685
Quoted 6 lines
drivers/clk/meson/g12a.c +1 -1
@@ -681,7 +682,7 @@ static struct clk_regmap g12b_cpub_clk = {
                      &g12a_sys_pll.hw
              },
              .num_parents = 2,
-             .flags = CLK_SET_RATE_PARENT,
+             .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
Why not. Neil what do you think of this ?
If nothing is claiming this clock and enabling it then I suppose it could make sense.

Quoted 7 lines

Quoted 2 lines
I would like core developers to handle this.
Sorry for the noise.
can you please resend this patch with only the change to g12b_cpub_clk? I have no G12B board myself so it would be great if you could take care of this!
Martin

Thanks, yes I will try again, but I have a question.

On eMMC module *cpub_clk* is not getting enabled, see below is clk_summay of eMMC.
[...]

          fclk_div2_div               1        1        0   999999985
        0     0  50000
             fclk_div2                2        2        0   999999985
        0     0  50000
                ff3f0000.ethernet#m250_sel       1        1        0
999999985          0     0  50000
                   ff3f0000.ethernet#m250_div       1        1
0   249999997          0     0  50000
                      ff3f0000.ethernet#fixed_div2       1        1
    0   124999998          0     0  50000
                         ff3f0000.ethernet#rgmii_tx_en       1
1        0   124999998          0     0  50000
                ffe07000.mmc#mux       1        1        0   999999985
         0     0  50000
                   ffe07000.mmc#div       1        1        0
199999997          0     0  50000
                cpub_clk_dyn1_sel       0        0        0
999999985          0     0  50000
                   cpub_clk_dyn1       0        0        0   999999985
         0     0  50000
                      cpub_clk_dyn       0        0        0

999999985 0 0 50000
cpub_clk 0 0 0
999999985 0 0 50000
cpub_clk_div8 0 0 0
124999998 0 0 50000
cpub_clk_div7 0 0 0
142857140 0 0 50000
cpub_clk_div6 0 0 0
166666664 0 0 50000
cpub_clk_trace_sel 0 0
0 166666664 0 0 50000
cpub_clk_trace 0 0
0 166666664 0 0 50000
cpub_clk_div5 0 0 0
199999997 0 0 50000
cpub_clk_apb_sel 0 0
0 199999997 0 0 50000
cpub_clk_apb 0 0
0 199999997 0 0 50000
cpub_clk_div4 0 0 0
249999996 0 0 50000
cpub_clk_div3 0 0 0
333333328 0 0 50000
cpub_clk_atb_sel 0 0
0 333333328 0 0 50000
cpub_clk_atb 0 0
0 333333328 0 0 50000
cpub_clk_div2 0 0 0
499999992 0 0 50000
cpub_clk_axi_sel 0 0
0 499999992 0 0 50000
cpub_clk_axi 0 0
0 499999992 0 0 50000
cpub_clk_div16_en 0 0
0 999999985 0 0 50000
cpub_clk_div16 0 0
0 62499999 0 0 50000

After enable *cpub_clk* flags with
.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, this clk is enabled on microSD card see clk_summary below. [...]

         fclk_div2_div               1        1        0   999999985
       0     0  50000
             fclk_div2                3        3        0   999999985
        0     0  50000
                ff3f0000.ethernet#m250_sel       1        1        0
999999985          0     0  50000
                   ff3f0000.ethernet#m250_div       1        1
0   249999997          0     0  50000
                      ff3f0000.ethernet#fixed_div2       1        1
    0   124999998          0     0  50000
                         ff3f0000.ethernet#rgmii_tx_en       1
1        0   124999998          0     0  50000
                ffe05000.sd#mux       1        1        0   999999985
        0     0  50000
                   ffe05000.sd#div       1        1        0
50000000          0     0  50000
                cpub_clk_dyn1_sel       1        1        0
999999985          0     0  50000
                   cpub_clk_dyn1       1        1        0   999999985
         0     0  50000
                      cpub_clk_dyn       1        1        0

999999985 0 0 50000
cpub_clk 1 1 0
999999985 0 0 50000
cpub_clk_div8 0 0 0
124999998 0 0 50000
cpub_clk_div7 0 0 0
142857140 0 0 50000
cpub_clk_div6 0 0 0
166666664 0 0 50000
cpub_clk_trace_sel 0 0
0 166666664 0 0 50000
cpub_clk_trace 0 0
0 166666664 0 0 50000
cpub_clk_div5 0 0 0
199999997 0 0 50000
cpub_clk_apb_sel 0 0
0 199999997 0 0 50000
cpub_clk_apb 0 0
0 199999997 0 0 50000
cpub_clk_div4 0 0 0
249999996 0 0 50000
cpub_clk_div3 0 0 0
333333328 0 0 50000
cpub_clk_atb_sel 0 0
0 333333328 0 0 50000
cpub_clk_atb 0 0
0 333333328 0 0 50000
cpub_clk_div2 0 0 0
499999992 0 0 50000
cpub_clk_axi_sel 0 0
0 499999992 0 0 50000
cpub_clk_axi 0 0
0 499999992 0 0 50000
cpub_clk_div16_en 0 0
0 999999985 0 0 50000
cpub_clk_div16 0 0
0 62499999 0 0 50000
cpub_clk_dyn1_div 0 0 0
999999985 0 0 50000

Is this correct approach to set the flags to enable *cpub_clk*. .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,

What I meant is their *Dyn_enable[26]* field for enable/disable for HHI_SYS_CPU_CLK_CNTL0 and HHI_SYS_CPUB_CLK_CNTL clk controller. in the S922X datasheets which could help resolve this issue. Any thought on this.

-Anand

Jerome Brunet

maintainer

2020-02-24 09:31 UTC #10 lore raw

Re: [PATCHv1 3/3] clk: meson: g12a: set cpu clock divider flags too CLK_IS_CRITICAL

Quoted 26 lines · Anand Moon

Hi Martin / Jerome / Neil,
Quoted 22 lines · Martin Blumenstingl
Hi Anand,
On Mon, Feb 17, 2020 at 2:30 PM Anand Moon <[email protected]> wrote:
[...]
Quoted 11 lines
Quoted 11 lines
drivers/clk/meson/g12a.c static struct clk_regmap g12b_cpub_clk = { · line 685
Quoted 6 lines
drivers/clk/meson/g12a.c +1 -1
@@ -681,7 +682,7 @@ static struct clk_regmap g12b_cpub_clk = {
                      &g12a_sys_pll.hw
              },
              .num_parents = 2,
-             .flags = CLK_SET_RATE_PARENT,
+             .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
Why not. Neil what do you think of this ?
If nothing is claiming this clock and enabling it then I suppose it could make sense.
Quoted 2 lines
I would like core developers to handle this.
Sorry for the noise.
can you please resend this patch with only the change to g12b_cpub_clk? I have no G12B board myself so it would be great if you could take care of this!

Quoted 6 lines

Quoted 1 line
Martin
Thanks, yes I will try again, but I have a question.
On eMMC module *cpub_clk* is not getting enabled, see below is clk_summay of eMMC.

I'm sorry but I don't understand the link between the cpu clock of the second cluster and MMC.

Quoted 51 lines

[...]
          fclk_div2_div               1        1        0   999999985
        0     0  50000
             fclk_div2                2        2        0   999999985
        0     0  50000
                ff3f0000.ethernet#m250_sel       1        1        0
999999985          0     0  50000
                   ff3f0000.ethernet#m250_div       1        1
0   249999997          0     0  50000
                      ff3f0000.ethernet#fixed_div2       1        1
    0   124999998          0     0  50000
                         ff3f0000.ethernet#rgmii_tx_en       1
1        0   124999998          0     0  50000
                ffe07000.mmc#mux       1        1        0   999999985
         0     0  50000
                   ffe07000.mmc#div       1        1        0
199999997          0     0  50000
                cpub_clk_dyn1_sel       0        0        0
999999985          0     0  50000
                   cpub_clk_dyn1       0        0        0   999999985
         0     0  50000
                      cpub_clk_dyn       0        0        0
999999985 0 0 50000
cpub_clk 0 0 0
999999985 0 0 50000
cpub_clk_div8 0 0 0
124999998 0 0 50000
cpub_clk_div7 0 0 0
142857140 0 0 50000
cpub_clk_div6 0 0 0
166666664 0 0 50000
cpub_clk_trace_sel 0 0
0 166666664 0 0 50000
cpub_clk_trace 0 0
0 166666664 0 0 50000
cpub_clk_div5 0 0 0
199999997 0 0 50000
cpub_clk_apb_sel 0 0
0 199999997 0 0 50000
cpub_clk_apb 0 0
0 199999997 0 0 50000
cpub_clk_div4 0 0 0
249999996 0 0 50000
cpub_clk_div3 0 0 0
333333328 0 0 50000
cpub_clk_atb_sel 0 0
0 333333328 0 0 50000
cpub_clk_atb 0 0
0 333333328 0 0 50000
cpub_clk_div2 0 0 0
499999992 0 0 50000

Quoted 8 lines

cpub_clk_axi_sel 0 0
0 499999992 0 0 50000
cpub_clk_axi 0 0
0 499999992 0 0 50000
cpub_clk_div16_en 0 0
0 999999985 0 0 50000
cpub_clk_div16 0 0
0 62499999 0 0 50000

I can't read that.

Quoted 4 lines

After enable *cpub_clk* flags with
.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, this clk is enabled on microSD card see clk_summary below.

Again, I don't get the relationship between cpub and sdcard (or eMMC)

Quoted 62 lines

[...]
         fclk_div2_div               1        1        0   999999985
       0     0  50000
             fclk_div2                3        3        0   999999985
        0     0  50000
                ff3f0000.ethernet#m250_sel       1        1        0
999999985          0     0  50000
                   ff3f0000.ethernet#m250_div       1        1
0   249999997          0     0  50000
                      ff3f0000.ethernet#fixed_div2       1        1
    0   124999998          0     0  50000
                         ff3f0000.ethernet#rgmii_tx_en       1
1        0   124999998          0     0  50000
                ffe05000.sd#mux       1        1        0   999999985
        0     0  50000
                   ffe05000.sd#div       1        1        0
50000000          0     0  50000
                cpub_clk_dyn1_sel       1        1        0
999999985          0     0  50000
                   cpub_clk_dyn1       1        1        0   999999985
         0     0  50000
                      cpub_clk_dyn       1        1        0
999999985 0 0 50000
cpub_clk 1 1 0
999999985 0 0 50000
cpub_clk_div8 0 0 0
124999998 0 0 50000
cpub_clk_div7 0 0 0
142857140 0 0 50000
cpub_clk_div6 0 0 0
166666664 0 0 50000
cpub_clk_trace_sel 0 0
0 166666664 0 0 50000
cpub_clk_trace 0 0
0 166666664 0 0 50000
cpub_clk_div5 0 0 0
199999997 0 0 50000
cpub_clk_apb_sel 0 0
0 199999997 0 0 50000
cpub_clk_apb 0 0
0 199999997 0 0 50000
cpub_clk_div4 0 0 0
249999996 0 0 50000
cpub_clk_div3 0 0 0
333333328 0 0 50000
cpub_clk_atb_sel 0 0
0 333333328 0 0 50000
cpub_clk_atb 0 0
0 333333328 0 0 50000
cpub_clk_div2 0 0 0
499999992 0 0 50000
cpub_clk_axi_sel 0 0
0 499999992 0 0 50000
cpub_clk_axi 0 0
0 499999992 0 0 50000
cpub_clk_div16_en 0 0
0 999999985 0 0 50000
cpub_clk_div16 0 0
0 62499999 0 0 50000
cpub_clk_dyn1_div 0 0 0
999999985 0 0 50000

Quoted 7 lines

Is this correct approach to set the flags to enable *cpub_clk*. .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
What I meant is their *Dyn_enable[26]* field for enable/disable for HHI_SYS_CPU_CLK_CNTL0 and HHI_SYS_CPUB_CLK_CNTL clk controller. in the S922X datasheets which could help resolve this issue. Any thought on this.

I sorry but I'm just lost. I don't understand anything above so I can't comment.

Quoted 2 lines

-Anand

Anand Moon

author

2020-02-24 10:17 UTC #11 lore raw

Re: [PATCHv1 3/3] clk: meson: g12a: set cpu clock divider flags too CLK_IS_CRITICAL

Hi Jerome,

Quoted 100 lines · Jerome Brunet

Quoted 26 lines · Anand Moon
Hi Martin / Jerome / Neil,
Quoted 22 lines · Martin Blumenstingl
Hi Anand,
On Mon, Feb 17, 2020 at 2:30 PM Anand Moon <[email protected]> wrote:
[...]
Quoted 11 lines
Quoted 11 lines
drivers/clk/meson/g12a.c static struct clk_regmap g12b_cpub_clk = { · line 685
Quoted 6 lines
drivers/clk/meson/g12a.c +1 -1
@@ -681,7 +682,7 @@ static struct clk_regmap g12b_cpub_clk = {
                      &g12a_sys_pll.hw
              },
              .num_parents = 2,
-             .flags = CLK_SET_RATE_PARENT,
+             .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
Why not. Neil what do you think of this ?
If nothing is claiming this clock and enabling it then I suppose it could make sense.
Quoted 2 lines
I would like core developers to handle this.
Sorry for the noise.
can you please resend this patch with only the change to g12b_cpub_clk? I have no G12B board myself so it would be great if you could take care of this!
Quoted 6 lines
Quoted 1 line
Martin
Thanks, yes I will try again, but I have a question.
On eMMC module *cpub_clk* is not getting enabled, see below is clk_summay of eMMC.
I'm sorry but I don't understand the link between the cpu clock of the second cluster and MMC.
Quoted 51 lines
[...]
          fclk_div2_div               1        1        0   999999985
        0     0  50000
             fclk_div2                2        2        0   999999985
        0     0  50000
                ff3f0000.ethernet#m250_sel       1        1        0
999999985          0     0  50000
                   ff3f0000.ethernet#m250_div       1        1
0   249999997          0     0  50000
                      ff3f0000.ethernet#fixed_div2       1        1
    0   124999998          0     0  50000
                         ff3f0000.ethernet#rgmii_tx_en       1
1        0   124999998          0     0  50000
                ffe07000.mmc#mux       1        1        0   999999985
         0     0  50000
                   ffe07000.mmc#div       1        1        0
199999997          0     0  50000
                cpub_clk_dyn1_sel       0        0        0
999999985          0     0  50000
                   cpub_clk_dyn1       0        0        0   999999985
         0     0  50000
                      cpub_clk_dyn       0        0        0
999999985 0 0 50000
cpub_clk 0 0 0
999999985 0 0 50000
cpub_clk_div8 0 0 0
124999998 0 0 50000
cpub_clk_div7 0 0 0
142857140 0 0 50000
cpub_clk_div6 0 0 0
166666664 0 0 50000
cpub_clk_trace_sel 0 0
0 166666664 0 0 50000
cpub_clk_trace 0 0
0 166666664 0 0 50000
cpub_clk_div5 0 0 0
199999997 0 0 50000
cpub_clk_apb_sel 0 0
0 199999997 0 0 50000
cpub_clk_apb 0 0
0 199999997 0 0 50000
cpub_clk_div4 0 0 0
249999996 0 0 50000
cpub_clk_div3 0 0 0
333333328 0 0 50000
cpub_clk_atb_sel 0 0
0 333333328 0 0 50000
cpub_clk_atb 0 0
0 333333328 0 0 50000
cpub_clk_div2 0 0 0
499999992 0 0 50000
Quoted 8 lines
cpub_clk_axi_sel 0 0
0 499999992 0 0 50000
cpub_clk_axi 0 0
0 499999992 0 0 50000
cpub_clk_div16_en 0 0
0 999999985 0 0 50000
cpub_clk_div16 0 0
0 62499999 0 0 50000

Quoted 9 lines

I can't read that.
Quoted 4 lines
After enable *cpub_clk* flags with
.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, this clk is enabled on microSD card see clk_summary below.
Again, I don't get the relationship between cpub and sdcard (or eMMC)

Yes their is not relation with the cpub and sdcard and eMMC, I understood that cpub_clk is not getting enable which is causing the staling at booting using sdcard.

sorry about this logs.

Quoted 65 lines

Quoted 65 lines
[...]
         fclk_div2_div               1        1        0   999999985
       0     0  50000
             fclk_div2                3        3        0   999999985
        0     0  50000
                ff3f0000.ethernet#m250_sel       1        1        0
999999985          0     0  50000
                   ff3f0000.ethernet#m250_div       1        1
0   249999997          0     0  50000
                      ff3f0000.ethernet#fixed_div2       1        1
    0   124999998          0     0  50000
                         ff3f0000.ethernet#rgmii_tx_en       1
1        0   124999998          0     0  50000
                ffe05000.sd#mux       1        1        0   999999985
        0     0  50000
                   ffe05000.sd#div       1        1        0
50000000          0     0  50000
                cpub_clk_dyn1_sel       1        1        0
999999985          0     0  50000
                   cpub_clk_dyn1       1        1        0   999999985
         0     0  50000
                      cpub_clk_dyn       1        1        0
999999985 0 0 50000
cpub_clk 1 1 0
999999985 0 0 50000
cpub_clk_div8 0 0 0
124999998 0 0 50000
cpub_clk_div7 0 0 0
142857140 0 0 50000
cpub_clk_div6 0 0 0
166666664 0 0 50000
cpub_clk_trace_sel 0 0
0 166666664 0 0 50000
cpub_clk_trace 0 0
0 166666664 0 0 50000
cpub_clk_div5 0 0 0
199999997 0 0 50000
cpub_clk_apb_sel 0 0
0 199999997 0 0 50000
cpub_clk_apb 0 0
0 199999997 0 0 50000
cpub_clk_div4 0 0 0
249999996 0 0 50000
cpub_clk_div3 0 0 0
333333328 0 0 50000
cpub_clk_atb_sel 0 0
0 333333328 0 0 50000
cpub_clk_atb 0 0
0 333333328 0 0 50000
cpub_clk_div2 0 0 0
499999992 0 0 50000
cpub_clk_axi_sel 0 0
0 499999992 0 0 50000
cpub_clk_axi 0 0
0 499999992 0 0 50000
cpub_clk_div16_en 0 0
0 999999985 0 0 50000
cpub_clk_div16 0 0
0 62499999 0 0 50000
cpub_clk_dyn1_div 0 0 0
999999985 0 0 50000
Is this correct approach to set the flags to enable *cpub_clk*. .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,

Quoted 7 lines

Quoted 4 lines
What I meant is their *Dyn_enable[26]* field for enable/disable for HHI_SYS_CPU_CLK_CNTL0 and HHI_SYS_CPUB_CLK_CNTL clk controller. in the S922X datasheets which could help resolve this issue. Any thought on this.
I sorry but I'm just lost. I don't understand anything above so I can't comment.

I am not able to express my self clearly,
I will try to submit the patch by enable cpub_clk with following flags.

.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,

-Anand