lkml.org 
[lkml]   [2015]   [Sep]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [Y2038] [PATCH] dummy_hcd: replace timeval with timespec64
From
Date


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 <pingbo.wen@linaro.org>
>> Cc: Y2038 <y2038@lists.linaro.org>
>> Cc: linux-kernel@vger.kernel.org
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Felipe Balbi <balbi@ti.com>
>
> 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
>


\
 
 \ /
  Last update: 2015-09-15 16:01    [W:0.055 / U:0.980 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site