* [PATCH 0/4] Introduce another options to set/get time with "ktime_t" type @ 2015-03-19 5:45 Baolin Wang 2015-03-19 5:45 ` [PATCH 1/4] ptp/chardev:Introduce another option to get/set time in ptp_clock_info structure Baolin Wang ` (3 more replies) 0 siblings, 4 replies; 15+ messages in thread From: Baolin Wang @ 2015-03-19 5:45 UTC (permalink / raw) To: richardcochran; +Cc: netdev, linux-kernel, john.stultz, tglx, arnd, baolin.wang This patch series change the 32-bit time type (timespec) to the 64-bit one (ktime_t), since 32-bit time types will break in the year 2038. This patch series introduce another two optinos to set/get time with "ktime_t" type, and remove the old ones with "timespec" type. Next step will replace the other drivers' "timespec" type with "ktime_t" type. Baolin Wang (4): ptp/chardev:Introduce another option to get/set time in ptp_clock_info structure ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type ptp/pch:Replace timespec with ktime_t in ptp_pch.c ptp/ixp46x:Replace timespec with ktime_t in ptp_ixp46x.c drivers/ptp/ptp_chardev.c | 27 ++++++++++++++++++--------- drivers/ptp/ptp_clock.c | 20 ++++++++++++++++++-- drivers/ptp/ptp_ixp46x.c | 21 ++++++--------------- drivers/ptp/ptp_pch.c | 21 ++++++--------------- include/linux/ptp_clock_kernel.h | 2 ++ 5 files changed, 50 insertions(+), 41 deletions(-) -- 1.7.9.5 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/4] ptp/chardev:Introduce another option to get/set time in ptp_clock_info structure 2015-03-19 5:45 [PATCH 0/4] Introduce another options to set/get time with "ktime_t" type Baolin Wang @ 2015-03-19 5:45 ` Baolin Wang 2015-03-19 7:48 ` Richard Cochran 2015-03-19 5:45 ` [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type Baolin Wang ` (2 subsequent siblings) 3 siblings, 1 reply; 15+ messages in thread From: Baolin Wang @ 2015-03-19 5:45 UTC (permalink / raw) To: richardcochran; +Cc: netdev, linux-kernel, john.stultz, tglx, arnd, baolin.wang This patch introduces two options with "ktime_t" type to get/set time in ptp_clock_info structure that will avoid breaking in the year 2038. In ptp_chardev.c file, replace the gettime interface with getktime interface using the "ktime_t" type to get the ptp clock time. The patch's goal is to remove the original code using the "timespec" type, and I expect to be done with that in the following merge window. Signed-off-by: Baolin Wang <baolin.wang@linaro.org> --- drivers/ptp/ptp_chardev.c | 27 ++++++++++++++++++--------- include/linux/ptp_clock_kernel.h | 2 ++ 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c index f8a7609..4e14cc6 100644 --- a/drivers/ptp/ptp_chardev.c +++ b/drivers/ptp/ptp_chardev.c @@ -125,8 +125,10 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg) struct ptp_clock_info *ops = ptp->info; struct ptp_clock_time *pct; struct timespec ts; + struct timespec64 ts64; int enable, err = 0; unsigned int i, pin_index; + ktime_t kt; switch (cmd) { @@ -197,18 +199,25 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg) } pct = &sysoff->ts[0]; for (i = 0; i < sysoff->n_samples; i++) { - getnstimeofday(&ts); - pct->sec = ts.tv_sec; - pct->nsec = ts.tv_nsec; + ktime_get_real_ts64(&ts64); + pct->sec = ts64.tv_sec; + pct->nsec = ts64.tv_nsec; pct++; - ptp->info->gettime(ptp->info, &ts); - pct->sec = ts.tv_sec; - pct->nsec = ts.tv_nsec; + if (ptp->info->getktime) { + ptp->info->getktime(ptp->info, &kt); + ts64 = ktime_to_timespec64(kt); + pct->sec = ts64.tv_sec; + pct->nsec = ts64.tv_nsec; + } else { + ptp->info->gettime(ptp->info, &ts); + pct->sec = ts.tv_sec; + pct->nsec = ts.tv_nsec; + } pct++; } - getnstimeofday(&ts); - pct->sec = ts.tv_sec; - pct->nsec = ts.tv_nsec; + ktime_get_real_ts64(&ts64); + pct->sec = ts64.tv_sec; + pct->nsec = ts64.tv_nsec; if (copy_to_user((void __user *)arg, sysoff, sizeof(*sysoff))) err = -EFAULT; break; diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h index 0d8ff3f..86decc2 100644 --- a/include/linux/ptp_clock_kernel.h +++ b/include/linux/ptp_clock_kernel.h @@ -105,7 +105,9 @@ struct ptp_clock_info { int (*adjfreq)(struct ptp_clock_info *ptp, s32 delta); int (*adjtime)(struct ptp_clock_info *ptp, s64 delta); int (*gettime)(struct ptp_clock_info *ptp, struct timespec *ts); + int (*getktime)(struct ptp_clock_info *ptp, ktime_t *kt); int (*settime)(struct ptp_clock_info *ptp, const struct timespec *ts); + int (*setktime)(struct ptp_clock_info *ptp, ktime_t kt); int (*enable)(struct ptp_clock_info *ptp, struct ptp_clock_request *request, int on); int (*verify)(struct ptp_clock_info *ptp, unsigned int pin, -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] ptp/chardev:Introduce another option to get/set time in ptp_clock_info structure 2015-03-19 5:45 ` [PATCH 1/4] ptp/chardev:Introduce another option to get/set time in ptp_clock_info structure Baolin Wang @ 2015-03-19 7:48 ` Richard Cochran 0 siblings, 0 replies; 15+ messages in thread From: Richard Cochran @ 2015-03-19 7:48 UTC (permalink / raw) To: Baolin Wang; +Cc: netdev, linux-kernel, john.stultz, tglx, arnd On Thu, Mar 19, 2015 at 01:45:06PM +0800, Baolin Wang wrote: > diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h > index 0d8ff3f..86decc2 100644 > --- a/include/linux/ptp_clock_kernel.h > +++ b/include/linux/ptp_clock_kernel.h > @@ -105,7 +105,9 @@ struct ptp_clock_info { > int (*adjfreq)(struct ptp_clock_info *ptp, s32 delta); > int (*adjtime)(struct ptp_clock_info *ptp, s64 delta); > int (*gettime)(struct ptp_clock_info *ptp, struct timespec *ts); > + int (*getktime)(struct ptp_clock_info *ptp, ktime_t *kt); > int (*settime)(struct ptp_clock_info *ptp, const struct timespec *ts); > + int (*setktime)(struct ptp_clock_info *ptp, ktime_t kt); You have added alternate methods that do exactly the same thing as gettime/settime. Then, you convert just two drivers over to the new interface. I don't want to have this job half done, with some driver being forgotten along the way. Please, just change the signatures of gettime/settime and convert *all* of the drivers. After all, there are not that many drivers to convert. Thanks, Richard > int (*enable)(struct ptp_clock_info *ptp, > struct ptp_clock_request *request, int on); > int (*verify)(struct ptp_clock_info *ptp, unsigned int pin, > -- > 1.7.9.5 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type 2015-03-19 5:45 [PATCH 0/4] Introduce another options to set/get time with "ktime_t" type Baolin Wang 2015-03-19 5:45 ` [PATCH 1/4] ptp/chardev:Introduce another option to get/set time in ptp_clock_info structure Baolin Wang @ 2015-03-19 5:45 ` Baolin Wang 2015-03-19 7:54 ` Richard Cochran 2015-03-19 5:45 ` [PATCH 3/4] ptp/pch:Replace timespec with ktime_t in ptp_pch.c Baolin Wang 2015-03-19 5:45 ` [PATCH 4/4] ptp/ixp46x:Replace timespec with ktime_t in ptp_ixp46x.c Baolin Wang 3 siblings, 1 reply; 15+ messages in thread From: Baolin Wang @ 2015-03-19 5:45 UTC (permalink / raw) To: richardcochran; +Cc: netdev, linux-kernel, john.stultz, tglx, arnd, baolin.wang This patch introduces another two options to get/set time with "ktime_t" type in ptp clock operation. Original code will set/get time through the settime/gettime interfaces with "timespec" type, that will cause break issue in year 2038. And now introducing the new setktime/getktime interfaces with "ktime_t" type to use firstly. Signed-off-by: Baolin Wang <baolin.wang@linaro.org> --- drivers/ptp/ptp_clock.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c index 296b0ec..46425ad 100644 --- a/drivers/ptp/ptp_clock.c +++ b/drivers/ptp/ptp_clock.c @@ -106,14 +106,30 @@ static int ptp_clock_getres(struct posix_clock *pc, struct timespec *tp) static int ptp_clock_settime(struct posix_clock *pc, const struct timespec *tp) { + ktime_t kt; struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock); - return ptp->info->settime(ptp->info, tp); + + if (ptp->info->setktime) { + kt = timespec_to_ktime(*tp); + return ptp->info->setktime(ptp->info, kt); + } else { + return ptp->info->settime(ptp->info, tp); + } } static int ptp_clock_gettime(struct posix_clock *pc, struct timespec *tp) { + ktime_t kt; + int ret; struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock); - return ptp->info->gettime(ptp->info, tp); + + if (ptp->info->getktime) { + ret = ptp->info->getktime(ptp->info, &kt); + *tp = ktime_to_timespec(kt); + return ret; + } else { + return ptp->info->gettime(ptp->info, tp); + } } static int ptp_clock_adjtime(struct posix_clock *pc, struct timex *tx) -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type 2015-03-19 5:45 ` [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type Baolin Wang @ 2015-03-19 7:54 ` Richard Cochran [not found] ` <CAMz4kuKyDFCqd-LfoSKJf+tkXdzzLwtPJdp-nF6NbKU8W5HHpQ@mail.gmail.com> 0 siblings, 1 reply; 15+ messages in thread From: Richard Cochran @ 2015-03-19 7:54 UTC (permalink / raw) To: Baolin Wang; +Cc: netdev, linux-kernel, john.stultz, tglx, arnd On Thu, Mar 19, 2015 at 01:45:07PM +0800, Baolin Wang wrote: > diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c > index 296b0ec..46425ad 100644 > --- a/drivers/ptp/ptp_clock.c > +++ b/drivers/ptp/ptp_clock.c > @@ -106,14 +106,30 @@ static int ptp_clock_getres(struct posix_clock *pc, struct timespec *tp) > > static int ptp_clock_settime(struct posix_clock *pc, const struct timespec *tp) > { > + ktime_t kt; > struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock); > - return ptp->info->settime(ptp->info, tp); > + > + if (ptp->info->setktime) { > + kt = timespec_to_ktime(*tp); > + return ptp->info->setktime(ptp->info, kt); > + } else { > + return ptp->info->settime(ptp->info, tp); > + } This conditional is just the kind of thing I *don't* want to see. In Documentation/ptp/ptp.txt we read: ** Writing clock drivers Clock drivers include include/linux/ptp_clock_kernel.h and register themselves by presenting a 'struct ptp_clock_info' to the registration method. Clock drivers must implement all of the functions in the interface. If a clock does not offer a particular ancillary feature, then the driver should just return -EOPNOTSUPP from those functions. Please don't litter the code with tests for NULL methods everywhere. Thanks, Richard ^ permalink raw reply [flat|nested] 15+ messages in thread
[parent not found: <CAMz4kuKyDFCqd-LfoSKJf+tkXdzzLwtPJdp-nF6NbKU8W5HHpQ@mail.gmail.com>]
* Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type [not found] ` <CAMz4kuKyDFCqd-LfoSKJf+tkXdzzLwtPJdp-nF6NbKU8W5HHpQ@mail.gmail.com> @ 2015-03-20 6:26 ` Richard Cochran 2015-03-20 13:43 ` Arnd Bergmann 0 siblings, 1 reply; 15+ messages in thread From: Richard Cochran @ 2015-03-20 6:26 UTC (permalink / raw) To: Baolin Wang; +Cc: netdev, linux-kernel, John Stultz, tglx, Arnd Bergmann On Fri, Mar 20, 2015 at 09:54:05AM +0800, Baolin Wang wrote: > Next patch series will contain all of the drivers which need to be changed. > But i think the conditional in ptp_clock.c can still in there. Why? > Because our plan is once all the drivers are converted, i will remove the > conditional, along with the original function pointer. > Is that OK? Thanks! I want to avoid a patch series that introduces something, only to remove it later on. Sometimes you have to do that way for a complex transformation, but this case is rather simple. You can change the gettime signature in one patch, and the settime in a second patch. Thanks, Richard ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type 2015-03-20 6:26 ` Richard Cochran @ 2015-03-20 13:43 ` Arnd Bergmann 2015-03-20 16:49 ` Richard Cochran 0 siblings, 1 reply; 15+ messages in thread From: Arnd Bergmann @ 2015-03-20 13:43 UTC (permalink / raw) To: Richard Cochran; +Cc: Baolin Wang, netdev, linux-kernel, John Stultz, tglx On Friday 20 March 2015, Richard Cochran wrote: > On Fri, Mar 20, 2015 at 09:54:05AM +0800, Baolin Wang wrote: > > Next patch series will contain all of the drivers which need to be changed. > > But i think the conditional in ptp_clock.c can still in there. > > Why? > > > Because our plan is once all the drivers are converted, i will remove the > > conditional, along with the original function pointer. > > Is that OK? Thanks! > > I want to avoid a patch series that introduces something, only to > remove it later on. Sometimes you have to do that way for a complex > transformation, but this case is rather simple. > > You can change the gettime signature in one patch, and the settime in > a second patch. We normally try to avoid doing those global API changes across many drivers that are maintained by different people. Introducing the new API first is the easiest way to get the per-driver patches reviewed individually by the respective maintainers. Doing gettime separately from settime would be rather silly here, so trying to avoid the conditional would mean doing a single large patch across all drivers. I do agree however that we should merge the entire series at once so we end up with a reasonable state afterwards, and we only need the conditional in order to have a bisectable git history. Arnd ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type 2015-03-20 13:43 ` Arnd Bergmann @ 2015-03-20 16:49 ` Richard Cochran 2015-03-21 1:16 ` Arnd Bergmann 2015-03-21 9:21 ` Richard Cochran 0 siblings, 2 replies; 15+ messages in thread From: Richard Cochran @ 2015-03-20 16:49 UTC (permalink / raw) To: Arnd Bergmann; +Cc: Baolin Wang, netdev, linux-kernel, John Stultz, tglx On Fri, Mar 20, 2015 at 02:43:42PM +0100, Arnd Bergmann wrote: > Doing gettime separately from settime would be rather silly here, so trying > to avoid the conditional would mean doing a single large patch across all > drivers. There is really no need for any dancing around here. There are seventeen users total. drivers/net/ethernet/adi/bfin_mac.c drivers/net/ethernet/amd/xgbe/xgbe-ptp.c drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c drivers/net/ethernet/broadcom/tg3.h drivers/net/ethernet/freescale/fec_ptp.c drivers/net/ethernet/freescale/gianfar_ptp.c drivers/net/ethernet/intel/e1000e/ptp.c drivers/net/ethernet/intel/fm10k/fm10k_ptp.c drivers/net/ethernet/intel/i40e/i40e_ptp.c drivers/net/ethernet/intel/igb/igb_ptp.c drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c drivers/net/ethernet/mellanox/mlx4/en_clock.c drivers/net/ethernet/sfc/ptp.c drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c drivers/net/ethernet/ti/cpts.c drivers/net/ethernet/tile/tilegx.c drivers/net/phy/dp83640.c Instead of changing to ktime_t, just use timespec64 instead. That way, each change will be a couple of lines per file. > I do agree however that we should merge the entire series at once so > we end up with a reasonable state afterwards, and we only need the conditional > in order to have a bisectable git history. It is still bisectable with one or two patches. Thanks, Richard ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type 2015-03-20 16:49 ` Richard Cochran @ 2015-03-21 1:16 ` Arnd Bergmann 2015-03-21 7:24 ` Richard Cochran 2015-03-21 9:21 ` Richard Cochran 1 sibling, 1 reply; 15+ messages in thread From: Arnd Bergmann @ 2015-03-21 1:16 UTC (permalink / raw) To: Richard Cochran; +Cc: Baolin Wang, netdev, linux-kernel, John Stultz, tglx On Friday 20 March 2015, Richard Cochran wrote: > Instead of changing to ktime_t, just use timespec64 instead. That > way, each change will be a couple of lines per file. This was the first idea, but it seems a bit silly when all the drivers use a 64-bit nanosecond value just like ktime_t. While both of the current users require a timespec at the moment, it's possible that there would one day be a third user that actually can make sense of a ktime_t, and then we'd avoid the expensive back-and-forth conversion. For now, using ktime_t in the interface merely simplifies the drivers by moving the conversion into the subsystem, but it is not any more or less efficient than the previous method. > > I do agree however that we should merge the entire series at once so > > we end up with a reasonable state afterwards, and we only need the conditional > > in order to have a bisectable git history. > > It is still bisectable with one or two patches. Of course, but it would be rather bad style. Arnd ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type 2015-03-21 1:16 ` Arnd Bergmann @ 2015-03-21 7:24 ` Richard Cochran 2015-03-21 16:52 ` Richard Cochran 2015-03-22 2:47 ` Arnd Bergmann 0 siblings, 2 replies; 15+ messages in thread From: Richard Cochran @ 2015-03-21 7:24 UTC (permalink / raw) To: Arnd Bergmann; +Cc: Baolin Wang, netdev, linux-kernel, John Stultz, tglx On Sat, Mar 21, 2015 at 02:16:41AM +0100, Arnd Bergmann wrote: > This was the first idea, but it seems a bit silly when all the drivers > use a 64-bit nanosecond value just like ktime_t. Not true of all drivers. In fact, the most capable devices (phyter and i210) have a split representation. > While both of the > current users require a timespec at the moment, it's possible that > there would one day be a third user that actually can make sense of > a ktime_t, and then we'd avoid the expensive back-and-forth conversion. Speculative, and has nothing to do with the 2038 issue. Let us solve that problem first. > For now, using ktime_t in the interface merely simplifies the drivers > by moving the conversion into the subsystem, Right now we have 17 drivers, tested and debugged, that perform the conversion for the particular hardware correctly. Who is going to test all of the "unconverted" code? Baolin? > but it is not any more > or less efficient than the previous method. Right, so no point in changing it. > Of course, but it would be rather bad style. Introducing useless code just to remove it again is also bad style. I disagree with the approach presented here. The problem at hand is the 2038 issue. Let's fix that first, in the easiest way, with the least churn, namely by using timespec64 in place of timespec. Once that is done, we can change over to ktime_t, if and when the need arises. Thanks, Richard ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type 2015-03-21 7:24 ` Richard Cochran @ 2015-03-21 16:52 ` Richard Cochran 2015-03-22 2:47 ` Arnd Bergmann 1 sibling, 0 replies; 15+ messages in thread From: Richard Cochran @ 2015-03-21 16:52 UTC (permalink / raw) To: Arnd Bergmann; +Cc: Baolin Wang, netdev, linux-kernel, John Stultz, tglx On Sat, Mar 21, 2015 at 08:24:09AM +0100, Richard Cochran wrote: > > I disagree with the approach presented here. The problem at hand is > the 2038 issue. Let's fix that first, in the easiest way, with the > least churn, namely by using timespec64 in place of timespec. Once > that is done, we can change over to ktime_t, if and when the need > arises. Just for laughs, I hacked up the change from timespec to timespec64, to get an idea of the dimension. The diffstat: drivers/net/ethernet/adi/bfin_mac.c | 4 ++-- drivers/net/ethernet/amd/xgbe/xgbe-ptp.c | 8 ++++---- drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 4 ++-- drivers/net/ethernet/broadcom/tg3.c | 6 +++--- drivers/net/ethernet/freescale/fec_ptp.c | 4 ++-- drivers/net/ethernet/freescale/gianfar_ptp.c | 8 ++++---- drivers/net/ethernet/intel/e1000e/ptp.c | 10 +++++----- drivers/net/ethernet/intel/fm10k/fm10k_ptp.c | 8 ++++---- drivers/net/ethernet/intel/i40e/i40e_ptp.c | 16 ++++++++++------ drivers/net/ethernet/intel/igb/igb_ptp.c | 22 +++++++++++++--------- drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c | 6 +++--- drivers/net/ethernet/mellanox/mlx4/en_clock.c | 6 +++--- drivers/net/ethernet/sfc/ptp.c | 18 +++++++++--------- drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c | 4 ++-- drivers/net/ethernet/ti/cpts.c | 8 ++++---- drivers/net/ethernet/tile/tilegx.c | 11 +++++++---- drivers/net/phy/dp83640.c | 7 ++++--- drivers/ptp/ptp_chardev.c | 6 +++--- drivers/ptp/ptp_ixp46x.c | 4 ++-- drivers/ptp/ptp_pch.c | 4 ++-- include/linux/ptp_clock_kernel.h | 4 ++-- 21 files changed, 90 insertions(+), 78 deletions(-) compares favorably with Baolin's 5 files changed, 50 insertions(+), 41 deletions(-) considering he adapted only two drivers out of nineteen. This exercise showed me that we really do need to convert the drivers one by one. Some of these drivers have 2038 issues at the hardware level, for example because the register level representation lacks the needed range. Fixing the PHC API won't solve those problems. I want flag these drivers now, so that they can be fixed later on. Already I had been thinking about how to fix the phyter, which has a 32 seconds counter. The solution I came up with will also help some of the other drivers, so it will become part of the core. However, for that to work, I really do want to keep the timespec64 representation. I'll reformat my changes into a proper series, to show what I mean... Thanks, Richard ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type 2015-03-21 7:24 ` Richard Cochran 2015-03-21 16:52 ` Richard Cochran @ 2015-03-22 2:47 ` Arnd Bergmann 1 sibling, 0 replies; 15+ messages in thread From: Arnd Bergmann @ 2015-03-22 2:47 UTC (permalink / raw) To: Richard Cochran; +Cc: Baolin Wang, netdev, linux-kernel, John Stultz, tglx On Saturday 21 March 2015, Richard Cochran wrote: > On Sat, Mar 21, 2015 at 02:16:41AM +0100, Arnd Bergmann wrote: > > This was the first idea, but it seems a bit silly when all the drivers > > use a 64-bit nanosecond value just like ktime_t. > > Not true of all drivers. In fact, the most capable devices (phyter > and i210) have a split representation. Ok, sorry for missing those earlier, I thought I'd looked at all of them and not found any that did it like this. > > but it is not any more > > or less efficient than the previous method. > > Right, so no point in changing it. > > > Of course, but it would be rather bad style. > > Introducing useless code just to remove it again is also bad style. > > I disagree with the approach presented here. The problem at hand is > the 2038 issue. Let's fix that first, in the easiest way, with the > least churn, namely by using timespec64 in place of timespec. Once > that is done, we can change over to ktime_t, if and when the need > arises. Ok, fair enough. I only saw this mail now after replying on the longer series, consider my comment on ktime_t withdrawn. Arnd ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type 2015-03-20 16:49 ` Richard Cochran 2015-03-21 1:16 ` Arnd Bergmann @ 2015-03-21 9:21 ` Richard Cochran 1 sibling, 0 replies; 15+ messages in thread From: Richard Cochran @ 2015-03-21 9:21 UTC (permalink / raw) To: Arnd Bergmann; +Cc: Baolin Wang, netdev, linux-kernel, John Stultz, tglx On Fri, Mar 20, 2015 at 05:49:51PM +0100, Richard Cochran wrote: > There is really no need for any dancing around here. There are > seventeen users total. > > drivers/net/ethernet/adi/bfin_mac.c > drivers/net/ethernet/amd/xgbe/xgbe-ptp.c > drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c > drivers/net/ethernet/broadcom/tg3.h > drivers/net/ethernet/freescale/fec_ptp.c > drivers/net/ethernet/freescale/gianfar_ptp.c > drivers/net/ethernet/intel/e1000e/ptp.c > drivers/net/ethernet/intel/fm10k/fm10k_ptp.c > drivers/net/ethernet/intel/i40e/i40e_ptp.c > drivers/net/ethernet/intel/igb/igb_ptp.c > drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c > drivers/net/ethernet/mellanox/mlx4/en_clock.c > drivers/net/ethernet/sfc/ptp.c > drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c > drivers/net/ethernet/ti/cpts.c > drivers/net/ethernet/tile/tilegx.c > drivers/net/phy/dp83640.c And with these two, drivers/ptp/ptp_ixp46x.c drivers/ptp/ptp_pch.c there are nineteen total. Thanks, Richard ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/4] ptp/pch:Replace timespec with ktime_t in ptp_pch.c 2015-03-19 5:45 [PATCH 0/4] Introduce another options to set/get time with "ktime_t" type Baolin Wang 2015-03-19 5:45 ` [PATCH 1/4] ptp/chardev:Introduce another option to get/set time in ptp_clock_info structure Baolin Wang 2015-03-19 5:45 ` [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type Baolin Wang @ 2015-03-19 5:45 ` Baolin Wang 2015-03-19 5:45 ` [PATCH 4/4] ptp/ixp46x:Replace timespec with ktime_t in ptp_ixp46x.c Baolin Wang 3 siblings, 0 replies; 15+ messages in thread From: Baolin Wang @ 2015-03-19 5:45 UTC (permalink / raw) To: richardcochran; +Cc: netdev, linux-kernel, john.stultz, tglx, arnd, baolin.wang This patch changes the 32-bit time type (timespec) to the 64-bit one (ktime_t), since 32-bit time types will break in the year 2038. This patch implements the getktime/setktime interfaces with "ktime_t" type, and removed the gettime/settime interfaces with "timespec" type in ptp_pch.c file. Signed-off-by: Baolin Wang <baolin.wang@linaro.org> --- drivers/ptp/ptp_pch.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/drivers/ptp/ptp_pch.c b/drivers/ptp/ptp_pch.c index 2554872..c759225 100644 --- a/drivers/ptp/ptp_pch.c +++ b/drivers/ptp/ptp_pch.c @@ -449,36 +449,27 @@ static int ptp_pch_adjtime(struct ptp_clock_info *ptp, s64 delta) return 0; } -static int ptp_pch_gettime(struct ptp_clock_info *ptp, struct timespec *ts) +static int ptp_pch_getktime(struct ptp_clock_info *ptp, ktime_t *kt) { - u64 ns; - u32 remainder; unsigned long flags; struct pch_dev *pch_dev = container_of(ptp, struct pch_dev, caps); struct pch_ts_regs __iomem *regs = pch_dev->regs; spin_lock_irqsave(&pch_dev->register_lock, flags); - ns = pch_systime_read(regs); + *kt = ns_to_ktime(pch_systime_read(regs)); spin_unlock_irqrestore(&pch_dev->register_lock, flags); - ts->tv_sec = div_u64_rem(ns, 1000000000, &remainder); - ts->tv_nsec = remainder; return 0; } -static int ptp_pch_settime(struct ptp_clock_info *ptp, - const struct timespec *ts) +static int ptp_pch_setktime(struct ptp_clock_info *ptp, ktime_t kt) { - u64 ns; unsigned long flags; struct pch_dev *pch_dev = container_of(ptp, struct pch_dev, caps); struct pch_ts_regs __iomem *regs = pch_dev->regs; - ns = ts->tv_sec * 1000000000ULL; - ns += ts->tv_nsec; - spin_lock_irqsave(&pch_dev->register_lock, flags); - pch_systime_write(regs, ns); + pch_systime_write(regs, ktime_to_ns(kt)); spin_unlock_irqrestore(&pch_dev->register_lock, flags); return 0; @@ -518,8 +509,8 @@ static struct ptp_clock_info ptp_pch_caps = { .pps = 0, .adjfreq = ptp_pch_adjfreq, .adjtime = ptp_pch_adjtime, - .gettime = ptp_pch_gettime, - .settime = ptp_pch_settime, + .getktime = ptp_pch_getktime, + .setktime = ptp_pch_setktime, .enable = ptp_pch_enable, }; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/4] ptp/ixp46x:Replace timespec with ktime_t in ptp_ixp46x.c 2015-03-19 5:45 [PATCH 0/4] Introduce another options to set/get time with "ktime_t" type Baolin Wang ` (2 preceding siblings ...) 2015-03-19 5:45 ` [PATCH 3/4] ptp/pch:Replace timespec with ktime_t in ptp_pch.c Baolin Wang @ 2015-03-19 5:45 ` Baolin Wang 3 siblings, 0 replies; 15+ messages in thread From: Baolin Wang @ 2015-03-19 5:45 UTC (permalink / raw) To: richardcochran; +Cc: netdev, linux-kernel, john.stultz, tglx, arnd, baolin.wang This patch changes the 32-bit time type (timespec) to the 64-bit one (ktime_t), since 32-bit time types will break in the year 2038. This patch implements the getktime/setktime interfaces with "ktime_t" type, and removes the gettime/settime interfaces with "timespec" type in ptp_ixp46x.c file. Signed-off-by: Baolin Wang <baolin.wang@linaro.org> --- drivers/ptp/ptp_ixp46x.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/drivers/ptp/ptp_ixp46x.c b/drivers/ptp/ptp_ixp46x.c index 604d340..102b673 100644 --- a/drivers/ptp/ptp_ixp46x.c +++ b/drivers/ptp/ptp_ixp46x.c @@ -175,39 +175,30 @@ static int ptp_ixp_adjtime(struct ptp_clock_info *ptp, s64 delta) return 0; } -static int ptp_ixp_gettime(struct ptp_clock_info *ptp, struct timespec *ts) +static int ptp_ixp_getktime(struct ptp_clock_info *ptp, ktime_t *kt) { - u64 ns; - u32 remainder; unsigned long flags; struct ixp_clock *ixp_clock = container_of(ptp, struct ixp_clock, caps); struct ixp46x_ts_regs *regs = ixp_clock->regs; spin_lock_irqsave(®ister_lock, flags); - ns = ixp_systime_read(regs); + *kt = ns_to_ktime(ixp_systime_read(regs)); spin_unlock_irqrestore(®ister_lock, flags); - ts->tv_sec = div_u64_rem(ns, 1000000000, &remainder); - ts->tv_nsec = remainder; return 0; } -static int ptp_ixp_settime(struct ptp_clock_info *ptp, - const struct timespec *ts) +static int ptp_ixp_setktime(struct ptp_clock_info *ptp, ktime_t kt) { - u64 ns; unsigned long flags; struct ixp_clock *ixp_clock = container_of(ptp, struct ixp_clock, caps); struct ixp46x_ts_regs *regs = ixp_clock->regs; - ns = ts->tv_sec * 1000000000ULL; - ns += ts->tv_nsec; - spin_lock_irqsave(®ister_lock, flags); - ixp_systime_write(regs, ns); + ixp_systime_write(regs, ktime_to_ns(kt)); spin_unlock_irqrestore(®ister_lock, flags); @@ -248,8 +239,8 @@ static struct ptp_clock_info ptp_ixp_caps = { .pps = 0, .adjfreq = ptp_ixp_adjfreq, .adjtime = ptp_ixp_adjtime, - .gettime = ptp_ixp_gettime, - .settime = ptp_ixp_settime, + .getktime = ptp_ixp_getktime, + .setktime = ptp_ixp_setktime, .enable = ptp_ixp_enable, }; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-03-22 2:47 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-03-19 5:45 [PATCH 0/4] Introduce another options to set/get time with "ktime_t" type Baolin Wang 2015-03-19 5:45 ` [PATCH 1/4] ptp/chardev:Introduce another option to get/set time in ptp_clock_info structure Baolin Wang 2015-03-19 7:48 ` Richard Cochran 2015-03-19 5:45 ` [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type Baolin Wang 2015-03-19 7:54 ` Richard Cochran [not found] ` <CAMz4kuKyDFCqd-LfoSKJf+tkXdzzLwtPJdp-nF6NbKU8W5HHpQ@mail.gmail.com> 2015-03-20 6:26 ` Richard Cochran 2015-03-20 13:43 ` Arnd Bergmann 2015-03-20 16:49 ` Richard Cochran 2015-03-21 1:16 ` Arnd Bergmann 2015-03-21 7:24 ` Richard Cochran 2015-03-21 16:52 ` Richard Cochran 2015-03-22 2:47 ` Arnd Bergmann 2015-03-21 9:21 ` Richard Cochran 2015-03-19 5:45 ` [PATCH 3/4] ptp/pch:Replace timespec with ktime_t in ptp_pch.c Baolin Wang 2015-03-19 5:45 ` [PATCH 4/4] ptp/ixp46x:Replace timespec with ktime_t in ptp_ixp46x.c Baolin Wang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.