On 28/10/2014 at 09:59:08 +0100, Maxime Ripard wrote : > Hi, > > On Tue, Oct 28, 2014 at 09:52:03AM +0100, Alexandre Belloni wrote: > > Hi, > > > > On 28/10/2014 at 08:50:53 +0100, Boris Brezillon wrote : > > > Hi, > > > > > > On Tue, 28 Oct 2014 00:09:29 +0100 > > > Alexandre Belloni <alexandre.bell...@free-electrons.com> wrote: > > > > > > > This series removes the mach/ headers dependency from the reset driver. > > > > It is > > > > also laying some groundwork for the necessary power management support > > > > rework. > > > > > > > > The first patch adds and export a function to shutdown the sdram from > > > > the sdramc > > > > driver. That function also take the RSTC CR register and a value as > > > > parameters > > > > to be able to reset the chip. This is a hackish way of doing it but it > > > > ensures > > > > that all the code fits in one cache line. We already have plan to start > > > > using > > > > the sram to have a cleaner way to execute that code safely as soon as > > > > that > > > > series goes in: > > > > http://lists.infradead.org/pipermail/linux-arm-kernel/2013-September/198778.html > > > > > > > > The second patch makes the sdramc driver usable from the board files. > > > > > > > > The third patch actually registers the sdramc driver from the boards > > > > files. > > > > The fourth patch does the same, only for sam9g45 and sam9rl to simplify > > > > future > > > > merging as the board files have been removed. Simply drop that patch. > > > > > > > > The fifth patch makes the at91-reset driver use the newly created > > > > at91_ramc_shutdown() function and removes the mach/ headers inclusion. > > > > > > I'm not a big fan of this approach. > > > > > > I definitely think each step of the reset process should be handled in > > > the appropriate block (and the patch series you pointed out would > > > definitely help in achieving this goal), but you're just moving all the > > > stuff done in the reset driver into the SDRAM one, which means you're > > > solving one design issue by introducing a new one. > > > > > > Moreover, the errata at the origin of this hack is attached to the RSTC > > > (Rest Controller) block in the datasheets. > > > > > > > I agree it is still not clean but it is a step in the good direction. > > The sdram shutdown will have to be down in the sdram driver at some > > point, even if the errata is attached to the reset controller. At least, > > the series introduces a function to do the sdram shutdown. > > Not really. AFAICS, this adds a function to reset the CPU. Actually, > it doesn't add anything at all, it just copies what was done in the > reset driver. > > > > I'd rather keep the reset driver as is and move SDRAM related macros > > > into a specific header (include/linux/memory/atmel-sdram.h or > > > include/soc/atmel/memory.h as you proposed) so that the reset driver > > > can reference them without including mach headers. > > > > > > > My personal opinion is that it is better to hide the registers/bits from > > the reset driver right now as we have two different IPs and the sdram > > driver already knows how to make the difference between them. > > The reset driver doesn't do anything anymore with these patches. Why > not just remove it altogether? >
It does, the reset driver knows about the reset registers. The plan is to move the actual reset back to that driver when the kernel will be able to easily execute code from sram. -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
