[openib-general] [PATCH v2 1/2] ofed_1_2 Changes to kernel_patches/ for Chelsio T3 Support.

[Steve Wise]

On Fri, 2007-01-12 at 00:33 +0200, Michael S. Tsirkin wrote:
> For genalloc,
> 1. It's a backport, why do you put it in fixes?   

Because its not exactly a backport.  The exact file exists in 2.6.20 but
it is only conditionally compiled into the kernel.  But I can put it
backport if you want.  That makes more sense I guess.

> 2. I think a better way would be to stick it under kernel_addons/backports than
>    as a patch. Please see my earlier mail on how this works.
>  

Um, what earlier mail?

>    It is *much* easier to maintain this way (you see the full file).
>    And also benefits all the subsystem and not just chelsio.
>    We currently have infrastructure to add headers only, so what you do
>    for C is stick iit under backports/<kernel>/src/genpool.c and
>    add a backport patch that just adds a small file pulling
>    in the real code, like this:
> 
> +#include "src/genpool.c"
> 
>   and also adds the relevant line to Makefile to stick it in core.
> 

Ok, so add this into ib_core then?

>   Look at how we did this for
>   kernel_addons/backport/2.6.5_sles9_sp3/include/src/stream.c
> 

ok.

> 
> Or, if you have an idea how to get rid of the two-line patch
> that would be fine too. Sticking all these things in core is ugly
> and creates Makefile patches that often trigger conflicts.
> Maybe we should have an ib_backports module built?
> This might be a good idea, I'll think about it next week.
> 

A backports modules sounds interesting...

> 3. Is there a real reason to call it iwch_gen_pool and not just gen_pool
> as in 2.6.20? The patch would be much smaller then.
> 

What if its already built in and export in the kernel we're trying to
load into?  Will this cause a load problem?  I was assuming it
would...that's why I changed the names.  Am I wrong?

> 4. I think you want to remove playing with EXTRA_CFLAGS
> for your driver: these things should be controlled centrally.
> Instead of -DDEBUG just do
> #ifdef CONFIG_INFINIBAND_CXGB3_DEBUG in your code.
> you really do not want to touch the global DEBUG macro.
> 

This was feedback we got from reviewers on the amso driver.  They said
use DEBUG.

> And -g is something user should ask explicitly.
> 

The user asks explicitly by requesting to compile the debug module.


Steve.