All of lore.kernel.org
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <k.kozlowski@samsung.com>
To: Javier Martinez Canillas <javier@osg.samsung.com>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Thomas Abraham <thomas.ab@samsung.com>,
	Sylwester Nawrocki <s.nawrocki@samsung.com>,
	Mike Turquette <mturquette@linaro.org>,
	Kukjin Kim <kgene.kim@samsung.com>, Kukjin Kim <kgene@kernel.org>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Ben Gamari <ben@smart-cactus.org>
Cc: Tomasz Figa <tomasz.figa@gmail.com>,
	Lukasz Majewski <l.majewski@samsung.com>,
	Heiko Stuebner <heiko@sntech.de>,
	Chanwoo Choi <cw00.choi@samsung.com>,
	Kevin Hilman <khilman@linaro.org>,
	Tobias Jakobi <tjakobi@math.uni-bielefeld.de>,
	Anand Moon <linux.amoon@gmail.com>,
	linux-samsung-soc@vger.kernel.org, linux-pm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 7/8] ARM: dts: Exynos5422: fix OPP tables
Date: Wed, 09 Dec 2015 09:34:09 +0900	[thread overview]
Message-ID: <56677701.10809@samsung.com> (raw)
In-Reply-To: <5666DE25.7090708@osg.samsung.com>

On 08.12.2015 22:41, Javier Martinez Canillas wrote:
> Hello Krzysztof,
> 
> On 12/08/2015 05:13 AM, Krzysztof Kozlowski wrote:
>> On 08.12.2015 03:18, Bartlomiej Zolnierkiewicz wrote:
>>> From: Ben Gamari <ben@smart-cactus.org>
>>>
>>> The Exynos 5422 is identical to the 5800 except for the fact that it
>>> boots from the A7 cores. Consequently, the core numbering is different:
>>> cores 0-3 are A7s whereas 4-7 are A15s.
>>>
>>> We can reuse the device tree of the 5800 for the 5422 but we must take
>>> care to override the OPP tables and CPU clocks.  These are otherwise
>>> inherited from the exynos5800 devicetree, which has the CPU clusters
>>> reversed compared to the 5422. This results in the A15 cores only
>>> reaching 1.4GHz, the maximum rate of the KFC clock.
>>>
>>> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
>>> Signed-off-by: Ben Gamari <ben@smart-cactus.org>
>>> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>>> ---
>>>  arch/arm/boot/dts/exynos5422-cpus.dtsi | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>
>> This looks like a very-non-atomic way of handling a change. You added
>> opp tables to exynos5420 before so at that time they will be applied to
>> Odroid XU3 family which uses different CPU order. After that you are
>> fixing the tables to proper CPU order. Direct bisectability probably
>> won't be an issue because all of DTS would go to separate branch... but
>> the logic behind confuses.
>>
> 
> Agreed.
> 
>> I think this should be squashed into 3/8.
>>
> 
> I think the patch should be split in two changes, the CPUs device nodes
> having the wrong clock for clusters is a bug and has to be fixed in a
> patch before adding the OPP tables and the OPP tables changes should be
> separated and merged with patch 3/8 as you suggest.

I don't get the point about wrong clock (bug). Where is the bug? Beside
of course what was introduced in 3/8 and it is not valid for reversed
cluster order.

Best regards,
Krzysztof


WARNING: multiple messages have this Message-ID (diff)
From: k.kozlowski@samsung.com (Krzysztof Kozlowski)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v4 7/8] ARM: dts: Exynos5422: fix OPP tables
Date: Wed, 09 Dec 2015 09:34:09 +0900	[thread overview]
Message-ID: <56677701.10809@samsung.com> (raw)
In-Reply-To: <5666DE25.7090708@osg.samsung.com>

On 08.12.2015 22:41, Javier Martinez Canillas wrote:
> Hello Krzysztof,
> 
> On 12/08/2015 05:13 AM, Krzysztof Kozlowski wrote:
>> On 08.12.2015 03:18, Bartlomiej Zolnierkiewicz wrote:
>>> From: Ben Gamari <ben@smart-cactus.org>
>>>
>>> The Exynos 5422 is identical to the 5800 except for the fact that it
>>> boots from the A7 cores. Consequently, the core numbering is different:
>>> cores 0-3 are A7s whereas 4-7 are A15s.
>>>
>>> We can reuse the device tree of the 5800 for the 5422 but we must take
>>> care to override the OPP tables and CPU clocks.  These are otherwise
>>> inherited from the exynos5800 devicetree, which has the CPU clusters
>>> reversed compared to the 5422. This results in the A15 cores only
>>> reaching 1.4GHz, the maximum rate of the KFC clock.
>>>
>>> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
>>> Signed-off-by: Ben Gamari <ben@smart-cactus.org>
>>> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>>> ---
>>>  arch/arm/boot/dts/exynos5422-cpus.dtsi | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>
>> This looks like a very-non-atomic way of handling a change. You added
>> opp tables to exynos5420 before so at that time they will be applied to
>> Odroid XU3 family which uses different CPU order. After that you are
>> fixing the tables to proper CPU order. Direct bisectability probably
>> won't be an issue because all of DTS would go to separate branch... but
>> the logic behind confuses.
>>
> 
> Agreed.
> 
>> I think this should be squashed into 3/8.
>>
> 
> I think the patch should be split in two changes, the CPUs device nodes
> having the wrong clock for clusters is a bug and has to be fixed in a
> patch before adding the OPP tables and the OPP tables changes should be
> separated and merged with patch 3/8 as you suggest.

I don't get the point about wrong clock (bug). Where is the bug? Beside
of course what was introduced in 3/8 and it is not valid for reversed
cluster order.

Best regards,
Krzysztof

  reply	other threads:[~2015-12-09  0:34 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-07 18:18 [PATCH v4 0/8] cpufreq: add generic cpufreq driver support for Exynos542x/5800 platforms Bartlomiej Zolnierkiewicz
2015-12-07 18:18 ` Bartlomiej Zolnierkiewicz
2015-12-07 18:18 ` [PATCH v4 1/8] ARM: dts: Exynos542x/5800: add cluster regulator supply properties Bartlomiej Zolnierkiewicz
2015-12-07 18:18   ` Bartlomiej Zolnierkiewicz
2015-12-08  6:48   ` Krzysztof Kozlowski
2015-12-08  6:48     ` Krzysztof Kozlowski
2015-12-08 10:16   ` Markus Reichl
2015-12-08 10:16     ` Markus Reichl
2015-12-08 10:16     ` Markus Reichl
2015-12-07 18:18 ` [PATCH v4 2/8] clk: samsung: exynos5420: add cpu clock configuration data and instantiate cpu clock Bartlomiej Zolnierkiewicz
2015-12-07 18:18   ` Bartlomiej Zolnierkiewicz
2015-12-07 18:18 ` [PATCH v4 3/8] ARM: dts: Exynos5420: add CPU OPP properties Bartlomiej Zolnierkiewicz
2015-12-07 18:18   ` Bartlomiej Zolnierkiewicz
2015-12-08  6:28   ` Viresh Kumar
2015-12-08  6:28     ` Viresh Kumar
2015-12-08  6:28     ` Viresh Kumar
2015-12-08  7:17   ` Krzysztof Kozlowski
2015-12-08  7:17     ` Krzysztof Kozlowski
2015-12-07 18:18 ` [PATCH v4 4/8] ARM: Exynos: use generic cpufreq driver for Exynos5420 Bartlomiej Zolnierkiewicz
2015-12-07 18:18   ` Bartlomiej Zolnierkiewicz
2015-12-08  7:36   ` Krzysztof Kozlowski
2015-12-08  7:36     ` Krzysztof Kozlowski
2015-12-10 14:18     ` Bartlomiej Zolnierkiewicz
2015-12-10 14:18       ` Bartlomiej Zolnierkiewicz
2015-12-11  1:13       ` Krzysztof Kozlowski
2015-12-11  1:13         ` Krzysztof Kozlowski
2015-12-07 18:18 ` [PATCH v4 5/8] clk: samsung: exynos5422/5800: fix cpu clock configuration data Bartlomiej Zolnierkiewicz
2015-12-07 18:18   ` Bartlomiej Zolnierkiewicz
2015-12-07 18:18 ` [PATCH v4 6/8] ARM: dts: Exynos5800: fix CPU OPP Bartlomiej Zolnierkiewicz
2015-12-07 18:18   ` Bartlomiej Zolnierkiewicz
2015-12-08  8:07   ` Krzysztof Kozlowski
2015-12-08  8:07     ` Krzysztof Kozlowski
2015-12-07 18:18 ` [PATCH v4 7/8] ARM: dts: Exynos5422: fix OPP tables Bartlomiej Zolnierkiewicz
2015-12-07 18:18   ` Bartlomiej Zolnierkiewicz
2015-12-08  8:13   ` Krzysztof Kozlowski
2015-12-08  8:13     ` Krzysztof Kozlowski
2015-12-08 13:41     ` Javier Martinez Canillas
2015-12-08 13:41       ` Javier Martinez Canillas
2015-12-09  0:34       ` Krzysztof Kozlowski [this message]
2015-12-09  0:34         ` Krzysztof Kozlowski
2015-12-09  2:19         ` Javier Martinez Canillas
2015-12-09  2:19           ` Javier Martinez Canillas
2015-12-10 15:57     ` Bartlomiej Zolnierkiewicz
2015-12-10 15:57       ` Bartlomiej Zolnierkiewicz
2015-12-07 18:18 ` [PATCH v4 8/8] ARM: Exynos: use generic cpufreq driver for Exynos5422/5800 Bartlomiej Zolnierkiewicz
2015-12-07 18:18   ` Bartlomiej Zolnierkiewicz
2015-12-08  6:17 ` [PATCH v4 0/8] cpufreq: add generic cpufreq driver support for Exynos542x/5800 platforms Viresh Kumar
2015-12-08  6:17   ` Viresh Kumar
2015-12-08  6:29   ` Viresh Kumar
2015-12-08  6:29     ` Viresh Kumar
2015-12-09 13:13 ` Javier Martinez Canillas
2015-12-09 13:13   ` Javier Martinez Canillas

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=56677701.10809@samsung.com \
    --to=k.kozlowski@samsung.com \
    --cc=b.zolnierkie@samsung.com \
    --cc=ben@smart-cactus.org \
    --cc=cw00.choi@samsung.com \
    --cc=heiko@sntech.de \
    --cc=javier@osg.samsung.com \
    --cc=kgene.kim@samsung.com \
    --cc=kgene@kernel.org \
    --cc=khilman@linaro.org \
    --cc=l.majewski@samsung.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=linux.amoon@gmail.com \
    --cc=mturquette@linaro.org \
    --cc=s.nawrocki@samsung.com \
    --cc=thomas.ab@samsung.com \
    --cc=tjakobi@math.uni-bielefeld.de \
    --cc=tomasz.figa@gmail.com \
    --cc=viresh.kumar@linaro.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.