Re: [PATCH] net: unisys: adding unisys virtnic driver

[Jes Sorensen <Jes.Sorensen@redhat.com>]

Erik Arfvidson <earfvids@redhat.com> writes:
> The purpose of this patch is to add Unisys virtual network driver
> into the network directory and also to start a discussion about
> the requirements needed.
>
> Signed-off-by: Erik Arfvidson <earfvids@redhat.com>

Erik,

I was discussing this with colleagues and I want to give you some
general comments on this. My comments are not specific to virtnic.c
itself.

Looking over the logs of drivers/staging/unisys, I noticed a fair amount
of cleanups has been applied, but not a lot of fixes addressing what I
would consider the real issues.

The first thing you should work on is to get rid of
drivers/staging/unisys/uislib - it looks to provide a lot of wrappers
and utility functions which already exist. You need to address things
like:

 - Custom atomic primitives (uisqueue.c)
 - List handlers (use list.h) and all the utility functions we provide
 - Functions for launching killing kernel threads (uisthread)
 - There is most of a bus implementation in there - is this really
   needed, ie. are the devices sitting on a PCI bus, or is this some
   special bus type?
 - Use proper data types - your code should contain no 'long long' ever!
   If you need data types of a specific size, use u8/u16/u32/u64, and
   please get rid of broken Windows stuff such as BOOL and #pragma.
 - /proc handlers - you should most likely be using /sys
   (configs/debugfs) and don't wrap things in libraries, do it near the
   code using it.

Basically I recommend you start working your way through uislib, and
once you have eliminated 90% of it, you should be a lot closer to code
that can go into mainline.

I know my colleague Neil has some issues on this specific driver, which
I have less insight into, so I think he'll post some comments on that
too.

I hope this is helpful!

Cheers,
Jes