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