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

[Nicolas Pitre]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/