Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752420AbcDRKCy (ORCPT ); Mon, 18 Apr 2016 06:02:54 -0400 Received: from mx07-00178001.pphosted.com ([62.209.51.94]:10878 "EHLO mx07-00178001.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752324AbcDRKCw convert rfc822-to-8bit (ORCPT ); Mon, 18 Apr 2016 06:02:52 -0400 From: Vincent ABRIOU To: Arnd Bergmann , "y2038@lists.linaro.org" CC: Tina Ruchandani , "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , David Airlie , Benjamin Gaignard Date: Mon, 18 Apr 2016 12:02:35 +0200 Subject: Re: [Y2038] [PATCH] drm/sti: Use 64-bit timestamps Thread-Topic: [Y2038] [PATCH] drm/sti: Use 64-bit timestamps Thread-Index: AdGZWW61PUYhr/MiQzOArZiD18TN/A== Message-ID: <5714B0BB.1020306@st.com> References: <20160413092802.GA99759@localhost> <7376001.70fALvhWdv@wuerfel> In-Reply-To: <7376001.70fALvhWdv@wuerfel> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: user-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 acceptlanguage: en-US Content-Type: text/plain; charset="Windows-1252" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-04-18_06:,, signatures=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1801 Lines: 52 On 04/17/2016 01:39 AM, Arnd Bergmann wrote: > On Wednesday 13 April 2016 02:28:02 Tina Ruchandani wrote: >> 'struct timespec' uses a 32-bit field for seconds, which >> will overflow in year 2038 and beyond. This patch is part >> of a larger attempt to remove instances of timeval, timespec >> and time_t, all of which suffer from the y2038 issue, from the >> kernel. >> >> Signed-off-by: Tina Ruchandani > > Looks good in principle. Two small points: > >> void sti_plane_update_fps(struct sti_plane *plane, >> bool new_frame, >> bool new_field) >> { >> - struct timespec now; >> + ktime_t now; >> struct sti_fps_info *fps; >> int fpks, fipks, ms_since_last, num_frames, num_fields; >> >> - getrawmonotonic(&now); >> + now = ktime_get(); > > It's unclear why the driver was using getrawmonotonic() here rather > than ktime_get_ts(). The code is fairly new, so Vincent can > probably explain this. > > If it was intentional, we should use ktime_get_raw() instead of > ktime_get(). > getrawmonotonic comes from a legacy code so the use is not intentional. Honestly, it is not clear to me the difference between monotonic and rawmonotonic. But in the debug context in which it is used, ktime_get and ktime_get_raw will deliver the same level of information we need. So implementation done by Tina is fine for me. Vincent >> @@ -76,7 +66,7 @@ void sti_plane_update_fps(struct sti_plane *plane, >> return; >> >> fps->curr_frame_counter++; >> - ms_since_last = sti_plane_timespec_ms_diff(now, fps->last_timestamp); >> + ms_since_last = ktime_to_ms(ktime_sub(now, fps->last_timestamp)); >> num_frames = fps->curr_frame_counter - fps->last_frame_counter; > > This could be expressed in a more compact way using ktime_ms_delta(). > > Arnd >