Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 10 Dec 2020 21:26:12 -0600
From:   Rob Herring <robh@kernel.org>
To:     Troy Lee <troy_lee@aspeedtech.com>
Cc:     openbmc@lists.ozlabs.org, Jean Delvare <jdelvare@suse.com>,
        Guenter Roeck <linux@roeck-us.net>,
        Joel Stanley <joel@jms.id.au>,
        Andrew Jeffery <andrew@aj.id.au>,
        Jonathan Corbet <corbet@lwn.net>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        "open list:HARDWARE MONITORING" <linux-hwmon@vger.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>,
        "moderated list:ARM/ASPEED MACHINE SUPPORT" 
        <linux-arm-kernel@lists.infradead.org>,
        "moderated list:ARM/ASPEED MACHINE SUPPORT" 
        <linux-aspeed@lists.ozlabs.org>,
        open list <linux-kernel@vger.kernel.org>,
        "open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
        leetroy@gmail.com, ryan_chen@aspeedtech.com,
        chiawei_wang@aspeedtech.com, billy_tsai@aspeedtech.com
Subject: Re: [PATCH 1/4] dt-bindings: hwmon: Add Aspeed AST2600 PWM/Fan
Message-ID: <20201211032612.GA3565720@robh.at.kernel.org>
References: <20201209075921.26689-1-troy_lee@aspeedtech.com>
 <20201209075921.26689-2-troy_lee@aspeedtech.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201209075921.26689-2-troy_lee@aspeedtech.com>
Precedence: bulk

On Wed, Dec 09, 2020 at 03:59:17PM +0800, Troy Lee wrote:
> For supporting a new AST2600 PWM/Fan hwmon driver, we add a new binding.
> 
> Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
> ---
>  .../bindings/hwmon/aspeed2600-pwm-tacho.txt   | 69 +++++++++++++++++++

Bindings are in DT schema format now.

>  1 file changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed2600-pwm-tacho.txt
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/aspeed2600-pwm-tacho.txt b/Documentation/devicetree/bindings/hwmon/aspeed2600-pwm-tacho.txt
> new file mode 100644
> index 000000000000..61b11914352f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/aspeed2600-pwm-tacho.txt
> @@ -0,0 +1,69 @@
> +ASPEED AST2600 PWM and Fan Tacho controller device driver
> +
> +The ASPEED PWM controller can support upto 16 PWM outputs. The ASPEED Fan Tacho
> +controller can support upto 16 Fan tachometer inputs.
> +
> +There can be upto 16 fans supported. Each fan can have one PWM output and
> +one Fan tach inputs.
> +
> +Required properties for pwm-tacho node:
> +- #address-cells : should be 1.
> +
> +- #size-cells : should be 0.
> +
> +- #cooling-cells: should be 2.
> +
> +- reg : address and length of the register set for the device.
> +
> +- pinctrl-names : a pinctrl state named "default" must be defined.
> +
> +- pinctrl-0 : phandle referencing pin configuration of the PWM ports.
> +
> +- compatible : should be "aspeed,ast2600-pwm-tachometer".
> +
> +- clocks : phandle to clock provider with the clock number in the second cell
> +
> +- resets : phandle to reset controller with the reset number in the second cell
> +
> +fan subnode format:
> +===================
> +Under fan subnode there can upto 16 child nodes, with each child node
> +representing a fan. There are 16 fans each fan can have one PWM port and one
> +Fan tach inputs.
> +For PWM port can be configured cooling-levels to create cooling device.
> +Cooling device could be bound to a thermal zone for the thermal control.
> +
> +Required properties for each child node:
> +- reg : should specify PWM source port.
> +	integer value in the range 0x00 to 0x0f with 0x00 indicating PWM port 0
> +	and 0x0f indicating PWM port F.
> +
> +- cooling-levels: PWM duty cycle values in a range from 0 to 255
> +                  which correspond to thermal cooling states.
> +
> +- aspeed,fan-tach-ch : should specify the Fan tach input channel.
> +                integer value in the range 0 through 15, with 0 indicating
> +		Fan tach channel 0 and 15 indicating Fan tach channel 15.
> +		Atleast one Fan tach input channel is required.

Already has 'fan-tach-ch' in npcm750-pwm-fan.txt.

> +
> +- aspeed,target-pwm : Specify the frequency of PWM. The value range from 24 to
> +		      780000. Default value will be set to 25000.
> +
> +- aspeed,pulse-pr : Specify tacho pulse per revolution of the fan. A general
> +		    parameter of pulse-pr is 2.

Already have 'pulses-per-revolution' property in pwm-fan.txt. Use that.

Really, all these should be in a common fan schema that you reference.

> +
> +Examples:
> +
> +&pwm_tacho {
> +	status = "okay";

Don't show status in examples.

> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_tach0_default>;
> +
> +	fan@0 {
> +		reg = <0x00>;
> +		aspeed,target-pwm = <25000>;
> +		cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
> +		aspeed,fan-tach-ch = /bits/ 8 <0x00>;
> +		aspeed,pulse-pr = <2>;
> +	};
> +};
> -- 
> 2.17.1
>