[PATCH v5 00/10] Per SoC descriptor

On Thursday 13 October 2011, Russell King - ARM Linux wrote:

On Thu, Oct 13, 2011 at 04:25:36PM +0100, Marc Zyngier wrote:

quoted

On 13/10/11 11:32, Russell King - ARM Linux wrote:

quoted

On Mon, Oct 10, 2011 at 03:02:12PM +0100, Marc Zyngier wrote:

[...]

quoted

quoted

 65 files changed, 569 insertions(+), 149 deletions(-)

This isn't particularly appealing for something that's supposed to
be a clean up - it's introducing 400+ new lines of code.  It looks like
most of this comes from the platform stuff rather than core stuff.

Can't we do any better with this?  We really need things to be going
in the other direction.

There is definitely room for improvement, by factoring in some common
code across platforms. But I see that as a second phase, and started by
just moving things to the SoC descriptor, all the code staying mostly
the same.

I was meaning without that - what concerns me is the size of increase
just to introduce this.  It seems needlessly large.

I think the main benefit of this series is that it allows the
cross-platform consolidation patches to be smaller, which unfortunately
is not easy to measure. The idea is that stuff that is currently
hardcoded per platform but should better be run-time determined
can be moved into a per-soc structure rather than the per-board
machine_desc. It would be good to list all this "stuff" to see how
much it will eventually be.

A number of things that we discussed about putting here have since
been obsoleted by better suggestions (e.g. arch_ioremap, VMALLOC_END,
__io, NR_IRQS, ...), but I would hope the tradeoff to still be
positive for this patch series.

On a related topic, I wonder how much of the existing machine_desc
we would actually be able to move out into a soc_desc, which is
something that would immediately save code lines and object size.

If it ends up being just the SMP and CPU-hotplug functions, then
the tradeoff is probably negative and we discard the series, but
if we expect it to have a postive long-term effect, I would still
merge it right away rather than waiting for the series to become
long enough to show the benefits in the diffstat.

I've also never been convinced by attempts to consolidate the hotplug
code - I've said this every time it's been suggested: I believe many
platforms just aren't trying to support hotplug CPU properly.

They've just copied the noddy Realview platform version - which is
noddy because ARMs platforms tend to have a total lack of power
management support on them.  The best we can do is put them in a WFI
loop waiting to be re-awoken.

Real platforms surely must have better PM support than that, and so
copying the Realview implementation does not make sense - it's good
to see that _some_ platforms have made an effort, though they still
just return from platform_cpu_die(), rather than the intended path of
re-awaking via the standard bringup path.  The return path from
platform_cpu_die() is supposed to be the last-ditch attempt if all else
has failed.

Good point, but I think that is another topic: 
Given that a bunch of architectures implement the same silly functions,
I think it definitely makes sense to consolidate them, even if we know
that they should actually be doing something else. By turning these
into function pointers, we can at least force platforms to include
a line like

	.cpu_disable = generic_silly_cpu_disable,

which hopefully leads people to think about why they are using this
version.

	Arnd