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@xxxxxxxxxx>
>> Cc: Y2038 <y2038@xxxxxxxxxxxxxxxx>
>> Cc: linux-kernel@xxxxxxxxxxxxxxx
>> Cc: Arnd Bergmann <arnd@xxxxxxxx>
>> Cc: Felipe Balbi <balbi@xxxxxx>
>
> 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-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
