Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with ktime_t type
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with ktime_t type
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with ktime_t type
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with ktime_t type
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with ktime_t type
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with ktime_t type
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with ktime_t type
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with ktime_t type
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with ktime_t type
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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with "ktime_t" type
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
---
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, );
+ *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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/4] ptp/clcok:Introduce the setktime/getktime interfaces with ktime_t type
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.w...@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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
