All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: Alexander Kochetkov <al.kochet@gmail.com>,
	Heiko Stuebner <heiko@sntech.de>,
	LKML <linux-kernel@vger.kernel.org>,
	devicetree@vger.kernel.org,
	LAK <linux-arm-kernel@lists.infradead.org>,
	linux-rockchip@lists.infradead.org
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Mark Rutland <mark.rutland@arm.com>,
	Rob Herring <robh+dt@kernel.org>,
	Russell King <linux@armlinux.org.uk>,
	Caesar Wang <wxt@rock-chips.com>,
	Huang Tao <huangtao@rock-chips.com>
Subject: Re: [PATCH v6 0/5] Implement clocksource for rockchip SoC using rockchip timer
Date: Tue, 21 Mar 2017 09:02:00 +0100	[thread overview]
Message-ID: <6b897768-d11f-bfd2-7eeb-bd7a41ea7669@linaro.org> (raw)
In-Reply-To: <F14EA552-9783-4083-A00F-9151AB7E5C99@gmail.com>

On 20/03/2017 14:18, Alexander Kochetkov wrote:
> Hello, Daniel.
> 
> Sorry for bothering you, but could you please take a look at v6[1] series and tell what do you think about it?
> I see your commit[2] in the git, but I don’t understand well how to use it in rockchip driver.
> 
> [1] https://lkml.org/lkml/2017/1/31/401
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/clocksource?id=376bc27150f180d9f5eddec6a14117780177589d

Hi Alexander,

I have been through the patchset and it is ok except for the patch 3/5.
I wanted to investigate a bit on this one because of the patch [2] you
are mentioning.

I think there is small missing piece of devicetree binding to specify
'clocksource' or 'clockevent'. Rob Herring is ok with an attribute of
this kind.

That would help to clearly separate these drivers without ambiguity.

However, it results on some duplicated code I'm not very happy with, so
that is my major issue with this approach I would like to solve. That is
the status today.

I wanted to take the opportunity of your changes to investigate this area.


>> 31 янв. 2017 г., в 15:43, Alexander Kochetkov <al.kochet@gmail.com> написал(а):
>>
>> From: Alexander Kochetkov <akochetkov@lintech.ru>
>>
>> Hello, Daniel, Heiko.
>>
>> Here is try 6 :) Thanks a lot for helping me to bring the code
>> into kernel!
>>
>> This patch series contain:
>> - devicetree bindings clarification for rockchip timers
>> - dts files fixes for rk3228-evb, rk3229-evb and rk3188
>> - implementation of clocksource and sched clock for rockchip SoC
>>
>> The clock supplying the arm-global-timer on the rk3188 is coming from the
>> the cpu clock itself and thus changes its rate everytime cpufreq adjusts
>> the cpu frequency making this timer unsuitable as a stable clocksource.
>>
>> The rk3188, rk3288 and following socs share a separate timer block already
>> handled by the rockchip-timer driver. Therefore adapt this driver to also
>> be able to act as clocksource on rk3188.
>>
>> In order to test clocksource you can run following commands and check
>> how much time it take in real. On rk3188 it take about ~45 seconds.
>>
>>       cpufreq-set -f 1.6GHZ
>>       date; sleep 60; date
>>
>> rk3288 (and probably anything newer) is irrelevant to this patch,
>> as it has the arch timer interface. This patch may be usefull
>> for Cortex-A9/A5 based parts.
>>
>> Regards,
>> Alexander.
>>
>> Changes in v6:
>> - Removed Reviewed-by: Heiko Stübner <heiko@sntech.de> tag
>> - Merge 5/8, 6/8, 7/8, 8/9 into single file
>> - split init paths into rk_clkevt_init() and rk_clksrc_init()
>>  so the driver code could be adopted to CLOCKEVENT_OF_DECLARE()
>> - clockevents implemented using clocksource_mmio_init()
>> - fixed commit message for 4/8 (thanks a lot Daniel)
>>
>> Changes in v5:
>> - Add Acked-by: Rob Herring <robh at kernel.org> to 1/8
>>  http://lists.infradead.org/pipermail/linux-rockchip/2016-December/013308.html
>> - Add Reviwed-by: Heiko Stübner <heiko@sntech.de> to series
>> - change timer compatible property in the rk322x.dtsi 2/8
>>  http://lists.infradead.org/pipermail/linux-rockchip/2017-January/013784.html
>> - updated comment message for 4/8
>>  http://lists.infradead.org/pipermail/linux-rockchip/2017-January/013786.html
>> - updated comment message for 5/8
>>  http://lists.infradead.org/pipermail/linux-rockchip/2017-January/013787.html
>> - fixed build error for 8/8
>>  http://lists.infradead.org/pipermail/linux-rockchip/2017-January/013789.html
>>
>> Alexander Kochetkov (5):
>>  dt-bindings: clarify compatible property for rockchip timers
>>  ARM: dts: rockchip: update compatible property for rk322x timer
>>  clocksource/drivers/rockchip_timer: implement clocksource timer
>>  ARM: dts: rockchip: add timer entries to rk3188 SoC
>>  ARM: dts: rockchip: disable arm-global-timer for rk3188
>>
>> .../bindings/timer/rockchip,rk-timer.txt           |   12 +-
>> arch/arm/boot/dts/rk3188.dtsi                      |   17 ++
>> arch/arm/boot/dts/rk322x.dtsi                      |    2 +-
>> drivers/clocksource/Kconfig                        |    1 +
>> drivers/clocksource/rockchip_timer.c               |  218 ++++++++++++++------
>> 5 files changed, 185 insertions(+), 65 deletions(-)
>>
>> -- 
>> 1.7.9.5
>>
> 


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

WARNING: multiple messages have this Message-ID (diff)
From: daniel.lezcano@linaro.org (Daniel Lezcano)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v6 0/5] Implement clocksource for rockchip SoC using rockchip timer
Date: Tue, 21 Mar 2017 09:02:00 +0100	[thread overview]
Message-ID: <6b897768-d11f-bfd2-7eeb-bd7a41ea7669@linaro.org> (raw)
In-Reply-To: <F14EA552-9783-4083-A00F-9151AB7E5C99@gmail.com>

On 20/03/2017 14:18, Alexander Kochetkov wrote:
> Hello, Daniel.
> 
> Sorry for bothering you, but could you please take a look at v6[1] series and tell what do you think about it?
> I see your commit[2] in the git, but I don?t understand well how to use it in rockchip driver.
> 
> [1] https://lkml.org/lkml/2017/1/31/401
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/clocksource?id=376bc27150f180d9f5eddec6a14117780177589d

Hi Alexander,

I have been through the patchset and it is ok except for the patch 3/5.
I wanted to investigate a bit on this one because of the patch [2] you
are mentioning.

I think there is small missing piece of devicetree binding to specify
'clocksource' or 'clockevent'. Rob Herring is ok with an attribute of
this kind.

That would help to clearly separate these drivers without ambiguity.

However, it results on some duplicated code I'm not very happy with, so
that is my major issue with this approach I would like to solve. That is
the status today.

I wanted to take the opportunity of your changes to investigate this area.


>> 31 ???. 2017 ?., ? 15:43, Alexander Kochetkov <al.kochet@gmail.com> ???????(?):
>>
>> From: Alexander Kochetkov <akochetkov@lintech.ru>
>>
>> Hello, Daniel, Heiko.
>>
>> Here is try 6 :) Thanks a lot for helping me to bring the code
>> into kernel!
>>
>> This patch series contain:
>> - devicetree bindings clarification for rockchip timers
>> - dts files fixes for rk3228-evb, rk3229-evb and rk3188
>> - implementation of clocksource and sched clock for rockchip SoC
>>
>> The clock supplying the arm-global-timer on the rk3188 is coming from the
>> the cpu clock itself and thus changes its rate everytime cpufreq adjusts
>> the cpu frequency making this timer unsuitable as a stable clocksource.
>>
>> The rk3188, rk3288 and following socs share a separate timer block already
>> handled by the rockchip-timer driver. Therefore adapt this driver to also
>> be able to act as clocksource on rk3188.
>>
>> In order to test clocksource you can run following commands and check
>> how much time it take in real. On rk3188 it take about ~45 seconds.
>>
>>       cpufreq-set -f 1.6GHZ
>>       date; sleep 60; date
>>
>> rk3288 (and probably anything newer) is irrelevant to this patch,
>> as it has the arch timer interface. This patch may be usefull
>> for Cortex-A9/A5 based parts.
>>
>> Regards,
>> Alexander.
>>
>> Changes in v6:
>> - Removed Reviewed-by: Heiko St?bner <heiko@sntech.de> tag
>> - Merge 5/8, 6/8, 7/8, 8/9 into single file
>> - split init paths into rk_clkevt_init() and rk_clksrc_init()
>>  so the driver code could be adopted to CLOCKEVENT_OF_DECLARE()
>> - clockevents implemented using clocksource_mmio_init()
>> - fixed commit message for 4/8 (thanks a lot Daniel)
>>
>> Changes in v5:
>> - Add Acked-by: Rob Herring <robh@kernel.org> to 1/8
>>  http://lists.infradead.org/pipermail/linux-rockchip/2016-December/013308.html
>> - Add Reviwed-by: Heiko St?bner <heiko@sntech.de> to series
>> - change timer compatible property in the rk322x.dtsi 2/8
>>  http://lists.infradead.org/pipermail/linux-rockchip/2017-January/013784.html
>> - updated comment message for 4/8
>>  http://lists.infradead.org/pipermail/linux-rockchip/2017-January/013786.html
>> - updated comment message for 5/8
>>  http://lists.infradead.org/pipermail/linux-rockchip/2017-January/013787.html
>> - fixed build error for 8/8
>>  http://lists.infradead.org/pipermail/linux-rockchip/2017-January/013789.html
>>
>> Alexander Kochetkov (5):
>>  dt-bindings: clarify compatible property for rockchip timers
>>  ARM: dts: rockchip: update compatible property for rk322x timer
>>  clocksource/drivers/rockchip_timer: implement clocksource timer
>>  ARM: dts: rockchip: add timer entries to rk3188 SoC
>>  ARM: dts: rockchip: disable arm-global-timer for rk3188
>>
>> .../bindings/timer/rockchip,rk-timer.txt           |   12 +-
>> arch/arm/boot/dts/rk3188.dtsi                      |   17 ++
>> arch/arm/boot/dts/rk322x.dtsi                      |    2 +-
>> drivers/clocksource/Kconfig                        |    1 +
>> drivers/clocksource/rockchip_timer.c               |  218 ++++++++++++++------
>> 5 files changed, 185 insertions(+), 65 deletions(-)
>>
>> -- 
>> 1.7.9.5
>>
> 


-- 
 <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

  reply	other threads:[~2017-03-21  8:02 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-31 12:43 [PATCH v6 0/5] Implement clocksource for rockchip SoC using rockchip timer Alexander Kochetkov
2017-01-31 12:43 ` Alexander Kochetkov
2017-01-31 12:43 ` Alexander Kochetkov
2017-01-31 12:43 ` [PATCH v6 1/5] dt-bindings: clarify compatible property for rockchip timers Alexander Kochetkov
2017-01-31 12:43   ` Alexander Kochetkov
2017-02-01  9:33   ` Heiko Stuebner
2017-02-01  9:33     ` Heiko Stuebner
2017-02-01  9:33     ` Heiko Stuebner
2017-01-31 12:43 ` [PATCH v6 2/5] ARM: dts: rockchip: update compatible property for rk322x timer Alexander Kochetkov
2017-01-31 12:43   ` Alexander Kochetkov
2017-01-31 12:43   ` Alexander Kochetkov
2017-02-01  9:34   ` Heiko Stuebner
2017-02-01  9:34     ` Heiko Stuebner
2017-02-01  9:34     ` Heiko Stuebner
2017-01-31 12:43 ` [PATCH v6 3/5] clocksource/drivers/rockchip_timer: implement clocksource timer Alexander Kochetkov
2017-01-31 12:43   ` Alexander Kochetkov
2017-01-31 12:43   ` Alexander Kochetkov
2017-01-31 12:43 ` [PATCH v6 4/5] ARM: dts: rockchip: add timer entries to rk3188 SoC Alexander Kochetkov
2017-01-31 12:43   ` Alexander Kochetkov
2017-01-31 12:43   ` Alexander Kochetkov
2017-02-01  9:43   ` Heiko Stuebner
2017-02-01  9:43     ` Heiko Stuebner
2017-01-31 12:43 ` [PATCH v6 5/5] ARM: dts: rockchip: disable arm-global-timer for rk3188 Alexander Kochetkov
2017-01-31 12:43   ` Alexander Kochetkov
2017-01-31 12:43   ` Alexander Kochetkov
2017-02-01  9:48   ` Heiko Stuebner
2017-02-01  9:48     ` Heiko Stuebner
2017-02-01  9:48     ` Heiko Stuebner
2017-03-20 13:18 ` [PATCH v6 0/5] Implement clocksource for rockchip SoC using rockchip timer Alexander Kochetkov
2017-03-20 13:18   ` Alexander Kochetkov
2017-03-20 13:18   ` Alexander Kochetkov
2017-03-21  8:02   ` Daniel Lezcano [this message]
2017-03-21  8:02     ` Daniel Lezcano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6b897768-d11f-bfd2-7eeb-bd7a41ea7669@linaro.org \
    --to=daniel.lezcano@linaro.org \
    --cc=al.kochet@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=heiko@sntech.de \
    --cc=huangtao@rock-chips.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=linux@armlinux.org.uk \
    --cc=mark.rutland@arm.com \
    --cc=robh+dt@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=wxt@rock-chips.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.