linux-amlogic.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Anand Moon <linux.amoon@gmail.com>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: devicetree <devicetree@vger.kernel.org>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Kevin Hilman <khilman@baylibre.com>,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	linux-amlogic@lists.infradead.org,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	Jerome Brunet <jbrunet@baylibre.com>
Subject: Re: [RFCv1 5/5] arm64/ARM: configs: Change CONFIG_PWM_MESON from m to y
Date: Mon, 21 Oct 2019 19:41:34 +0530	[thread overview]
Message-ID: <CANAwSgRs2DUXwvhJD5qpXg04qEdP_Nt-wQqRbD2FpY2SWnHpAA@mail.gmail.com> (raw)
In-Reply-To: <CAFBinCBdwqxA2kLMAA9gtOcXevYK-J4x12odHwpQOAWakgWiEg@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3766 bytes --]

Hi Martin,

On Fri, 18 Oct 2019 at 23:40, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> Hi Anand,
>
> On Fri, Oct 18, 2019 at 4:04 PM Anand Moon <linux.amoon@gmail.com> wrote:
> [...]
> > > Next step it to try narrow down the clock causing the issue.
> > > Remove clk_ignore_unused from the command line and add CLK_INGORE_UNUSED
> > > to the flag of some clocks your clock controller (g12a I think) until
> > >
> > > The peripheral clock gates already have this flag (something we should
> > > fix someday) so don't bother looking there.
> > >
> > > Most likely the source of the pwm is getting disabled between the
> > > late_init call and the probe of the PWM module. Since the pwm is already
> > > active (w/o a driver), gating the clock source shuts dowm the power to
> > > the cores.
> > >
> > > Looking a the possible inputs in pwm driver, I'd bet on fdiv4.
> > >
> >
> > I had give this above steps a try but with little success.
> > I am still looking into this much close.
> it's not clear to me if you have only tested with the PWM and/or
> FCLK_DIV4 clocks. can you please describe what you have tested so far?
>
Sorry for delayed response.

I had just looked into clk related to SD_EMMC_A/B/C,
with adding CLK_IGNORE/CRITICAL.
Also looked into clk_summary for eMMC and microSD card,
to identify the root cause, but I failed to move ahead.

> for reference - my way of debugging this in the past was:
> 1. add some printks to clk_disable_unused_subtree (right after the
> clk_core_is_enabled check) to see which clocks are being disabled
> 2. add CLK_IGNORE_UNUSED or CLK_IS_CRITICAL to the clocks which are
> being disabled based on the information from step #1
> 3. (at some point I had a working kernel with lots of clocks with
> CLK_IGNORE_UNUSED/CLK_IS_CRITICAL)
> 4. start dropping the CLK_IGNORE_UNUSED/CLK_IS_CRITICAL flags again
> until you have traced it down to the clocks that are the actual issue
> (so far I always had only one clock which caused issues, but it may be
> multiple)
> 5. investigate (and/or ask on the mailing list, Amlogic developers are
> reading the mails here as well) for the few clocks from step #4
>

Thanks for you valuable suggestion. I have your patch to debug this
[0]  https://patchwork.kernel.org/patch/9725921/mbox/

So from the fist step I could identify that all the clk were getting closed
after some core cpu clk was failing. Here is the log.

step1: [1] https://pastebin.com/p13F9HGG

so I marked these clk as CLK_IGNORE_UNUSED and finally
I made it to boot using microSD card.

After this just I converted these CLK to CLK_IS_CRITICAL
as mostly these are used the CPU clk for now.
Here is boot log successful for as of now.

Finally: [2]  https://pastebin.com/qB6pMyGQ

I know clk maintainer are against marking flags as *CLK_IS_CRITICAL*
But this is just the step to move ahead.

Attach is my local clk and dts patch.Just for testing.
[3] clk_critical.patch

Plz share your thought on this.

> > Well I am not the expert in clk or bus configuration.
> > but after looking into the datasheet of for clk configuration
> > I found some bus are not configured correctly.
> did you find any reason which indicates that the problem is related to a bus?
> the issues I had were due to clocks not being assigned to their
> consumers in .dts - that can be anything (from a bus to something
> different).
>

Yes I feel each core bus should be independent
as each clk PLL controls these bus.

for example datasheet: *6-5 Clock Connections*

What I feel currently missing with bus are
clock gating (enable/disable of features).
clock-controller
reset-controller

Here is the current overview of bus topology
using latest u-boot (dm tree).

[4] https://pastebin.com/MZ25bgiP

Bet Regards
-Anand

[-- Attachment #2: clk_critical.patch --]
[-- Type: application/octet-stream, Size: 5614 bytes --]

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts
index 42f15405750c..4f8d89f472a2 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dts
@@ -54,6 +54,9 @@
 		gpio = <&gpio_ao GPIOAO_8 GPIO_ACTIVE_HIGH>;
 		enable-active-high;
 		regulator-always-on;
+
+		/* FC8731-09VF05NRR */
+		vin-supply = <&vddao_3v3>;
 	};
 
 	tf_io: gpio-regulator-tf_io {
@@ -68,6 +71,8 @@
 
 		states = <3300000 0>,
 			 <1800000 1>;
+		/* RT9179GB */
+		vin-supply = <&vcc_5v>;
 	};
 
 	flash_1v8: regulator-flash_1v8 {
@@ -429,7 +434,6 @@
 	cd-gpios = <&gpio GPIOC_6 GPIO_ACTIVE_LOW>;
 	vmmc-supply = <&tflash_vdd>;
 	vqmmc-supply = <&tf_io>;
-
 };
 
 /* eMMC */
diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
index b3af61cc6fb9..81c6e33621df 100644
--- a/drivers/clk/meson/g12a.c
+++ b/drivers/clk/meson/g12a.c
@@ -283,6 +283,8 @@ 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,
+		/* Anand */
+		.flags = CLK_IS_CRITICAL,
 	},
 };
 
@@ -298,6 +300,8 @@ static struct clk_regmap g12a_fclk_div2 = {
 			&g12a_fclk_div2_div.hw
 		},
 		.num_parents = 1,
+		/* Anand */
+		.flags = CLK_IS_CRITICAL,
 	},
 };
 
@@ -375,7 +379,7 @@ static struct clk_regmap g12a_cpu_clk_premux1 = {
 		},
 		.num_parents = 3,
 		/* This sub-tree is used a parking clock */
-		.flags = CLK_SET_RATE_NO_REPARENT
+		.flags = CLK_SET_RATE_NO_REPARENT,
 	},
 };
 
@@ -604,7 +608,7 @@ static struct clk_regmap g12b_cpub_clk_premux1 = {
 		},
 		.num_parents = 3,
 		/* This sub-tree is used a parking clock */
-		.flags = CLK_SET_RATE_NO_REPARENT,
+		.flags = CLK_SET_RATE_NO_REPARENT | CLK_IS_CRITICAL,
 	},
 };
 
@@ -622,6 +626,8 @@ static struct clk_regmap g12b_cpub_clk_mux1_div = {
 			&g12b_cpub_clk_premux1.hw
 		},
 		.num_parents = 1,
+		/* Anand */
+		.flags = CLK_IS_CRITICAL,
 	},
 };
 
@@ -641,7 +647,8 @@ static struct clk_regmap g12b_cpub_clk_postmux1 = {
 		},
 		.num_parents = 2,
 		/* This sub-tree is used a parking clock */
-		.flags = CLK_SET_RATE_NO_REPARENT,
+		/* Anand */
+		.flags = CLK_SET_RATE_NO_REPARENT | CLK_IS_CRITICAL,
 	},
 };
 
@@ -681,7 +688,8 @@ static struct clk_regmap g12b_cpub_clk = {
 			&g12a_sys_pll.hw
 		},
 		.num_parents = 2,
-		.flags = CLK_SET_RATE_PARENT,
+		/* Anand */
+		.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
 	},
 };
 
@@ -1151,6 +1159,8 @@ static struct clk_regmap g12b_cpub_clk_div16_en = {
 		 * This clock is used to debug the cpu_clk range
 		 * Linux should not change it at runtime
 		 */
+		/* Anand */
+		.flags = CLK_IGNORE_UNUSED,
 	},
 };
 
@@ -1164,6 +1174,8 @@ static struct clk_fixed_factor g12a_cpu_clk_div16 = {
 			&g12a_cpu_clk_div16_en.hw
 		},
 		.num_parents = 1,
+		/* Anand */
+		.flags = CLK_IS_CRITICAL,
 	},
 };
 
@@ -1177,6 +1189,8 @@ static struct clk_fixed_factor g12b_cpub_clk_div16 = {
 			&g12b_cpub_clk_div16_en.hw
 		},
 		.num_parents = 1,
+		/* Anand */
+		.flags = CLK_IS_CRITICAL,
 	},
 };
 
@@ -1336,6 +1350,8 @@ static struct clk_fixed_factor g12b_cpub_clk_div2 = {
 			&g12b_cpub_clk.hw
 		},
 		.num_parents = 1,
+		/* Anand */
+		.flags = CLK_IS_CRITICAL,
 	},
 };
 
@@ -1349,6 +1365,8 @@ static struct clk_fixed_factor g12b_cpub_clk_div3 = {
 			&g12b_cpub_clk.hw
 		},
 		.num_parents = 1,
+		/* Anand */
+		.flags = CLK_IS_CRITICAL,
 	},
 };
 
@@ -1362,6 +1380,8 @@ static struct clk_fixed_factor g12b_cpub_clk_div4 = {
 			&g12b_cpub_clk.hw
 		},
 		.num_parents = 1,
+		/* Anand */
+		.flags = CLK_IS_CRITICAL,
 	},
 };
 
@@ -1375,6 +1395,8 @@ static struct clk_fixed_factor g12b_cpub_clk_div5 = {
 			&g12b_cpub_clk.hw
 		},
 		.num_parents = 1,
+		/* Anand */
+		.flags = CLK_IS_CRITICAL,
 	},
 };
 
@@ -1388,6 +1410,8 @@ static struct clk_fixed_factor g12b_cpub_clk_div6 = {
 			&g12b_cpub_clk.hw
 		},
 		.num_parents = 1,
+		/* Anand */
+		.flags = CLK_IS_CRITICAL,
 	},
 };
 
@@ -1401,6 +1425,8 @@ static struct clk_fixed_factor g12b_cpub_clk_div7 = {
 			&g12b_cpub_clk.hw
 		},
 		.num_parents = 1,
+		/* Anand */
+		.flags = CLK_IS_CRITICAL,
 	},
 };
 
@@ -1414,6 +1440,8 @@ static struct clk_fixed_factor g12b_cpub_clk_div8 = {
 			&g12b_cpub_clk.hw
 		},
 		.num_parents = 1,
+		/* Anand */
+		.flags = CLK_IS_CRITICAL,
 	},
 };
 
@@ -1438,6 +1466,8 @@ static struct clk_regmap g12b_cpub_clk_apb_sel = {
 			&g12b_cpub_clk_div8.hw
 		},
 		.num_parents = 7,
+		/* Anand */
+		.flags = CLK_IS_CRITICAL,
 	},
 };
 
@@ -1458,6 +1488,8 @@ static struct clk_regmap g12b_cpub_clk_apb = {
 		 * This clock is set by the ROM monitor code,
 		 * Linux should not change it at runtime
 		 */
+		/* Anand */
+		.flags = CLK_IS_CRITICAL,
 	},
 };
 
@@ -1481,6 +1513,8 @@ static struct clk_regmap g12b_cpub_clk_atb_sel = {
 			&g12b_cpub_clk_div8.hw
 		},
 		.num_parents = 7,
+		/* Anand */
+		.flags = CLK_IS_CRITICAL,
 	},
 };
 
@@ -1501,6 +1535,8 @@ static struct clk_regmap g12b_cpub_clk_atb = {
 		 * This clock is set by the ROM monitor code,
 		 * Linux should not change it at runtime
 		 */
+		/* Anand */
+		.flags = CLK_IS_CRITICAL,
 	},
 };
 
@@ -1524,6 +1560,8 @@ static struct clk_regmap g12b_cpub_clk_axi_sel = {
 			&g12b_cpub_clk_div8.hw
 		},
 		.num_parents = 7,
+		/* Anand */
+		.flags = CLK_IS_CRITICAL,
 	},
 };
 
@@ -1544,6 +1582,8 @@ static struct clk_regmap g12b_cpub_clk_axi = {
 		 * This clock is set by the ROM monitor code,
 		 * Linux should not change it at runtime
 		 */
+		/* Anand */
+		.flags = CLK_IS_CRITICAL,
 	},
 };
 

[-- Attachment #3: Type: text/plain, Size: 167 bytes --]

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

  reply	other threads:[~2019-10-21 14:12 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-07 13:16 [RFCv1 0/5] Odroid N2 failes to boot using upstream kernel & u-boot Anand Moon
2019-10-07 13:16 ` [RFCv1 1/5] arm64: dts: meson: Add missing 5V_EN gpio signal for VCC5V regulator Anand Moon
2019-10-07 14:19   ` Neil Armstrong
2019-10-07 15:28     ` Anand Moon
2019-10-07 13:16 ` [RFCv1 2/5] arm64: dts: meson: Add missing pwm control gpio signal for pwm-regulator Anand Moon
2019-10-07 14:20   ` Neil Armstrong
2019-10-07 15:30     ` Anand Moon
2019-10-07 13:16 ` [RFCv1 3/5] arm64: dts: meson: Add missing regulator linked to VDDAO_3V3 regulator to FLASH_VDD Anand Moon
2019-10-07 14:20   ` Neil Armstrong
2019-10-07 15:53     ` Anand Moon
2019-10-07 13:16 ` [RFCv1 4/5] arm64: dts: meson: Add missing regulator linked to VCCV5 regulator to VDDIO_C/TF_IO Anand Moon
2019-10-07 14:21   ` Neil Armstrong
2019-10-07 15:54     ` Anand Moon
2019-10-07 13:16 ` [RFCv1 5/5] arm64/ARM: configs: Change CONFIG_PWM_MESON from m to y Anand Moon
2019-10-07 14:25   ` Neil Armstrong
2019-10-07 15:52     ` Anand Moon
2019-10-07 20:10   ` Martin Blumenstingl
2019-10-07 22:57     ` Kevin Hilman
2019-10-08 14:38       ` Anand Moon
2019-10-08 17:40         ` Martin Blumenstingl
2019-10-09  8:48           ` Anand Moon
2019-10-09 12:04             ` Jerome Brunet
2019-10-18 14:04               ` Anand Moon
2019-10-18 14:13                 ` Neil Armstrong
2019-10-18 15:21                   ` Anand Moon
2019-10-18 18:10                 ` Martin Blumenstingl
2019-10-21 14:11                   ` Anand Moon [this message]
2019-10-21 14:25                     ` Neil Armstrong
2019-10-21 15:41                       ` Anand Moon
2019-10-26 18:56                         ` Anand Moon
2019-10-24 20:20                     ` Martin Blumenstingl
2019-10-09 17:05             ` Martin Blumenstingl
2019-10-08  7:19     ` Anand Moon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CANAwSgRs2DUXwvhJD5qpXg04qEdP_Nt-wQqRbD2FpY2SWnHpAA@mail.gmail.com \
    --to=linux.amoon@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jbrunet@baylibre.com \
    --cc=khilman@baylibre.com \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.blumenstingl@googlemail.com \
    --cc=narmstrong@baylibre.com \
    --subject='Re: [RFCv1 5/5] arm64/ARM: configs: Change CONFIG_PWM_MESON from m to y' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).