Re: [LNG] Re: [PATCH V3 6/8] clockevents: drivers/: migrate to ->set_dev_mode()

On 29 May 2014 16:50, Preeti U Murthy preeti@linux.vnet.ibm.com wrote:

On 05/23/2014 08:55 PM, Viresh Kumar wrote:

...

diff --git a/drivers/clocksource/qcom-timer.c b/drivers/clocksource/qcom-timer.c

...

-static void msm_timer_set_mode(enum clock_event_mode mode, +static int msm_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { u32 ctrl; @@ -85,16 +85,17 @@ static void msm_timer_set_mode(enum clock_event_mode mode,

  switch (mode) {
  case CLOCK_EVT_MODE_RESUME:

• case CLOCK_EVT_MODE_PERIODIC:
  

•         break;
  

Sorry about repeatedly pointing this. I may be missing something. But why are the above two lines removed?

mode isn't supported..

...

  case CLOCK_EVT_MODE_ONESHOT:
          /* Timer is enabled in set_next_event */
          break;
  case CLOCK_EVT_MODE_UNUSED:
  case CLOCK_EVT_MODE_SHUTDOWN:
          break;

• default:
  

•         return -ENOSYS;
  }
  writel_relaxed(ctrl, event_base + TIMER_ENABLE);
  

Would it be fine to skip this ^^ in the default case?

We shouldn't be executing this for unsupported modes, right?

...

• return 0;
  

}

[snip]

...

diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c index 0451e62..8aa35c1 100644 --- a/drivers/clocksource/time-armada-370-xp.c +++ b/drivers/clocksource/time-armada-370-xp.c @@ -120,12 +120,12 @@ armada_370_xp_clkevt_next_event(unsigned long delta, return 0; }

-static void +static int armada_370_xp_clkevt_mode(enum clock_event_mode mode, struct clock_event_device *dev) {

• if (mode == CLOCK_EVT_MODE_PERIODIC) {
  

• switch (mode) {
  

• case CLOCK_EVT_MODE_PERIODIC:
          /*
           * Setup timer to fire at 1/HZ intervals.
           */
  

@@ -136,7 +136,11 @@ armada_370_xp_clkevt_mode(enum clock_event_mode mode, * Enable timer. */ local_timer_ctrl_clrset(0, TIMER0_RELOAD_EN | enable_mask);

• } else {
  

•         break;
  

• case CLOCK_EVT_MODE_ONESHOT:
  

• case CLOCK_EVT_MODE_UNUSED:
  

• case CLOCK_EVT_MODE_SHUTDOWN:
  

• case CLOCK_EVT_MODE_RESUME:
          /*
           * Disable timer.
           */
  

@@ -146,7 +150,11 @@ armada_370_xp_clkevt_mode(enum clock_event_mode mode, * ACK pending timer interrupt. */ writel(TIMER0_CLR_MASK, local_base + LCL_TIMER_EVENTS_STATUS);

This line in the right place right? I mean do you need it in default case also? I am unable to make out.

default cases now are only for unsupported modes and we shouldn't call anything in that..

...

+++ b/drivers/clocksource/vt8500_timer.c @@ -88,12 +88,11 @@ static int vt8500_timer_set_next_event(unsigned long cycles, return 0; }

-static void vt8500_timer_set_mode(enum clock_event_mode mode, +static int vt8500_timer_set_mode(enum clock_event_mode mode, struct clock_event_device *evt) { switch (mode) { case CLOCK_EVT_MODE_RESUME:

• case CLOCK_EVT_MODE_PERIODIC:
          break;
  

Is this deletion right?

mode isn't supported..