linux-kernel - Re: [PATCH v2 0/8] ARM: at91: Remove mach/ includes from the reset driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20141028085053.41c6ce46@bbrezillon>
Date:	Tue, 28 Oct 2014 08:50:53 +0100
From:	Boris Brezillon <boris.brezillon@...e-electrons.com>
To:	Alexandre Belloni <alexandre.belloni@...e-electrons.com>
Cc:	Nicolas Ferre <nicolas.ferre@...el.com>,
	Sebastian Reichel <sre@...nel.org>,
	Jean-Christophe Plagniol-Villard <plagnioj@...osoft.com>,
	Dmitry Eremin-Solenikov <dbaryshkov@...il.com>,
	David Woodhouse <dwmw2@...radead.org>,
	Maxime Ripard <maxime.ripard@...e-electrons.com>,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	linux-pm@...r.kernel.org
Subject: Re: [PATCH v2 0/8] ARM: at91: Remove mach/ includes from the reset
 driver

Hi,

On Tue, 28 Oct 2014 00:09:29 +0100
Alexandre Belloni <alexandre.belloni@...e-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'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.


Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/