Revert "h8300: clocksource"
Subsystems (2)
PATCH 1/1 Revert "h8300: clocksource"
This reverts commit 618b902d8c098f2fff188119da7b3184c4bc5483.
The renesas architecture have been dropped and re-introduced two years after with a supposed improvement of the code.
While looking at the timer drivers, I see it did not passed a correct code review process. No maintainters were in Cc, not code review and no ack from them (Thomas and I).
Even if those drivers where already in the tree several years ago, that does not prevent them, when re-introduced, to be reviewed again and at least acked by the maintainer.
Even if the patches where on lkml@, that does not prevent the different sub-system maintainers to be in Cc.
Instead of that, these drivers passed silently through another tree (arm-soc I guess).
Revert these drivers in order to have them re-submitted and reviewed again.
There is legacy code to be removed.
Conflicts:
drivers/clocksource/Kconfig drivers/clocksource/Makefile drivers/clocksource/h8300_timer16.c drivers/clocksource/h8300_timer8.c drivers/clocksource/h8300_tpu.c --- .../bindings/timer/renesas,16bit-timer.txt | 25 -- .../bindings/timer/renesas,8bit-timer.txt | 25 -- .../devicetree/bindings/timer/renesas,tpu.txt | 21 -- drivers/clocksource/Kconfig | 7 - drivers/clocksource/Makefile | 3 - drivers/clocksource/h8300_timer16.c | 253 ---------------- drivers/clocksource/h8300_timer8.c | 317 --------------------- drivers/clocksource/h8300_tpu.c | 206 ------------- 8 files changed, 857 deletions(-) delete mode 100644 Documentation/devicetree/bindings/timer/renesas,16bit-timer.txt delete mode 100644 Documentation/devicetree/bindings/timer/renesas,8bit-timer.txt delete mode 100644 Documentation/devicetree/bindings/timer/renesas,tpu.txt delete mode 100644 drivers/clocksource/h8300_timer16.c delete mode 100644 drivers/clocksource/h8300_timer8.c delete mode 100644 drivers/clocksource/h8300_tpu.c
Documentation/devicetree/bindings/timer/renesas,16bit-timer.txt deleted +0 -25
@@ -1,25 +0,0 @@ -* Renesas H8/300 16bit timer - -The 16bit timer is a 16bit timer/counter with configurable clock inputs and -programmable compare match. - -Required Properties: - - - compatible: must contain "renesas,16bit-timer" - - reg: base address and length of the registers block for the timer module. - - interrupts: interrupt-specifier for the timer, IMIA - - clocks: a list of phandle, one for each entry in clock-names. - - clock-names: must contain "peripheral_clk" for the functional clock. - - renesas,channel: timer channel number. - -Example: - - timer16: timer@ffff68 { - compatible = "reneas,16bit-timer"; - reg = <0xffff68 8>, <0xffff60 8>; - interrupts = <24>; - renesas,channel = <0>; - clocks = <&pclk>; - clock-names = "peripheral_clk"; - }; -
Documentation/devicetree/bindings/timer/renesas,8bit-timer.txt deleted +0 -25
@@ -1,25 +0,0 @@ -* Renesas H8/300 8bit timer - -The 8bit timer is a 8bit timer/counter with configurable clock inputs and -programmable compare match. - -This implement only supported cascade mode. - -Required Properties: - - - compatible: must contain "renesas,8bit-timer" - - reg: base address and length of the registers block for the timer module. - - interrupts: interrupt-specifier for the timer, CMIA and TOVI - - clocks: a list of phandle, one for each entry in clock-names. - - clock-names: must contain "fck" for the functional clock. - -Example: - - timer8_0: timer@ffff80 { - compatible = "renesas,8bit-timer"; - reg = <0xffff80 10>; - interrupts = <36>; - clocks = <&fclk>; - clock-names = "fck"; - }; -
Documentation/devicetree/bindings/timer/renesas,tpu.txt deleted +0 -21
@@ -1,21 +0,0 @@ -* Renesas H8/300 Timer Pluse Unit - -The TPU is a 16bit timer/counter with configurable clock inputs and -programmable compare match. -This implementation support only cascade mode. - -Required Properties: - - - compatible: must contain "renesas,tpu" - - reg: base address and length of the registers block in 2 channel. - - clocks: a list of phandle, one for each entry in clock-names. - - clock-names: must contain "peripheral_clk" for the functional clock. - - -Example: - tpu: tpu@ffffe0 { - compatible = "renesas,tpu"; - reg = <0xffffe0 16>, <0xfffff0 12>; - clocks = <&pclk>; - clock-names = "peripheral_clk"; - };
drivers/clocksource/Kconfig +0 -7
@@ -290,12 +290,6 @@ config CLKSRC_PXA This enables OST0 support available on PXA and SA-11x0 platforms. -config H8300_TMR16 - bool - -config H8300_TPU - bool - config CLKSRC_IMX_GPT bool "Clocksource using i.MX GPT" if COMPILE_TEST depends on ARM && CLKDEV_LOOKUP
@@ -308,5 +302,4 @@ config CLKSRC_ST_LPC help Enable this option to use the Low Power controller timer as clocksource. - endmenu
drivers/clocksource/Makefile +0 -3
@@ -59,7 +59,4 @@ obj-$(CONFIG_CLKSRC_MIPS_GIC) += mips-gic-timer.o obj-$(CONFIG_CLKSRC_TANGO_XTAL) += tango_xtal.o obj-$(CONFIG_CLKSRC_IMX_GPT) += timer-imx-gpt.o obj-$(CONFIG_ASM9260_TIMER) += asm9260_timer.o -obj-$(CONFIG_H8300) += h8300_timer8.o -obj-$(CONFIG_H8300_TMR16) += h8300_timer16.o -obj-$(CONFIG_H8300_TPU) += h8300_tpu.o obj-$(CONFIG_CLKSRC_ST_LPC) += clksrc_st_lpc.o
drivers/clocksource/h8300_timer16.c deleted +0 -253
253 lines not on this page · raw message
drivers/clocksource/h8300_timer8.c deleted +0 -317
317 lines not on this page · raw message
drivers/clocksource/h8300_tpu.c deleted +0 -206
206 lines not on this page · raw message
Signature · 1 line
1.9.1
Quoted 9 lines · On Friday 30 October 2015 11:26:08 Daniel Lezcano wrote:
This reverts commit
618b902d8c098f2fff188119da7b3184c4bc5483.The renesas architecture have been dropped and re-introduced two years after with a supposed improvement of the code.
While looking at the timer drivers, I see it did not passed a correct code review process. No maintainters were in Cc, not code review and no ack from them (Thomas and I).
Quoted 6 lines
Even if those drivers where already in the tree several years ago, that does not prevent them, when re-introduced, to be reviewed again and at least acked by the maintainer.
Even if the patches where on lkml@, that does not prevent the different sub-system maintainers to be in Cc.
Agreed.
Quoted 2 lines
Instead of that, these drivers passed silently through another tree (arm-soc I guess).
No need to speculate when you can look it up easily (it wasn't us):
$ git log --oneline --merges --ancestry-path 618b902d8c098f2fff188119da7b3184c4bc5483.. | tail -n1 55a7d4b85ca1 Merge tag 'for-4.2' of git://git.sourceforge.jp/gitroot/uclinux-h8/linux $ git show 55a7d4b85ca1 commit 55a7d4b85ca1f723d26b8956e8faeff730d0d240 Merge: aefbef10e3ae 07834743f42b Author: Linus Torvalds <[email protected]> Date: Thu Jun 25 13:07:24 2015 -0700
Merge tag 'for-4.2' of git://git.sourceforge.jp/gitroot/uclinux-h8/linux
Pull Renesas H8/300 architecture re-introduction from Yoshinori Sato.
We dropped arch/h8300 two years ago as stale and old, this is a new and more modern rewritten arch support for the same architecture.
I reviewed the series before it was merged, but failed to notice that you were not on Cc, and I did not look at the clocksource drivers in much detail (if at all).
Quoted 3 lines
There is legacy code to be removed.
Signed-off-by Daniel Lezcano <[email protected]>
I found that Yoshinori Sato has always addressed all review comments very quickly by rewriting the code in a proper manner when mistakes were pointed out in the other drivers. Maybe you can describe now what the problems are with the three drivers to give him a chance to fix them?
Arnd
Quoted 2 lines · Arnd Bergmann
Quoted 1 line · On Friday 30 October 2015 11:26:08 Daniel Lezcano wrote:
This reverts commit
618b902d8c098f2fff188119da7b3184c4bc5483.
[ ... ]
Quoted 4 lines
I found that Yoshinori Sato has always addressed all review comments very quickly by rewriting the code in a proper manner when mistakes were pointed out in the other drivers. Maybe you can describe now what the problems are with the three drivers to give him a chance to fix them?
I agree it is a good argument to give him the opportunity to fix the code.
Yoshinori Sato, can you please do a cleanup in the init functions to comply with the other drivers, use CLOCKSOURCE_OF_DECLARE and remove the module exit code ?
Thanks.
-- Daniel
Signature · 5 lines
<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
Quoted 12 lines · Daniel Lezcano
Quoted 2 lines · Arnd Bergmann
Quoted 1 line · On Friday 30 October 2015 11:26:08 Daniel Lezcano wrote:
This reverts commit
618b902d8c098f2fff188119da7b3184c4bc5483.[ ... ]
Quoted 4 lines
I found that Yoshinori Sato has always addressed all review comments very quickly by rewriting the code in a proper manner when mistakes were pointed out in the other drivers. Maybe you can describe now what the problems are with the three drivers to give him a chance to fix them?
Quoted 7 lines
I agree it is a good argument to give him the opportunity to fix the code.
Yoshinori Sato, can you please do a cleanup in the init functions to comply with the other drivers, use
CLOCKSOURCE_OF_DECLAREand remove the module exit code ?Thanks.
OK.
Please wait few days.
Quoted 9 lines
-- Daniel
Signature · 5 lines
<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
Signature · 2 lines
Yoshinori Sato <[email protected]>