Hi Tomasz, On 6 May 2014 23:39, Sachin Kamat <sachin.kamat@xxxxxxxxxx> wrote: > Hi Tomasz, > > On 6 May 2014 22:14, Tomasz Figa <tomasz.figa@xxxxxxxxx> wrote: >> Hi Sachin, >> >> Thanks for addressing the comments. I need to verify few things on Universal >> C210 board first, before I could give my Reviewed-by tag or further >> comments. >> >> I also have some general comments that I missed before, due to limited time >> for review. Please see inline. > > Thanks for your review. > >> >> >> On 06.05.2014 10:10, Sachin Kamat wrote: >>> >>> Instead of hardcoding the SYSRAM details for each SoC, >>> pass this information through device tree (DT) and make >>> the code SoC agnostic. Generic SRAM bindings are used >>> for achieving this. >>> >>> Signed-off-by: Sachin Kamat <sachin.kamat@xxxxxxxxxx> >>> Acked-by: Arnd Bergmann <arnd@xxxxxxxx> >>> Acked-by: Heiko Stuebner <heiko@xxxxxxxxx> >>> --- [snip] >>> >>> static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr) >>> { >>> - void __iomem *boot_reg = S5P_VA_SYSRAM_NS + 0x1c + 4*cpu; >>> + void __iomem *boot_reg; >>> + >>> + if (!sram_ns_base_addr) >>> + return 0; >> >> >> Shouldn't this return an error instead? I'm not sure which one would be >> appropriate, though, probably one of -ENODEV, -ENXIO or -EFAULT. > > IIRC, returning error here causes the system to hang and even primary > cpu does not boot. > Since any error or absence of sram nodes should atleast boot the > primary CPU, I thought > this is better. Small correction. The above explanation was for the absence of the check. Sorry about the confusion. Will add -ENODEV here. -- With warm regards, Sachin -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html