Subject: Re: patch for bge driver
To: None <muk@msu.edu>
From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
List: tech-kern
Date: 11/18/2006 17:23:26
muk@msu.edu wrote:

> I generated a patch with cvs diff, if a better format should be
> utilized please let me know and I will do it right.

IMO, your patch is not so large so using plain text would be better.

> +	{ BGE_CHIPID_BCM5752_A1,
> +	  BGE_QUIRK_ONLY_PHY_1|BGE_QUIRK_5705_CORE,
> +	  "BCM5751 A1" },
> +
> +	{ BGE_CHIPID_BCM5752_A2,
> +	  BGE_QUIRK_ONLY_PHY_1|BGE_QUIRK_5705_CORE,
> +	  "BCM5751 A2" },

Should these be BCM5752 (not 5751)?

>  	  "unknown BCM5752 family" },
>  
> -
>  	{ BGE_ASICREV_BCM5780,

It's better to separate such cosmetics from functional changes.

> +	if (sc->bge_chipid == BGE_CHIPID_BCM5752_A0 ||
> +	    sc->bge_chipid == BGE_CHIPID_BCM5752_A1 ||
> +	    sc->bge_chipid == BGE_CHIPID_BCM5752_A2)
> +		CSR_WRITE_4(sc, BGE_FASTBOOT_PC, 0);
> +

It looks this should be handled by a quirk flag like
BGE_QUIRK_FASTBOOT or so rather than checking each chipid.
Is there any evidence why this setting is needed on these BCM5752?

> RCS file: /cvsroot/src/sys/dev/pci/pcidevs.h,v
> retrieving revision 1.815.2.7
> diff -u -r1.815.2.7 pcidevs.h
> --- sys/dev/pci/pcidevs.h	15 Sep 2006 11:54:30 -0000	1.815.2.7
> +++ sys/dev/pci/pcidevs.h	14 Nov 2006 19:24:22 -0000

These files are generated files so you have to change pcidevs
and then regenerate pcidevs.h and pcidevs_data.h.
But I'll commit this part first.

> Thanks!

And thank you for submitting a patch :-)
---
Izumi Tsutsui