Garrett, Thank you for reviewing new sfe.
I answer your comments. I have sent the bug list seperately. ----- Original Message ----- >Date: Wed, 11 Jun 2008 09:02:00 -0700 >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. I'll remove the blank line. I'll check other extra blank lines too. >sfe.c: line 1170: I guess these changes are to avoid side effects from >reads? It is for performance. As LE_32 a macro, the original lines causes several memory refereces to the same location by a pointer, which is difficult for compilers to optimize. >sfe.c: line 1188, 1243, 1296, 1450: These are not for CR 6655415, but >fix another bug, please CR it separately They are yet another code improvements, not bugs. The rx/tx ring sizes should not be constants in rx/tx descriptor accessing methods, I think. >sfe.c: line 1268 is yet another, separate bug fixed, and also needs its >own CR (seems a few dp38165 changes) We will discuss it later. >sfe.c: line 1644 - is this yet another bug fix? Not bug, it's yet another improvement to protect PHY registers from unexpected writing. >sfe.c: line 1654-1699, this is new functionality a different part, >another CR It is yet another improvement for recent NS DP83815/83816 chipset. We'll discuss how we put it back later. >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? the timestump *now* is used to set *tx_blocked*, which is also used as a boolean that represents whether the downstream block or not. So, the timestump should not be zero. -masa >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. > >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. > > -- 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
