Content-Type: text/plain; charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\))
Subject: Re: [PATCH 7/9] clocksource/drivers/rockchip_timer: implement clocksource timer
From: Alexander Kochetkov <al.kochet@gmail.com>
In-Reply-To: <1800621.EhepfMxccR@diego>
Date: Thu, 24 Nov 2016 16:05:55 +0300
Cc: daniel.lezcano@linaro.org, tglx@linutronix.de,
        linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-rockchip@lists.infradead.org
Message-Id: <0E972AC9-1132-4D0B-BF25-918BC1AA1A49@gmail.com>
References: <1479922177-20136-1-git-send-email-al.kochet@gmail.com> <1479922177-20136-7-git-send-email-al.kochet@gmail.com> <FFD6E644-F68B-4880-BC76-88F6063D070E@gmail.com> <1800621.EhepfMxccR@diego>
To: =?utf-8?Q?Heiko_St=C3=BCbner?= <heiko@sntech.de>
Sender: linux-kernel-owner@vger.kernel.org
Content-Transfer-Encoding: 8bit
Content-Length: 2250
Lines: 65

Hello Heiko!

> 24 нояб. 2016 г., в 15:17, Heiko Stübner <heiko@sntech.de> написал(а):
> 
> In your dts-patch you reuse the rk3288-timer compatible value, which is also 
> non-ideal.

rockchip,rk-timer.txt states what I should use rockchip,rk3288-timer for rk3188 timers:

Required properties:
- compatible: shall be one of:
  "rockchip,rk3288-timer" - for rk3066, rk3036, rk3188, rk322x, rk3288, rk3368

Should I add "rockchip,rk3188-timer» (or better "rockchip,rk3036-timer») ? Or may be second
approach should be used?

> What you may want to do is introduce a rockchip,rk3188-timer compatible and 
> then make the timer-driver act accordingly, as you then know you are on a 
> rk3188-board ... see drivers attaching specific structs to the of_device_id 
> entries. From the documentation

May be it is better to prepend compatible string with "rockchip,rk3188-timer» in the dts
file only? elinux[1] recommend build compatible string from "exact device» string and
«other devices» that the device is compatible with.

As «other devices» string defined already, I use it.

[1] http://elinux.org/Device_Tree_Usage#Understanding_the_compatible_Property

	timer0: timer@20038000 {
		compatible = "rockchip,rk3188-timer", "rockchip,rk3288-timer»;
                …
	};

Just found what rk3036 already declared such way:

rk3036.dtsi:	timer: timer@20044000 {
rk3036.dtsi:		compatible = "rockchip,rk3036-timer", "rockchip,rk3288-timer»;

> 
> it also shouldn't really matter which timer 
> you use as clocksource, as on the rk3188 it seems all of them act the same way 
> (except timer3 being always on).

You are right.

I sent dts file with general timer settings, without clockevent enabled, so one could pick
up timer and setup it in the board dts (radxarock, for example) like:

&timer0 {
    rockchip,clocksource;
    status = «okay»;
};

> When touching devicetree-properties, please also adapt the binding document
> 	Documentation/devicetree/bindings/timer,rockchip,rk-timer.txt
> in this case and also include the devicetree maintainers.

If the patch[2] ok, I'll resend it and include devicetree maintainers.

[2] http://lists.infradead.org/pipermail/linux-rockchip/2016-November/013148.html

Regards,
Alexander.