See below On 2014-05-15 15:56, Michal Simek wrote: > On 05/15/2014 02:20 PM, Arnd Bergmann wrote: >> On Thursday 15 May 2014 11:48:52 Michal Simek wrote: >>> On 05/14/2014 01:26 PM, Bart Tanghe wrote: >>>> add Xilinx PWM support - axi timer hardware block >>>> >>>> Signed-off-by: Bart Tanghe <bart.tanghe@xxxxxxxxxxxxx> >>>> diff --git a/Documentation/devicetree/bindings/pwm/pwm-xlnx.txt b/Documentation/devicetree/bindings/pwm/pwm-xlnx.txt >>>> new file mode 100644 >>>> index 0000000..cb48926 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/pwm/pwm-xlnx.txt >>>> @@ -0,0 +1,20 @@ >>>> +Xilinx PWM controller >>>> + >>>> +Required properties: >>>> +- compatible: should be "xlnx,pwm-xlnx" >>>> +- add a clock source to the description >>>> + >>>> +Examples: >>>> + >>>> + axi_timer_0: timer@42800000 { >>>> + clock-frequency = <100000000>; >>> >>> just remove this from example it is not needed and unused. >>> >>> >>>> + clocks = <&clkc 15>; >>>> + compatible = "xlnx,xlnx-pwm"; >>>> + reg = <0x42800000 0x10000>; >>>> + xlnx,count-width = <0x20>; >>>> + xlnx,gen0-assert = <0x1>; >>>> + xlnx,gen1-assert = <0x1>; >>>> + xlnx,one-timer-only = <0x0>; >>>> + xlnx,trig0-assert = <0x1>; >>>> + xlnx,trig1-assert = <0x1>; >>>> + } ; >>> >>> ok. This has to be done completely differently. >>> I have looked around and found one a little bit older >>> example but it is in the Linux kernel. >>> It is pwm-atmel-tcb.c driver and atmel_tclib.c and tcb_clksrc.c. >>> >>> Arnd: Isn't there any newer better example how to manage it? >> >> Note that we have two atmel pwm drivers: drivers/misc/atmel_pwm.c >> and drivers/pwm/pwm-atmel.c. If you want to take that as an example, >> make sure you use base on the latter. > > Yes, I have seen that there are two drivers. atmel_pwm is older one > without using that tclib. > >> >>> Back to atmel example - they are maintaining >>> internal list of timers (atmel_tclib.c) >>> and then clocksource driver (tcb_clksrc.c) is asking with atmel_tc_alloc() for it. >>> The same is for pwm driver (pwm-atmel-tcb.c). >>> >>> They probably have all timers the same which is not >>> our case because they can be different but this can be solved >>> with flags. >>> >>> From DT point of view I think this is the reasonable description >>> >>> axi_timer_0: timer@42800000 { >>> clocks = <&clkc 15>; >>> compatible = "xlnx,xps-timer-1.00.a"; >>> interrupt-parent = <&xps_intc_0>; >>> interrupts = < 3 2 >; >>> reg = <0x42800000 0x10000>; >>> xlnx,count-width = <0x20>; >>> xlnx,gen0-assert = <0x1>; >>> xlnx,gen1-assert = <0x1>; >>> xlnx,one-timer-only = <0x0>; >>> xlnx,trig0-assert = <0x1>; >>> xlnx,trig1-assert = <0x1>; >>> } ; >>> >>> >>> pwm { >>> compatible = "xlnx,timer-pwm"; >>> #pwm-cells = <2>; >>> timer = <&axi_timer_0>; >>> }; >>> >>> >>> Allocation function will also require to do a change >>> in clocksource driver because currently the first >>> timer in the DTS/system is taken for clocksource/clockevents >>> (others are just ignored). >>> That's why will be necessary to add clock-handle property >>> to cpu node to exactly describe which timer is system timer. >>> The same as is for interrupt-handle (more intc's can be in design). >> >> If there is just one set of registers, I wouldn't object to >> having just a single node in DT, and just one driver that registers >> to both the clocksource and the PWM interfaces. That might >> simplify the code. > > IP is configurable as is normal for us. > You can select IP with just one timer. > It means register locations for specific timer are fixed. > http://www.xilinx.com/support/documentation/ip_documentation/xps_timer.pdf > > timer0 - offset 0x0 > timer1 - offset 0x10 (doesn't need to be synthesized) > > There is one interrupt for both timers. > > Timers can be as timers (up/down count/ reload with or without IRQs) > But then one options is to use both timers and generate PWM signal. > From full ip description in DT you can see xlnx,gen0-assert = <1>; > which can suggest that this IP can output PMW signal. > (We can also detect if PWM0 signal is connected just to be sure > that PWM can be enabled). > > There is also capture trigger mode where external signal start/stop > timer counting. > > It means there are 3 modes - timer, capture and PWM. > Timer (clocksource, clockevent) requires specific handling, > PWM has own subsystem and not sure if there is any subsystem for > capture mode. Is there any? > > Not every timer configuration is suitable for all things > but fully configured timer can be used in all 3 modes. > > > I think that make sense to register and map all timers in the system > and classify them with flags (like can't be used for capture or PMW > if there are not outputs connected) and then use the best > timer for clocksource and clockevent. The best candidate is IP > with the lowest IRQ number in dual timer mode but in general > whatever timer can be used that's why we can't probably avoid > timer specification in DT. > In spite of this smells because you are saying via DT > to Linux which timer should be used for what purpose. > Can anyone help me with a correct example devicetree description of this block? As metioned above, this can be used as timer, pwm and capture device. Is a description as below a good starting point? axi_timer_0: timer@42800000 { clocks = <&clkc 15>; compatible = "xlnx,xps-timer-1.00.a"; interrupt-parent = <&xps_intc_0>; interrupts = < 3 2 >; reg = <0x42800000 0x10000>; xlnx,count-width = <0x20>; xlnx,gen0-assert = <0x1>; xlnx,gen1-assert = <0x1>; xlnx,one-timer-only = <0x0>; xlnx,trig0-assert = <0x1>; xlnx,trig1-assert = <0x1>; } ; pwm { compatible = "xlnx,timer-pwm"; #pwm-cells = <2>; timer = <&axi_timer_0>; }; clocksource { compatible = "xlnx,timer-clk"; timer = <&axi_timer_0>; }; IIOcapture { compatible = "xlnx,timer-IIO"; timer = <&axi_timer_0>; }; > Thanks, > Michal > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html