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? Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
signature.asc
Description: Digital signature
