| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAEzqOZvHbtK+CxfDFGz1b5isQz1wXWVJ3hQpqQNj1mDwz5Burg@mail.gmail.com>
Date: Mon, 11 May 2015 14:22:15 +0530
From: Vishnu Patekar <vishnupatekar0510@...il.com>
To: Maxime Ripard <maxime.ripard@...e-electrons.com>
Cc: emilio@...pez.com.ar, Linus Walleij <linus.walleij@...aro.org>,
"robh+dt@...nel.org" <robh+dt@...nel.org>,
Hans de Goede <hdegoede@...hat.com>,
Chen-Yu Tsai <wens@...e.org>, Jens Kuske <jenskuske@...il.com>,
Arnd Bergmann <arnd@...db.de>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-sunxi@...glegroups.com" <linux-sunxi@...glegroups.com>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH 1/6] ARM: sunxi: Add Machine support for A33
Hi,
On Sun, May 10, 2015 at 4:03 PM, Maxime Ripard
<maxime.ripard@...e-electrons.com> wrote:
> Hi,
>
> On Sun, May 10, 2015 at 12:16:18PM +0530, Vishnu Patekar wrote:
>> Allwinnner A33 quad core cortex-a7 based SOC.
>
> There's one n to many in Allwinner, and having a verb in that sentence
> would help
Yes, Correct.
>
>> It is similar to A23.
>>
>> Renamed cpu method to "allwinner,sun8i" for common sun8i smp.
>> smp code is generic for A23, A33 and hopefully H3.
>
> Please do only one thing in a patch.
OKie.
>
>> Signed-off-by: Vishnu Patekar <vishnupatekar0510@...il.com>
>> ---
>> Documentation/devicetree/bindings/arm/sunxi.txt | 1 +
>> arch/arm/mach-sunxi/Kconfig | 2 +-
>> arch/arm/mach-sunxi/platsmp.c | 2 +-
>> arch/arm/mach-sunxi/sunxi.c | 4 ++--
>> 4 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/sunxi.txt b/Documentation/devicetree/bindings/arm/sunxi.txt
>> index 42941fd..e32f082 100644
>> --- a/Documentation/devicetree/bindings/arm/sunxi.txt
>> +++ b/Documentation/devicetree/bindings/arm/sunxi.txt
>> @@ -9,4 +9,5 @@ using one of the following compatible strings:
>> allwinner,sun6i-a31
>> allwinner,sun7i-a20
>> allwinner,sun8i-a23
>> + allwinner,sun8i-a33
>
> Here you're introducing a new compatible for a machine that is
> sun8i-a33.... [1]
>
>> allwinner,sun9i-a80
>> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
>> index 81502b9..38bedd8 100644
>> --- a/arch/arm/mach-sunxi/Kconfig
>> +++ b/arch/arm/mach-sunxi/Kconfig
>> @@ -35,7 +35,7 @@ config MACH_SUN7I
>> select SUN5I_HSTIMER
>>
>> config MACH_SUN8I
>> - bool "Allwinner A23 (sun8i) SoCs support"
>> + bool "Allwinner (sun8i) SoCs support"
>> default ARCH_SUNXI
>> select ARM_GIC
>> select MFD_SUN6I_PRCM
>> diff --git a/arch/arm/mach-sunxi/platsmp.c b/arch/arm/mach-sunxi/platsmp.c
>> index e8483ec..c56b501 100644
>> --- a/arch/arm/mach-sunxi/platsmp.c
>> +++ b/arch/arm/mach-sunxi/platsmp.c
>> @@ -189,4 +189,4 @@ struct smp_operations sun8i_smp_ops __initdata = {
>> .smp_prepare_cpus = sun8i_smp_prepare_cpus,
>> .smp_boot_secondary = sun8i_smp_boot_secondary,
>> };
>> -CPU_METHOD_OF_DECLARE(sun8i_a23_smp, "allwinner,sun8i-a23", &sun8i_smp_ops);
>> +CPU_METHOD_OF_DECLARE(sun8i_smp, "allwinner,sun8i", &sun8i_smp_ops);
>
> Like I was saying, this is an unrelated thing, it should be in a
> separate patch.
>
> And this is wrong.
>
> A compatible should be made for the first IP that uses it. The first
> user of that particular method has been the A23, it should be what's
> in the compatible.
>
> If the A33 is by chance using the exact same code, then we have two
> choices, either reuse that compatible, or introduce a new one if it
> slightly differs. And since the A33 has more cores than the A23, it
> does differ.
>
> So please add a new compatible.
>
> That also breaks the SMP code in the A23 which is a no-go, since the
> compatible would have changed but not the DT.
I think adding something like below is good way to enable smp on a33
as we are going to use separate dtsi for a33,
and a23 for now.
CPU_METHOD_OF_DECLARE(sun8i_smp_a33, "allwinner,sun8i-a33", &sun8i_smp_ops);
>
>> diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c
>> index 1bc811a..8937d0d 100644
>> --- a/arch/arm/mach-sunxi/sunxi.c
>> +++ b/arch/arm/mach-sunxi/sunxi.c
>> @@ -66,11 +66,11 @@ DT_MACHINE_START(SUN7I_DT, "Allwinner sun7i (A20) Family")
>> MACHINE_END
>>
>> static const char * const sun8i_board_dt_compat[] = {
>> - "allwinner,sun8i-a23",
>> + "allwinner,sun8i",
>
> [1] ... And here, you don't introduce that new machine compatible, but
> remove one a use another one instead....
>
> Apart from the documentation mismatch, you really shouldn't do that.
>
> The machine compatible should be a identifier for the board and the
> SoC, so that we can identify both easily, and possibly enable
> quirks. The only question you should ask yourself whenever you add a
> new compatible is "is this exactly the same IP" ?
>
> In such a case, is the A23 *exactly* the same as the H3 and the A33?
>
> The answer is obviously no, otherwise we would not have this patchset
> in the first place.
>
> So you just need to introduce a new compatible for the A33, just like
> you did in the Documentation, and add that new compatible in the machine.
I'll add new compatible, "allwinner,sun8i-a33"
>
>> NULL,
>> };
>>
>> -DT_MACHINE_START(SUN8I_DT, "Allwinner sun8i (A23) Family")
>> +DT_MACHINE_START(SUN8I_DT, "Allwinner sun8i Family")
>> .dt_compat = sun8i_board_dt_compat,
>> .init_late = sunxi_dt_cpufreq_init,
>> MACHINE_END
>> --
>> 1.9.1
>>
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists