[PATCH 0/4] Introduce another options to set/get time with "ktime_t" type

[PATCH 0/4] Introduce another options to set/get time with "ktime_t" type

[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

[PATCH 1/4] ptp/chardev:Introduce another option to get/set time in ptp_clock_info structure

[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

[PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type

[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

[PATCH 3/4] ptp/pch:Replace timespec with ktime_t in ptp_pch.c

[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

[PATCH 4/4] ptp/ixp46x:Replace timespec with ktime_t in ptp_ixp46x.c

[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(&register_lock, flags);
 
-	ns = ixp_systime_read(regs);
+	*kt = ns_to_ktime(ixp_systime_read(regs));
 
 	spin_unlock_irqrestore(&register_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(&register_lock, flags);
 
-	ixp_systime_write(regs, ns);
+	ixp_systime_write(regs, ktime_to_ns(kt));
 
 	spin_unlock_irqrestore(&register_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

Re: [PATCH 1/4] ptp/chardev:Introduce another option to get/set time in ptp_clock_info structure

[Richard Cochran]

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
> 

Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type

[Richard Cochran]

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

Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type

[Richard Cochran]

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

Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type

[Arnd Bergmann]

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

Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type

[Richard Cochran]

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

Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type

[Arnd Bergmann]

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

Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type

[Richard Cochran]

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

Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type

[Richard Cochran]

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

Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type

[Richard Cochran]

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

Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type

[Arnd Bergmann]

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