Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752633AbbEJKfJ (ORCPT ); Sun, 10 May 2015 06:35:09 -0400 Received: from down.free-electrons.com ([37.187.137.238]:54310 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752420AbbEJKfF (ORCPT ); Sun, 10 May 2015 06:35:05 -0400 Date: Sun, 10 May 2015 12:33:00 +0200 From: Maxime Ripard To: Vishnu Patekar Cc: emilio@elopez.com.ar, linus.walleij@linaro.org, robh+dt@kernel.org, hdegoede@redhat.com, wens@csie.org, jenskuske@gmail.com, arnd@arndb.de, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-sunxi@googlegroups.com, devicetree@vger.kernel.org Subject: Re: [PATCH 1/6] ARM: sunxi: Add Machine support for A33 Message-ID: <20150510103300.GD11057@lukather> References: <1431240383-12763-1-git-send-email-vishnupatekar0510@gmail.com> <1431240383-12763-2-git-send-email-vishnupatekar0510@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="dBTySC83OhS/t4p9" Content-Disposition: inline In-Reply-To: <1431240383-12763-2-git-send-email-vishnupatekar0510@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5695 Lines: 163 --dBTySC83OhS/t4p9 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 > It is similar to A23. >=20 > 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. > Signed-off-by: Vishnu Patekar > --- > 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(-) >=20 > diff --git a/Documentation/devicetree/bindings/arm/sunxi.txt b/Documentat= ion/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 > =20 > 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 =3D { > .smp_prepare_cpus =3D sun8i_smp_prepare_cpus, > .smp_boot_secondary =3D sun8i_smp_boot_secondary, > }; > -CPU_METHOD_OF_DECLARE(sun8i_a23_smp, "allwinner,sun8i-a23", &sun8i_smp_o= ps); > +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. > 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) Fam= ily") > MACHINE_END > =20 > static const char * const sun8i_board_dt_compat[] =3D { > - "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. > NULL, > }; > =20 > -DT_MACHINE_START(SUN8I_DT, "Allwinner sun8i (A23) Family") > +DT_MACHINE_START(SUN8I_DT, "Allwinner sun8i Family") > .dt_compat =3D sun8i_board_dt_compat, > .init_late =3D sunxi_dt_cpufreq_init, > MACHINE_END > --=20 > 1.9.1 >=20 --=20 Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com --dBTySC83OhS/t4p9 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVTzPcAAoJEBx+YmzsjxAgMnkQAKC0kRTsL/4TuaefjqzjAseG mq+L4bcwsGGPLbN3YZAS/HF4UZG20fgLiMpdvwI20A8scGkawM8kT5Tkj3Y2BO8h vZxTHznMuL4qOSPIhq6uiKFWf/84ld91ubH0VdKFLDPG372YpklhgPFAFBUcgv1P sGLEsZiGAtPGazVxXycvXQqGriMyoTEjUPSdGNK2dsNKt2Cj3VELfvsN/dim3IKN pBIspljTLt8OKvZz69mMxvWaAbwdLdAemEVZYtfjhsRbmiMa13I8oQktCvSq6pBN Yu3cDdUlPcY0vRKZnOrOW0rmQ7RoqAtb4fCnYJyKwDqGos4ZksAp9jc8CaMVVluA 66Pw+2wcqEv9DBu7oCsFb+czFTLbycHGNcrR+xltcloSUI4rc+dVYm9bNlLcfpw3 qdGONiu3Hzo3YZ8zKrhPnGRMJPhZ1Lxie8r0tNQqQ95kCv/EtATctv/ms3rhDNhJ YcGnUARYYrWEgNyxugzel982D9kAPpWvhS6F8c5md47+eeSM8rIOrPx4kHaY2sQo KAJj3YVMn4ta35CpunT+Nb3gus1A4yrjjvJk2a/1U8Rwjc3r511OgnpDwVzKTe7N hsWy/Uo0OlFytukGsH1SSfi8aLiNieV+yPuhCaI0+MwH4BMz2A+eNz5htuwckGRo ewLTZDBp02OYSDiMtoT6 =1XcF -----END PGP SIGNATURE----- --dBTySC83OhS/t4p9-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/