Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1529274

Re: [PATCH 7/9] clocksource/drivers/rockchip_timer: implement clocksource timer

Show all headers | View raw

Hi Alexander,

Am Donnerstag, 24. November 2016, 16:05:55 schrieb Alexander Kochetkov:
> 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»;

correct, use both but also update the binding, like 
mmc/rockchip-dw-mshc.txt does.


> > 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»;
> };

what I actually meant was that the driver could also recognize the rk3188-
timer compatible as "we need a clocksource" and it shouldn't matter which 
timer actually gets used for this.


> > 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.ht
> ml

Only devicetree people can really tell you if that is ok :-) .

The devicetree is supposed to be a hardware-description and implementation-
independent, so rockchip,clocksource sounds pretty much like linux-specific 
configuration things leaking into the devicetree.


Heiko

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 1/9] clocksource/drivers/rockchip_timer: split bc_timer into rk_timer and rk_clock_event_device Alexander Kochetkov <al.kochet@gmail.com> - 2016-11-23 18:30 +0100
  [PATCH 5/9] clocksource/drivers/rockchip_timer: implement loading 64bit value into timer Alexander Kochetkov <al.kochet@gmail.com> - 2016-11-23 18:30 +0100
  [PATCH 7/9] clocksource/drivers/rockchip_timer: implement clocksource timer Alexander Kochetkov <al.kochet@gmail.com> - 2016-11-23 18:40 +0100
    Re: [PATCH 7/9] clocksource/drivers/rockchip_timer: implement clocksource timer Alexander Kochetkov <al.kochet@gmail.com> - 2016-11-24 10:40 +0100
      Re: [PATCH 7/9] clocksource/drivers/rockchip_timer: implement clocksource timer Heiko Stübner <heiko@sntech.de> - 2016-11-24 13:20 +0100
        Re: [PATCH 7/9] clocksource/drivers/rockchip_timer: implement clocksource timer Alexander Kochetkov <al.kochet@gmail.com> - 2016-11-24 14:10 +0100
          Re: [PATCH 7/9] clocksource/drivers/rockchip_timer: implement clocksource timer Heiko Stübner <heiko@sntech.de> - 2016-11-24 14:30 +0100
            Re: [PATCH 7/9] clocksource/drivers/rockchip_timer: implement clocksource timer Alexander Kochetkov <al.kochet@gmail.com> - 2016-11-24 15:20 +0100
              Re: [PATCH 7/9] clocksource/drivers/rockchip_timer: implement clocksource timer Heiko Stübner <heiko@sntech.de> - 2016-11-24 15:40 +0100
  [PATCH 8/9] dt-bindings: add rockchip,clocksource property to rk-timer Alexander Kochetkov <al.kochet@gmail.com> - 2016-11-23 18:40 +0100
  [PATCH 2/9] clocksource/drivers/rockchip_timer: low level routines take rk_timer as parameter Alexander Kochetkov <al.kochet@gmail.com> - 2016-11-23 18:40 +0100
  [PATCH 6/9] clocksource/drivers/rockchip_timer: implement reading 64bit value from timer Alexander Kochetkov <al.kochet@gmail.com> - 2016-11-23 18:40 +0100
  [PATCH 3/9] clocksource/drivers/rockchip_timer: drop unused rk_base() and rk_ctrl() Alexander Kochetkov <al.kochet@gmail.com> - 2016-11-23 18:40 +0100
  [PATCH 4/9] clocksource/drivers/rockchip_timer: move TIMER_INT_UNMASK out of rk_timer_enable() Alexander Kochetkov <al.kochet@gmail.com> - 2016-11-23 18:40 +0100
  [PATCH 9/9] ARM: dts: rockchip: add timer entries to rk3188.dtsi Alexander Kochetkov <al.kochet@gmail.com> - 2016-11-23 18:40 +0100
  Re: [PATCH 1/9] clocksource/drivers/rockchip_timer: split bc_timer into rk_timer and rk_clock_event_device Heiko Stübner <heiko@sntech.de> - 2016-11-24 13:10 +0100
    Re: [PATCH 1/9] clocksource/drivers/rockchip_timer: split bc_timer into rk_timer and rk_clock_event_device Alexander Kochetkov <al.kochet@gmail.com> - 2016-11-24 13:20 +0100

csiph-web