On Tuesday, September 15, 2015 09:14 PM, Arnd Bergmann wrote:
> On Tuesday 15 September 2015 20:56:15 WEN Pingbo wrote:
>> The millisecond of the last second will be normal if tv_sec is
>> overflowed. But for y2038 consistency and demonstration purpose,
>> and avoiding further risks, we still need to fix it here,
>> to avoid similair problems.
>>
>> Signed-off-by: Pingbo Wen <[email protected]>
>> Cc: Y2038 <[email protected]>
>> Cc: [email protected]
>> Cc: Arnd Bergmann <[email protected]>
>> Cc: Felipe Balbi <[email protected]>
> 
> I just replied to the thread of the first mail, and it's good to see you had
> the same thought about adding the usb folks to the Cc list.
> 
> When sending a new version of a patch you have already sent before, it's
> common practice to use [PATCH v2] in the subject, so please do that for the
> next version.
> 

Ok, I thought previous thread is just draft, so I renewed one. Losing some 
history,
sorry for that.

> Your changelog text is better than the first version, but can still be 
> improved.
> I would at least mention that we want to remove all uses of 'timeval' from
> device drivers in order to better scan for y2038 problems in the drivers that
> still use them.
> 
> Also, you don't mention at all the discussion we had about real time vs.
> monotonic time for this driver. I think using monotonic time 
> (ktime_get_ts64())
> would be more appropriate here, but whichever you choose, you should explain
> in the changelog why you think that one is better than the other.
> Most of the time, we end up changing from real time to monotonic time in the
> same patch when converting a driver to avoid 32-bit time_t, so even you don't
> change it, you should explain why not.

ktime_get_ts64() is another choice. As we discussed in previous thread, only 
using
jiffies can not keep the precise, and we should also handle the jiffies 
overflowing
case, so I kept the old implementation.

> 
>>  drivers/usb/gadget/udc/dummy_hcd.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c 
>> b/drivers/usb/gadget/udc/dummy_hcd.c
>> index 1379ad4..7be721dad 100644
>> --- a/drivers/usb/gadget/udc/dummy_hcd.c
>> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
>> @@ -833,10 +833,10 @@ static const struct usb_ep_ops dummy_ep_ops = {
>>  /* there are both host and device side versions of this call ... */
>>  static int dummy_g_get_frame(struct usb_gadget *_gadget)
>>  {
>> -    struct timeval  tv;
>> +    struct timespec64 tv;
>>  
>> -    do_gettimeofday(&tv);
>> -    return tv.tv_usec / 1000;
>> +    getnstimeofday64(&tv);
>> +    return tv.tv_nsec / 1000000L;
>>  }
>>  
> 
> As in your other patch, I think the use of NSEC_PER_MSEC would make this
> slightly more understandable.
> 
>       Arnd
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to