[RFC][BNX2X] .h files rewrite

[RFC][BNX2X] .h files rewrite

[Eliezer Tamir]

Dave,

Please take a look and tell me if this is better than the last attempt.

Main changes:

* Rewrite of the init code: 
	1. Full rewrite of the generated code into something easier to read.
	2. Removed zeroes by adding a zero command.
	3. Big chucks of data are read by the chip using the DMA engine (instead of being written by the driver).
	4. Gzipped more of the data.

* Removed BE microcode, the swapping is done in the driver instead.

* Rewrite of the HSI - broke it into three files bnx2x_hsi.h bnx2x_reg.h and bnx2x_fw_defs.h

* Fixed SPARSE warnings.

* Fixed all but three checkpatch.pl warnings. (IMHO they are false positives)

The diff is against latest net-2.6.

ftp link:
ftp://Net_sys_anon@ftp1.broadcom.com/0001-bnx2x-h-file-rewrite.txt

gzipped:
ftp://Net_sys_anon@ftp1.broadcom.com/0001-bnx2x-h-file-rewrite.txt.gz

(will also attache patch inline as reply to this email for review)

Thanks
Eliezer

Re: [RFC][BNX2X] .h files rewrite

[David Miller]

From: "Eliezer Tamir" <eliezert@broadcom.com>
Date: Sun, 28 Oct 2007 22:21:14 +0200

Overall things look significantly better, thanks a lot!

However, there is still one set of magic constants in here
which I hope you can clear up:

> +static const struct raw_op init_ops[] = {
> +#define PRS_COMMON_START        0
> +	{OP_WR, PRS_REG_INC_VALUE, 0xf},
> +	{OP_WR, PRS_REG_EVENT_ID_1, 0x45},
> +	{OP_WR, PRS_REG_EVENT_ID_2, 0x84},
> +	{OP_WR, PRS_REG_EVENT_ID_3, 0x6},
> +	{OP_WR, PRS_REG_NO_MATCH_EVENT_ID, 0x4},
> +	{OP_WR, PRS_REG_CM_HDR_TYPE_0, 0x0},
> +	{OP_WR, PRS_REG_CM_HDR_TYPE_1, 0x12170000},
> +	{OP_WR, PRS_REG_CM_HDR_TYPE_2, 0x22170000},
> +	{OP_WR, PRS_REG_CM_HDR_TYPE_3, 0x32170000},
> +	{OP_ZR, PRS_REG_CM_HDR_TYPE_4, 0x5},
> +	{OP_WR, PRS_REG_CM_HDR_LOOPBACK_TYPE_1, 0x12150000},
> +	{OP_WR, PRS_REG_CM_HDR_LOOPBACK_TYPE_2, 0x22150000},
> +	{OP_WR, PRS_REG_CM_HDR_LOOPBACK_TYPE_3, 0x32150000},
> +	{OP_ZR, PRS_REG_CM_HDR_LOOPBACK_TYPE_4, 0x4},
 etc. etc.

Take care.

Re: [RFC][BNX2X] .h files rewrite

[Eliezer Tamir]

(Sorry It took so long to answer, I've had to go over a lot of stuff to
make sure I'm giving you accurate answers.)

On Mon, 2007-10-29 at 01:39 -0700, David Miller wrote:
> From: "Eliezer Tamir" <eliezert@broadcom.com>
> Date: Sun, 28 Oct 2007 22:21:14 +0200
> 
> Overall things look significantly better, thanks a lot!
> 
> However, there is still one set of magic constants in here
> which I hope you can clear up:
> 
> > +static const struct raw_op init_ops[] = {
> > +#define PRS_COMMON_START        0
> > +     {OP_WR, PRS_REG_INC_VALUE, 0xf},
> > +     {OP_WR, PRS_REG_EVENT_ID_1, 0x45},
> > +     {OP_WR, PRS_REG_EVENT_ID_2, 0x84},
> > +     {OP_WR, PRS_REG_EVENT_ID_3, 0x6},
> > +     {OP_WR, PRS_REG_NO_MATCH_EVENT_ID, 0x4},
> > +     {OP_WR, PRS_REG_CM_HDR_TYPE_0, 0x0},
> > +     {OP_WR, PRS_REG_CM_HDR_TYPE_1, 0x12170000},
> > +     {OP_WR, PRS_REG_CM_HDR_TYPE_2, 0x22170000},
> > +     {OP_WR, PRS_REG_CM_HDR_TYPE_3, 0x32170000},
> > +     {OP_ZR, PRS_REG_CM_HDR_TYPE_4, 0x5},
> > +     {OP_WR, PRS_REG_CM_HDR_LOOPBACK_TYPE_1, 0x12150000},
> > +     {OP_WR, PRS_REG_CM_HDR_LOOPBACK_TYPE_2, 0x22150000},
> > +     {OP_WR, PRS_REG_CM_HDR_LOOPBACK_TYPE_3, 0x32150000},
> > +     {OP_ZR, PRS_REG_CM_HDR_LOOPBACK_TYPE_4, 0x4},
>  etc. etc.

These are the steps performed to initialize the chip and load the
microcode.

Each element in the array is an operation in the form:
{operation, chip address, value}

Where the operation is one of:
OP_WR - write a value to the chip.
OP_RD - read a register (this is a read with a side effect, usually a
clear on read register).
OP_SW - string write to the chip (parts of the microcode).
OP_SI - string write using indirect write registers.
OP_ZR - clear a range of memory.
OP_ZP - unzip and copy using DMAE.
OP_WB - string copy using DMAE.

(I will add these explanations to bnx2x_init.h)

As you can see there are quite a lot of them, that is because most of
them are setting up the microcode internal structures.
Maybe it would have been better if the microcode were to do these things
but because of size issues it currently is not an option.

We will add comments to all register definitions explaining as much as
possible about each register.

All of the registers that are related to the driver flow are accessed
explicitly inside the driver code in the main .c file.

Writes to memories are simply parts of the microcode.

However, almost all of the registers written to here, are numeric values
not bit constants. The values are derived from HW definitions (and that
was the source of the extra-ugly generated code that we replaced).

In a perfect world almost all of these would have been the reset values
of the registers. 
#include "bitching_about_HW_guys.h"

The added documentation for each register in the register file will
provide all the info that I have about the values.
I feel bad about adding several hundred lines of defines that are only
used once, and do not add any real information about the values.

Your insights would be appreciated.

Thanks,
Eliezer

Re: [RFC][BNX2X] .h files rewrite

[David Miller]

From: "Eliezer Tamir" <eliezert@broadcom.com>
Date: Wed, 31 Oct 2007 12:14:47 +0200

> I feel bad about adding several hundred lines of defines that are only
> used once, and do not add any real information about the values.

Ok, let's skip this for now.

Re: [RFC][BNX2X] .h files rewrite

[Max Asbock]

On Sun, 2007-10-28 at 22:17 +0200, Eliezer Tamir wrote:
> Dave,
> 
> Please take a look and tell me if this is better than the last attempt.
> 
> Main changes:
> 
> * Rewrite of the init code: 
> 	1. Full rewrite of the generated code into something easier to read.
> 	2. Removed zeroes by adding a zero command.
> 	3. Big chucks of data are read by the chip using the DMA engine (instead of being written by the driver).
> 	4. Gzipped more of the data.
> 
> * Removed BE microcode, the swapping is done in the driver instead.
> 
> * Rewrite of the HSI - broke it into three files bnx2x_hsi.h bnx2x_reg.h and bnx2x_fw_defs.h
> 
> * Fixed SPARSE warnings.
> 
> * Fixed all but three checkpatch.pl warnings. (IMHO they are false positives)
> 
> The diff is against latest net-2.6.
> 
> ftp link:
> ftp://Net_sys_anon@ftp1.broadcom.com/0001-bnx2x-h-file-rewrite.txt
> 
> gzipped:
> ftp://Net_sys_anon@ftp1.broadcom.com/0001-bnx2x-h-file-rewrite.txt.gz
> 
> (will also attache patch inline as reply to this email for review)
> 

I built the newest bnx2x code against the net-2.6 kernel and ran a
number of stress tests with netperf and pktgen. I did not encounter any
errors.

Max

Re: [RFC][BNX2X] .h files rewrite

[Eliezer Tamir]

On Fri, 2007-11-02 at 16:35 -0700, Max Asbock wrote:

> I built the newest bnx2x code against the net-2.6 kernel and ran a
> number of stress tests with netperf and pktgen. I did not encounter
> any
> errors.
> 
> Max
> 
> 
Thanks,
Eliezer

Re: [RFC][BNX2X] .h files rewrite

[Ram Pai]

On Sun, 2007-11-04 at 09:47 +0200, Eliezer Tamir wrote:
> On Fri, 2007-11-02 at 16:35 -0700, Max Asbock wrote:
> 
> > I built the newest bnx2x code against the net-2.6 kernel and ran a
> > number of stress tests with netperf and pktgen. I did not encounter
> > any
> > errors.
> > 
> > Max
> > 
> > 
> Thanks,

Dave/Jeff,
	
        What is the current outlook for this driver? 
	Is it ready for upstream?

thanks,
RP


> Eliezer
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC][BNX2X] .h files rewrite

[David Miller]

From: Ram Pai <linuxram@us.ibm.com>
Date: Mon, 12 Nov 2007 15:13:23 -0800

> 
> 
> On Sun, 2007-11-04 at 09:47 +0200, Eliezer Tamir wrote:
> > On Fri, 2007-11-02 at 16:35 -0700, Max Asbock wrote:
> > 
> > > I built the newest bnx2x code against the net-2.6 kernel and ran a
> > > number of stress tests with netperf and pktgen. I did not encounter
> > > any
> > > errors.
> > > 
> > > Max
> > > 
> > > 
> > Thanks,
> 
> Dave/Jeff,
> 	
>         What is the current outlook for this driver? 
> 	Is it ready for upstream?

Yes, I intend to add it for 2.6.25