>-----Original Message----- >From: Andy Shevchenko [mailto:andy.shevchenko@xxxxxxxxx] >Sent: Wednesday, July 25, 2018 8:55 PM >To: Anurag Kumar Vulisha <anuragku@xxxxxxxxxx> >Cc: Felipe Balbi <balbi@xxxxxxxxxx>; Greg Kroah-Hartman ><gregkh@xxxxxxxxxxxxxxxxxxx>; v.anuragkumar@xxxxxxxxx; USB <linux- >usb@xxxxxxxxxxxxxxx>; Linux Kernel Mailing List <linux-kernel@xxxxxxxxxxxxxxx> >Subject: Re: [PATCH 4/8] usb: dwc3: implement stream transfer timeout > >On Wed, Jul 25, 2018 at 6:14 PM, Anurag Kumar Vulisha ><anuragku@xxxxxxxxxx> wrote: > >>>> +/* >>>> + * Timeout value in msecs used by stream_timeout_timer when streams >>>> +are enabled */ >>>> +#define STREAM_TIMEOUT 50 >>> >>>Perhaps, STREAM_TIMEOUT_MS 50 >>> >>>Dunno about this driver, but it's a usual practice to help reader with understanding >>>code on the first glance. >>> >> >> Since I have mentioned "msecs" in comment above describing the >STREAM_TIMEOUT, >> thought it would suffice. But if you feel I should change it, I will fix it in v2 > >But you didn't put that comment before each occurrence of this >constant in the code, right? >That's what I'm talking about. If reader looks into the code, there is >no need to understand the order of the value for timeout, since units >are part of the name. > >Of course, you may ignore this comment. > I agree with your comments , will wait for other comments on this patch series and address them altogether in v2 Thanks, Anurag Kumar Vulisha >-- >With Best Regards, >Andy Shevchenko ��.n��������+%������w��{.n�����{���)��jg��������ݢj����G�������j:+v���w�m������w�������h�����٥