All of lore.kernel.org
 help / color / mirror / Atom feed
From: mingo@kernel.org (Ingo Molnar)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] clockevents: Add (missing) default case for switch blocks
Date: Mon, 23 Feb 2015 17:37:06 +0100	[thread overview]
Message-ID: <20150223163706.GA12551@gmail.com> (raw)
In-Reply-To: <CAKohponv=aR7Ae_z7zbnDDeJ-mCFwdxB3wR-O4boBeMKYdUJLA@mail.gmail.com>


* Viresh Kumar <viresh.kumar@linaro.org> wrote:

> On 20 February 2015 at 19:34, Ingo Molnar 
> <mingo@kernel.org> wrote:
> >
> > * Viresh Kumar <viresh.kumar@linaro.org> wrote:
> 
> >> Unused. Initially all clockevent devices are supposed 
> >> to be in this mode but later if another device 
> >> replaces an existing one, the existing one is put into 
> >> this mode.
> >
> > I'd suggest to rename it to MODE_INIT - at first glance 
> > it gave me the impression that it's some sort of API 
> > placeholder - i.e. an unused flag or so.
> 
> Sorry, if I wasn't able to clarify this earlier. The 
> impression you got at first glance is correct. And it 
> should be UNUSED only instead of INIT. Its not about if 
> the the device is initialized or not, but if it is used 
> or not. And that's why there is no callback for this in 
> the new per-mode callbacks.

Ok, could we rename it to something like DETACHED?

'UNUSED' really gives me the wrong impression - it's what 
we do for unused fields, unused ABI enumertion constants, 
etc.

> > Also, I'd suggest to rename all 'modes' to true state 
> > machine naming: STATE_INITIALIZED, STATE_SHUT_DOWN, 
> > STATE_PERIODIC, STATE_RESUMED, etc.: if these are enums 
> > for
> 
> I thought so initially and it looked like the diff will 
> be huge as all the variables for the enum, i.e. 'mode', 
> need to be renamed to 'state'..
> 
> But, if you are okay with it then I would be happy to do 
> that..

Well, how does the diffstat look like?

But if the conversion was scripted (i.e. is relatively 
secure from typos) then that's still fine IMO. Peter?

Thanks,

	Ingo

WARNING: multiple messages have this Message-ID (diff)
From: Ingo Molnar <mingo@kernel.org>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	Linaro Kernel Mailman List <linaro-kernel@lists.linaro.org>,
	Kevin Hilman <khilman@linaro.org>,
	Preeti U Murthy <preeti@linux.vnet.ibm.com>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Linaro Networking <linaro-networking@linaro.org>,
	Steven Miao <realmz6@gmail.com>, Mark Salter <msalter@redhat.com>,
	Michal Simek <monstr@monstr.eu>,
	Ralf Baechle <ralf@linux-mips.org>,
	Ley Foon Tan <lftan@altera.com>, Jonas Bonn <jonas@southpole.se>,
	"David S. Miller" <davem@davemloft.net>,
	Jeff Dike <jdike@addtoit.com>, Guan Xuetao <gxt@mprc.pku.edu.cn>
Subject: Re: [PATCH] clockevents: Add (missing) default case for switch blocks
Date: Mon, 23 Feb 2015 17:37:06 +0100	[thread overview]
Message-ID: <20150223163706.GA12551@gmail.com> (raw)
In-Reply-To: <CAKohponv=aR7Ae_z7zbnDDeJ-mCFwdxB3wR-O4boBeMKYdUJLA@mail.gmail.com>


* Viresh Kumar <viresh.kumar@linaro.org> wrote:

> On 20 February 2015 at 19:34, Ingo Molnar 
> <mingo@kernel.org> wrote:
> >
> > * Viresh Kumar <viresh.kumar@linaro.org> wrote:
> 
> >> Unused. Initially all clockevent devices are supposed 
> >> to be in this mode but later if another device 
> >> replaces an existing one, the existing one is put into 
> >> this mode.
> >
> > I'd suggest to rename it to MODE_INIT - at first glance 
> > it gave me the impression that it's some sort of API 
> > placeholder - i.e. an unused flag or so.
> 
> Sorry, if I wasn't able to clarify this earlier. The 
> impression you got at first glance is correct. And it 
> should be UNUSED only instead of INIT. Its not about if 
> the the device is initialized or not, but if it is used 
> or not. And that's why there is no callback for this in 
> the new per-mode callbacks.

Ok, could we rename it to something like DETACHED?

'UNUSED' really gives me the wrong impression - it's what 
we do for unused fields, unused ABI enumertion constants, 
etc.

> > Also, I'd suggest to rename all 'modes' to true state 
> > machine naming: STATE_INITIALIZED, STATE_SHUT_DOWN, 
> > STATE_PERIODIC, STATE_RESUMED, etc.: if these are enums 
> > for
> 
> I thought so initially and it looked like the diff will 
> be huge as all the variables for the enum, i.e. 'mode', 
> need to be renamed to 'state'..
> 
> But, if you are okay with it then I would be happy to do 
> that..

Well, how does the diffstat look like?

But if the conversion was scripted (i.e. is relatively 
secure from typos) then that's still fine IMO. Peter?

Thanks,

	Ingo

  reply	other threads:[~2015-02-23 16:37 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-20  6:32 [PATCH] clockevents: Add (missing) default case for switch blocks Viresh Kumar
2015-02-20  6:32 ` Viresh Kumar
2015-02-20  8:38 ` Ingo Molnar
2015-02-20  8:38   ` Ingo Molnar
2015-02-20  8:48   ` Peter Zijlstra
2015-02-20  8:48     ` Peter Zijlstra
2015-02-20  9:36     ` Ingo Molnar
2015-02-20  9:36       ` Ingo Molnar
2015-02-20 10:12       ` Viresh Kumar
2015-02-20 10:12         ` Viresh Kumar
2015-02-20 10:52         ` Ingo Molnar
2015-02-20 10:52           ` Ingo Molnar
2015-02-20 11:37       ` Peter Zijlstra
2015-02-20 11:37         ` Peter Zijlstra
2015-02-20 11:41         ` Ingo Molnar
2015-02-20 11:41           ` Ingo Molnar
2015-02-20 11:56           ` Viresh Kumar
2015-02-20 11:56             ` Viresh Kumar
2015-02-20 13:22             ` Ingo Molnar
2015-02-20 13:22               ` Ingo Molnar
2015-02-20 13:58               ` Viresh Kumar
2015-02-20 13:58                 ` Viresh Kumar
2015-02-20 14:04                 ` Ingo Molnar
2015-02-20 14:04                   ` Ingo Molnar
2015-02-23  5:33                   ` Viresh Kumar
2015-02-23  5:33                     ` Viresh Kumar
2015-02-23 16:37                     ` Ingo Molnar [this message]
2015-02-23 16:37                       ` Ingo Molnar
2015-02-24 11:11                       ` viresh kumar
2015-02-24 11:11                         ` viresh kumar
2015-02-24 14:54                         ` Ingo Molnar
2015-02-24 14:54                           ` Ingo Molnar
2015-02-24 15:12                           ` Viresh Kumar
2015-02-24 15:12                             ` Viresh Kumar

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=20150223163706.GA12551@gmail.com \
    --to=mingo@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.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 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.