[PATCH 3/9] 8xx: Add pin and clock setting functions.

[Scott Wood]

On Wed, Sep 05, 2007 at 11:39:57AM +0400, Vitaly Bordug wrote:
> > Note that this code is mostly duplicated from the existing CPM2
> > version.
> > 
> Then it would be good to mention that in short comment block before function.

The code would very quickly become unreadable if we kept comments around
for every movement of code.  Given the current state of things, it's
pretty much a given that most functions in commproc.c have a
corresponding function in cpm2_common.c, and vice versa.

I'd like to merge some of it at some point.

> > > We're adding helper functions and should be ready that something
> > > somewhere won't work as expected.  
> > 
> > Could you elaborate on what you expect to possibly not work, or what
> > we should do to "be ready"?
> Some new PQ-like board being added to powerpc. I mean, each newly-added
> non-static function should have some sort of description unless
> it(function) is completely self-explanatory.

I thought it was completely self-explanatory.

> Just a few words either here as a comment or in patch description, what
> the function supposed to do and which way, so that someone not familiar
> with cpm/cpm2, would quickly understand what's happening in there.

If they're not familiar with CPM, and want to understand how hardware
setup for it works, the user's manual for one of the chips would be a
good place to start...  As it is, it's pretty clear without knowledge of
the CPM that it's a helper function that sets and clears bits in certain
registers.

Since we seem to have differing views of the target audience, could you
suggest a specific wording that you're looking for?

-Scott