linux-kernel - Re: [PATCH 0/2] Common struct clk implementation, v14

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: <alpine.LFD.2.00.1104141154390.24613@xanadu.home>
Date:	Thu, 14 Apr 2011 12:06:40 -0400 (EDT)
From:	Nicolas Pitre <nicolas.pitre@...aro.org>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
cc:	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Jeremy Kerr <jeremy.kerr@...onical.com>,
	Lorenzo Pieralisi <Lorenzo.Pieralisi@....com>,
	Vincent Guittot <vincent.guittot@...aro.org>,
	linux-sh@...r.kernel.org, Sascha Hauer <s.hauer@...gutronix.de>,
	Paul Mundt <lethal@...ux-sh.org>,
	lkml <linux-kernel@...r.kernel.org>,
	Dima Zavin <dmitriyz@...gle.com>,
	Saravana Kannan <skannan@...eaurora.org>,
	Ben Dooks <ben-linux@...ff.org>,
	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 0/2] Common struct clk implementation, v14

On Thu, 14 Apr 2011, Russell King - ARM Linux wrote:

> On Thu, Apr 14, 2011 at 09:39:58AM -0400, Nicolas Pitre wrote:
> > However we can and must do better in consolidation work in order to make 
> > the tree more maintainable of course.  _That_ is the real issue and 
> > _that_ is what the ARM tree sucks at.  The fact that this consolidation 
> > work might reduce the size of the ARM code is only a side effect, not 
> > the primary goal.  So let's not get too hung up about LOC but focus on 
> > improving the infrastructure instead.
> 
> Look, this is what is in amongst the 6K lines of new code - these are
> the top two in the diffstat with the highest number of additional lines:
> 
>  arch/arm/mach-ux500/clock-db8500.c                 | 1291 +++++++++++++
>  arch/arm/mach-ux500/prcmu-db8500.c                 | 2018 ++++++++++++++++++++
> 
> These comprise 50% of the 6K of new code.  clock-db8500.c is a mixture
> of code and data declarations for individual clocks - which is essentially
> what Linus picked up with his complaint on under OMAP.  That in itself
> makes me worried.

Sure.  So... let's see what we can do about it.  Having 1300 lines only 
for clock stuff on only one platform certainly looks silly.  Either we 
can consolidate some of that stuff with another similar-enough platform, 
or we're stuck with the ST-E hardware being just too different from, 
say, the OMAP clock hardware for those to be consolidated.  In the 
former case we can work towards a solution and that code currently in 
next won't make it to mainline, or in the later case... well... we'll 
have to live with it.  If in doubt let's simply ask the opinion of those 
mainline people who said they're willing to help with advices.  If in 
the end they have no better solutions than what we have now then it will 
be harder for them to complain.

> Looking at the other file, prcmu-db8500.c seems to contain at least 5 sets
> of mailbox support code.  Take a look at "Subject: [PATCH 9/9] mach-ux500:
> add DB5500 PRCMU interface" from Linus Walleij on 31st March on this list
> for the patch.  Are we sure that this couldn't be consolidated in some
> way, at least at file level?
> 
> So while you can say about not getting hung up about LOC, LOC is a good
> indication that things are still going wrong in much the same way.

Agreed.  My point is, we shouldn't be afraid to add more code if that 
code is allowing for some consolidation to happen down the road, which 
should be the case with the initial subject of this thread.


Nicolas
--
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/