On Wed, Feb 09, 2005 at 11:01:45AM -0800, Nishanth Aravamudan wrote:
> On Wed, Feb 09, 2005 at 10:33:02AM -0800, David Brownell wrote:
> > On Tuesday 08 February 2005 10:05 pm, Nishanth Aravamudan wrote:
> > > -         USB_REQ_CLEAR_FEATURE, USB_RT_HUB, feature, 0, NULL, 0, HZ);
> > > +         USB_REQ_CLEAR_FEATURE, USB_RT_HUB, feature, 0, NULL, 0, 1000);
> > 
> > You know, changing from symbols to magic numbers is not a win.
> > People know what something involving HZ is intended to mean,
> > but 1000 is just another magic number ...
> 
> To be honest, I had a similar concern. But if the API is clear that the
> timeout parameter is in real time units (milliseconds, in this case), I
> do not think 1000 is as much of a "magic number" as HZ was, or
> minimally, it's the same. HZ always meant, since the last parameter was
> a jiffies-unit expression, the number of jiffies in a second (if I
> didn't include the "since...expression" part, HZ just means 100, or
> 1000, or whatever value it has). 1000 always means, since the last
> parameter is now a msecs-unit expression, the number of milliseconds in
> a second (if I didn't include the "since...expression" part this time,
> though, the statement is still true :) ). But I think the change you
> suggest below may make it just a little clearer... Greg, what do you
> think?
> 
> But, I would like to say, when a symbol begins to lose it's meaning,
> then I think we have to be concerned. HZ should have no correlation to
> actual time units. Especially when HZ can be changed to not correspond
> at all to the actual hardware (dynamic-HZ patch being discussed on LKML
> being a good example, from what I understand).
> 
> > Rather than 1000 how about something like "1 * MS_PER_SEC" or something
> > similar?  I especially don't like to see magic numbers added to usbcore,
> > but the same issue applies to a lot of drivers too.
> 
> *sigh* That would mean sending out all ~63 patches again, but so it
> goes. If Greg would prefer it this way, I'd be happy to comply.

No, if we say, "this paramater is in milliseconds", like you did, I
don't see the need to change this everywhere.  I think the 1000 value is
even clearer than before (no need to go remember that HZ is number of
jiffies per second, etc.)

David, you still object?

thanks,

greg k-h


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to