Path: csiph.com!news.mixmin.net!aioe.org!bofh.it!news.nic.it!robomod From: Rob Herring Newsgroups: linux.kernel Subject: Re: [PATCH v6 1/8] MFD: add bindings for STM32 Timers driver Date: Tue, 13 Dec 2016 22:10:02 +0100 Message-ID: References: X-Original-To: Benjamin Gaignard X-Gm-Message-State: AKaTC00FgBCmk81qBRFvLUxVUWJpMCE62rapsf9PU48oL1h8iGdXZH1NK4ftZ23M9SiP3V+lsEmUTE9/qjn1gA== X-Received: by 10.37.75.5 with SMTP id y5mr46674502yba.59.1481663255265; Tue, 13 Dec 2016 13:07:35 -0800 (PST) MIME-Version: 1.0 X-Gmail-Original-Message-ID: Content-Type: text/plain; charset=UTF-8 Sender: robomod@news.nic.it List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Approved: robomod@news.nic.it Lines: 101 Organization: linux.* mail to news gateway X-Original-Cc: Lee Jones , Mark Rutland , Alexandre Torgue , "devicetree@vger.kernel.org" , Linux Kernel Mailing List , Thierry Reding , Linux PWM List , Jonathan Cameron , Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald-Stadler , "linux-iio@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , Fabrice Gasnier , Gerald Baeza , Arnaud Pouliquen , Linus Walleij , Linaro Kernel Mailman List , Benjamin Gaignard X-Original-Date: Tue, 13 Dec 2016 15:07:14 -0600 X-Original-Message-ID: X-Original-References: <1481292919-26587-1-git-send-email-benjamin.gaignard@st.com> <1481292919-26587-2-git-send-email-benjamin.gaignard@st.com> <20161212185149.rt3xqpn3mbaavb4l@rob-hp-laptop> X-Original-Sender: linux-kernel-owner@vger.kernel.org Xref: csiph.com linux.kernel:1541526 On Tue, Dec 13, 2016 at 3:29 AM, Benjamin Gaignard wrote: > 2016-12-12 19:51 GMT+01:00 Rob Herring : >> On Fri, Dec 09, 2016 at 03:15:12PM +0100, Benjamin Gaignard wrote: >>> Add bindings information for STM32 Timers >>> >>> version 6: >>> - rename stm32-gtimer to stm32-timers >>> - change compatible >>> - add description about the IPs >>> >>> version 2: >>> - rename stm32-mfd-timer to stm32-gptimer >>> - only keep one compatible string >>> >>> Signed-off-by: Benjamin Gaignard >>> --- >>> .../devicetree/bindings/mfd/stm32-timers.txt | 46 ++++++++++++++++++++++ >>> 1 file changed, 46 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/mfd/stm32-timers.txt >>> >>> diff --git a/Documentation/devicetree/bindings/mfd/stm32-timers.txt b/Documentation/devicetree/bindings/mfd/stm32-timers.txt >>> new file mode 100644 >>> index 0000000..b30868e >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/mfd/stm32-timers.txt >>> @@ -0,0 +1,46 @@ >>> +STM32 Timers driver bindings >>> + >>> +This IP provides 3 types of timer along with PWM functionality: >>> +- advanced-control timers consist of a 16-bit auto-reload counter driven by a programmable >>> + prescaler, break input feature, PWM outputs and complementary PWM ouputs channels. >>> +- general-purpose timers consist of a 16-bit or 32-bit auto-reload counter driven by a >>> + programmable prescaler and PWM outputs. >>> +- basic timers consist of a 16-bit auto-reload counter driven by a programmable prescaler. >>> + >>> +Required parameters: >>> +- compatible: must be "st,stm32-timers" >>> + >>> +- reg: Physical base address and length of the controller's >>> + registers. >>> +- clock-names: Set to "clk_int". >> >> 'clk' is redundant. Also, you don't really need -names when there is >> only one of them. > > I use devm_regmap_init_mmio_clk() which get the clock by it name so > I have to define it in DT. Are you sure NULL is not allowed? I don't know, but at least clk_get() allows NULL. It's fine to keep, just drop the "clk_" part. > >>> +- clocks: Phandle to the clock used by the timer module. >>> + For Clk properties, please refer to ../clock/clock-bindings.txt >>> + >>> +Optional parameters: >>> +- resets: Phandle to the parent reset controller. >>> + See ../reset/st,stm32-rcc.txt >>> + >>> +Optional subnodes: >>> +- pwm: See ../pwm/pwm-stm32.txt >>> +- timer: See ../iio/timer/stm32-timer-trigger.txt >>> + >>> +Example: >>> + timers@40010000 { >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + compatible = "st,stm32-timers"; >>> + reg = <0x40010000 0x400>; >>> + clocks = <&rcc 0 160>; >>> + clock-names = "clk_int"; >>> + >>> + pwm { >>> + compatible = "st,stm32-pwm"; >>> + pinctrl-0 = <&pwm1_pins>; >>> + pinctrl-names = "default"; >>> + }; >>> + >>> + timer { >>> + compatible = "st,stm32-timer-trigger"; >>> + reg = <0>; >> >> You don't need reg here as there is only one. In turn, you don't need >> #address-cells or #size-cells. > > I use "reg" to set each timer configuration. > From hardware point of view they are all the same except for which hardware > signals they could consume and/or send. This sounds okay, but... > "reg" is used as index of the two tables in driver code. this statement doesn't really sound like valid use of reg. If you keep reg, then the node needs a unit address (timer@0). Rob