Re: [PATCH 1/2]Blackfin archtecture patche for 2.6.16

[Andrew Morton <akpm@osdl.org>]

Bernd Schmidt <bernds_cb1@t-online.de> wrote:
>
> Luke is probably still asleep at this time of night, so I'll try to 
> answer what I can...
> 
> Andrew Morton wrote:
> > "Luke Yang" <luke.adi@gmail.com> wrote:
> >>    This is the Blackfin archtecture patch for kernel 2.6.16.
> > 
> > - We don't want to be putting 44000 lines of new code in the kernel and
> >   then have it rot.  Who will support this in the long-term?  What
> >   resources are behind it?  IOW: what can you say to convince us that it
> >   won't rot?
> 
> We're a team inside Analog Devices who are maintaining a GNU toolchain, 
> uClinux kernel, and user space apps for the Blackfin.  All of this is 
> available on our blackfin.uclinux.org site.  We do not expect to go away 
> anytime soon.

OK.  Thanks for the contributions.

> ...
> >   We'd need to see some sort of authorisation from the original authors
> >   for the inclusion of their code.  Preferably in the form of
> >   Signed-off-by:s.  
> 
> I'll pass that along to the right people.  Would a "Signed-off-by: 
> Analog Devices" (similar to our FSF copyright assignments) be ok or does 
> it have to be individuals?  I believe the port actually predates the 
> involvement of most of the people working on it now.

I think names of individuals would be preferred - the Signed-off-by: is
often used when hunting down maintainers/developers to bug about problems. 
Although as it's a single megapatch, that's less useful in this case.

If we go with the single signed-off-by: I guess it would be best if that
was a person within AD who is in a position to authorise the merge.  If you
say that person is yourself or Luke then fine.

> > - Do you really need to support old_mmap()?
> 
>  From what I can tell, no we don't, although we'll have to make a small 
> change to our uClibc.  (A lot of this code got copied from the m68k port 
> initially... that may explain a few things).
> 
> > - Too much use of open-coded `volatile'.  The objective should be to have
> >   zero occurrences in .c files.  And volatile sometimes creates suspicion
> >   even when it's used in .h files.
> 
> Are you referring to the ones in 
> include/asm-blackfin/mach-bf533/cdefBF532.h?  These are memory-mapped 
> hardware registers (MMRs); do you have any better suggestions how to 
> access these?  That file actually comes from our in-house Visual DSP 
> compiler, and while there may be better ways of accessing the register 
> than those macros, there is something to be said for being able to drop 
> in a replacement if future chips have different addresses for these 
> registers.
> 
> The Blackfin has a lot of peripherals sitting on the same die as the 
> core, and they're all accessed through MMRs.

readl/writel and friends would be the preferred way of accessing
memory-mapped registers.  If that doesn't work then at least you should
wrap the volatile cast into a single inlined function somewhere so it's not
splattered everywhere.  That way the code is more pleasing to read and we
eliminate the risk that someone forgets to add the cast.