On 10/15/07, Matt Sealey <[EMAIL PROTECTED]> wrote: > > Grant Likely wrote: > > On 10/15/07, Matt Sealey <[EMAIL PROTECTED]> wrote: > >> My nits: > >> > >> Grant Likely wrote: > >>> From: Sylvain Munaut <[EMAIL PROTECTED]> > >>> +static int __devinit > >>> +bcom_engine_init(void) > >> Why "bcom" and not "bestcomm"? > > > > I can type 'bcom' twice as fast. :-) bcom is a suitable shortening; > > I'm not concerned about it. > > I hate acronyms and shortening for the sake of it. > > My IDE highlights known symbols from includes and lets me tab complete them :D > > After all once all these APIs are fixed and most of the drivers are > implemented > (most of them are, already, anyway, and have been from TaskSomething to sdma_ > to bcom_ changes and major API reworks), I wonder why we have to constantly > cut every function definition down to 4 characters rhp_bjz_ywh_moo_purr() > > I'd level the same thing at bcom_eng (what's an Eng when it's at home? > English? > Engraved? Surely Engine but.. come on :) > > There's no real good need to shorten the names of things except when those > shortenings are also used in the documentation - after all, PSC is what > Freescale > call a PSC, we wouldn't be making structures called > mpc52xx_programmable_serial_controller, > that's redundant, I don't think calling it "bestcomm" (which is it's name) > over > "bcom" (which isn't) works to anyone's advantage here.
bcom is used consistently within this file and its use is unambiguous. It doesn't need to be changed for this submission. > > >>> + /* Disable COMM Bus Prefetch, apparently it's not reliable yet */ > >>> + /* FIXME: This should be done on 5200 and not 5200B ... */ > >>> + out_be16(&bcom_eng->regs->PtdCntrl, > >>> in_be16(&bcom_eng->regs->PtdCntrl) | 1); > >> This really, really shouldn't even be here, could it be moved to a platform > >> init, or switched on a PVR/SVR here? > > > > I think I'd like to leave it here for getting this series merged; it > > may not be good to have it here; but it's not dangerous either. I'm > > trying to keep churn on this series down to a minimum. > > Why not just accept the churn, and remove those two lines, and someone will > submit a patch to make it work on the 5200 in the appropriate place later? Simple; it's not my series. I'm taking the viewpoint of only changing what is critical to change to get the code in. Those 2 lines may be sub-optimal; but they are not *bad* or *dangerous* and they're easily removed later. I'm pushing this change with my maintainer hat on; not as the device driver developer and as such only making necessary changes. My view is that it is *safe* and *good* to merge this driver as is so that the FEC and other drivers can finally get unblocked. Send me a patch to change it. g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. [EMAIL PROTECTED] (403) 399-0195 _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
