[ag-automation] [RFC] OSADL Board Support Package Specification, Revision 0.1

[Wolfgang Denk]

Dear Robert,

in message <20060913161615.GF10251 at pengutronix.de> you wrote:
> 
> "configurations" sounds more like changing things in .config. Board

You are right, we probably need a different, better name.

> Support Packages usually do more than that, especially on non-PCI
> platforms. For example on most ARM cpus you have chip specific hardware,
> module specific hardware and often also baseboard specific hardware.
> Usually a BSP for that kind of boards has something like
> arch/arm/mach-pxa/mymodule.c and mymodule-somebaseboard.c, containing
> driver model registration and specific stuff for these ressources. 

Agreed. But for this the term "extension" is IMHO not optimal either.
When I hear  "extension",  I  think  of  completely  new,  additional
features which have not been available in the kernel before. But here
we  usually  have  several examples (other BSPs) which can be used as
reference, and how ofthen this is mostly a copy & paste job?

So I guess we need yet another name...

> Although it is common and convenient to give a complete source tree to a
> customer, it is IMHO much more important that the BSP is structured in a
> reviewable and manageble way and that the patch flow is clear,
> documented and reproducable. 

Agreed. I think we should make clear that  different  representations
of  the  BSP  have to be provided for different purposes: on one side
the ready-to-use tarball for the end user who does not know (nor need
or want to know) about tools like quilt or stgit or git or  patch  or
...,  and on the other side the patch set (in which representation it
may come) which can be used to resume developmont of the code.

> I've seen so many BSPs out there where vendors have added tons of
> undefined patches without any documentation (not even if a specific

I know exactly  what  you  are  speaking  about,  and  I  share  your
repugnance.

> changed things in a random way that it should be the aim of this
> specification to make sure that
> 
> 	* the BSP user knows the well-known-kernel version it depends on
> 	* patch stacks can be applied and de-applied in a clean way.

Agreed.

> >   A better definition of "vanilla  kernel  version"  is  needed.  For
> >   example,  it  is  not  clear  if  "2.6.18-rc6-gbd314d97"  is such a
> >   "vanilla" version, or  if  I  have  to  use  "2.6.18-rc6"  or  even
> >   "2.6.17" instead.
> 
> Do you have a suggestion? IMHO any of the "well known kernel trees"
> could make it, which is anything which is archived on kernel.org in a
> structured way (2.6.17, 2.6.18-rc6, ...).

Kernel development is based on git, so we  should  use  git  IDs  for
reference.  I  think  we  should  recommend to use tagged versions as
reference so  that  the  identification  would  be  a  git  tag  like
"v2.6.18-rc7",  but  also allow for any commit ID from the Linus tree
to be used.

> >   "Patches are never allowed to delete parts of the tree" - please
> >   delete this. When I find I can refactor identical code used by
> >   several BSP I maintain into common code I should of course be able
> >   to get rid of the resulting dead files.
> 
> That's not the point here. We have seen BSPs for example for ARM where
> the vendor has deleted all other arch/{powerpc,mips,...} directories and
> everything else he thought nobody would need; this makes it pretty
> impossible to diff the BSP against original versions and find out the
> changes.

I know what you mean, and agree with that, but the text is different.

> Any suggestion how we could phrase this in a way it becomes more clear?

Maybe: "Patches should always only make the nesessary changes. They
should be orthogonal and must not touch unrelated files." ?

> >   "Files already existing in the vanilla kernel tree are not allowed
> >   to be replaced by a file with the same name but other contention." -
> >   please delte this. This is exactly what a patch does: changing the
> >   content of existing files.
> 
> You modify a file, but you don't replace for example
> drivers/net/some_driver.c by a file called some_driver.c which is
> something completely new (for example another driver for the same
> device, written by other authors and being completely different).

Why not? If I find I need a working FOO driver because the existing
one is broken or inefficient or otherwise unsuitable, and I have a
better implementation, I will replace the old FOO driver with my new
implementation. That's exactly what I do now with any kernel file.

I think we should keep in mind that the patch set of auch a BSP  will
quite  often  not  only  consist  of strictly "board support" related
changes, but we may find it necessary  to  add  fixes  or  extensions
(remember  RT  support)  that have not been added to the vanilla tree
yet. IMHO it would be a major  mistake  if  we  disallowed  for  such
patches here (but of course we agree that all possible efforts should
be  taken  to  have  such patches integrated into the vanilla tree as
quickly as possible - but you know how slow soime things go).

Strictly speaking, any patched file  is  "something  completely  new"
(and  will  become  a  completely  new  object  in the git repo). The
question is how much the patch changes - 2 lines or 5% or 50% or 90%.
Where to draw the line? Eventually a two-line  change  can  radically
change the whole system behaviour.


I think we should be careful here not to restrict ourselfes in a way
that makes practical work impossible.


> >   " Patches should not destroy other architectures or sub
> >   archtectures." - please delete this; it is redundant to the
> >   statement " Patches are never allowed to delete parts of the tree,
> >   especially not other architectures." above.
> 
> It is meant differently. The paragraph above talks about removing for
> example arch/powerpc in an ARM BSP.
> 
> For example, I've recently seen a BSP by a big chip vendor which
> introduced a new CPU core, and because they had problems with the caches
> they had added hand crufted assembler instructions for that CPU,
> flushing the cache, to several random places in mm/, which is generic
> code. In result that kernel could not even be configured for x86 any
> more, although the kernel has all the abstractions. This method is ok
> for doing a quick hack, but if a BSP product is a quick hack it should
> be labled as such.

Again, I agree. But maybe it's sufficient to ask for "minimal impact"
patches, which don't modify unrelated code. I feel tempted  to  agree
to  some phrase like "don't break other architectures or boards", but
I think we can only have this as a recommendation, as a goal.  It  is
impossible  for  anyone  to actually test the resulting code on *all*
architectures and boards, so all we can ask for is a best effort.

> In reality such BSPs are out there, and the spec will give the customer
> a classification scheme for quality. You are right that "uggly hack" is
> not seriously business compatible, so please suggest another
> declaration.

"incompatible test version" ?

> Your example is not what I would consider to be "generic files". If you
> add a new option for a $arch board, it is surely the right way to add it
> to arch/$arch/Kconfig. Generic files means "generic code" here, for

Where is the limit? A file is a file is a file ...

> example if you want to change some memory management stuff which is ARM
> related, fix it in arch/arm/mm, not in mm. If you add something which is
> PXA related, put it in arch/arm/mach-pxa, not somewhere else.

I agree completely (and I guess you know that). I just want to  point
out deficiencies in the current text.

> I do not agree that the levels are not hierarchic. 
> 
> - IMHO the lowest level of sophistication is a BSP that "just boots" and
>   doesn't follow any of the usual kernel developers' best practise.
> 
> - OBSPS-Patch-Level-1 does only follows the idea of fixing and adding
>   stuff in the right places, nothing else.
> 
> - OBSPS-Patch-Level-2 adds following the coding style and taking care of
>   compiler warnings and survives static code analysis.
> 
> - Level 3 adds long term maintenance & community review.

When reading this for the first time it was not really  clear  to  me
that the requirements of the lover levels apply to the higher levels,
too.  On  second  reading  I  understood  that  this probably was the
intended meaning, but the text can be interpreted differently.

> I disagree with your disagreement :-)

;-)

> Several points why toolchain specification is important:

I did not say they are not important! Of cours ethey are, but I think
this is outside the scope  of  a  BSP  specification.  This  probably
should  be  addressed  in  a  "toolchain  compatibility"  document or
something like that.

> - My very personal opinion is that a vendor should supply the scripts
>   used to build the toolchain, so if something goes wrong one has the
>   chance to file a bugzilla entry for upstream projects. But that's
>   surely a matter of what you want to achieve.

Agreed.

> - Newer kernels have other toolchain requirements, note things like gcc3
>   vs. gcc4 etc.

I am all too aware of these issues.

> I don't doubt that usually vendors give their customers a toolchain that
> "just works" in compiling the kernel. IMHO the spec should make sure
> that, if the vendor supplies a toolchain, the OSADL BSP Spec conformance
> process either declares it to be "binary-and-we-don't-know-what-it-is"
> or "this follows a well documented process". 
> 
> Anyway, suggestions for better wording are always welcome.

See above - I wouldlike to keep the BSP document as smal and  focused
on  the  topic  as possible. The toolchain issue is complex enough in
itself to be handled in a separate document.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The games have always  strengthened  us.  Death  becomes  a  familiar
pattern. We don't fear it as you do.
	-- Proconsul Marcus Claudius, "Bread and Circuses",
	   stardate 4041.2