On 5/13/21 10:33 AM, Sean Anderson wrote: > > > On 5/12/21 10:16 PM, Rob Herring wrote: > > On Tue, May 11, 2021 at 03:12:37PM -0400, Sean Anderson wrote: > >> This adds a binding for the Xilinx LogiCORE IP AXI Timer. This device is > >> a "soft" block, so it has many parameters which would not be > >> configurable in most hardware. This binding is usually automatically > >> generated by Xilinx's tools, so the names and values of some properties > >> must be kept as they are. Replacement properties have been provided for > >> new device trees. > > > > Because you have some tool generating properties is not a reason we have > > to accept them upstream. > > These properties are already in arch/microblaze/boot/dts/system.dts and > in the devicetree supplied to Linux by qemu. Removing these properties > will break existing setups, which I would like to avoid. > > > 'deprecated' is for what *we* have deprecated. > > Ok. I will remove that then. > > > > > In this case, I don't really see the point in defining new properties > > just to have bool. > > I don't either, but it was requested, by Michal... Err, your comment on the original bindings was > Can't all these be boolean? And Michal commented > I think in this case you should described what it is used by current > driver in Microblaze and these options are required. The rest are by > design optional. > If you want to change them to different value then current binding > should be deprecated and have any transition time with code alignment. So that is what I tried to accomplish with this revision. I also tried allowing something like xlnx,one-timer-only = <0>; /* two timers */ xlnx,one-timer-only = <1>; /* one timer */ xlnx,one-timer-only; /* one timer */ /* property absent means two timers */ but I was unable to figure out how to express this with json-schema. I don't think it's the best design either... --Sean > > > > >> > >> Because we need to init timer devices so early in boot, the easiest way > >> to configure things is to use a device tree property. For the moment > >> this is 'xlnx,pwm', but this could be extended/renamed/etc. in the > >> future if these is a need for a generic property. > > > > No... > > > >> Signed-off-by: Sean Anderson <sean.anderson@xxxxxxxx> > >> --- > >> How should the clocking situation be documented? For the moment I have > >> just left clock as optional, but should clock-frequency be documented? > >> > >> Changes in v3: > >> - Mark all boolean-as-int properties as deprecated > >> - Add xlnx,pwm and xlnx,gen?-active-low properties. > >> - Make newer replacement properties mutually-exclusive with what they > >> replace > >> - Add an example with non-deprecated properties only. > >> > >> Changes in v2: > >> - Use 32-bit addresses for example binding > >> > >> .../bindings/pwm/xlnx,axi-timer.yaml | 142 ++++++++++++++++++ > >> 1 file changed, 142 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/pwm/xlnx,axi-timer.yaml > >> > >> diff --git a/Documentation/devicetree/bindings/pwm/xlnx,axi-timer.yaml b/Documentation/devicetree/bindings/pwm/xlnx,axi-timer.yaml > >> new file mode 100644 > >> index 000000000000..a5e90658e31a > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/pwm/xlnx,axi-timer.yaml > >> @@ -0,0 +1,142 @@ > >> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause > >> +%YAML 1.2 > >> +--- > >> +$id: http://devicetree.org/schemas/pwm/xlnx,axi-timer.yaml# > >> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >> + > >> +title: Xilinx LogiCORE IP AXI Timer Device Tree Binding > >> + > >> +maintainers: > >> + - Sean Anderson <sean.anderson@xxxxxxxx> > >> + > >> +properties: > >> + compatible: > >> + oneOf: > >> + - items: > >> + - const: xlnx,axi-timer-2.0 > >> + - const: xlnx,xps-timer-1.00.a > >> + - const: xlnx,xps-timer-1.00.a > >> + > >> + clocks: > >> + maxItems: 1 > >> + > >> + clock-names: > >> + const: s_axi_aclk > >> + > >> + reg: > >> + maxItems: 1 > >> + > >> + xlnx,count-width: > >> + $ref: /schemas/types.yaml#/definitions/uint32 > >> + minimum: 8 > >> + maximum: 32 > >> + description: > >> + The width of the counter(s), in bits. > >> + > >> + xlnx,gen0-assert: > >> + $ref: /schemas/types.yaml#/definitions/uint32 > >> + enum: [ 0, 1 ] > >> + default: 1 > >> + deprecated: true > >> + description: > >> + The polarity of the generateout0 signal. 0 for active-low, 1 for active-high. > >> + > >> + xlnx,gen0-active-low: > >> + $ref: /schemas/types.yaml#/definitions/flag > >> + description: > >> + The generate0 signal is active-low. > >> + > >> + xlnx,gen1-assert: > >> + $ref: /schemas/types.yaml#/definitions/uint32 > >> + enum: [ 0, 1 ] > >> + default: 1 > >> + deprecated: true > >> + description: > >> + The polarity of the generateout1 signal. 0 for active-low, 1 for active-high. > >> + > >> + xlnx,gen1-active-low: > >> + $ref: /schemas/types.yaml#/definitions/flag > >> + description: > >> + The generate1 signal is active-low. > >> + > >> + xlnx,one-timer-only: > >> + $ref: /schemas/types.yaml#/definitions/uint32 > >> + enum: [ 0, 1 ] > >> + deprecated: true > >> + description: > >> + Whether only one timer is present in this block. > >> + > >> + xlnx,single-timer: > >> + $ref: /schemas/types.yaml#/definitions/flag > >> + description: > >> + Only one timer is present in this block. > >> + > >> + xlnx,pwm: > >> + $ref: /schemas/types.yaml#/definitions/flag > >> + description: > >> + This timer should be configured as a PWM. > > > > If a PWM, perhaps you want a '#pwm-cells' property which can serve as > > the hint to configure as a PWM. > > Ok, that's a good idea. > > --Sean > > > > >> + > >> +required: > >> + - compatible > >> + - reg > >> + - xlnx,count-width > >> + > >> +allOf: > >> + - if: > >> + required: > >> + - clocks > >> + then: > >> + required: > >> + - clock-names > >> + > >> + - if: > >> + required: > >> + - xlnx,gen0-active-low > >> + then: > >> + not: > >> + required: > >> + - xlnx,gen0-assert > >> + > >> + - if: > >> + required: > >> + - xlnx,gen0-active-low > >> + then: > >> + not: > >> + required: > >> + - xlnx,gen0-assert > >> + > >> + - if: > >> + required: > >> + - xlnx,one-timer-only > >> + then: > >> + not: > >> + required: > >> + - xlnx,single-timer > >> + > >> +additionalProperties: true > >> + > >> +examples: > >> + - | > >> + axi_timer_0: timer@800e0000 { > >> + clock-names = "s_axi_aclk"; > >> + clocks = <&zynqmp_clk 71>; > >> + compatible = "xlnx,axi-timer-2.0", "xlnx,xps-timer-1.00.a"; > >> + reg = <0x800e0000 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>; > >> + }; > >> + > >> + - | > >> + axi_timer_0: timer@800e0000 { > >> + clock-names = "s_axi_aclk"; > >> + clocks = <&zynqmp_clk 71>; > >> + compatible = "xlnx,axi-timer-2.0", "xlnx,xps-timer-1.00.a"; > >> + reg = <0x800e0000 0x10000>; > >> + xlnx,count-width = <0x20>; > >> + xlnx,gen0-active-low; > >> + xlnx,single-timer; > >> + }; > >> -- > >> 2.25.1 > >>
