<div dir="ltr">Hi Rob,<div class="gmail_extra"><br><div class="gmail_quote">On 26 June 2018 at 18:03, 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"><div class="HOEnZb"><div class="h5">On Mon, Jun 25, 2018 at 4:20 PM Tomer Maimon <<a href="mailto:tmaimon77@gmail.com">tmaimon77@gmail.com</a>> wrote:<br>
><br>
> Hi Rob,<br>
><br>
><br>
> On 25 June 2018 at 20:14, Rob Herring <<a href="mailto:robh@kernel.org">robh@kernel.org</a>> wrote:<br>
>><br>
>> 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>
>> Bindings are for h/w, not drivers.<br>
>><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>
>> 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>
><br>
><br>
> We do not have the same H/W as Aspeed, I believe in the near future we will need to add<br>
> more DT properties that will used only in the NPCM7xx module.<br>
<br>
</div></div>I didn't say it was the same. Both are multi-channel PWMs with tach<br>
inputs. Presumably, they can attach to the same types of fans as there<br>
are only a limited number of types of fans and none of them are<br>
specific to any fan controller.<br>
<span class=""><br>
>> structure and common properties.<br>
><br>
><br>
> what do you mean by common structure and common properties?<br>
<br>
</span>When we have multiple bindings for the same class of device/hw, we<br>
define all the common parts in a common binding doc. This often<br>
doesn't happen at first, so we end up with a variety of bindings until<br>
we see some commonality.<br>
<br>
In this case, for structure, having sub-nodes for fans. As fans are<br>
not specific to the controllers, their node should not be defined by<br>
the controller binding. It's also how you describe the fan type,<br>
number of fans, the PWM connections, the tach connections, etc.<br>
<span class="HOEnZb"><font color="#888888"><br>
Rob<br></font></span></blockquote><div><br></div><div>Thanks a lot for the clarification, indeed we can do a common binding doc.</div><div>is it O.K. for now to stay with the same binding document (V5 - that  i will send toady),<br></div><div>and later on we will work on creating <span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">common binding document for the fan-nodes.</span></div><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><div>also I will like to know how to represent the <span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">fan-nodes properties in the <span style="text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">common binding document,</span></span></div><div>the property name "<font color="#000000"><span style="font-family:"Ubuntu Mono",monospace;font-size:0.9em;white-space:pre">aspeed,fan-tach-ch" </span><font face="arial, helvetica, sans-serif"><span style="white-space:pre">is </span><span style="white-space:pre">something </span><span style="white-space:pre">related Aspeed.</span></font></font></div><div><font color="#000000" face="Ubuntu Mono, monospace"><span style="font-size:0.9em;white-space:pre"><br></span></font></div><div><font color="#000000"><font face="arial, helvetica, sans-serif"><span style="white-space:pre">Do I need to ask the Aspeed hwmon maintainer to modify his driver to use  </span></font><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="font-family:"Ubuntu Mono",monospace;font-size:11.7px;white-space:pre">fan-tach-ch </span><span style="white-space:pre"><font face="arial, helvetica, sans-serif">property?</font></span></span><font face="arial, helvetica, sans-serif"> </font></font></div><div><font color="#000000"><span style="white-space:pre"><font face="arial, helvetica, sans-serif">or to mention both fan-tach</font><font face="Ubuntu Mono, monospace"><span style="font-size:11.7px"> </span></font></span><span style="font-family:"Ubuntu Mono",monospace;font-size:0.9em;white-space:pre"><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;white-space:normal;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">properties<span> (NPCM and Aspeed) in the 

<span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">common binding document</span> ?</span></span></span></font></div><div><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;white-space:normal;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span></span></span>

</div><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><div><span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">Thanks,</span></div><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><div><span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">Tomer</span></div><div><span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="font-size:12.8px;text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span> </span></span>

</span></div></div><br></div></div>