From: khilman@baylibre.com (Kevin Hilman)
To: linus-amlogic@lists.infradead.org
Subject: [RFC PATCH] clk: meson: gxbb: remove the "cpu_clk"
Date: Mon, 27 Mar 2017 09:59:48 -0700 [thread overview]
Message-ID: <m2mvc6d5a3.fsf@baylibre.com> (raw)
In-Reply-To: <1490550707.29739.21.camel@baylibre.com> (Jerome Brunet's message of "Sun, 26 Mar 2017 19:51:47 +0200")
Jerome Brunet <jbrunet@baylibre.com> writes:
> On Sun, 2017-03-26 at 13:06 +0200, Martin Blumenstingl wrote:
>> It seems that the "cpu_clk" was carried over from the meson8b clock
>> controller driver. On Meson GX (GXBB/GXL/GXM) the registers which are
>> used by the cpu_clk have a different purpose (in other words: they don't
>> control the CPU clock anymore). HHI_SYS_CPU_CLK_CNTL1 bits 31:24 are
>> reserved according to the public S905 datasheet, while bit 23 is the
>> "A53_trace_clk_DIS" gate (which according to the datasheet should only
>> be used in case a silicon bug is discovered) and bits 22:20 are a
>> divider (A53_trace_clk). The meson clk-cpu code however expects that
>> bits 28:20 are reserved for a divider (according to the public S805
>> datasheet this "SCALE_DIV: This value represents an N+1 divider of the
>> input clock.").
>>
>> The CPU clock on Meson GX SoCs is provided by the SCPI DVFS clock
>> driver instead. Two examples from a Meson GXL S905X SoC:
>> - vcpu (SCPI DVFS clock 0) rate: 1000000000 / cpu_clk rate: 708000000
>> - vcpu (SCPI DVFS clock 0) rate: 1512000000 / cpu_clk rate: 708000000
>>
>
> Thanks for catching this Martin! Looking at the difference between the S805 and
> S905 register description, it makes no sense that we used the same clock driver
> for what's behind HHI_SYS_CPU_CLK_CNTL1. I agree, it should be removed from the
> gxbb clock controller.
>
>> Unfortunately the CLKID_CPUCLK was already exported (but is currently
>> not used) to DT.
>>
>> This is an RFC because I would like to hear other people's opinion on
>> how to clean up the CLKID_CPUCLK (as it leaves a "hole" in
>> gxbb_hw_onecell_data and changes the DT API (by removing a currently
>> unused #define).
>>
>
> In general the DT ABI should be stable but since no one is using this ID, and
> what it refers to is clearly wrong, I don't think we would disrupt anything by
> removing it.
>
> For the "hole" in the ids, Neil and I had similar problem while working other
> topics. We managed to dodge it but I think it was only a matter of time ...
>
> In the end, it is not a big deal if there is holes in the onecell_data, as long
> as we take care to avoid it when we register the clocks:
Agreed. It's better to have holes in the clkid space than to introduce
DT incompatibility.
Kevin
WARNING: multiple messages have this Message-ID (diff)
From: Kevin Hilman <khilman@baylibre.com>
To: Jerome Brunet <jbrunet@baylibre.com>
Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
linux-amlogic@lists.infradead.org, linux-clk@vger.kernel.org,
mturquette@baylibre.com, linux-arm-kernel@lists.infradead.org,
narmstrong@baylibre.com, carlo@caione.org,
sboyd@codeaurora.org
Subject: Re: [RFC PATCH] clk: meson: gxbb: remove the "cpu_clk"
Date: Mon, 27 Mar 2017 09:59:48 -0700 [thread overview]
Message-ID: <m2mvc6d5a3.fsf@baylibre.com> (raw)
In-Reply-To: <1490550707.29739.21.camel@baylibre.com> (Jerome Brunet's message of "Sun, 26 Mar 2017 19:51:47 +0200")
Jerome Brunet <jbrunet@baylibre.com> writes:
> On Sun, 2017-03-26 at 13:06 +0200, Martin Blumenstingl wrote:
>> It seems that the "cpu_clk" was carried over from the meson8b clock
>> controller driver. On Meson GX (GXBB/GXL/GXM) the registers which are
>> used by the cpu_clk have a different purpose (in other words: they don't
>> control the CPU clock anymore). HHI_SYS_CPU_CLK_CNTL1 bits 31:24 are
>> reserved according to the public S905 datasheet, while bit 23 is the
>> "A53_trace_clk_DIS" gate (which according to the datasheet should only
>> be used in case a silicon bug is discovered) and bits 22:20 are a
>> divider (A53_trace_clk). The meson clk-cpu code however expects that
>> bits 28:20 are reserved for a divider (according to the public S805
>> datasheet this "SCALE_DIV: This value represents an N+1 divider of the
>> input clock.").
>>
>> The CPU clock on Meson GX SoCs is provided by the SCPI DVFS clock
>> driver instead. Two examples from a Meson GXL S905X SoC:
>> - vcpu (SCPI DVFS clock 0) rate: 1000000000 / cpu_clk rate: 708000000
>> - vcpu (SCPI DVFS clock 0) rate: 1512000000 / cpu_clk rate: 708000000
>>
>
> Thanks for catching this Martin! Looking at the difference between the S805 and
> S905 register description, it makes no sense that we used the same clock driver
> for what's behind HHI_SYS_CPU_CLK_CNTL1. I agree, it should be removed from the
> gxbb clock controller.
>
>> Unfortunately the CLKID_CPUCLK was already exported (but is currently
>> not used) to DT.
>>
>> This is an RFC because I would like to hear other people's opinion on
>> how to clean up the CLKID_CPUCLK (as it leaves a "hole" in
>> gxbb_hw_onecell_data and changes the DT API (by removing a currently
>> unused #define).
>>
>
> In general the DT ABI should be stable but since no one is using this ID, and
> what it refers to is clearly wrong, I don't think we would disrupt anything by
> removing it.
>
> For the "hole" in the ids, Neil and I had similar problem while working other
> topics. We managed to dodge it but I think it was only a matter of time ...
>
> In the end, it is not a big deal if there is holes in the onecell_data, as long
> as we take care to avoid it when we register the clocks:
Agreed. It's better to have holes in the clkid space than to introduce
DT incompatibility.
Kevin
WARNING: multiple messages have this Message-ID (diff)
From: khilman@baylibre.com (Kevin Hilman)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH] clk: meson: gxbb: remove the "cpu_clk"
Date: Mon, 27 Mar 2017 09:59:48 -0700 [thread overview]
Message-ID: <m2mvc6d5a3.fsf@baylibre.com> (raw)
In-Reply-To: <1490550707.29739.21.camel@baylibre.com> (Jerome Brunet's message of "Sun, 26 Mar 2017 19:51:47 +0200")
Jerome Brunet <jbrunet@baylibre.com> writes:
> On Sun, 2017-03-26 at 13:06 +0200, Martin Blumenstingl wrote:
>> It seems that the "cpu_clk" was carried over from the meson8b clock
>> controller driver. On Meson GX (GXBB/GXL/GXM) the registers which are
>> used by the cpu_clk have a different purpose (in other words: they don't
>> control the CPU clock anymore). HHI_SYS_CPU_CLK_CNTL1 bits 31:24 are
>> reserved according to the public S905 datasheet, while bit 23 is the
>> "A53_trace_clk_DIS" gate (which according to the datasheet should only
>> be used in case a silicon bug is discovered) and bits 22:20 are a
>> divider (A53_trace_clk). The meson clk-cpu code however expects that
>> bits 28:20 are reserved for a divider (according to the public S805
>> datasheet this "SCALE_DIV: This value represents an N+1 divider of the
>> input clock.").
>>
>> The CPU clock on Meson GX SoCs is provided by the SCPI DVFS clock
>> driver instead. Two examples from a Meson GXL S905X SoC:
>> - vcpu (SCPI DVFS clock 0) rate: 1000000000 / cpu_clk rate: 708000000
>> - vcpu (SCPI DVFS clock 0) rate: 1512000000 / cpu_clk rate: 708000000
>>
>
> Thanks for catching this Martin! Looking at the difference between the S805 and
> S905 register description, it makes no sense that we used the same clock driver
> for what's behind HHI_SYS_CPU_CLK_CNTL1. I agree, it should be removed from the
> gxbb clock controller.
>
>> Unfortunately the CLKID_CPUCLK was already exported (but is currently
>> not used) to DT.
>>
>> This is an RFC because I would like to hear other people's opinion on
>> how to clean up the CLKID_CPUCLK (as it leaves a "hole" in
>> gxbb_hw_onecell_data and changes the DT API (by removing a currently
>> unused #define).
>>
>
> In general the DT ABI should be stable but since no one is using this ID, and
> what it refers to is clearly wrong, I don't think we would disrupt anything by
> removing it.
>
> For the "hole" in the ids, Neil and I had similar problem while working other
> topics. We managed to dodge it but I think it was only a matter of time ...
>
> In the end, it is not a big deal if there is holes in the onecell_data, as long
> as we take care to avoid it when we register the clocks:
Agreed. It's better to have holes in the clkid space than to introduce
DT incompatibility.
Kevin
next prev parent reply other threads:[~2017-03-27 16:59 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-26 11:06 [RFC PATCH] clk: meson: gxbb: remove the "cpu_clk" Martin Blumenstingl
2017-03-26 11:06 ` Martin Blumenstingl
2017-03-26 11:06 ` Martin Blumenstingl
2017-03-26 17:51 ` Jerome Brunet
2017-03-26 17:51 ` Jerome Brunet
2017-03-26 17:51 ` Jerome Brunet
2017-03-27 16:59 ` Kevin Hilman [this message]
2017-03-27 16:59 ` Kevin Hilman
2017-03-27 16:59 ` Kevin Hilman
2017-03-28 21:30 ` Martin Blumenstingl
2017-03-28 21:30 ` Martin Blumenstingl
2017-03-28 21:30 ` Martin Blumenstingl
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=m2mvc6d5a3.fsf@baylibre.com \
--to=khilman@baylibre.com \
--cc=linus-amlogic@lists.infradead.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.