From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
To: Kevin Hilman <khilman@baylibre.com>
Cc: Anand Moon <linux.amoon@gmail.com>,
Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Neil Armstrong <narmstrong@baylibre.com>,
Jerome Brunet <jbrunet@baylibre.com>,
Michael Turquette <mturquette@baylibre.com>,
Stephen Boyd <sboyd@kernel.org>,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org,
linux-clk@vger.kernel.org
Subject: Re: [PATCHv2 2/2] clk: meson: g12a: set cpub_clk flags to CLK_IS_CRITICAL
Date: Fri, 20 Mar 2020 00:39:17 +0100 [thread overview]
Message-ID: <CAFBinCB2WXZNRg4wdFD0RJ5k4hHqcfAOCHemvHzZE42-Mo5vzA@mail.gmail.com> (raw)
In-Reply-To: <7hlfoir8rj.fsf@baylibre.com>
Hi Kevin,
On Mon, Mar 2, 2020 at 6:01 PM Kevin Hilman <khilman@baylibre.com> wrote:
[...]
> > updating flags to CLK_IS_CRITICAL which help enable all the parent for
> > cpub_clk.
>
> With current mainline, I've tested DVFS using CPUfreq on both clusters
> on odroid-n2, and both clusters are booting, so I don't understand the
> need for this patch.
I *think* there is a race condition at kernel boot between cpufreq and
disabling orphaned clocks
I'm not sure I fully understand it though and I don't have any G12B
board to verify it
my understanding is that u-boot runs Linux off CPU0 which is clocked by cpub_clk
this means we need to keep cpub_clk enabled as long as Linux wants the
CPU0 processor to be enabled (on 32-bit ARM platforms that would be
smp_operations.cpu_{kill,die})
cpufreq does not call clk_prepare_enable on the CPU clocks so this
means that the orphaned clock cleanup mechanism can disable it "at any
time", killing everything running on CPU0 and CPU1 (which are both
clocked by cpub_clk)
I have no explanation why this depends on booting from SD or eMMC.
for the 32-bit SoCs we have CLK_IS_CRITICAL on the CPU clock as well
since commit 0dad1ec65bc30a
on G12A we have CLK_IS_CRITICAL on the sys_pll clocks, however my
understanding is that cpub_clk could also be fed by one of the
fixed_pll derived clocks (which have a gate as well, which may or may
not be turned off by the orphaned clock cleanup - that is pure
speculation from my side though).
Martin
WARNING: multiple messages have this Message-ID (diff)
From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
To: Kevin Hilman <khilman@baylibre.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
devicetree@vger.kernel.org,
Neil Armstrong <narmstrong@baylibre.com>,
Anand Moon <linux.amoon@gmail.com>,
Michael Turquette <mturquette@baylibre.com>,
linux-kernel@vger.kernel.org, Stephen Boyd <sboyd@kernel.org>,
Rob Herring <robh+dt@kernel.org>,
linux-amlogic@lists.infradead.org, linux-clk@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
Jerome Brunet <jbrunet@baylibre.com>
Subject: Re: [PATCHv2 2/2] clk: meson: g12a: set cpub_clk flags to CLK_IS_CRITICAL
Date: Fri, 20 Mar 2020 00:39:17 +0100 [thread overview]
Message-ID: <CAFBinCB2WXZNRg4wdFD0RJ5k4hHqcfAOCHemvHzZE42-Mo5vzA@mail.gmail.com> (raw)
In-Reply-To: <7hlfoir8rj.fsf@baylibre.com>
Hi Kevin,
On Mon, Mar 2, 2020 at 6:01 PM Kevin Hilman <khilman@baylibre.com> wrote:
[...]
> > updating flags to CLK_IS_CRITICAL which help enable all the parent for
> > cpub_clk.
>
> With current mainline, I've tested DVFS using CPUfreq on both clusters
> on odroid-n2, and both clusters are booting, so I don't understand the
> need for this patch.
I *think* there is a race condition at kernel boot between cpufreq and
disabling orphaned clocks
I'm not sure I fully understand it though and I don't have any G12B
board to verify it
my understanding is that u-boot runs Linux off CPU0 which is clocked by cpub_clk
this means we need to keep cpub_clk enabled as long as Linux wants the
CPU0 processor to be enabled (on 32-bit ARM platforms that would be
smp_operations.cpu_{kill,die})
cpufreq does not call clk_prepare_enable on the CPU clocks so this
means that the orphaned clock cleanup mechanism can disable it "at any
time", killing everything running on CPU0 and CPU1 (which are both
clocked by cpub_clk)
I have no explanation why this depends on booting from SD or eMMC.
for the 32-bit SoCs we have CLK_IS_CRITICAL on the CPU clock as well
since commit 0dad1ec65bc30a
on G12A we have CLK_IS_CRITICAL on the sys_pll clocks, however my
understanding is that cpub_clk could also be fed by one of the
fixed_pll derived clocks (which have a gate as well, which may or may
not be turned off by the orphaned clock cleanup - that is pure
speculation from my side though).
Martin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
To: Kevin Hilman <khilman@baylibre.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
devicetree@vger.kernel.org,
Neil Armstrong <narmstrong@baylibre.com>,
Anand Moon <linux.amoon@gmail.com>,
Michael Turquette <mturquette@baylibre.com>,
linux-kernel@vger.kernel.org, Stephen Boyd <sboyd@kernel.org>,
Rob Herring <robh+dt@kernel.org>,
linux-amlogic@lists.infradead.org, linux-clk@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
Jerome Brunet <jbrunet@baylibre.com>
Subject: Re: [PATCHv2 2/2] clk: meson: g12a: set cpub_clk flags to CLK_IS_CRITICAL
Date: Fri, 20 Mar 2020 00:39:17 +0100 [thread overview]
Message-ID: <CAFBinCB2WXZNRg4wdFD0RJ5k4hHqcfAOCHemvHzZE42-Mo5vzA@mail.gmail.com> (raw)
In-Reply-To: <7hlfoir8rj.fsf@baylibre.com>
Hi Kevin,
On Mon, Mar 2, 2020 at 6:01 PM Kevin Hilman <khilman@baylibre.com> wrote:
[...]
> > updating flags to CLK_IS_CRITICAL which help enable all the parent for
> > cpub_clk.
>
> With current mainline, I've tested DVFS using CPUfreq on both clusters
> on odroid-n2, and both clusters are booting, so I don't understand the
> need for this patch.
I *think* there is a race condition at kernel boot between cpufreq and
disabling orphaned clocks
I'm not sure I fully understand it though and I don't have any G12B
board to verify it
my understanding is that u-boot runs Linux off CPU0 which is clocked by cpub_clk
this means we need to keep cpub_clk enabled as long as Linux wants the
CPU0 processor to be enabled (on 32-bit ARM platforms that would be
smp_operations.cpu_{kill,die})
cpufreq does not call clk_prepare_enable on the CPU clocks so this
means that the orphaned clock cleanup mechanism can disable it "at any
time", killing everything running on CPU0 and CPU1 (which are both
clocked by cpub_clk)
I have no explanation why this depends on booting from SD or eMMC.
for the 32-bit SoCs we have CLK_IS_CRITICAL on the CPU clock as well
since commit 0dad1ec65bc30a
on G12A we have CLK_IS_CRITICAL on the sys_pll clocks, however my
understanding is that cpub_clk could also be fed by one of the
fixed_pll derived clocks (which have a gate as well, which may or may
not be turned off by the orphaned clock cleanup - that is pure
speculation from my side though).
Martin
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
next prev parent reply other threads:[~2020-03-19 23:39 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-02 12:53 [PATCHv2 0/2] Odroid N2 failes to boot using upstream kernel using microSD card Anand Moon
2020-03-02 12:53 ` Anand Moon
2020-03-02 12:53 ` Anand Moon
2020-03-02 12:53 ` [PATCHv2 1/2] arm64: dts: meson: Add missing regulator linked to VDDAO_3V3 regulator to FLASH_VDD Anand Moon
2020-03-02 12:53 ` Anand Moon
2020-03-02 12:53 ` Anand Moon
2020-03-02 17:18 ` Kevin Hilman
2020-03-02 17:18 ` Kevin Hilman
2020-03-02 17:18 ` Kevin Hilman
2020-03-03 14:11 ` Anand Moon
2020-03-03 14:11 ` Anand Moon
2020-03-03 14:11 ` Anand Moon
2020-03-02 12:53 ` [PATCHv2 2/2] clk: meson: g12a: set cpub_clk flags to CLK_IS_CRITICAL Anand Moon
2020-03-02 12:53 ` Anand Moon
2020-03-02 12:53 ` Anand Moon
2020-03-02 17:01 ` Kevin Hilman
2020-03-02 17:01 ` Kevin Hilman
2020-03-02 17:01 ` Kevin Hilman
2020-03-03 14:01 ` Anand Moon
2020-03-03 14:01 ` Anand Moon
2020-03-19 23:39 ` Martin Blumenstingl [this message]
2020-03-19 23:39 ` Martin Blumenstingl
2020-03-19 23:39 ` Martin Blumenstingl
2020-03-20 6:42 ` Anand Moon
2020-03-20 6:42 ` Anand Moon
2020-03-20 6:42 ` Anand Moon
2020-03-23 9:03 ` Jerome Brunet
2020-03-23 9:03 ` Jerome Brunet
2020-03-23 9:03 ` Jerome Brunet
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=CAFBinCB2WXZNRg4wdFD0RJ5k4hHqcfAOCHemvHzZE42-Mo5vzA@mail.gmail.com \
--to=martin.blumenstingl@googlemail.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-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux.amoon@gmail.com \
--cc=mark.rutland@arm.com \
--cc=mturquette@baylibre.com \
--cc=narmstrong@baylibre.com \
--cc=robh+dt@kernel.org \
--cc=sboyd@kernel.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.