Re: [PATCH 07/36] ARM: exynos: use private samsung_cpu_id copy

From: Arnd Bergmann <arnd@arndb.de>
To: Krzysztof Kozlowski <krzk@kernel.org>
Cc: "moderated list:ARM/SAMSUNG EXYNOS ARM ARCHITECTURES" 
	<linux-samsung-soc@vger.kernel.org>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Pankaj Dubey <pankaj.dubey@samsung.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Tomasz Figa <tomasz.figa@gmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Kukjin Kim <kgene@kernel.org>,
	Linux PM list <linux-pm@vger.kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Marek Szyprowski <m.szyprowski@samsung.com>
Subject: Re: [PATCH 07/36] ARM: exynos: use private samsung_cpu_id copy
Date: Wed, 23 Oct 2019 14:37:47 +0200	[thread overview]
Message-ID: <CAK8P3a3wxaUR4Nx9a6JBu3DjtJ90ZRH6+-V8-g=6mcEih34Qnw@mail.gmail.com> (raw)
In-Reply-To: <20191023105443.GE10630@pi3>

On Wed, Oct 23, 2019 at 12:56 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Thu, Oct 10, 2019 at 10:29:51PM +0200, Arnd Bergmann wrote:
> > diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
> > index 9aa483366ebc..da9300d655c6 100644
> > --- a/arch/arm/mach-exynos/exynos.c
> > +++ b/arch/arm/mach-exynos/exynos.c
> >
> > +#define S5P_VA_CHIPID        ((void __iomem __force *)0xF8000000)
>
> How about keeping the S3C_VA and S3C_ADDR_BASE macros/defines and using
> them?

Ok, done now.

> They still appear in arch/arm/include/debug/exynos.S so they could
> be integrated into one header, unless you plan to remove it in further
> patches.

I don't think it actually helps, but it doesn't hurt either:

arch/arm/include/debug/exynos.S cannot #include any mach/*.h header files,
so the definition has to remain duplicated, unless I'm missing something.

Also, the addresses should be completely independent, as long as the virtual
address for the uart does not overlap with the virtual address for the chipid.

One possible cleanup here would be to completely remove the S5P_VA_CHIPID
static map and use ioremap(), but doing that requires that the first call to
soc_is_exynosXXXX() happens after the ioremap.

> > diff --git a/arch/arm/plat-samsung/include/plat/cpu.h b/arch/arm/plat-samsung/include/plat/cpu.h
> > index fadcddbea064..02d7f991d5a3 100644
> > --- a/arch/arm/plat-samsung/include/plat/cpu.h
> > +++ b/arch/arm/plat-samsung/include/plat/cpu.h
> > @@ -111,8 +111,6 @@ extern void s3c24xx_init_io(struct map_desc *mach_desc, int size);
> >  extern void s3c64xx_init_cpu(void);
> >  extern void s5p_init_cpu(const void __iomem *cpuid_addr);
>
> You can remove it as well.

Ok, removed.

        Thanks,