| Message ID | 1375740158-10012-1-git-send-email-dinguyen@altera.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/05/2013 04:02 PM, dinguyen@altera.com wrote: > From: Dinh Nguyen <dinguyen@altera.com> > > "dw-apb-timer-osc" and "dw-apb-timer-sp" are the same implementation of the > DW APB timer, just fed by different clocks. I assume patch 1/1 made similar changes to the driver? > diff --git a/Documentation/devicetree/bindings/rtc/dw-apb.txt b/Documentation/devicetree/bindings/rtc/dw-apb.txt > Required properties: > -- compatible: "snps,dw-apb-timer-sp" or "snps,dw-apb-timer-osc" > +- compatible: "snps,dw-apb-timer" This change is problematic w.r.t device tree as an ABI. any DT that uses the new value "snps,dw-apb-timer" will not work with older software that was written to expect the old values. Similarly, if the driver was edited to only support the new value listed above, then old DTs will not work with the new driver. Instead, I think what you need to do is: a) Update the driver to support all 3 compatible values, so all DTs will work with the new driver. b) Modify the *.dtsi files to uses one of the old compatible values, so the new DTs will work with the old driver. c) Modify the binding document to mention all 3 compatible values, but mark the old 2 as deprecated.
On Thu 2013-08-08 14:17:46, Stephen Warren wrote: > On 08/05/2013 04:02 PM, dinguyen@altera.com wrote: > > From: Dinh Nguyen <dinguyen@altera.com> > > > > "dw-apb-timer-osc" and "dw-apb-timer-sp" are the same implementation of the > > DW APB timer, just fed by different clocks. > > I assume patch 1/1 made similar changes to the driver? > > > diff --git a/Documentation/devicetree/bindings/rtc/dw-apb.txt b/Documentation/devicetree/bindings/rtc/dw-apb.txt > > > Required properties: > > -- compatible: "snps,dw-apb-timer-sp" or "snps,dw-apb-timer-osc" > > +- compatible: "snps,dw-apb-timer" > > This change is problematic w.r.t device tree as an ABI. any DT that uses > the new value "snps,dw-apb-timer" will not work with older software that > was written to expect the old values. I'm pretty sure there's no such software out there. Actually, I don't think timers ever worked properly on any mainline kernels. So I believe we can still fix it the right way... it is same hardware with different labels after all. Thanks, Pavel
On 08/08/2013 03:01 PM, Pavel Machek wrote: > On Thu 2013-08-08 14:17:46, Stephen Warren wrote: >> On 08/05/2013 04:02 PM, dinguyen@altera.com wrote: >>> From: Dinh Nguyen <dinguyen@altera.com> >>> >>> "dw-apb-timer-osc" and "dw-apb-timer-sp" are the same implementation of the >>> DW APB timer, just fed by different clocks. >> >> I assume patch 1/1 made similar changes to the driver? >> >>> diff --git a/Documentation/devicetree/bindings/rtc/dw-apb.txt b/Documentation/devicetree/bindings/rtc/dw-apb.txt >> >>> Required properties: >>> -- compatible: "snps,dw-apb-timer-sp" or "snps,dw-apb-timer-osc" >>> +- compatible: "snps,dw-apb-timer" >> >> This change is problematic w.r.t device tree as an ABI. any DT that uses >> the new value "snps,dw-apb-timer" will not work with older software that >> was written to expect the old values. > > I'm pretty sure there's no such software out there. Actually, I don't > think timers ever worked properly on any mainline kernels. > > So I believe we can still fix it the right way... it is same hardware > with different labels after all. If even Linux 3.11 isn't going to work properly with the current DT content, then I think breaking ABI compatibility is fine.
diff --git a/Documentation/devicetree/bindings/rtc/dw-apb.txt b/Documentation/devicetree/bindings/rtc/dw-apb.txt index eb2327b..0a1020e 100644 --- a/Documentation/devicetree/bindings/rtc/dw-apb.txt +++ b/Documentation/devicetree/bindings/rtc/dw-apb.txt @@ -1,7 +1,7 @@ * Designware APB timer Required properties: -- compatible: "snps,dw-apb-timer-sp" or "snps,dw-apb-timer-osc" +- compatible: "snps,dw-apb-timer" - reg: physical base address of the controller and length of memory mapped region. - interrupts: IRQ line for the timer. @@ -20,23 +20,8 @@ systems may use one. Example: - - timer1: timer@ffc09000 { - compatible = "snps,dw-apb-timer-sp"; - interrupts = <0 168 4>; - clock-frequency = <200000000>; - reg = <0xffc09000 0x1000>; - }; - - timer2: timer@ffd00000 { - compatible = "snps,dw-apb-timer-osc"; - interrupts = <0 169 4>; - clock-frequency = <200000000>; - reg = <0xffd00000 0x1000>; - }; - - timer3: timer@ffe00000 { - compatible = "snps,dw-apb-timer-osc"; + timer1: timer@ffe00000 { + compatible = "snps,dw-apb-timer"; interrupts = <0 170 4>; reg = <0xffe00000 0x1000>; clocks = <&timer_clk>, <&timer_pclk>; diff --git a/arch/arm/boot/dts/rk3066a.dtsi b/arch/arm/boot/dts/rk3066a.dtsi index 56bfac9..2dff468 100644 --- a/arch/arm/boot/dts/rk3066a.dtsi +++ b/arch/arm/boot/dts/rk3066a.dtsi @@ -71,7 +71,7 @@ }; timer@20038000 { - compatible = "snps,dw-apb-timer-osc"; + compatible = "snps,dw-apb-timer"; reg = <0x20038000 0x100>; interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>; clocks = <&clk_gates1 0>, <&clk_gates7 7>; @@ -79,7 +79,7 @@ }; timer@2003a000 { - compatible = "snps,dw-apb-timer-osc"; + compatible = "snps,dw-apb-timer"; reg = <0x2003a000 0x100>; interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>; clocks = <&clk_gates1 1>, <&clk_gates7 8>; @@ -87,7 +87,7 @@ }; timer@2000e000 { - compatible = "snps,dw-apb-timer-osc"; + compatible = "snps,dw-apb-timer"; reg = <0x2000e000 0x100>; interrupts = <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>; clocks = <&clk_gates1 2>, <&clk_gates7 9>; diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi index 93ee655..4e8291b 100644 --- a/arch/arm/boot/dts/socfpga.dtsi +++ b/arch/arm/boot/dts/socfpga.dtsi @@ -26,10 +26,6 @@ ethernet1 = &gmac1; serial0 = &uart0; serial1 = &uart1; - timer0 = &timer0; - timer1 = &timer1; - timer2 = &timer2; - timer3 = &timer3; }; cpus { @@ -486,28 +482,32 @@ interrupts = <1 13 0xf04>; }; - timer0: timer0@ffc08000 { - compatible = "snps,dw-apb-timer-sp"; + timer0: timer@ffc08000 { + compatible = "snps,dw-apb-timer"; interrupts = <0 167 4>; reg = <0xffc08000 0x1000>; + clocks = <&osc>; }; - timer1: timer1@ffc09000 { - compatible = "snps,dw-apb-timer-sp"; + timer1: timer@ffc09000 { + compatible = "snps,dw-apb-timer"; interrupts = <0 168 4>; reg = <0xffc09000 0x1000>; + clocks = <&osc>; }; - timer2: timer2@ffd00000 { - compatible = "snps,dw-apb-timer-osc"; + timer2: timer@ffd00000 { + compatible = "snps,dw-apb-timer"; interrupts = <0 169 4>; reg = <0xffd00000 0x1000>; + clocks = <&l4_sp_clk>; }; - timer3: timer3@ffd01000 { - compatible = "snps,dw-apb-timer-osc"; + timer3: timer@ffd01000 { + compatible = "snps,dw-apb-timer"; interrupts = <0 170 4>; reg = <0xffd01000 0x1000>; + clocks = <&l4_sp_clk>; }; uart0: serial0@ffc02000 { diff --git a/arch/arm/boot/dts/socfpga_cyclone5.dts b/arch/arm/boot/dts/socfpga_cyclone5.dts index 102c4d8..54b8483 100644 --- a/arch/arm/boot/dts/socfpga_cyclone5.dts +++ b/arch/arm/boot/dts/socfpga_cyclone5.dts @@ -67,19 +67,19 @@ }; }; - timer0@ffc08000 { + timer@ffc08000 { clock-frequency = <100000000>; }; - timer1@ffc09000 { + timer@ffc09000 { clock-frequency = <100000000>; }; - timer2@ffd00000 { + timer@ffd00000 { clock-frequency = <25000000>; }; - timer3@ffd01000 { + timer@ffd01000 { clock-frequency = <25000000>; }; diff --git a/arch/arm/boot/dts/socfpga_vt.dts b/arch/arm/boot/dts/socfpga_vt.dts index d93deb0..6a1d8b7 100644 --- a/arch/arm/boot/dts/socfpga_vt.dts +++ b/arch/arm/boot/dts/socfpga_vt.dts @@ -58,19 +58,19 @@ }; }; - timer0@ffc08000 { + timer@ffc08000 { clock-frequency = <7000000>; }; - timer1@ffc09000 { + timer@ffc09000 { clock-frequency = <7000000>; }; - timer2@ffd00000 { + timer@ffd00000 { clock-frequency = <7000000>; }; - timer3@ffd01000 { + timer@ffd01000 { clock-frequency = <7000000>; };