<div dir="ltr">Hi Rob,<div><br></div><div><br></div><div class="gmail_extra"><div class="gmail_quote">On 25 June 2018 at 20:14, Rob Herring <span dir="ltr"><<a href="mailto:robh@kernel.org" target="_blank">robh@kernel.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Sun, Jun 24, 2018 at 03:41:54PM +0300, Tomer Maimon wrote:<br>
> Added device tree binding documentation for Nuvoton BMC<br>
> NPCM7xx Pulse Width Modulation (PWM) and Fan tach controller.<br>
> The PWM controller can support upto 8 PWM output ports.<br>
> The Fan tach controller can support upto 16 tachometer inputs.<br>
> <br>
> Signed-off-by: Tomer Maimon <<a href="mailto:tmaimon77@gmail.com">tmaimon77@gmail.com</a>><br>
> ---<br>
> .../devicetree/bindings/hwmon/<wbr>npcm750-pwm-fan.txt | 84 ++++++++++++++++++++++<br>
> 1 file changed, 84 insertions(+)<br>
> create mode 100644 Documentation/devicetree/<wbr>bindings/hwmon/npcm750-pwm-<wbr>fan.txt<br>
> <br>
> diff --git a/Documentation/devicetree/<wbr>bindings/hwmon/npcm750-pwm-<wbr>fan.txt b/Documentation/devicetree/<wbr>bindings/hwmon/npcm750-pwm-<wbr>fan.txt<br>
> new file mode 100644<br>
> index 000000000000..a9eacda34f92<br>
> --- /dev/null<br>
> +++ b/Documentation/devicetree/<wbr>bindings/hwmon/npcm750-pwm-<wbr>fan.txt<br>
> @@ -0,0 +1,84 @@<br>
> +Nuvoton NPCM7xx PWM and Fan Tacho controller device driver<br>
<br>
</span>Bindings are for h/w, not drivers.<br>
<span class=""><br>
> +<br>
> +The NPCM7xx has two identical Pulse-width modulation (PWM) controller modules,<br>
> +Each PWM module has four PWM controller outputs, Totally 8 PWM controller outputs.<br>
> +<br>
> +The NPCM7xx has eight identical Fan tachometer controller modules,<br>
> +Each Fan module has two Fan controller inputs, Totally 16 Fan controller inputs.<br>
<br>
</span>Have you looked at other fan ctrlr bindings?f This looks like similar <br>
h/w to ASpeed. Really, I'd like to see a common doc that describes the <br></blockquote><div> </div><div>
<div style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial">We do not have the same H/W as Aspeed, I believe in the near future we will need to add </div><div style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial">more DT properties that will used only in the NPCM7xx module. </div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
structure and common properties.<br></blockquote><div><br></div>
<span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">what do you mean by common </span><span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">structure and common properties?</span>
<div><span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br></span></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
> +<br>
> +Required properties for pwm-fan node:<br>
> +- compatible : "nuvoton,npcm750-pwm-fan" for Poleg NPCM7XX.<br>
> +- reg : specifies physical base address and size of the registers.<br>
> +- reg-names : must contain:<br>
> + * "pwm-base" for the PWM registers.<br>
> + * "fan-base" for the Fan registers.<br>
<br>
</span>'-base' is redundant. And your example doesn't match.<br>
<div><div class="h5"><br>
> +- clocks : phandle of reference clocks.<br>
> +- clock-names : must contain<br>
> + * "clk_apb3" for clock of PWM controller.<br>
> + * "clk_apb4" for clock of Fan controller.<br>
> +- interrupts : contain the Fan interrupts with flags for falling edge.<br>
> +- pinctrl-names : a pinctrl state named "default" must be defined.<br>
> +- pinctrl-0 : phandle referencing pin configuration of the PWM and Fan<br>
> + controller ports.<br>
> +<br>
> +pwm subnode format:<br>
> +===================<br>
> +Under pwm subnode can be upto 8 child nodes, each child node representing a PWM channel.<br>
> +Each pwm subnode must have one PWM channel and atleast one Fan tach channel.<br>
> +<br>
> +For PWM channel can be configured cooling-levels to create cooling device.<br>
> +Cooling device could be bound to a thermal zone for the thermal control.<br>
> +<br>
> +Required properties for each child node:<br>
> +- pwm-ch : specify the PWM output channel.<br>
> + integer value in the range 0 through 7, that represent<br>
> + the PWM channel number that used.<br>
> +<br>
> +- fan-ch : specify the Fan input channel.<br>
> + integer value in the range 0 through 15, that represent<br>
> + the Fan channel number that used.<br>
<br>
</div></div>This is really the tach channel.<br>
<span class=""><br>
<br>
> +<br>
> + At least one Fan tach input channel is required<br>
> +<br>
> +Optional property for each child node:<br>
> +- cooling-levels: PWM duty cycle values in a range from 0 to 255<br>
> + which correspond to thermal cooling states.<br>
> +<br>
> +Examples:<br>
> +<br>
> +pwm_fan:pwm_fan_controller@<wbr>103000 {<br>
<br>
</span>fan-controller@...<br>
<span class=""><br>
> + compatible = "nuvoton,npcm750-pwm-fan";<br>
> + reg = <0x103000 0x2000>,<br>
> + <0x180000 0x8000>;<br>
> + reg-names = "pwm_base", "fan_base";<br>
> + clocks = <&clk NPCM7XX_CLK_APB3>,<br>
> + <&clk NPCM7XX_CLK_APB4>;<br>
> + clock-names = "clk_apb3","clk_apb4";<br>
> + interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>,<br>
> + <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>,<br>
> + <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>,<br>
> + <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>,<br>
> + <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,<br>
> + <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>,<br>
> + <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,<br>
> + <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;<br>
> + pinctrl-names = "default";<br>
> + pinctrl-0 = <&pwm0_pins &pwm1_pins &pwm2_pins<br>
> + &fanin0_pins &fanin1_pins &fanin2_pins<br>
> + &fanin3_pins &fanin4_pins>;<br>
> +<br>
> + pwm@0 {<br>
<br>
</span>The sub-nodes should be "fan" nodes.<br>
<br>
A unit-address without reg property is not valid.<br>
<span class=""><br>
> + pwm-ch = /bits/ 8 <0x00>;<br>
<br>
</span>Use 'reg' like ASpeed binding. <br>
<span class=""><br>
> + fan-ch = /bits/ 8 <0x00 0x01>;<br>
> + cooling-levels = <127 255>;<br>
> + };<br>
> + pwm@1 {<br>
> + pwm-ch = /bits/ 8 <0x01>;<br>
> + fan-ch = /bits/ 8 <0x02 0x03>;<br>
> + };<br>
> + pwm@2 {<br>
> + pwm-ch = /bits/ 8 <0x02>;<br>
> + fan-ch = /bits/ 8 <0x04>;<br>
> + };<br>
> +<br>
> +};<br>
> \ No newline at end of file<br>
<br>
</span>^^^<br>
<span class="HOEnZb"><font color="#888888"><br>
> -- <br>
> 2.14.1<br>
> <br>
> --<br>
> To unsubscribe from this list: send the line "unsubscribe devicetree" in<br>
> the body of a message to <a href="mailto:majordomo@vger.kernel.org">majordomo@vger.kernel.org</a><br>
> More majordomo info at <a href="http://vger.kernel.org/majordomo-info.html" rel="noreferrer" target="_blank">http://vger.kernel.org/<wbr>majordomo-info.html</a></font></span></blockquote><div><br></div><div>Thanks!</div><div><br></div><div>Tomer </div></div><br></div></div>