Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-24 Thread Viresh Kumar
On 24 February 2015 at 20:24, Ingo Molnar wrote: > And here's the diffstat: Sorry for not providing that :( > arch/arm/common/bL_switcher.c |8 +-- > include/linux/clockchips.h| 48 -- > kernel/time/clockevents.c | 109 >

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-24 Thread Ingo Molnar
* viresh kumar wrote: > On Monday 23 February 2015 10:07 PM, Ingo Molnar wrote:> > > * Viresh Kumar wrote: > > > 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, >

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-24 Thread viresh kumar
On Monday 23 February 2015 10:07 PM, Ingo Molnar wrote:> > * Viresh Kumar wrote: > 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. Sure. >>> Also, I'd suggest to

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-24 Thread viresh kumar
On Monday 23 February 2015 10:07 PM, Ingo Molnar wrote: * Viresh Kumar viresh.ku...@linaro.org wrote: 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. Sure. Also,

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-24 Thread Viresh Kumar
On 24 February 2015 at 20:24, Ingo Molnar mi...@kernel.org wrote: And here's the diffstat: Sorry for not providing that :( arch/arm/common/bL_switcher.c |8 +-- include/linux/clockchips.h| 48 -- kernel/time/clockevents.c | 109

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-24 Thread Ingo Molnar
* viresh kumar viresh.ku...@linaro.org wrote: On Monday 23 February 2015 10:07 PM, Ingo Molnar wrote: * Viresh Kumar viresh.ku...@linaro.org wrote: Ok, could we rename it to something like DETACHED? 'UNUSED' really gives me the wrong impression - it's what we do for unused fields,

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-23 Thread Ingo Molnar
* Viresh Kumar wrote: > On 20 February 2015 at 19:34, Ingo Molnar > wrote: > > > > * Viresh Kumar 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

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-23 Thread Ingo Molnar
* Viresh Kumar viresh.ku...@linaro.org wrote: On 20 February 2015 at 19:34, Ingo Molnar mi...@kernel.org wrote: * Viresh Kumar viresh.ku...@linaro.org wrote: Unused. Initially all clockevent devices are supposed to be in this mode but later if another device replaces an existing

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-22 Thread Viresh Kumar
On 20 February 2015 at 19:34, Ingo Molnar wrote: > > * Viresh Kumar 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 -

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-22 Thread Viresh Kumar
On 20 February 2015 at 19:34, Ingo Molnar mi...@kernel.org wrote: * Viresh Kumar viresh.ku...@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

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-20 Thread Ingo Molnar
* Viresh Kumar wrote: > On 20 February 2015 at 18:52, Ingo Molnar wrote: > > > > * Viresh Kumar wrote: > >> + CLOCK_EVT_DEV_MODE_UNUSED = 0, > > > > What is 'unused' - not initialized yet? > > Unused. Initially all clockevent devices are supposed to > be in this mode but later if

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-20 Thread Viresh Kumar
On 20 February 2015 at 18:52, Ingo Molnar wrote: > > * Viresh Kumar wrote: >> + CLOCK_EVT_DEV_MODE_UNUSED = 0, > > What is 'unused' - not initialized yet? Unused. Initially all clockevent devices are supposed to be in this mode but later if another device replaces an existing one, the

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-20 Thread Ingo Molnar
* Viresh Kumar wrote: > On 20 February 2015 at 17:11, Ingo Molnar wrote: > > > > * Peter Zijlstra wrote: > > >> Maybe we should break that enum into two; one for devices > >> and one for the core interface and avoid the problem that > >> way. > > > > Yeah, that would do the trick. > >

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-20 Thread Viresh Kumar
On 20 February 2015 at 17:11, Ingo Molnar wrote: > > * Peter Zijlstra wrote: >> Maybe we should break that enum into two; one for devices >> and one for the core interface and avoid the problem that >> way. > > Yeah, that would do the trick. Thanks for your suggestions. Just to confirm (before

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-20 Thread Ingo Molnar
* Peter Zijlstra wrote: > On Fri, Feb 20, 2015 at 10:36:59AM +0100, Ingo Molnar wrote: > > > But it does mean we need to be able to add values to > > > the enum. > > > > So I'm confused: if we are using proper callbacks (like > > my example outlined) , why is a 'mode enum' needed at > >

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-20 Thread Peter Zijlstra
On Fri, Feb 20, 2015 at 10:36:59AM +0100, Ingo Molnar wrote: > > But it does mean we need to be able to add values to the > > enum. > > So I'm confused: if we are using proper callbacks (like my > example outlined) , why is a 'mode enum' needed at all? Ah, its because the enum is shared

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-20 Thread Ingo Molnar
* Viresh Kumar wrote: > > So why is a 'default' mode needed then? It makes the > > addition of new modes to the legacy handler easier, > > which looks backwards. > > The requirement was to add another mode ONESHOT_STOPPED > [1], to be supported only by the new per-mode callbacks.. Why

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-20 Thread Viresh Kumar
On 20 February 2015 at 15:06, Ingo Molnar wrote: > > * Peter Zijlstra wrote: > >> > So this whole approach looks fragile for several reasons: >> > >> >- 'mode setting' callbacks are just bad by design >> > because they mix several functions into the same entry >> > point,

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-20 Thread Ingo Molnar
* Peter Zijlstra wrote: > > So this whole approach looks fragile for several reasons: > > > >- 'mode setting' callbacks are just bad by design > > because they mix several functions into the same entry > > point, complicating the handler functions > > unnecessarily. We

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-20 Thread Peter Zijlstra
On Fri, Feb 20, 2015 at 09:38:42AM +0100, Ingo Molnar wrote: > > * Viresh Kumar wrote: > > > Many clockevent drivers are using a switch block for handling modes in their > > ->set_mode() callback. Some of these do not have a 'default' case and > > adding a > > new mode in the 'enum

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-20 Thread Ingo Molnar
* Viresh Kumar wrote: > Many clockevent drivers are using a switch block for handling modes in their > ->set_mode() callback. Some of these do not have a 'default' case and adding a > new mode in the 'enum clock_event_mode', starts giving following warnings for > these platforms about unhandled

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-20 Thread Viresh Kumar
On 20 February 2015 at 17:11, Ingo Molnar mi...@kernel.org wrote: * Peter Zijlstra pet...@infradead.org wrote: Maybe we should break that enum into two; one for devices and one for the core interface and avoid the problem that way. Yeah, that would do the trick. Thanks for your

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-20 Thread Ingo Molnar
* Viresh Kumar viresh.ku...@linaro.org wrote: So why is a 'default' mode needed then? It makes the addition of new modes to the legacy handler easier, which looks backwards. The requirement was to add another mode ONESHOT_STOPPED [1], to be supported only by the new per-mode

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-20 Thread Peter Zijlstra
On Fri, Feb 20, 2015 at 10:36:59AM +0100, Ingo Molnar wrote: But it does mean we need to be able to add values to the enum. So I'm confused: if we are using proper callbacks (like my example outlined) , why is a 'mode enum' needed at all? Ah, its because the enum is shared between two

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-20 Thread Ingo Molnar
* Peter Zijlstra pet...@infradead.org wrote: On Fri, Feb 20, 2015 at 10:36:59AM +0100, Ingo Molnar wrote: But it does mean we need to be able to add values to the enum. So I'm confused: if we are using proper callbacks (like my example outlined) , why is a 'mode enum' needed at

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-20 Thread Peter Zijlstra
On Fri, Feb 20, 2015 at 09:38:42AM +0100, Ingo Molnar wrote: * Viresh Kumar viresh.ku...@linaro.org wrote: Many clockevent drivers are using a switch block for handling modes in their -set_mode() callback. Some of these do not have a 'default' case and adding a new mode in the 'enum

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-20 Thread Ingo Molnar
* Viresh Kumar viresh.ku...@linaro.org wrote: Many clockevent drivers are using a switch block for handling modes in their -set_mode() callback. Some of these do not have a 'default' case and adding a new mode in the 'enum clock_event_mode', starts giving following warnings for these

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-20 Thread Ingo Molnar
* Peter Zijlstra pet...@infradead.org wrote: So this whole approach looks fragile for several reasons: - 'mode setting' callbacks are just bad by design because they mix several functions into the same entry point, complicating the handler functions unnecessarily.

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-20 Thread Viresh Kumar
On 20 February 2015 at 15:06, Ingo Molnar mi...@kernel.org wrote: * Peter Zijlstra pet...@infradead.org wrote: So this whole approach looks fragile for several reasons: - 'mode setting' callbacks are just bad by design because they mix several functions into the same entry

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-20 Thread Ingo Molnar
* Viresh Kumar viresh.ku...@linaro.org wrote: On 20 February 2015 at 17:11, Ingo Molnar mi...@kernel.org wrote: * Peter Zijlstra pet...@infradead.org wrote: Maybe we should break that enum into two; one for devices and one for the core interface and avoid the problem that way.

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-20 Thread Viresh Kumar
On 20 February 2015 at 18:52, Ingo Molnar mi...@kernel.org wrote: * Viresh Kumar viresh.ku...@linaro.org wrote: + CLOCK_EVT_DEV_MODE_UNUSED = 0, What is 'unused' - not initialized yet? Unused. Initially all clockevent devices are supposed to be in this mode but later if another device

Re: [PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-20 Thread Ingo Molnar
* Viresh Kumar viresh.ku...@linaro.org wrote: On 20 February 2015 at 18:52, Ingo Molnar mi...@kernel.org wrote: * Viresh Kumar viresh.ku...@linaro.org wrote: + CLOCK_EVT_DEV_MODE_UNUSED = 0, What is 'unused' - not initialized yet? Unused. Initially all clockevent devices are

[PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-19 Thread Viresh Kumar
Many clockevent drivers are using a switch block for handling modes in their ->set_mode() callback. Some of these do not have a 'default' case and adding a new mode in the 'enum clock_event_mode', starts giving following warnings for these platforms about unhandled modes (e.g. XXX).

[PATCH] clockevents: Add (missing) default case for switch blocks

2015-02-19 Thread Viresh Kumar
Many clockevent drivers are using a switch block for handling modes in their -set_mode() callback. Some of these do not have a 'default' case and adding a new mode in the 'enum clock_event_mode', starts giving following warnings for these platforms about unhandled modes (e.g. XXX).