From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.3 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E1854C11F6C for ; Wed, 30 Jun 2021 13:47:25 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C8DDE61446 for ; Wed, 30 Jun 2021 13:47:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235220AbhF3Ntx (ORCPT ); Wed, 30 Jun 2021 09:49:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45518 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235178AbhF3Ntu (ORCPT ); Wed, 30 Jun 2021 09:49:50 -0400 Received: from mail-ej1-x62a.google.com (mail-ej1-x62a.google.com [IPv6:2a00:1450:4864:20::62a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8CAB5C061787 for ; Wed, 30 Jun 2021 06:47:20 -0700 (PDT) Received: by mail-ej1-x62a.google.com with SMTP id nd37so4342191ejc.3 for ; Wed, 30 Jun 2021 06:47:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monstr-eu.20150623.gappssmtp.com; s=20150623; h=from:subject:to:cc:references:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=NiFfnM4nxPKv3wBf/KQmGM6lG+GXst7adkovnLuCU8U=; b=wQ/r+hCBG+g9+ksz+k4ZRvsyRx0gxoQDcA+TuwHe8mxhBDxQ0rPrEFZGazh7JncXtc yYVWG8d6O7IOUocV6gfxaHea7kuewGWzhuXl2fmW4lF/KJTUcwvQplKndHLHkpEdlKR2 wtlPBg2u9Hlz4+xy1m3THQwLwcxAVllSBERvcXscx/4vwbUUawRuM29w63EaITUiEeGp ibVGzMn9gY9Q3VAGLnNqg4IX0YHOI0eqya93IxLsJvxaicqDAkDynqW4fBF6XRGAmaOy cF2tQYSrK3uJ4wfpJfXUGToIJbTQxUJJBe/RMkm66nE3cVv92G6bbEdxyqvB8unwLi2d FhnQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:subject:to:cc:references:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=NiFfnM4nxPKv3wBf/KQmGM6lG+GXst7adkovnLuCU8U=; b=i3n8PyCrYkNP1JCDAgKVwmr+f/gz804RPmuaXD3QUj6XHqFOiQLJlQBhuDrS+NXch6 sbzOf++Wj5sW4/ttMc7ZAenbC+RuDNiq6QsLxR6EiiFNDVmeWfq4KtHztXZr5aduzscB JGIe3ldcz8T3bbTfbO5L8gHQFst6OltBayJKDGjC2XUQ0yyi3dIzJehEbvs+HGHVq6tn zd810HI8+oGQeOpApyMXWwgCIVvpx2ox1/2sgCfjOU1yiT/76OqV4GPJRD3rp4JwTanX dD9BfRM5h+CmKKiT9YpOqL2GsqPgXGCRt7CfTrEXWcMpmTDUdlOug0lE4Yi5uhCo5LaY jFNA== X-Gm-Message-State: AOAM532gmrtIFmEsHefMIL4lIDVFSohoPre8Afh4EyEi2B27OOhcgARj ncvSeiQ/zRVnnwVeOf9NFfjnSQ== X-Google-Smtp-Source: ABdhPJxsqeSQfJuyngC91sLJZ+6xnm0UeG2PQ+/LMc3+fz5GGaWtZwgPqPtgtUI3SwnHPFIXlNQ59g== X-Received: by 2002:a17:906:c010:: with SMTP id e16mr35364761ejz.214.1625060839125; Wed, 30 Jun 2021 06:47:19 -0700 (PDT) Received: from ?IPv6:2a02:768:2307:40d6::648? ([2a02:768:2307:40d6::648]) by smtp.gmail.com with ESMTPSA id s5sm12876834edi.93.2021.06.30.06.47.18 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 30 Jun 2021 06:47:18 -0700 (PDT) From: Michal Simek Subject: Re: [PATCH v4 1/3] dt-bindings: pwm: Add Xilinx AXI Timer To: Sean Anderson , linux-pwm@vger.kernel.org, devicetree@vger.kernel.org, =?UTF-8?Q?Uwe_Kleine-K=c3=b6nig?= , Sascha Hauer Cc: michal.simek@xilinx.com, linux-kernel@vger.kernel.org, Alvaro Gamez , linux-arm-kernel@lists.infradead.org, Rob Herring References: <20210528214522.617435-1-sean.anderson@seco.com> Message-ID: <13c9345f-b3e5-cc97-437b-c342777fcf3c@monstr.eu> Date: Wed, 30 Jun 2021 15:47:18 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <20210528214522.617435-1-sean.anderson@seco.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On 5/28/21 11:45 PM, 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 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. > > Signed-off-by: Sean Anderson > --- > > Changes in v4: > - Remove references to generate polarity so this can get merged > - Predicate PWM driver on the presence of #pwm-cells > - Make some properties optional for clocksource drivers > > 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 | 85 +++++++++++++++++++ > 1 file changed, 85 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..48a280f96e63 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pwm/xlnx,axi-timer.yaml I don't think this is the right location for this. I have done some grepping and I think this should be done in a different way. I pretty much like solution around "ti,omap3430-timer" which is calling dmtimer_systimer_select_best() and later dmtimer_is_preferred() which in this case would allow us to get rid of cases which are not suitable for clocksource and clockevent. And there is drivers/pwm/pwm-omap-dmtimer.c which has link to timer which is providing functions for it's functionality. I have also looked at Documentation/devicetree/bindings/timer/nxp,tpm-timer.yaml which is also the same device. And sort of curious if you look at https://www.xilinx.com/support/documentation/ip_documentation/axi_timer/v2_0/pg079-axi-timer.pdf ( Figure 1-1) that PWM is taking input from generate out 0 and generate out 1 which is maybe can be modeled is any output and pwm driver can register inputs for pwm driver. > @@ -0,0 +1,85 @@ > +# 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 > + > +properties: > + compatible: > + oneOf: > + - items: > + - const: xlnx,axi-timer-2.0 > + - const: xlnx,xps-timer-1.00.a > + - items: > + - const: xlnx,xps-timer-1.00.a > + > + clocks: > + maxItems: 1 > + > + clock-names: > + const: s_axi_aclk Origin driver is not using this clock name and it is only one that's why it shouldn't be listed. > + > + interrupts: > + maxItems: 1 > + > + reg: > + maxItems: 1 > + > + xlnx,count-width: > + $ref: /schemas/types.yaml#/definitions/uint32 > + minimum: 8 > + maximum: 32 > + default: 32 This is not accurate. It should be enum because only 8/16/32 are valid values here. > + description: > + The width of the counter(s), in bits. > + > + xlnx,one-timer-only: > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [ 0, 1 ] > + description: > + Whether only one timer is present in this block. > + > +required: > + - compatible > + - reg > + - xlnx,one-timer-only > + > +allOf: > + - if: > + required: > + - '#pwm-cells' Let's discussed this usage based on design. > + then: > + allOf: > + - required: > + - clocks > + - properties: > + xlnx,one-timer-only: > + const: 0 > + else: > + required: > + - interrupts > + - if: > + required: > + - clocks > + then: > + required: > + - clock-names And this checking should be removed too. > + > +additionalProperties: true > + > +examples: > + - | > + axi_timer_0: timer@800e0000 { label is useless here and should be removed. > + #pwm-cells = <0>; > + 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,one-timer-only = <0x0>; > + }; > I would list example without pwm-cells first as it is valid and reflect current status. Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Xilinx Microblaze Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs