From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757268AbcFAGQp (ORCPT ); Wed, 1 Jun 2016 02:16:45 -0400 Received: from mail-wm0-f53.google.com ([74.125.82.53]:34650 "EHLO mail-wm0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750697AbcFAGQo (ORCPT ); Wed, 1 Jun 2016 02:16:44 -0400 Subject: Re: [PATCH 4/5] clocksource: rockchip: add support for rk3399 SoC To: "Huang, Tao" , Caesar Wang , Heiko Stuebner References: <1464169802-6033-1-git-send-email-wxt@rock-chips.com> <1464169802-6033-5-git-send-email-wxt@rock-chips.com> <574CCCB4.1030001@linaro.org> <574D95AF.2020905@rock-chips.com> <574D9A66.4090209@linaro.org> <574E4130.8090600@rock-chips.com> Cc: dianders@chromium.org, briannorris@google.com, smbarber@google.com, linux-rockchip@lists.infradead.org, Thomas Gleixner , cf@rock-chips.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org From: Daniel Lezcano Message-ID: <574E7DC9.4000102@linaro.org> Date: Wed, 1 Jun 2016 08:16:41 +0200 User-Agent: Mozilla/5.0 (X11; Linux i686; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <574E4130.8090600@rock-chips.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/01/2016 03:58 AM, Huang, Tao wrote: > Hi Daniel: > On 2016年05月31日 22:06, Daniel Lezcano wrote: [ ... ] >>> -CLOCKSOURCE_OF_DECLARE(rk_timer, "rockchip,rk3288-timer", rk_timer_init); >>> +static void __init rk3288_timer_init(struct device_node *np) >>> +{ >>> + bc_timer.ctrl = TIMER_CONTROL_REG3288; >>> + rk_timer_init(np); >> >> rk_timer_init(np); >> bc_timer.ctrl = bc_timer.base + TIMER_CONTROL_REG3288; > > No. It's not such simple. You will access null pointer when > rk_timer_init, if we keep rk_timer_disable call in init or after > request_irq/clockevents_config_and_register and interrupt happen > immediately. > > So the code maybe: > static void __init rk3288_timer_init(struct device_node *np) > { > bc_timer.base = of_iomap(np, 0); > if (!bc_timer.base) { > pr_err("Failed to get base address for '%s'\n", TIMER_NAME); > return; > } > bc_timer.ctrl = bc_timer.base + TIMER_CONTROL_REG3288; > rk_imter_init(np); // of course remove of_iomap from init. > > Is this what you want? Not necessarily. There are plenty of variants. eg. rk_timer_init(np, TIMER_CONTROL_REG3288); -- Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog