mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: "pang.xunlei" <pang.xunlei@linaro.org>
Cc: lkml <linux-kernel@vger.kernel.org>,
	"rtc-linux@googlegroups.com" <rtc-linux@googlegroups.com>,
	xen-devel@lists.xenproject.org,
	John Stultz <john.stultz@linaro.org>,
	Alessandro Zummo <a.zummo@towertech.it>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Subject: Re: [RFC PATCH v2 03/11] time: Add rtc_tm_to_time64() safe version(using time64_t)
Date: Thu, 30 Oct 2014 21:49:38 +0100 (CET)	[thread overview]
Message-ID: <alpine.DEB.2.11.1410302118170.5308@nanos> (raw)
In-Reply-To: <CADcy93WzP-y8HXNrfx-vOCkruEdXVOhP2Up=KqW=vLuxHM7_YQ@mail.gmail.com>

On Fri, 31 Oct 2014, pang.xunlei wrote:
> On 30 October 2014 21:55, Thomas Gleixner <tglx@linutronix.de> wrote:
> > What's wrong with changing it to:
> >
> > time64_t rtc_tm_to_time64(struct rtc_time *tm)
> > {
> >         return mktime64(......);
> > }
> 
> Thank you for reminding me of this, and also a valuable principle when
> doing things.
> 
> I wanna change it to:
> void rtc_tm_to_time64(struct rtc_time *tm, time64_t *time)
> {
>     time = mktime64(......);
> }
> 
> Just keep it the similar format as rtc_time64_to_tm(), is this acceptable?

Why on earth want you do that? Just because it looks the same or what?

Come on, it's not that hard and this is not the CS 'Java for dummies'
course where you get an A for implementing 'looks the same' nonsense.

For rtc_time64_to_tm() you really want to hand in the pointer for the
result because returning the full structure is overkill as it involves
heavy stack operations.

But for rtc_tm_to_time64() this is a different story. All relevant
32bit architectures can deal with a u64 return value very well and in
most cases it creates better code.

Just do the simple experiment of implementing both, converting a dozen
drivers and comparing the resulting assembler code on the call sites
and the function itself. That should give you an objective decision
criteria instead of "wanna change just because I feel it looks better".

Another valuable principle of doing things to remember, right?

Thanks,

	tglx







  reply	other threads:[~2014-10-30 20:49 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-30 11:15 [RFC PATCH v2 00/11] Convert do_settimeofday() to use timespec64 pang.xunlei
2014-10-30 11:15 ` [RFC PATCH v2 01/11] time: Add do_settimeofday64() safe version(using timespec64) pang.xunlei
2014-10-30 13:24   ` Thomas Gleixner
2014-10-30 11:15 ` [RFC PATCH v2 02/11] time: Add mktime64() safe version(using time64_t) pang.xunlei
2014-10-30 13:43   ` Thomas Gleixner
2014-10-30 11:15 ` [RFC PATCH v2 03/11] time: Add rtc_tm_to_time64() " pang.xunlei
2014-10-30 13:55   ` Thomas Gleixner
2014-10-30 16:30     ` pang.xunlei
2014-10-30 20:49       ` Thomas Gleixner [this message]
2014-10-30 11:15 ` [RFC PATCH v2 04/11] time: Add rtc_time_to_tm64() " pang.xunlei
2014-10-30 14:07   ` Thomas Gleixner
2014-10-30 11:15 ` [RFC PATCH v2 05/11] time: Convert all users of do_settimeofday() to use do_settimeofday64() pang.xunlei
2014-10-30 14:49   ` Thomas Gleixner
2014-10-30 15:27     ` John Stultz
2014-10-30 16:06       ` Thomas Gleixner
2014-10-30 11:15 ` [RFC PATCH v2 06/11] time: Remove do_settimeofday() pang.xunlei
2014-10-30 11:15 ` [RFC PATCH v2 07/11] time: Convert alarm_set_rtc() to use timespec64 pang.xunlei
2014-10-30 14:52   ` Thomas Gleixner
2014-10-30 11:15 ` [RFC PATCH v2 08/11] time: Convert xen_read_wallclock() " pang.xunlei
     [not found]   ` <54522159.8070204@citrix.com>
2014-10-30 14:58     ` [Xen-devel] " Thomas Gleixner
2014-10-30 11:15 ` [RFC PATCH v2 09/11] time: Convert pvclock_read_wallclock() " pang.xunlei
2014-10-30 14:57   ` Thomas Gleixner
2014-10-30 11:15 ` [RFC PATCH v2 10/11] time: Convert x86_platform.set_wallclock()to " pang.xunlei
2014-10-30 11:15 ` [RFC PATCH v2 11/11] time: Convert x86_platform.get_wallclock()to " pang.xunlei

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.11.1410302118170.5308@nanos \
    --to=tglx@linutronix.de \
    --cc=a.zummo@towertech.it \
    --cc=john.stultz@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pang.xunlei@linaro.org \
    --cc=rtc-linux@googlegroups.com \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®