On 18 November 2016 at 19:02, Arnd Bergmann <a...@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-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel