linux-kernel - Re: [PATCH 3/3] clocksource: Add clockevent support to NPS400 driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 21 Oct 2016 17:08:59 +0200
From:   Daniel Lezcano <daniel.lezcano@...aro.org>
To:     Noam Camus <noamca@...lanox.com>
Cc:     robh+dt@...nel.org, mark.rutland@....com, tglx@...utronix.de,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        John Stultz <john.stultz@...aro.org>
Subject: Re: [PATCH 3/3] clocksource: Add clockevent support to NPS400 driver

On Thu, Oct 13, 2016 at 05:52:30PM +0300, Noam Camus wrote:
> From: Noam Camus <noamca@...lanox.com>
> 
> Till now we used clockevent from generic ARC driver.
> This was enough as long as we worked with simple multicore SoC.
> When we are working with multithread SoC each HW thread can be
> scheduled to receive timer interrupt using timer mask register
> (TSI1).
> 
> This patch will provide a way to control clock events per
> HW thread.

The description is not very clear. Can you elaborate ?
 
> Driver can be used from device tree by:
> compatible = "ezchip,nps400-timer0" <-- for clocksource
> compatible = "ezchip,nps400-timer1" <-- for clockevent
> 
> Cc: Daniel Lezcano <daniel.lezcano@...aro.org>
> Cc: Rob Herring <robh+dt@...nel.org>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: John Stultz <john.stultz@...aro.org>
> Signed-off-by: Noam Camus <noamca@...lanox.com>
> ---

[ ... ]

> -static int __init nps_setup_clocksource(struct device_node *node,
> -					struct clk *clk)
> +static void __init nps_setup_clocksource(struct device_node *node)

CLOCKSOURCE_OF_DECLARE is expecting an int __init (*func)(struct device_node *)

This function returns void at the beginning then it is not changed and returns
an error value.

linux/drivers/clocksource/timer-nps.c: In function ‘nps_setup_clocksource’:
linux/drivers/clocksource/timer-nps.c:77:10: warning: ‘return’ with a value, in function returning void
   return ret;
          ^
linux/drivers/clocksource/timer-nps.c:89:9: warning: ‘return’ with a value, in function returning void
  return ret;
         ^
linux/drivers/clocksource/timer-nps.c: At top level:
linux/drivers/clocksource/timer-nps.c:92:216: warning: comparison of distinct pointer types lacks a cast

Don't submit changes which are not correctly compiled / tested, except you want to live
dangerously and watch your karma collapse.

I will let you fix this before reviewing the patches in details.

BTW, have a look at the hotplug callbacks, they changed to a finite state machine [1] and their usage
changed [2].

  -- Daniel

[1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=710d60cbf1b312a8075a2158cbfbbd9c66132dcc
[2] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=eb0a9d8c672dc01db41352afa646405d035ee7a4