Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU

From: Pankaj Dubey <pankaj.dubey@samsung.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: "linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	Andrew Lunn <andrew@lunn.ch>, Heiko Stuebner <heiko@sntech.de>,
	geert+renesas@glider.be, Linus Walleij <linus.walleij@linaro.org>,
	Liviu Dudau <liviu.dudau@arm.com>,
	Michal Simek <michal.simek@xilinx.com>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Jisheng Zhang <jszhang@marvell.com>,
	Magnus Damm <magnus.damm@gmail.com>,
	Russell King - ARM Linux <linux@armlinux.org.uk>,
	Wei Xu <xuwei5@hisilicon.com>,
	"thomas.ab@samsung.com" <thomas.ab@samsung.com>,
	"cpgs ." <cpgs@samsung.com>,
	Stephen Warren <swarren@wwwdotorg.org>,
	Ray Jui <rjui@broadcom.com>, Simon Horman <horms@verge.net.au>,
	Dinh Nguyen <dinguyen@opensource.altera.com>,
	Shawn Guo <shawnguo@kernel.org>,
	Patrice Chotard <patrice.chotard@st.com>,
	linux-kernel@vger.kernel.org, vireshk@kernel.org,
	Jun Nie <jun.nie@linaro.org>,
	Shiraz Hashim <shiraz.linux.kernel@gmail.com>
Subject: Re: [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU
Date: Thu, 8 Dec 2016 20:48:08 +0530	[thread overview]
Message-ID: <CAGcde9GZkaqN=7Xaenm9YsNUGoaX0vHDpRhn07heoqLMdBw=FA@mail.gmail.com> (raw)
In-Reply-To: <2339821.zs8sl71mMv@wuerfel>

On 18 November 2016 at 19:02, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday, November 18, 2016 12:48:07 PM CET Russell King - ARM Linux wrote:
>> On Fri, Nov 18, 2016 at 01:14:35PM +0100, Arnd Bergmann wrote:
>> > @@ -41,6 +43,9 @@ void scu_enable(void __iomem *scu_base)
>> >  {
>> >       u32 scu_ctrl;
>> >
>> > +     if (scu_base)
>> > +             scu_base = scu_base_addr;
>> > +
>>
>> This looks to me like nonsense.
>>
>> >  #ifdef CONFIG_ARM_ERRATA_764369
>> >       /* Cortex-A9 only */
>> >       if ((read_cpuid_id() & 0xff0ffff0) == 0x410fc090) {
>> > @@ -85,6 +90,9 @@ int scu_power_mode(void __iomem *scu_base, unsigned int mode)
>> >       unsigned int val;
>> >       int cpu = MPIDR_AFFINITY_LEVEL(cpu_logical_map(smp_processor_id()), 0);
>> >
>> > +     if (scu_base)
>> > +             scu_base = scu_base_addr;
>> > +
>>
>> Ditto.
>>
>> Rather than doing this, I'd much prefer to always store the SCU base in
>> the SCU code, and remove the "void __iomem *scu_base" argment from all
>> these functions.
>
> Ok, then we just need one scu_probe_*() variant for each of the
> four methods of initializing it (iotable, of_iomap,
> ioremap(scu_a9_get_base) and hardcoded.
>
> The intention of doing the fallback for the NULL argument was
> to avoid having to add lots of new API while also allowing
> the change to be done one platform at a time.
>
> If we remove the argument from the other functions, they either
> need to get a new name, or we change them all to the new prototype
> at once. Either way works fine, do you have a preference between
> them?
>

Russell,

Any opinion on this. Are you OK, with the approach suggested by Arnd?

Thanks,
Pankaj Dubey

>         Arnd
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel