----- Original Message ----- >Date: Wed, 11 Jun 2008 09:02:00 -0700 >From: "Garrett D'Amore" <[EMAIL PROTECTED]> >To: Alan DuBoff <[EMAIL PROTECTED]> >Cc: driver-discuss@opensolaris.org >Subject: Re: [driver-discuss] Codereview for CR# 6655415 sfe auto-negotiate > > >Generally, don't include the PSARC case number in wx comments for >integration of sfe if sfe has already been integrated (it has). > >sfe.c: line 902, extra blank line added. >sfe.c: line 1170: I guess these changes are to avoid side effects from >reads? >sfe.c: line 1188, 1243, 1296, 1450: These are not for CR 6655415, but >fix another bug, please CR it separately >sfe.c: line 1268 is yet another, separate bug fixed, and also needs its >own CR (seems a few dp38165 changes) >sfe.c: line 1644 - is this yet another bug fix? >sfe.c: line 1654-1699, this is new functionality a different part, >another CR >sfe_util.c: the changes here look like yet other, unrelated changes (and >there are more than one in here) >sfe_util.c: line 1066, looks fishy to me. why does now need to be >nonzero? suggests that maybe a problem exists *elsewhere* that this is >just a bandaid for? > > >There are a lot of changes here... too many for me to finish right now. >I got to line 1300ish in sfe_util.c. I strongly suggest you fix up your >bug lists, because at the moment there are a lot of fixes in here, and >I'm having a hard time seeing all of the reasons for all of them.
Garrett, Thank you very much for reviewing new sfe. I'll make the bug list which I found and fixed. As I'll take a little time, I'll do that at this weekend. I have changed too many lines for struggling vlan hang issue on sparc platforms. >In the future, smaller, more incremental updates, are a lot easier to >manage, rather than trying to collect up a few dozen fixes into a single >mondo-update. Like patches in Linux, it agree to divide the bunch of changes into simple changes, I think. -masa > -- Garrett > >Alan DuBoff wrote: >> We're looking for a codereview for the sfe driver, and internal folks at >> Sun can access my workspace at: >> >> /net/konocti.sfbay/export/home/builds/sfe/ >> >> For those of you playing at home and curious what a webrev is, you can >> download it at: >> >> http://www.opensolaris.org/os/community/device_drivers/files/sfe-2.6.1t21-w ebrev.tar.bz2 >> >> A webrev is produced with wx. It is included in the onbld tools which are >> available for OpenSolaris, but works slightly differently internally at >> Sun, as we use Teamware still. That is changing, as the sources move >> outside the firewall, which I hear is happening soon. >> >> Anyone internal that can offer a codereview, Masa will be appreciative. >> >> -- >> >> Alan DuBoff - Solaris x86 IHV/OEM Group >> _______________________________________________ >> driver-discuss mailing list >> driver-discuss@opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/driver-discuss >> > >_______________________________________________ >driver-discuss mailing list >driver-discuss@opensolaris.org >http://mail.opensolaris.org/mailman/listinfo/driver-discuss _______________________________________________ driver-discuss mailing list driver-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/driver-discuss
